Merge branch 'dev' into release-2.6

packages/app-cli/tests/enex_to_html/quoted-attributes.enex

@@ -0,0 +1,3 @@
+<en-note>
+  <h1 style="box-sizing:inherit;font-family:&quot;Guardian TextSans Web&quot;, &quot;Helvetica Neue&quot;, Helvetica, Arial, sans-serif;margin-top:0.2em;margin-bottom:0.35em;font-size:2.125em;font-weight:600;line-height:1.3;">Association Between mRNA Vaccination and COVID-19 Hospitalization and Disease Severity</h1>
+</en-note>

packages/app-cli/tests/enex_to_html/quoted-attributes.html

@@ -0,0 +1,3 @@
+<en-note>
+  <h1 style="box-sizing:inherit;font-family:&quot;Guardian TextSans Web&quot;, &quot;Helvetica Neue&quot;, Helvetica, Arial, sans-serif;margin-top:0.2em;margin-bottom:0.35em;font-size:2.125em;font-weight:600;line-height:1.3;">Association Between mRNA Vaccination and COVID-19 Hospitalization and Disease Severity</h1>
+</en-note>

packages/app-cli/tests/enex_to_md/highlight.html

@@ -1 +1,3 @@
 <span style="background-color: rgb(255, 250, 165);-evernote-highlight:true;">I&apos;ll highlight some text.</span>
+<br/>
+<span style="--en-highlight:yellow;background-color: #ffef9e;">this text is yellow</span>

packages/app-cli/tests/enex_to_md/highlight.md

@@ -1 +1,2 @@
 ==I'll highlight some text.==
+==this text is yellow==

packages/app-cli/tests/html_to_html/anchor_missing_href.dest.html

@@ -0,0 +1 @@
+<a data-from-md href='#'>test</a>

packages/app-cli/tests/html_to_html/anchor_missing_href.src.html

@@ -0,0 +1 @@
+<a>test</a>

packages/lib/import-enex-html-gen.test.js

@@ -0,0 +1,122 @@
+
+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
+ * `<note>...</note>` node in an `.enex` file already extracted from
+ * `<content><![CDATA[...]]</content>`.
+ */
+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}`;
+
+	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',
+	});
+
+	compareOutputToExpected({
+		testName: 'svg',
+	});
+
+	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,
+		}],
+	});
+
+	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 = [];
+	// 	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');
+	// 	});
+	// }));
+
+});

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;
+
+		for (const propName of propNames) {
 			const prop = o.stylesheet.rules[0].declarations.find((d: any) => d.property.toLowerCase() === propName);
-		return prop && prop.value ? prop.value.trim().toLowerCase() : null;
+			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;
 }

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)

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) ? '/>' : '>';

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<Options> => {
-		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);
 

packages/server/src/migrations/20211105183559_storage.ts

@@ -5,10 +5,16 @@ export async function up(db: DbConnection): Promise<any> {
 	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<any> {
 	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<any> {

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 { Config, StorageDriverConfig, StorageDriverMode } from '../utils/types';
-import { NewModelFactoryHandler, Options } from './factory';
+import { NewModelFactoryHandler } from './factory';
+import loadStorageDriver from './items/storage/loadStorageDriver';
 
 const mimeUtils = require('@joplin/lib/mime-utils.js').mime;
 
@@ -49,14 +50,16 @@ export interface ItemLoadOptions extends LoadOptions {
 export default class ItemModel extends BaseModel<Item> {
 
 	private updatingTotalSizes_: boolean = false;
-	private storageDriver_: StorageDriverBase = null;
+	private storageDriverConfig_: StorageDriverConfig;
-	private storageDriverFallback_: StorageDriverBase = null;
+	private storageDriverConfigFallback_: StorageDriverConfig;
 
-	public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config, options: Options) {
+	private static storageDrivers_: Map<StorageDriverConfig, StorageDriverBase> = new Map();
+
+	public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) {
 		super(db, modelFactory, config);
 
-		this.storageDriver_ = options.storageDriver;
+		this.storageDriverConfig_ = config.storageDriver;
-		this.storageDriverFallback_ = options.storageDriverFallback;
+		this.storageDriverConfigFallback_ = config.storageDriverFallback;
 	}
 
 	protected get tableName(): string {
@@ -75,6 +78,26 @@ export default class ItemModel extends BaseModel<Item> {
 		return Object.keys(databaseSchema[this.tableName]).filter(f => f !== 'content');
 	}
 
+	private async loadStorageDriver(config: StorageDriverConfig): Promise<StorageDriverBase> {
+		let driver = ItemModel.storageDrivers_.get(config);
+
+		if (!driver) {
+			driver = await loadStorageDriver(config, this.db);
+			ItemModel.storageDrivers_.set(config, driver);
+		}
+
+		return driver;
+	}
+
+	public async storageDriver(): Promise<StorageDriverBase> {
+		return this.loadStorageDriver(this.storageDriverConfig_);
+	}
+
+	public async storageDriverFallback(): Promise<StorageDriverBase> {
+		if (!this.storageDriverConfigFallback_) return null;
+		return this.loadStorageDriver(this.storageDriverConfigFallback_);
+	}
+
 	public async checkIfAllowed(user: User, action: AclAction, resource: Item = null): Promise<void> {
 		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<Item> {
 	}
 
 	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_) {
+		await storageDriver.write(itemId, content, context);
-			if (this.storageDriverFallback_.mode === StorageDriverMode.ReadWrite) {
+
-				await this.storageDriverFallback_.write(itemId, content, context);
+		if (storageDriverFallback) {
-			} else if (this.storageDriverFallback_.mode === StorageDriverMode.ReadOnly) {
+			if (storageDriverFallback.mode === StorageDriverMode.ReadWrite) {
-				await this.storageDriverFallback_.write(itemId, Buffer.from(''), context);
+				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)) {
+		const storageDriver = await this.storageDriver();
-			return this.storageDriver_.read(itemId, context);
+		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}`);
+			if (!storageDriverFallback) throw new Error(`Content does not exist but fallback content driver is not defined: ${itemId}`);
-			return this.storageDriverFallback_.read(itemId, context);
+			return storageDriverFallback.read(itemId, context);
 		}
 	}
 
