From 66849e0677fd274be9e502b7a5a0bb9b86a7f0a3 Mon Sep 17 00:00:00 2001 From: David Festal Date: Mon, 28 Aug 2023 17:52:06 +0200 Subject: [PATCH] Fix review comments Signed-off-by: David Festal --- packages/backend-plugin-manager/api-report.md | 42 +++++++++---------- packages/backend-plugin-manager/config.d.ts | 2 - packages/backend-plugin-manager/package.json | 4 +- .../src/__testUtils__/testUtils.ts | 12 +++--- packages/backend-plugin-manager/src/index.ts | 7 ++-- .../src/loader/index.ts | 17 ++++++++ .../src/manager/index.ts | 38 +++++++++++++++++ .../src/manager/plugin-manager.test.ts | 14 +++---- .../src/manager/plugin-manager.ts | 6 +-- .../src/manager/types.ts | 23 ++++++---- .../src/scanner/index.ts | 17 ++++++++ .../scanner/plugin-scanner-watcher.test.ts | 38 ++++++++++------- .../src/scanner/plugin-scanner.ts | 6 ++- yarn.lock | 1 + 14 files changed, 158 insertions(+), 69 deletions(-) create mode 100644 packages/backend-plugin-manager/src/loader/index.ts create mode 100644 packages/backend-plugin-manager/src/manager/index.ts create mode 100644 packages/backend-plugin-manager/src/scanner/index.ts diff --git a/packages/backend-plugin-manager/api-report.md b/packages/backend-plugin-manager/api-report.md index e10f71ba15..026b1d0553 100644 --- a/packages/backend-plugin-manager/api-report.md +++ b/packages/backend-plugin-manager/api-report.md @@ -101,11 +101,11 @@ export function isBackendDynamicPluginInstaller( // @public @deprecated (undocumented) export interface LegacyBackendPluginInstaller { // (undocumented) - catalog?(builder: CatalogBuilder, env: PluginEnvironment): void; + catalog?(builder: CatalogBuilder, env: LegacyPluginEnvironment): void; // (undocumented) events?( eventsBackend: EventsBackend, - env: PluginEnvironment, + env: LegacyPluginEnvironment, ): HttpPostIngressOptions[]; // (undocumented) kind: 'legacy'; @@ -116,18 +116,34 @@ export interface LegacyBackendPluginInstaller { // (undocumented) router?: { pluginID: string; - createPlugin(env: PluginEnvironment): Promise; + createPlugin(env: LegacyPluginEnvironment): Promise; }; // (undocumented) - scaffolder?(env: PluginEnvironment): TemplateAction[]; + scaffolder?(env: LegacyPluginEnvironment): TemplateAction[]; // (undocumented) search?( indexBuilder: IndexBuilder, schedule: TaskRunner, - env: PluginEnvironment, + env: LegacyPluginEnvironment, ): void; } +// @public @deprecated (undocumented) +export type LegacyPluginEnvironment = { + logger: Logger; + cache: PluginCacheManager; + database: PluginDatabaseManager; + config: Config; + reader: UrlReader; + discovery: PluginEndpointDiscovery; + tokenManager: TokenManager; + permissions: PermissionEvaluator; + scheduler: PluginTaskScheduler; + identity: IdentityApi; + eventBroker: EventBroker; + pluginProvider: BackendPluginProvider; +}; + // @public (undocumented) export interface ModuleLoader { // (undocumented) @@ -146,22 +162,6 @@ export interface NewBackendPluginInstaller { kind: 'new'; } -// @public (undocumented) -export type PluginEnvironment = { - logger: Logger; - cache: PluginCacheManager; - database: PluginDatabaseManager; - config: Config; - reader: UrlReader; - discovery: PluginEndpointDiscovery; - tokenManager: TokenManager; - permissions: PermissionEvaluator; - scheduler: PluginTaskScheduler; - identity: IdentityApi; - eventBroker: EventBroker; - pluginProvider: BackendPluginProvider; -}; - // @public (undocumented) export class PluginManager implements BackendPluginProvider { // (undocumented) diff --git a/packages/backend-plugin-manager/config.d.ts b/packages/backend-plugin-manager/config.d.ts index edbc99ce89..9427458a23 100644 --- a/packages/backend-plugin-manager/config.d.ts +++ b/packages/backend-plugin-manager/config.d.ts @@ -15,11 +15,9 @@ */ export interface Config { - /** @visibility frontend */ dynamicPlugins?: { /** * The local path, relative to the backstage root, that contains dynamic plugins. - * @visibility frontend */ rootDirectory: string; }; diff --git a/packages/backend-plugin-manager/package.json b/packages/backend-plugin-manager/package.json index 12bfd179fb..de9fb8fa63 100644 --- a/packages/backend-plugin-manager/package.json +++ b/packages/backend-plugin-manager/package.json @@ -1,8 +1,7 @@ { "name": "@backstage/backend-plugin-manager", - "packageManager": "yarn@3.2.3", "description": "Backstage plugin management backend", - "version": "0.1.0", + "version": "0.0.0", "main": "src/index.ts", "types": "src/index.ts", "publishConfig": { @@ -47,6 +46,7 @@ "@types/express": "^4.17.6", "chokidar": "^3.5.3", "express": "^4.17.1", + "lodash": "^4.17.21", "winston": "^3.2.1" }, "devDependencies": { diff --git a/packages/backend-plugin-manager/src/__testUtils__/testUtils.ts b/packages/backend-plugin-manager/src/__testUtils__/testUtils.ts index 57bd9e4973..11a4421239 100644 --- a/packages/backend-plugin-manager/src/__testUtils__/testUtils.ts +++ b/packages/backend-plugin-manager/src/__testUtils__/testUtils.ts @@ -17,13 +17,13 @@ import { LoggerService } from '@backstage/backend-plugin-api'; import { JsonObject } from '@backstage/types'; -export type logContent = { message: string; meta?: Error | JsonObject }; +export type LogContent = { message: string; meta?: Error | JsonObject }; export type Logs = { - errors?: logContent[]; - warns?: logContent[]; - infos?: logContent[]; - debugs?: logContent[]; + errors?: LogContent[]; + warns?: LogContent[]; + infos?: LogContent[]; + debugs?: LogContent[]; }; export class MockedLogger implements LoggerService { @@ -59,7 +59,7 @@ export class MockedLogger implements LoggerService { } } -function toLogContent(message: string, meta?: Error | JsonObject): logContent { +function toLogContent(message: string, meta?: Error | JsonObject): LogContent { if (!meta) { return { message }; } diff --git a/packages/backend-plugin-manager/src/index.ts b/packages/backend-plugin-manager/src/index.ts index 997913454e..5570d8ee29 100644 --- a/packages/backend-plugin-manager/src/index.ts +++ b/packages/backend-plugin-manager/src/index.ts @@ -14,7 +14,6 @@ * limitations under the License. */ -export * from './scanner/types'; -export * from './loader/types'; -export * from './manager/types'; -export * from './manager/plugin-manager'; +export * from './loader'; +export * from './scanner'; +export * from './manager'; diff --git a/packages/backend-plugin-manager/src/loader/index.ts b/packages/backend-plugin-manager/src/loader/index.ts new file mode 100644 index 0000000000..ff0892e66d --- /dev/null +++ b/packages/backend-plugin-manager/src/loader/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 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. + */ + +export type { ModuleLoader } from './types'; diff --git a/packages/backend-plugin-manager/src/manager/index.ts b/packages/backend-plugin-manager/src/manager/index.ts new file mode 100644 index 0000000000..a9cd163617 --- /dev/null +++ b/packages/backend-plugin-manager/src/manager/index.ts @@ -0,0 +1,38 @@ +/* + * Copyright 2023 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. + */ + +export { isBackendDynamicPluginInstaller } from './types'; + +export type { + BaseDynamicPlugin, + DynamicPlugin, + FrontendDynamicPlugin, + BackendDynamicPlugin, + BackendDynamicPluginInstaller, + NewBackendPluginInstaller, + LegacyBackendPluginInstaller, + LegacyPluginEnvironment, + BackendPluginProvider, +} from './types'; + +export { + PluginManager, + dynamicPluginsFeatureDiscoveryServiceFactory, + dynamicPluginsServiceFactory, + dynamicPluginsServiceRef, +} from './plugin-manager'; + +export type { DynamicPluginsFactoryOptions } from './plugin-manager'; diff --git a/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts b/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts index 2d0966abbe..9d086092b9 100644 --- a/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts +++ b/packages/backend-plugin-manager/src/manager/plugin-manager.test.ts @@ -30,7 +30,7 @@ import { DynamicPlugin, LegacyBackendPluginInstaller, NewBackendPluginInstaller, - PluginEnvironment, + LegacyPluginEnvironment, } from './types'; import { ScannedPluginManifest, ScannedPluginPackage } from '../scanner/types'; import { randomUUID } from 'crypto'; @@ -40,7 +40,7 @@ import { rootLifecycleServiceFactory, } from '@backstage/backend-app-api'; import { ConfigSources } from '@backstage/config-loader'; -import { Logs, MockedLogger, logContent } from '../__testUtils__/testUtils'; +import { Logs, MockedLogger, LogContent } from '../__testUtils__/testUtils'; import { PluginScanner } from '../scanner/plugin-scanner'; import { findPaths } from '@backstage/cli-common'; @@ -59,10 +59,10 @@ describe('backend-plugin-manager', () => { content?: string; }; expectedLogs?(location: URL): { - errors?: logContent[]; - warns?: logContent[]; - infos?: logContent[]; - debugs?: logContent[]; + errors?: LogContent[]; + warns?: LogContent[]; + infos?: LogContent[]; + debugs?: LogContent[]; }; checkLoadedPlugins: (loadedPlugins: BaseDynamicPlugin[]) => void; }; @@ -327,7 +327,7 @@ describe('backend-plugin-manager', () => { ]); const installer = (plugins[0] as BackendDynamicPlugin) .installer as LegacyBackendPluginInstaller; - expect(installer.scaffolder!({} as PluginEnvironment)).toEqual< + expect(installer.scaffolder!({} as LegacyPluginEnvironment)).toEqual< TemplateAction[] >([]); }, diff --git a/packages/backend-plugin-manager/src/manager/plugin-manager.ts b/packages/backend-plugin-manager/src/manager/plugin-manager.ts index b4cfaeeb64..89ea144426 100644 --- a/packages/backend-plugin-manager/src/manager/plugin-manager.ts +++ b/packages/backend-plugin-manager/src/manager/plugin-manager.ts @@ -20,9 +20,11 @@ import { isBackendDynamicPluginInstaller, DynamicPlugin, } from './types'; +import { ScannedPluginPackage } from '../scanner'; import { PluginScanner } from '../scanner/plugin-scanner'; +import { ModuleLoader } from '../loader'; +import { CommonJSModuleLoader } from '../loader/CommonJSModuleLoader'; import * as url from 'url'; -import { ScannedPluginPackage } from '../scanner/types'; import { BackendFeature, LoggerService, @@ -34,8 +36,6 @@ import { PackageRoles } from '@backstage/cli-node'; import { findPaths } from '@backstage/cli-common'; import path from 'path'; import * as fs from 'fs'; -import { ModuleLoader } from '../loader/types'; -import { CommonJSModuleLoader } from '../loader/CommonJSModuleLoader'; import { FeatureDiscoveryService, featureDiscoveryServiceRef, diff --git a/packages/backend-plugin-manager/src/manager/types.ts b/packages/backend-plugin-manager/src/manager/types.ts index 8c220b7699..201941292d 100644 --- a/packages/backend-plugin-manager/src/manager/types.ts +++ b/packages/backend-plugin-manager/src/manager/types.ts @@ -42,8 +42,17 @@ import { PermissionPolicy } from '@backstage/plugin-permission-node'; /** * @public + * + * @deprecated + * + * Support for the legacy backend system will be removed in the future. + * + * When adding a legacy plugin installer entrypoint in your plugin, + * you should always take the opportunity to also implement support + * for the new backend system if not already done. + * */ -export type PluginEnvironment = { +export type LegacyPluginEnvironment = { logger: Logger; cache: PluginCacheManager; database: PluginDatabaseManager; @@ -115,7 +124,7 @@ export interface NewBackendPluginInstaller { * @public * @deprecated * - * Support for the legacy backend suystem will be removed in the future. + * Support for the legacy backend system will be removed in the future. * * When adding a legacy plugin installer entrypoint in your plugin, * you should always take the opportunity to also implement support @@ -127,19 +136,19 @@ export interface LegacyBackendPluginInstaller { router?: { pluginID: string; - createPlugin(env: PluginEnvironment): Promise; + createPlugin(env: LegacyPluginEnvironment): Promise; }; - catalog?(builder: CatalogBuilder, env: PluginEnvironment): void; - scaffolder?(env: PluginEnvironment): TemplateAction[]; + catalog?(builder: CatalogBuilder, env: LegacyPluginEnvironment): void; + scaffolder?(env: LegacyPluginEnvironment): TemplateAction[]; search?( indexBuilder: IndexBuilder, schedule: TaskRunner, - env: PluginEnvironment, + env: LegacyPluginEnvironment, ): void; events?( eventsBackend: EventsBackend, - env: PluginEnvironment, + env: LegacyPluginEnvironment, ): HttpPostIngressOptions[]; permissions?: { policy?: PermissionPolicy; diff --git a/packages/backend-plugin-manager/src/scanner/index.ts b/packages/backend-plugin-manager/src/scanner/index.ts new file mode 100644 index 0000000000..c5574cc38e --- /dev/null +++ b/packages/backend-plugin-manager/src/scanner/index.ts @@ -0,0 +1,17 @@ +/* + * Copyright 2023 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. + */ + +export type { ScannedPluginManifest, ScannedPluginPackage } from './types'; diff --git a/packages/backend-plugin-manager/src/scanner/plugin-scanner-watcher.test.ts b/packages/backend-plugin-manager/src/scanner/plugin-scanner-watcher.test.ts index 2644024cd7..5ced3cfbab 100644 --- a/packages/backend-plugin-manager/src/scanner/plugin-scanner-watcher.test.ts +++ b/packages/backend-plugin-manager/src/scanner/plugin-scanner-watcher.test.ts @@ -19,7 +19,8 @@ import { Logs, MockedLogger } from '../__testUtils__/testUtils'; import { Config, ConfigReader } from '@backstage/config'; import path, { join } from 'path'; import { ScannedPluginPackage } from './types'; -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'; +import { mkdtempSync, rmSync } from 'fs'; +import { mkdir, writeFile, rm } from 'fs/promises'; import { tmpdir } from 'os'; import waitForExpect from 'wait-for-expect'; @@ -33,15 +34,20 @@ describe('plugin-scanner', () => { }); afterEach(() => { if (backstageRootDirectory) { - rmSync(backstageRootDirectory, { recursive: true }); + rmSync(backstageRootDirectory, { recursive: true, force: true }); + } + }); + process.on('beforeExit', () => { + if (backstageRootDirectory) { + rmSync(backstageRootDirectory, { recursive: true, force: true }); } }); it('watch for config and file system changes', async () => { const logger = new MockedLogger(); - mkdirSync(join(backstageRootDirectory, 'first-dir')); - mkdirSync(join(backstageRootDirectory, 'second-dir')); + await mkdir(join(backstageRootDirectory, 'first-dir')); + await mkdir(join(backstageRootDirectory, 'second-dir')); const config: Config = new ConfigReader({ dynamicPlugins: { @@ -77,10 +83,10 @@ describe('plugin-scanner', () => { }); pluginScanner.subscribeToRootDirectoryChange(rootDirectorySubscriber); - mkdirSync( + await mkdir( join(backstageRootDirectory, 'first-dir', 'test-backend-plugin'), ); - writeFileSync( + await writeFile( join( backstageRootDirectory, 'first-dir', @@ -96,7 +102,7 @@ describe('plugin-scanner', () => { ); await waitForExpect(() => { - expect(rootDirectorySubscriber).toHaveBeenCalledTimes(2); + expect(rootDirectorySubscriber).toHaveBeenCalledTimes(1); }); rootDirectorySubscriber.mockClear(); await waitForExpect(() => { @@ -151,14 +157,14 @@ describe('plugin-scanner', () => { }); logger.logs = {}; - mkdirSync( + await mkdir( join( backstageRootDirectory, 'second-dir', 'second-test-backend-plugin', ), ); - writeFileSync( + await writeFile( join( backstageRootDirectory, 'second-dir', @@ -174,7 +180,7 @@ describe('plugin-scanner', () => { ); await waitForExpect(() => { - expect(rootDirectorySubscriber).toHaveBeenCalledTimes(2); + expect(rootDirectorySubscriber).toHaveBeenCalledTimes(1); }); rootDirectorySubscriber.mockClear(); await waitForExpect(() => { @@ -210,7 +216,7 @@ describe('plugin-scanner', () => { const debug = jest.spyOn(logger, 'debug'); // Check that not all file changes trigger a new scan of plugins - mkdirSync( + await mkdir( join( backstageRootDirectory, 'second-dir', @@ -221,7 +227,7 @@ describe('plugin-scanner', () => { await waitForExpect(() => { expect(debug).toHaveBeenCalledTimes(1); }); - writeFileSync( + await writeFile( join( backstageRootDirectory, 'second-dir', @@ -233,7 +239,7 @@ describe('plugin-scanner', () => { await waitForExpect(() => { expect(debug).toHaveBeenCalledTimes(2); }); - rmSync( + await rm( join( backstageRootDirectory, 'second-dir', @@ -244,7 +250,7 @@ describe('plugin-scanner', () => { await waitForExpect(() => { expect(debug).toHaveBeenCalledTimes(3); }); - rmSync( + await rm( join( backstageRootDirectory, 'second-dir', @@ -277,7 +283,7 @@ describe('plugin-scanner', () => { // Now check that removal of some plugin home directory triggers a new scan of plugins - rmSync( + await rm( join( backstageRootDirectory, 'second-dir', @@ -315,7 +321,7 @@ describe('plugin-scanner', () => { }); logger.logs = {}; - rmSync( + await rm( join( backstageRootDirectory, 'second-dir', diff --git a/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts b/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts index 201f8cae8e..39d55bdb5c 100644 --- a/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts +++ b/packages/backend-plugin-manager/src/scanner/plugin-scanner.ts @@ -20,6 +20,7 @@ import { Stats, lstatSync } from 'fs'; import * as chokidar from 'chokidar'; import * as path from 'path'; import * as url from 'url'; +import debounce from 'lodash/debounce'; import { PackagePlatform, PackageRoles } from '@backstage/cli-node'; import { LoggerService } from '@backstage/backend-plugin-api'; @@ -210,6 +211,9 @@ export class PluginScanner { resolve(); return; } + const callSubscribers = debounce(() => { + this.subscribers.forEach(s => s()); + }, 500); let ready = false; this.rootDirectoryWatcher = chokidar .watch(this._rootDirectory, { @@ -234,7 +238,7 @@ export class PluginScanner { this.logger.info( `rootDirectory changed (${event} - ${eventPath}): scanning plugins again`, ); - this.subscribers.forEach(s => s()); + callSubscribers(); } else { this.logger.debug( `rootDirectory changed (${event} - ${eventPath}): no need to scan plugins again`, diff --git a/yarn.lock b/yarn.lock index a2ca6a4f76..6d6bfca66f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3498,6 +3498,7 @@ __metadata: "@types/express": ^4.17.6 chokidar: ^3.5.3 express: ^4.17.1 + lodash: ^4.17.21 mock-fs: ^5.2.0 wait-for-expect: ^3.0.2 winston: ^3.2.1