diff --git a/.changeset/pink-ladybugs-share.md b/.changeset/pink-ladybugs-share.md new file mode 100644 index 0000000000..83f5b8b78a --- /dev/null +++ b/.changeset/pink-ladybugs-share.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-common': patch +--- + +Each plugin now saves to a separate sqlite database file when `connection.filename` is provided in the sqlite config. +Any existing sqlite database files will be ignored. diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index bd2e7b9f71..a48b1584aa 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -15,6 +15,7 @@ */ import { ConfigReader } from '@backstage/config'; import { omit } from 'lodash'; +import path from 'path'; import { createDatabaseClient, ensureDatabaseExists, @@ -170,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({ @@ -214,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(); @@ -282,7 +365,10 @@ describe('DatabaseManager', () => { expect(baseConfig.get().client).toEqual('sqlite3'); // sqlite3 uses 'filename' instead of 'database' - expect(overrides).toHaveProperty('connection.filename'); + expect(overrides).toHaveProperty( + 'connection.filename', + 'plugin_with_different_client', + ); }); it('provides database client specific base from plugin connection string when client set under plugin', async () => { diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 8e23a219ec..5959c60c83 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -28,6 +28,7 @@ import { normalizeConnection, } from './connection'; import { PluginDatabaseManager } from './types'; +import path from 'path'; /** * Provides a config lookup path for a plugin's config block. @@ -114,10 +115,18 @@ export class DatabaseManager { const connection = this.getConnectionConfig(pluginId); if (this.getClientType(pluginId).client === 'sqlite3') { - // sqlite database name should fallback to ':memory:' as a special case - return ( - (connection as Knex.Sqlite3ConnectionConfig)?.filename ?? ':memory:' - ); + const sqliteFilename: string | undefined = ( + connection as Knex.Sqlite3ConnectionConfig + ).filename; + + if (sqliteFilename === ':memory:') { + return sqliteFilename; + } + + const sqliteDirectory = + (connection as { directory?: string }).directory ?? '.'; + + return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); } const databaseName = (connection as Knex.ConnectionConfig)?.database; @@ -205,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 diff --git a/packages/backend-common/src/database/connectors/sqlite3.test.ts b/packages/backend-common/src/database/connectors/sqlite3.test.ts index b9da19d247..cfc9f61527 100644 --- a/packages/backend-common/src/database/connectors/sqlite3.test.ts +++ b/packages/backend-common/src/database/connectors/sqlite3.test.ts @@ -73,28 +73,6 @@ describe('sqlite3', () => { }); }); - it('builds a persistent connection per database', () => { - expect( - buildSqliteDatabaseConfig( - createConfig({ - filename: path.join('path', 'to', 'foo'), - }), - { - connection: { - database: 'my-database', - }, - }, - ), - ).toEqual({ - client: 'sqlite3', - connection: { - filename: path.join('path', 'to', 'foo', 'my-database.sqlite'), - database: 'my-database', - }, - useNullAsDefault: true, - }); - }); - it('replaces the connection with an override', () => { expect( buildSqliteDatabaseConfig(createConfig(':memory:'), { diff --git a/packages/backend-common/src/database/connectors/sqlite3.ts b/packages/backend-common/src/database/connectors/sqlite3.ts index 3dfecd7e6b..41f80ed293 100644 --- a/packages/backend-common/src/database/connectors/sqlite3.ts +++ b/packages/backend-common/src/database/connectors/sqlite3.ts @@ -86,19 +86,6 @@ export function buildSqliteDatabaseConfig( overrides, ); - // If we don't create an in-memory database, interpret the connection string - // as a directory that contains multiple sqlite files based on the database - // name. - const database = (config.connection as Knex.ConnectionConfig).database; - const sqliteConnection = config.connection as Knex.Sqlite3ConnectionConfig; - - if (database && sqliteConnection.filename !== ':memory:') { - sqliteConnection.filename = path.join( - sqliteConnection.filename, - `${database}.sqlite`, - ); - } - return config; }