Merge pull request #8364 from backstage/sqlite-separate-files

Overwrite sqlite filename per plugin

.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.

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 () => {

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

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:'), {

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;
 }