force the passing of deps to the database manager

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

.changeset/wise-forks-play.md

@@ -6,5 +6,6 @@
 **BREAKING**: Simplifications and cleanup as part of the Backend System 1.0 work.
 
 - The deprecated `dropDatabase` function has now been removed, without replacement.
-- The deprecated `LegacyRootDatabaseService` type has now been removed. The implication is that you now need to pass deps to a `DatabaseManager` when constructing one.
+- The deprecated `LegacyRootDatabaseService` type has now been removed.
 - The return type from `DatabaseManager.forPlugin` is now directly a `DatabaseService`, as arguably expected.
+- `DatabaseManager.forPlugin` now requires the `deps` argument, with the logger and lifecycle services.

packages/backend-common/api-report.md

@@ -157,7 +157,10 @@ export class DatabaseManager {
   // (undocumented)
   static fromConfig(
     config: Config,
-    options?: DatabaseManagerOptions,
+    options?: {
+      migrations?: DatabaseService['migrations'];
+      logger?: LoggerService;
+    },
   ): DatabaseManager;
 }
 

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

@@ -46,6 +46,8 @@ import {
   isChildPath as _isChildPath,
   LifecycleService,
   PluginMetadataService,
+  DatabaseService,
+  LoggerService,
 } from '@backstage/backend-plugin-api';
 
 export * from './hot';