@@ -417,7 +446,8 @@ export default class ItemModel extends BaseModel<Item> {
 				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<Item> {
 		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() });
+			await storageDriver.delete(ids, { models: this.models() });
-			if (this.storageDriverFallback_) await this.storageDriverFallback_.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<Item> {
 		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) {

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 {
+export default function newModelFactory(db: DbConnection, config: Config): Models {
-	return new Models(db, config, options);
+	return new Models(db, config);
 }

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(newConfig());
-		await shouldWriteToContentAndReadItBack(driver);
 	});
 
 	test('should delete the content', async function() {
-		const driver = newDriver();
+		await shouldDeleteContent(newConfig());
-		await shouldDeleteContent(driver);
 	});
 
 	test('should not create the item if the content cannot be saved', async function() {
-		const driver = newDriver();
+		await shouldNotCreateItemIfContentNotSaved(newConfig());
-		await shouldNotCreateItemIfContentNotSaved(driver);
 	});
 
 	test('should not update the item if the content cannot be saved', async function() {
-		const driver = newDriver();
+		await shouldNotUpdateItemIfContentNotSaved(newConfig());
-		await shouldNotUpdateItemIfContentNotSaved(driver);
 	});
 
 	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 });
 	});
 
 });

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(newConfig());
-		await shouldWriteToContentAndReadItBack(driver);
 	});
 
 	test('should delete the content', async function() {
-		const driver = newDriver();
+		await shouldDeleteContent(newConfig());
-		await shouldDeleteContent(driver);
 	});
 
 	test('should not create the item if the content cannot be saved', async function() {
-		const driver = newDriver();
+		await shouldNotCreateItemIfContentNotSaved(newConfig());
-		await shouldNotCreateItemIfContentNotSaved(driver);
 	});
 
 	test('should not update the item if the content cannot be saved', async function() {
-		const driver = newDriver();
+		await shouldNotUpdateItemIfContentNotSaved(newConfig());
-		await shouldNotUpdateItemIfContentNotSaved(driver);
 	});
 
 	test('should write to a file and read it back', async function() {

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(newConfig());
-		await shouldWriteToContentAndReadItBack(driver);
 	});
 
 	test('should delete the content', async function() {
-		const driver = new StorageDriverMemory(1);
+		await shouldDeleteContent(newConfig());
-		await shouldDeleteContent(driver);
 	});
 
 	test('should not create the item if the content cannot be saved', async function() {
-		const driver = new StorageDriverMemory(1);
+		await shouldNotCreateItemIfContentNotSaved(newConfig());
-		await shouldNotCreateItemIfContentNotSaved(driver);
 	});
 
 	test('should not update the item if the content cannot be saved', async function() {
-		const driver = new StorageDriverMemory(1);
+		await shouldNotUpdateItemIfContentNotSaved(newConfig());
-		await shouldNotUpdateItemIfContentNotSaved(driver);
 	});
 
 });

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(),
+			})
+		);
+	});
+
+});
+

