From 055b75b39f60406d77b91c39eb05407d7214744b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 30 Aug 2024 13:20:18 +0200 Subject: [PATCH 1/5] remove deprecated exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/wise-forks-play.md | 10 ++++++ packages/backend-common/api-report.md | 12 +------ .../backend-common/src/deprecated/index.ts | 16 +--------- .../backend-defaults/api-report-database.md | 14 +------- .../entrypoints/database/DatabaseManager.ts | 32 ++----------------- .../src/entrypoints/database/index.ts | 2 -- 6 files changed, 15 insertions(+), 71 deletions(-) create mode 100644 .changeset/wise-forks-play.md diff --git a/.changeset/wise-forks-play.md b/.changeset/wise-forks-play.md new file mode 100644 index 0000000000..84a6da9003 --- /dev/null +++ b/.changeset/wise-forks-play.md @@ -0,0 +1,10 @@ +--- +'@backstage/backend-common': minor +'@backstage/backend-defaults': minor +--- + +**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 return type from `DatabaseManager.forPlugin` is now directly a `DatabaseService`, as arguably expected. diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 7de2731518..2555ab6f5b 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -143,7 +143,7 @@ export function createStatusCheckRouter(options: { }): Promise; // @public @deprecated (undocumented) -export class DatabaseManager implements LegacyRootDatabaseService { +export class DatabaseManager { // (undocumented) forPlugin( pluginId: string, @@ -173,11 +173,6 @@ export class DockerContainerRunner implements ContainerRunner { runContainer(options: RunContainerOptions): Promise; } -// Warning: (ae-forgotten-export) The symbol "dropDatabase_2" needs to be exported by the entry point index.d.ts -// -// @public @deprecated (undocumented) -export const dropDatabase: typeof dropDatabase_2; - // @public @deprecated export function errorHandler( options?: ErrorHandlerOptions, @@ -367,11 +362,6 @@ export const legacyPlugin: ( }>, ) => BackendFeature; -// Warning: (ae-forgotten-export) The symbol "LegacyRootDatabaseService_2" needs to be exported by the entry point index.d.ts -// -// @public @deprecated (undocumented) -export type LegacyRootDatabaseService = LegacyRootDatabaseService_2; - // @public @deprecated export function loadBackendConfig(options: { logger: LoggerService; diff --git a/packages/backend-common/src/deprecated/index.ts b/packages/backend-common/src/deprecated/index.ts index 5dfcd322ed..d34b81aa92 100644 --- a/packages/backend-common/src/deprecated/index.ts +++ b/packages/backend-common/src/deprecated/index.ts @@ -30,10 +30,8 @@ import { // eslint-disable-next-line @backstage/no-relative-monorepo-imports import { - dropDatabase as _dropDatabase, DatabaseManager as _DatabaseManager, type DatabaseManagerOptions as _DatabaseManagerOptions, - type LegacyRootDatabaseService as _LegacyRootDatabaseService, } from '../../../backend-defaults/src/entrypoints/database/DatabaseManager'; import { @@ -169,7 +167,7 @@ export type CacheClientOptions = CacheServiceOptions; * @public * @deprecated Use `DatabaseManager` from the `@backstage/backend-defaults` package instead */ -export class DatabaseManager implements LegacyRootDatabaseService { +export class DatabaseManager { private constructor(private readonly _databaseManager: _DatabaseManager) {} static fromConfig( @@ -202,18 +200,6 @@ export type DatabaseManagerOptions = _DatabaseManagerOptions; */ export type PluginDatabaseManager = _PluginDatabaseManager; -/** - * @public - * @deprecated Use `LegacyRootDatabaseService` from the `@backstage/backend-defaults` package instead - */ -export type LegacyRootDatabaseService = _LegacyRootDatabaseService; - -/** - * @public - * @deprecated Use `dropDatabase` from the `@backstage/backend-defaults` package instead - */ -export const dropDatabase = _dropDatabase; - /** * @public * @deprecated This function is deprecated and will be removed in a future release, see https://github.com/backstage/backstage/issues/24493. diff --git a/packages/backend-defaults/api-report-database.md b/packages/backend-defaults/api-report-database.md index 49ffa75ca7..8f3857fe23 100644 --- a/packages/backend-defaults/api-report-database.md +++ b/packages/backend-defaults/api-report-database.md @@ -3,7 +3,6 @@ > Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). ```ts -import { Config } from '@backstage/config'; import { DatabaseService } from '@backstage/backend-plugin-api'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { LoggerService } from '@backstage/backend-plugin-api'; @@ -12,7 +11,7 @@ import { RootConfigService } from '@backstage/backend-plugin-api'; import { ServiceFactory } from '@backstage/backend-plugin-api'; // @public -export class DatabaseManager implements LegacyRootDatabaseService { +export class DatabaseManager { forPlugin( pluginId: string, deps?: { @@ -39,16 +38,5 @@ export const databaseServiceFactory: ServiceFactory< 'singleton' >; -// @public @deprecated -export function dropDatabase( - dbConfig: Config, - ...databaseNames: string[] -): Promise; - -// @public -export type LegacyRootDatabaseService = { - forPlugin(pluginId: string): DatabaseService; -}; - // (No @packageDocumentation comment for this package) ``` diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts index 88d95443bb..1230280e07 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts +++ b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts @@ -46,18 +46,10 @@ export type DatabaseManagerOptions = { logger?: LoggerService; }; -/** - * An interface that represents the legacy global DatabaseManager implementation. - * @public - */ -export type LegacyRootDatabaseService = { - forPlugin(pluginId: string): DatabaseService; -}; - /** * Testable implementation class for {@link DatabaseManager} below. */ -export class DatabaseManagerImpl implements LegacyRootDatabaseService { +export class DatabaseManagerImpl { constructor( private readonly config: Config, private readonly connectors: Record, @@ -193,7 +185,7 @@ export class DatabaseManagerImpl implements LegacyRootDatabaseService { * set `prefix` which is used to prefix generated database names if config is * not provided. */ -export class DatabaseManager implements LegacyRootDatabaseService { +export class DatabaseManager { /** * Creates a {@link DatabaseManager} from `backend.database` config. * @@ -241,23 +233,3 @@ export class DatabaseManager implements LegacyRootDatabaseService { return this.impl.forPlugin(pluginId, deps); } } - -/** - * Helper for deleting databases. - * - * @public - * @deprecated Will be removed in a future release. - */ -export async function dropDatabase( - dbConfig: Config, - ...databaseNames: string[] -): Promise { - const client = dbConfig.getString('client'); - const prefix = dbConfig.getOptionalString('prefix') || 'backstage_plugin_'; - - if (client === 'pg') { - await new PgConnector(dbConfig, prefix).dropDatabase(...databaseNames); - } else if (client === 'mysql' || client === 'mysql2') { - await new MysqlConnector(dbConfig, prefix).dropDatabase(...databaseNames); - } -} diff --git a/packages/backend-defaults/src/entrypoints/database/index.ts b/packages/backend-defaults/src/entrypoints/database/index.ts index 7d6221b856..e30d06cbdd 100644 --- a/packages/backend-defaults/src/entrypoints/database/index.ts +++ b/packages/backend-defaults/src/entrypoints/database/index.ts @@ -18,6 +18,4 @@ export { databaseServiceFactory } from './databaseServiceFactory'; export { DatabaseManager, type DatabaseManagerOptions, - type LegacyRootDatabaseService, - dropDatabase, } from './DatabaseManager'; From fd66330d4a375555b4891190cb83805297d12986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 30 Aug 2024 13:42:38 +0200 Subject: [PATCH 2/5] remove unused DatabaseConnector type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/entrypoints/database/types.ts | 65 ------------------- 1 file changed, 65 deletions(-) diff --git a/packages/backend-defaults/src/entrypoints/database/types.ts b/packages/backend-defaults/src/entrypoints/database/types.ts index a9cceaa1f9..74f41581f6 100644 --- a/packages/backend-defaults/src/entrypoints/database/types.ts +++ b/packages/backend-defaults/src/entrypoints/database/types.ts @@ -18,75 +18,10 @@ import { LifecycleService, PluginMetadataService, } from '@backstage/backend-plugin-api'; -import { Config } from '@backstage/config'; import { Knex } from 'knex'; export type { DatabaseService as PluginDatabaseManager } from '@backstage/backend-plugin-api'; -/** - * Manages an underlying Knex database driver. - */ -export interface DatabaseConnector { - /** - * Provides an instance of a knex database connector. - */ - createClient( - dbConfig: Config, - overrides?: Partial, - deps?: { - lifecycle: LifecycleService; - pluginMetadata: PluginMetadataService; - }, - ): Knex; - - /** - * Provides a partial knex config sufficient to override a database name. - */ - createNameOverride(name: string): Partial; - - /** - * Provides a partial knex config sufficient to override a PostgreSQL schema - * name within utilizing the `searchPath` knex configuration. - */ - createSchemaOverride?(name: string): Partial; - - /** - * Produces a knex connection config object representing a database connection - * string. - */ - parseConnectionString( - connectionString: string, - client?: string, - ): Knex.StaticConnectionConfig; - - /** - * Performs a side-effect to ensure database names passed in are present. - * - * Calling this function on databases which already exist should do nothing. - * Missing databases should be created if needed. - */ - ensureDatabaseExists?( - dbConfig: Config, - ...databases: Array - ): Promise; - - /** - * Performs a side-effect to ensure schema names passed in are present. - * - * Calling this function on schemas which already exist should do nothing. - * Missing schemas should be created if needed. - */ - ensureSchemaExists?( - dbConfig: Config, - ...schemas: Array - ): Promise; - - /** - * Deletes databases. - */ - dropDatabase?(dbConfig: Config, ...databases: Array): Promise; -} - export interface Connector { getClient( pluginId: string, From e46754a18b1bcd9e63e3ae6a56b80be3eaf52c7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 30 Aug 2024 13:46:40 +0200 Subject: [PATCH 3/5] remove the unused dropDatabase in the connector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/entrypoints/database/DatabaseManager.test.ts | 6 ------ .../src/entrypoints/database/connectors/mysql.ts | 4 ---- .../src/entrypoints/database/connectors/postgres.ts | 4 ---- .../src/entrypoints/database/connectors/sqlite3.ts | 4 ---- packages/backend-defaults/src/entrypoints/database/types.ts | 2 -- 5 files changed, 20 deletions(-) diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts index 0d52ff16ff..fca6080329 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts +++ b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts @@ -26,11 +26,9 @@ describe('DatabaseManagerImpl', () => { it('calls the right connector, only once per plugin id', async () => { const connector1 = { getClient: jest.fn(), - dropDatabase: jest.fn(), } satisfies Connector; const connector2 = { getClient: jest.fn(), - dropDatabase: jest.fn(), } satisfies Connector; const impl = new DatabaseManagerImpl( @@ -62,11 +60,9 @@ describe('DatabaseManagerImpl', () => { it('respects per-plugin overridden connectors', async () => { const connector1 = { getClient: jest.fn(), - dropDatabase: jest.fn(), } satisfies Connector; const connector2 = { getClient: jest.fn(), - dropDatabase: jest.fn(), } satisfies Connector; const impl = new DatabaseManagerImpl( @@ -99,7 +95,6 @@ describe('DatabaseManagerImpl', () => { it('migration skip options take precedence over config', async () => { const connector = { getClient: jest.fn(), - dropDatabase: jest.fn(), } satisfies Connector; const impl = new DatabaseManagerImpl( @@ -133,7 +128,6 @@ describe('DatabaseManagerImpl', () => { it('plugin can skip migrations using config', async () => { const connector = { getClient: jest.fn(), - dropDatabase: jest.fn(), } satisfies Connector; const impl = new DatabaseManagerImpl( diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts b/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts index b11b659a6d..f06db5999d 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts @@ -293,10 +293,6 @@ export class MysqlConnector implements Connector { return client; } - async dropDatabase(...databaseNames: string[]): Promise { - return await dropMysqlDatabase(this.config, ...databaseNames); - } - /** * Provides the canonical database name for a given plugin. * diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index 46b958789d..18418a271a 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts @@ -312,10 +312,6 @@ export class PgConnector implements Connector { return client; } - async dropDatabase(...databaseNames: string[]): Promise { - return await dropPgDatabase(this.config, ...databaseNames); - } - /** * Provides the canonical database name for a given plugin. * diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts b/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts index 97ba6d46fd..77e8303390 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts +++ b/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts @@ -210,10 +210,6 @@ export class Sqlite3Connector implements Connector { return client; } - async dropDatabase(..._databaseNames: string[]): Promise { - // do nothing - } - /** * Provides the canonical database name for a given plugin. * diff --git a/packages/backend-defaults/src/entrypoints/database/types.ts b/packages/backend-defaults/src/entrypoints/database/types.ts index 74f41581f6..57b85ff3c1 100644 --- a/packages/backend-defaults/src/entrypoints/database/types.ts +++ b/packages/backend-defaults/src/entrypoints/database/types.ts @@ -30,6 +30,4 @@ export interface Connector { pluginMetadata: PluginMetadataService; }, ): Promise; - - dropDatabase(...databaseNames: string[]): Promise; } From 85f50d43a183a8ef03749908ce3b7c2888e9ec67 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 30 Aug 2024 14:29:52 +0200 Subject: [PATCH 4/5] force the passing of deps to the database manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/wise-forks-play.md | 3 +- packages/backend-common/api-report.md | 5 ++- .../backend-common/src/deprecated/index.ts | 29 ++++++++++++-- .../backend-defaults/api-report-database.md | 6 +-- .../database/DatabaseManager.test.ts | 40 +++++++++++-------- .../entrypoints/database/DatabaseManager.ts | 26 +++++++----- .../entrypoints/database/connectors/mysql.ts | 9 ++--- .../database/connectors/postgres.ts | 9 ++--- .../database/connectors/sqlite3.test.ts | 10 ++++- .../database/connectors/sqlite3.ts | 21 +++++----- .../database/databaseServiceFactory.ts | 5 ++- .../src/entrypoints/database/types.ts | 9 ++--- 12 files changed, 102 insertions(+), 70 deletions(-) diff --git a/.changeset/wise-forks-play.md b/.changeset/wise-forks-play.md index 84a6da9003..c5348a8add 100644 --- a/.changeset/wise-forks-play.md +++ b/.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. diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 2555ab6f5b..8a895dd037 100644 --- a/packages/backend-common/api-report.md +++ b/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; } diff --git a/packages/backend-common/src/deprecated/index.ts b/packages/backend-common/src/deprecated/index.ts index d34b81aa92..3b4b518de1 100644 --- a/packages/backend-common/src/deprecated/index.ts +++ b/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 }); } } diff --git a/packages/backend-defaults/api-report-database.md b/packages/backend-defaults/api-report-database.md index 8f3857fe23..d9e9e9e287 100644 --- a/packages/backend-defaults/api-report-database.md +++ b/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 diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts index fca6080329..26c2d54231 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.test.ts +++ b/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, }); }); diff --git a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts b/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts index 1230280e07..46d4ee8cf7 100644 --- a/packages/backend-defaults/src/entrypoints/database/DatabaseManager.ts +++ b/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 { 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); diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts b/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts index f06db5999d..d5bd88deef 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/mysql.ts +++ b/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 { const pluginConfig = new ConfigReader( diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts b/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts index 18418a271a..b5cfffd1f4 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/postgres.ts +++ b/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 { const pluginConfig = new ConfigReader( diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.test.ts b/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.test.ts index 2bd517d373..769d402eda 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.test.ts +++ b/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(); }); }); }); diff --git a/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts b/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts index 77e8303390..67dd9d975f 100644 --- a/packages/backend-defaults/src/entrypoints/database/connectors/sqlite3.ts +++ b/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 { const pluginConfig = new ConfigReader( @@ -202,9 +200,10 @@ export class Sqlite3Connector implements Connector { ); const client = createSqliteDatabaseClient( + pluginId, pluginConfig, - databaseClientOverrides, deps, + databaseClientOverrides, ); return client; diff --git a/packages/backend-defaults/src/entrypoints/database/databaseServiceFactory.ts b/packages/backend-defaults/src/entrypoints/database/databaseServiceFactory.ts index 3bdf1790f3..167779e807 100644 --- a/packages/backend-defaults/src/entrypoints/database/databaseServiceFactory.ts +++ b/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, }); }, }); diff --git a/packages/backend-defaults/src/entrypoints/database/types.ts b/packages/backend-defaults/src/entrypoints/database/types.ts index 57b85ff3c1..beded85d65 100644 --- a/packages/backend-defaults/src/entrypoints/database/types.ts +++ b/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; } From 988c1454bdb3c414eaebc9c593c396daafa2b191 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Fri, 30 Aug 2024 15:16:12 +0200 Subject: [PATCH 5/5] put back the types in common MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/chilly-birds-shout.md | 7 +++++++ .changeset/wise-forks-play.md | 1 - packages/backend-common/api-report.md | 7 ++++++- packages/backend-common/src/deprecated/index.ts | 10 +++++++++- 4 files changed, 22 insertions(+), 3 deletions(-) create mode 100644 .changeset/chilly-birds-shout.md diff --git a/.changeset/chilly-birds-shout.md b/.changeset/chilly-birds-shout.md new file mode 100644 index 0000000000..dfa99b6f5d --- /dev/null +++ b/.changeset/chilly-birds-shout.md @@ -0,0 +1,7 @@ +--- +'@backstage/backend-common': minor +--- + +**BREAKING**: Simplifications and cleanup as part of the Backend System 1.0 work. + +- The deprecated `dropDatabase` function has now been removed, without replacement. diff --git a/.changeset/wise-forks-play.md b/.changeset/wise-forks-play.md index c5348a8add..3846b3a6f0 100644 --- a/.changeset/wise-forks-play.md +++ b/.changeset/wise-forks-play.md @@ -1,5 +1,4 @@ --- -'@backstage/backend-common': minor '@backstage/backend-defaults': minor --- diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 8a895dd037..450bee9063 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -143,7 +143,7 @@ export function createStatusCheckRouter(options: { }): Promise; // @public @deprecated (undocumented) -export class DatabaseManager { +export class DatabaseManager implements LegacyRootDatabaseService { // (undocumented) forPlugin( pluginId: string, @@ -365,6 +365,11 @@ export const legacyPlugin: ( }>, ) => BackendFeature; +// @public @deprecated (undocumented) +export type LegacyRootDatabaseService = { + forPlugin(pluginId: string): DatabaseService; +}; + // @public @deprecated export function loadBackendConfig(options: { logger: LoggerService; diff --git a/packages/backend-common/src/deprecated/index.ts b/packages/backend-common/src/deprecated/index.ts index 3b4b518de1..7f18bd188e 100644 --- a/packages/backend-common/src/deprecated/index.ts +++ b/packages/backend-common/src/deprecated/index.ts @@ -169,7 +169,7 @@ export type CacheClientOptions = CacheServiceOptions; * @public * @deprecated Use `DatabaseManager` from the `@backstage/backend-defaults` package instead */ -export class DatabaseManager { +export class DatabaseManager implements LegacyRootDatabaseService { private constructor( private readonly _databaseManager: _DatabaseManager, private readonly logger?: LoggerService, @@ -221,6 +221,14 @@ export type DatabaseManagerOptions = _DatabaseManagerOptions; */ export type PluginDatabaseManager = _PluginDatabaseManager; +/** + * @public + * @deprecated Use `DatabaseManager` from `@backstage/backend-defaults/database` instead, or migrate to the new backend system and use `coreServices.database` + */ +export type LegacyRootDatabaseService = { + forPlugin(pluginId: string): DatabaseService; +}; + /** * @public * @deprecated This function is deprecated and will be removed in a future release, see https://github.com/backstage/backstage/issues/24493.