Merge pull request #2173 from andrewthauer/dbtools-update

Cleanup & renames to common database code

packages/backend-common/src/database/connection.test.ts

@@ -15,7 +15,7 @@
  */
 
 import { ConfigReader } from '@backstage/config';
-import { createDatabase } from './connection';
+import { createDatabaseClient } from './connection';
 
 describe('database connection', () => {
   const createConfig = (data: any) =>
@@ -26,10 +26,10 @@ describe('database connection', () => {
       },
     ]);
 
-  describe(createDatabase, () => {
+  describe(createDatabaseClient, () => {
     it('returns a postgres connection', () => {
       expect(
-        createDatabase(
+        createDatabaseClient(
           createConfig({
             client: 'pg',
             connection: {
@@ -45,7 +45,7 @@ describe('database connection', () => {
 
     it('returns an sqlite connection', () => {
       expect(
-        createDatabase(
+        createDatabaseClient(
           createConfig({
             client: 'sqlite3',
             connection: ':memory:',
@@ -56,7 +56,7 @@ describe('database connection', () => {
 
     it('tries to create a mysql connection as a passthrough', () => {
       expect(() =>
-        createDatabase(
+        createDatabaseClient(
           createConfig({
             client: 'mysql',
             connection: {
@@ -72,7 +72,7 @@ describe('database connection', () => {
 
     it('accepts overrides', () => {
       expect(
-        createDatabase(
+        createDatabaseClient(
           createConfig({
             client: 'pg',
             connection: {
@@ -93,7 +93,7 @@ describe('database connection', () => {
 
     it('throws an error without a client', () => {
       expect(() =>
-        createDatabase(
+        createDatabaseClient(
           createConfig({
             connection: '',
           }),
@@ -103,7 +103,7 @@ describe('database connection', () => {
 
     it('throws an error without a connection', () => {
       expect(() =>
-        createDatabase(
+        createDatabaseClient(
           createConfig({
             client: 'pg',
           }),

packages/backend-common/src/database/connection.ts

@@ -15,30 +15,36 @@
  */
 
 import knex from 'knex';
-import { ConfigReader } from '@backstage/config';
+import { Config } from '@backstage/config';
 import { mergeDatabaseConfig } from './config';
-import { createPgDatabase } from './postgres';
-import { createSqlite3Database } from './sqlite3';
+import { createPgDatabaseClient } from './postgres';
+import { createSqliteDatabaseClient } from './sqlite3';
 
 type DatabaseClient = 'pg' | 'sqlite3' | string;
 
 /**
  * Creates a knex database connection
  *
- * @param config The database config
+ * @param dbConfig The database config
  * @param overrides Additional options to merge with the config
  */
-export function createDatabase(
-  config: ConfigReader,
+export function createDatabaseClient(
+  dbConfig: Config,
   overrides?: Partial<knex.Config>,
 ) {
-  const client: DatabaseClient = config.getString('client');
+  const client: DatabaseClient = dbConfig.getString('client');
 
   if (client === 'pg') {
-    return createPgDatabase(config, overrides);
+    return createPgDatabaseClient(dbConfig, overrides);
   } else if (client === 'sqlite3') {
-    return createSqlite3Database(config);
+    return createSqliteDatabaseClient(dbConfig);
   }
 
-  return knex(mergeDatabaseConfig(config.get(), overrides));
+  return knex(mergeDatabaseConfig(dbConfig.get(), overrides));
 }
+
+/**
+ * Alias for createDatabaseClient
+ * @deprecated Use createDatabaseClient instead
+ */
+export const createDatabase = createDatabaseClient;

packages/backend-common/src/database/postgres.test.ts

@@ -14,15 +14,26 @@
  * limitations under the License.
  */
 
-import { ConfigReader } from '@backstage/config';
+import { Config, ConfigReader } from '@backstage/config';
 import {
-  parsePgConnectionString,
+  createPgDatabaseClient,
   buildPgDatabaseConfig,
-  createPgDatabase,
+  getPgConnectionConfig,
+  parsePgConnectionString,
 } from './postgres';
 
 describe('postgres', () => {
-  const createConfig = (connection: any) =>
+  const createMockConnection = () => ({
+    host: 'acme',
+    user: 'foo',
+    password: 'bar',
+    database: 'foodb',
+  });
+
+  const createMockConnectionString = () =>
+    'postgresql://foo:bar@acme:5432/foodb';
+
+  const createConfig = (connection: any): Config =>
     ConfigReader.fromConfigs([
       {
         context: '',
@@ -35,86 +46,58 @@ describe('postgres', () => {
 
   describe(buildPgDatabaseConfig, () => {
     it('builds a postgres config', () => {
-      expect(
-        buildPgDatabaseConfig(
-          createConfig({
-            host: 'acme',
-            user: 'foo',
-            password: 'bar',
-            port: '5432',
-            database: 'foodb',
-          }),
-        ),
-      ).toEqual({
+      const mockConnection = createMockConnection();
+
+      expect(buildPgDatabaseConfig(createConfig(mockConnection))).toEqual({
         client: 'pg',
-        connection: {
-          host: 'acme',
-          user: 'foo',
-          password: 'bar',
-          port: '5432',
-          database: 'foodb',
-        },
+        connection: mockConnection,
         useNullAsDefault: true,
       });
     });
 
     it('builds a connection string config', () => {
-      expect(
-        buildPgDatabaseConfig(
-          createConfig('postgresql://foo:bar@acme:5432/foodb'),
-        ),
-      ).toEqual({
-        client: 'pg',
-        connection: 'postgresql://foo:bar@acme:5432/foodb',
-        useNullAsDefault: true,
-      });
+      const mockConnectionString = createMockConnectionString();
+
+      expect(buildPgDatabaseConfig(createConfig(mockConnectionString))).toEqual(
+        {
+          client: 'pg',
+          connection: mockConnectionString,
+          useNullAsDefault: true,
+        },
+      );
     });
 
     it('overrides the database name', () => {
+      const mockConnection = createMockConnection();
+
       expect(
-        buildPgDatabaseConfig(
-          createConfig({
-            host: 'somehost',
-            user: 'postgres',
-            password: 'pass',
-            database: 'foo',
-          }),
-          { connection: { database: 'foodb' } },
-        ),
+        buildPgDatabaseConfig(createConfig(mockConnection), {
+          connection: { database: 'other_db' },
+        }),
       ).toEqual({
         client: 'pg',
         connection: {
-          host: 'somehost',
-          user: 'postgres',
-          password: 'pass',
-          database: 'foodb',
+          ...mockConnection,
+          database: 'other_db',
         },
         useNullAsDefault: true,
       });
     });
 
     it('adds additional config settings', () => {
+      const mockConnection = createMockConnection();
+
       expect(
-        buildPgDatabaseConfig(
-          createConfig({
-            host: 'somehost',
-            user: 'postgres',
-            password: 'pass',
-            database: 'foo',
-          }),
-          {
-            connection: { database: 'foodb' },
-            pool: { min: 0, max: 7 },
-            debug: true,
-          },
-        ),
+        buildPgDatabaseConfig(createConfig(mockConnection), {
+          connection: { database: 'other_db' },
+          pool: { min: 0, max: 7 },
+          debug: true,
+        }),
       ).toEqual({
         client: 'pg',
         connection: {
-          host: 'somehost',
-          user: 'postgres',
-          password: 'pass',
-          database: 'foodb',
+          ...mockConnection,
+          database: 'other_db',
         },
         useNullAsDefault: true,
         pool: { min: 0, max: 7 },
@@ -123,37 +106,72 @@ describe('postgres', () => {
     });
 
     it('overrides the database from connection string', () => {
+      const mockConnectionString = createMockConnectionString();
+      const mockConnection = createMockConnection();
+
       expect(
-        buildPgDatabaseConfig(
-          createConfig('postgresql://postgres:pass@localhost:5432/dbname'),
-          { connection: { database: 'foodb' } },
-        ),
+        buildPgDatabaseConfig(createConfig(mockConnectionString), {
+          connection: { database: 'other_db' },
+        }),
       ).toEqual({
         client: 'pg',
         connection: {
-          host: 'localhost',
-          user: 'postgres',
-          password: 'pass',
+          ...mockConnection,
           port: '5432',
-          database: 'foodb',
+          database: 'other_db',
         },
         useNullAsDefault: true,
       });
     });
   });
 
-  describe(createPgDatabase, () => {
+  describe(getPgConnectionConfig, () => {
+    it('returns the connection object back', () => {
+      const mockConnection = createMockConnection();
+      const config = createConfig(mockConnection);
+
+      expect(getPgConnectionConfig(config)).toEqual(mockConnection);
+    });
+
+    it('does not parse the connection string', () => {
+      const mockConnection = createMockConnection();
+      const config = createConfig(mockConnection);
+
+      expect(getPgConnectionConfig(config, true)).toEqual(mockConnection);
+    });
+
+    it('automatically parses the connection string', () => {
+      const mockConnection = createMockConnection();
+      const mockConnectionString = createMockConnectionString();
+      const config = createConfig(mockConnectionString);
+
+      expect(getPgConnectionConfig(config)).toEqual({
+        ...mockConnection,
+        port: '5432',
+      });
+    });
+
+    it('parses the connection string', () => {
+      const mockConnection = createMockConnection();
+      const mockConnectionString = createMockConnectionString();
+      const config = createConfig(mockConnectionString);
+
+      expect(getPgConnectionConfig(config, true)).toEqual({
+        ...mockConnection,
+        port: '5432',
+      });
+    });
+  });
+
+  describe(createPgDatabaseClient, () => {
     it('creates a postgres knex instance', () => {
       expect(
-        createPgDatabase(
+        createPgDatabaseClient(
           createConfig({
-            client: 'pg',
-            connection: {
-              host: 'acme',
-              user: 'foo',
-              password: 'bar',
-              database: 'foodb',
-            },
+            host: 'acme',
+            user: 'foo',
+            password: 'bar',
+            database: 'foodb',
           }),
         ),
       ).toBeTruthy();
@@ -161,7 +179,7 @@ describe('postgres', () => {
 
     it('attempts to read an ssl cert', () => {
       expect(() =>
-        createPgDatabase(
+        createPgDatabaseClient(
           createConfig(
             'postgresql://postgres:pass@localhost:5432/dbname?sslrootcert=/path/to/file',
           ),

packages/backend-common/src/database/postgres.ts

@@ -14,18 +14,18 @@
  * limitations under the License.
  */
 
-import knex from 'knex';
-import { ConfigReader } from '@backstage/config';
+import knex, { PgConnectionConfig } from 'knex';
+import { Config } from '@backstage/config';
 import { mergeDatabaseConfig } from './config';
 
 /**
- * Creates a knex sqlite3 database connection
+ * Creates a knex postgres database connection
  *
  * @param dbConfig The database config
  * @param overrides Additional options to merge with the config
  */
-export function createPgDatabase(
-  dbConfig: ConfigReader,
+export function createPgDatabaseClient(
+  dbConfig: Config,
   overrides?: knex.Config,
 ) {
   const knexConfig = buildPgDatabaseConfig(dbConfig, overrides);
@@ -40,26 +40,43 @@ export function createPgDatabase(
  * @param overrides Additional options to merge with the config
  */
 export function buildPgDatabaseConfig(
-  dbConfig: ConfigReader,
+  dbConfig: Config,
   overrides?: knex.Config,
 ) {
-  const connection = dbConfig.get('connection') as any;
-
   return mergeDatabaseConfig(
     dbConfig.get(),
     {
-      // Only parse the connection string when overrides are provided
-      connection:
-        overrides &&
-        (typeof connection === 'string' || connection instanceof String)
-          ? parsePgConnectionString(connection as string)
-          : connection,
+      connection: getPgConnectionConfig(dbConfig, !!overrides),
       useNullAsDefault: true,
     },
     overrides,
   );
 }
 
+/**
+ * Gets the postgres connection config
+ *
+ * @param dbConfig The database config
+ * @param parseConnectionString Flag to explictly control connection string parsing
+ */
+export function getPgConnectionConfig(
+  dbConfig: Config,
+  parseConnectionString?: boolean,
+): PgConnectionConfig | string {
+  const connection = dbConfig.get('connection') as any;
+  const isConnectionString =
+    typeof connection === 'string' || connection instanceof String;
+  const autoParse = typeof parseConnectionString !== 'boolean';
+
+  const shouldParseConnectionString = autoParse
+    ? isConnectionString
+    : parseConnectionString && isConnectionString;
+
+  return shouldParseConnectionString
+    ? parsePgConnectionString(connection as string)
+    : connection;
+}
+
 /**
  * Parses a connection string using pg-connection-string
  *

packages/backend-common/src/database/sqlite3.test.ts

@@ -15,7 +15,10 @@
  */
 
 import { ConfigReader } from '@backstage/config';
-import { buildSqlite3DatabaseConfig, createSqlite3Database } from './sqlite3';
+import {
+  buildSqliteDatabaseConfig,
+  createSqliteDatabaseClient,
+} from './sqlite3';
 
 describe('sqlite3', () => {
   const createConfig = (connection: any) =>
@@ -29,9 +32,9 @@ describe('sqlite3', () => {
       },
     ]);
 
-  describe(buildSqlite3DatabaseConfig, () => {
+  describe(buildSqliteDatabaseConfig, () => {
     it('buidls a string connection', () => {
-      expect(buildSqlite3DatabaseConfig(createConfig(':memory:'))).toEqual({
+      expect(buildSqliteDatabaseConfig(createConfig(':memory:'))).toEqual({
         client: 'sqlite3',
         connection: ':memory:',
         useNullAsDefault: true,
@@ -40,7 +43,7 @@ describe('sqlite3', () => {
 
     it('builds a filename connection', () => {
       expect(
-        buildSqlite3DatabaseConfig(
+        buildSqliteDatabaseConfig(
           createConfig({
             filename: '/path/to/foo',
           }),
@@ -56,7 +59,7 @@ describe('sqlite3', () => {
 
     it('replaces the connection with an override', () => {
       expect(
-        buildSqlite3DatabaseConfig(createConfig(':memory:'), {
+        buildSqliteDatabaseConfig(createConfig(':memory:'), {
           connection: { filename: '/path/to/foo' },
         }),
       ).toEqual({
@@ -69,10 +72,10 @@ describe('sqlite3', () => {
     });
   });
 
-  describe(createSqlite3Database, () => {
+  describe(createSqliteDatabaseClient, () => {
     it('creates an in memory knex instance', () => {
       expect(
-        createSqlite3Database(
+        createSqliteDatabaseClient(
           createConfig({
             client: 'sqlite3',
             connection: ':memory:',

packages/backend-common/src/database/sqlite3.ts

@@ -15,7 +15,7 @@
  */
 
 import knex from 'knex';
-import { ConfigReader } from '@backstage/config';
+import { Config } from '@backstage/config';
 import { mergeDatabaseConfig } from './config';
 
 /**
@@ -24,11 +24,11 @@ import { mergeDatabaseConfig } from './config';
  * @param dbConfig The database config
  * @param overrides Additional options to merge with the config
  */
-export function createSqlite3Database(
-  dbConfig: ConfigReader,
+export function createSqliteDatabaseClient(
+  dbConfig: Config,
   overrides?: knex.Config,
 ) {
-  const knexConfig = buildSqlite3DatabaseConfig(dbConfig, overrides);
+  const knexConfig = buildSqliteDatabaseConfig(dbConfig, overrides);
   const database = knex(knexConfig);
 
   database.client.pool.on('createSuccess', (_eventId: any, resource: any) => {
@@ -44,8 +44,8 @@ export function createSqlite3Database(
  * @param dbConfig The database config
  * @param overrides Additional options to merge with the config
  */
-export function buildSqlite3DatabaseConfig(
-  dbConfig: ConfigReader,
+export function buildSqliteDatabaseConfig(
+  dbConfig: Config,
   overrides?: knex.Config,
 ) {
   return mergeDatabaseConfig(

packages/backend/src/index.ts

@@ -23,7 +23,7 @@
  */
 
 import {
-  createDatabase,
+  createDatabaseClient,
   createServiceBuilder,
   loadBackendConfig,
   getRootLogger,
@@ -48,11 +48,14 @@ function makeCreateEnv(loadedConfigs: AppConfig[]) {
 
   return (plugin: string): PluginEnvironment => {
     const logger = getRootLogger().child({ type: 'plugin', plugin });
-    const database = createDatabase(config.getConfig('backend.database'), {
-      connection: {
-        database: `backstage_plugin_${plugin}`,
+    const database = createDatabaseClient(
+      config.getConfig('backend.database'),
+      {
+        connection: {
+          database: `backstage_plugin_${plugin}`,
+        },
       },
-    });
+    );
     return { logger, database, config };
   };
 }

packages/create-app/templates/default-app/packages/backend/src/index.ts

@@ -7,7 +7,7 @@
  */
 
 import {
-  createDatabase,
+  createDatabaseClient,
   createServiceBuilder,
   loadBackendConfig,
   getRootLogger,
@@ -27,11 +27,14 @@ function makeCreateEnv(loadedConfigs: AppConfig[]) {
 
   return (plugin: string): PluginEnvironment => {
     const logger = getRootLogger().child({ type: 'plugin', plugin });
-    const database = createDatabase(config.getConfig('backend.database'), {
-      connection: {
-        database: `backstage_plugin_${plugin}`,
+    const database = createDatabaseClient(
+      config.getConfig('backend.database'),
+      {
+        connection: {
+          database: `backstage_plugin_${plugin}`,
+        },
       },
-    });
+    );
     return { logger, database, config };
   };
 }