@@ -168,14 +170,20 @@ export type CacheClientOptions = CacheServiceOptions;
  * @deprecated Use `DatabaseManager` from the `@backstage/backend-defaults` package instead
  */
 export class DatabaseManager {
-  private constructor(private readonly _databaseManager: _DatabaseManager) {}
+  private constructor(
+    private readonly _databaseManager: _DatabaseManager,
+    private readonly logger?: LoggerService,
+  ) {}
 
   static fromConfig(
     config: Config,
-    options?: DatabaseManagerOptions,
+    options?: {
+      migrations?: DatabaseService['migrations'];
+      logger?: LoggerService;
+    },
   ): DatabaseManager {
     const _databaseManager = _DatabaseManager.fromConfig(config, options);
-    return new DatabaseManager(_databaseManager);
+    return new DatabaseManager(_databaseManager, options?.logger);
   }
 
   forPlugin(
@@ -184,7 +192,20 @@ export class DatabaseManager {
       | { lifecycle: LifecycleService; pluginMetadata: PluginMetadataService }
       | undefined,
   ): PluginDatabaseManager {
-    return this._databaseManager.forPlugin(pluginId, deps);
+    const logger: LoggerService = this.logger ?? {
+      debug() {},
+      info() {},
+      warn() {},
+      error() {},
+      child() {
+        return this;
+      },
+    };
+    const lifecycle: LifecycleService = deps?.lifecycle ?? {
+      addShutdownHook() {},
+      addStartupHook() {},
+    };
+    return this._databaseManager.forPlugin(pluginId, { logger, lifecycle });
   }
 }
 

packages/backend-defaults/api-report-database.md

@@ -6,7 +6,6 @@
 import { DatabaseService } from '@backstage/backend-plugin-api';
 import { LifecycleService } from '@backstage/backend-plugin-api';
 import { LoggerService } from '@backstage/backend-plugin-api';
-import { PluginMetadataService } from '@backstage/backend-plugin-api';
 import { RootConfigService } from '@backstage/backend-plugin-api';
 import { ServiceFactory } from '@backstage/backend-plugin-api';
 
@@ -14,9 +13,9 @@ import { ServiceFactory } from '@backstage/backend-plugin-api';
 export class DatabaseManager {
   forPlugin(
     pluginId: string,
-    deps?: {
+    deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): DatabaseService;
   static fromConfig(
@@ -28,7 +27,6 @@ export class DatabaseManager {
 // @public
 export type DatabaseManagerOptions = {
   migrations?: DatabaseService['migrations'];
-  logger?: LoggerService;
 };
 
 // @public

packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts

@@ -17,12 +17,18 @@
 import { ConfigReader } from '@backstage/config';
 import { DatabaseManagerImpl } from './DatabaseManager';
 import { Connector } from './types';
+import { mockServices } from '@backstage/backend-test-utils';
 
 describe('DatabaseManagerImpl', () => {
   afterEach(() => {
     jest.clearAllMocks();
   });
 
+  const deps = {
+    logger: mockServices.logger.mock(),
+    lifecycle: mockServices.lifecycle.mock(),
+  };
+
   it('calls the right connector, only once per plugin id', async () => {
     const connector1 = {
       getClient: jest.fn(),
@@ -41,19 +47,19 @@ describe('DatabaseManagerImpl', () => {
       },
     );
 
-    await impl.forPlugin('plugin1').getClient();
+    await impl.forPlugin('plugin1', deps).getClient();
     expect(connector1.getClient).toHaveBeenCalledTimes(1);
-    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
+    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
     expect(connector2.getClient).toHaveBeenCalledTimes(0);
 
-    await impl.forPlugin('plugin1').getClient();
+    await impl.forPlugin('plugin1', deps).getClient();
     expect(connector1.getClient).toHaveBeenCalledTimes(1);
-    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
+    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
     expect(connector2.getClient).toHaveBeenCalledTimes(0);
 
-    await impl.forPlugin('plugin2').getClient();
+    await impl.forPlugin('plugin2', deps).getClient();
     expect(connector1.getClient).toHaveBeenCalledTimes(2);
-    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin2', undefined);
+    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin2', deps);
     expect(connector2.getClient).toHaveBeenCalledTimes(0);
   });
 
@@ -80,16 +86,16 @@ describe('DatabaseManagerImpl', () => {
       },
     );
 
-    await impl.forPlugin('plugin1').getClient();
+    await impl.forPlugin('plugin1', deps).getClient();
     expect(connector1.getClient).toHaveBeenCalledTimes(1);
-    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
+    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
     expect(connector2.getClient).toHaveBeenCalledTimes(0);
 
-    await impl.forPlugin('plugin2').getClient();
+    await impl.forPlugin('plugin2', deps).getClient();
     expect(connector1.getClient).toHaveBeenCalledTimes(1);
-    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
+    expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
     expect(connector2.getClient).toHaveBeenCalledTimes(1);
-    expect(connector2.getClient).toHaveBeenLastCalledWith('plugin2', undefined);
+    expect(connector2.getClient).toHaveBeenLastCalledWith('plugin2', deps);
   });
 
   it('migration skip options take precedence over config', async () => {
@@ -112,7 +118,7 @@ describe('DatabaseManagerImpl', () => {
       },
       { migrations: { skip: false } },
     );
-    expect((await impl.forPlugin('plugin1')).migrations).toEqual({
+    expect((await impl.forPlugin('plugin1', deps)).migrations).toEqual({
       skip: false,
     });
 
@@ -120,7 +126,7 @@ describe('DatabaseManagerImpl', () => {
       pg: connector,
     });
 
-    expect((await impl1.forPlugin('plugin1')).migrations).toEqual({
+    expect((await impl1.forPlugin('plugin1', deps)).migrations).toEqual({
       skip: false,
     });
   });
@@ -142,10 +148,10 @@ describe('DatabaseManagerImpl', () => {
       },
     );
 
-    expect((await impl.forPlugin('plugin1')).migrations).toEqual({
+    expect((await impl.forPlugin('plugin1', deps)).migrations).toEqual({
       skip: true,
     });
-    expect((await impl.forPlugin('plugin2')).migrations).toEqual({
+    expect((await impl.forPlugin('plugin2', deps)).migrations).toEqual({
       skip: false,
     });
 
@@ -163,10 +169,10 @@ describe('DatabaseManagerImpl', () => {
         pg: connector,
       },
     );
-    expect((await impl2.forPlugin('plugin1')).migrations).toEqual({
+    expect((await impl2.forPlugin('plugin1', deps)).migrations).toEqual({
       skip: false,
     });
-    expect((await impl2.forPlugin('plugin2')).migrations).toEqual({
+    expect((await impl2.forPlugin('plugin2', deps)).migrations).toEqual({
       skip: true,
     });
   });

packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts

@@ -18,7 +18,6 @@ import {
   DatabaseService,
   LifecycleService,
   LoggerService,
-  PluginMetadataService,
   RootConfigService,
 } from '@backstage/backend-plugin-api';
 import { Config } from '@backstage/config';
@@ -43,7 +42,6 @@ function pluginPath(pluginId: string): string {
  */
 export type DatabaseManagerOptions = {
   migrations?: DatabaseService['migrations'];
-  logger?: LoggerService;
 };
 
 /**
@@ -66,9 +64,9 @@ export class DatabaseManagerImpl {
    */
   forPlugin(
     pluginId: string,
-    deps?: {
+    deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): PluginDatabaseManager {
     const client = this.getClientType(pluginId).client;
@@ -128,9 +126,9 @@ export class DatabaseManagerImpl {
   private async getDatabase(
     pluginId: string,
     connector: Connector,
-    deps?: {
+    deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): Promise<Knex> {
     if (this.databaseCache.has(pluginId)) {
@@ -141,13 +139,19 @@ export class DatabaseManagerImpl {
     this.databaseCache.set(pluginId, clientPromise);
 
     if (process.env.NODE_ENV !== 'test') {
-      clientPromise.then(client => this.startKeepaliveLoop(pluginId, client));
+      clientPromise.then(client =>
+        this.startKeepaliveLoop(pluginId, client, deps.logger),
+      );
     }
 
     return clientPromise;
   }
 
-  private startKeepaliveLoop(pluginId: string, client: Knex): void {
+  private startKeepaliveLoop(
+    pluginId: string,
+    client: Knex,
+    logger: LoggerService,
+  ): void {
     let lastKeepaliveFailed = false;
 
     setInterval(() => {
@@ -160,7 +164,7 @@ export class DatabaseManagerImpl {
         (error: unknown) => {
           if (!lastKeepaliveFailed) {
             lastKeepaliveFailed = true;
-            this.options?.logger?.warn(
+            logger.warn(
               `Database keepalive failed for plugin ${pluginId}, ${stringifyError(
                 error,
               )}`,
@@ -225,9 +229,9 @@ export class DatabaseManager {
    */
   forPlugin(
     pluginId: string,
-    deps?: {
+    deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): PluginDatabaseManager {
     return this.impl.forPlugin(pluginId, deps);

packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts

@@ -14,10 +14,7 @@
  * limitations under the License.
  */
 
-import {
-  LifecycleService,
-  PluginMetadataService,
-} from '@backstage/backend-plugin-api';
+import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
 import { Config, ConfigReader } from '@backstage/config';
 import { InputError } from '@backstage/errors';
 import { JsonObject } from '@backstage/types';
@@ -253,9 +250,9 @@ export class MysqlConnector implements Connector {
 
   async getClient(
     pluginId: string,
-    _deps?: {
+    _deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): Promise<Knex> {
     const pluginConfig = new ConfigReader(

packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts

@@ -14,10 +14,7 @@
  * limitations under the License.
  */
 
-import {
-  LifecycleService,
-  PluginMetadataService,
-} from '@backstage/backend-plugin-api';
+import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
 import { Config, ConfigReader } from '@backstage/config';
 import { ForwardedError } from '@backstage/errors';
 import { JsonObject } from '@backstage/types';
@@ -261,9 +258,9 @@ export class PgConnector implements Connector {
 
   async getClient(
     pluginId: string,
-    _deps?: {
+    _deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): Promise<Knex> {
     const pluginConfig = new ConfigReader(

packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.test.ts

@@ -20,11 +20,17 @@ import {
   buildSqliteDatabaseConfig,
   createSqliteDatabaseClient,
 } from './sqlite3';
+import { mockServices } from '@backstage/backend-test-utils';
 
 describe('better-sqlite3', () => {
   const createConfig = (connection: any) =>
     new ConfigReader({ client: 'better-sqlite3', connection });
 
+  const deps = {
+    logger: mockServices.logger.mock(),
+    lifecycle: mockServices.lifecycle.mock(),
+  };
+
   describe('buildSqliteDatabaseConfig', () => {
     it('builds an in-memory connection', () => {
       expect(buildSqliteDatabaseConfig(createConfig(':memory:'))).toEqual({
@@ -90,7 +96,9 @@ describe('better-sqlite3', () => {
 
   describe('createSqliteDatabaseClient', () => {
     it('creates an in memory knex instance', () => {
-      expect(createSqliteDatabaseClient(createConfig(':memory:'))).toBeTruthy();
+      expect(
+        createSqliteDatabaseClient('p', createConfig(':memory:'), deps),
+      ).toBeTruthy();
     });
   });
 });

packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts

@@ -15,10 +15,7 @@
  */
 
 import { DevDataStore } from '@backstage/backend-dev-utils';
-import {
-  LifecycleService,
-  PluginMetadataService,
-} from '@backstage/backend-plugin-api';
+import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
 import { Config, ConfigReader } from '@backstage/config';
 import { JsonObject } from '@backstage/types';
 import { ensureDirSync } from 'fs-extra';
@@ -35,12 +32,13 @@ import { mergeDatabaseConfig } from './mergeDatabaseConfig';
  * @param overrides - Additional options to merge with the config
  */
 export function createSqliteDatabaseClient(
+  pluginId: string,
   dbConfig: Config,
-  overrides?: Knex.Config,
-  deps?: {
+  deps: {
+    logger: LoggerService;
     lifecycle: LifecycleService;
-    pluginMetadata: PluginMetadataService;
   },
+  overrides?: Knex.Config,
 ) {
   const knexConfig = buildSqliteDatabaseConfig(dbConfig, overrides);
   const connConfig = knexConfig.connection as Knex.Sqlite3ConnectionConfig;
@@ -62,7 +60,7 @@ export function createSqliteDatabaseClient(
     const devStore = DevDataStore.get();
 
     if (devStore) {
-      const dataKey = `sqlite3-db-${deps.pluginMetadata.getId()}`;
+      const dataKey = `sqlite3-db-${pluginId}`;
 
       const connectionLoader = async () => {
         // If seed data is available, use it tconnectionLoader restore the database
@@ -180,9 +178,9 @@ export class Sqlite3Connector implements Connector {
 
   async getClient(
     pluginId: string,
-    deps?: {
+    deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): Promise<Knex> {
     const pluginConfig = new ConfigReader(
@@ -202,9 +200,10 @@ export class Sqlite3Connector implements Connector {
     );
 
     const client = createSqliteDatabaseClient(
+      pluginId,
       pluginConfig,
-      databaseClientOverrides,
       deps,
+      databaseClientOverrides,
     );
 
     return client;

packages/backend-defaults/src/entrypoints/database/databaseServiceFactory.ts

@@ -35,6 +35,7 @@ export const databaseServiceFactory = createServiceFactory({
   deps: {
     config: coreServices.rootConfig,
     lifecycle: coreServices.lifecycle,
+    logger: coreServices.logger,
     pluginMetadata: coreServices.pluginMetadata,
   },
   async createRootContext({ config }) {
@@ -48,10 +49,10 @@ export const databaseServiceFactory = createServiceFactory({
           }),
         );
   },
-  async factory({ pluginMetadata, lifecycle }, databaseManager) {
+  async factory({ pluginMetadata, lifecycle, logger }, databaseManager) {
     return databaseManager.forPlugin(pluginMetadata.getId(), {
-      pluginMetadata,
       lifecycle,
+      logger,
     });
   },
 });

packages/backend-defaults/src/entrypoints/database/types.ts

@@ -14,10 +14,7 @@
  * limitations under the License.
  */
 
-import {
-  LifecycleService,
-  PluginMetadataService,
-} from '@backstage/backend-plugin-api';
+import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
 import { Knex } from 'knex';
 
 export type { DatabaseService as PluginDatabaseManager } from '@backstage/backend-plugin-api';
@@ -25,9 +22,9 @@ export type { DatabaseService as PluginDatabaseManager } from '@backstage/backen
 export interface Connector {
   getClient(
     pluginId: string,
-    deps?: {
+    deps: {
+      logger: LoggerService;
       lifecycle: LifecycleService;
-      pluginMetadata: PluginMetadataService;
     },
   ): Promise<Knex>;
 }