packages/server/src/models/items/storage/loadStorageDriver.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) {
+		if (!storage) {
-			storageId = existingStorage.id;
+			await models.storage().save({
-		} else {
-			const 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) {

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 { createUserAndSession, db, makeNoteSerializedBody, models } from '../../../utils/testing/testUtils';
-import { StorageDriverMode } from '../../../utils/types';
+import { Config, StorageDriverConfig, StorageDriverMode } from '../../../utils/types';
-import StorageDriverBase, { Context } from './StorageDriverBase';
+import newModelFactory from '../../factory';
+import { Context } from './StorageDriverBase';
 
-const testModels = (driver: StorageDriverBase) => {
+const newTestModels = (driverConfig: StorageDriverConfig, driverConfigFallback: StorageDriverConfig = null) => {
-	return models({ storageDriver: driver });
+	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);
+	expect((await testModels.item().all()).length).toBe(1);
-	await testModels(driver).item().delete(item.id);
+	await testModels.item().delete(item.id);
-	expect((await testModels(driver).item().all()).length).toBe(0);
+	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({
+	const testModelWithFallback = newTestModels(driverConfig, fallbackDriverConfig);
-		storageDriver: driver,
-		storageDriverFallback: fallbackDriver,
-	});
 
 	// 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) {
+export async function shouldSupportFallbackDriverInReadWriteMode(driverConfig: StorageDriverConfig, fallbackDriverConfig: StorageDriverConfig) {
-	if (fallbackDriver.mode !== StorageDriverMode.ReadWrite) throw new Error('Content driver must be configured in RW mode for this test');
+	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({
+	const testModelWithFallback = newTestModels(driverConfig, fallbackDriverConfig);
-		storageDriver: driver,
-		storageDriverFallback: fallbackDriver,
-	});
 
 	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) {
+export async function shouldUpdateContentStorageIdAfterSwitchingDriver(oldDriverConfig: StorageDriverConfig, newDriverConfig: StorageDriverConfig) {
-	if (oldDriver.storageId === newDriver.storageId) throw new Error('Drivers must be different for this test');
+	if (oldDriverConfig.type === newDriverConfig.type) throw new Error('Drivers must be different for this test');
 
 	const { user } = await createUserAndSession(1);
 
-	const oldDriverModel = models({
+	const oldDriverModel = newTestModels(oldDriverConfig);
-		storageDriver: oldDriver,
+	const newDriverModel = newTestModels(newDriverConfig);
-	});
+	const oldDriver = await oldDriverModel.item().storageDriver();
-
+	const newDriver = await newDriverModel.item().storageDriver();
-	const newDriverModel = models({
-		storageDriver: newDriver,
-	});
 
 	const output = await oldDriverModel.item().saveFromRawContent(user, [{
 		name: '00000000000000000000000000000001.md',

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' },

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, {
+	const models = newModelFactory(db, config);
-		// storageDriver: new StorageDriverDatabase(1, { dbClientType: clientType(db) }),
-		storageDriver: await storageDriverFromConfig(config.storageDriver, db), // new StorageDriverDatabase(1, { dbClientType: clientType(db) }),
-	});
 
 	if (options.count) {
 		const users: User[] = [];

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<AppContext> {
+export default async function(appContext: AppContext, env: Env, dbConnection: DbConnection, appLogger: ()=> LoggerWrapper): Promise<AppContext> {
-	const models = newModelFactory(dbConnection, config(), options);
+	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

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() {
-
+	return modelFactory(db(), config());
-export function models(options: ModelFactoryOptions = null) {
-	options = {
-		storageDriver: storageDriverMemory,
-		storageDriverFallback: null,
-		...options,
-	};
-
-	return modelFactory(db(), config(), options);
 }
 
 export function parseHtml(html: string): Document {
@@ -445,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 {