From 89179c2776905790e0b15baabc8a13d3c34a8fa2 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 9 Nov 2021 16:42:50 +0000 Subject: [PATCH 1/6] Desktop, Cli: Add support for more style of highlighted texts when importing ENEX files --- .../app-cli/tests/enex_to_md/highlight.html | 4 ++- .../app-cli/tests/enex_to_md/highlight.md | 3 +- packages/lib/import-enex-md-gen.ts | 36 +++++++++++++++---- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/packages/app-cli/tests/enex_to_md/highlight.html b/packages/app-cli/tests/enex_to_md/highlight.html index 0230649d0b..6e6589c363 100644 --- a/packages/app-cli/tests/enex_to_md/highlight.html +++ b/packages/app-cli/tests/enex_to_md/highlight.html @@ -1 +1,3 @@ -I'll highlight some text. \ No newline at end of file +I'll highlight some text. +
+this text is yellow \ No newline at end of file diff --git a/packages/app-cli/tests/enex_to_md/highlight.md b/packages/app-cli/tests/enex_to_md/highlight.md index ef6876cfdd..36f301ee70 100644 --- a/packages/app-cli/tests/enex_to_md/highlight.md +++ b/packages/app-cli/tests/enex_to_md/highlight.md @@ -1 +1,2 @@ -==I'll highlight some text.== \ No newline at end of file +==I'll highlight some text.== +==this text is yellow== \ No newline at end of file diff --git a/packages/lib/import-enex-md-gen.ts b/packages/lib/import-enex-md-gen.ts index ef4066aad9..0dd84951ea 100644 --- a/packages/lib/import-enex-md-gen.ts +++ b/packages/lib/import-enex-md-gen.ts @@ -426,14 +426,21 @@ function attributeToLowerCase(node: any) { return output; } -function cssValue(context: any, style: string, propName: string): string { +function cssValue(context: any, style: string, propName: string | string[]): string { if (!style) return null; + const propNames = Array.isArray(propName) ? propName : [propName]; + try { const o = cssParser.parse(`pre {${style}}`); if (!o.stylesheet.rules.length) return null; - const prop = o.stylesheet.rules[0].declarations.find((d: any) => d.property.toLowerCase() === propName); - return prop && prop.value ? prop.value.trim().toLowerCase() : null; + + for (const propName of propNames) { + const prop = o.stylesheet.rules[0].declarations.find((d: any) => d.property.toLowerCase() === propName); + if (prop && prop.value) return prop.value.trim().toLowerCase(); + } + + return null; } catch (error) { displaySaxWarning(context, error.message); return null; @@ -507,7 +514,13 @@ function isCodeBlock(context: any, nodeName: string, attributes: any) { // Yes, this property sometimes appears as -en-codeblock, sometimes as // --en-codeblock. Would be too easy to import ENEX data otherwise. // https://github.com/laurent22/joplin/issues/4965 - const enCodeBlock = cssValue(context, attributes.style, '-en-codeblock') || cssValue(context, attributes.style, '--en-codeblock'); + const enCodeBlock = cssValue(context, attributes.style, [ + '-en-codeblock', + '--en-codeblock', + '-evernote-codeblock', + '--evernote-codeblock', + ]); + if (enCodeBlock && enCodeBlock.toLowerCase() === 'true') return true; } return false; @@ -518,8 +531,19 @@ function isHighlight(context: any, _nodeName: string, attributes: any) { // Evernote uses various inconsistent CSS prefixes: so far I've found // "--en", "-en", "-evernote", so I'm guessing "--evernote" probably // exists too. - const enHighlight = cssValue(context, attributes.style, '-evernote-highlight') || cssValue(context, attributes.style, '--evernote-highlight'); - if (enHighlight && enHighlight.toLowerCase() === 'true') return true; + + const enHighlight = cssValue(context, attributes.style, [ + '-evernote-highlight', + '--evernote-highlight', + '-en-highlight', + '--en-highlight', + ]); + + // Value can be any colour or "true". I guess if it's set at all it + // should be highlighted but just in case handle case where it's + // "false". + + if (enHighlight && enHighlight.toLowerCase() !== 'false') return true; } return false; } From f8d9601ff7ccad50bc45aad7233b6c8bcafbc126 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 9 Nov 2021 17:49:05 +0000 Subject: [PATCH 2/6] Tools: Restored HTML to ENEX tests that had been deleted in a refactoring --- packages/lib/import-enex-html-gen.test.js | 115 ++++++++++++++++++++++ 1 file changed, 115 insertions(+) create mode 100644 packages/lib/import-enex-html-gen.test.js diff --git a/packages/lib/import-enex-html-gen.test.js b/packages/lib/import-enex-html-gen.test.js new file mode 100644 index 0000000000..348d9df96a --- /dev/null +++ b/packages/lib/import-enex-html-gen.test.js @@ -0,0 +1,115 @@ + +const { setupDatabaseAndSynchronizer, switchClient, supportDir } = require('./testing/test-utils.js'); +const shim = require('./shim').default; +const { enexXmlToHtml } = require('./import-enex-html-gen.js'); +const cleanHtml = require('clean-html'); + +const fileWithPath = (filename) => + `${supportDir}/../enex_to_html/${filename}`; + +const audioResource = { + filename: 'audio test', + id: '9168ee833d03c5ea7c730ac6673978c1', + mime: 'audio/x-m4a', + size: 82011, + title: 'audio test', +}; + +// All the test HTML files are beautified ones, so we need to run +// this before the comparison. Before, beautifying was done by `enexXmlToHtml` +// but that was removed due to problems with the clean-html package. +const beautifyHtml = (html) => { + return new Promise((resolve) => { + try { + cleanHtml.clean(html, { wrap: 0 }, (...cleanedHtml) => resolve(cleanedHtml.join(''))); + } catch (error) { + console.warn(`Could not clean HTML - the "unclean" version will be used: ${error.message}: ${html.trim().substr(0, 512).replace(/[\n\r]/g, ' ')}...`); + resolve([html].join('')); + } + }); +}; + +/** + * Tests the importer for a single note, checking that the result of + * processing the given `.enex` input file matches the contents of the given + * `.html` file. + * + * Note that this does not test the importing of an entire exported `.enex` + * archive, but rather a single node of such a file. Thus, the test data files + * (e.g. `./enex_to_html/code1.enex`) correspond to the contents of a single + * `...` node in an `.enex` file already extracted from + * ``. + */ +const compareOutputToExpected = (options) => { + const inputFile = fileWithPath(`${options.testName}.enex`); + const outputFile = fileWithPath(`${options.testName}.html`); + const testTitle = `should convert from Enex to Html: ${options.testName}`; + + it(testTitle, (async () => { + const enexInput = await shim.fsDriver().readFile(inputFile); + const expectedOutput = await shim.fsDriver().readFile(outputFile); + const actualOutput = await beautifyHtml(await enexXmlToHtml(enexInput, options.resources)); + expect(actualOutput).toEqual(expectedOutput); + })); +}; + +describe('EnexToHtml', function() { + beforeEach(async (done) => { + await setupDatabaseAndSynchronizer(1); + await switchClient(1); + done(); + }); + + compareOutputToExpected({ + testName: 'checklist-list', + resources: [], + }); + + compareOutputToExpected({ + testName: 'svg', + resources: [], + }); + + compareOutputToExpected({ + testName: 'en-media--image', + resources: [{ + filename: '', + id: '89ce7da62c6b2832929a6964237e98e9', // Mock id + mime: 'image/jpeg', + size: 50347, + title: '', + }], + }); + + compareOutputToExpected({ + testName: 'en-media--audio', + resources: [audioResource], + }); + + compareOutputToExpected({ + testName: 'attachment', + resources: [{ + filename: 'attachment-1', + id: '21ca2b948f222a38802940ec7e2e5de3', + mime: 'application/pdf', // Any non-image/non-audio mime type will do + size: 1000, + }], + }); + + // it('fails when not given a matching resource', (async () => { + // // To test the promise-unexpectedly-resolved case, add `audioResource` to the array. + // const resources = []; + // const inputFile = fileWithPath('en-media--image.enex'); + // const enexInput = await shim.fsDriver().readFile(inputFile); + // const promisedOutput = enexXmlToHtml(enexInput, resources); + + // promisedOutput.then(() => { + // // Promise should not be resolved + // expect(false).toEqual(true); + // }, (reason) => { + // expect(reason) + // .toBe('Hash with no associated resource: 89ce7da62c6b2832929a6964237e98e9'); + // }); + // })); + +}); From 4deeed0d5c0e38513bad304b2784cf8d9cf2dde1 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 9 Nov 2021 18:33:28 +0000 Subject: [PATCH 3/6] Desktop, Mobile: Fixes #5687: Fixed issue with parts of HTML notes not being displayed in some cases --- .../app-cli/tests/enex_to_html/quoted-attributes.enex | 3 +++ .../app-cli/tests/enex_to_html/quoted-attributes.html | 3 +++ .../tests/html_to_html/anchor_missing_href.dest.html | 1 + .../tests/html_to_html/anchor_missing_href.src.html | 1 + packages/lib/import-enex-html-gen.test.js | 11 +++++++++-- packages/lib/resourceUtils.js | 2 +- packages/renderer/htmlUtils.ts | 9 +++++++++ 7 files changed, 27 insertions(+), 3 deletions(-) create mode 100644 packages/app-cli/tests/enex_to_html/quoted-attributes.enex create mode 100644 packages/app-cli/tests/enex_to_html/quoted-attributes.html create mode 100644 packages/app-cli/tests/html_to_html/anchor_missing_href.dest.html create mode 100644 packages/app-cli/tests/html_to_html/anchor_missing_href.src.html diff --git a/packages/app-cli/tests/enex_to_html/quoted-attributes.enex b/packages/app-cli/tests/enex_to_html/quoted-attributes.enex new file mode 100644 index 0000000000..7be1ad101a --- /dev/null +++ b/packages/app-cli/tests/enex_to_html/quoted-attributes.enex @@ -0,0 +1,3 @@ + +

Association Between mRNA Vaccination and COVID-19 Hospitalization and Disease Severity

+
diff --git a/packages/app-cli/tests/enex_to_html/quoted-attributes.html b/packages/app-cli/tests/enex_to_html/quoted-attributes.html new file mode 100644 index 0000000000..2957f4fc5f --- /dev/null +++ b/packages/app-cli/tests/enex_to_html/quoted-attributes.html @@ -0,0 +1,3 @@ + +

Association Between mRNA Vaccination and COVID-19 Hospitalization and Disease Severity

+
\ No newline at end of file diff --git a/packages/app-cli/tests/html_to_html/anchor_missing_href.dest.html b/packages/app-cli/tests/html_to_html/anchor_missing_href.dest.html new file mode 100644 index 0000000000..0a4898926f --- /dev/null +++ b/packages/app-cli/tests/html_to_html/anchor_missing_href.dest.html @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/packages/app-cli/tests/html_to_html/anchor_missing_href.src.html b/packages/app-cli/tests/html_to_html/anchor_missing_href.src.html new file mode 100644 index 0000000000..da987591ce --- /dev/null +++ b/packages/app-cli/tests/html_to_html/anchor_missing_href.src.html @@ -0,0 +1 @@ +test \ No newline at end of file diff --git a/packages/lib/import-enex-html-gen.test.js b/packages/lib/import-enex-html-gen.test.js index 348d9df96a..93722af112 100644 --- a/packages/lib/import-enex-html-gen.test.js +++ b/packages/lib/import-enex-html-gen.test.js @@ -41,6 +41,11 @@ const beautifyHtml = (html) => { * ``. */ const compareOutputToExpected = (options) => { + options = { + resources: [], + ...options, + }; + const inputFile = fileWithPath(`${options.testName}.enex`); const outputFile = fileWithPath(`${options.testName}.html`); const testTitle = `should convert from Enex to Html: ${options.testName}`; @@ -62,12 +67,10 @@ describe('EnexToHtml', function() { compareOutputToExpected({ testName: 'checklist-list', - resources: [], }); compareOutputToExpected({ testName: 'svg', - resources: [], }); compareOutputToExpected({ @@ -96,6 +99,10 @@ describe('EnexToHtml', function() { }], }); + compareOutputToExpected({ + testName: 'quoted-attributes', + }); + // it('fails when not given a matching resource', (async () => { // // To test the promise-unexpectedly-resolved case, add `audioResource` to the array. // const resources = []; diff --git a/packages/lib/resourceUtils.js b/packages/lib/resourceUtils.js index 51e6cbda3a..cab51da61f 100644 --- a/packages/lib/resourceUtils.js +++ b/packages/lib/resourceUtils.js @@ -39,7 +39,7 @@ const imageMimeTypes = [ 'image/vnd.xiff', ]; -const escapeQuotes = (str) => str.replace(/"/g, '"'); +const escapeQuotes = (str) => str.replace(/"/g, '"'); const attributesToStr = (attributes) => Object.entries(attributes) diff --git a/packages/renderer/htmlUtils.ts b/packages/renderer/htmlUtils.ts index 9a27fb31d5..0e24f21be2 100644 --- a/packages/renderer/htmlUtils.ts +++ b/packages/renderer/htmlUtils.ts @@ -192,6 +192,15 @@ class HtmlUtils { } } + // For some reason, entire parts of HTML notes don't show up in + // the viewer when there's an anchor tag without an "href" + // attribute. It doesn't always happen and it seems to depend on + // what else is in the note but in any case adding the "href" + // fixes it. https://github.com/laurent22/joplin/issues/5687 + if (name.toLowerCase() === 'a' && !attrs['href']) { + attrs['href'] = '#'; + } + let attrHtml = this.attributesHtml(attrs); if (attrHtml) attrHtml = ` ${attrHtml}`; const closingSign = this.isSelfClosingTag(name) ? '/>' : '>'; From 7431da9f3ad4791ecb10ad8b063ad85054a872de Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 10 Nov 2021 11:48:06 +0000 Subject: [PATCH 4/6] Server: Lazy-load storage drivers --- packages/server/schema.sqlite | Bin 319488 -> 319488 bytes packages/server/src/app.ts | 16 +-- .../src/migrations/20211105183559_storage.ts | 10 ++ packages/server/src/models/ItemModel.ts | 77 +++++++++---- packages/server/src/models/factory.ts | 20 +--- .../storage/StorageDriverDatabase.test.ts | 27 ++--- .../items/storage/StorageDriverFs.test.ts | 20 ++-- .../items/storage/StorageDriverMemory.test.ts | 20 ++-- .../items/storage/storageDriverFromConfig.ts | 14 +-- .../src/models/items/storage/testUtils.ts | 105 ++++++++++-------- .../server/src/services/database/types.ts | 4 + packages/server/src/tools/debugTools.ts | 6 +- packages/server/src/utils/setupAppContext.ts | 6 +- .../server/src/utils/testing/testUtils.ts | 17 +-- 14 files changed, 191 insertions(+), 151 deletions(-) diff --git a/packages/server/schema.sqlite b/packages/server/schema.sqlite index 79fb1e7e2aa6c397bd0a41014b65e95dded5517d..c43503892781f75603e6a015771151e0de620217 100644 GIT binary patch delta 1480 zcmZoTAlz_3c!P{IYb|Fc-@eI;(gtAGmdRn#Dqz;;$@S6-VAh<;OQco7teKO~L1m{; z{w}Q!7MnU*Swzx zVsFdj4q0umSpMW4vJiDSlb_2%T#!APM-JkutjVTwYG8F4lN011vgwms9!?0wHZgDcTmmNQ6<_5*T;nD+HMuM0sW#IJp&D8kZ-VnTNS2duKWNghd*A7$^G# z=1#wHo>7(?DHy>nAf#*C1I7pP1kGMF!C=t>g-r{%1-5f1FkOu&7&F@rFwro*6Pbv% zGk;(rA{7BWw`l>R!gj_0W>yE52MqiVw(~7uxyG*`$H>Z{&dM56S&(Y$l30?Mlvtb! zN(X#fAtX3d1Qs0-ShN5bJP?M$wgeWo0IeJb{;T{o{E~bJc+c?K@I-U}<6gq;#ct0L z%Av*fjIE!|h;=_}G|Ow2ob3i1SZW!kzum~9U~XUu%(_|hwh^rbWo64zy)iq2`kb}t9Pi~cixS(e8S~)$i zZ1v;^auD}bP3Dn@=t!7s4rRqpPLhY{h@IRe4>2Wb@_KoQj)=*(p<-d16k7D_JyOeZ z!-9fxLmh)7Esgz>oC-a>9m4}H{L9=!)5=^*5>s+B^Bi*WlM{0?@{3DUj1pB04OAk+ z3N8K3j19~}Lc#((4V-++4ay@^o&76(lCmwlLL$P`l$m`w37O5sz_6J);4VKeio(rr z<3D2vyd~;1BN*zJIW(205q8|_v?9_(4{0b;e#S&hd zCe+MN@Hg-wMr})A+>x)~Yvfv)WSC}>o);1B9pN8XVL$+gX~nbM0W!U=|0OY=}kY KcF)}`+f@LttORcW diff --git a/packages/server/src/app.ts b/packages/server/src/app.ts index 5e1bac201c..d3d25f30d1 100644 --- a/packages/server/src/app.ts +++ b/packages/server/src/app.ts @@ -5,7 +5,7 @@ import * as Koa from 'koa'; import * as fs from 'fs-extra'; import Logger, { LoggerWrapper, TargetType } from '@joplin/lib/Logger'; import config, { initConfig, runningInDocker } from './config'; -import { migrateLatest, waitForConnection, sqliteDefaultDir, latestMigration, DbConnection } from './db'; +import { migrateLatest, waitForConnection, sqliteDefaultDir, latestMigration } from './db'; import { AppContext, Env, KoaNext } from './utils/types'; import FsDriverNode from '@joplin/lib/fs-driver-node'; import routeHandler from './middleware/routeHandler'; @@ -17,11 +17,10 @@ import startServices from './utils/startServices'; import { credentialFile } from './utils/testing/testUtils'; import apiVersionHandler from './middleware/apiVersionHandler'; import clickJackingHandler from './middleware/clickJackingHandler'; -import newModelFactory, { Options } from './models/factory'; +import newModelFactory from './models/factory'; import setupCommands from './utils/setupCommands'; import { RouteResponseFormat, routeResponseFormat } from './utils/routeUtils'; import { parseEnv } from './env'; -import storageDriverFromConfig from './models/items/storage/storageDriverFromConfig'; interface Argv { env?: Env; @@ -222,13 +221,6 @@ async function main() { fs.writeFileSync(pidFile, `${process.pid}`); } - const newModelFactoryOptions = async (db: DbConnection): Promise => { - return { - storageDriver: await storageDriverFromConfig(config().storageDriver, db, { assignDriverId: env !== 'buildTypes' }), - storageDriverFallback: await storageDriverFromConfig(config().storageDriverFallback, db, { assignDriverId: env !== 'buildTypes' }), - }; - }; - let runCommandAndExitApp = true; if (selectedCommand) { @@ -245,7 +237,7 @@ async function main() { }); } else { const connectionCheck = await waitForConnection(config().database); - const models = newModelFactory(connectionCheck.connection, config(), await newModelFactoryOptions(connectionCheck.connection)); + const models = newModelFactory(connectionCheck.connection, config()); await selectedCommand.run(commandArgv, { db: connectionCheck.connection, @@ -275,7 +267,7 @@ async function main() { appLogger().info('Connection check:', connectionCheckLogInfo); const ctx = app.context as AppContext; - await setupAppContext(ctx, env, connectionCheck.connection, appLogger, await newModelFactoryOptions(connectionCheck.connection)); + await setupAppContext(ctx, env, connectionCheck.connection, appLogger); await initializeJoplinUtils(config(), ctx.joplinBase.models, ctx.joplinBase.services.mustache); diff --git a/packages/server/src/migrations/20211105183559_storage.ts b/packages/server/src/migrations/20211105183559_storage.ts index 94f6a05fa1..c2367f44a3 100644 --- a/packages/server/src/migrations/20211105183559_storage.ts +++ b/packages/server/src/migrations/20211105183559_storage.ts @@ -5,10 +5,16 @@ export async function up(db: DbConnection): Promise { await db.schema.createTable('storages', (table: Knex.CreateTableBuilder) => { table.increments('id').unique().primary().notNullable(); table.text('connection_string').notNullable(); + table.bigInteger('updated_time').notNullable(); + table.bigInteger('created_time').notNullable(); }); + const now = Date.now(); + await db('storages').insert({ connection_string: 'Type=Database', + updated_time: now, + created_time: now, }); // First we create the column and set a default so as to populate the @@ -21,6 +27,10 @@ export async function up(db: DbConnection): Promise { await db.schema.alterTable('items', (table: Knex.CreateTableBuilder) => { table.integer('content_storage_id').notNullable().alter(); }); + + await db.schema.alterTable('storages', (table: Knex.CreateTableBuilder) => { + table.unique(['connection_string']); + }); } export async function down(db: DbConnection): Promise { diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index d532532cb3..87680b83db 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -9,8 +9,9 @@ import { ChangePreviousItem } from './ChangeModel'; import { unique } from '../utils/array'; import StorageDriverBase, { Context } from './items/storage/StorageDriverBase'; import { DbConnection } from '../db'; -import { Config, StorageDriverMode } from '../utils/types'; -import { NewModelFactoryHandler, Options } from './factory'; +import { Config, StorageDriverConfig, StorageDriverMode } from '../utils/types'; +import { NewModelFactoryHandler } from './factory'; +import storageDriverFromConfig from './items/storage/storageDriverFromConfig'; const mimeUtils = require('@joplin/lib/mime-utils.js').mime; @@ -49,14 +50,16 @@ export interface ItemLoadOptions extends LoadOptions { export default class ItemModel extends BaseModel { private updatingTotalSizes_: boolean = false; - private storageDriver_: StorageDriverBase = null; - private storageDriverFallback_: StorageDriverBase = null; + private storageDriverConfig_: StorageDriverConfig; + private storageDriverConfigFallback_: StorageDriverConfig; - public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config, options: Options) { + private static storageDrivers_: Map = new Map(); + + public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) { super(db, modelFactory, config); - this.storageDriver_ = options.storageDriver; - this.storageDriverFallback_ = options.storageDriverFallback; + this.storageDriverConfig_ = config.storageDriver; + this.storageDriverConfigFallback_ = config.storageDriverFallback; } protected get tableName(): string { @@ -75,6 +78,26 @@ export default class ItemModel extends BaseModel { return Object.keys(databaseSchema[this.tableName]).filter(f => f !== 'content'); } + private async storageDriverFromConfig(config: StorageDriverConfig): Promise { + let driver = ItemModel.storageDrivers_.get(config); + + if (!driver) { + driver = await storageDriverFromConfig(config, this.db); + ItemModel.storageDrivers_.set(config, driver); + } + + return driver; + } + + public async storageDriver(): Promise { + return this.storageDriverFromConfig(this.storageDriverConfig_); + } + + public async storageDriverFallback(): Promise { + if (!this.storageDriverConfigFallback_) return null; + return this.storageDriverFromConfig(this.storageDriverConfigFallback_); + } + public async checkIfAllowed(user: User, action: AclAction, resource: Item = null): Promise { if (action === AclAction.Create) { if (!(await this.models().shareUser().isShareParticipant(resource.jop_share_id, user.id))) throw new ErrorForbidden('user has no access to this share'); @@ -136,25 +159,31 @@ export default class ItemModel extends BaseModel { } private async storageDriverWrite(itemId: Uuid, content: Buffer, context: Context) { - await this.storageDriver_.write(itemId, content, context); + const storageDriver = await this.storageDriver(); + const storageDriverFallback = await this.storageDriverFallback(); - if (this.storageDriverFallback_) { - if (this.storageDriverFallback_.mode === StorageDriverMode.ReadWrite) { - await this.storageDriverFallback_.write(itemId, content, context); - } else if (this.storageDriverFallback_.mode === StorageDriverMode.ReadOnly) { - await this.storageDriverFallback_.write(itemId, Buffer.from(''), context); + await storageDriver.write(itemId, content, context); + + if (storageDriverFallback) { + if (storageDriverFallback.mode === StorageDriverMode.ReadWrite) { + await storageDriverFallback.write(itemId, content, context); + } else if (storageDriverFallback.mode === StorageDriverMode.ReadOnly) { + await storageDriverFallback.write(itemId, Buffer.from(''), context); } else { - throw new Error(`Unsupported fallback mode: ${this.storageDriverFallback_.mode}`); + throw new Error(`Unsupported fallback mode: ${storageDriverFallback.mode}`); } } } private async storageDriverRead(itemId: Uuid, context: Context) { - if (await this.storageDriver_.exists(itemId, context)) { - return this.storageDriver_.read(itemId, context); + const storageDriver = await this.storageDriver(); + const storageDriverFallback = await this.storageDriverFallback(); + + if (await storageDriver.exists(itemId, context)) { + return storageDriver.read(itemId, context); } else { - if (!this.storageDriverFallback_) throw new Error(`Content does not exist but fallback content driver is not defined: ${itemId}`); - return this.storageDriverFallback_.read(itemId, context); + if (!storageDriverFallback) throw new Error(`Content does not exist but fallback content driver is not defined: ${itemId}`); + return storageDriverFallback.read(itemId, context); } } @@ -417,7 +446,8 @@ export default class ItemModel extends BaseModel { try { const content = itemToSave.content; delete itemToSave.content; - itemToSave.content_storage_id = this.storageDriver_.storageId; + + itemToSave.content_storage_id = (await this.storageDriver()).storageId; itemToSave.content_size = content ? content.byteLength : 0; @@ -624,14 +654,17 @@ export default class ItemModel extends BaseModel { const ids = typeof id === 'string' ? [id] : id; if (!ids.length) return; + const storageDriver = await this.storageDriver(); + const storageDriverFallback = await this.storageDriverFallback(); + const shares = await this.models().share().byItemIds(ids); await this.withTransaction(async () => { await this.models().share().delete(shares.map(s => s.id)); await this.models().userItem().deleteByItemIds(ids); await this.models().itemResource().deleteByItemIds(ids); - await this.storageDriver_.delete(ids, { models: this.models() }); - if (this.storageDriverFallback_) await this.storageDriverFallback_.delete(ids, { models: this.models() }); + await storageDriver.delete(ids, { models: this.models() }); + if (storageDriverFallback) await storageDriverFallback.delete(ids, { models: this.models() }); await super.delete(ids, options); }, 'ItemModel::delete'); @@ -679,7 +712,7 @@ export default class ItemModel extends BaseModel { let previousItem: ChangePreviousItem = null; if (item.content && !item.content_storage_id) { - item.content_storage_id = this.storageDriver_.storageId; + item.content_storage_id = (await this.storageDriver()).storageId; } if (isNew) { diff --git a/packages/server/src/models/factory.ts b/packages/server/src/models/factory.ts index 581394400c..4e410d8107 100644 --- a/packages/server/src/models/factory.ts +++ b/packages/server/src/models/factory.ts @@ -72,39 +72,29 @@ import SubscriptionModel from './SubscriptionModel'; import UserFlagModel from './UserFlagModel'; import EventModel from './EventModel'; import { Config } from '../utils/types'; -import StorageDriverBase from './items/storage/StorageDriverBase'; import LockModel from './LockModel'; import StorageModel from './StorageModel'; -export interface Options { - storageDriver: StorageDriverBase; - storageDriverFallback?: StorageDriverBase; -} - export type NewModelFactoryHandler = (db: DbConnection)=> Models; export class Models { private db_: DbConnection; private config_: Config; - private options_: Options; - public constructor(db: DbConnection, config: Config, options: Options) { + public constructor(db: DbConnection, config: Config) { this.db_ = db; this.config_ = config; - this.options_ = options; - - // if (!options.storageDriver) throw new Error('StorageDriver is required'); this.newModelFactory = this.newModelFactory.bind(this); } private newModelFactory(db: DbConnection) { - return new Models(db, this.config_, this.options_); + return new Models(db, this.config_); } public item() { - return new ItemModel(this.db_, this.newModelFactory, this.config_, this.options_); + return new ItemModel(this.db_, this.newModelFactory, this.config_); } public user() { @@ -177,6 +167,6 @@ export class Models { } -export default function newModelFactory(db: DbConnection, config: Config, options: Options): Models { - return new Models(db, config, options); +export default function newModelFactory(db: DbConnection, config: Config): Models { + return new Models(db, config); } diff --git a/packages/server/src/models/items/storage/StorageDriverDatabase.test.ts b/packages/server/src/models/items/storage/StorageDriverDatabase.test.ts index 026d9fb781..f4aeaab642 100644 --- a/packages/server/src/models/items/storage/StorageDriverDatabase.test.ts +++ b/packages/server/src/models/items/storage/StorageDriverDatabase.test.ts @@ -1,8 +1,7 @@ import { clientType } from '../../../db'; import { afterAllTests, beforeAllDb, beforeEachDb, db, expectNotThrow, expectThrow, models } from '../../../utils/testing/testUtils'; -import { StorageDriverMode } from '../../../utils/types'; +import { StorageDriverConfig, StorageDriverMode, StorageDriverType } from '../../../utils/types'; import StorageDriverDatabase from './StorageDriverDatabase'; -import StorageDriverMemory from './StorageDriverMemory'; import { shouldDeleteContent, shouldNotCreateItemIfContentNotSaved, shouldNotUpdateItemIfContentNotSaved, shouldSupportFallbackDriver, shouldSupportFallbackDriverInReadWriteMode, shouldUpdateContentStorageIdAfterSwitchingDriver, shouldWriteToContentAndReadItBack } from './testUtils'; const newDriver = () => { @@ -11,6 +10,12 @@ const newDriver = () => { }); }; +const newConfig = (): StorageDriverConfig => { + return { + type: StorageDriverType.Database, + }; +}; + describe('StorageDriverDatabase', function() { beforeAll(async () => { @@ -26,23 +31,19 @@ describe('StorageDriverDatabase', function() { }); test('should write to content and read it back', async function() { - const driver = newDriver(); - await shouldWriteToContentAndReadItBack(driver); + await shouldWriteToContentAndReadItBack(newConfig()); }); test('should delete the content', async function() { - const driver = newDriver(); - await shouldDeleteContent(driver); + await shouldDeleteContent(newConfig()); }); test('should not create the item if the content cannot be saved', async function() { - const driver = newDriver(); - await shouldNotCreateItemIfContentNotSaved(driver); + await shouldNotCreateItemIfContentNotSaved(newConfig()); }); test('should not update the item if the content cannot be saved', async function() { - const driver = newDriver(); - await shouldNotUpdateItemIfContentNotSaved(driver); + await shouldNotUpdateItemIfContentNotSaved(newConfig()); }); test('should fail if the item row does not exist', async function() { @@ -56,15 +57,15 @@ describe('StorageDriverDatabase', function() { }); test('should support fallback content drivers', async function() { - await shouldSupportFallbackDriver(newDriver(), new StorageDriverMemory(2)); + await shouldSupportFallbackDriver(newConfig(), { type: StorageDriverType.Memory }); }); test('should support fallback content drivers in rw mode', async function() { - await shouldSupportFallbackDriverInReadWriteMode(newDriver(), new StorageDriverMemory(2, { mode: StorageDriverMode.ReadWrite })); + await shouldSupportFallbackDriverInReadWriteMode(newConfig(), { type: StorageDriverType.Memory, mode: StorageDriverMode.ReadWrite }); }); test('should update content storage ID after switching driver', async function() { - await shouldUpdateContentStorageIdAfterSwitchingDriver(newDriver(), new StorageDriverMemory(2)); + await shouldUpdateContentStorageIdAfterSwitchingDriver(newConfig(), { type: StorageDriverType.Memory }); }); }); diff --git a/packages/server/src/models/items/storage/StorageDriverFs.test.ts b/packages/server/src/models/items/storage/StorageDriverFs.test.ts index 74ecac39e2..5bbab1748c 100644 --- a/packages/server/src/models/items/storage/StorageDriverFs.test.ts +++ b/packages/server/src/models/items/storage/StorageDriverFs.test.ts @@ -1,5 +1,6 @@ import { pathExists, remove } from 'fs-extra'; import { afterAllTests, beforeAllDb, beforeEachDb, expectNotThrow, expectThrow, tempDirPath } from '../../../utils/testing/testUtils'; +import { StorageDriverConfig, StorageDriverType } from '../../../utils/types'; import StorageDriverFs from './StorageDriverFs'; import { shouldDeleteContent, shouldNotCreateItemIfContentNotSaved, shouldNotUpdateItemIfContentNotSaved, shouldWriteToContentAndReadItBack } from './testUtils'; @@ -9,6 +10,13 @@ const newDriver = () => { return new StorageDriverFs(1, { path: basePath_ }); }; +const newConfig = (): StorageDriverConfig => { + return { + type: StorageDriverType.Filesystem, + path: basePath_, + }; +}; + describe('StorageDriverFs', function() { beforeAll(async () => { @@ -30,23 +38,19 @@ describe('StorageDriverFs', function() { }); test('should write to content and read it back', async function() { - const driver = newDriver(); - await shouldWriteToContentAndReadItBack(driver); + await shouldWriteToContentAndReadItBack(newConfig()); }); test('should delete the content', async function() { - const driver = newDriver(); - await shouldDeleteContent(driver); + await shouldDeleteContent(newConfig()); }); test('should not create the item if the content cannot be saved', async function() { - const driver = newDriver(); - await shouldNotCreateItemIfContentNotSaved(driver); + await shouldNotCreateItemIfContentNotSaved(newConfig()); }); test('should not update the item if the content cannot be saved', async function() { - const driver = newDriver(); - await shouldNotUpdateItemIfContentNotSaved(driver); + await shouldNotUpdateItemIfContentNotSaved(newConfig()); }); test('should write to a file and read it back', async function() { diff --git a/packages/server/src/models/items/storage/StorageDriverMemory.test.ts b/packages/server/src/models/items/storage/StorageDriverMemory.test.ts index d382f76b93..7a8bccef65 100644 --- a/packages/server/src/models/items/storage/StorageDriverMemory.test.ts +++ b/packages/server/src/models/items/storage/StorageDriverMemory.test.ts @@ -1,7 +1,13 @@ import { afterAllTests, beforeAllDb, beforeEachDb } from '../../../utils/testing/testUtils'; -import StorageDriverMemory from './StorageDriverMemory'; +import { StorageDriverConfig, StorageDriverType } from '../../../utils/types'; import { shouldDeleteContent, shouldNotCreateItemIfContentNotSaved, shouldNotUpdateItemIfContentNotSaved, shouldWriteToContentAndReadItBack } from './testUtils'; +const newConfig = (): StorageDriverConfig => { + return { + type: StorageDriverType.Memory, + }; +}; + describe('StorageDriverMemory', function() { beforeAll(async () => { @@ -17,23 +23,19 @@ describe('StorageDriverMemory', function() { }); test('should write to content and read it back', async function() { - const driver = new StorageDriverMemory(1); - await shouldWriteToContentAndReadItBack(driver); + await shouldWriteToContentAndReadItBack(newConfig()); }); test('should delete the content', async function() { - const driver = new StorageDriverMemory(1); - await shouldDeleteContent(driver); + await shouldDeleteContent(newConfig()); }); test('should not create the item if the content cannot be saved', async function() { - const driver = new StorageDriverMemory(1); - await shouldNotCreateItemIfContentNotSaved(driver); + await shouldNotCreateItemIfContentNotSaved(newConfig()); }); test('should not update the item if the content cannot be saved', async function() { - const driver = new StorageDriverMemory(1); - await shouldNotUpdateItemIfContentNotSaved(driver); + await shouldNotUpdateItemIfContentNotSaved(newConfig()); }); }); diff --git a/packages/server/src/models/items/storage/storageDriverFromConfig.ts b/packages/server/src/models/items/storage/storageDriverFromConfig.ts index c6db08f688..0c168a8feb 100644 --- a/packages/server/src/models/items/storage/storageDriverFromConfig.ts +++ b/packages/server/src/models/items/storage/storageDriverFromConfig.ts @@ -23,19 +23,19 @@ export default async function(config: StorageDriverConfig, db: DbConnection, opt let storageId: number = 0; if (options.assignDriverId) { - const models = newModelFactory(db, globalConfig(), { storageDriver: null }); + const models = newModelFactory(db, globalConfig()); const connectionString = serializeStorageConfig(config); - const existingStorage = await models.storage().byConnectionString(connectionString); + let storage = await models.storage().byConnectionString(connectionString); - if (existingStorage) { - storageId = existingStorage.id; - } else { - const storage = await models.storage().save({ + if (!storage) { + await models.storage().save({ connection_string: connectionString, }); - storageId = storage.id; + storage = await models.storage().byConnectionString(connectionString); } + + storageId = storage.id; } if (config.type === StorageDriverType.Database) { diff --git a/packages/server/src/models/items/storage/testUtils.ts b/packages/server/src/models/items/storage/testUtils.ts index 8f72f58983..55224e8664 100644 --- a/packages/server/src/models/items/storage/testUtils.ts +++ b/packages/server/src/models/items/storage/testUtils.ts @@ -1,20 +1,30 @@ +import config from '../../../config'; import { Item } from '../../../services/database/types'; -import { createUserAndSession, makeNoteSerializedBody, models } from '../../../utils/testing/testUtils'; -import { StorageDriverMode } from '../../../utils/types'; -import StorageDriverBase, { Context } from './StorageDriverBase'; +import { createUserAndSession, db, makeNoteSerializedBody, models } from '../../../utils/testing/testUtils'; +import { Config, StorageDriverConfig, StorageDriverMode } from '../../../utils/types'; +import newModelFactory from '../../factory'; +import { Context } from './StorageDriverBase'; -const testModels = (driver: StorageDriverBase) => { - return models({ storageDriver: driver }); +const newTestModels = (driverConfig: StorageDriverConfig, driverConfigFallback: StorageDriverConfig = null) => { + const newConfig: Config = { + ...config(), + storageDriver: driverConfig, + storageDriverFallback: driverConfigFallback, + }; + return newModelFactory(db(), newConfig); }; -export async function shouldWriteToContentAndReadItBack(driver: StorageDriverBase) { +export async function shouldWriteToContentAndReadItBack(driverConfig: StorageDriverConfig) { const { user } = await createUserAndSession(1); const noteBody = makeNoteSerializedBody({ id: '00000000000000000000000000000001', title: 'testing driver', }); - const output = await testModels(driver).item().saveFromRawContent(user, [{ + const testModels = newTestModels(driverConfig); + const driver = await testModels.item().storageDriver(); + + const output = await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(noteBody), }]); @@ -22,38 +32,43 @@ export async function shouldWriteToContentAndReadItBack(driver: StorageDriverBas const result = output['00000000000000000000000000000001.md']; expect(result.error).toBeFalsy(); - const item = await testModels(driver).item().loadWithContent(result.item.id); + const item = await testModels.item().loadWithContent(result.item.id); expect(item.content.byteLength).toBe(item.content_size); expect(item.content_storage_id).toBe(driver.storageId); const rawContent = await driver.read(item.id, { models: models() }); expect(rawContent.byteLength).toBe(item.content_size); - const jopItem = testModels(driver).item().itemToJoplinItem(item); + const jopItem = testModels.item().itemToJoplinItem(item); expect(jopItem.id).toBe('00000000000000000000000000000001'); expect(jopItem.title).toBe('testing driver'); } -export async function shouldDeleteContent(driver: StorageDriverBase) { +export async function shouldDeleteContent(driverConfig: StorageDriverConfig) { const { user } = await createUserAndSession(1); const noteBody = makeNoteSerializedBody({ id: '00000000000000000000000000000001', title: 'testing driver', }); - const output = await testModels(driver).item().saveFromRawContent(user, [{ + const testModels = newTestModels(driverConfig); + + const output = await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(noteBody), }]); const item: Item = output['00000000000000000000000000000001.md'].item; - expect((await testModels(driver).item().all()).length).toBe(1); - await testModels(driver).item().delete(item.id); - expect((await testModels(driver).item().all()).length).toBe(0); + expect((await testModels.item().all()).length).toBe(1); + await testModels.item().delete(item.id); + expect((await testModels.item().all()).length).toBe(0); } -export async function shouldNotCreateItemIfContentNotSaved(driver: StorageDriverBase) { +export async function shouldNotCreateItemIfContentNotSaved(driverConfig: StorageDriverConfig) { + const testModels = newTestModels(driverConfig); + const driver = await testModels.item().storageDriver(); + const previousWrite = driver.write; driver.write = () => { throw new Error('not working!'); }; @@ -64,26 +79,29 @@ export async function shouldNotCreateItemIfContentNotSaved(driver: StorageDriver title: 'testing driver', }); - const output = await testModels(driver).item().saveFromRawContent(user, [{ + const output = await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(noteBody), }]); expect(output['00000000000000000000000000000001.md'].error.message).toBe('not working!'); - expect((await testModels(driver).item().all()).length).toBe(0); + expect((await testModels.item().all()).length).toBe(0); } finally { driver.write = previousWrite; } } -export async function shouldNotUpdateItemIfContentNotSaved(driver: StorageDriverBase) { +export async function shouldNotUpdateItemIfContentNotSaved(driverConfig: StorageDriverConfig) { const { user } = await createUserAndSession(1); const noteBody = makeNoteSerializedBody({ id: '00000000000000000000000000000001', title: 'testing driver', }); - await testModels(driver).item().saveFromRawContent(user, [{ + const testModels = newTestModels(driverConfig); + const driver = await testModels.item().storageDriver(); + + await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(noteBody), }]); @@ -93,12 +111,12 @@ export async function shouldNotUpdateItemIfContentNotSaved(driver: StorageDriver title: 'updated 1', }); - await testModels(driver).item().saveFromRawContent(user, [{ + await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(noteBodyMod1), }]); - const itemMod1 = testModels(driver).item().itemToJoplinItem(await testModels(driver).item().loadByJopId(user.id, '00000000000000000000000000000001', { withContent: true })); + const itemMod1 = testModels.item().itemToJoplinItem(await testModels.item().loadByJopId(user.id, '00000000000000000000000000000001', { withContent: true })); expect(itemMod1.title).toBe('updated 1'); const noteBodyMod2 = makeNoteSerializedBody({ @@ -110,23 +128,26 @@ export async function shouldNotUpdateItemIfContentNotSaved(driver: StorageDriver driver.write = () => { throw new Error('not working!'); }; try { - const output = await testModels(driver).item().saveFromRawContent(user, [{ + const output = await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(noteBodyMod2), }]); expect(output['00000000000000000000000000000001.md'].error.message).toBe('not working!'); - const itemMod2 = testModels(driver).item().itemToJoplinItem(await testModels(driver).item().loadByJopId(user.id, '00000000000000000000000000000001', { withContent: true })); + const itemMod2 = testModels.item().itemToJoplinItem(await testModels.item().loadByJopId(user.id, '00000000000000000000000000000001', { withContent: true })); expect(itemMod2.title).toBe('updated 1'); // Check it has not been updated } finally { driver.write = previousWrite; } } -export async function shouldSupportFallbackDriver(driver: StorageDriverBase, fallbackDriver: StorageDriverBase) { +export async function shouldSupportFallbackDriver(driverConfig: StorageDriverConfig, fallbackDriverConfig: StorageDriverConfig) { const { user } = await createUserAndSession(1); - const output = await testModels(driver).item().saveFromRawContent(user, [{ + const testModels = newTestModels(driverConfig); + const driver = await testModels.item().storageDriver(); + + const output = await testModels.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', body: Buffer.from(makeNoteSerializedBody({ id: '00000000000000000000000000000001', @@ -144,10 +165,7 @@ export async function shouldSupportFallbackDriver(driver: StorageDriverBase, fal previousByteLength = content.byteLength; } - const testModelWithFallback = models({ - storageDriver: driver, - storageDriverFallback: fallbackDriver, - }); + const testModelWithFallback = newTestModels(driverConfig, fallbackDriverConfig); // If the item content is not on the main content driver, it should get // it from the fallback one. @@ -165,6 +183,8 @@ export async function shouldSupportFallbackDriver(driver: StorageDriverBase, fal }]); { + const fallbackDriver = await testModelWithFallback.item().storageDriverFallback(); + // Check that it has cleared the fallback driver content const context: Context = { models: models() }; const fallbackContent = await fallbackDriver.read(itemId, context); @@ -176,15 +196,12 @@ export async function shouldSupportFallbackDriver(driver: StorageDriverBase, fal } } -export async function shouldSupportFallbackDriverInReadWriteMode(driver: StorageDriverBase, fallbackDriver: StorageDriverBase) { - if (fallbackDriver.mode !== StorageDriverMode.ReadWrite) throw new Error('Content driver must be configured in RW mode for this test'); +export async function shouldSupportFallbackDriverInReadWriteMode(driverConfig: StorageDriverConfig, fallbackDriverConfig: StorageDriverConfig) { + if (fallbackDriverConfig.mode !== StorageDriverMode.ReadWrite) throw new Error('Content driver must be configured in RW mode for this test'); const { user } = await createUserAndSession(1); - const testModelWithFallback = models({ - storageDriver: driver, - storageDriverFallback: fallbackDriver, - }); + const testModelWithFallback = newTestModels(driverConfig, fallbackDriverConfig); const output = await testModelWithFallback.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', @@ -197,6 +214,9 @@ export async function shouldSupportFallbackDriverInReadWriteMode(driver: Storage const itemId = output['00000000000000000000000000000001.md'].item.id; { + const driver = await testModelWithFallback.item().storageDriver(); + const fallbackDriver = await testModelWithFallback.item().storageDriverFallback(); + // Check that it has written the content to both drivers const context: Context = { models: models() }; const fallbackContent = await fallbackDriver.read(itemId, context); @@ -207,18 +227,15 @@ export async function shouldSupportFallbackDriverInReadWriteMode(driver: Storage } } -export async function shouldUpdateContentStorageIdAfterSwitchingDriver(oldDriver: StorageDriverBase, newDriver: StorageDriverBase) { - if (oldDriver.storageId === newDriver.storageId) throw new Error('Drivers must be different for this test'); +export async function shouldUpdateContentStorageIdAfterSwitchingDriver(oldDriverConfig: StorageDriverConfig, newDriverConfig: StorageDriverConfig) { + if (oldDriverConfig.type === newDriverConfig.type) throw new Error('Drivers must be different for this test'); const { user } = await createUserAndSession(1); - const oldDriverModel = models({ - storageDriver: oldDriver, - }); - - const newDriverModel = models({ - storageDriver: newDriver, - }); + const oldDriverModel = newTestModels(oldDriverConfig); + const newDriverModel = newTestModels(newDriverConfig); + const oldDriver = await oldDriverModel.item().storageDriver(); + const newDriver = await newDriverModel.item().storageDriver(); const output = await oldDriverModel.item().saveFromRawContent(user, [{ name: '00000000000000000000000000000001.md', diff --git a/packages/server/src/services/database/types.ts b/packages/server/src/services/database/types.ts index f9e25ad5fe..e7109a1315 100644 --- a/packages/server/src/services/database/types.ts +++ b/packages/server/src/services/database/types.ts @@ -249,6 +249,8 @@ export interface Event extends WithUuid { export interface Storage { id?: number; connection_string?: string; + updated_time?: string; + created_time?: string; } export interface Item extends WithDates, WithUuid { @@ -427,6 +429,8 @@ export const databaseSchema: DatabaseTables = { storages: { id: { type: 'number' }, connection_string: { type: 'string' }, + updated_time: { type: 'string' }, + created_time: { type: 'string' }, }, items: { id: { type: 'string' }, diff --git a/packages/server/src/tools/debugTools.ts b/packages/server/src/tools/debugTools.ts index bfc8f71ec5..6737e5a691 100644 --- a/packages/server/src/tools/debugTools.ts +++ b/packages/server/src/tools/debugTools.ts @@ -1,7 +1,6 @@ import time from '@joplin/lib/time'; import { DbConnection, dropTables, migrateLatest } from '../db'; import newModelFactory from '../models/factory'; -import storageDriverFromConfig from '../models/items/storage/storageDriverFromConfig'; import { AccountType } from '../models/UserModel'; import { User, UserFlagType } from '../services/database/types'; import { Config } from '../utils/types'; @@ -35,10 +34,7 @@ export async function createTestUsers(db: DbConnection, config: Config, options: const password = 'hunter1hunter2hunter3'; - const models = newModelFactory(db, config, { - // storageDriver: new StorageDriverDatabase(1, { dbClientType: clientType(db) }), - storageDriver: await storageDriverFromConfig(config.storageDriver, db), // new StorageDriverDatabase(1, { dbClientType: clientType(db) }), - }); + const models = newModelFactory(db, config); if (options.count) { const users: User[] = []; diff --git a/packages/server/src/utils/setupAppContext.ts b/packages/server/src/utils/setupAppContext.ts index f53c9a4a11..d3dce8e655 100644 --- a/packages/server/src/utils/setupAppContext.ts +++ b/packages/server/src/utils/setupAppContext.ts @@ -1,7 +1,7 @@ import { LoggerWrapper } from '@joplin/lib/Logger'; import config from '../config'; import { DbConnection } from '../db'; -import newModelFactory, { Models, Options as ModelFactoryOptions } from '../models/factory'; +import newModelFactory, { Models } from '../models/factory'; import { AppContext, Config, Env } from './types'; import routes from '../routes/routes'; import ShareService from '../services/ShareService'; @@ -23,8 +23,8 @@ async function setupServices(env: Env, models: Models, config: Config): Promise< return output; } -export default async function(appContext: AppContext, env: Env, dbConnection: DbConnection, appLogger: ()=> LoggerWrapper, options: ModelFactoryOptions): Promise { - const models = newModelFactory(dbConnection, config(), options); +export default async function(appContext: AppContext, env: Env, dbConnection: DbConnection, appLogger: ()=> LoggerWrapper): Promise { + const models = newModelFactory(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/testing/testUtils.ts b/packages/server/src/utils/testing/testUtils.ts index c6f936c877..0f227cbb21 100644 --- a/packages/server/src/utils/testing/testUtils.ts +++ b/packages/server/src/utils/testing/testUtils.ts @@ -1,7 +1,7 @@ import { DbConnection, connectDb, disconnectDb, truncateTables } from '../../db'; import { User, Session, Item, Uuid } from '../../services/database/types'; import { createDb, CreateDbOptions } from '../../tools/dbTools'; -import modelFactory, { Options as ModelFactoryOptions } from '../../models/factory'; +import modelFactory from '../../models/factory'; import { AppContext, Env } from '../types'; import config, { initConfig } from '../../config'; import Logger from '@joplin/lib/Logger'; @@ -23,7 +23,6 @@ import MustacheService from '../../services/MustacheService'; import uuidgen from '../uuidgen'; import { createCsrfToken } from '../csrf'; import { cookieSet } from '../cookies'; -import StorageDriverMemory from '../../models/items/storage/StorageDriverMemory'; import { parseEnv } from '../../env'; // Takes into account the fact that this file will be inside the /dist directory @@ -195,7 +194,7 @@ export async function koaAppContext(options: AppContextTestOptions = null): Prom const appLogger = Logger.create('AppTest'); - const baseAppContext = await setupAppContext({} as any, Env.Dev, db_, () => appLogger, { storageDriver: new StorageDriverMemory(1) }); + const baseAppContext = await setupAppContext({} as any, Env.Dev, db_, () => appLogger); // Set type to "any" because the Koa context has many properties and we // don't need to mock all of them. @@ -243,16 +242,8 @@ export function db() { return db_; } -const storageDriverMemory = new StorageDriverMemory(1); - -export function models(options: ModelFactoryOptions = null) { - options = { - storageDriver: storageDriverMemory, - storageDriverFallback: null, - ...options, - }; - - return modelFactory(db(), config(), options); +export function models() { + return modelFactory(db(), config()); } export function parseHtml(html: string): Document { From 9ffa29f658350e674bde5dec0a9f8232ce4afe4c Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 10 Nov 2021 12:03:45 +0000 Subject: [PATCH 5/6] Chore: Add tests for storage driver loading --- packages/server/src/models/ItemModel.ts | 10 ++-- .../items/storage/loadStorageDriver.test.ts | 50 +++++++++++++++++++ ...iverFromConfig.ts => loadStorageDriver.ts} | 0 .../server/src/utils/testing/testUtils.ts | 2 +- 4 files changed, 56 insertions(+), 6 deletions(-) create mode 100644 packages/server/src/models/items/storage/loadStorageDriver.test.ts rename packages/server/src/models/items/storage/{storageDriverFromConfig.ts => loadStorageDriver.ts} (100%) diff --git a/packages/server/src/models/ItemModel.ts b/packages/server/src/models/ItemModel.ts index 87680b83db..a5f4bee4f7 100644 --- a/packages/server/src/models/ItemModel.ts +++ b/packages/server/src/models/ItemModel.ts @@ -11,7 +11,7 @@ import StorageDriverBase, { Context } from './items/storage/StorageDriverBase'; import { DbConnection } from '../db'; import { Config, StorageDriverConfig, StorageDriverMode } from '../utils/types'; import { NewModelFactoryHandler } from './factory'; -import storageDriverFromConfig from './items/storage/storageDriverFromConfig'; +import loadStorageDriver from './items/storage/loadStorageDriver'; const mimeUtils = require('@joplin/lib/mime-utils.js').mime; @@ -78,11 +78,11 @@ export default class ItemModel extends BaseModel { return Object.keys(databaseSchema[this.tableName]).filter(f => f !== 'content'); } - private async storageDriverFromConfig(config: StorageDriverConfig): Promise { + private async loadStorageDriver(config: StorageDriverConfig): Promise { let driver = ItemModel.storageDrivers_.get(config); if (!driver) { - driver = await storageDriverFromConfig(config, this.db); + driver = await loadStorageDriver(config, this.db); ItemModel.storageDrivers_.set(config, driver); } @@ -90,12 +90,12 @@ export default class ItemModel extends BaseModel { } public async storageDriver(): Promise { - return this.storageDriverFromConfig(this.storageDriverConfig_); + return this.loadStorageDriver(this.storageDriverConfig_); } public async storageDriverFallback(): Promise { if (!this.storageDriverConfigFallback_) return null; - return this.storageDriverFromConfig(this.storageDriverConfigFallback_); + return this.loadStorageDriver(this.storageDriverConfigFallback_); } public async checkIfAllowed(user: User, action: AclAction, resource: Item = null): Promise { diff --git a/packages/server/src/models/items/storage/loadStorageDriver.test.ts b/packages/server/src/models/items/storage/loadStorageDriver.test.ts new file mode 100644 index 0000000000..49bad61a14 --- /dev/null +++ b/packages/server/src/models/items/storage/loadStorageDriver.test.ts @@ -0,0 +1,50 @@ +import { afterAllTests, beforeAllDb, beforeEachDb, db, expectThrow, models } from '../../../utils/testing/testUtils'; +import { StorageDriverType } from '../../../utils/types'; +import loadStorageDriver from './loadStorageDriver'; + +describe('loadStorageDriver', function() { + + beforeAll(async () => { + await beforeAllDb('loadStorageDriver'); + }); + + afterAll(async () => { + await afterAllTests(); + }); + + beforeEach(async () => { + await beforeEachDb(); + }); + + test('should load a driver and assign an ID to it', async function() { + { + const newDriver = await loadStorageDriver({ type: StorageDriverType.Memory }, db()); + expect(newDriver.storageId).toBe(1); + expect((await models().storage().count())).toBe(1); + } + + { + const newDriver = await loadStorageDriver({ type: StorageDriverType.Filesystem, path: '/just/testing' }, db()); + expect(newDriver.storageId).toBe(2); + expect((await models().storage().count())).toBe(2); + } + }); + + test('should not record the same storage connection twice', async function() { + await db()('storages').insert({ + connection_string: 'Type=Database', + updated_time: Date.now(), + created_time: Date.now(), + }); + + await expectThrow(async () => + await db()('storages').insert({ + connection_string: 'Type=Database', + updated_time: Date.now(), + created_time: Date.now(), + }) + ); + }); + +}); + diff --git a/packages/server/src/models/items/storage/storageDriverFromConfig.ts b/packages/server/src/models/items/storage/loadStorageDriver.ts similarity index 100% rename from packages/server/src/models/items/storage/storageDriverFromConfig.ts rename to packages/server/src/models/items/storage/loadStorageDriver.ts diff --git a/packages/server/src/utils/testing/testUtils.ts b/packages/server/src/utils/testing/testUtils.ts index 0f227cbb21..f4113aa05a 100644 --- a/packages/server/src/utils/testing/testUtils.ts +++ b/packages/server/src/utils/testing/testUtils.ts @@ -436,7 +436,7 @@ export async function expectThrow(asyncFn: Function, errorCode: any = undefined) if (!hasThrown) { expect('not throw').toBe('throw'); - } else if (thrownError.code !== errorCode) { + } else if (errorCode !== undefined && thrownError.code !== errorCode) { console.error(thrownError); expect(`error code: ${thrownError.code}`).toBe(`error code: ${errorCode}`); } else { From cc23a8b70b1537fd825e4e359f291f9e66019d45 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Tue, 9 Nov 2021 16:12:29 +0000 Subject: [PATCH 6/6] Server v2.6.4 --- packages/server/package-lock.json | 4 ++-- packages/server/package.json | 2 +- readme/changelog_server.md | 4 ++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/server/package-lock.json b/packages/server/package-lock.json index 4bc2c672c6..9f85096268 100644 --- a/packages/server/package-lock.json +++ b/packages/server/package-lock.json @@ -1,12 +1,12 @@ { "name": "@joplin/server", - "version": "2.6.3", + "version": "2.6.4", "lockfileVersion": 2, "requires": true, "packages": { "": { "name": "@joplin/server", - "version": "2.6.3", + "version": "2.6.4", "dependencies": { "@aws-sdk/client-s3": "^3.40.0", "@fortawesome/fontawesome-free": "^5.15.1", diff --git a/packages/server/package.json b/packages/server/package.json index 44e82e0605..93230adb3c 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -1,6 +1,6 @@ { "name": "@joplin/server", - "version": "2.6.3", + "version": "2.6.4", "private": true, "scripts": { "start-dev": "npm run build && JOPLIN_IS_TESTING=1 nodemon --config nodemon.json --ext ts,js,mustache,css,tsx dist/app.js --env dev", diff --git a/readme/changelog_server.md b/readme/changelog_server.md index d66aa7bb3c..98bdfa16a7 100644 --- a/readme/changelog_server.md +++ b/readme/changelog_server.md @@ -1,5 +1,9 @@ # Joplin Server Changelog +## [server-v2.6.4](https://github.com/laurent22/joplin/releases/tag/server-v2.6.4) - 2021-11-09T16:12:09Z + +- Improved: Allow storing item content in database, filesystem or S3 (depending on config) (#5602) + ## [server-v2.6.3](https://github.com/laurent22/joplin/releases/tag/server-v2.6.3) - 2021-11-08T15:20:41Z - New: Add support for sharing notes when E2EE is enabled (#5529)