Fix review comments

Signed-off-by: David Festal <dfestal@redhat.com>

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<Router>;
+    createPlugin(env: LegacyPluginEnvironment): Promise<Router>;
   };
   // (undocumented)
-  scaffolder?(env: PluginEnvironment): TemplateAction<any>[];
+  scaffolder?(env: LegacyPluginEnvironment): TemplateAction<any>[];
   // (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)

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;
   };

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": {

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 };
   }

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';

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';

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';

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<any>[]
           >([]);
         },

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,

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<Router>;
+    createPlugin(env: LegacyPluginEnvironment): Promise<Router>;
   };
 
-  catalog?(builder: CatalogBuilder, env: PluginEnvironment): void;
-  scaffolder?(env: PluginEnvironment): TemplateAction<any>[];
+  catalog?(builder: CatalogBuilder, env: LegacyPluginEnvironment): void;
+  scaffolder?(env: LegacyPluginEnvironment): TemplateAction<any>[];
   search?(
     indexBuilder: IndexBuilder,
     schedule: TaskRunner,
-    env: PluginEnvironment,
+    env: LegacyPluginEnvironment,
   ): void;
   events?(
     eventsBackend: EventsBackend,
-    env: PluginEnvironment,
+    env: LegacyPluginEnvironment,
   ): HttpPostIngressOptions[];
   permissions?: {
     policy?: PermissionPolicy;

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';

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',

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`,

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