From 1da71d800c3e0c7e86c53755844d71b116eacf3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 28 Apr 2024 11:33:59 +0200 Subject: [PATCH 1/8] Make individual connector classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/database/DatabaseManager.ts | 36 +- .../src/database/connection.test.ts | 308 --------------- .../backend-common/src/database/connection.ts | 355 +++++++++--------- .../src/database/connectors/mysql.ts | 336 ++++++++++++++++- .../src/database/connectors/postgres.ts | 341 ++++++++++++++++- .../src/database/connectors/sqlite3.ts | 339 ++++++++++++++++- packages/backend-common/src/database/types.ts | 28 +- 7 files changed, 1224 insertions(+), 519 deletions(-) delete mode 100644 packages/backend-common/src/database/connection.test.ts diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 8a4a4b50fa..776f05aed4 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -19,14 +19,6 @@ import { JsonObject } from '@backstage/types'; import { Knex } from 'knex'; import { merge, omit } from 'lodash'; import { mergeDatabaseConfig } from './config'; -import { - createDatabaseClient, - createNameOverride, - createSchemaOverride, - ensureDatabaseExists, - ensureSchemaExists, - normalizeConnection, -} from './connection'; import { PluginDatabaseManager } from './types'; import path from 'path'; import { @@ -36,6 +28,9 @@ import { PluginMetadataService, } from '@backstage/backend-plugin-api'; import { stringifyError } from '@backstage/errors'; +import { PgConnector } from './connectors/postgres'; +import { Sqlite3Connector } from './connectors/sqlite3'; +import { MysqlConnector } from './connectors/mysql'; /** * Provides a config lookup path for a plugin's config block. @@ -86,10 +81,16 @@ export class DatabaseManager implements LegacyRootDatabaseService { options?: DatabaseManagerOptions, ): DatabaseManager { const databaseConfig = config.getConfig('backend.database'); - return new DatabaseManager( databaseConfig, databaseConfig.getOptionalString('prefix'), + { + pg: new PgConnector(config, prefix, options), + sqlite3: new Sqlite3Connector(config, prefix, options), + 'better-sqlite3': new Sqlite3Connector(config, prefix, options), + mysql: new MysqlConnector(config, prefix, options), + mysql2: new MysqlConnector(config, prefix, options), + }, options, ); } @@ -97,6 +98,18 @@ export class DatabaseManager implements LegacyRootDatabaseService { private constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', + private readonly connectors: Record< + string, + { + getClient( + pluginId: string, + deps?: { + lifecycle: LifecycleService; + pluginMetadata: PluginMetadataService; + }, + ): Promise; + } + >, private readonly options?: DatabaseManagerOptions, private readonly databaseCache: Map> = new Map(), ) {} @@ -115,6 +128,11 @@ export class DatabaseManager implements LegacyRootDatabaseService { pluginMetadata: PluginMetadataService; }, ): PluginDatabaseManager { + const client = this.getClientType(pluginId).client; + const connector = this.connectors[client]; + if (!connector) { + throw new Error(`Unsupported database client type '${client}'`); + } const getClient = () => this.getDatabase(pluginId, deps); const migrations = { skip: false, ...this.options?.migrations }; return { getClient, migrations }; diff --git a/packages/backend-common/src/database/connection.test.ts b/packages/backend-common/src/database/connection.test.ts deleted file mode 100644 index 3028d46cae..0000000000 --- a/packages/backend-common/src/database/connection.test.ts +++ /dev/null @@ -1,308 +0,0 @@ -/* - * Copyright 2020 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { ConfigReader } from '@backstage/config'; -import { - createDatabaseClient, - createNameOverride, - createSchemaOverride, - dropDatabase, - ensureSchemaExists, - parseConnectionString, -} from './connection'; -import { mysqlConnector, pgConnector } from './connectors'; - -const mocked = (f: Function) => f as jest.Mock; - -jest.mock('./connectors', () => { - const connectors = jest.requireActual('./connectors'); - return { - ...connectors, - mysqlConnector: { - ...connectors.mysqlConnector, - dropDatabase: jest.fn(), - }, - pgConnector: { - ...connectors.pgConnector, - dropDatabase: jest.fn(), - ensureSchemaExists: jest.fn(), - }, - }; -}); - -describe('database connection', () => { - describe('createDatabaseClient', () => { - it('returns a postgres connection', () => { - expect( - createDatabaseClient( - new ConfigReader({ - client: 'pg', - connection: { - host: 'acme', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }), - ), - ).toBeTruthy(); - }); - - it('returns an sqlite connection', () => { - expect( - createDatabaseClient( - new ConfigReader({ - client: 'better-sqlite3', - connection: ':memory:', - }), - ), - ).toBeTruthy(); - }); - - it('returns a mysql connection', () => { - expect(() => - createDatabaseClient( - new ConfigReader({ - client: 'mysql2', - connection: { - host: '127.0.0.1', - user: 'foo', - password: 'bar', - database: 'dbname', - }, - }), - ), - ).toBeTruthy(); - }); - - it('accepts overrides', () => { - expect( - createDatabaseClient( - new ConfigReader({ - client: 'pg', - connection: { - host: 'acme', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }), - { - connection: { - database: 'foo', - }, - }, - ), - ).toBeTruthy(); - }); - - it('throws an error without a client', () => { - expect(() => - createDatabaseClient( - new ConfigReader({ - connection: '', - }), - ), - ).toThrow(); - }); - - it('throws an error without a connection', () => { - expect(() => - createDatabaseClient( - new ConfigReader({ - client: 'pg', - }), - ), - ).toThrow(); - }); - }); - - describe('createNameOverride', () => { - it('returns Knex config for postgres', () => { - expect(createNameOverride('pg', 'testpg')).toHaveProperty( - 'connection.database', - 'testpg', - ); - }); - - it('returns Knex config for sqlite', () => { - expect(createNameOverride('better-sqlite3', 'testsqlite')).toHaveProperty( - 'connection.filename', - 'testsqlite', - ); - }); - - it('returns Knex config for mysql', () => { - expect(createNameOverride('mysql', 'testmysql')).toHaveProperty( - 'connection.database', - 'testmysql', - ); - }); - - it('throws an error for unknown connection', () => { - expect(() => createNameOverride('unknown', 'testname')).toThrow(); - }); - }); - - describe('parseConnectionString', () => { - it('returns parsed Knex.StaticConnectionConfig for postgres', () => { - expect( - parseConnectionString('postgresql://foo:bar@acme:5432/foodb', 'pg'), - ).toHaveProperty('database', 'foodb'); - }); - - it('returns parsed Knex.StaticConnectionConfig for mysql2', () => { - expect( - parseConnectionString('mysql://foo:bar@acme:3306/foodb', 'mysql2'), - ).toHaveProperty('database', 'foodb'); - }); - - it('throws an error if client hint is not provided', () => { - expect(() => parseConnectionString('sqlite://')).toThrow(); - }); - }); - - describe('createSchemaOverride', () => { - it('returns Knex config for postgres', () => { - expect(createSchemaOverride('pg', 'testpg')).toHaveProperty( - 'searchPath', - ['testpg'], - ); - }); - - it('throws error for sqlite', () => { - expect( - createSchemaOverride('better-sqlite3', 'testsqlite'), - ).toBeUndefined(); - }); - - it('returns Knex config for mysql', () => { - expect(createSchemaOverride('mysql', 'testmysql')).toBeUndefined(); - }); - - it('throws an error for unknown connection', () => { - expect(createSchemaOverride('unknown', 'testname')).toBeUndefined(); - }); - }); - - describe('ensureSchemaExists', () => { - it('returns successfully with pg client', async () => { - await ensureSchemaExists( - new ConfigReader({ - client: 'pg', - schema: 'catalog', - connection: 'postgresql://testuser:testpass@acme:5432/userdbname', - }), - 'catalog', - ); - - const mockCalls = mocked( - pgConnector.ensureSchemaExists as Function, - ).mock.calls.splice(-1); - const [baseConfig, schemaName] = mockCalls[0]; - - expect(baseConfig.get()).toMatchObject({ - client: 'pg', - connection: 'postgresql://testuser:testpass@acme:5432/userdbname', - }); - - expect(schemaName).toEqual('catalog'); - }); - - it('throws error for non pg client', () => { - return expect( - ensureSchemaExists( - new ConfigReader({ - client: 'better-sqlite3', - schema: 'catalog', - connection: ':memory:', - }), - 'catalog', - ), - ).resolves.toBeUndefined(); - }); - }); - - describe('dropDatabase', () => { - it('returns successfully with pg client', async () => { - await dropDatabase( - new ConfigReader({ - client: 'pg', - schema: 'catalog', - connection: 'postgresql://testuser:testpass@acme:5432/userdbname', - }), - 'backstage_plugin_foobar', - ); - - const mockCalls = mocked( - pgConnector.dropDatabase as Function, - ).mock.calls.splice(-1); - const [baseConfig, databaseName] = mockCalls[0]; - - expect(baseConfig.get()).toMatchObject({ - client: 'pg', - connection: 'postgresql://testuser:testpass@acme:5432/userdbname', - }); - - expect(databaseName).toEqual('backstage_plugin_foobar'); - }); - - it('returns successfully with mysql client', async () => { - await dropDatabase( - new ConfigReader({ - client: 'mysql2', - connection: { - host: '127.0.0.1', - user: 'foo', - password: 'bar', - database: 'dbname', - }, - }), - 'backstage_plugin_foobar', - ); - - const mockCalls = mocked( - mysqlConnector.dropDatabase as Function, - ).mock.calls.splice(-1); - const [baseConfig, databaseName] = mockCalls[0]; - - expect(baseConfig.get()).toMatchObject({ - client: 'mysql2', - connection: { - host: '127.0.0.1', - user: 'foo', - password: 'bar', - database: 'dbname', - }, - }); - - expect(databaseName).toEqual('backstage_plugin_foobar'); - }); - - it('does nothing in other database drivers', () => { - return expect( - dropDatabase( - new ConfigReader({ - client: 'better-sqlite3', - schema: 'catalog', - connection: ':memory:', - }), - 'catalog', - ), - ).resolves.toBeUndefined(); - }); - }); -}); diff --git a/packages/backend-common/src/database/connection.ts b/packages/backend-common/src/database/connection.ts index c0e031516f..875734aa19 100644 --- a/packages/backend-common/src/database/connection.ts +++ b/packages/backend-common/src/database/connection.ts @@ -1,5 +1,5 @@ /* - * Copyright 2020 The Backstage Authors + * Copyright 2024 The Backstage Authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,190 +14,205 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; -import { JsonObject } from '@backstage/types'; -import { InputError } from '@backstage/errors'; -import knexFactory, { Knex } from 'knex'; -import limiterFactory from 'p-limit'; -import { mergeDatabaseConfig } from './config'; -import { DatabaseConnector } from './types'; +// * Copyright 2020 The Backstage Authors +// * +// * Licensed under the Apache License, Version 2.0 (the "License"); +// * you may not use this file except in compliance with the License. +// * You may obtain a copy of the License at +// * +// * http://www.apache.org/licenses/LICENSE-2.0 +// * +// * Unless required by applicable law or agreed to in writing, software +// * distributed under the License is distributed on an "AS IS" BASIS, +// * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// * See the License for the specific language governing permissions and +// * limitations under the License. +// */ -import { mysqlConnector, pgConnector, sqlite3Connector } from './connectors'; -import { - LifecycleService, - PluginMetadataService, -} from '@backstage/backend-plugin-api'; +// import { Config } from '@backstage/config'; +// import { JsonObject } from '@backstage/types'; +// import { InputError } from '@backstage/errors'; +// import knexFactory, { Knex } from 'knex'; +// import limiterFactory from 'p-limit'; +// import { mergeDatabaseConfig } from './config'; +// import { DatabaseConnector } from './types'; -type DatabaseClient = - | 'pg' - | 'better-sqlite3' - | 'sqlite3' - | 'mysql' - | 'mysql2' - | string; +// import { mysqlConnector, pgConnector, sqlite3Connector } from './connectors'; +// import { +// LifecycleService, +// PluginMetadataService, +// } from '@backstage/backend-plugin-api'; -// This limits the number of concurrent CREATE DATABASE and CREATE SCHEMA -// commands, globally, to just one. This is overly defensive, and was added as -// an attempt to counteract the pool issues on recent node versions. See -// https://github.com/backstage/backstage/pull/19988 -const ddlLimiter = limiterFactory(1); +// type DatabaseClient = +// | 'pg' +// | 'better-sqlite3' +// | 'sqlite3' +// | 'mysql' +// | 'mysql2' +// | string; -/** - * Mapping of client type to supported database connectors - * - * Database connectors can be aliased here, for example mysql2 uses - * the same connector as mysql. - */ -const ConnectorMapping: Record = { - pg: pgConnector, - 'better-sqlite3': sqlite3Connector, - sqlite3: sqlite3Connector, - mysql: mysqlConnector, - mysql2: mysqlConnector, -}; +// // This limits the number of concurrent CREATE DATABASE and CREATE SCHEMA +// // commands, globally, to just one. This is overly defensive, and was added as +// // an attempt to counteract the pool issues on recent node versions. See +// // https://github.com/backstage/backstage/pull/19988 +// const ddlLimiter = limiterFactory(1); -/** - * Creates a knex database connection - * - * @public - * @param dbConfig - The database config - * @param overrides - Additional options to merge with the config - */ -export function createDatabaseClient( - dbConfig: Config, - overrides?: Partial, - deps?: { - lifecycle: LifecycleService; - pluginMetadata: PluginMetadataService; - }, -) { - const client: DatabaseClient = dbConfig.getString('client'); +// /** +// * Mapping of client type to supported database connectors +// * +// * Database connectors can be aliased here, for example mysql2 uses +// * the same connector as mysql. +// */ +// const ConnectorMapping: Record = { +// pg: pgConnector, +// 'better-sqlite3': sqlite3Connector, +// sqlite3: sqlite3Connector, +// mysql: mysqlConnector, +// mysql2: mysqlConnector, +// }; - return ( - ConnectorMapping[client]?.createClient(dbConfig, overrides, deps) ?? - knexFactory(mergeDatabaseConfig(dbConfig.get(), overrides)) - ); -} +// /** +// * Creates a knex database connection +// * +// * @public +// * @param dbConfig - The database config +// * @param overrides - Additional options to merge with the config +// */ +// export function createDatabaseClient( +// dbConfig: Config, +// overrides?: Partial, +// deps?: { +// lifecycle: LifecycleService; +// pluginMetadata: PluginMetadataService; +// }, +// ) { +// const client: DatabaseClient = dbConfig.getString('client'); -/** - * Ensures that the given databases all exist, creating them if they do not. - * - * @public - */ -export async function ensureDatabaseExists( - dbConfig: Config, - ...databases: Array -): Promise { - const client: DatabaseClient = dbConfig.getString('client'); +// return ( +// ConnectorMapping[client]?.createClient(dbConfig, overrides, deps) ?? +// knexFactory(mergeDatabaseConfig(dbConfig.get(), overrides)) +// ); +// } - return await ddlLimiter(() => - ConnectorMapping[client]?.ensureDatabaseExists?.(dbConfig, ...databases), - ); -} +// /** +// * Ensures that the given databases all exist, creating them if they do not. +// * +// * @public +// */ +// export async function ensureDatabaseExists( +// dbConfig: Config, +// ...databases: Array +// ): Promise { +// const client: DatabaseClient = dbConfig.getString('client'); -/** - * Drops the given databases. - * - * @public - */ -export async function dropDatabase( - dbConfig: Config, - ...databases: Array -): Promise { - const client: DatabaseClient = dbConfig.getString('client'); +// return await ddlLimiter(() => +// ConnectorMapping[client]?.ensureDatabaseExists?.(dbConfig, ...databases), +// ); +// } - return await ddlLimiter(() => - ConnectorMapping[client]?.dropDatabase?.(dbConfig, ...databases), - ); -} +// /** +// * Drops the given databases. +// * +// * @public +// */ +// export async function dropDatabase( +// dbConfig: Config, +// ...databases: Array +// ): Promise { +// const client: DatabaseClient = dbConfig.getString('client'); -/** - * Ensures that the given schemas all exist, creating them if they do not. - * - * @public - */ -export async function ensureSchemaExists( - dbConfig: Config, - ...schemas: Array -): Promise { - const client: DatabaseClient = dbConfig.getString('client'); +// return await ddlLimiter(() => +// ConnectorMapping[client]?.dropDatabase?.(dbConfig, ...databases), +// ); +// } - return await ddlLimiter(() => - ConnectorMapping[client]?.ensureSchemaExists?.(dbConfig, ...schemas), - ); -} +// /** +// * Ensures that the given schemas all exist, creating them if they do not. +// * +// * @public +// */ +// export async function ensureSchemaExists( +// dbConfig: Config, +// ...schemas: Array +// ): Promise { +// const client: DatabaseClient = dbConfig.getString('client'); -/** - * Provides a `Knex.Config` object with the provided database name for a given - * client. - */ -export function createNameOverride( - client: string, - name: string, -): Partial { - try { - return ConnectorMapping[client].createNameOverride(name); - } catch (e) { - throw new InputError( - `Unable to create database name override for '${client}' connector`, - e, - ); - } -} +// return await ddlLimiter(() => +// ConnectorMapping[client]?.ensureSchemaExists?.(dbConfig, ...schemas), +// ); +// } -/** - * Provides a `Knex.Config` object with the provided database schema for a given - * client. Currently only supported by `pg`. - */ -export function createSchemaOverride( - client: string, - name: string, -): Partial { - try { - return ConnectorMapping[client]?.createSchemaOverride?.(name); - } catch (e) { - throw new InputError( - `Unable to create database schema override for '${client}' connector`, - e, - ); - } -} +// /** +// * Provides a `Knex.Config` object with the provided database name for a given +// * client. +// */ +// export function createNameOverride( +// client: string, +// name: string, +// ): Partial { +// try { +// return ConnectorMapping[client].createNameOverride(name); +// } catch (e) { +// throw new InputError( +// `Unable to create database name override for '${client}' connector`, +// e, +// ); +// } +// } -/** - * Parses a connection string for a given client and provides a connection config. - */ -export function parseConnectionString( - connectionString: string, - client?: string, -): Knex.StaticConnectionConfig { - if (typeof client === 'undefined' || client === null) { - throw new InputError( - 'Database connection string client type auto-detection is not yet supported.', - ); - } +// /** +// * Provides a `Knex.Config` object with the provided database schema for a given +// * client. Currently only supported by `pg`. +// */ +// export function createSchemaOverride( +// client: string, +// name: string, +// ): Partial { +// try { +// return ConnectorMapping[client]?.createSchemaOverride?.(name); +// } catch (e) { +// throw new InputError( +// `Unable to create database schema override for '${client}' connector`, +// e, +// ); +// } +// } - try { - return ConnectorMapping[client].parseConnectionString(connectionString); - } catch (e) { - throw new InputError( - `Unable to parse connection string for '${client}' connector`, - ); - } -} +// /** +// * Parses a connection string for a given client and provides a connection config. +// */ +// export function parseConnectionString( +// connectionString: string, +// client?: string, +// ): Knex.StaticConnectionConfig { +// if (typeof client === 'undefined' || client === null) { +// throw new InputError( +// 'Database connection string client type auto-detection is not yet supported.', +// ); +// } -/** - * Normalizes a connection config or string into an object which can be passed - * to Knex. - */ -export function normalizeConnection( - connection: Knex.StaticConnectionConfig | JsonObject | string | undefined, - client: string, -): Partial { - if (typeof connection === 'undefined' || connection === null) { - return {}; - } +// try { +// return ConnectorMapping[client].parseConnectionString(connectionString); +// } catch (e) { +// throw new InputError( +// `Unable to parse connection string for '${client}' connector`, +// ); +// } +// } - return typeof connection === 'string' || connection instanceof String - ? parseConnectionString(connection as string, client) - : connection; -} +// /** +// * Normalizes a connection config or string into an object which can be passed +// * to Knex. +// */ +// export function normalizeConnection( +// connection: Knex.StaticConnectionConfig | JsonObject | string | undefined, +// client: string, +// ): Partial { +// if (typeof connection === 'undefined' || connection === null) { +// return {}; +// } + +// return typeof connection === 'string' || connection instanceof String +// ? parseConnectionString(connection as string, client) +// : connection; +// } diff --git a/packages/backend-common/src/database/connectors/mysql.ts b/packages/backend-common/src/database/connectors/mysql.ts index a4bac3f7e1..63464fa6f4 100644 --- a/packages/backend-common/src/database/connectors/mysql.ts +++ b/packages/backend-common/src/database/connectors/mysql.ts @@ -14,11 +14,17 @@ * limitations under the License. */ -import knexFactory, { Knex } from 'knex'; -import yn from 'yn'; - -import { Config } from '@backstage/config'; +import { + LifecycleService, + PluginMetadataService, +} from '@backstage/backend-plugin-api'; +import { Config, ConfigReader } from '@backstage/config'; import { InputError } from '@backstage/errors'; +import { JsonObject } from '@backstage/types'; +import knexFactory, { Knex } from 'knex'; +import { merge, omit } from 'lodash'; +import path from 'path'; +import yn from 'yn'; import { mergeDatabaseConfig } from '../config'; import { DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; @@ -229,3 +235,325 @@ export const mysqlConnector: DatabaseConnector = Object.freeze({ parseConnectionString: parseMysqlConnectionString, dropDatabase: dropMysqlDatabase, }); + +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +/** + * Provides a config lookup path for a plugin's config block. + */ +function pluginPath(pluginId: string): string { + return `plugin.${pluginId}`; +} + +function normalizeConnection( + connection: Knex.StaticConnectionConfig | JsonObject | string | undefined, + client: string, +): Partial { + if (typeof connection === 'undefined' || connection === null) { + return {}; + } + + return typeof connection === 'string' || connection instanceof String + ? mysqlConnector.parseConnectionString(connection as string, client) + : connection; +} + +function createSchemaOverride( + client: string, + name: string, +): Partial { + try { + return mysqlConnector.createSchemaOverride?.(name); + } catch (e) { + throw new InputError( + `Unable to create database schema override for '${client}' connector`, + e, + ); + } +} + +function createNameOverride( + client: string, + name: string, +): Partial { + try { + return mysqlConnector.createNameOverride(name); + } catch (e) { + throw new InputError( + `Unable to create database name override for '${client}' connector`, + e, + ); + } +} + +export class MysqlConnector { + constructor( + private readonly config: Config, + private readonly prefix: string = 'backstage_plugin_', + ) {} + + async getClient( + pluginId: string, + deps?: { + lifecycle: LifecycleService; + pluginMetadata: PluginMetadataService; + }, + ): Promise { + const pluginConfig = new ConfigReader( + this.getConfigForPlugin(pluginId) as JsonObject, + ); + + const databaseName = this.getDatabaseName(pluginId); + if (databaseName && this.getEnsureExistsConfig(pluginId)) { + try { + await mysqlConnector.ensureDatabaseExists(pluginConfig, databaseName); + } catch (error) { + throw new Error( + `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, + ); + } + } + + let schemaOverrides; + if (this.getPluginDivisionModeConfig() === 'schema') { + schemaOverrides = this.getSchemaOverrides(pluginId); + if (this.getEnsureExistsConfig(pluginId)) { + try { + await mysqlConnector.ensureSchemaExists(pluginConfig, pluginId); + } catch (error) { + throw new Error( + `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, + ); + } + } + } + + const databaseClientOverrides = mergeDatabaseConfig( + {}, + this.getDatabaseOverrides(pluginId), + schemaOverrides, + ); + + const client = mysqlConnector.createClient( + pluginConfig, + databaseClientOverrides, + deps, + ); + + return client; + } + + /** + * Provides the canonical database name for a given plugin. + * + * 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 `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 + */ + private getDatabaseName(pluginId: string): string | undefined { + const connection = this.getConnectionConfig(pluginId); + + if (this.getClientType(pluginId).client.includes('sqlite3')) { + const sqliteFilename: string | undefined = ( + connection as Knex.Sqlite3ConnectionConfig + ).filename; + + if (sqliteFilename === ':memory:') { + return sqliteFilename; + } + + const sqliteDirectory = + (connection as { directory?: string }).directory ?? '.'; + + return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); + } + + const databaseName = (connection as Knex.ConnectionConfig)?.database; + + // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database + if (this.getPluginDivisionModeConfig() === 'schema') { + return databaseName; + } + + // all other supported databases should fallback to an auto-prefixed name + return databaseName ?? `${this.prefix}${pluginId}`; + } + + /** + * Provides the client type which should be used for a given plugin. + * + * The client type is determined by plugin specific config if present. + * Otherwise the base client is used as the fallback. + * + * @param pluginId - Plugin to get the client type for + * @returns Object with client type returned as `client` and boolean + * representing whether or not the client was overridden as + * `overridden` + */ + private getClientType(pluginId: string): { + client: string; + overridden: boolean; + } { + const pluginClient = this.config.getOptionalString( + `${pluginPath(pluginId)}.client`, + ); + + const baseClient = this.config.getString('client'); + const client = pluginClient ?? baseClient; + return { + client, + overridden: client !== baseClient, + }; + } + + private getRoleConfig(pluginId: string): string | undefined { + return ( + this.config.getOptionalString(`${pluginPath(pluginId)}.role`) ?? + this.config.getOptionalString('role') + ); + } + + /** + * Provides the knexConfig which should be used for a given plugin. + * + * @param pluginId - Plugin to get the knexConfig for + * @returns The merged knexConfig value or undefined if it isn't specified + */ + private getAdditionalKnexConfig(pluginId: string): JsonObject | undefined { + const pluginConfig = this.config + .getOptionalConfig(`${pluginPath(pluginId)}.knexConfig`) + ?.get(); + + const baseConfig = this.config + .getOptionalConfig('knexConfig') + ?.get(); + + return merge(baseConfig, pluginConfig); + } + + private getEnsureExistsConfig(pluginId: string): boolean { + const baseConfig = this.config.getOptionalBoolean('ensureExists') ?? true; + return ( + this.config.getOptionalBoolean(`${pluginPath(pluginId)}.ensureExists`) ?? + baseConfig + ); + } + + private getPluginDivisionModeConfig(): string { + return this.config.getOptionalString('pluginDivisionMode') ?? 'database'; + } + + /** + * Provides a Knex connection plugin config by combining base and plugin + * config. + * + * This method provides a baseConfig for a plugin database connector. If the + * client type 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 `pluginDivisionMode` is set + * to `schema`. + */ + private getConnectionConfig(pluginId: string): Knex.StaticConnectionConfig { + const { client, overridden } = this.getClientType(pluginId); + + let baseConnection = normalizeConnection( + this.config.get('connection'), + this.config.getString('client'), + ); + + if ( + client.includes('sqlite3') && + 'filename' in baseConnection && + baseConnection.filename !== ':memory:' + ) { + throw new Error( + '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', + ); + } + + // 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.getPluginDivisionModeConfig() !== 'schema') { + baseConnection = omit(baseConnection, 'database'); + } + + // get and normalize optional plugin specific database connection + const connection = normalizeConnection( + this.config.getOptional(`${pluginPath(pluginId)}.connection`), + client, + ); + + if (client === 'pg') { + ( + baseConnection as Knex.PgConnectionConfig + ).application_name ||= `backstage_plugin_${pluginId}`; + } + + return { + // include base connection if client type has not been overridden + ...(overridden ? {} : baseConnection), + ...connection, + } as Knex.StaticConnectionConfig; + } + + /** + * Provides a Knex database config for a given plugin. + * + * This method provides a Knex configuration object along with the plugin's + * client type. + * + * @param pluginId - The plugin that the database config should correspond with + */ + private getConfigForPlugin(pluginId: string): Knex.Config { + const { client } = this.getClientType(pluginId); + const role = this.getRoleConfig(pluginId); + + return { + ...this.getAdditionalKnexConfig(pluginId), + client, + connection: this.getConnectionConfig(pluginId), + ...(role && { role }), + }; + } + + /** + * Provides a partial `Knex.Config` database schema override for a given + * plugin. + * + * @param pluginId - Target plugin to get database schema override + * @returns Partial `Knex.Config` with database schema override + */ + private getSchemaOverrides(pluginId: string): Knex.Config | undefined { + return createSchemaOverride(this.getClientType(pluginId).client, pluginId); + } + + /** + * Provides a partial `Knex.Config`• database name override for a given plugin. + * + * @param pluginId - Target plugin to get database name override + * @returns Partial `Knex.Config` with database name override + */ + private getDatabaseOverrides(pluginId: string): Knex.Config { + const databaseName = this.getDatabaseName(pluginId); + return databaseName + ? createNameOverride(this.getClientType(pluginId).client, databaseName) + : {}; + } +} diff --git a/packages/backend-common/src/database/connectors/postgres.ts b/packages/backend-common/src/database/connectors/postgres.ts index dc9b19f6ad..0a3aa6cb74 100644 --- a/packages/backend-common/src/database/connectors/postgres.ts +++ b/packages/backend-common/src/database/connectors/postgres.ts @@ -14,15 +14,21 @@ * limitations under the License. */ +import { + LifecycleService, + PluginMetadataService, +} from '@backstage/backend-plugin-api'; +import { Config, ConfigReader } from '@backstage/config'; +import { ForwardedError, InputError } from '@backstage/errors'; +import { JsonObject } from '@backstage/types'; import knexFactory, { Knex } from 'knex'; - -import { Config } from '@backstage/config'; -import { ForwardedError } from '@backstage/errors'; +import { merge, omit } from 'lodash'; +import path from 'path'; +import { Client } from 'pg'; import { mergeDatabaseConfig } from '../config'; import { DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; import defaultSchemaOverride from './defaultSchemaOverride'; -import { Client } from 'pg'; /** * Creates a knex postgres database connection @@ -217,11 +223,6 @@ export async function dropPgDatabase( ); } -/** - * PostgreSQL database connector. - * - * Exposes database connector functionality via an immutable object. - */ export const pgConnector: DatabaseConnector = Object.freeze({ createClient: createPgDatabaseClient, ensureDatabaseExists: ensurePgDatabaseExists, @@ -231,3 +232,325 @@ export const pgConnector: DatabaseConnector = Object.freeze({ parseConnectionString: parsePgConnectionString, dropDatabase: dropPgDatabase, }); + +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +/** + * Provides a config lookup path for a plugin's config block. + */ +function pluginPath(pluginId: string): string { + return `plugin.${pluginId}`; +} + +function normalizeConnection( + connection: Knex.StaticConnectionConfig | JsonObject | string | undefined, + client: string, +): Partial { + if (typeof connection === 'undefined' || connection === null) { + return {}; + } + + return typeof connection === 'string' || connection instanceof String + ? pgConnector.parseConnectionString(connection as string, client) + : connection; +} + +function createSchemaOverride( + client: string, + name: string, +): Partial { + try { + return pgConnector.createSchemaOverride?.(name); + } catch (e) { + throw new InputError( + `Unable to create database schema override for '${client}' connector`, + e, + ); + } +} + +function createNameOverride( + client: string, + name: string, +): Partial { + try { + return pgConnector.createNameOverride(name); + } catch (e) { + throw new InputError( + `Unable to create database name override for '${client}' connector`, + e, + ); + } +} + +export class PgConnector { + constructor( + private readonly config: Config, + private readonly prefix: string = 'backstage_plugin_', + ) {} + + async getClient( + pluginId: string, + deps?: { + lifecycle: LifecycleService; + pluginMetadata: PluginMetadataService; + }, + ): Promise { + const pluginConfig = new ConfigReader( + this.getConfigForPlugin(pluginId) as JsonObject, + ); + + const databaseName = this.getDatabaseName(pluginId); + if (databaseName && this.getEnsureExistsConfig(pluginId)) { + try { + await pgConnector.ensureDatabaseExists(pluginConfig, databaseName); + } catch (error) { + throw new Error( + `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, + ); + } + } + + let schemaOverrides; + if (this.getPluginDivisionModeConfig() === 'schema') { + schemaOverrides = this.getSchemaOverrides(pluginId); + if (this.getEnsureExistsConfig(pluginId)) { + try { + await pgConnector.ensureSchemaExists(pluginConfig, pluginId); + } catch (error) { + throw new Error( + `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, + ); + } + } + } + + const databaseClientOverrides = mergeDatabaseConfig( + {}, + this.getDatabaseOverrides(pluginId), + schemaOverrides, + ); + + const client = pgConnector.createClient( + pluginConfig, + databaseClientOverrides, + deps, + ); + + return client; + } + + /** + * Provides the canonical database name for a given plugin. + * + * 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 `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 + */ + private getDatabaseName(pluginId: string): string | undefined { + const connection = this.getConnectionConfig(pluginId); + + if (this.getClientType(pluginId).client.includes('sqlite3')) { + const sqliteFilename: string | undefined = ( + connection as Knex.Sqlite3ConnectionConfig + ).filename; + + if (sqliteFilename === ':memory:') { + return sqliteFilename; + } + + const sqliteDirectory = + (connection as { directory?: string }).directory ?? '.'; + + return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); + } + + const databaseName = (connection as Knex.ConnectionConfig)?.database; + + // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database + if (this.getPluginDivisionModeConfig() === 'schema') { + return databaseName; + } + + // all other supported databases should fallback to an auto-prefixed name + return databaseName ?? `${this.prefix}${pluginId}`; + } + + /** + * Provides the client type which should be used for a given plugin. + * + * The client type is determined by plugin specific config if present. + * Otherwise the base client is used as the fallback. + * + * @param pluginId - Plugin to get the client type for + * @returns Object with client type returned as `client` and boolean + * representing whether or not the client was overridden as + * `overridden` + */ + private getClientType(pluginId: string): { + client: string; + overridden: boolean; + } { + const pluginClient = this.config.getOptionalString( + `${pluginPath(pluginId)}.client`, + ); + + const baseClient = this.config.getString('client'); + const client = pluginClient ?? baseClient; + return { + client, + overridden: client !== baseClient, + }; + } + + private getRoleConfig(pluginId: string): string | undefined { + return ( + this.config.getOptionalString(`${pluginPath(pluginId)}.role`) ?? + this.config.getOptionalString('role') + ); + } + + /** + * Provides the knexConfig which should be used for a given plugin. + * + * @param pluginId - Plugin to get the knexConfig for + * @returns The merged knexConfig value or undefined if it isn't specified + */ + private getAdditionalKnexConfig(pluginId: string): JsonObject | undefined { + const pluginConfig = this.config + .getOptionalConfig(`${pluginPath(pluginId)}.knexConfig`) + ?.get(); + + const baseConfig = this.config + .getOptionalConfig('knexConfig') + ?.get(); + + return merge(baseConfig, pluginConfig); + } + + private getEnsureExistsConfig(pluginId: string): boolean { + const baseConfig = this.config.getOptionalBoolean('ensureExists') ?? true; + return ( + this.config.getOptionalBoolean(`${pluginPath(pluginId)}.ensureExists`) ?? + baseConfig + ); + } + + private getPluginDivisionModeConfig(): string { + return this.config.getOptionalString('pluginDivisionMode') ?? 'database'; + } + + /** + * Provides a Knex connection plugin config by combining base and plugin + * config. + * + * This method provides a baseConfig for a plugin database connector. If the + * client type 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 `pluginDivisionMode` is set + * to `schema`. + */ + private getConnectionConfig(pluginId: string): Knex.StaticConnectionConfig { + const { client, overridden } = this.getClientType(pluginId); + + let baseConnection = normalizeConnection( + this.config.get('connection'), + this.config.getString('client'), + ); + + if ( + client.includes('sqlite3') && + 'filename' in baseConnection && + baseConnection.filename !== ':memory:' + ) { + throw new Error( + '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', + ); + } + + // 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.getPluginDivisionModeConfig() !== 'schema') { + baseConnection = omit(baseConnection, 'database'); + } + + // get and normalize optional plugin specific database connection + const connection = normalizeConnection( + this.config.getOptional(`${pluginPath(pluginId)}.connection`), + client, + ); + + if (client === 'pg') { + ( + baseConnection as Knex.PgConnectionConfig + ).application_name ||= `backstage_plugin_${pluginId}`; + } + + return { + // include base connection if client type has not been overridden + ...(overridden ? {} : baseConnection), + ...connection, + } as Knex.StaticConnectionConfig; + } + + /** + * Provides a Knex database config for a given plugin. + * + * This method provides a Knex configuration object along with the plugin's + * client type. + * + * @param pluginId - The plugin that the database config should correspond with + */ + private getConfigForPlugin(pluginId: string): Knex.Config { + const { client } = this.getClientType(pluginId); + const role = this.getRoleConfig(pluginId); + + return { + ...this.getAdditionalKnexConfig(pluginId), + client, + connection: this.getConnectionConfig(pluginId), + ...(role && { role }), + }; + } + + /** + * Provides a partial `Knex.Config` database schema override for a given + * plugin. + * + * @param pluginId - Target plugin to get database schema override + * @returns Partial `Knex.Config` with database schema override + */ + private getSchemaOverrides(pluginId: string): Knex.Config | undefined { + return createSchemaOverride(this.getClientType(pluginId).client, pluginId); + } + + /** + * Provides a partial `Knex.Config`• database name override for a given plugin. + * + * @param pluginId - Target plugin to get database name override + * @returns Partial `Knex.Config` with database name override + */ + private getDatabaseOverrides(pluginId: string): Knex.Config { + const databaseName = this.getDatabaseName(pluginId); + return databaseName + ? createNameOverride(this.getClientType(pluginId).client, databaseName) + : {}; + } +} diff --git a/packages/backend-common/src/database/connectors/sqlite3.ts b/packages/backend-common/src/database/connectors/sqlite3.ts index 3cb0f57de6..5714b036e3 100644 --- a/packages/backend-common/src/database/connectors/sqlite3.ts +++ b/packages/backend-common/src/database/connectors/sqlite3.ts @@ -14,17 +14,20 @@ * limitations under the License. */ -import { Config } from '@backstage/config'; -import { ensureDirSync } from 'fs-extra'; -import knexFactory, { Knex } from 'knex'; -import path from 'path'; import { DevDataStore } from '@backstage/backend-dev-utils'; -import { mergeDatabaseConfig } from '../config'; -import { DatabaseConnector } from '../types'; import { LifecycleService, PluginMetadataService, } from '@backstage/backend-plugin-api'; +import { Config, ConfigReader } from '@backstage/config'; +import { InputError } from '@backstage/errors'; +import { JsonObject } from '@backstage/types'; +import { ensureDirSync } from 'fs-extra'; +import knexFactory, { Knex } from 'knex'; +import { merge, omit } from 'lodash'; +import path from 'path'; +import { mergeDatabaseConfig } from '../config'; +import { DatabaseConnector } from '../types'; /** * Creates a knex SQLite3 database connection @@ -159,8 +162,330 @@ export function parseSqliteConnectionString( * * Exposes database connector functionality via an immutable object. */ -export const sqlite3Connector: DatabaseConnector = Object.freeze({ +export const sqliteConnector: DatabaseConnector = Object.freeze({ createClient: createSqliteDatabaseClient, createNameOverride: createSqliteNameOverride, parseConnectionString: parseSqliteConnectionString, }); + +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// +// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// + +/** + * Provides a config lookup path for a plugin's config block. + */ +function pluginPath(pluginId: string): string { + return `plugin.${pluginId}`; +} + +function normalizeConnection( + connection: Knex.StaticConnectionConfig | JsonObject | string | undefined, + client: string, +): Partial { + if (typeof connection === 'undefined' || connection === null) { + return {}; + } + + return typeof connection === 'string' || connection instanceof String + ? sqliteConnector.parseConnectionString(connection as string, client) + : connection; +} + +function createSchemaOverride( + client: string, + name: string, +): Partial { + try { + return sqliteConnector.createSchemaOverride?.(name); + } catch (e) { + throw new InputError( + `Unable to create database schema override for '${client}' connector`, + e, + ); + } +} + +function createNameOverride( + client: string, + name: string, +): Partial { + try { + return sqliteConnector.createNameOverride(name); + } catch (e) { + throw new InputError( + `Unable to create database name override for '${client}' connector`, + e, + ); + } +} + +export class Sqlite3Connector implements DatabaseConnector { + constructor( + private readonly config: Config, + private readonly prefix: string = 'backstage_plugin_', + ) {} + + async getClient( + pluginId: string, + deps?: { + lifecycle: LifecycleService; + pluginMetadata: PluginMetadataService; + }, + ): Promise { + const pluginConfig = new ConfigReader( + this.getConfigForPlugin(pluginId) as JsonObject, + ); + + const databaseName = this.getDatabaseName(pluginId); + if (databaseName && this.getEnsureExistsConfig(pluginId)) { + try { + await sqliteConnector.ensureDatabaseExists(pluginConfig, databaseName); + } catch (error) { + throw new Error( + `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, + ); + } + } + + let schemaOverrides; + if (this.getPluginDivisionModeConfig() === 'schema') { + schemaOverrides = this.getSchemaOverrides(pluginId); + if (this.getEnsureExistsConfig(pluginId)) { + try { + await sqliteConnector.ensureSchemaExists(pluginConfig, pluginId); + } catch (error) { + throw new Error( + `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, + ); + } + } + } + + const databaseClientOverrides = mergeDatabaseConfig( + {}, + this.getDatabaseOverrides(pluginId), + schemaOverrides, + ); + + const client = sqliteConnector.createClient( + pluginConfig, + databaseClientOverrides, + deps, + ); + + return client; + } + + /** + * Provides the canonical database name for a given plugin. + * + * 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 `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 + */ + private getDatabaseName(pluginId: string): string | undefined { + const connection = this.getConnectionConfig(pluginId); + + if (this.getClientType(pluginId).client.includes('sqlite3')) { + const sqliteFilename: string | undefined = ( + connection as Knex.Sqlite3ConnectionConfig + ).filename; + + if (sqliteFilename === ':memory:') { + return sqliteFilename; + } + + const sqliteDirectory = + (connection as { directory?: string }).directory ?? '.'; + + return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); + } + + const databaseName = (connection as Knex.ConnectionConfig)?.database; + + // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database + if (this.getPluginDivisionModeConfig() === 'schema') { + return databaseName; + } + + // all other supported databases should fallback to an auto-prefixed name + return databaseName ?? `${this.prefix}${pluginId}`; + } + + /** + * Provides the client type which should be used for a given plugin. + * + * The client type is determined by plugin specific config if present. + * Otherwise the base client is used as the fallback. + * + * @param pluginId - Plugin to get the client type for + * @returns Object with client type returned as `client` and boolean + * representing whether or not the client was overridden as + * `overridden` + */ + private getClientType(pluginId: string): { + client: string; + overridden: boolean; + } { + const pluginClient = this.config.getOptionalString( + `${pluginPath(pluginId)}.client`, + ); + + const baseClient = this.config.getString('client'); + const client = pluginClient ?? baseClient; + return { + client, + overridden: client !== baseClient, + }; + } + + private getRoleConfig(pluginId: string): string | undefined { + return ( + this.config.getOptionalString(`${pluginPath(pluginId)}.role`) ?? + this.config.getOptionalString('role') + ); + } + + /** + * Provides the knexConfig which should be used for a given plugin. + * + * @param pluginId - Plugin to get the knexConfig for + * @returns The merged knexConfig value or undefined if it isn't specified + */ + private getAdditionalKnexConfig(pluginId: string): JsonObject | undefined { + const pluginConfig = this.config + .getOptionalConfig(`${pluginPath(pluginId)}.knexConfig`) + ?.get(); + + const baseConfig = this.config + .getOptionalConfig('knexConfig') + ?.get(); + + return merge(baseConfig, pluginConfig); + } + + private getEnsureExistsConfig(pluginId: string): boolean { + const baseConfig = this.config.getOptionalBoolean('ensureExists') ?? true; + return ( + this.config.getOptionalBoolean(`${pluginPath(pluginId)}.ensureExists`) ?? + baseConfig + ); + } + + private getPluginDivisionModeConfig(): string { + return this.config.getOptionalString('pluginDivisionMode') ?? 'database'; + } + + /** + * Provides a Knex connection plugin config by combining base and plugin + * config. + * + * This method provides a baseConfig for a plugin database connector. If the + * client type 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 `pluginDivisionMode` is set + * to `schema`. + */ + private getConnectionConfig(pluginId: string): Knex.StaticConnectionConfig { + const { client, overridden } = this.getClientType(pluginId); + + let baseConnection = normalizeConnection( + this.config.get('connection'), + this.config.getString('client'), + ); + + if ( + client.includes('sqlite3') && + 'filename' in baseConnection && + baseConnection.filename !== ':memory:' + ) { + throw new Error( + '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', + ); + } + + // 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.getPluginDivisionModeConfig() !== 'schema') { + baseConnection = omit(baseConnection, 'database'); + } + + // get and normalize optional plugin specific database connection + const connection = normalizeConnection( + this.config.getOptional(`${pluginPath(pluginId)}.connection`), + client, + ); + + if (client === 'pg') { + ( + baseConnection as Knex.PgConnectionConfig + ).application_name ||= `backstage_plugin_${pluginId}`; + } + + return { + // include base connection if client type has not been overridden + ...(overridden ? {} : baseConnection), + ...connection, + } as Knex.StaticConnectionConfig; + } + + /** + * Provides a Knex database config for a given plugin. + * + * This method provides a Knex configuration object along with the plugin's + * client type. + * + * @param pluginId - The plugin that the database config should correspond with + */ + private getConfigForPlugin(pluginId: string): Knex.Config { + const { client } = this.getClientType(pluginId); + const role = this.getRoleConfig(pluginId); + + return { + ...this.getAdditionalKnexConfig(pluginId), + client, + connection: this.getConnectionConfig(pluginId), + ...(role && { role }), + }; + } + + /** + * Provides a partial `Knex.Config` database schema override for a given + * plugin. + * + * @param pluginId - Target plugin to get database schema override + * @returns Partial `Knex.Config` with database schema override + */ + private getSchemaOverrides(pluginId: string): Knex.Config | undefined { + return createSchemaOverride(this.getClientType(pluginId).client, pluginId); + } + + /** + * Provides a partial `Knex.Config`• database name override for a given plugin. + * + * @param pluginId - Target plugin to get database name override + * @returns Partial `Knex.Config` with database name override + */ + private getDatabaseOverrides(pluginId: string): Knex.Config { + const databaseName = this.getDatabaseName(pluginId); + return databaseName + ? createNameOverride(this.getClientType(pluginId).client, databaseName) + : {}; + } +} diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 3e9832aec5..c73540a9c1 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -24,11 +24,11 @@ import { Knex } from 'knex'; export type { DatabaseService as PluginDatabaseManager } from '@backstage/backend-plugin-api'; /** - * DatabaseConnector manages an underlying Knex database driver. + * Manages an underlying Knex database driver. */ export interface DatabaseConnector { /** - * createClient provides an instance of a knex database connector. + * Provides an instance of a knex database connector. */ createClient( dbConfig: Config, @@ -38,27 +38,29 @@ export interface DatabaseConnector { pluginMetadata: PluginMetadataService; }, ): Knex; + /** - * createNameOverride provides a partial knex config sufficient to override a - * database name. + * Provides a partial knex config sufficient to override a database name. */ createNameOverride(name: string): Partial; + /** - * createSchemaOverride provides a partial knex config sufficient to override a - * PostgreSQL schema name within utilizing the `searchPath` knex configuration. + * Provides a partial knex config sufficient to override a PostgreSQL schema + * name within utilizing the `searchPath` knex configuration. */ createSchemaOverride?(name: string): Partial; + /** - * parseConnectionString produces a knex connection config object representing - * a database connection string. + * Produces a knex connection config object representing a database connection + * string. */ parseConnectionString( connectionString: string, client?: string, ): Knex.StaticConnectionConfig; + /** - * ensureDatabaseExists performs a side-effect to ensure database names passed in are - * present. + * 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. @@ -69,8 +71,7 @@ export interface DatabaseConnector { ): Promise; /** - * ensureSchemaExists performs a side-effect to ensure schema names passed in are - * present. + * 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. @@ -80,5 +81,8 @@ export interface DatabaseConnector { ...schemas: Array ): Promise; + /** + * Deletes databases. + */ dropDatabase?(dbConfig: Config, ...databases: Array): Promise; } From c936bfc67cdea60edf8f6c6704cf9ec7d8616a58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 28 Apr 2024 11:47:53 +0200 Subject: [PATCH 2/8] slim down the origin database manager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/database/DatabaseManager.test.ts | 2 +- .../src/database/DatabaseManager.ts | 275 ++---------------- .../connectors/defaultNameOverride.test.ts | 1 + .../connectors/defaultNameOverride.ts | 1 + .../connectors/defaultSchemaOverride.test.ts | 1 + .../connectors/defaultSchemaOverride.ts | 1 + .../src/database/connectors/index.ts | 1 + .../mergeDatabaseConfig.test.ts} | 4 +- .../mergeDatabaseConfig.ts} | 0 .../src/database/connectors/mysql.ts | 6 +- .../src/database/connectors/postgres.test.ts | 2 +- .../src/database/connectors/postgres.ts | 6 +- .../src/database/connectors/sqlite3.ts | 6 +- packages/backend-common/src/database/index.ts | 2 +- packages/backend-common/src/database/types.ts | 10 + 15 files changed, 50 insertions(+), 268 deletions(-) rename packages/backend-common/src/database/{config.test.ts => connectors/mergeDatabaseConfig.test.ts} (97%) rename packages/backend-common/src/database/{config.ts => connectors/mergeDatabaseConfig.ts} (100%) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index bf4af17d05..6a461fad55 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -16,12 +16,12 @@ import { ConfigReader } from '@backstage/config'; import { omit } from 'lodash'; import path from 'path'; +import { DatabaseManager } from './DatabaseManager'; import { createDatabaseClient, ensureDatabaseExists, ensureSchemaExists, } from './connection'; -import { DatabaseManager } from './DatabaseManager'; jest.mock('./connection', () => ({ ...jest.requireActual('./connection'), diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 776f05aed4..a261cd13fc 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -14,23 +14,19 @@ * limitations under the License. */ -import { Config, ConfigReader } from '@backstage/config'; -import { JsonObject } from '@backstage/types'; -import { Knex } from 'knex'; -import { merge, omit } from 'lodash'; -import { mergeDatabaseConfig } from './config'; -import { PluginDatabaseManager } from './types'; -import path from 'path'; import { DatabaseService, LifecycleService, LoggerService, PluginMetadataService, } from '@backstage/backend-plugin-api'; +import { Config } from '@backstage/config'; import { stringifyError } from '@backstage/errors'; +import { Knex } from 'knex'; +import { MysqlConnector } from './connectors/mysql'; import { PgConnector } from './connectors/postgres'; import { Sqlite3Connector } from './connectors/sqlite3'; -import { MysqlConnector } from './connectors/mysql'; +import { Connector, PluginDatabaseManager } from './types'; /** * Provides a config lookup path for a plugin's config block. @@ -81,15 +77,15 @@ export class DatabaseManager implements LegacyRootDatabaseService { options?: DatabaseManagerOptions, ): DatabaseManager { const databaseConfig = config.getConfig('backend.database'); + const prefix = databaseConfig.getOptionalString('prefix'); return new DatabaseManager( databaseConfig, - databaseConfig.getOptionalString('prefix'), { - pg: new PgConnector(config, prefix, options), - sqlite3: new Sqlite3Connector(config, prefix, options), - 'better-sqlite3': new Sqlite3Connector(config, prefix, options), - mysql: new MysqlConnector(config, prefix, options), - mysql2: new MysqlConnector(config, prefix, options), + pg: new PgConnector(config, prefix), + sqlite3: new Sqlite3Connector(config, prefix), + 'better-sqlite3': new Sqlite3Connector(config, prefix), + mysql: new MysqlConnector(config, prefix), + mysql2: new MysqlConnector(config, prefix), }, options, ); @@ -97,19 +93,7 @@ export class DatabaseManager implements LegacyRootDatabaseService { private constructor( private readonly config: Config, - private readonly prefix: string = 'backstage_plugin_', - private readonly connectors: Record< - string, - { - getClient( - pluginId: string, - deps?: { - lifecycle: LifecycleService; - pluginMetadata: PluginMetadataService; - }, - ): Promise; - } - >, + private readonly connectors: Record, private readonly options?: DatabaseManagerOptions, private readonly databaseCache: Map> = new Map(), ) {} @@ -131,54 +115,15 @@ export class DatabaseManager implements LegacyRootDatabaseService { const client = this.getClientType(pluginId).client; const connector = this.connectors[client]; if (!connector) { - throw new Error(`Unsupported database client type '${client}'`); + throw new Error( + `Unsupported database client type '${client}' specified for plugin '${pluginId}'`, + ); } - const getClient = () => this.getDatabase(pluginId, deps); + const getClient = () => this.getDatabase(pluginId, connector, deps); const migrations = { skip: false, ...this.options?.migrations }; return { getClient, migrations }; } - /** - * Provides the canonical database name for a given plugin. - * - * 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 `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 - */ - private getDatabaseName(pluginId: string): string | undefined { - const connection = this.getConnectionConfig(pluginId); - - if (this.getClientType(pluginId).client.includes('sqlite3')) { - const sqliteFilename: string | undefined = ( - connection as Knex.Sqlite3ConnectionConfig - ).filename; - - if (sqliteFilename === ':memory:') { - return sqliteFilename; - } - - const sqliteDirectory = - (connection as { directory?: string }).directory ?? '.'; - - return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); - } - - const databaseName = (connection as Knex.ConnectionConfig)?.database; - - // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database - if (this.getPluginDivisionModeConfig() === 'schema') { - return databaseName; - } - - // all other supported databases should fallback to an auto-prefixed name - return databaseName ?? `${this.prefix}${pluginId}`; - } - /** * Provides the client type which should be used for a given plugin. * @@ -206,143 +151,6 @@ export class DatabaseManager implements LegacyRootDatabaseService { }; } - private getRoleConfig(pluginId: string): string | undefined { - return ( - this.config.getOptionalString(`${pluginPath(pluginId)}.role`) ?? - this.config.getOptionalString('role') - ); - } - - /** - * Provides the knexConfig which should be used for a given plugin. - * - * @param pluginId - Plugin to get the knexConfig for - * @returns The merged knexConfig value or undefined if it isn't specified - */ - private getAdditionalKnexConfig(pluginId: string): JsonObject | undefined { - const pluginConfig = this.config - .getOptionalConfig(`${pluginPath(pluginId)}.knexConfig`) - ?.get(); - - const baseConfig = this.config - .getOptionalConfig('knexConfig') - ?.get(); - - return merge(baseConfig, pluginConfig); - } - - private getEnsureExistsConfig(pluginId: string): boolean { - const baseConfig = this.config.getOptionalBoolean('ensureExists') ?? true; - return ( - this.config.getOptionalBoolean(`${pluginPath(pluginId)}.ensureExists`) ?? - baseConfig - ); - } - - private getPluginDivisionModeConfig(): string { - return this.config.getOptionalString('pluginDivisionMode') ?? 'database'; - } - - /** - * Provides a Knex connection plugin config by combining base and plugin - * config. - * - * This method provides a baseConfig for a plugin database connector. If the - * client type 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 `pluginDivisionMode` is set - * to `schema`. - */ - private getConnectionConfig(pluginId: string): Knex.StaticConnectionConfig { - const { client, overridden } = this.getClientType(pluginId); - - let baseConnection = normalizeConnection( - this.config.get('connection'), - this.config.getString('client'), - ); - - if ( - client.includes('sqlite3') && - 'filename' in baseConnection && - baseConnection.filename !== ':memory:' - ) { - throw new Error( - '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', - ); - } - - // 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.getPluginDivisionModeConfig() !== 'schema') { - baseConnection = omit(baseConnection, 'database'); - } - - // get and normalize optional plugin specific database connection - const connection = normalizeConnection( - this.config.getOptional(`${pluginPath(pluginId)}.connection`), - client, - ); - - if (client === 'pg') { - ( - baseConnection as Knex.PgConnectionConfig - ).application_name ||= `backstage_plugin_${pluginId}`; - } - - return { - // include base connection if client type has not been overridden - ...(overridden ? {} : baseConnection), - ...connection, - } as Knex.StaticConnectionConfig; - } - - /** - * Provides a Knex database config for a given plugin. - * - * This method provides a Knex configuration object along with the plugin's - * client type. - * - * @param pluginId - The plugin that the database config should correspond with - */ - private getConfigForPlugin(pluginId: string): Knex.Config { - const { client } = this.getClientType(pluginId); - const role = this.getRoleConfig(pluginId); - - return { - ...this.getAdditionalKnexConfig(pluginId), - client, - connection: this.getConnectionConfig(pluginId), - ...(role && { role }), - }; - } - - /** - * Provides a partial `Knex.Config` database schema override for a given - * plugin. - * - * @param pluginId - Target plugin to get database schema override - * @returns Partial `Knex.Config` with database schema override - */ - private getSchemaOverrides(pluginId: string): Knex.Config | undefined { - return createSchemaOverride(this.getClientType(pluginId).client, pluginId); - } - - /** - * Provides a partial `Knex.Config`• database name override for a given plugin. - * - * @param pluginId - Target plugin to get database name override - * @returns Partial `Knex.Config` with database name override - */ - private getDatabaseOverrides(pluginId: string): Knex.Config { - const databaseName = this.getDatabaseName(pluginId); - return databaseName - ? createNameOverride(this.getClientType(pluginId).client, databaseName) - : {}; - } - /** * Provides a scoped Knex client for a plugin as per application config. * @@ -352,6 +160,7 @@ export class DatabaseManager implements LegacyRootDatabaseService { */ private async getDatabase( pluginId: string, + connector: Connector, deps?: { lifecycle: LifecycleService; pluginMetadata: PluginMetadataService; @@ -361,55 +170,13 @@ export class DatabaseManager implements LegacyRootDatabaseService { return this.databaseCache.get(pluginId)!; } - const clientPromise = Promise.resolve().then(async () => { - const pluginConfig = new ConfigReader( - this.getConfigForPlugin(pluginId) as JsonObject, - ); - - const databaseName = this.getDatabaseName(pluginId); - if (databaseName && this.getEnsureExistsConfig(pluginId)) { - try { - await ensureDatabaseExists(pluginConfig, databaseName); - } catch (error) { - throw new Error( - `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, - ); - } - } - - let schemaOverrides; - if (this.getPluginDivisionModeConfig() === 'schema') { - schemaOverrides = this.getSchemaOverrides(pluginId); - if (this.getEnsureExistsConfig(pluginId)) { - try { - await ensureSchemaExists(pluginConfig, pluginId); - } catch (error) { - throw new Error( - `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, - ); - } - } - } - - const databaseClientOverrides = mergeDatabaseConfig( - {}, - this.getDatabaseOverrides(pluginId), - schemaOverrides, - ); - - const client = createDatabaseClient( - pluginConfig, - databaseClientOverrides, - deps, - ); - if (process.env.NODE_ENV !== 'test') { - this.startKeepaliveLoop(pluginId, client); - } - return client; - }); - + const clientPromise = connector.getClient(pluginId, deps).then(); this.databaseCache.set(pluginId, clientPromise); + if (process.env.NODE_ENV !== 'test') { + clientPromise.then(client => this.startKeepaliveLoop(pluginId, client)); + } + return clientPromise; } diff --git a/packages/backend-common/src/database/connectors/defaultNameOverride.test.ts b/packages/backend-common/src/database/connectors/defaultNameOverride.test.ts index 1da8e6c11e..59f7b5b137 100644 --- a/packages/backend-common/src/database/connectors/defaultNameOverride.test.ts +++ b/packages/backend-common/src/database/connectors/defaultNameOverride.test.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import defaultNameOverride from './defaultNameOverride'; describe('defaultNameOverride()', () => { diff --git a/packages/backend-common/src/database/connectors/defaultNameOverride.ts b/packages/backend-common/src/database/connectors/defaultNameOverride.ts index bc467eb7f6..3f610c51c6 100644 --- a/packages/backend-common/src/database/connectors/defaultNameOverride.ts +++ b/packages/backend-common/src/database/connectors/defaultNameOverride.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { Knex } from 'knex'; /** diff --git a/packages/backend-common/src/database/connectors/defaultSchemaOverride.test.ts b/packages/backend-common/src/database/connectors/defaultSchemaOverride.test.ts index 2db5ecd454..505b9051f7 100644 --- a/packages/backend-common/src/database/connectors/defaultSchemaOverride.test.ts +++ b/packages/backend-common/src/database/connectors/defaultSchemaOverride.test.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import defaultSchemaOverride from './defaultSchemaOverride'; describe('defaultNameOverride()', () => { diff --git a/packages/backend-common/src/database/connectors/defaultSchemaOverride.ts b/packages/backend-common/src/database/connectors/defaultSchemaOverride.ts index 4e76308542..85c6f1ec65 100644 --- a/packages/backend-common/src/database/connectors/defaultSchemaOverride.ts +++ b/packages/backend-common/src/database/connectors/defaultSchemaOverride.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { Knex } from 'knex'; /** diff --git a/packages/backend-common/src/database/connectors/index.ts b/packages/backend-common/src/database/connectors/index.ts index df84ec66ba..9d18e7460a 100644 --- a/packages/backend-common/src/database/connectors/index.ts +++ b/packages/backend-common/src/database/connectors/index.ts @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + export * from './mysql'; export * from './postgres'; export * from './sqlite3'; diff --git a/packages/backend-common/src/database/config.test.ts b/packages/backend-common/src/database/connectors/mergeDatabaseConfig.test.ts similarity index 97% rename from packages/backend-common/src/database/config.test.ts rename to packages/backend-common/src/database/connectors/mergeDatabaseConfig.test.ts index a5bf4a79d2..84177a1208 100644 --- a/packages/backend-common/src/database/config.test.ts +++ b/packages/backend-common/src/database/connectors/mergeDatabaseConfig.test.ts @@ -14,9 +14,9 @@ * limitations under the License. */ -import { mergeDatabaseConfig } from './config'; +import { mergeDatabaseConfig } from './mergeDatabaseConfig'; -describe('config', () => { +describe('mergeDatabaseConfig', () => { describe('mergeDatabaseConfig', () => { it('does not mutate the input object', () => { const input = { diff --git a/packages/backend-common/src/database/config.ts b/packages/backend-common/src/database/connectors/mergeDatabaseConfig.ts similarity index 100% rename from packages/backend-common/src/database/config.ts rename to packages/backend-common/src/database/connectors/mergeDatabaseConfig.ts diff --git a/packages/backend-common/src/database/connectors/mysql.ts b/packages/backend-common/src/database/connectors/mysql.ts index 63464fa6f4..a7cbad271f 100644 --- a/packages/backend-common/src/database/connectors/mysql.ts +++ b/packages/backend-common/src/database/connectors/mysql.ts @@ -25,9 +25,9 @@ import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; import path from 'path'; import yn from 'yn'; -import { mergeDatabaseConfig } from '../config'; -import { DatabaseConnector } from '../types'; +import { Connector, DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; +import { mergeDatabaseConfig } from './mergeDatabaseConfig'; /** * Creates a knex mysql database connection @@ -295,7 +295,7 @@ function createNameOverride( } } -export class MysqlConnector { +export class MysqlConnector implements Connector { constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', diff --git a/packages/backend-common/src/database/connectors/postgres.test.ts b/packages/backend-common/src/database/connectors/postgres.test.ts index 39c9af5ef3..1e07984967 100644 --- a/packages/backend-common/src/database/connectors/postgres.test.ts +++ b/packages/backend-common/src/database/connectors/postgres.test.ts @@ -16,8 +16,8 @@ import { Config, ConfigReader } from '@backstage/config'; import { - createPgDatabaseClient, buildPgDatabaseConfig, + createPgDatabaseClient, getPgConnectionConfig, parsePgConnectionString, } from './postgres'; diff --git a/packages/backend-common/src/database/connectors/postgres.ts b/packages/backend-common/src/database/connectors/postgres.ts index 0a3aa6cb74..4e3ed9aa1d 100644 --- a/packages/backend-common/src/database/connectors/postgres.ts +++ b/packages/backend-common/src/database/connectors/postgres.ts @@ -25,10 +25,10 @@ import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; import path from 'path'; import { Client } from 'pg'; -import { mergeDatabaseConfig } from '../config'; -import { DatabaseConnector } from '../types'; +import { Connector, DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; import defaultSchemaOverride from './defaultSchemaOverride'; +import { mergeDatabaseConfig } from './mergeDatabaseConfig'; /** * Creates a knex postgres database connection @@ -292,7 +292,7 @@ function createNameOverride( } } -export class PgConnector { +export class PgConnector implements Connector { constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', diff --git a/packages/backend-common/src/database/connectors/sqlite3.ts b/packages/backend-common/src/database/connectors/sqlite3.ts index 5714b036e3..514af6d69f 100644 --- a/packages/backend-common/src/database/connectors/sqlite3.ts +++ b/packages/backend-common/src/database/connectors/sqlite3.ts @@ -26,8 +26,8 @@ import { ensureDirSync } from 'fs-extra'; import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; import path from 'path'; -import { mergeDatabaseConfig } from '../config'; -import { DatabaseConnector } from '../types'; +import { Connector, DatabaseConnector } from '../types'; +import { mergeDatabaseConfig } from './mergeDatabaseConfig'; /** * Creates a knex SQLite3 database connection @@ -227,7 +227,7 @@ function createNameOverride( } } -export class Sqlite3Connector implements DatabaseConnector { +export class Sqlite3Connector implements Connector { constructor( private readonly config: Config, private readonly prefix: string = 'backstage_plugin_', diff --git a/packages/backend-common/src/database/index.ts b/packages/backend-common/src/database/index.ts index 330429f0ef..f8bdc22f6d 100644 --- a/packages/backend-common/src/database/index.ts +++ b/packages/backend-common/src/database/index.ts @@ -22,8 +22,8 @@ export * from './DatabaseManager'; */ export { createDatabaseClient, - ensureDatabaseExists, dropDatabase, + ensureDatabaseExists, } from './connection'; export type { PluginDatabaseManager } from './types'; diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index c73540a9c1..967de6966e 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -86,3 +86,13 @@ export interface DatabaseConnector { */ dropDatabase?(dbConfig: Config, ...databases: Array): Promise; } + +export interface Connector { + getClient( + pluginId: string, + deps?: { + lifecycle: LifecycleService; + pluginMetadata: PluginMetadataService; + }, + ): Promise; +} From 5aad3c0d594a2c859ff8d024bee58d445328fee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 28 Apr 2024 12:05:14 +0200 Subject: [PATCH 3/8] strip out client-type checks in each connector MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/database/DatabaseManager.ts | 2 +- .../src/database/connectors/mysql.ts | 39 ++---------------- .../src/database/connectors/postgres.ts | 41 +++---------------- .../src/database/connectors/sqlite3.ts | 6 --- packages/backend-common/src/database/index.ts | 10 ----- 5 files changed, 10 insertions(+), 88 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index a261cd13fc..66fef056bc 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -170,7 +170,7 @@ export class DatabaseManager implements LegacyRootDatabaseService { return this.databaseCache.get(pluginId)!; } - const clientPromise = connector.getClient(pluginId, deps).then(); + const clientPromise = connector.getClient(pluginId, deps); this.databaseCache.set(pluginId, clientPromise); if (process.env.NODE_ENV !== 'test') { diff --git a/packages/backend-common/src/database/connectors/mysql.ts b/packages/backend-common/src/database/connectors/mysql.ts index a7cbad271f..418466e3c6 100644 --- a/packages/backend-common/src/database/connectors/mysql.ts +++ b/packages/backend-common/src/database/connectors/mysql.ts @@ -367,21 +367,6 @@ export class MysqlConnector implements Connector { private getDatabaseName(pluginId: string): string | undefined { const connection = this.getConnectionConfig(pluginId); - if (this.getClientType(pluginId).client.includes('sqlite3')) { - const sqliteFilename: string | undefined = ( - connection as Knex.Sqlite3ConnectionConfig - ).filename; - - if (sqliteFilename === ':memory:') { - return sqliteFilename; - } - - const sqliteDirectory = - (connection as { directory?: string }).directory ?? '.'; - - return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); - } - const databaseName = (connection as Knex.ConnectionConfig)?.database; // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database @@ -464,9 +449,8 @@ export class MysqlConnector implements Connector { * This method provides a baseConfig for a plugin database connector. If the * client type 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 `pluginDivisionMode` is set - * to `schema`. + * connection take precedence over the base. Base database name is omitted + * unless `pluginDivisionMode` is set to `schema`. */ private getConnectionConfig(pluginId: string): Knex.StaticConnectionConfig { const { client, overridden } = this.getClientType(pluginId); @@ -476,20 +460,9 @@ export class MysqlConnector implements Connector { this.config.getString('client'), ); - if ( - client.includes('sqlite3') && - 'filename' in baseConnection && - baseConnection.filename !== ':memory:' - ) { - throw new Error( - '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', - ); - } - // 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`. + // is set to `schema`. if (this.getPluginDivisionModeConfig() !== 'schema') { baseConnection = omit(baseConnection, 'database'); } @@ -500,12 +473,6 @@ export class MysqlConnector implements Connector { client, ); - if (client === 'pg') { - ( - baseConnection as Knex.PgConnectionConfig - ).application_name ||= `backstage_plugin_${pluginId}`; - } - return { // include base connection if client type has not been overridden ...(overridden ? {} : baseConnection), diff --git a/packages/backend-common/src/database/connectors/postgres.ts b/packages/backend-common/src/database/connectors/postgres.ts index 4e3ed9aa1d..5cc81591ad 100644 --- a/packages/backend-common/src/database/connectors/postgres.ts +++ b/packages/backend-common/src/database/connectors/postgres.ts @@ -364,21 +364,6 @@ export class PgConnector implements Connector { private getDatabaseName(pluginId: string): string | undefined { const connection = this.getConnectionConfig(pluginId); - if (this.getClientType(pluginId).client.includes('sqlite3')) { - const sqliteFilename: string | undefined = ( - connection as Knex.Sqlite3ConnectionConfig - ).filename; - - if (sqliteFilename === ':memory:') { - return sqliteFilename; - } - - const sqliteDirectory = - (connection as { directory?: string }).directory ?? '.'; - - return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); - } - const databaseName = (connection as Knex.ConnectionConfig)?.database; // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database @@ -461,9 +446,8 @@ export class PgConnector implements Connector { * This method provides a baseConfig for a plugin database connector. If the * client type 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 `pluginDivisionMode` is set - * to `schema`. + * connection take precedence over the base. Base database name is omitted + * unless `pluginDivisionMode` is set to `schema`. */ private getConnectionConfig(pluginId: string): Knex.StaticConnectionConfig { const { client, overridden } = this.getClientType(pluginId); @@ -473,20 +457,9 @@ export class PgConnector implements Connector { this.config.getString('client'), ); - if ( - client.includes('sqlite3') && - 'filename' in baseConnection && - baseConnection.filename !== ':memory:' - ) { - throw new Error( - '`connection.filename` is not supported for the base sqlite connection. Prefer `connection.directory` or provide a filename for the plugin connection instead.', - ); - } - // 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`. + // is set to `schema`. if (this.getPluginDivisionModeConfig() !== 'schema') { baseConnection = omit(baseConnection, 'database'); } @@ -497,11 +470,9 @@ export class PgConnector implements Connector { client, ); - if (client === 'pg') { - ( - baseConnection as Knex.PgConnectionConfig - ).application_name ||= `backstage_plugin_${pluginId}`; - } + ( + baseConnection as Knex.PgConnectionConfig + ).application_name ||= `backstage_plugin_${pluginId}`; return { // include base connection if client type has not been overridden diff --git a/packages/backend-common/src/database/connectors/sqlite3.ts b/packages/backend-common/src/database/connectors/sqlite3.ts index 514af6d69f..a9c2b93d1c 100644 --- a/packages/backend-common/src/database/connectors/sqlite3.ts +++ b/packages/backend-common/src/database/connectors/sqlite3.ts @@ -432,12 +432,6 @@ export class Sqlite3Connector implements Connector { client, ); - if (client === 'pg') { - ( - baseConnection as Knex.PgConnectionConfig - ).application_name ||= `backstage_plugin_${pluginId}`; - } - return { // include base connection if client type has not been overridden ...(overridden ? {} : baseConnection), diff --git a/packages/backend-common/src/database/index.ts b/packages/backend-common/src/database/index.ts index f8bdc22f6d..464cf4a4de 100644 --- a/packages/backend-common/src/database/index.ts +++ b/packages/backend-common/src/database/index.ts @@ -16,15 +16,5 @@ export * from './DatabaseManager'; -/* - * Undocumented API surface from connection is being reduced for future deprecation. - * Avoid exporting additional symbols. - */ -export { - createDatabaseClient, - dropDatabase, - ensureDatabaseExists, -} from './connection'; - export type { PluginDatabaseManager } from './types'; export { isDatabaseConflictError } from './util'; From 419d4cac7ccca602108b29ef119a3d667f5c0363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Sun, 28 Apr 2024 22:17:12 +0200 Subject: [PATCH 4/8] clean up and expose dropDatabase again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- packages/backend-common/api-report.md | 20 +- .../src/database/DatabaseManager.test.ts | 106 ++++++--- .../src/database/DatabaseManager.ts | 121 +++++++--- .../backend-common/src/database/connection.ts | 218 ------------------ .../src/database/connectors/mysql.ts | 80 ++----- .../src/database/connectors/postgres.ts | 36 ++- .../src/database/connectors/sqlite3.ts | 110 ++------- packages/backend-common/src/database/index.ts | 6 +- packages/backend-common/src/database/types.ts | 2 + 9 files changed, 218 insertions(+), 481 deletions(-) delete mode 100644 packages/backend-common/src/database/connection.ts diff --git a/packages/backend-common/api-report.md b/packages/backend-common/api-report.md index f9e0d76aab..9e64b84777 100644 --- a/packages/backend-common/api-report.md +++ b/packages/backend-common/api-report.md @@ -34,8 +34,6 @@ import { HostDiscovery as HostDiscovery_2 } from '@backstage/backend-app-api'; import { HttpAuthService } from '@backstage/backend-plugin-api'; import { IdentityService } from '@backstage/backend-plugin-api'; import { isChildPath } from '@backstage/cli-common'; -import { Knex } from 'knex'; -import knexFactory from 'knex'; import { KubeConfig } from '@kubernetes/client-node'; import { LifecycleService } from '@backstage/backend-plugin-api'; import { LoadConfigOptionsRemote } from '@backstage/config-loader'; @@ -225,16 +223,6 @@ export interface ContainerRunner { runContainer(opts: RunContainerOptions): Promise; } -// @public -export function createDatabaseClient( - dbConfig: Config, - overrides?: Partial, - deps?: { - lifecycle: LifecycleService; - pluginMetadata: PluginMetadataService; - }, -): knexFactory.Knex; - // @public export function createLegacyAuthAdapters< TOptions extends { @@ -315,13 +303,7 @@ export class DockerContainerRunner implements ContainerRunner { // @public export function dropDatabase( dbConfig: Config, - ...databases: Array -): Promise; - -// @public -export function ensureDatabaseExists( - dbConfig: Config, - ...databases: Array + ...databaseNames: string[] ): Promise; // @public diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index 6a461fad55..ba911b68b9 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -13,44 +13,91 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + import { ConfigReader } from '@backstage/config'; -import { omit } from 'lodash'; -import path from 'path'; -import { DatabaseManager } from './DatabaseManager'; -import { - createDatabaseClient, - ensureDatabaseExists, - ensureSchemaExists, -} from './connection'; +import { DatabaseManagerImpl } from './DatabaseManager'; +import { Connector } from './types'; -jest.mock('./connection', () => ({ - ...jest.requireActual('./connection'), - createDatabaseClient: jest.fn(), - ensureDatabaseExists: jest.fn(), - ensureSchemaExists: jest.fn(), -})); +describe('DatabaseManagerImpl', () => { + afterEach(() => { + jest.clearAllMocks(); + }); -describe('DatabaseManager', () => { - // This is similar to the ts-jest `mocked` helper. - const mocked = (f: Function) => f as jest.Mock; + 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; - afterEach(() => jest.resetAllMocks()); + const impl = new DatabaseManagerImpl( + new ConfigReader({ + client: 'pg', + }), + { + pg: connector1, + notpg: connector2, + }, + ); - describe('DatabaseManager.fromConfig', () => { - const backendConfig = { - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', + await impl.forPlugin('plugin1').getClient(); + expect(connector1.getClient).toHaveBeenCalledTimes(1); + expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined); + expect(connector2.getClient).toHaveBeenCalledTimes(0); + + await impl.forPlugin('plugin1').getClient(); + expect(connector1.getClient).toHaveBeenCalledTimes(1); + expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined); + expect(connector2.getClient).toHaveBeenCalledTimes(0); + + await impl.forPlugin('plugin2').getClient(); + expect(connector1.getClient).toHaveBeenCalledTimes(2); + expect(connector1.getClient).toHaveBeenLastCalledWith('plugin2', undefined); + expect(connector2.getClient).toHaveBeenCalledTimes(0); + }); + + 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( + new ConfigReader({ + client: 'pg', + plugin: { + plugin2: { + client: 'mysql', }, }, + }), + { + pg: connector1, + mysql: connector2, }, - }; + ); + await impl.forPlugin('plugin1').getClient(); + expect(connector1.getClient).toHaveBeenCalledTimes(1); + expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined); + expect(connector2.getClient).toHaveBeenCalledTimes(0); + + await impl.forPlugin('plugin2').getClient(); + expect(connector1.getClient).toHaveBeenCalledTimes(1); + expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined); + expect(connector2.getClient).toHaveBeenCalledTimes(1); + expect(connector2.getClient).toHaveBeenLastCalledWith('plugin2', undefined); + }); + + // eslint-disable-next-line jest/no-commented-out-tests + /* it('accesses the backend.database key', () => { const config = new ConfigReader(backendConfig); const getConfigSpy = jest.spyOn(config, 'getConfig'); @@ -848,4 +895,5 @@ describe('DatabaseManager', () => { ); }); }); + */ }); diff --git a/packages/backend-common/src/database/DatabaseManager.ts b/packages/backend-common/src/database/DatabaseManager.ts index 66fef056bc..dd36cc3b4a 100644 --- a/packages/backend-common/src/database/DatabaseManager.ts +++ b/packages/backend-common/src/database/DatabaseManager.ts @@ -54,44 +54,10 @@ export type LegacyRootDatabaseService = { }; /** - * Manages database connections for Backstage backend plugins. - * - * @public - * @remarks - * - * The database manager allows the user to set connection and client settings on - * a per pluginId basis by defining a database config block under - * `plugin.` in addition to top level defaults. Optionally, a user may - * set `prefix` which is used to prefix generated database names if config is - * not provided. + * Testable implementation class for {@link DatabaseManager} below. */ -export class DatabaseManager implements LegacyRootDatabaseService { - /** - * Creates a {@link DatabaseManager} from `backend.database` config. - * - * @param config - The loaded application configuration. - * @param options - An optional configuration object. - */ - static fromConfig( - config: Config, - options?: DatabaseManagerOptions, - ): DatabaseManager { - const databaseConfig = config.getConfig('backend.database'); - const prefix = databaseConfig.getOptionalString('prefix'); - return new DatabaseManager( - databaseConfig, - { - pg: new PgConnector(config, prefix), - sqlite3: new Sqlite3Connector(config, prefix), - 'better-sqlite3': new Sqlite3Connector(config, prefix), - mysql: new MysqlConnector(config, prefix), - mysql2: new MysqlConnector(config, prefix), - }, - options, - ); - } - - private constructor( +export class DatabaseManagerImpl implements LegacyRootDatabaseService { + constructor( private readonly config: Config, private readonly connectors: Record, private readonly options?: DatabaseManagerOptions, @@ -204,3 +170,84 @@ export class DatabaseManager implements LegacyRootDatabaseService { }, 60 * 1000); } } + +// NOTE: This class looks odd but is kept around for API compatibility reasons +/** + * Manages database connections for Backstage backend plugins. + * + * @public + * @remarks + * + * The database manager allows the user to set connection and client settings on + * a per pluginId basis by defining a database config block under + * `plugin.` in addition to top level defaults. Optionally, a user may + * set `prefix` which is used to prefix generated database names if config is + * not provided. + */ +export class DatabaseManager implements LegacyRootDatabaseService { + /** + * Creates a {@link DatabaseManager} from `backend.database` config. + * + * @param config - The loaded application configuration. + * @param options - An optional configuration object. + */ + static fromConfig( + config: Config, + options?: DatabaseManagerOptions, + ): DatabaseManager { + const databaseConfig = config.getConfig('backend.database'); + const prefix = + databaseConfig.getOptionalString('prefix') || 'backstage_plugin_'; + return new DatabaseManager( + new DatabaseManagerImpl( + databaseConfig, + { + pg: new PgConnector(databaseConfig, prefix), + sqlite3: new Sqlite3Connector(databaseConfig), + 'better-sqlite3': new Sqlite3Connector(databaseConfig), + mysql: new MysqlConnector(databaseConfig, prefix), + mysql2: new MysqlConnector(databaseConfig, prefix), + }, + options, + ), + ); + } + + private constructor(private readonly impl: DatabaseManagerImpl) {} + + /** + * Generates a PluginDatabaseManager for consumption by plugins. + * + * @param pluginId - The plugin that the database manager should be created for. Plugin names + * should be unique as they are used to look up database config overrides under + * `backend.database.plugin`. + */ + forPlugin( + pluginId: string, + deps?: { + lifecycle: LifecycleService; + pluginMetadata: PluginMetadataService; + }, + ): PluginDatabaseManager { + return this.impl.forPlugin(pluginId, deps); + } +} + +/** + * Helper for deleting databases, only exists for backend-test-utils for now. + * + * @public + */ +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-common/src/database/connection.ts b/packages/backend-common/src/database/connection.ts deleted file mode 100644 index 875734aa19..0000000000 --- a/packages/backend-common/src/database/connection.ts +++ /dev/null @@ -1,218 +0,0 @@ -/* - * Copyright 2024 The Backstage Authors - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -// * Copyright 2020 The Backstage Authors -// * -// * Licensed under the Apache License, Version 2.0 (the "License"); -// * you may not use this file except in compliance with the License. -// * You may obtain a copy of the License at -// * -// * http://www.apache.org/licenses/LICENSE-2.0 -// * -// * Unless required by applicable law or agreed to in writing, software -// * distributed under the License is distributed on an "AS IS" BASIS, -// * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// * See the License for the specific language governing permissions and -// * limitations under the License. -// */ - -// import { Config } from '@backstage/config'; -// import { JsonObject } from '@backstage/types'; -// import { InputError } from '@backstage/errors'; -// import knexFactory, { Knex } from 'knex'; -// import limiterFactory from 'p-limit'; -// import { mergeDatabaseConfig } from './config'; -// import { DatabaseConnector } from './types'; - -// import { mysqlConnector, pgConnector, sqlite3Connector } from './connectors'; -// import { -// LifecycleService, -// PluginMetadataService, -// } from '@backstage/backend-plugin-api'; - -// type DatabaseClient = -// | 'pg' -// | 'better-sqlite3' -// | 'sqlite3' -// | 'mysql' -// | 'mysql2' -// | string; - -// // This limits the number of concurrent CREATE DATABASE and CREATE SCHEMA -// // commands, globally, to just one. This is overly defensive, and was added as -// // an attempt to counteract the pool issues on recent node versions. See -// // https://github.com/backstage/backstage/pull/19988 -// const ddlLimiter = limiterFactory(1); - -// /** -// * Mapping of client type to supported database connectors -// * -// * Database connectors can be aliased here, for example mysql2 uses -// * the same connector as mysql. -// */ -// const ConnectorMapping: Record = { -// pg: pgConnector, -// 'better-sqlite3': sqlite3Connector, -// sqlite3: sqlite3Connector, -// mysql: mysqlConnector, -// mysql2: mysqlConnector, -// }; - -// /** -// * Creates a knex database connection -// * -// * @public -// * @param dbConfig - The database config -// * @param overrides - Additional options to merge with the config -// */ -// export function createDatabaseClient( -// dbConfig: Config, -// overrides?: Partial, -// deps?: { -// lifecycle: LifecycleService; -// pluginMetadata: PluginMetadataService; -// }, -// ) { -// const client: DatabaseClient = dbConfig.getString('client'); - -// return ( -// ConnectorMapping[client]?.createClient(dbConfig, overrides, deps) ?? -// knexFactory(mergeDatabaseConfig(dbConfig.get(), overrides)) -// ); -// } - -// /** -// * Ensures that the given databases all exist, creating them if they do not. -// * -// * @public -// */ -// export async function ensureDatabaseExists( -// dbConfig: Config, -// ...databases: Array -// ): Promise { -// const client: DatabaseClient = dbConfig.getString('client'); - -// return await ddlLimiter(() => -// ConnectorMapping[client]?.ensureDatabaseExists?.(dbConfig, ...databases), -// ); -// } - -// /** -// * Drops the given databases. -// * -// * @public -// */ -// export async function dropDatabase( -// dbConfig: Config, -// ...databases: Array -// ): Promise { -// const client: DatabaseClient = dbConfig.getString('client'); - -// return await ddlLimiter(() => -// ConnectorMapping[client]?.dropDatabase?.(dbConfig, ...databases), -// ); -// } - -// /** -// * Ensures that the given schemas all exist, creating them if they do not. -// * -// * @public -// */ -// export async function ensureSchemaExists( -// dbConfig: Config, -// ...schemas: Array -// ): Promise { -// const client: DatabaseClient = dbConfig.getString('client'); - -// return await ddlLimiter(() => -// ConnectorMapping[client]?.ensureSchemaExists?.(dbConfig, ...schemas), -// ); -// } - -// /** -// * Provides a `Knex.Config` object with the provided database name for a given -// * client. -// */ -// export function createNameOverride( -// client: string, -// name: string, -// ): Partial { -// try { -// return ConnectorMapping[client].createNameOverride(name); -// } catch (e) { -// throw new InputError( -// `Unable to create database name override for '${client}' connector`, -// e, -// ); -// } -// } - -// /** -// * Provides a `Knex.Config` object with the provided database schema for a given -// * client. Currently only supported by `pg`. -// */ -// export function createSchemaOverride( -// client: string, -// name: string, -// ): Partial { -// try { -// return ConnectorMapping[client]?.createSchemaOverride?.(name); -// } catch (e) { -// throw new InputError( -// `Unable to create database schema override for '${client}' connector`, -// e, -// ); -// } -// } - -// /** -// * Parses a connection string for a given client and provides a connection config. -// */ -// export function parseConnectionString( -// connectionString: string, -// client?: string, -// ): Knex.StaticConnectionConfig { -// if (typeof client === 'undefined' || client === null) { -// throw new InputError( -// 'Database connection string client type auto-detection is not yet supported.', -// ); -// } - -// try { -// return ConnectorMapping[client].parseConnectionString(connectionString); -// } catch (e) { -// throw new InputError( -// `Unable to parse connection string for '${client}' connector`, -// ); -// } -// } - -// /** -// * Normalizes a connection config or string into an object which can be passed -// * to Knex. -// */ -// export function normalizeConnection( -// connection: Knex.StaticConnectionConfig | JsonObject | string | undefined, -// client: string, -// ): Partial { -// if (typeof connection === 'undefined' || connection === null) { -// return {}; -// } - -// return typeof connection === 'string' || connection instanceof String -// ? parseConnectionString(connection as string, client) -// : connection; -// } diff --git a/packages/backend-common/src/database/connectors/mysql.ts b/packages/backend-common/src/database/connectors/mysql.ts index 418466e3c6..25594d5021 100644 --- a/packages/backend-common/src/database/connectors/mysql.ts +++ b/packages/backend-common/src/database/connectors/mysql.ts @@ -23,7 +23,6 @@ import { InputError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; -import path from 'path'; import yn from 'yn'; import { Connector, DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; @@ -236,17 +235,6 @@ export const mysqlConnector: DatabaseConnector = Object.freeze({ dropDatabase: dropMysqlDatabase, }); -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - /** * Provides a config lookup path for a plugin's config block. */ @@ -267,20 +255,6 @@ function normalizeConnection( : connection; } -function createSchemaOverride( - client: string, - name: string, -): Partial { - try { - return mysqlConnector.createSchemaOverride?.(name); - } catch (e) { - throw new InputError( - `Unable to create database schema override for '${client}' connector`, - e, - ); - } -} - function createNameOverride( client: string, name: string, @@ -298,7 +272,7 @@ function createNameOverride( export class MysqlConnector implements Connector { constructor( private readonly config: Config, - private readonly prefix: string = 'backstage_plugin_', + private readonly prefix: string, ) {} async getClient( @@ -315,7 +289,7 @@ export class MysqlConnector implements Connector { const databaseName = this.getDatabaseName(pluginId); if (databaseName && this.getEnsureExistsConfig(pluginId)) { try { - await mysqlConnector.ensureDatabaseExists(pluginConfig, databaseName); + await mysqlConnector.ensureDatabaseExists!(pluginConfig, databaseName); } catch (error) { throw new Error( `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, @@ -323,24 +297,16 @@ export class MysqlConnector implements Connector { } } - let schemaOverrides; - if (this.getPluginDivisionModeConfig() === 'schema') { - schemaOverrides = this.getSchemaOverrides(pluginId); - if (this.getEnsureExistsConfig(pluginId)) { - try { - await mysqlConnector.ensureSchemaExists(pluginConfig, pluginId); - } catch (error) { - throw new Error( - `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, - ); - } - } + const pluginDivisionMode = this.getPluginDivisionModeConfig(); + if (pluginDivisionMode !== 'database') { + throw new Error( + `The MySQL driver does not suppoert plugin division mode '${pluginDivisionMode}'`, + ); } const databaseClientOverrides = mergeDatabaseConfig( {}, this.getDatabaseOverrides(pluginId), - schemaOverrides, ); const client = mysqlConnector.createClient( @@ -352,29 +318,24 @@ 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. * - * 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 `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. + * This method provides the effective database name which is determined using + * global and plugin specific database config. If no explicit database name, + * this method will provide a generated name which is the pluginId prefixed + * with 'backstage_plugin_'. * * @param pluginId - Lookup the database name for given plugin * @returns String representing the plugin's database name */ private getDatabaseName(pluginId: string): string | undefined { const connection = this.getConnectionConfig(pluginId); - const databaseName = (connection as Knex.ConnectionConfig)?.database; - - // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database - if (this.getPluginDivisionModeConfig() === 'schema') { - return databaseName; - } - - // all other supported databases should fallback to an auto-prefixed name return databaseName ?? `${this.prefix}${pluginId}`; } @@ -500,17 +461,6 @@ export class MysqlConnector implements Connector { }; } - /** - * Provides a partial `Knex.Config` database schema override for a given - * plugin. - * - * @param pluginId - Target plugin to get database schema override - * @returns Partial `Knex.Config` with database schema override - */ - private getSchemaOverrides(pluginId: string): Knex.Config | undefined { - return createSchemaOverride(this.getClientType(pluginId).client, pluginId); - } - /** * Provides a partial `Knex.Config`• database name override for a given plugin. * diff --git a/packages/backend-common/src/database/connectors/postgres.ts b/packages/backend-common/src/database/connectors/postgres.ts index 5cc81591ad..39a1073a30 100644 --- a/packages/backend-common/src/database/connectors/postgres.ts +++ b/packages/backend-common/src/database/connectors/postgres.ts @@ -23,7 +23,6 @@ import { ForwardedError, InputError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; -import path from 'path'; import { Client } from 'pg'; import { Connector, DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; @@ -216,11 +215,15 @@ export async function dropPgDatabase( ...databases: Array ) { const admin = createPgDatabaseClient(dbConfig); - await Promise.all( - databases.map(async database => { - await admin.raw(`DROP DATABASE ??`, [database]); - }), - ); + try { + await Promise.all( + databases.map(async database => { + await admin.raw(`DROP DATABASE ??`, [database]); + }), + ); + } finally { + await admin.destroy(); + } } export const pgConnector: DatabaseConnector = Object.freeze({ @@ -233,17 +236,6 @@ export const pgConnector: DatabaseConnector = Object.freeze({ dropDatabase: dropPgDatabase, }); -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - /** * Provides a config lookup path for a plugin's config block. */ @@ -295,7 +287,7 @@ function createNameOverride( export class PgConnector implements Connector { constructor( private readonly config: Config, - private readonly prefix: string = 'backstage_plugin_', + private readonly prefix: string, ) {} async getClient( @@ -312,7 +304,7 @@ export class PgConnector implements Connector { const databaseName = this.getDatabaseName(pluginId); if (databaseName && this.getEnsureExistsConfig(pluginId)) { try { - await pgConnector.ensureDatabaseExists(pluginConfig, databaseName); + await pgConnector.ensureDatabaseExists!(pluginConfig, databaseName); } catch (error) { throw new Error( `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, @@ -325,7 +317,7 @@ export class PgConnector implements Connector { schemaOverrides = this.getSchemaOverrides(pluginId); if (this.getEnsureExistsConfig(pluginId)) { try { - await pgConnector.ensureSchemaExists(pluginConfig, pluginId); + await pgConnector.ensureSchemaExists!(pluginConfig, pluginId); } catch (error) { throw new Error( `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, @@ -349,6 +341,10 @@ 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-common/src/database/connectors/sqlite3.ts b/packages/backend-common/src/database/connectors/sqlite3.ts index a9c2b93d1c..46ff756854 100644 --- a/packages/backend-common/src/database/connectors/sqlite3.ts +++ b/packages/backend-common/src/database/connectors/sqlite3.ts @@ -168,17 +168,6 @@ export const sqliteConnector: DatabaseConnector = Object.freeze({ parseConnectionString: parseSqliteConnectionString, }); -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// -// ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// - /** * Provides a config lookup path for a plugin's config block. */ @@ -199,20 +188,6 @@ function normalizeConnection( : connection; } -function createSchemaOverride( - client: string, - name: string, -): Partial { - try { - return sqliteConnector.createSchemaOverride?.(name); - } catch (e) { - throw new InputError( - `Unable to create database schema override for '${client}' connector`, - e, - ); - } -} - function createNameOverride( client: string, name: string, @@ -228,10 +203,7 @@ function createNameOverride( } export class Sqlite3Connector implements Connector { - constructor( - private readonly config: Config, - private readonly prefix: string = 'backstage_plugin_', - ) {} + constructor(private readonly config: Config) {} async getClient( pluginId: string, @@ -244,35 +216,16 @@ export class Sqlite3Connector implements Connector { this.getConfigForPlugin(pluginId) as JsonObject, ); - const databaseName = this.getDatabaseName(pluginId); - if (databaseName && this.getEnsureExistsConfig(pluginId)) { - try { - await sqliteConnector.ensureDatabaseExists(pluginConfig, databaseName); - } catch (error) { - throw new Error( - `Failed to connect to the database to make sure that '${databaseName}' exists, ${error}`, - ); - } - } - - let schemaOverrides; - if (this.getPluginDivisionModeConfig() === 'schema') { - schemaOverrides = this.getSchemaOverrides(pluginId); - if (this.getEnsureExistsConfig(pluginId)) { - try { - await sqliteConnector.ensureSchemaExists(pluginConfig, pluginId); - } catch (error) { - throw new Error( - `Failed to connect to the database to make sure that schema for plugin '${pluginId}' exists, ${error}`, - ); - } - } + const pluginDivisionMode = this.getPluginDivisionModeConfig(); + if (pluginDivisionMode !== 'database') { + throw new Error( + `The SQLite driver does not suppoert plugin division mode '${pluginDivisionMode}'`, + ); } const databaseClientOverrides = mergeDatabaseConfig( {}, this.getDatabaseOverrides(pluginId), - schemaOverrides, ); const client = sqliteConnector.createClient( @@ -284,6 +237,10 @@ export class Sqlite3Connector implements Connector { return client; } + async dropDatabase(..._databaseNames: string[]): Promise { + // do nothing + } + /** * Provides the canonical database name for a given plugin. * @@ -299,30 +256,18 @@ export class Sqlite3Connector implements Connector { private getDatabaseName(pluginId: string): string | undefined { const connection = this.getConnectionConfig(pluginId); - if (this.getClientType(pluginId).client.includes('sqlite3')) { - const sqliteFilename: string | undefined = ( - connection as Knex.Sqlite3ConnectionConfig - ).filename; + const sqliteFilename: string | undefined = ( + connection as Knex.Sqlite3ConnectionConfig + ).filename; - if (sqliteFilename === ':memory:') { - return sqliteFilename; - } - - const sqliteDirectory = - (connection as { directory?: string }).directory ?? '.'; - - return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); + if (sqliteFilename === ':memory:') { + return sqliteFilename; } - const databaseName = (connection as Knex.ConnectionConfig)?.database; + const sqliteDirectory = + (connection as { directory?: string }).directory ?? '.'; - // `pluginDivisionMode` as `schema` should use overridden databaseName if supplied or fallback to default knex database - if (this.getPluginDivisionModeConfig() === 'schema') { - return databaseName; - } - - // all other supported databases should fallback to an auto-prefixed name - return databaseName ?? `${this.prefix}${pluginId}`; + return path.join(sqliteDirectory, sqliteFilename ?? `${pluginId}.sqlite`); } /** @@ -377,14 +322,6 @@ export class Sqlite3Connector implements Connector { return merge(baseConfig, pluginConfig); } - private getEnsureExistsConfig(pluginId: string): boolean { - const baseConfig = this.config.getOptionalBoolean('ensureExists') ?? true; - return ( - this.config.getOptionalBoolean(`${pluginPath(pluginId)}.ensureExists`) ?? - baseConfig - ); - } - private getPluginDivisionModeConfig(): string { return this.config.getOptionalString('pluginDivisionMode') ?? 'database'; } @@ -459,17 +396,6 @@ export class Sqlite3Connector implements Connector { }; } - /** - * Provides a partial `Knex.Config` database schema override for a given - * plugin. - * - * @param pluginId - Target plugin to get database schema override - * @returns Partial `Knex.Config` with database schema override - */ - private getSchemaOverrides(pluginId: string): Knex.Config | undefined { - return createSchemaOverride(this.getClientType(pluginId).client, pluginId); - } - /** * Provides a partial `Knex.Config`• database name override for a given plugin. * diff --git a/packages/backend-common/src/database/index.ts b/packages/backend-common/src/database/index.ts index 464cf4a4de..a0a61ccdd0 100644 --- a/packages/backend-common/src/database/index.ts +++ b/packages/backend-common/src/database/index.ts @@ -14,7 +14,11 @@ * limitations under the License. */ -export * from './DatabaseManager'; +export { DatabaseManager, dropDatabase } from './DatabaseManager'; +export type { + DatabaseManagerOptions, + LegacyRootDatabaseService, +} from './DatabaseManager'; export type { PluginDatabaseManager } from './types'; export { isDatabaseConflictError } from './util'; diff --git a/packages/backend-common/src/database/types.ts b/packages/backend-common/src/database/types.ts index 967de6966e..a9cceaa1f9 100644 --- a/packages/backend-common/src/database/types.ts +++ b/packages/backend-common/src/database/types.ts @@ -95,4 +95,6 @@ export interface Connector { pluginMetadata: PluginMetadataService; }, ): Promise; + + dropDatabase(...databaseNames: string[]): Promise; } From db744f7a5195581434185650279608b57a94e701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 29 Apr 2024 09:52:46 +0200 Subject: [PATCH 5/8] clean up manager tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../src/database/DatabaseManager.test.ts | 808 +----------------- 1 file changed, 16 insertions(+), 792 deletions(-) diff --git a/packages/backend-common/src/database/DatabaseManager.test.ts b/packages/backend-common/src/database/DatabaseManager.test.ts index ba911b68b9..347ec742c0 100644 --- a/packages/backend-common/src/database/DatabaseManager.test.ts +++ b/packages/backend-common/src/database/DatabaseManager.test.ts @@ -96,804 +96,28 @@ describe('DatabaseManagerImpl', () => { expect(connector2.getClient).toHaveBeenLastCalledWith('plugin2', undefined); }); - // eslint-disable-next-line jest/no-commented-out-tests - /* - it('accesses the backend.database key', () => { - const config = new ConfigReader(backendConfig); - const getConfigSpy = jest.spyOn(config, 'getConfig'); - DatabaseManager.fromConfig(config); + it('retains the migration skip info', async () => { + const connector = { + getClient: jest.fn(), + dropDatabase: jest.fn(), + } satisfies Connector; - expect(getConfigSpy).toHaveBeenCalledWith('backend.database'); + const impl1 = new DatabaseManagerImpl(new ConfigReader({ client: 'pg' }), { + pg: connector, }); - it('handles default options', () => { - const config = new ConfigReader(backendConfig); - const database = DatabaseManager.fromConfig(config); - const client = database.forPlugin('test'); + const impl2 = new DatabaseManagerImpl( + new ConfigReader({ client: 'pg' }), + { pg: connector }, + { migrations: { skip: true } }, + ); - expect(client.migrations?.skip).toBe(false); + expect((await impl1.forPlugin('plugin1')).migrations).toEqual({ + skip: false, }); - it('handles migrations options', () => { - const config = new ConfigReader(backendConfig); - const database = DatabaseManager.fromConfig(config, { - migrations: { skip: true }, - }); - const client = database.forPlugin('test'); - - expect(client.migrations?.skip).toBe(true); + expect((await impl2.forPlugin('plugin1')).migrations).toEqual({ + skip: true, }); }); - - describe('DatabaseManager.forPlugin', () => { - const config = { - backend: { - database: { - client: 'pg', - prefix: 'test_prefix_', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - plugin: { - testdbname: { - connection: { - database: 'database_name_overridden', - }, - }, - differentclient: { - client: 'better-sqlite3', - connection: { - filename: 'plugin_with_different_client', - }, - }, - differentclientconnstring: { - client: 'better-sqlite3', - connection: ':memory:', - }, - stringoverride: { - connection: 'postgresql://testuser:testpass@acme:5432/userdbname', - }, - }, - }, - }, - }; - let manager: DatabaseManager; - - beforeEach(() => { - manager = DatabaseManager.fromConfig(new ConfigReader(config)); - }); - - it('connects to a plugin database using default config', async () => { - const pluginId = 'pluginwithoutconfig'; - - await manager.forPlugin(pluginId).getClient(); - expect(mocked(createDatabaseClient)).toHaveBeenCalledTimes(1); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - // default config should be passed through to underlying connector - expect(baseConfig.get()).toMatchObject({ - client: 'pg', - connection: omit(config.backend.database.connection, ['database']), - }); - - // override using database name generated from pluginId and prefix - expect(overrides).toMatchObject({ - connection: { - database: `${config.backend.database.prefix}${pluginId}`, - }, - }); - }); - - it('provides a plugin db which uses components from top level connection string', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: 'postgresql://foo:bar@acme:5432/foodb', - }, - }, - }), - ); - - await testManager.forPlugin('pluginwithoutconfig').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - // parsed connection string **without** db name should be passed through - expect(baseConfig.get()).toMatchObject({ - connection: { - host: 'acme', - user: 'foo', - password: 'bar', - port: '5432', - application_name: 'backstage_plugin_pluginwithoutconfig', - }, - }); - - // we expect a pg database name override with ${prefix} followed by pluginId - expect(overrides).toHaveProperty( - 'connection.database', - expect.stringContaining('pluginwithoutconfig'), - ); - }); - - it('provides an inmemory sqlite database if top level is also inmemory and plugin config is not present', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - connection: ':memory:', - }, - }, - }), - ); - - await testManager.forPlugin('pluginwithoutconfig').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.filename', - expect.stringContaining(':memory:'), - ); - }); - - it('throws if top level sqlite filename is provided', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - connection: 'some-file-path', - }, - }, - }), - ); - - await expect( - testManager.forPlugin('pluginwithoutconfig').getClient(), - ).rejects.toBeInstanceOf(Error); - }); - - it('creates plugin-specific sqlite files when plugin config is not present', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - connection: { - directory: 'sqlite-files', - }, - }, - }, - }), - ); - - await testManager.forPlugin('pluginwithoutconfig').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.filename', - path.join('sqlite-files', 'pluginwithoutconfig.sqlite'), - ); - }); - - it('uses sqlite directory from top level config and filename from plugin config', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - connection: { - directory: 'sqlite-files', - }, - plugin: { - test: { - connection: { - filename: 'other.sqlite', - }, - }, - }, - }, - }, - }), - ); - - await testManager.forPlugin('test').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.filename', - path.join('sqlite-files', 'other.sqlite'), - ); - }); - - it('uses sqlite directory and filename from plugin config', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - connection: { - directory: 'sqlite-files', - }, - plugin: { - test: { - connection: { - directory: 'custom-sqlite-files', - filename: 'other.sqlite', - }, - }, - }, - }, - }, - }), - ); - - await testManager.forPlugin('test').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.filename', - path.join('custom-sqlite-files', 'other.sqlite'), - ); - }); - - it('connects to a plugin database using a specific database name', async () => { - // testdbname.connection.database is set in config - await manager.forPlugin('testdbname').getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_baseConfig, overrides] = mockCalls[0]; - - // simple case where only database name is overridden - expect(overrides).toMatchObject({ - connection: { - database: 'database_name_overridden', - }, - }); - }); - - it('ensure plugin specific database is created', async () => { - const pluginId = 'testdbname'; - // testdbname.connection.database is set in config - await manager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(ensureDatabaseExists).mock.calls.splice(-1); - const [_, dbname] = mockCalls[0]; - - expect(dbname).toEqual( - config.backend.database.plugin[pluginId].connection.database, - ); - }); - - it('provides different plugins with their own databases', async () => { - await manager.forPlugin('plugin1').getClient(); - await manager.forPlugin('plugin2').getClient(); - - expect(mocked(createDatabaseClient)).toHaveBeenCalledTimes(2); - - const mockCalls = mocked(createDatabaseClient).mock.calls; - const [plugin1CallArgs, plugin2CallArgs] = mockCalls; - - // database name overrides should be different - expect(plugin1CallArgs[1].connection.database).not.toEqual( - plugin2CallArgs[1].connection.database, - ); - }); - - it('returns the same client for the same pluginId', async () => { - const [client1, client2] = await Promise.all([ - manager.forPlugin('plugin1').getClient(), - manager.forPlugin('plugin1').getClient(), - ]); - expect(mocked(createDatabaseClient)).toHaveBeenCalledTimes(1); - - expect(client1).toBe(client2); - }); - - it('uses plugin connection as base if default client is different from plugin client', async () => { - const pluginId = 'differentclient'; - await manager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, _overrides] = mockCalls[0]; - - // plugin connection should be used as base config, client is different - expect(baseConfig.get()).toMatchObject({ - client: 'better-sqlite3', - connection: config.backend.database.plugin[pluginId].connection, - }); - }); - - it('provides database client specific base and override when client set under plugin', async () => { - const pluginId = 'differentclient'; - await manager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - // plugin client should be better-sqlite3 - expect(baseConfig.get().client).toEqual('better-sqlite3'); - - // SQLite uses 'filename' instead of 'database' - expect(overrides).toHaveProperty( - 'connection.filename', - 'plugin_with_different_client', - ); - }); - - it('provides database client specific base from plugin connection string when client set under plugin', async () => { - const pluginId = 'differentclientconnstring'; - await manager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - expect(baseConfig.get().client).toEqual('better-sqlite3'); - - expect(overrides).toHaveProperty('connection.filename', ':memory:'); - }); - - it('generates a database name override when prefix is not explicitly set', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }), - ); - - await testManager.forPlugin('testplugin').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_baseConfig, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.database', - expect.stringContaining('backstage_plugin_'), - ); - }); - - it('generates a database name override when prefix is not explicitly set for mysql', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'mysql', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }), - ); - - await testManager.forPlugin('testplugin').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_baseConfig, overrides] = mockCalls[0]; - - expect(overrides).toHaveProperty( - 'connection.database', - expect.stringContaining('backstage_plugin_'), - ); - }); - - it('uses values from plugin connection string if top level client should be used', async () => { - const pluginId = 'stringoverride'; - await manager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - // plugin client should be pg - expect(baseConfig.get().client).toEqual('pg'); - - expect(overrides).toHaveProperty( - 'connection.database', - expect.stringContaining('userdbname'), - ); - }); - - it('plugin sets schema override for pg client', async () => { - const overrideConfig = { - backend: { - database: { - client: 'pg', - pluginDivisionMode: 'schema', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }; - const testManager = DatabaseManager.fromConfig( - new ConfigReader(overrideConfig), - ); - const pluginId = 'schemaoverride'; - await testManager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - expect(baseConfig.get()).toMatchObject({ - client: 'pg', - connection: config.backend.database.connection, - }); - - expect(overrides).toMatchObject({ - searchPath: [pluginId], - }); - }); - - it('plugin does not provide schema override for non pg client', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'better-sqlite3', - pluginDivisionMode: 'schema', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }), - ); - const pluginId = 'any-plugin'; - await testManager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - expect(baseConfig.get()).toMatchObject({ - client: 'better-sqlite3', - connection: config.backend.database.connection, - }); - - expect(overrides).not.toHaveProperty('searchPath'); - }); - - it('plugin does not provide schema override if pluginDivisionMode is set to database', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - pluginDivisionMode: 'database', - connection: 'some-file-path', - }, - }, - }), - ); - - const pluginId = 'any-plugin'; - await testManager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_baseConfig, overrides] = mockCalls[0]; - - expect(overrides).not.toHaveProperty('searchPath'); - }); - - it('plugin does not provide schema override if pluginDivisionMode is not set', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }), - ); - - const pluginId = 'schemaoverride'; - await testManager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [_baseConfig, overrides] = mockCalls[0]; - - expect(overrides).not.toHaveProperty('searchPath'); - }); - - it('pluginDivisionMode ensures that each plugin schema exists', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - pluginDivisionMode: 'schema', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }), - ); - const pluginId = 'testdbname'; - await testManager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(ensureSchemaExists).mock.calls; - const [_, schemaName] = mockCalls[0]; - - expect(schemaName).toEqual('testdbname'); - }); - - it('pluginDivisionMode allows connection overrides for plugins', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - pluginDivisionMode: 'schema', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - plugin: { - testdbname: { - connection: { - database: 'database_name_overridden', - host: 'newhost', - }, - }, - }, - }, - }, - }), - ); - const pluginId = 'testdbname'; - await testManager.forPlugin(pluginId).getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - - expect(baseConfig.get()).toMatchObject({ - client: 'pg', - connection: { - database: 'database_name_overridden', - host: 'newhost', - user: 'foo', - password: 'bar', - }, - }); - expect(overrides).toHaveProperty('searchPath', ['testdbname']); - expect(overrides).toHaveProperty( - 'connection.database', - 'database_name_overridden', - ); - }); - - it('ensureExists does not create database or schema when false', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - pluginDivisionMode: 'schema', - ensureExists: false, - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - }, - }, - }, - }), - ); - const pluginId = 'testdbname'; - await testManager.forPlugin(pluginId).getClient(); - - expect(mocked(ensureDatabaseExists)).toHaveBeenCalledTimes(0); - expect(mocked(ensureSchemaExists)).toHaveBeenCalledTimes(0); - }); - - it('fetches and merges additional knex config', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - database: 'foodb', - }, - knexConfig: { - something: false, - }, - plugin: { - testdbname: { - knexConfig: { - debug: true, - }, - }, - }, - }, - }, - }), - ); - await testManager.forPlugin('testdbname').getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig] = mockCalls[0]; - - expect(baseConfig.data).toEqual( - expect.objectContaining({ - debug: true, - something: false, - }), - ); - }); - - it('sets the owner config for plugin using default config', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - database: 'foodb', - }, - role: 'backstage', - plugin: { - testowner: {}, - }, - }, - }, - }), - ); - await testManager.forPlugin('testowner').getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig] = mockCalls[0]; - - expect(baseConfig.data.role).toEqual('backstage'); - }); - - it('sets the owner config for plugin using plugin config', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - database: 'foodb', - }, - role: 'backstage', - plugin: { - testowner: { - role: 'backstage-plugin', - }, - }, - }, - }, - }), - ); - await testManager.forPlugin('testowner').getClient(); - - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig] = mockCalls[0]; - - expect(baseConfig.data.role).toEqual('backstage-plugin'); - }); - - it('Defaults the application_name for postgres clients', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: {}, - }, - }, - }), - ); - - await testManager.forPlugin('testplugin').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, _] = mockCalls[0]; - expect(baseConfig.get()).toMatchObject({ - connection: { - application_name: 'backstage_plugin_testplugin', - }, - }); - }); - - it('Allows manually setting the application_name for postgres clients', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - application_name: 'backstage_custom_app_name', - }, - }, - }, - }), - ); - - await testManager.forPlugin('testplugin').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, overrides] = mockCalls[0]; - expect(baseConfig.get().connection.application_name).toBe( - 'backstage_custom_app_name', - ); - expect(overrides.connection.application_name).toBeUndefined(); - }); - - it('Allows manually setting the application_name for individual plugin client', async () => { - const testManager = DatabaseManager.fromConfig( - new ConfigReader({ - backend: { - database: { - client: 'pg', - connection: { - host: 'localhost', - user: 'foo', - password: 'bar', - database: 'foodb', - application_name: 'backstage_custom_app_name', - }, - plugin: { - overrideplugin: { - connection: { - application_name: 'custom_plugin', - }, - }, - }, - }, - }, - }), - ); - - await testManager.forPlugin('overrideplugin').getClient(); - const mockCalls = mocked(createDatabaseClient).mock.calls.splice(-1); - const [baseConfig, _] = mockCalls[0]; - expect(baseConfig.get().connection.application_name).toBe( - 'custom_plugin', - ); - }); - }); - */ }); From ed83f855352307de4a67b878af97b6dbdb191d5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Mon, 29 Apr 2024 10:16:44 +0200 Subject: [PATCH 6/8] add changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/mean-ravens-dance.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/mean-ravens-dance.md diff --git a/.changeset/mean-ravens-dance.md b/.changeset/mean-ravens-dance.md new file mode 100644 index 0000000000..e409f57c79 --- /dev/null +++ b/.changeset/mean-ravens-dance.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Internal refactor of the database code From 8786b93ef22f96255fcf5215f19d266f6298c63a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 30 Apr 2024 09:31:44 +0200 Subject: [PATCH 7/8] add back ddl limiter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../backend-common/src/database/connectors/mysql.ts | 8 ++++++-- .../src/database/connectors/postgres.ts | 12 +++++++++--- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/packages/backend-common/src/database/connectors/mysql.ts b/packages/backend-common/src/database/connectors/mysql.ts index 25594d5021..1ad8b0b3be 100644 --- a/packages/backend-common/src/database/connectors/mysql.ts +++ b/packages/backend-common/src/database/connectors/mysql.ts @@ -23,11 +23,15 @@ import { InputError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; +import limiterFactory from 'p-limit'; import yn from 'yn'; import { Connector, DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; import { mergeDatabaseConfig } from './mergeDatabaseConfig'; +// Limits the number of concurrent DDL operations to 1 +const ddlLimiter = limiterFactory(1); + /** * Creates a knex mysql database connection * @@ -174,7 +178,7 @@ export async function ensureMysqlDatabaseExists( let lastErr: Error | undefined = undefined; for (let i = 0; i < 3; i++) { try { - return await ensureDatabase(database); + return await ddlLimiter(() => ensureDatabase(database)); } catch (err) { lastErr = err; } @@ -214,7 +218,7 @@ export async function dropMysqlDatabase( }; await Promise.all( databases.map(async database => { - return await dropDatabase(database); + return await ddlLimiter(() => dropDatabase(database)); }), ); } finally { diff --git a/packages/backend-common/src/database/connectors/postgres.ts b/packages/backend-common/src/database/connectors/postgres.ts index 39a1073a30..1ddc096e05 100644 --- a/packages/backend-common/src/database/connectors/postgres.ts +++ b/packages/backend-common/src/database/connectors/postgres.ts @@ -23,12 +23,16 @@ import { ForwardedError, InputError } from '@backstage/errors'; import { JsonObject } from '@backstage/types'; import knexFactory, { Knex } from 'knex'; import { merge, omit } from 'lodash'; +import limiterFactory from 'p-limit'; import { Client } from 'pg'; import { Connector, DatabaseConnector } from '../types'; import defaultNameOverride from './defaultNameOverride'; import defaultSchemaOverride from './defaultSchemaOverride'; import { mergeDatabaseConfig } from './mergeDatabaseConfig'; +// Limits the number of concurrent DDL operations to 1 +const ddlLimiter = limiterFactory(1); + /** * Creates a knex postgres database connection * @@ -159,7 +163,7 @@ export async function ensurePgDatabaseExists( let lastErr: Error | undefined = undefined; for (let i = 0; i < 3; i++) { try { - return await ensureDatabase(database); + return await ddlLimiter(() => ensureDatabase(database)); } catch (err) { lastErr = err; } @@ -198,7 +202,9 @@ export async function ensurePgSchemaExists( } }; - await Promise.all(schemas.map(ensureSchema)); + await Promise.all( + schemas.map(database => ddlLimiter(() => ensureSchema(database))), + ); } finally { await admin.destroy(); } @@ -218,7 +224,7 @@ export async function dropPgDatabase( try { await Promise.all( databases.map(async database => { - await admin.raw(`DROP DATABASE ??`, [database]); + await ddlLimiter(() => admin.raw(`DROP DATABASE ??`, [database])); }), ); } finally { From 1cd13b181b0ddde659a7d20c49626b9dd19d8386 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 30 Apr 2024 12:00:30 +0200 Subject: [PATCH 8/8] upgrade the changeset MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/mean-ravens-dance.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.changeset/mean-ravens-dance.md b/.changeset/mean-ravens-dance.md index e409f57c79..061e12cd9e 100644 --- a/.changeset/mean-ravens-dance.md +++ b/.changeset/mean-ravens-dance.md @@ -1,5 +1,7 @@ --- -'@backstage/backend-common': patch +'@backstage/backend-common': minor --- -Internal refactor of the database code +Internal refactor of the database code. + +**BREAKING**: The helper functions `createDatabaseClient` and `ensureDatabaseExists` have been removed from the public interface, since they have no usage within the repository and never were suitable for calling from the outside. Please consider using `coreServices.database` or `DatabaseManager` directly wherever possible instead.