From 6c99c1cae6b90132d23015219c7c785d3bc0eb2c Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 7 May 2024 22:54:20 +0100 Subject: [PATCH] init --- packages/server/src/app.ts | 6 +-- .../server/src/commands/StorageCommand.ts | 2 +- packages/server/src/models/BaseModel.ts | 8 ++- packages/server/src/models/ChangeModel.ts | 12 ++--- packages/server/src/models/ItemModel.test.ts | 18 +++---- packages/server/src/models/ItemModel.ts | 8 +-- packages/server/src/models/UserModel.ts | 4 +- packages/server/src/models/factory.ts | 52 ++++++++++--------- .../items/storage/loadStorageDriver.test.ts | 6 +-- .../models/items/storage/loadStorageDriver.ts | 6 +-- .../src/models/items/storage/testUtils.ts | 6 +-- packages/server/src/tools/debugTools.ts | 4 +- packages/server/src/utils/setupAppContext.ts | 2 +- .../src/utils/storageConnectionCheck.ts | 4 +- .../server/src/utils/testing/testUtils.ts | 6 ++- 15 files changed, 78 insertions(+), 66 deletions(-) diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 77499bdd9e..fb912da47e 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -270,7 +270,7 @@ async function main() { }); } else { const connectionCheck = await waitForConnection(config().database); - const models = newModelFactory(connectionCheck.connection, config()); + const models = newModelFactory(connectionCheck.connection, connectionCheck.connection, config()); await selectedCommand.run(commandArgv, { db: connectionCheck.connection, @@ -333,11 +333,11 @@ async function main() { await initializeJoplinUtils(config(), ctx.joplinBase.models, ctx.joplinBase.services.mustache); appLogger().info('Performing main storage check...'); - appLogger().info(await storageConnectionCheck(config().storageDriver, ctx.joplinBase.db, ctx.joplinBase.models)); + appLogger().info(await storageConnectionCheck(config().storageDriver, ctx.joplinBase.db, ctx.joplinBase.db, ctx.joplinBase.models)); if (config().storageDriverFallback) { appLogger().info('Performing fallback storage check...'); - appLogger().info(await storageConnectionCheck(config().storageDriverFallback, ctx.joplinBase.db, ctx.joplinBase.models)); + appLogger().info(await storageConnectionCheck(config().storageDriverFallback, ctx.joplinBase.db, ctx.joplinBase.db, ctx.joplinBase.models)); } appLogger().info('Starting services...'); diff --git a/packages/server/src/commands/StorageCommand.ts b/packages/server/src/commands/StorageCommand.ts index 3b9fa13948..1ba93cb50b 100644 --- a/packages/server/src/commands/StorageCommand.ts +++ b/packages/server/src/commands/StorageCommand.ts @@ -87,7 +87,7 @@ export default class StorageCommand extends BaseCommand { }, [ArgvCommand.CheckConnection]: async () => { - logger.info(await storageConnectionCheck(argv.connection, runContext.db, runContext.models)); + logger.info(await storageConnectionCheck(argv.connection, runContext.db, runContext.db, runContext.models)); }, [ArgvCommand.DeleteDatabaseContentColumn]: async () => { diff --git a/packages/server/src/models/BaseModel.ts b/packages/server/src/models/BaseModel.ts index 70c412e9d3..6575c989f7 100644 --- a/packages/server/src/models/BaseModel.ts +++ b/packages/server/src/models/BaseModel.ts @@ -64,13 +64,15 @@ export default abstract class BaseModel { private defaultFields_: string[] = []; private db_: DbConnection; + private dbReader_: DbConnection; private transactionHandler_: TransactionHandler; private modelFactory_: NewModelFactoryHandler; private config_: Config; private savePoints_: SavePoint[] = []; - public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { + public constructor(db: DbConnection, dbReader: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { this.db_ = db; + this.dbReader_ = dbReader; this.modelFactory_ = modelFactory; this.config_ = config; @@ -113,6 +115,10 @@ export default abstract class BaseModel { return this.db_; } + public get dbReader(): DbConnection { + return this.dbReader_; + } + protected get defaultFields(): string[] { if (!this.defaultFields_.length) { this.defaultFields_ = Object.keys(databaseSchema[this.tableName]); diff --git a/packages/server/src/models/ChangeModel.ts b/packages/server/src/models/ChangeModel.ts index 58e5aa5f10..623775c644 100644 --- a/packages/server/src/models/ChangeModel.ts +++ b/packages/server/src/models/ChangeModel.ts @@ -57,8 +57,8 @@ export default class ChangeModel extends BaseModel { public deltaIncludesItems_: boolean; - public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { - super(db, modelFactory, config); + public constructor(db: DbConnection, dbReader: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { + super(db, dbReader, modelFactory, config); this.deltaIncludesItems_ = config.DELTA_INCLUDES_ITEMS; } @@ -199,8 +199,8 @@ export default class ChangeModel extends BaseModel { if (!doCountQuery) { finalParams.push(limit); - if (isPostgres(this.db)) { - query = this.db.raw(` + if (isPostgres(this.dbReader)) { + query = this.dbReader.raw(` WITH cte1 AS MATERIALIZED ( ${subQuery1} ) @@ -214,7 +214,7 @@ export default class ChangeModel extends BaseModel { LIMIT ? `, finalParams); } else { - query = this.db.raw(` + query = this.dbReader.raw(` SELECT ${fieldsSql} FROM (${subQuery1}) as sub1 UNION ALL SELECT ${fieldsSql} FROM (${subQuery2}) as sub2 @@ -223,7 +223,7 @@ export default class ChangeModel extends BaseModel { `, finalParams); } } else { - query = this.db.raw(` + query = this.dbReader.raw(` SELECT count(*) as total FROM ( (${subQuery1}) diff --git a/packages/server/src/models/ItemModel.test.ts b/packages/server/src/models/ItemModel.test.ts index 0d18063ce4..0769f4a4cb 100644 --- a/packages/server/src/models/ItemModel.test.ts +++ b/packages/server/src/models/ItemModel.test.ts @@ -1,4 +1,4 @@ -import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, createItemTree, createResource, createNote, createItemTree3, db, tempDir, expectNotThrow, expectHttpError } from '../utils/testing/testUtils'; +import { createUserAndSession, beforeAllDb, afterAllTests, beforeEachDb, models, createItemTree, createResource, createNote, createItemTree3, db, tempDir, expectNotThrow, expectHttpError, dbReader } from '../utils/testing/testUtils'; import { shareFolderWithUser } from '../utils/testing/shareApiUtils'; import { resourceBlobPath } from '../utils/joplinUtils'; import newModelFactory from './factory'; @@ -275,7 +275,7 @@ describe('ItemModel', () => { test('should respect the hard item size limit', async () => { const { user: user1 } = await createUserAndSession(1); - let models = newModelFactory(db(), config()); + let models = newModelFactory(db(), dbReader(), config()); let result = await models.item().saveFromRawContent(user1, { body: Buffer.from('1234'), @@ -285,7 +285,7 @@ describe('ItemModel', () => { const item = result['test1.txt'].item; config().itemSizeHardLimit = 3; - models = newModelFactory(db(), config()); + models = newModelFactory(db(), dbReader(), config()); result = await models.item().saveFromRawContent(user1, { body: Buffer.from('1234'), @@ -297,7 +297,7 @@ describe('ItemModel', () => { await expectHttpError(async () => models.item().loadWithContent(item.id), ErrorPayloadTooLarge.httpCode); config().itemSizeHardLimit = 1000; - models = newModelFactory(db(), config()); + models = newModelFactory(db(), dbReader(), config()); await expectNotThrow(async () => models.item().loadWithContent(item.id)); }); @@ -316,18 +316,18 @@ describe('ItemModel', () => { path: tempDir2, }; - const fromModels = newModelFactory(db(), { + const fromModels = newModelFactory(db(), dbReader(), { ...config(), storageDriver: fromStorageConfig, }); - const toModels = newModelFactory(db(), { + const toModels = newModelFactory(db(), dbReader(), { ...config(), storageDriver: toStorageConfig, }); - const fromDriver = await loadStorageDriver(fromStorageConfig, db()); - const toDriver = await loadStorageDriver(toStorageConfig, db()); + const fromDriver = await loadStorageDriver(fromStorageConfig, db(), dbReader()); + const toDriver = await loadStorageDriver(toStorageConfig, db(), dbReader()); return { fromStorageConfig, @@ -364,7 +364,7 @@ describe('ItemModel', () => { await msleep(2); - const toModels = newModelFactory(db(), { + const toModels = newModelFactory(db(), dbReader(), { ...config(), storageDriver: toStorageConfig, }); diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index 54e67d2c12..56e035e62b 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -75,8 +75,8 @@ export default class ItemModel extends BaseModel { private static storageDrivers_: Map = new Map(); - public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { - super(db, modelFactory, config); + public constructor(db: DbConnection, dbReader: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { + super(db, dbReader, modelFactory, config); this.storageDriverConfig_ = config.storageDriver; this.storageDriverConfigFallback_ = config.storageDriverFallback; @@ -102,7 +102,7 @@ export default class ItemModel extends BaseModel { let driver = ItemModel.storageDrivers_.get(config); if (!driver) { - driver = await loadStorageDriver(config, this.db); + driver = await loadStorageDriver(config, this.db, this.dbReader); ItemModel.storageDrivers_.set(config, driver); } @@ -331,7 +331,7 @@ export default class ItemModel extends BaseModel { let fromDriver: StorageDriverBase = drivers[item.content_storage_id]; if (!fromDriver) { - fromDriver = await loadStorageDriver(item.content_storage_id, this.db); + fromDriver = await loadStorageDriver(item.content_storage_id, this.db, this.dbReader); drivers[item.content_storage_id] = fromDriver; } diff --git a/packages/server/src/models/UserModel.ts b/packages/server/src/models/UserModel.ts index 6b9619c365..7a3529bb87 100644 --- a/packages/server/src/models/UserModel.ts +++ b/packages/server/src/models/UserModel.ts @@ -118,8 +118,8 @@ export default class UserModel extends BaseModel { private ldapConfig_: LdapConfig[]; - public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { - super(db, modelFactory, config); + public constructor(db: DbConnection, dbReader: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { + super(db, dbReader, modelFactory, config); this.ldapConfig_ = config.ldap; } diff --git a/packages/server/src/models/factory.ts b/packages/server/src/models/factory.ts index 40850bf296..e4ad5d5f0b 100644 --- a/packages/server/src/models/factory.ts +++ b/packages/server/src/models/factory.ts @@ -83,105 +83,107 @@ export type NewModelFactoryHandler = (db: DbConnection)=> Models; export class Models { private db_: DbConnection; + private dbReader_: DbConnection; private config_: Config; - public constructor(db: DbConnection, config: Config) { + public constructor(db: DbConnection, dbReader_: DbConnection, config: Config) { this.db_ = db; + this.dbReader_ = dbReader_; this.config_ = config; this.newModelFactory = this.newModelFactory.bind(this); } private newModelFactory(db: DbConnection) { - return new Models(db, this.config_); + return new Models(db, this.dbReader_, this.config_); } public item() { - return new ItemModel(this.db_, this.newModelFactory, this.config_); + return new ItemModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public user() { - return new UserModel(this.db_, this.newModelFactory, this.config_); + return new UserModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public email() { - return new EmailModel(this.db_, this.newModelFactory, this.config_); + return new EmailModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public userItem() { - return new UserItemModel(this.db_, this.newModelFactory, this.config_); + return new UserItemModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public token() { - return new TokenModel(this.db_, this.newModelFactory, this.config_); + return new TokenModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public itemResource() { - return new ItemResourceModel(this.db_, this.newModelFactory, this.config_); + return new ItemResourceModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public apiClient() { - return new ApiClientModel(this.db_, this.newModelFactory, this.config_); + return new ApiClientModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public session() { - return new SessionModel(this.db_, this.newModelFactory, this.config_); + return new SessionModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public change() { - return new ChangeModel(this.db_, this.newModelFactory, this.config_); + return new ChangeModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public notification() { - return new NotificationModel(this.db_, this.newModelFactory, this.config_); + return new NotificationModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public share() { - return new ShareModel(this.db_, this.newModelFactory, this.config_); + return new ShareModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public shareUser() { - return new ShareUserModel(this.db_, this.newModelFactory, this.config_); + return new ShareUserModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public keyValue() { - return new KeyValueModel(this.db_, this.newModelFactory, this.config_); + return new KeyValueModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public subscription() { - return new SubscriptionModel(this.db_, this.newModelFactory, this.config_); + return new SubscriptionModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public userFlag() { - return new UserFlagModel(this.db_, this.newModelFactory, this.config_); + return new UserFlagModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public event() { - return new EventModel(this.db_, this.newModelFactory, this.config_); + return new EventModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public lock() { - return new LockModel(this.db_, this.newModelFactory, this.config_); + return new LockModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public storage() { - return new StorageModel(this.db_, this.newModelFactory, this.config_); + return new StorageModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public userDeletion() { - return new UserDeletionModel(this.db_, this.newModelFactory, this.config_); + return new UserDeletionModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public backupItem() { - return new BackupItemModel(this.db_, this.newModelFactory, this.config_); + return new BackupItemModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } public taskState() { - return new TaskStateModel(this.db_, this.newModelFactory, this.config_); + return new TaskStateModel(this.db_, this.dbReader_, this.newModelFactory, this.config_); } } -export default function newModelFactory(db: DbConnection, config: Config): Models { - return new Models(db, config); +export default function newModelFactory(db: DbConnection, dbReader: DbConnection, config: Config): Models { + return new Models(db, dbReader, config); } diff --git a/packages/server/src/models/items/storage/loadStorageDriver.test.ts b/packages/server/src/models/items/storage/loadStorageDriver.test.ts index a5d4cc5991..7bef4d5daf 100644 --- a/packages/server/src/models/items/storage/loadStorageDriver.test.ts +++ b/packages/server/src/models/items/storage/loadStorageDriver.test.ts @@ -1,4 +1,4 @@ -import { afterAllTests, beforeAllDb, beforeEachDb, db, expectThrow, models } from '../../../utils/testing/testUtils'; +import { afterAllTests, beforeAllDb, beforeEachDb, db, dbReader, expectThrow, models } from '../../../utils/testing/testUtils'; import { StorageDriverType } from '../../../utils/types'; import loadStorageDriver from './loadStorageDriver'; @@ -18,13 +18,13 @@ describe('loadStorageDriver', () => { test('should load a driver and assign an ID to it', async () => { { - const newDriver = await loadStorageDriver({ type: StorageDriverType.Memory }, db()); + const newDriver = await loadStorageDriver({ type: StorageDriverType.Memory }, db(), dbReader()); expect(newDriver.storageId).toBe(1); expect((await models().storage().count())).toBe(1); } { - const newDriver = await loadStorageDriver({ type: StorageDriverType.Filesystem, path: '/just/testing' }, db()); + const newDriver = await loadStorageDriver({ type: StorageDriverType.Filesystem, path: '/just/testing' }, db(), dbReader()); expect(newDriver.storageId).toBe(2); expect((await models().storage().count())).toBe(2); } diff --git a/packages/server/src/models/items/storage/loadStorageDriver.ts b/packages/server/src/models/items/storage/loadStorageDriver.ts index 42fe1d40d6..fb32c617e6 100644 --- a/packages/server/src/models/items/storage/loadStorageDriver.ts +++ b/packages/server/src/models/items/storage/loadStorageDriver.ts @@ -14,7 +14,7 @@ export interface Options { assignDriverId?: boolean; } -export default async function(config: StorageDriverConfig | number, db: DbConnection, options: Options = null): Promise { +export default async function(config: StorageDriverConfig | number, db: DbConnection, dbReader: DbConnection, options: Options = null): Promise { if (!config) return null; options = { @@ -27,14 +27,14 @@ export default async function(config: StorageDriverConfig | number, db: DbConnec if (typeof config === 'number') { storageId = config; - const models = newModelFactory(db, globalConfig()); + const models = newModelFactory(db, dbReader, globalConfig()); const storage = await models.storage().byId(storageId); if (!storage) throw new Error(`No such storage ID: ${storageId}`); config = parseStorageDriverConnectionString(storage.connection_string); } else { if (options.assignDriverId) { - const models = newModelFactory(db, globalConfig()); + const models = newModelFactory(db, dbReader, globalConfig()); const connectionString = serializeStorageConfig(config); let storage = await models.storage().byConnectionString(connectionString); diff --git a/packages/server/src/models/items/storage/testUtils.ts b/packages/server/src/models/items/storage/testUtils.ts index 1108f3b915..adf082d103 100644 --- a/packages/server/src/models/items/storage/testUtils.ts +++ b/packages/server/src/models/items/storage/testUtils.ts @@ -3,7 +3,7 @@ import config from '../../../config'; import { Item } from '../../../services/database/types'; import { CustomErrorCode } from '../../../utils/errors'; -import { createUserAndSession, db, makeNoteSerializedBody, models } from '../../../utils/testing/testUtils'; +import { createUserAndSession, db, dbReader, makeNoteSerializedBody, models } from '../../../utils/testing/testUtils'; import { Config, StorageDriverConfig, StorageDriverMode } from '../../../utils/types'; import newModelFactory from '../../factory'; import loadStorageDriver from './loadStorageDriver'; @@ -15,7 +15,7 @@ const newTestModels = (driverConfig: StorageDriverConfig, driverConfigFallback: storageDriver: driverConfig, storageDriverFallback: driverConfigFallback, }; - return newModelFactory(db(), newConfig); + return newModelFactory(db(), dbReader(), newConfig); }; export function shouldWriteToContentAndReadItBack(driverConfig: StorageDriverConfig) { @@ -281,7 +281,7 @@ export function shouldUpdateContentStorageIdAfterSwitchingDriver(oldDriverConfig export function shouldThrowNotFoundIfNotExist(driverConfig: StorageDriverConfig) { test('should throw not found if item does not exist', async () => { - const driver = await loadStorageDriver(driverConfig, db()); + const driver = await loadStorageDriver(driverConfig, db(), dbReader()); // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied let error: any = null; diff --git a/packages/server/src/tools/debugTools.ts b/packages/server/src/tools/debugTools.ts index c38ab95be0..5f2d6d2cab 100644 --- a/packages/server/src/tools/debugTools.ts +++ b/packages/server/src/tools/debugTools.ts @@ -54,7 +54,7 @@ export async function createTestUsers(db: DbConnection, config: Config, options: }; const password = '111111'; - const models = newModelFactory(db, config); + const models = newModelFactory(db, db, config); await truncateTables(db, includedTables); @@ -127,7 +127,7 @@ export async function createTestUsers(db: DbConnection, config: Config, options: } export async function createUserDeletions(db: DbConnection, config: Config) { - const models = newModelFactory(db, config); + const models = newModelFactory(db, db, config); const users = await models.user().all(); diff --git a/packages/server/src/utils/setupAppContext.ts b/packages/server/src/utils/setupAppContext.ts index ff571fb813..853a9f86bf 100644 --- a/packages/server/src/utils/setupAppContext.ts +++ b/packages/server/src/utils/setupAppContext.ts @@ -29,7 +29,7 @@ async function setupServices(env: Env, models: Models, config: Config): Promise< } export default async function(appContext: AppContext, env: Env, dbConnection: DbConnection, appLogger: ()=> LoggerWrapper): Promise { - const models = newModelFactory(dbConnection, config()); + const models = newModelFactory(dbConnection, dbConnection, config()); // The joplinBase object is immutable because it is shared by all requests. // Then a "joplin" context property is created from it per request, which diff --git a/packages/server/src/utils/storageConnectionCheck.ts b/packages/server/src/utils/storageConnectionCheck.ts index 0538a0d85d..38deedbaa0 100644 --- a/packages/server/src/utils/storageConnectionCheck.ts +++ b/packages/server/src/utils/storageConnectionCheck.ts @@ -6,12 +6,12 @@ import { Context } from '../models/items/storage/StorageDriverBase'; import { StorageDriverConfig, StorageDriverType } from './types'; import { uuidgen } from '@joplin/lib/uuid'; -export default async function(connection: string | StorageDriverConfig, db: DbConnection, models: Models): Promise { +export default async function(connection: string | StorageDriverConfig, db: DbConnection, dbReader: DbConnection, models: Models): Promise { const storageConfig = typeof connection === 'string' ? parseStorageConnectionString(connection) : connection; if (storageConfig.type === StorageDriverType.Database) return 'Database storage is special and cannot be checked this way. If the connection to the database was successful then the storage driver should work too.'; - const driver = await loadStorageDriver(storageConfig, db, { assignDriverId: false }); + const driver = await loadStorageDriver(storageConfig, db, dbReader, { assignDriverId: false }); const itemId = `testingconnection${uuidgen(8)}`; const itemContent = Buffer.from(uuidgen(8)); const context: Context = { models }; diff --git a/packages/server/src/utils/testing/testUtils.ts b/packages/server/src/utils/testing/testUtils.ts index beacdadcec..4ee6357d4c 100644 --- a/packages/server/src/utils/testing/testUtils.ts +++ b/packages/server/src/utils/testing/testUtils.ts @@ -257,8 +257,12 @@ export function db() { return db_; } +export function dbReader() { + return db_; +} + export function models() { - return modelFactory(db(), config()); + return modelFactory(db(), dbReader(), config()); } export function parseHtml(html: string): Document {