From 7715cadf2daa5c7f8065c96795c7baf3a8c19999 Mon Sep 17 00:00:00 2001 From: Greg Bomkamp Date: Thu, 11 Nov 2021 10:37:27 -0500 Subject: [PATCH] Update config option from usePluginSchemas to pluginDivisionMode, Clean up Var names for clarify, fix tests Signed-off-by: Greg Bomkamp --- .changeset/pretty-moons-drive.md | 2 +- packages/backend-common/config.d.ts | 13 ++++--- .../src/database/DatabaseManager.test.ts | 18 +++++----- .../src/database/DatabaseManager.ts | 35 +++++++++---------- 4 files changed, 34 insertions(+), 34 deletions(-) diff --git a/.changeset/pretty-moons-drive.md b/.changeset/pretty-moons-drive.md index 3c3b7968c6..51dc69332e 100644 --- a/.changeset/pretty-moons-drive.md +++ b/.changeset/pretty-moons-drive.md @@ -2,4 +2,4 @@ '@backstage/backend-common': patch --- -Adding config prop `usePluginSchemas` to allow plugins using the `pg` client to create their own management schemas in the db. This allows `pg` client plugins to work in separate schemas in the same db. +Adding config prop `pluginDivisionMode` to allow plugins using the `pg` client to create their own management schemas in the db. This allows `pg` client plugins to work in separate schemas in the same db. diff --git a/packages/backend-common/config.d.ts b/packages/backend-common/config.d.ts index 4476a946cf..aafda9b7d4 100644 --- a/packages/backend-common/config.d.ts +++ b/packages/backend-common/config.d.ts @@ -70,15 +70,18 @@ export interface Config { */ ensureExists?: boolean; /** - * Whether plugins should use their own schemas instead of databases. If enabled, - * each plugin will create a schema in the configured database instance - * using the `pluginId` as its schema name. + * How plugins databases are managed/divided in the provided database instance. + * + * `database` -> Plugins are each given their own database to manage their schemas/tables. + * + * `schema` -> Plugins will be given their own schema (in the specified/default database) + * to manage their tables. * * NOTE: Currently only supported by the `pg` client. * - * @default false + * @default database */ - usePluginSchemas?: boolean; + pluginDivisionMode?: 'database' | 'schema'; /** Plugin specific database configuration and client override */ plugin?: { [pluginId: string]: { diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index e639f14ee8..f2fe855234 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -325,7 +325,7 @@ describe('DatabaseManager', () => { backend: { database: { client: 'pg', - usePluginSchemas: true, + pluginDivisionMode: 'schema', connection: { host: 'localhost', user: 'foo', @@ -360,7 +360,7 @@ describe('DatabaseManager', () => { backend: { database: { client: 'sqlite3', - usePluginSchemas: true, + pluginDivisionMode: 'schema', connection: { host: 'localhost', user: 'foo', @@ -385,13 +385,13 @@ describe('DatabaseManager', () => { expect(overrides).not.toHaveProperty('searchPath'); }); - it('plugin does not provide schema override if usePluginSchemas is not provided', async () => { + it('plugin does not provide schema override if pluginDivisionMode is set to database', async () => { const testManager = DatabaseManager.fromConfig( new ConfigReader({ backend: { database: { client: 'pg', - usePluginSchemas: false, + pluginDivisionMode: 'database', connection: 'some-file-path', }, }, @@ -407,7 +407,7 @@ describe('DatabaseManager', () => { expect(overrides).not.toHaveProperty('searchPath'); }); - it('plugin does not provide schema override if usePluginSchemas is false', async () => { + it('plugin does not provide schema override if pluginDivisionMode is not set', async () => { const testManager = DatabaseManager.fromConfig( new ConfigReader({ backend: { @@ -433,13 +433,13 @@ describe('DatabaseManager', () => { expect(overrides).not.toHaveProperty('searchPath'); }); - it('usePluginSchemas ensures that each plugin schema exists', async () => { + it('pluginDivisionMode ensures that each plugin schema exists', async () => { const testManager = DatabaseManager.fromConfig( new ConfigReader({ backend: { database: { client: 'pg', - usePluginSchemas: true, + pluginDivisionMode: 'schema', connection: { host: 'localhost', user: 'foo', @@ -459,13 +459,13 @@ describe('DatabaseManager', () => { expect(schemaName).toEqual('testdbname'); }); - it('usePluginSchemas allows connection overrides for plugins', async () => { + it('pluginDivisionMode allows connection overrides for plugins', async () => { const testManager = DatabaseManager.fromConfig( new ConfigReader({ backend: { database: { client: 'pg', - usePluginSchemas: true, + pluginDivisionMode: 'schema', connection: { host: 'localhost', user: 'foo', diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index efd779e4ed..6a2b6cff60 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -83,9 +83,9 @@ export class DatabaseManager { * * This method provides the effective database name which is determined using global * and plugin specific database config. If no explicit database name is configured - * and usePluginSchemas is not enabled, this method will provide a generated name - * which is the pluginId prefixed with 'backstage_plugin_'. If `usePluginSchemas` is - * enabled, it will fallback to using the default database for the knex instance. + * and `pluginDivisionMode` is not `schema`, this method will provide a generated name + * which is the pluginId prefixed with 'backstage_plugin_'. If `pluginDivisionMode` is + * `schema`, it will fallback to using the default database for the knex instance. * * @param pluginId Lookup the database name for given plugin * @returns String representing the plugin's database name @@ -102,8 +102,8 @@ export class DatabaseManager { const databaseName = (connection as Knex.ConnectionConfig)?.database; - // usePluginSchemas enabled should use overridden databaseName if supplied or fallback to default knex database - if (this.getUsePluginSchemasConfig()) { + // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database + if (this.getPluginDivisionModeConfig() === 'schema') { return databaseName; } @@ -145,8 +145,8 @@ export class DatabaseManager { ); } - private getUsePluginSchemasConfig(): boolean { - return this.config.getOptionalBoolean('usePluginSchemas') ?? false; + private getPluginDivisionModeConfig(): string { + return this.config.getOptionalString('pluginDivisionMode') ?? 'database'; } /** @@ -156,7 +156,7 @@ export class DatabaseManager { * has not been overridden, the global connection config will be included with plugin * specific config as the base. Values from the plugin connection take precedence over the * base. Base database name is omitted for all supported databases excluding SQLite unless - * `usePluginSchemas` is enabled. + * `pluginDivisionMode` is set to `schema`. */ private getConnectionConfig( pluginId: string, @@ -167,11 +167,11 @@ export class DatabaseManager { this.config.get('connection'), this.config.getString('client'), ); - // Databases cannot be shared unless the usePluginSchemas is enabled. The - // `database` property from the base connection is omitted unless usePluginSchemas - // is enabled. SQLite3's `filename` property is an exception as this is used as a + // Databases cannot be shared unless the `pluginDivisionMode` is set to `schema`. The + // `database` property from the base connection is omitted unless `pluginDivisionMode` + // is set to `schema`. SQLite3's `filename` property is an exception as this is used as a // directory elsewhere so we preserve `filename`. - if (!this.getUsePluginSchemasConfig()) { + if (this.getPluginDivisionModeConfig() !== 'schema') { baseConnection = omit(baseConnection, 'database'); } @@ -221,12 +221,9 @@ export class DatabaseManager { * @returns Partial Knex.Config with database name override */ private getDatabaseOverrides(pluginId: string): Knex.Config { - const databaseNameOverride = this.getDatabaseName(pluginId); - return databaseNameOverride - ? createNameOverride( - this.getClientType(pluginId).client, - databaseNameOverride, - ) + const databaseName = this.getDatabaseName(pluginId); + return databaseName + ? createNameOverride(this.getClientType(pluginId).client, databaseName) : {}; } @@ -253,7 +250,7 @@ export class DatabaseManager { } let schemaOverrides; - if (this.getUsePluginSchemasConfig()) { + if (this.getPluginDivisionModeConfig() === 'schema') { try { schemaOverrides = this.getSchemaOverrides(pluginId); await ensureSchemaExists(pluginConfig, pluginId);