Add connection.directory support for sqlite

Signed-off-by: Joe Porpeglia <josephp@spotify.com>
2021-12-09 17:59:56 -05:00
parent d9a286bd56
commit 63f5bc5f2c
2 changed files with 125 additions and 30 deletions
@@ -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',
      );
    });

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