From a354b54a086477496bff485a7ea55b3570bac912 Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Wed, 3 Apr 2024 18:53:56 +0300 Subject: [PATCH 1/4] server-ts: Fix getContent in updateNoteData --- src/becca/entities/rows.ts | 2 +- src/services/notes.ts | 12 +++++------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/becca/entities/rows.ts b/src/becca/entities/rows.ts index 4f2c089bd..4f2113f2e 100644 --- a/src/becca/entities/rows.ts +++ b/src/becca/entities/rows.ts @@ -5,7 +5,7 @@ export interface AttachmentRow { ownerId?: string; role: string; mime: string; - title?: string; + title: string; position?: number; blobId?: string; isProtected?: boolean; diff --git a/src/services/notes.ts b/src/services/notes.ts index 6449081eb..d6efbf31b 100644 --- a/src/services/notes.ts +++ b/src/services/notes.ts @@ -744,7 +744,7 @@ function saveRevisionIfNeeded(note: BNote) { } } -function updateNoteData(noteId: string, content: string, attachments: BAttachment[] = []) { +function updateNoteData(noteId: string, content: string, attachments: AttachmentRow[] = []) { const note = becca.getNote(noteId); if (!note || !note.isContentAvailable()) { @@ -760,11 +760,7 @@ function updateNoteData(noteId: string, content: string, attachments: BAttachmen if (attachments?.length > 0) { const existingAttachmentsByTitle = utils.toMap(note.getAttachments({includeContentLength: false}), 'title'); - for (const attachment of attachments) { - // TODO: The content property was extracted directly instead of `getContent`. To investigate. - const {attachmentId, role, mime, title, position} = attachment; - const content = attachment.getContent(); - + for (const {attachmentId, role, mime, title, position, content} of attachments) { if (attachmentId || !(title in existingAttachmentsByTitle)) { note.saveAttachment({attachmentId, role, mime, title, content, position}); } else { @@ -772,7 +768,9 @@ function updateNoteData(noteId: string, content: string, attachments: BAttachmen existingAttachment.role = role; existingAttachment.mime = mime; existingAttachment.position = position; - existingAttachment.setContent(content, {forceSave: true}); + if (content) { + existingAttachment.setContent(content, {forceSave: true}); + } } } } From f857b8a9bb9d86840ff4fda9afdbd9c882df3dbe Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Wed, 3 Apr 2024 19:05:10 +0300 Subject: [PATCH 2/4] server-ts: Refactor out abstract init in entities --- src/becca/entities/abstract_becca_entity.ts | 4 +++- src/becca/entities/battachment.ts | 4 ---- src/becca/entities/bblob.ts | 4 ---- src/becca/entities/boption.ts | 4 ---- src/becca/entities/brecent_note.ts | 4 ---- src/becca/entities/brevision.ts | 4 ---- 6 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/becca/entities/abstract_becca_entity.ts b/src/becca/entities/abstract_becca_entity.ts index 0edcd04ea..00bf78e80 100644 --- a/src/becca/entities/abstract_becca_entity.ts +++ b/src/becca/entities/abstract_becca_entity.ts @@ -92,7 +92,9 @@ abstract class AbstractBeccaEntity> { abstract getPojo(): {}; - abstract init(): void; + init() { + // Do nothing by default, can be overriden in derived classes. + } abstract updateFromRow(row: unknown): void; diff --git a/src/becca/entities/battachment.ts b/src/becca/entities/battachment.ts index f70cff9e2..7b203839a 100644 --- a/src/becca/entities/battachment.ts +++ b/src/becca/entities/battachment.ts @@ -83,10 +83,6 @@ class BAttachment extends AbstractBeccaEntity { this.contentLength = row.contentLength; } - init(): void { - // Do nothing. - } - copy(): BAttachment { return new BAttachment({ ownerId: this.ownerId, diff --git a/src/becca/entities/bblob.ts b/src/becca/entities/bblob.ts index cb2301a5b..40d1c5885 100644 --- a/src/becca/entities/bblob.ts +++ b/src/becca/entities/bblob.ts @@ -26,10 +26,6 @@ class BBlob extends AbstractBeccaEntity { this.utcDateModified = row.utcDateModified; } - init() { - // Nothing to do. - } - getPojo() { return { blobId: this.blobId, diff --git a/src/becca/entities/boption.ts b/src/becca/entities/boption.ts index b06a23431..48abee024 100644 --- a/src/becca/entities/boption.ts +++ b/src/becca/entities/boption.ts @@ -32,10 +32,6 @@ class BOption extends AbstractBeccaEntity { this.utcDateModified = row.utcDateModified; } - init(): void { - // Do nothing. - } - beforeSaving() { super.beforeSaving(); diff --git a/src/becca/entities/brecent_note.ts b/src/becca/entities/brecent_note.ts index da94feaa9..c19a83603 100644 --- a/src/becca/entities/brecent_note.ts +++ b/src/becca/entities/brecent_note.ts @@ -29,10 +29,6 @@ class BRecentNote extends AbstractBeccaEntity { this.utcDateCreated = row.utcDateCreated || dateUtils.utcNowDateTime(); } - init(): void { - // Do nothing. - } - getPojo() { return { noteId: this.noteId, diff --git a/src/becca/entities/brevision.ts b/src/becca/entities/brevision.ts index 48d131f0a..101506858 100644 --- a/src/becca/entities/brevision.ts +++ b/src/becca/entities/brevision.ts @@ -68,10 +68,6 @@ class BRevision extends AbstractBeccaEntity { this.contentLength = row.contentLength; } - init() { - // Do nothing. - } - getNote() { return becca.notes[this.noteId]; } From 5d452a15258a0ec512c56cbe69dfac1b8db03db8 Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Wed, 3 Apr 2024 19:22:49 +0300 Subject: [PATCH 3/4] server-ts: Address review --- src/becca/entities/abstract_becca_entity.ts | 2 +- src/becca/similarity.ts | 2 +- src/services/auth.ts | 26 ++++++++++----------- src/services/date_utils.ts | 2 +- src/services/etapi_tokens.ts | 4 ++-- src/services/keyboard_actions.ts | 2 +- src/services/notes.ts | 2 +- src/services/sql.ts | 2 +- 8 files changed, 20 insertions(+), 22 deletions(-) diff --git a/src/becca/entities/abstract_becca_entity.ts b/src/becca/entities/abstract_becca_entity.ts index 00bf78e80..ed9901b2f 100644 --- a/src/becca/entities/abstract_becca_entity.ts +++ b/src/becca/entities/abstract_becca_entity.ts @@ -11,7 +11,7 @@ import protectedSessionService = require('../../services/protected_session'); import blobService = require('../../services/blob'); import Becca, { ConstructorData } from '../becca-interface'; -let becca: Becca | null = null; +let becca: Becca; interface ContentOpts { forceSave?: boolean; diff --git a/src/becca/similarity.ts b/src/becca/similarity.ts index 600e6dc2b..e6721d0df 100644 --- a/src/becca/similarity.ts +++ b/src/becca/similarity.ts @@ -153,7 +153,7 @@ function buildRewardMap(note: BNote) { const mimeCache: Record = {}; -function trimMime(mime?: string) { +function trimMime(mime: string) { if (!mime || mime === 'text/html') { return; } diff --git a/src/services/auth.ts b/src/services/auth.ts index 30f5687f3..0e96f75cb 100644 --- a/src/services/auth.ts +++ b/src/services/auth.ts @@ -7,13 +7,11 @@ import utils = require('./utils'); import passwordEncryptionService = require('./encryption/password_encryption'); import config = require('./config'); import passwordService = require('./encryption/password'); +import type { Request } from 'express'; const noAuthentication = config.General && config.General.noAuthentication === true; -// TODO: We are using custom types for request & response because couldn't extract those pesky express types. -interface Request { - method: string; - path: string; +interface AppRequest extends Request { headers: { authorization?: string; "trilium-cred"?: string; @@ -30,7 +28,7 @@ interface Response { type Callback = () => void; -function checkAuth(req: Request, res: Response, next: Callback) { +function checkAuth(req: AppRequest, res: Response, next: Callback) { if (!sqlInit.isDbInitialized()) { res.redirect("setup"); } @@ -44,7 +42,7 @@ function checkAuth(req: Request, res: Response, next: Callback) { // for electron things which need network stuff // currently, we're doing that for file upload because handling form data seems to be difficult -function checkApiAuthOrElectron(req: Request, res: Response, next: Callback) { +function checkApiAuthOrElectron(req: AppRequest, res: Response, next: Callback) { if (!req.session.loggedIn && !utils.isElectron() && !noAuthentication) { reject(req, res, "Logged in session not found"); } @@ -53,7 +51,7 @@ function checkApiAuthOrElectron(req: Request, res: Response, next: Callback) { } } -function checkApiAuth(req: Request, res: Response, next: Callback) { +function checkApiAuth(req: AppRequest, res: Response, next: Callback) { if (!req.session.loggedIn && !noAuthentication) { reject(req, res, "Logged in session not found"); } @@ -62,7 +60,7 @@ function checkApiAuth(req: Request, res: Response, next: Callback) { } } -function checkAppInitialized(req: Request, res: Response, next: Callback) { +function checkAppInitialized(req: AppRequest, res: Response, next: Callback) { if (!sqlInit.isDbInitialized()) { res.redirect("setup"); } @@ -71,7 +69,7 @@ function checkAppInitialized(req: Request, res: Response, next: Callback) { } } -function checkPasswordSet(req: Request, res: Response, next: Callback) { +function checkPasswordSet(req: AppRequest, res: Response, next: Callback) { if (!utils.isElectron() && !passwordService.isPasswordSet()) { res.redirect("set-password"); } else { @@ -79,7 +77,7 @@ function checkPasswordSet(req: Request, res: Response, next: Callback) { } } -function checkPasswordNotSet(req: Request, res: Response, next: Callback) { +function checkPasswordNotSet(req: AppRequest, res: Response, next: Callback) { if (!utils.isElectron() && passwordService.isPasswordSet()) { res.redirect("login"); } else { @@ -87,7 +85,7 @@ function checkPasswordNotSet(req: Request, res: Response, next: Callback) { } } -function checkAppNotInitialized(req: Request, res: Response, next: Callback) { +function checkAppNotInitialized(req: AppRequest, res: Response, next: Callback) { if (sqlInit.isDbInitialized()) { reject(req, res, "App already initialized."); } @@ -96,7 +94,7 @@ function checkAppNotInitialized(req: Request, res: Response, next: Callback) { } } -function checkEtapiToken(req: Request, res: Response, next: Callback) { +function checkEtapiToken(req: AppRequest, res: Response, next: Callback) { if (etapiTokenService.isValidAuthHeader(req.headers.authorization)) { next(); } @@ -105,7 +103,7 @@ function checkEtapiToken(req: Request, res: Response, next: Callback) { } } -function reject(req: Request, res: Response, message: string) { +function reject(req: AppRequest, res: Response, message: string) { log.info(`${req.method} ${req.path} rejected with 401 ${message}`); res.setHeader("Content-Type", "text/plain") @@ -113,7 +111,7 @@ function reject(req: Request, res: Response, message: string) { .send(message); } -function checkCredentials(req: Request, res: Response, next: Callback) { +function checkCredentials(req: AppRequest, res: Response, next: Callback) { if (!sqlInit.isDbInitialized()) { res.setHeader("Content-Type", "text/plain") .status(400) diff --git a/src/services/date_utils.ts b/src/services/date_utils.ts index 82347504c..88b6ecb69 100644 --- a/src/services/date_utils.ts +++ b/src/services/date_utils.ts @@ -80,7 +80,7 @@ function validateLocalDateTime(str: string | null | undefined) { } } -function validateUtcDateTime(str?: string) { +function validateUtcDateTime(str: string | undefined) { if (!str) { return; } diff --git a/src/services/etapi_tokens.ts b/src/services/etapi_tokens.ts index 46e9c65df..2989d3923 100644 --- a/src/services/etapi_tokens.ts +++ b/src/services/etapi_tokens.ts @@ -25,7 +25,7 @@ function createToken(tokenName: string) { }; } -function parseAuthToken(auth?: string | null) { +function parseAuthToken(auth: string | undefined) { if (!auth) { return null; } @@ -64,7 +64,7 @@ function parseAuthToken(auth?: string | null) { } } -function isValidAuthHeader(auth?: string | null) { +function isValidAuthHeader(auth: string | undefined) { const parsed = parseAuthToken(auth); if (!parsed) { diff --git a/src/services/keyboard_actions.ts b/src/services/keyboard_actions.ts index 8bd830cfe..b2cbaad29 100644 --- a/src/services/keyboard_actions.ts +++ b/src/services/keyboard_actions.ts @@ -623,7 +623,7 @@ function getKeyboardActions() { for (const option of optionService.getOptions()) { if (option.name.startsWith('keyboardShortcuts')) { - let actionName = option.name.substr(17); + let actionName = option.name.substring(17); actionName = actionName.charAt(0).toLowerCase() + actionName.slice(1); const action = actions.find(ea => ea.actionName === actionName); diff --git a/src/services/notes.ts b/src/services/notes.ts index d6efbf31b..a402a2f16 100644 --- a/src/services/notes.ts +++ b/src/services/notes.ts @@ -873,7 +873,7 @@ function getUndeletedParentBranchIds(noteId: string, deleteId: string) { AND parentNote.isDeleted = 0`, [noteId, deleteId]); } -function scanForLinks(note: BNote | null, content: string) { +function scanForLinks(note: BNote, content: string) { if (!note || !['text', 'relationMap'].includes(note.type)) { return; } diff --git a/src/services/sql.ts b/src/services/sql.ts index 1945efa92..671b136d2 100644 --- a/src/services/sql.ts +++ b/src/services/sql.ts @@ -144,7 +144,7 @@ function getRows(query: string, params: Params = []): T[] { } function getRawRows(query: string, params: Params = []): T[] { - return (wrap(query, s => s.raw().all(params)) as T[] | null) || []; + return (wrap(query, s => s.raw().all(params)) as T[]) || []; } function iterateRows(query: string, params: Params = []) { From 17c7e2d8e7b5b0db3239cbd8591aab769991d3de Mon Sep 17 00:00:00 2001 From: Elian Doran Date: Wed, 3 Apr 2024 20:04:20 +0300 Subject: [PATCH 4/4] server-ts: Address further suggestions --- src/services/auth.ts | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/services/auth.ts b/src/services/auth.ts index 0e96f75cb..d54a1ea96 100644 --- a/src/services/auth.ts +++ b/src/services/auth.ts @@ -7,7 +7,7 @@ import utils = require('./utils'); import passwordEncryptionService = require('./encryption/password_encryption'); import config = require('./config'); import passwordService = require('./encryption/password'); -import type { Request } from 'express'; +import type { NextFunction, Request, Response } from 'express'; const noAuthentication = config.General && config.General.noAuthentication === true; @@ -21,14 +21,7 @@ interface AppRequest extends Request { } } -interface Response { - redirect(url: string): void; - setHeader(key: string, value: string): any -} - -type Callback = () => void; - -function checkAuth(req: AppRequest, res: Response, next: Callback) { +function checkAuth(req: AppRequest, res: Response, next: NextFunction) { if (!sqlInit.isDbInitialized()) { res.redirect("setup"); } @@ -42,7 +35,7 @@ function checkAuth(req: AppRequest, res: Response, next: Callback) { // for electron things which need network stuff // currently, we're doing that for file upload because handling form data seems to be difficult -function checkApiAuthOrElectron(req: AppRequest, res: Response, next: Callback) { +function checkApiAuthOrElectron(req: AppRequest, res: Response, next: NextFunction) { if (!req.session.loggedIn && !utils.isElectron() && !noAuthentication) { reject(req, res, "Logged in session not found"); } @@ -51,7 +44,7 @@ function checkApiAuthOrElectron(req: AppRequest, res: Response, next: Callback) } } -function checkApiAuth(req: AppRequest, res: Response, next: Callback) { +function checkApiAuth(req: AppRequest, res: Response, next: NextFunction) { if (!req.session.loggedIn && !noAuthentication) { reject(req, res, "Logged in session not found"); } @@ -60,7 +53,7 @@ function checkApiAuth(req: AppRequest, res: Response, next: Callback) { } } -function checkAppInitialized(req: AppRequest, res: Response, next: Callback) { +function checkAppInitialized(req: AppRequest, res: Response, next: NextFunction) { if (!sqlInit.isDbInitialized()) { res.redirect("setup"); } @@ -69,7 +62,7 @@ function checkAppInitialized(req: AppRequest, res: Response, next: Callback) { } } -function checkPasswordSet(req: AppRequest, res: Response, next: Callback) { +function checkPasswordSet(req: AppRequest, res: Response, next: NextFunction) { if (!utils.isElectron() && !passwordService.isPasswordSet()) { res.redirect("set-password"); } else { @@ -77,7 +70,7 @@ function checkPasswordSet(req: AppRequest, res: Response, next: Callback) { } } -function checkPasswordNotSet(req: AppRequest, res: Response, next: Callback) { +function checkPasswordNotSet(req: AppRequest, res: Response, next: NextFunction) { if (!utils.isElectron() && passwordService.isPasswordSet()) { res.redirect("login"); } else { @@ -85,7 +78,7 @@ function checkPasswordNotSet(req: AppRequest, res: Response, next: Callback) { } } -function checkAppNotInitialized(req: AppRequest, res: Response, next: Callback) { +function checkAppNotInitialized(req: AppRequest, res: Response, next: NextFunction) { if (sqlInit.isDbInitialized()) { reject(req, res, "App already initialized."); } @@ -94,7 +87,7 @@ function checkAppNotInitialized(req: AppRequest, res: Response, next: Callback) } } -function checkEtapiToken(req: AppRequest, res: Response, next: Callback) { +function checkEtapiToken(req: AppRequest, res: Response, next: NextFunction) { if (etapiTokenService.isValidAuthHeader(req.headers.authorization)) { next(); } @@ -111,7 +104,7 @@ function reject(req: AppRequest, res: Response, message: string) { .send(message); } -function checkCredentials(req: AppRequest, res: Response, next: Callback) { +function checkCredentials(req: AppRequest, res: Response, next: NextFunction) { if (!sqlInit.isDbInitialized()) { res.setHeader("Content-Type", "text/plain") .status(400)