From 63f5bc5f2cf699b0a479237331ad823dadd54ff5 Mon Sep 17 00:00:00 2001 From: Joe Porpeglia Date: Thu, 9 Dec 2021 17:59:56 -0500 Subject: [PATCH] Add connection.directory support for sqlite Signed-off-by: Joe Porpeglia --- .../src/database/DatabaseManager.test.ts | 128 ++++++++++++++---- .../src/database/DatabaseManager.ts | 27 +++- 2 files changed, 125 insertions(+), 30 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index c72c740765..a48b1584aa 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -171,28 +171,6 @@ describe('DatabaseManager', () => { ); }); - it('uses top level sqlite database filename if plugin config is not present', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'sqlite3', - connection: 'some-file-path', - }, - }, - }), - ); - - await testManager.forPlugin('pluginwithoutconfig').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.filename', - expect.stringContaining('some-file-path'), - ); - }); - it('provides an inmemory sqlite database if top level is also inmemory and plugin config is not present', async () => { const testManager = DatabaseManager.fromConfig( new ConfigReader({ @@ -215,6 +193,110 @@ describe('DatabaseManager', () => { ); }); + it('throws if top level sqlite filename is provided', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'sqlite3', + connection: 'some-file-path', + }, + }, + }), + ); + + await expect( + testManager.forPlugin('pluginwithoutconfig').getClient(), + ).rejects.toBeInstanceOf(Error); + }); + + it('creates plugin-specific sqlite files when plugin config is not present', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'sqlite3', + connection: { + directory: 'sqlite-files', + }, + }, + }, + }), + ); + + await testManager.forPlugin('pluginwithoutconfig').getClient(); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + const [_, overrides] = mockCalls[0]; + + expect(overrides).toHaveProperty( + 'connection.filename', + path.join('sqlite-files', 'pluginwithoutconfig.sqlite'), + ); + }); + + it('uses sqlite directory from top level config and filename from plugin config', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'sqlite3', + connection: { + directory: 'sqlite-files', + }, + plugin: { + test: { + connection: { + filename: 'other.sqlite', + }, + }, + }, + }, + }, + }), + ); + + await testManager.forPlugin('test').getClient(); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + const [_, overrides] = mockCalls[0]; + + expect(overrides).toHaveProperty( + 'connection.filename', + path.join('sqlite-files', 'other.sqlite'), + ); + }); + + it('uses sqlite directory and filename from plugin config', async () => { + const testManager = DatabaseManager.fromConfig( + new ConfigReader({ + backend: { + database: { + client: 'sqlite3', + connection: { + directory: 'sqlite-files', + }, + plugin: { + test: { + connection: { + directory: 'custom-sqlite-files', + filename: 'other.sqlite', + }, + }, + }, + }, + }, + }), + ); + + await testManager.forPlugin('test').getClient(); + const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); + const [_, overrides] = mockCalls[0]; + + expect(overrides).toHaveProperty( + 'connection.filename', + path.join('custom-sqlite-files', 'other.sqlite'), + ); + }); + it('connects to a plugin database using a specific database name', async () => { // testdbname.connection.database is set in config await manager.forPlugin('testdbname').getClient(); @@ -285,7 +367,7 @@ describe('DatabaseManager', () => { // sqlite3 uses 'filename' instead of 'database' expect(overrides).toHaveProperty( 'connection.filename', - path.join('plugin_with_different_client', `${pluginId}.sqlite`), + 'plugin_with_different_client', ); }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index a00a4991cb..5959c60c83 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -115,16 +115,18 @@ export class DatabaseManager { const connection = this.getConnectionConfig(pluginId); if (this.getClientType(pluginId).client === 'sqlite3') { - const sqliteFilename = (connection as Knex.Sqlite3ConnectionConfig) - ?.filename; + const sqliteFilename: string | undefined = ( + connection as Knex.Sqlite3ConnectionConfig + ).filename; - // if persisting to a file, create separate files per plugin to avoid db migration issues. - if (sqliteFilename && sqliteFilename !== ':memory:') { - return path.join(sqliteFilename, `${pluginId}.sqlite`); + if (sqliteFilename === ':memory:') { + return sqliteFilename; } - // sqlite database name should fallback to ':memory:' as a special case - return ':memory:'; + const sqliteDirectory = + (connection as { directory?: string }).directory ?? '.'; + + return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); } const databaseName = (connection as Knex.ConnectionConfig)?.database; @@ -212,6 +214,17 @@ export class DatabaseManager { this.config.get('connection'), this.config.getString('client'), ); + + if ( + client === 'sqlite3' && + 'filename' in baseConnection && + baseConnection.filename !== ':memory:' + ) { + throw new Error( + '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', + ); + } + // Databases cannot be shared unless the `pluginDivisionMode` is set to `schema`. The // `database` property from the base connection is omitted unless `pluginDivisionMode` // is set to `schema`. SQLite3's `filename` property is an exception as this is used as a