Fix review comments

- Combine the various exported types; the only API surface is now the
  `PluginDatabaseClientFactory` which is a free function.
- Rename to the SingleConnectionManager class.
- Refactor the PluginEnvironment type to use databaseClientFactory for
  clarity.
- Fix mergeDatabaseConfig to not override the input dictionary.

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

@@ -17,43 +17,39 @@
 import Knex from 'knex';
 import { Config } from '@backstage/config';
 import { createDatabaseClient, ensureDatabaseExists } from './connection';
-import {
-  DatabaseConfiguration,
-  PluginDatabaseFactory,
-  PluginDatabaseManager,
-} from './types';
+import { PluginDatabaseClientFactory } from './types';
 
-export class SingleDatabaseConfiguration implements DatabaseConfiguration {
+/**
+ * Implements a Database Manager which will automatically create new databases for plugins when requested. All
+ * requested databases are created with the credentials provided.
+ */
+export class SingleConnectionManager {
   private readonly config: Config;
 
   constructor(config: Config) {
     this.config = config;
   }
 
-  getDatabaseConfig(_pluginId: string): Config {
+  private getDatabaseConfig(): Config {
     return this.config;
   }
 
-  getAdminConfig(): Config {
+  private getAdminConfig(): Config {
     return this.config;
   }
-}
 
-export class SingleDatabaseManager implements PluginDatabaseManager {
-  private readonly config: DatabaseConfiguration;
-
-  constructor(config: DatabaseConfiguration) {
-    this.config = config;
+  /**
+   * Generates a PluginDatabaseClientFactory for consumption by plugins.
+   */
+  getDatabaseClientFactory(pluginId: string): PluginDatabaseClientFactory {
+    return (database?: string): Promise<Knex> => {
+      return this.getDatabase(pluginId, database);
+    };
   }
 
-  getDatabaseFactory(pluginId: string): PluginDatabaseFactory {
-    // eslint-disable-next-line no-use-before-define
-    return new SinglePluginDatabaseFactory(this, pluginId);
-  }
-
-  async getDatabase(pluginId: string, suffix?: string): Promise<Knex> {
-    const config = this.config.getDatabaseConfig(pluginId);
-    const overrides = SingleDatabaseManager.getDatabaseOverrides(
+  private async getDatabase(pluginId: string, suffix?: string): Promise<Knex> {
+    const config = this.getDatabaseConfig();
+    const overrides = SingleConnectionManager.getDatabaseOverrides(
       pluginId,
       suffix,
     );
@@ -76,21 +72,7 @@ export class SingleDatabaseManager implements PluginDatabaseManager {
   }
 
   private async ensureDatabase(database: string) {
-    const config = this.config.getAdminConfig();
+    const config = this.getAdminConfig();
     await ensureDatabaseExists(config, database);
   }
 }
-
-export class SinglePluginDatabaseFactory implements PluginDatabaseFactory {
-  private manager: SingleDatabaseManager;
-  private readonly pluginId: string;
-
-  constructor(manager: SingleDatabaseManager, pluginId: string) {
-    this.manager = manager;
-    this.pluginId = pluginId;
-  }
-
-  getDatabase(database?: string): Promise<Knex> {
-    return this.manager.getDatabase(this.pluginId, database);
-  }
-}

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

@@ -23,5 +23,5 @@ import { merge } from 'lodash';
  * @param overrides Any additional overrides
  */
 export function mergeDatabaseConfig(config: any, ...overrides: any[]) {
-  return merge(config, ...overrides);
+  return merge({}, config, ...overrides);
 }

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

@@ -16,4 +16,4 @@
 
 export * from './connection';
 export * from './types';
-export * from './SingleDatabase';
+export * from './SingleConnection';

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

@@ -15,7 +15,6 @@
  */
 
 import knex, { PgConnectionConfig } from 'knex';
-import { cloneDeep } from 'lodash';
 import { Config } from '@backstage/config';
 import { mergeDatabaseConfig } from './config';
 
@@ -45,7 +44,7 @@ export function buildPgDatabaseConfig(
   overrides?: knex.Config,
 ) {
   return mergeDatabaseConfig(
-    cloneDeep(dbConfig.get()),
+    dbConfig.get(),
     {
       connection: getPgConnectionConfig(dbConfig, !!overrides),
       useNullAsDefault: true,

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

@@ -15,45 +15,13 @@
  */
 
 import knex from 'knex';
-import { Config } from '@backstage/config';
 
 /**
- * DatabaseConfiguration exposes the Application configuration to PluginDatabaseManager instances.
- */
-export interface DatabaseConfiguration {
-  /**
-   * Returns the database configuration for the given Plugin.
-   */
-  getDatabaseConfig(pluginId: string): Config;
-
-  /**
-   * Returns the database configuration for administrating the database server.
-   */
-  getAdminConfig(): Config;
-}
-
-/**
- * The PluginDatabaseManager produces PluginDatabaseFactory objects for plugins to consume and store their data.
- */
-export type PluginDatabaseManager = {
-  /**
-   * getDatabaseFactory returns the database factory object for plugins to obtain database connections.
-   */
-  getDatabaseFactory(pluginId: string): PluginDatabaseFactory;
-};
-
-/**
- * The PluginDatabaseFactory is used to provide a mechanism for backend plugins to obtain database connections for
- * itself.
+ * The PluginDatabaseClientFactory is used to provide a mechanism for backend plugins to obtain database connections
+ * for itself.
  *
- * The purpose of this factory is to allow plugins to get isolated data stores so that plugins are discouraged from database integration.
+ * The purpose of this factory is to allow plugins to get isolated data stores so that plugins are discouraged from
+ * database integration. Plugins can omit the `database` parameter to get the default plugin database, or
+ * provide an identifier that will be used to identify a separate database from the default.
  */
-export type PluginDatabaseFactory = {
-  /**
-   * Returns a database connection. Plugins can omit the `database` parameter to get the default plugin database, or
-   * provide an identifier that will be used to identify a separate database from the default.
-   *
-   * Plugins completely own and manage the state of the database (including schema/migrations/data.)
-   */
-  getDatabase(database?: String): Promise<knex>;
-};
+export type PluginDatabaseClientFactory = (database?: string) => Promise<knex>;

packages/backend/src/index.ts

@@ -29,8 +29,7 @@ import {
   getRootLogger,
   useHotMemoize,
   notFoundHandler,
-  SingleDatabaseConfiguration,
-  SingleDatabaseManager,
+  SingleConnectionManager,
   SingleHostDiscovery,
   UrlReaders,
 } from '@backstage/backend-common';
@@ -55,15 +54,14 @@ function makeCreateEnv(config: ConfigReader) {
 
   root.info(`Created UrlReader ${reader}`);
 
-  const databaseConfig = new SingleDatabaseConfiguration(
+  const databaseManager = new SingleConnectionManager(
     config.getConfig('backend.database'),
   );
-  const databaseManager = new SingleDatabaseManager(databaseConfig);
 
   return (plugin: string): PluginEnvironment => {
     const logger = root.child({ type: 'plugin', plugin });
-    const databaseFactory = databaseManager.getDatabaseFactory(plugin);
-    return { logger, database: databaseFactory, config, reader, discovery };
+    const databaseFactory = databaseManager.getDatabaseClientFactory(plugin);
+    return { logger, databaseClientFactory: databaseFactory, config, reader, discovery };
   };
 }
 

packages/backend/src/plugins/auth.ts

@@ -19,9 +19,14 @@ import { PluginEnvironment } from '../types';
 
 export default async function createPlugin({
   logger,
-  database,
+  databaseClientFactory,
   config,
   discovery,
 }: PluginEnvironment) {
-  return await createRouter({ logger, config, database, discovery });
+  return await createRouter({
+    logger,
+    config,
+    database: databaseClientFactory,
+    discovery,
+  });
 }

packages/backend/src/plugins/catalog.ts

@@ -30,12 +30,12 @@ export default async function createPlugin({
   logger,
   config,
   reader,
-  database,
+  databaseClientFactory,
 }: PluginEnvironment) {
   const locationReader = new LocationReaders({ logger, reader, config });
 
   const db = await DatabaseManager.createDatabase(
-    await database.getDatabase(),
+    await databaseClientFactory(),
     { logger },
   );
   const entitiesCatalog = new DatabaseEntitiesCatalog(db);

packages/backend/src/types.ts

@@ -17,14 +17,14 @@
 import { Logger } from 'winston';
 import { Config } from '@backstage/config';
 import {
-  PluginDatabaseFactory,
+  PluginDatabaseClientFactory,
   PluginEndpointDiscovery,
   UrlReader
 } from '@backstage/backend-common';
 
 export type PluginEnvironment = {
   logger: Logger;
-  database: PluginDatabaseFactory;
+  databaseClientFactory: PluginDatabaseClientFactory;
   config: Config;
   reader: UrlReader;
   discovery: PluginEndpointDiscovery;

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

@@ -8,14 +8,12 @@
 
 import Router from 'express-promise-router';
 import {
-  ensureDatabaseExists,
   createServiceBuilder,
   loadBackendConfig,
   getRootLogger,
   useHotMemoize,
   notFoundHandler,
-  SingleDatabaseConfiguration,
-  SingleDatabaseManager,
+  SingleConnectionManager,
   SingleHostDiscovery,
   UrlReaders,
 } from '@backstage/backend-common';
@@ -34,15 +32,20 @@ function makeCreateEnv(config: ConfigReader) {
 
   root.info(`Created UrlReader ${reader}`);
 
-  const databaseConfig = new SingleDatabaseConfiguration(
-    config.getConfig('backend.database'),
-  )
-  const databaseManager = new SingleDatabaseManager(databaseConfig);
+  const databaseManager = new SingleConnectionManager(
+    config.getConfig('backend.database')
+  );
 
   return (plugin: string): PluginEnvironment => {
     const logger = root.child({ type: 'plugin', plugin });
-    const databaseFactory = databaseManager.getDatabaseFactory(plugin);
-    return { logger, database: databaseFactory, config, reader, discovery };
+    const databaseFactory = databaseManager.getDatabaseClientFactory(plugin);
+    return {
+      logger,
+      databaseClientFactory: databaseFactory,
+      config,
+      reader,
+      discovery
+    };
   };
 }
 
@@ -50,11 +53,6 @@ async function main() {
   const configs = await loadBackendConfig();
   const configReader = ConfigReader.fromConfigs(configs);
   const createEnv = makeCreateEnv(configReader);
-  await ensureDatabaseExists(
-    configReader.getConfig('backend.database'),
-    'backstage_plugin_catalog',
-    'backstage_plugin_auth',
-  );
 
   const catalogEnv = useHotMemoize(module, () => createEnv('catalog'));
   const scaffolderEnv = useHotMemoize(module, () => createEnv('scaffolder'));

packages/create-app/templates/default-app/packages/backend/src/plugins/auth.ts

@@ -3,9 +3,14 @@ import { PluginEnvironment } from '../types';
 
 export default async function createPlugin({
   logger,
-  database,
+  databaseClientFactory,
   config,
   discovery,
 }: PluginEnvironment) {
-  return await createRouter({ logger, config, database, discovery });
+  return await createRouter({
+    logger,
+    config,
+    database: databaseClientFactory,
+    discovery
+  });
 }

packages/create-app/templates/default-app/packages/backend/src/plugins/catalog.ts

@@ -14,11 +14,14 @@ export default async function createPlugin({
   logger,
   config,
   reader,
-  database,
+  databaseClientFactory,
 }: PluginEnvironment) {
   const locationReader = new LocationReaders({ logger, reader, config });
 
-  const db = await DatabaseManager.createDatabase(database, { logger });
+  const db = await DatabaseManager.createDatabase(
+    await databaseClientFactory(),
+    { logger },
+  );
   const entitiesCatalog = new DatabaseEntitiesCatalog(db);
   const locationsCatalog = new DatabaseLocationsCatalog(db);
   const higherOrderOperation = new HigherOrderOperations(

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

@@ -1,11 +1,14 @@
-import Knex from 'knex';
 import { Logger } from 'winston';
 import { Config } from '@backstage/config';
-import { PluginEndpointDiscovery, UrlReader } from '@backstage/backend-common';
+import {
+  PluginDatabaseClientFactory,
+  PluginEndpointDiscovery,
+  UrlReader
+} from '@backstage/backend-common';
 
 export type PluginEnvironment = {
   logger: Logger;
-  database: Knex;
+  databaseClientFactory: PluginDatabaseClientFactory;
   config: Config;
   reader: UrlReader
   discovery: PluginEndpointDiscovery;

plugins/auth-backend/src/service/router.ts

@@ -24,12 +24,12 @@ import { DatabaseKeyStore, TokenFactory, createOidcRouter } from '../identity';
 import {
   NotFoundError,
   PluginEndpointDiscovery,
-  PluginDatabaseFactory,
+  PluginDatabaseClientFactory,
 } from '@backstage/backend-common';
 
 export interface RouterOptions {
   logger: Logger;
-  database: PluginDatabaseFactory;
+  database: PluginDatabaseClientFactory;
   config: Config;
   discovery: PluginEndpointDiscovery;
 }
@@ -48,7 +48,7 @@ export async function createRouter({
   const keyDurationSeconds = 3600;
 
   const keyStore = await DatabaseKeyStore.create({
-    database: await database.getDatabase(),
+    database: await database(),
   });
   const tokenIssuer = new TokenFactory({
     issuer: authUrl,

plugins/auth-backend/src/service/standaloneServer.ts

@@ -53,10 +53,8 @@ export async function startStandaloneServer(
   const router = await createRouter({
     logger,
     config,
-    database: {
-      async getDatabase(_database?: string) {
-        return database;
-      },
+    database: async () => {
+      return database;
     },
     discovery,
   });