From cb7fc410ed99055977d986f5ef11f20627a0e662 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Wed, 6 Sep 2023 16:05:43 +0200 Subject: [PATCH] backend-app-api: make feature discovery only discovery default exports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: Camila Belo Co-authored-by: Johan Haals Co-authored-by: Philipp Hugenroth Signed-off-by: Patrik Oldsberg --- .changeset/ninety-horses-remember.md | 5 + packages/backend-app-api/api-report.md | 9 +- .../featureDiscoveryServiceFactory.test.ts | 167 +++++++++--------- .../alpha/featureDiscoveryServiceFactory.ts | 25 ++- .../src/wiring/BackendInitializer.ts | 4 +- .../src/wiring/BackstageBackend.ts | 2 +- 6 files changed, 108 insertions(+), 104 deletions(-) create mode 100644 .changeset/ninety-horses-remember.md diff --git a/.changeset/ninety-horses-remember.md b/.changeset/ninety-horses-remember.md new file mode 100644 index 0000000000..22ef53b340 --- /dev/null +++ b/.changeset/ninety-horses-remember.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +The experimental backend feature discovery now only considers default exports from packages. It no longer filters packages to include based on the package role, except that `'cli'` packages are ignored. However, the `"backstage"` field is still required in `package.json`. diff --git a/packages/backend-app-api/api-report.md b/packages/backend-app-api/api-report.md index 0fa1ff376e..3bddcc48d5 100644 --- a/packages/backend-app-api/api-report.md +++ b/packages/backend-app-api/api-report.md @@ -43,7 +43,14 @@ import { UrlReader } from '@backstage/backend-common'; // @public (undocumented) export interface Backend { // (undocumented) - add(feature: BackendFeature | (() => BackendFeature)): void; + add( + feature: + | BackendFeature + | (() => BackendFeature) + | Promise<{ + default: BackendFeature | (() => BackendFeature); + }>, + ): void; // (undocumented) start(): Promise; // (undocumented) diff --git a/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.test.ts b/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.test.ts index f054c1a365..448f1a8ff7 100644 --- a/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.test.ts +++ b/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.test.ts @@ -18,10 +18,6 @@ import mockFs from 'mock-fs'; import { resolve as resolvePath, dirname } from 'path'; import { startTestBackend, mockServices } from '@backstage/backend-test-utils'; import { featureDiscoveryServiceFactory } from './featureDiscoveryServiceFactory'; -import { - coreServices, - createServiceFactory, -} from '@backstage/backend-plugin-api'; const rootDir = dirname(process.argv[1]); @@ -35,6 +31,7 @@ describe('featureDiscoveryServiceFactory', () => { 'detected-plugin': '0.0.0', 'detected-module': '0.0.0', 'detected-plugin-with-alpha': '0.0.0', + 'detected-library': '0.0.0', }, }), }, @@ -48,13 +45,13 @@ describe('featureDiscoveryServiceFactory', () => { }), 'index.js': ` const { createBackendPlugin, coreServices } = require('@backstage/backend-plugin-api'); - exports.detectedPlugin = createBackendPlugin({ + exports.default = createBackendPlugin({ pluginId: 'detected', register(env) { env.registerInit({ - deps: { identity: coreServices.identity }, - async init({ identity }) { - identity.getIdentity('detected-plugin'); + deps: { logger: coreServices.rootLogger }, + async init({ logger }) { + logger.warn('detected-plugin'); }, }); }, @@ -71,14 +68,14 @@ describe('featureDiscoveryServiceFactory', () => { }), 'index.js': ` const { createBackendModule, coreServices } = require('@backstage/backend-plugin-api'); - exports.detectedModuleDerp = createBackendModule({ + exports.default = createBackendModule({ pluginId: 'detected', moduleId: 'derp', register(env) { env.registerInit({ - deps: { identity: coreServices.identity }, - async init({ identity }) { - identity.getIdentity('detected-module'); + deps: { logger: coreServices.rootLogger }, + async init({ logger }) { + logger.warn('detected-module'); }, }); }, @@ -102,22 +99,42 @@ describe('featureDiscoveryServiceFactory', () => { role: 'backend-plugin', }, }), - 'index.js': `exports.detectedPlugin = undefined;`, + 'index.js': `exports.default = undefined;`, 'alpha.js': ` const { createBackendPlugin, coreServices } = require('@backstage/backend-plugin-api'); - exports.detectedPluginAlpha = createBackendPlugin({ + exports.default = createBackendPlugin({ pluginId: 'detected-alpha', register(env) { env.registerInit({ - deps: { identity: coreServices.identity }, - async init({ identity }) { - identity.getIdentity('detected-plugin-with-alpha'); + deps: { logger: coreServices.rootLogger }, + async init({ logger }) { + logger.warn('detected-plugin-with-alpha'); }, }); }, }); `, }, + [resolvePath(rootDir, 'node_modules/detected-library')]: { + 'package.json': JSON.stringify({ + name: 'detected-library', + main: 'index.js', + backstage: { + role: 'node-library', + }, + }), + 'index.js': ` + const { createServiceFactory, createServiceRef, coreServices } = require('@backstage/backend-plugin-api'); + exports.default = createServiceFactory({ + service: createServiceRef({ id: 'test', scope: 'root' }), + deps: { logger: coreServices.rootLogger }, + factory({ logger }) { + logger.warn('detected-library'); + return {}; + }, + }); + `, + }, }); }); @@ -126,15 +143,11 @@ describe('featureDiscoveryServiceFactory', () => { }); it('should detect plugin and module packages when "all" is specified', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { backend: { packages: 'all' } }, @@ -142,27 +155,28 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).toHaveBeenCalledWith('detected-plugin'); - expect(fn).toHaveBeenCalledWith('detected-module'); - expect(fn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin'); + expect(mock.warn).toHaveBeenCalledWith('detected-module'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).toHaveBeenCalledWith('detected-library'); }); it('detects only the packages that are listed as included', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { backend: { packages: { - include: ['detected-plugin', 'detected-plugin-with-alpha'], + include: [ + 'detected-plugin', + 'detected-plugin-with-alpha', + 'detected-library', + ], }, }, }, @@ -170,21 +184,18 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).toHaveBeenCalledWith('detected-plugin'); - expect(fn).toHaveBeenCalledWith('detected-plugin-with-alpha'); - expect(fn).not.toHaveBeenCalledWith('detected-module'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).toHaveBeenCalledWith('detected-library'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-module'); }); it('does not detect packages when included is an empty list', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { @@ -198,21 +209,18 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).not.toHaveBeenCalledWith('detected-plugin'); - expect(fn).not.toHaveBeenCalledWith('detected-plugin-with-alpha'); - expect(fn).not.toHaveBeenCalledWith('detected-module'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-plugin'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-module'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-library'); }); it('does not detect an excluded packages', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { @@ -226,21 +234,18 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).not.toHaveBeenCalledWith('detected-plugin'); - expect(fn).not.toHaveBeenCalledWith('detected-module'); - expect(fn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-plugin'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-module'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).toHaveBeenCalledWith('detected-library'); }); it('does not excluded packages when it is an empty list', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { @@ -254,21 +259,18 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).toHaveBeenCalledWith('detected-plugin'); - expect(fn).toHaveBeenCalledWith('detected-module'); - expect(fn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin'); + expect(mock.warn).toHaveBeenCalledWith('detected-module'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).toHaveBeenCalledWith('detected-library'); }); it('does not detect packages that are included and excluded', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { @@ -287,21 +289,18 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).not.toHaveBeenCalledWith('detected-plugin'); - expect(fn).toHaveBeenCalledWith('detected-module'); - expect(fn).toHaveBeenCalledWith('detected-plugin-with-alpha'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-plugin'); + expect(mock.warn).not.toHaveBeenCalledWith('detected-library'); + expect(mock.warn).toHaveBeenCalledWith('detected-module'); + expect(mock.warn).toHaveBeenCalledWith('detected-plugin-with-alpha'); }); it('does not detect any packages when "packages" is empty', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { backend: { packages: {} } }, @@ -309,19 +308,15 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).not.toHaveBeenCalled(); + expect(mock.warn).not.toHaveBeenCalled(); }); it('does not detect any packages when "packages" is not present', async () => { - const fn = jest.fn().mockResolvedValue({}); + const mock = mockServices.rootLogger.mock({ child: () => mock }); await startTestBackend({ features: [ - createServiceFactory({ - service: coreServices.identity, - deps: {}, - factory: () => ({ getIdentity: fn }), - }), + mock.factory, featureDiscoveryServiceFactory(), mockServices.rootConfig.factory({ data: { backend: {} }, @@ -329,6 +324,6 @@ describe('featureDiscoveryServiceFactory', () => { ], }); - expect(fn).not.toHaveBeenCalled(); + expect(mock.warn).not.toHaveBeenCalled(); }); }); diff --git a/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.ts b/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.ts index 07d5c4627d..b68475a77b 100644 --- a/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.ts +++ b/packages/backend-app-api/src/alpha/featureDiscoveryServiceFactory.ts @@ -29,8 +29,6 @@ import { resolve as resolvePath, dirname } from 'path'; import fs from 'fs-extra'; import { BackstagePackageJson } from '@backstage/cli-node'; -const LOADED_PACKAGE_ROLES = ['backend-plugin', 'backend-plugin-module']; - /** @internal */ async function findClosestPackageDir( searchDir: string, @@ -108,8 +106,8 @@ class PackageDiscoveryService implements FeatureDiscoveryService { const depPkg = require(require.resolve(`${name}/package.json`, { paths: [packageDir], })) as BackstagePackageJson; - if (!LOADED_PACKAGE_ROLES.includes(depPkg?.backstage?.role ?? '')) { - continue; + if (!depPkg?.backstage || depPkg?.backstage?.role === 'cli') { + continue; // Not a backstage package, ignore } const exportedModulePaths = [ @@ -128,16 +126,15 @@ class PackageDiscoveryService implements FeatureDiscoveryService { } for (const modulePath of exportedModulePaths) { - const module = require(modulePath); - for (const [exportName, exportValue] of Object.entries(module)) { - if (isBackendFeature(exportValue)) { - this.logger.info(`Detected: ${name}#${exportName}`); - features.push(exportValue); - } - if (isBackendFeatureFactory(exportValue)) { - this.logger.info(`Detected: ${name}#${exportName}`); - features.push(exportValue()); - } + const mod = require(modulePath); + + if (isBackendFeature(mod.default)) { + this.logger.info(`Detected: ${name}`); + features.push(mod.default); + } + if (isBackendFeatureFactory(mod.default)) { + this.logger.info(`Detected: ${name}`); + features.push(mod.default()); } } } diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index bc74c21542..4969f82176 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -91,11 +91,11 @@ export class BackendInitializer { return Object.fromEntries(result); } - add(feature: Promise) { + add(feature: BackendFeature | Promise) { if (this.#startPromise) { throw new Error('feature can not be added after the backend has started'); } - this.#registeredFeatures.push(feature); + this.#registeredFeatures.push(Promise.resolve(feature)); } #addFeature(feature: BackendFeature) { diff --git a/packages/backend-app-api/src/wiring/BackstageBackend.ts b/packages/backend-app-api/src/wiring/BackstageBackend.ts index 5d2a8716a0..2b916908de 100644 --- a/packages/backend-app-api/src/wiring/BackstageBackend.ts +++ b/packages/backend-app-api/src/wiring/BackstageBackend.ts @@ -34,7 +34,7 @@ export class BackstageBackend implements Backend { if (isPromise(feature)) { this.#initializer.add(feature.then(f => unwrapFeature(f.default))); } else { - this.#initializer.add(Promise.resolve(unwrapFeature(feature))); + this.#initializer.add(unwrapFeature(feature)); } }