From 86e1fbde1d73d1c10fac5271c42a8b9097ca01e1 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Wed, 1 Dec 2021 14:17:49 +0100 Subject: [PATCH 01/10] Add runMigrations argument to DatabaseManager Signed-off-by: Marcus Eide --- .../backend-common/src/database/DatabaseManager.ts | 12 +++++++++++- packages/backend-common/src/database/types.ts | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index cf5e801d66..495dcfd303 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -47,19 +47,25 @@ export class DatabaseManager { * names if config is not provided. * * @param config - The loaded application configuration. + * @param runMigrations - Controls whether or not to perform database migrations. */ - static fromConfig(config: Config): DatabaseManager { + static fromConfig( + config: Config, + runMigrations?: boolean | (() => boolean), + ): DatabaseManager { const databaseConfig = config.getConfig('backend.database'); return new DatabaseManager( databaseConfig, databaseConfig.getOptionalString('prefix'), + runMigrations, ); } private constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', + private readonly runMigrations: boolean | (() => boolean) = true, ) {} /** @@ -76,6 +82,10 @@ export class DatabaseManager { getClient(): Promise { return _this.getDatabase(pluginId); }, + runMigrations: + typeof _this.runMigrations === 'function' + ? _this.runMigrations() + : _this.runMigrations, }; } diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index e96f86980b..3dad57ff62 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -30,6 +30,14 @@ export interface PluginDatabaseManager { * stores so that plugins are discouraged from database integration. */ getClient(): Promise; + + /** + * runMigrations can be used to determine if database migrations + * should be performed. + * + * Useful if connecting to a read-only database. + */ + runMigrations: boolean; } /** From b4588ffdb1db84762ea0d748931962ca23ae45a5 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Wed, 1 Dec 2021 14:18:53 +0100 Subject: [PATCH 02/10] Conditionally run db migrations in catalog backend Signed-off-by: Marcus Eide --- plugins/catalog-backend/src/service/NextCatalogBuilder.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts index c9ebc8f723..d85fd741a5 100644 --- a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts @@ -336,7 +336,10 @@ export class NextCatalogBuilder { const parser = this.parser || defaultEntityDataParser; const dbClient = await database.getClient(); - await applyDatabaseMigrations(dbClient); + if (database.runMigrations) { + logger.info('Performing database migration'); + await applyDatabaseMigrations(dbClient); + } const db = new CommonDatabase(dbClient, logger); From 5cb156ef3b0b6c23467d3980e58520a3d06b181e Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Wed, 1 Dec 2021 14:19:21 +0100 Subject: [PATCH 03/10] Add tests for runMigrations argument when creating a database manager Signed-off-by: Marcus Eide --- .../src/database/DatabaseManager.test.ts | 52 ++++++++++++++----- 1 file changed, 40 insertions(+), 12 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index f2fe855234..56fb8ddc3e 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -36,25 +36,53 @@ describe('DatabaseManager', () => { afterEach(() => jest.resetAllMocks()); describe('DatabaseManager.fromConfig', () => { - it('accesses the backend.database key', () => { - const config = new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, + const backendConfig = { + backend: { + database: { + client: 'pg', + connection: { + host: 'localhost', + user: 'foo', + password: 'bar', + database: 'foodb', }, }, - }); + }, + }; + + it('accesses the backend.database key', () => { + const config = new ConfigReader(backendConfig); const getConfigSpy = jest.spyOn(config, 'getConfig'); DatabaseManager.fromConfig(config); expect(getConfigSpy).toHaveBeenCalledWith('backend.database'); }); + + it('runMigrate default value', () => { + const config = new ConfigReader(backendConfig); + const database = DatabaseManager.fromConfig(config); + const client = database.forPlugin('test'); + + expect(client.runMigrations).toBe(true); + }); + + it('runMigrate as a function', () => { + const config = new ConfigReader(backendConfig); + const runMigrate = jest.fn().mockReturnValue(false); + const database = DatabaseManager.fromConfig(config, runMigrate); + const client = database.forPlugin('test'); + + expect(runMigrate).toHaveBeenCalledTimes(1); + expect(client.runMigrations).toBe(false); + }); + + it('runMigrate as a boolean', () => { + const config = new ConfigReader(backendConfig); + const database = DatabaseManager.fromConfig(config, false); + const client = database.forPlugin('test'); + + expect(client.runMigrations).toBe(false); + }); }); describe('DatabaseManager.forPlugin', () => { From 2f45b2987573abb267c4bdfbbcdb5f7846cbe7f4 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Wed, 1 Dec 2021 16:21:40 +0100 Subject: [PATCH 04/10] Change runMigrations to only accept a boolean Signed-off-by: Marcus Eide --- .../src/database/DatabaseManager.test.ts | 14 ++------------ .../backend-common/src/database/DatabaseManager.ts | 12 +++--------- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index 56fb8ddc3e..895afb4af4 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -58,7 +58,7 @@ describe('DatabaseManager', () => { expect(getConfigSpy).toHaveBeenCalledWith('backend.database'); }); - it('runMigrate default value', () => { + it('runMigrations defaults to true', () => { const config = new ConfigReader(backendConfig); const database = DatabaseManager.fromConfig(config); const client = database.forPlugin('test'); @@ -66,17 +66,7 @@ describe('DatabaseManager', () => { expect(client.runMigrations).toBe(true); }); - it('runMigrate as a function', () => { - const config = new ConfigReader(backendConfig); - const runMigrate = jest.fn().mockReturnValue(false); - const database = DatabaseManager.fromConfig(config, runMigrate); - const client = database.forPlugin('test'); - - expect(runMigrate).toHaveBeenCalledTimes(1); - expect(client.runMigrations).toBe(false); - }); - - it('runMigrate as a boolean', () => { + it('runMigrations can be set', () => { const config = new ConfigReader(backendConfig); const database = DatabaseManager.fromConfig(config, false); const client = database.forPlugin('test'); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 495dcfd303..d5dbd90b79 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -49,10 +49,7 @@ export class DatabaseManager { * @param config - The loaded application configuration. * @param runMigrations - Controls whether or not to perform database migrations. */ - static fromConfig( - config: Config, - runMigrations?: boolean | (() => boolean), - ): DatabaseManager { + static fromConfig(config: Config, runMigrations?: boolean): DatabaseManager { const databaseConfig = config.getConfig('backend.database'); return new DatabaseManager( @@ -65,7 +62,7 @@ export class DatabaseManager { private constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', - private readonly runMigrations: boolean | (() => boolean) = true, + private readonly runMigrations: boolean = true, ) {} /** @@ -82,10 +79,7 @@ export class DatabaseManager { getClient(): Promise { return _this.getDatabase(pluginId); }, - runMigrations: - typeof _this.runMigrations === 'function' - ? _this.runMigrations() - : _this.runMigrations, + runMigrations: _this.runMigrations, }; } From 70c46a708b626eb6b81a44280a59c22ed1a92fbc Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Thu, 2 Dec 2021 13:05:31 +0100 Subject: [PATCH 05/10] Add options with migrations category Signed-off-by: Marcus Eide --- .../src/database/DatabaseManager.test.ts | 12 +++++++----- .../src/database/DatabaseManager.ts | 17 ++++++++++++----- packages/backend-common/src/database/types.ts | 17 ++++++++++++----- .../src/service/NextCatalogBuilder.ts | 2 +- 4 files changed, 32 insertions(+), 16 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index 895afb4af4..2905775494 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -58,20 +58,22 @@ describe('DatabaseManager', () => { expect(getConfigSpy).toHaveBeenCalledWith('backend.database'); }); - it('runMigrations defaults to true', () => { + it('handles default options', () => { const config = new ConfigReader(backendConfig); const database = DatabaseManager.fromConfig(config); const client = database.forPlugin('test'); - expect(client.runMigrations).toBe(true); + expect(client.migrations?.apply).toBe(true); }); - it('runMigrations can be set', () => { + it('handles migrations options', () => { const config = new ConfigReader(backendConfig); - const database = DatabaseManager.fromConfig(config, false); + const database = DatabaseManager.fromConfig(config, { + migrations: { apply: false }, + }); const client = database.forPlugin('test'); - expect(client.runMigrations).toBe(false); + expect(client.migrations?.apply).toBe(false); }); }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index d5dbd90b79..2c76c0f163 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -36,6 +36,10 @@ function pluginPath(pluginId: string): string { return `plugin.${pluginId}`; } +type Options = { + migrations?: PluginDatabaseManager['migrations']; +}; + /** @public */ export class DatabaseManager { /** @@ -47,22 +51,22 @@ export class DatabaseManager { * names if config is not provided. * * @param config - The loaded application configuration. - * @param runMigrations - Controls whether or not to perform database migrations. + * @param options - An optional configuration object. */ - static fromConfig(config: Config, runMigrations?: boolean): DatabaseManager { + static fromConfig(config: Config, options?: Options): DatabaseManager { const databaseConfig = config.getConfig('backend.database'); return new DatabaseManager( databaseConfig, databaseConfig.getOptionalString('prefix'), - runMigrations, + options, ); } private constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', - private readonly runMigrations: boolean = true, + private readonly options?: Options, ) {} /** @@ -74,12 +78,15 @@ export class DatabaseManager { */ forPlugin(pluginId: string): PluginDatabaseManager { const _this = this; + const defaultMigrationOptions = { + apply: true, + }; return { getClient(): Promise { return _this.getDatabase(pluginId); }, - runMigrations: _this.runMigrations, + migrations: _this.options?.migrations ?? defaultMigrationOptions, }; } diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 3dad57ff62..3c5bbf19bb 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -32,12 +32,19 @@ export interface PluginDatabaseManager { getClient(): Promise; /** - * runMigrations can be used to determine if database migrations - * should be performed. - * - * Useful if connecting to a read-only database. + * This optional property is used to control the behavior of database migrations. */ - runMigrations: boolean; + migrations?: { + /** + * apply can be used to determine if database migrations + * should be performed. + * + * Useful if connecting to a read-only database. + * + * @default true + */ + apply: boolean; + }; } /** diff --git a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts index d85fd741a5..712ab0ae44 100644 --- a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts @@ -336,7 +336,7 @@ export class NextCatalogBuilder { const parser = this.parser || defaultEntityDataParser; const dbClient = await database.getClient(); - if (database.runMigrations) { + if (database.migrations?.apply) { logger.info('Performing database migration'); await applyDatabaseMigrations(dbClient); } From 7b61f2c3b6d9dbe1c495b5dbe518412cb9937d31 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Thu, 2 Dec 2021 14:19:02 +0100 Subject: [PATCH 06/10] Make migrations object required Signed-off-by: Marcus Eide --- .../src/database/DatabaseManager.test.ts | 4 ++-- .../backend-common/src/database/DatabaseManager.ts | 10 +++++----- packages/backend-common/src/database/types.ts | 4 ++-- packages/backend-tasks/src/tasks/TaskScheduler.test.ts | 1 + plugins/auth-backend/src/service/standaloneServer.ts | 1 + plugins/bazaar-backend/src/service/standaloneServer.ts | 2 +- .../src/legacy/service/CatalogBuilder.test.ts | 2 +- .../catalog-backend/src/service/NextCatalogBuilder.ts | 2 +- .../catalog-backend/src/service/standaloneServer.ts | 2 +- .../src/service/standaloneServer.ts | 2 +- .../tech-insights-backend/src/service/router.test.ts | 1 + 11 files changed, 17 insertions(+), 14 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index 2905775494..4a44feec4a 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -63,7 +63,7 @@ describe('DatabaseManager', () => { const database = DatabaseManager.fromConfig(config); const client = database.forPlugin('test'); - expect(client.migrations?.apply).toBe(true); + expect(client.migrations.apply).toBe(true); }); it('handles migrations options', () => { @@ -73,7 +73,7 @@ describe('DatabaseManager', () => { }); const client = database.forPlugin('test'); - expect(client.migrations?.apply).toBe(false); + expect(client.migrations.apply).toBe(false); }); }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 2c76c0f163..e564f2e0e6 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -37,7 +37,7 @@ function pluginPath(pluginId: string): string { } type Options = { - migrations?: PluginDatabaseManager['migrations']; + migrations: PluginDatabaseManager['migrations']; }; /** @public */ @@ -78,15 +78,15 @@ export class DatabaseManager { */ forPlugin(pluginId: string): PluginDatabaseManager { const _this = this; - const defaultMigrationOptions = { - apply: true, - }; return { getClient(): Promise { return _this.getDatabase(pluginId); }, - migrations: _this.options?.migrations ?? defaultMigrationOptions, + migrations: { + apply: true, + ..._this.options?.migrations, + }, }; } diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 3c5bbf19bb..4cfc86e240 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -32,9 +32,9 @@ export interface PluginDatabaseManager { getClient(): Promise; /** - * This optional property is used to control the behavior of database migrations. + * This property is used to control the behavior of database migrations. */ - migrations?: { + migrations: { /** * apply can be used to determine if database migrations * should be performed. diff --git a/packages/backend-tasks/src/tasks/TaskScheduler.test.ts b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts index ce8e797503..6c9a6989c7 100644 --- a/packages/backend-tasks/src/tasks/TaskScheduler.test.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts @@ -33,6 +33,7 @@ describe('TaskScheduler', () => { const databaseManager: Partial = { forPlugin: () => ({ getClient: async () => knex, + migrations: { apply: true }, }), }; return databaseManager as DatabaseManager; diff --git a/plugins/auth-backend/src/service/standaloneServer.ts b/plugins/auth-backend/src/service/standaloneServer.ts index 15ffe1d053..9009af4aa6 100644 --- a/plugins/auth-backend/src/service/standaloneServer.ts +++ b/plugins/auth-backend/src/service/standaloneServer.ts @@ -56,6 +56,7 @@ export async function startStandaloneServer( async getClient() { return database; }, + migrations: { apply: true }, }, discovery, }); diff --git a/plugins/bazaar-backend/src/service/standaloneServer.ts b/plugins/bazaar-backend/src/service/standaloneServer.ts index b229f5bcf8..4ef46b7f66 100644 --- a/plugins/bazaar-backend/src/service/standaloneServer.ts +++ b/plugins/bazaar-backend/src/service/standaloneServer.ts @@ -52,7 +52,7 @@ export async function startStandaloneServer( const router = await createRouter({ logger, - database: { getClient: async () => db }, + database: { getClient: async () => db, migrations: { apply: true } }, config: config, }); diff --git a/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts b/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts index 926aa67635..3ba9340716 100644 --- a/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts +++ b/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts @@ -49,7 +49,7 @@ describe('CatalogBuilder', () => { }; const env: CatalogEnvironment = { logger: getVoidLogger(), - database: { getClient: async () => db }, + database: { getClient: async () => db, migrations: { apply: true } }, config: new ConfigReader({}), reader, }; diff --git a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts index 712ab0ae44..382cac362e 100644 --- a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts @@ -336,7 +336,7 @@ export class NextCatalogBuilder { const parser = this.parser || defaultEntityDataParser; const dbClient = await database.getClient(); - if (database.migrations?.apply) { + if (database.migrations.apply) { logger.info('Performing database migration'); await applyDatabaseMigrations(dbClient); } diff --git a/plugins/catalog-backend/src/service/standaloneServer.ts b/plugins/catalog-backend/src/service/standaloneServer.ts index 7aae3cd47c..66154b0ddc 100644 --- a/plugins/catalog-backend/src/service/standaloneServer.ts +++ b/plugins/catalog-backend/src/service/standaloneServer.ts @@ -46,7 +46,7 @@ export async function startStandaloneServer( logger.debug('Creating application...'); const builder = new CatalogBuilder({ logger, - database: { getClient: () => db }, + database: { getClient: () => db, migrations: { apply: true } }, config, reader, }); diff --git a/plugins/code-coverage-backend/src/service/standaloneServer.ts b/plugins/code-coverage-backend/src/service/standaloneServer.ts index 291f78ffc5..ca913a2a67 100644 --- a/plugins/code-coverage-backend/src/service/standaloneServer.ts +++ b/plugins/code-coverage-backend/src/service/standaloneServer.ts @@ -54,7 +54,7 @@ export async function startStandaloneServer( logger.debug('Starting application server...'); const router = await createRouter({ - database: { getClient: async () => db }, + database: { getClient: async () => db, migrations: { apply: true } }, config, discovery: SingleHostDiscovery.fromConfig(config), urlReader: UrlReaders.default({ logger, config }), diff --git a/plugins/tech-insights-backend/src/service/router.test.ts b/plugins/tech-insights-backend/src/service/router.test.ts index 0b7d3b7c45..b435136d4d 100644 --- a/plugins/tech-insights-backend/src/service/router.test.ts +++ b/plugins/tech-insights-backend/src/service/router.test.ts @@ -53,6 +53,7 @@ describe('Tech Insights router tests', () => { }, }) as unknown as Promise; }, + migrations: { apply: true }, }, logger: getVoidLogger(), factRetrievers: [], From 259922bfb27c92d0f93e89ab7316b254cece2788 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Thu, 2 Dec 2021 14:30:49 +0100 Subject: [PATCH 07/10] Update api-reports Signed-off-by: Marcus Eide --- packages/backend-common/api-report.md | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 9c66c0573e..09a95b00ea 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -175,7 +175,8 @@ export function createStatusCheckRouter(options: { // @public (undocumented) export class DatabaseManager { forPlugin(pluginId: string): PluginDatabaseManager; - static fromConfig(config: Config): DatabaseManager; + // Warning: (ae-forgotten-export) The symbol "Options" needs to be exported by the entry point index.d.ts + static fromConfig(config: Config, options?: Options): DatabaseManager; } // @public (undocumented) @@ -395,6 +396,9 @@ export type PluginCacheManager = { // @public export interface PluginDatabaseManager { getClient(): Promise; + migrations: { + apply: boolean; + }; } // @public @@ -642,4 +646,8 @@ export function useHotCleanup( // @public export function useHotMemoize(_module: NodeModule, valueFactory: () => T): T; + +// Warnings were encountered during analysis: +// +// src/database/types.d.ts:26:12 - (tsdoc-undefined-tag) The TSDoc tag "@default" is not defined in this configuration ``` From 98a9c35f0cdd668f23f652b94587e967494ac023 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Thu, 2 Dec 2021 15:38:56 +0100 Subject: [PATCH 08/10] Add changeset Signed-off-by: Marcus Eide --- .changeset/old-dingos-shave.md | 5 +++++ .changeset/seven-rabbits-shave.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/old-dingos-shave.md create mode 100644 .changeset/seven-rabbits-shave.md diff --git a/.changeset/old-dingos-shave.md b/.changeset/old-dingos-shave.md new file mode 100644 index 0000000000..3263a89c0d --- /dev/null +++ b/.changeset/old-dingos-shave.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-backend': patch +--- + +Honor database migration configuration diff --git a/.changeset/seven-rabbits-shave.md b/.changeset/seven-rabbits-shave.md new file mode 100644 index 0000000000..9e14cde2e7 --- /dev/null +++ b/.changeset/seven-rabbits-shave.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Add options argument to support additional database migrations configuration From 5c1840c16e2aff35d482d0d9801bce7f46c76bc1 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Tue, 7 Dec 2021 10:06:26 +0100 Subject: [PATCH 09/10] Rename to DatabaseManagerOptions and export Signed-off-by: Marcus Eide --- packages/backend-common/api-report.md | 11 +++++++++-- .../backend-common/src/database/DatabaseManager.ts | 14 +++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index 09a95b00ea..d6edfb0ce4 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -175,10 +175,17 @@ export function createStatusCheckRouter(options: { // @public (undocumented) export class DatabaseManager { forPlugin(pluginId: string): PluginDatabaseManager; - // Warning: (ae-forgotten-export) The symbol "Options" needs to be exported by the entry point index.d.ts - static fromConfig(config: Config, options?: Options): DatabaseManager; + static fromConfig( + config: Config, + options?: DatabaseManagerOptions, + ): DatabaseManager; } +// @public +export type DatabaseManagerOptions = { + migrations: PluginDatabaseManager['migrations']; +}; + // @public (undocumented) export class DockerContainerRunner implements ContainerRunner { constructor({ dockerClient }: { dockerClient: Docker }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index e564f2e0e6..befdd51218 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -36,7 +36,12 @@ function pluginPath(pluginId: string): string { return `plugin.${pluginId}`; } -type Options = { +/** + * Configuration options object. + * + * @public + */ +export type DatabaseManagerOptions = { migrations: PluginDatabaseManager['migrations']; }; @@ -53,7 +58,10 @@ export class DatabaseManager { * @param config - The loaded application configuration. * @param options - An optional configuration object. */ - static fromConfig(config: Config, options?: Options): DatabaseManager { + static fromConfig( + config: Config, + options?: DatabaseManagerOptions, + ): DatabaseManager { const databaseConfig = config.getConfig('backend.database'); return new DatabaseManager( @@ -66,7 +74,7 @@ export class DatabaseManager { private constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', - private readonly options?: Options, + private readonly options?: DatabaseManagerOptions, ) {} /** From dac55f3cc76482c58ac61b994483a9397f0fef26 Mon Sep 17 00:00:00 2001 From: Marcus Eide Date: Tue, 7 Dec 2021 12:44:16 +0100 Subject: [PATCH 10/10] Make optional Signed-off-by: Marcus Eide --- packages/backend-common/api-report.md | 8 ++++---- .../src/database/DatabaseManager.test.ts | 6 +++--- .../backend-common/src/database/DatabaseManager.ts | 4 ++-- packages/backend-common/src/database/types.ts | 11 ++++------- .../backend-tasks/src/tasks/TaskScheduler.test.ts | 1 - plugins/auth-backend/src/service/standaloneServer.ts | 1 - .../bazaar-backend/src/service/standaloneServer.ts | 2 +- .../src/legacy/service/CatalogBuilder.test.ts | 2 +- .../catalog-backend/src/service/NextCatalogBuilder.ts | 2 +- .../catalog-backend/src/service/standaloneServer.ts | 2 +- .../src/service/standaloneServer.ts | 2 +- .../tech-insights-backend/src/service/router.test.ts | 1 - 12 files changed, 18 insertions(+), 24 deletions(-) diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index d6edfb0ce4..e623f656ab 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -183,7 +183,7 @@ export class DatabaseManager { // @public export type DatabaseManagerOptions = { - migrations: PluginDatabaseManager['migrations']; + migrations?: PluginDatabaseManager['migrations']; }; // @public (undocumented) @@ -403,8 +403,8 @@ export type PluginCacheManager = { // @public export interface PluginDatabaseManager { getClient(): Promise; - migrations: { - apply: boolean; + migrations?: { + skip?: boolean; }; } @@ -656,5 +656,5 @@ export function useHotMemoize(_module: NodeModule, valueFactory: () => T): T; // Warnings were encountered during analysis: // -// src/database/types.d.ts:26:12 - (tsdoc-undefined-tag) The TSDoc tag "@default" is not defined in this configuration +// src/database/types.d.ts:23:12 - (tsdoc-undefined-tag) The TSDoc tag "@default" is not defined in this configuration ``` diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index 4a44feec4a..4931a52f35 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -63,17 +63,17 @@ describe('DatabaseManager', () => { const database = DatabaseManager.fromConfig(config); const client = database.forPlugin('test'); - expect(client.migrations.apply).toBe(true); + expect(client.migrations?.skip).toBe(false); }); it('handles migrations options', () => { const config = new ConfigReader(backendConfig); const database = DatabaseManager.fromConfig(config, { - migrations: { apply: false }, + migrations: { skip: true }, }); const client = database.forPlugin('test'); - expect(client.migrations.apply).toBe(false); + expect(client.migrations?.skip).toBe(true); }); }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index befdd51218..ee3e8b1a89 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -42,7 +42,7 @@ function pluginPath(pluginId: string): string { * @public */ export type DatabaseManagerOptions = { - migrations: PluginDatabaseManager['migrations']; + migrations?: PluginDatabaseManager['migrations']; }; /** @public */ @@ -92,7 +92,7 @@ export class DatabaseManager { return _this.getDatabase(pluginId); }, migrations: { - apply: true, + skip: false, ..._this.options?.migrations, }, }; diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 4cfc86e240..344f1088b8 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -34,16 +34,13 @@ export interface PluginDatabaseManager { /** * This property is used to control the behavior of database migrations. */ - migrations: { + migrations?: { /** - * apply can be used to determine if database migrations - * should be performed. + * skip database migrations. Useful if connecting to a read-only database. * - * Useful if connecting to a read-only database. - * - * @default true + * @default false */ - apply: boolean; + skip?: boolean; }; } diff --git a/packages/backend-tasks/src/tasks/TaskScheduler.test.ts b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts index 6c9a6989c7..ce8e797503 100644 --- a/packages/backend-tasks/src/tasks/TaskScheduler.test.ts +++ b/packages/backend-tasks/src/tasks/TaskScheduler.test.ts @@ -33,7 +33,6 @@ describe('TaskScheduler', () => { const databaseManager: Partial = { forPlugin: () => ({ getClient: async () => knex, - migrations: { apply: true }, }), }; return databaseManager as DatabaseManager; diff --git a/plugins/auth-backend/src/service/standaloneServer.ts b/plugins/auth-backend/src/service/standaloneServer.ts index 9009af4aa6..15ffe1d053 100644 --- a/plugins/auth-backend/src/service/standaloneServer.ts +++ b/plugins/auth-backend/src/service/standaloneServer.ts @@ -56,7 +56,6 @@ export async function startStandaloneServer( async getClient() { return database; }, - migrations: { apply: true }, }, discovery, }); diff --git a/plugins/bazaar-backend/src/service/standaloneServer.ts b/plugins/bazaar-backend/src/service/standaloneServer.ts index 4ef46b7f66..b229f5bcf8 100644 --- a/plugins/bazaar-backend/src/service/standaloneServer.ts +++ b/plugins/bazaar-backend/src/service/standaloneServer.ts @@ -52,7 +52,7 @@ export async function startStandaloneServer( const router = await createRouter({ logger, - database: { getClient: async () => db, migrations: { apply: true } }, + database: { getClient: async () => db }, config: config, }); diff --git a/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts b/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts index 3ba9340716..926aa67635 100644 --- a/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts +++ b/plugins/catalog-backend/src/legacy/service/CatalogBuilder.test.ts @@ -49,7 +49,7 @@ describe('CatalogBuilder', () => { }; const env: CatalogEnvironment = { logger: getVoidLogger(), - database: { getClient: async () => db, migrations: { apply: true } }, + database: { getClient: async () => db }, config: new ConfigReader({}), reader, }; diff --git a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts index 382cac362e..1618ef8d7b 100644 --- a/plugins/catalog-backend/src/service/NextCatalogBuilder.ts +++ b/plugins/catalog-backend/src/service/NextCatalogBuilder.ts @@ -336,7 +336,7 @@ export class NextCatalogBuilder { const parser = this.parser || defaultEntityDataParser; const dbClient = await database.getClient(); - if (database.migrations.apply) { + if (!database.migrations?.skip) { logger.info('Performing database migration'); await applyDatabaseMigrations(dbClient); } diff --git a/plugins/catalog-backend/src/service/standaloneServer.ts b/plugins/catalog-backend/src/service/standaloneServer.ts index 66154b0ddc..7aae3cd47c 100644 --- a/plugins/catalog-backend/src/service/standaloneServer.ts +++ b/plugins/catalog-backend/src/service/standaloneServer.ts @@ -46,7 +46,7 @@ export async function startStandaloneServer( logger.debug('Creating application...'); const builder = new CatalogBuilder({ logger, - database: { getClient: () => db, migrations: { apply: true } }, + database: { getClient: () => db }, config, reader, }); diff --git a/plugins/code-coverage-backend/src/service/standaloneServer.ts b/plugins/code-coverage-backend/src/service/standaloneServer.ts index ca913a2a67..291f78ffc5 100644 --- a/plugins/code-coverage-backend/src/service/standaloneServer.ts +++ b/plugins/code-coverage-backend/src/service/standaloneServer.ts @@ -54,7 +54,7 @@ export async function startStandaloneServer( logger.debug('Starting application server...'); const router = await createRouter({ - database: { getClient: async () => db, migrations: { apply: true } }, + database: { getClient: async () => db }, config, discovery: SingleHostDiscovery.fromConfig(config), urlReader: UrlReaders.default({ logger, config }), diff --git a/plugins/tech-insights-backend/src/service/router.test.ts b/plugins/tech-insights-backend/src/service/router.test.ts index b435136d4d..0b7d3b7c45 100644 --- a/plugins/tech-insights-backend/src/service/router.test.ts +++ b/plugins/tech-insights-backend/src/service/router.test.ts @@ -53,7 +53,6 @@ describe('Tech Insights router tests', () => { }, }) as unknown as Promise; }, - migrations: { apply: true }, }, logger: getVoidLogger(), factRetrievers: [],