Merge pull request #26386 from backstage/freben/cleanup-database-defaults

Clean up the backend-defaults database report
This commit is contained in:
Fredrik Adelöw
2024-08-30 15:49:01 +02:00
committed by GitHub
14 changed files with 127 additions and 217 deletions
+7
View File
@@ -0,0 +1,7 @@
---
'@backstage/backend-common': minor
---
**BREAKING**: Simplifications and cleanup as part of the Backend System 1.0 work.
- The deprecated `dropDatabase` function has now been removed, without replacement.
+10
View File
@@ -0,0 +1,10 @@
---
'@backstage/backend-defaults': minor
---
**BREAKING**: Simplifications and cleanup as part of the Backend System 1.0 work.
- The deprecated `dropDatabase` function has now been removed, without replacement.
- The deprecated `LegacyRootDatabaseService` type has now been removed.
- The return type from `DatabaseManager.forPlugin` is now directly a `DatabaseService`, as arguably expected.
- `DatabaseManager.forPlugin` now requires the `deps` argument, with the logger and lifecycle services.
+7 -9
View File
@@ -157,7 +157,10 @@ export class DatabaseManager implements LegacyRootDatabaseService {
// (undocumented)
static fromConfig(
config: Config,
options?: DatabaseManagerOptions,
options?: {
migrations?: DatabaseService['migrations'];
logger?: LoggerService;
},
): DatabaseManager;
}
@@ -173,11 +176,6 @@ export class DockerContainerRunner implements ContainerRunner {
runContainer(options: RunContainerOptions): Promise<void>;
}
// Warning: (ae-forgotten-export) The symbol "dropDatabase_2" needs to be exported by the entry point index.d.ts
//
// @public @deprecated (undocumented)
export const dropDatabase: typeof dropDatabase_2;
// @public @deprecated
export function errorHandler(
options?: ErrorHandlerOptions,
@@ -367,10 +365,10 @@ export const legacyPlugin: (
}>,
) => BackendFeature;
// Warning: (ae-forgotten-export) The symbol "LegacyRootDatabaseService_2" needs to be exported by the entry point index.d.ts
//
// @public @deprecated (undocumented)
export type LegacyRootDatabaseService = LegacyRootDatabaseService_2;
export type LegacyRootDatabaseService = {
forPlugin(pluginId: string): DatabaseService;
};
// @public @deprecated
export function loadBackendConfig(options: {
+29 -14
View File
@@ -30,10 +30,8 @@ import {
// eslint-disable-next-line @backstage/no-relative-monorepo-imports
import {
dropDatabase as _dropDatabase,
DatabaseManager as _DatabaseManager,
type DatabaseManagerOptions as _DatabaseManagerOptions,
type LegacyRootDatabaseService as _LegacyRootDatabaseService,
} from '../../../backend-defaults/src/entrypoints/database/DatabaseManager';
import {
@@ -48,6 +46,8 @@ import {
isChildPath as _isChildPath,
LifecycleService,
PluginMetadataService,
DatabaseService,
LoggerService,
} from '@backstage/backend-plugin-api';
export * from './hot';
@@ -170,14 +170,20 @@ export type CacheClientOptions = CacheServiceOptions;
* @deprecated Use `DatabaseManager` from the `@backstage/backend-defaults` package instead
*/
export class DatabaseManager implements LegacyRootDatabaseService {
private constructor(private readonly _databaseManager: _DatabaseManager) {}
private constructor(
private readonly _databaseManager: _DatabaseManager,
private readonly logger?: LoggerService,
) {}
static fromConfig(
config: Config,
options?: DatabaseManagerOptions,
options?: {
migrations?: DatabaseService['migrations'];
logger?: LoggerService;
},
): DatabaseManager {
const _databaseManager = _DatabaseManager.fromConfig(config, options);
return new DatabaseManager(_databaseManager);
return new DatabaseManager(_databaseManager, options?.logger);
}
forPlugin(
@@ -186,7 +192,20 @@ export class DatabaseManager implements LegacyRootDatabaseService {
| { lifecycle: LifecycleService; pluginMetadata: PluginMetadataService }
| undefined,
): PluginDatabaseManager {
return this._databaseManager.forPlugin(pluginId, deps);
const logger: LoggerService = this.logger ?? {
debug() {},
info() {},
warn() {},
error() {},
child() {
return this;
},
};
const lifecycle: LifecycleService = deps?.lifecycle ?? {
addShutdownHook() {},
addStartupHook() {},
};
return this._databaseManager.forPlugin(pluginId, { logger, lifecycle });
}
}
@@ -204,15 +223,11 @@ export type PluginDatabaseManager = _PluginDatabaseManager;
/**
* @public
* @deprecated Use `LegacyRootDatabaseService` from the `@backstage/backend-defaults` package instead
* @deprecated Use `DatabaseManager` from `@backstage/backend-defaults/database` instead, or migrate to the new backend system and use `coreServices.database`
*/
export type LegacyRootDatabaseService = _LegacyRootDatabaseService;
/**
* @public
* @deprecated Use `dropDatabase` from the `@backstage/backend-defaults` package instead
*/
export const dropDatabase = _dropDatabase;
export type LegacyRootDatabaseService = {
forPlugin(pluginId: string): DatabaseService;
};
/**
* @public
@@ -3,21 +3,19 @@
> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/).
```ts
import { Config } from '@backstage/config';
import { DatabaseService } from '@backstage/backend-plugin-api';
import { LifecycleService } from '@backstage/backend-plugin-api';
import { LoggerService } from '@backstage/backend-plugin-api';
import { PluginMetadataService } from '@backstage/backend-plugin-api';
import { RootConfigService } from '@backstage/backend-plugin-api';
import { ServiceFactory } from '@backstage/backend-plugin-api';
// @public
export class DatabaseManager implements LegacyRootDatabaseService {
export class DatabaseManager {
forPlugin(
pluginId: string,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): DatabaseService;
static fromConfig(
@@ -29,7 +27,6 @@ export class DatabaseManager implements LegacyRootDatabaseService {
// @public
export type DatabaseManagerOptions = {
migrations?: DatabaseService['migrations'];
logger?: LoggerService;
};
// @public
@@ -39,16 +36,5 @@ export const databaseServiceFactory: ServiceFactory<
'singleton'
>;
// @public @deprecated
export function dropDatabase(
dbConfig: Config,
...databaseNames: string[]
): Promise<void>;
// @public
export type LegacyRootDatabaseService = {
forPlugin(pluginId: string): DatabaseService;
};
// (No @packageDocumentation comment for this package)
```
@@ -17,20 +17,24 @@
import { ConfigReader } from '@backstage/config';
import { DatabaseManagerImpl } from './DatabaseManager';
import { Connector } from './types';
import { mockServices } from '@backstage/backend-test-utils';
describe('DatabaseManagerImpl', () => {
afterEach(() => {
jest.clearAllMocks();
});
const deps = {
logger: mockServices.logger.mock(),
lifecycle: mockServices.lifecycle.mock(),
};
it('calls the right connector, only once per plugin id', async () => {
const connector1 = {
getClient: jest.fn(),
dropDatabase: jest.fn(),
} satisfies Connector;
const connector2 = {
getClient: jest.fn(),
dropDatabase: jest.fn(),
} satisfies Connector;
const impl = new DatabaseManagerImpl(
@@ -43,30 +47,28 @@ describe('DatabaseManagerImpl', () => {
},
);
await impl.forPlugin('plugin1').getClient();
await impl.forPlugin('plugin1', deps).getClient();
expect(connector1.getClient).toHaveBeenCalledTimes(1);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
expect(connector2.getClient).toHaveBeenCalledTimes(0);
await impl.forPlugin('plugin1').getClient();
await impl.forPlugin('plugin1', deps).getClient();
expect(connector1.getClient).toHaveBeenCalledTimes(1);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
expect(connector2.getClient).toHaveBeenCalledTimes(0);
await impl.forPlugin('plugin2').getClient();
await impl.forPlugin('plugin2', deps).getClient();
expect(connector1.getClient).toHaveBeenCalledTimes(2);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin2', undefined);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin2', deps);
expect(connector2.getClient).toHaveBeenCalledTimes(0);
});
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(
@@ -84,22 +86,21 @@ describe('DatabaseManagerImpl', () => {
},
);
await impl.forPlugin('plugin1').getClient();
await impl.forPlugin('plugin1', deps).getClient();
expect(connector1.getClient).toHaveBeenCalledTimes(1);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
expect(connector2.getClient).toHaveBeenCalledTimes(0);
await impl.forPlugin('plugin2').getClient();
await impl.forPlugin('plugin2', deps).getClient();
expect(connector1.getClient).toHaveBeenCalledTimes(1);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', undefined);
expect(connector1.getClient).toHaveBeenLastCalledWith('plugin1', deps);
expect(connector2.getClient).toHaveBeenCalledTimes(1);
expect(connector2.getClient).toHaveBeenLastCalledWith('plugin2', undefined);
expect(connector2.getClient).toHaveBeenLastCalledWith('plugin2', deps);
});
it('migration skip options take precedence over config', async () => {
const connector = {
getClient: jest.fn(),
dropDatabase: jest.fn(),
} satisfies Connector;
const impl = new DatabaseManagerImpl(
@@ -117,7 +118,7 @@ describe('DatabaseManagerImpl', () => {
},
{ migrations: { skip: false } },
);
expect((await impl.forPlugin('plugin1')).migrations).toEqual({
expect((await impl.forPlugin('plugin1', deps)).migrations).toEqual({
skip: false,
});
@@ -125,7 +126,7 @@ describe('DatabaseManagerImpl', () => {
pg: connector,
});
expect((await impl1.forPlugin('plugin1')).migrations).toEqual({
expect((await impl1.forPlugin('plugin1', deps)).migrations).toEqual({
skip: false,
});
});
@@ -133,7 +134,6 @@ describe('DatabaseManagerImpl', () => {
it('plugin can skip migrations using config', async () => {
const connector = {
getClient: jest.fn(),
dropDatabase: jest.fn(),
} satisfies Connector;
const impl = new DatabaseManagerImpl(
@@ -148,10 +148,10 @@ describe('DatabaseManagerImpl', () => {
},
);
expect((await impl.forPlugin('plugin1')).migrations).toEqual({
expect((await impl.forPlugin('plugin1', deps)).migrations).toEqual({
skip: true,
});
expect((await impl.forPlugin('plugin2')).migrations).toEqual({
expect((await impl.forPlugin('plugin2', deps)).migrations).toEqual({
skip: false,
});
@@ -169,10 +169,10 @@ describe('DatabaseManagerImpl', () => {
pg: connector,
},
);
expect((await impl2.forPlugin('plugin1')).migrations).toEqual({
expect((await impl2.forPlugin('plugin1', deps)).migrations).toEqual({
skip: false,
});
expect((await impl2.forPlugin('plugin2')).migrations).toEqual({
expect((await impl2.forPlugin('plugin2', deps)).migrations).toEqual({
skip: true,
});
});
@@ -18,7 +18,6 @@ import {
DatabaseService,
LifecycleService,
LoggerService,
PluginMetadataService,
RootConfigService,
} from '@backstage/backend-plugin-api';
import { Config } from '@backstage/config';
@@ -43,21 +42,12 @@ function pluginPath(pluginId: string): string {
*/
export type DatabaseManagerOptions = {
migrations?: DatabaseService['migrations'];
logger?: LoggerService;
};
/**
* An interface that represents the legacy global DatabaseManager implementation.
* @public
*/
export type LegacyRootDatabaseService = {
forPlugin(pluginId: string): DatabaseService;
};
/**
* Testable implementation class for {@link DatabaseManager} below.
*/
export class DatabaseManagerImpl implements LegacyRootDatabaseService {
export class DatabaseManagerImpl {
constructor(
private readonly config: Config,
private readonly connectors: Record<string, Connector>,
@@ -74,9 +64,9 @@ export class DatabaseManagerImpl implements LegacyRootDatabaseService {
*/
forPlugin(
pluginId: string,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): PluginDatabaseManager {
const client = this.getClientType(pluginId).client;
@@ -136,9 +126,9 @@ export class DatabaseManagerImpl implements LegacyRootDatabaseService {
private async getDatabase(
pluginId: string,
connector: Connector,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): Promise<Knex> {
if (this.databaseCache.has(pluginId)) {
@@ -149,13 +139,19 @@ export class DatabaseManagerImpl implements LegacyRootDatabaseService {
this.databaseCache.set(pluginId, clientPromise);
if (process.env.NODE_ENV !== 'test') {
clientPromise.then(client => this.startKeepaliveLoop(pluginId, client));
clientPromise.then(client =>
this.startKeepaliveLoop(pluginId, client, deps.logger),
);
}
return clientPromise;
}
private startKeepaliveLoop(pluginId: string, client: Knex): void {
private startKeepaliveLoop(
pluginId: string,
client: Knex,
logger: LoggerService,
): void {
let lastKeepaliveFailed = false;
setInterval(() => {
@@ -168,7 +164,7 @@ export class DatabaseManagerImpl implements LegacyRootDatabaseService {
(error: unknown) => {
if (!lastKeepaliveFailed) {
lastKeepaliveFailed = true;
this.options?.logger?.warn(
logger.warn(
`Database keepalive failed for plugin ${pluginId}, ${stringifyError(
error,
)}`,
@@ -193,7 +189,7 @@ export class DatabaseManagerImpl implements LegacyRootDatabaseService {
* set `prefix` which is used to prefix generated database names if config is
* not provided.
*/
export class DatabaseManager implements LegacyRootDatabaseService {
export class DatabaseManager {
/**
* Creates a {@link DatabaseManager} from `backend.database` config.
*
@@ -233,31 +229,11 @@ export class DatabaseManager implements LegacyRootDatabaseService {
*/
forPlugin(
pluginId: string,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): PluginDatabaseManager {
return this.impl.forPlugin(pluginId, deps);
}
}
/**
* Helper for deleting databases.
*
* @public
* @deprecated Will be removed in a future release.
*/
export async function dropDatabase(
dbConfig: Config,
...databaseNames: string[]
): Promise<void> {
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);
}
}
@@ -14,10 +14,7 @@
* limitations under the License.
*/
import {
LifecycleService,
PluginMetadataService,
} from '@backstage/backend-plugin-api';
import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
import { Config, ConfigReader } from '@backstage/config';
import { InputError } from '@backstage/errors';
import { JsonObject } from '@backstage/types';
@@ -253,9 +250,9 @@ export class MysqlConnector implements Connector {
async getClient(
pluginId: string,
_deps?: {
_deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): Promise<Knex> {
const pluginConfig = new ConfigReader(
@@ -293,10 +290,6 @@ export class MysqlConnector implements Connector {
return client;
}
async dropDatabase(...databaseNames: string[]): Promise<void> {
return await dropMysqlDatabase(this.config, ...databaseNames);
}
/**
* Provides the canonical database name for a given plugin.
*
@@ -14,10 +14,7 @@
* limitations under the License.
*/
import {
LifecycleService,
PluginMetadataService,
} from '@backstage/backend-plugin-api';
import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
import { Config, ConfigReader } from '@backstage/config';
import { ForwardedError } from '@backstage/errors';
import { JsonObject } from '@backstage/types';
@@ -261,9 +258,9 @@ export class PgConnector implements Connector {
async getClient(
pluginId: string,
_deps?: {
_deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): Promise<Knex> {
const pluginConfig = new ConfigReader(
@@ -312,10 +309,6 @@ export class PgConnector implements Connector {
return client;
}
async dropDatabase(...databaseNames: string[]): Promise<void> {
return await dropPgDatabase(this.config, ...databaseNames);
}
/**
* Provides the canonical database name for a given plugin.
*
@@ -20,11 +20,17 @@ import {
buildSqliteDatabaseConfig,
createSqliteDatabaseClient,
} from './sqlite3';
import { mockServices } from '@backstage/backend-test-utils';
describe('better-sqlite3', () => {
const createConfig = (connection: any) =>
new ConfigReader({ client: 'better-sqlite3', connection });
const deps = {
logger: mockServices.logger.mock(),
lifecycle: mockServices.lifecycle.mock(),
};
describe('buildSqliteDatabaseConfig', () => {
it('builds an in-memory connection', () => {
expect(buildSqliteDatabaseConfig(createConfig(':memory:'))).toEqual({
@@ -90,7 +96,9 @@ describe('better-sqlite3', () => {
describe('createSqliteDatabaseClient', () => {
it('creates an in memory knex instance', () => {
expect(createSqliteDatabaseClient(createConfig(':memory:'))).toBeTruthy();
expect(
createSqliteDatabaseClient('p', createConfig(':memory:'), deps),
).toBeTruthy();
});
});
});
@@ -15,10 +15,7 @@
*/
import { DevDataStore } from '@backstage/backend-dev-utils';
import {
LifecycleService,
PluginMetadataService,
} from '@backstage/backend-plugin-api';
import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
import { Config, ConfigReader } from '@backstage/config';
import { JsonObject } from '@backstage/types';
import { ensureDirSync } from 'fs-extra';
@@ -35,12 +32,13 @@ import { mergeDatabaseConfig } from './mergeDatabaseConfig';
* @param overrides - Additional options to merge with the config
*/
export function createSqliteDatabaseClient(
pluginId: string,
dbConfig: Config,
overrides?: Knex.Config,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
overrides?: Knex.Config,
) {
const knexConfig = buildSqliteDatabaseConfig(dbConfig, overrides);
const connConfig = knexConfig.connection as Knex.Sqlite3ConnectionConfig;
@@ -62,7 +60,7 @@ export function createSqliteDatabaseClient(
const devStore = DevDataStore.get();
if (devStore) {
const dataKey = `sqlite3-db-${deps.pluginMetadata.getId()}`;
const dataKey = `sqlite3-db-${pluginId}`;
const connectionLoader = async () => {
// If seed data is available, use it tconnectionLoader restore the database
@@ -180,9 +178,9 @@ export class Sqlite3Connector implements Connector {
async getClient(
pluginId: string,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): Promise<Knex> {
const pluginConfig = new ConfigReader(
@@ -202,18 +200,15 @@ export class Sqlite3Connector implements Connector {
);
const client = createSqliteDatabaseClient(
pluginId,
pluginConfig,
databaseClientOverrides,
deps,
databaseClientOverrides,
);
return client;
}
async dropDatabase(..._databaseNames: string[]): Promise<void> {
// do nothing
}
/**
* Provides the canonical database name for a given plugin.
*
@@ -35,6 +35,7 @@ export const databaseServiceFactory = createServiceFactory({
deps: {
config: coreServices.rootConfig,
lifecycle: coreServices.lifecycle,
logger: coreServices.logger,
pluginMetadata: coreServices.pluginMetadata,
},
async createRootContext({ config }) {
@@ -48,10 +49,10 @@ export const databaseServiceFactory = createServiceFactory({
}),
);
},
async factory({ pluginMetadata, lifecycle }, databaseManager) {
async factory({ pluginMetadata, lifecycle, logger }, databaseManager) {
return databaseManager.forPlugin(pluginMetadata.getId(), {
pluginMetadata,
lifecycle,
logger,
});
},
});
@@ -18,6 +18,4 @@ export { databaseServiceFactory } from './databaseServiceFactory';
export {
DatabaseManager,
type DatabaseManagerOptions,
type LegacyRootDatabaseService,
dropDatabase,
} from './DatabaseManager';
@@ -14,87 +14,17 @@
* limitations under the License.
*/
import {
LifecycleService,
PluginMetadataService,
} from '@backstage/backend-plugin-api';
import { Config } from '@backstage/config';
import { LifecycleService, LoggerService } from '@backstage/backend-plugin-api';
import { Knex } from 'knex';
export type { DatabaseService as PluginDatabaseManager } from '@backstage/backend-plugin-api';
/**
* Manages an underlying Knex database driver.
*/
export interface DatabaseConnector {
/**
* Provides an instance of a knex database connector.
*/
createClient(
dbConfig: Config,
overrides?: Partial<Knex.Config>,
deps?: {
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): Knex;
/**
* Provides a partial knex config sufficient to override a database name.
*/
createNameOverride(name: string): Partial<Knex.Config>;
/**
* Provides a partial knex config sufficient to override a PostgreSQL schema
* name within utilizing the `searchPath` knex configuration.
*/
createSchemaOverride?(name: string): Partial<Knex.Config>;
/**
* Produces a knex connection config object representing a database connection
* string.
*/
parseConnectionString(
connectionString: string,
client?: string,
): Knex.StaticConnectionConfig;
/**
* Performs a side-effect to ensure database names passed in are present.
*
* Calling this function on databases which already exist should do nothing.
* Missing databases should be created if needed.
*/
ensureDatabaseExists?(
dbConfig: Config,
...databases: Array<string>
): Promise<void>;
/**
* Performs a side-effect to ensure schema names passed in are present.
*
* Calling this function on schemas which already exist should do nothing.
* Missing schemas should be created if needed.
*/
ensureSchemaExists?(
dbConfig: Config,
...schemas: Array<string>
): Promise<void>;
/**
* Deletes databases.
*/
dropDatabase?(dbConfig: Config, ...databases: Array<string>): Promise<void>;
}
export interface Connector {
getClient(
pluginId: string,
deps?: {
deps: {
logger: LoggerService;
lifecycle: LifecycleService;
pluginMetadata: PluginMetadataService;
},
): Promise<Knex>;
dropDatabase(...databaseNames: string[]): Promise<void>;
}