diff --git a/.changeset/eleven-fans-love.md b/.changeset/eleven-fans-love.md new file mode 100644 index 0000000000..1b8f96be45 --- /dev/null +++ b/.changeset/eleven-fans-love.md @@ -0,0 +1,6 @@ +--- +'@backstage/backend-test-utils': patch +'@backstage/backend-defaults': patch +--- + +Updated to make sure that service implementations replace default service implementations. diff --git a/.changeset/nine-falcons-appear.md b/.changeset/nine-falcons-appear.md new file mode 100644 index 0000000000..2011f4d992 --- /dev/null +++ b/.changeset/nine-falcons-appear.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +The `createSpecializedBackend` function will now throw an error if duplicate service implementations are provided. diff --git a/packages/backend-app-api/src/wiring/types.ts b/packages/backend-app-api/src/wiring/types.ts index 46ac690702..c591d5cb55 100644 --- a/packages/backend-app-api/src/wiring/types.ts +++ b/packages/backend-app-api/src/wiring/types.ts @@ -63,9 +63,25 @@ export interface EnumerableServiceHolder extends ServiceHolder { export function createSpecializedBackend( options: CreateSpecializedBackendOptions, ): Backend { - return new BackstageBackend( - options.services.map(s => (typeof s === 'function' ? s() : s)), + const services = options.services.map(sf => + typeof sf === 'function' ? sf() : sf, ); + + const exists = new Set(); + const duplicates = new Set(); + for (const { service } of services) { + if (exists.has(service.id)) { + duplicates.add(service.id); + } else { + exists.add(service.id); + } + } + if (duplicates.size > 0) { + const ids = Array.from(duplicates).join(', '); + throw new Error(`Duplicate service implementations provided for ${ids}`); + } + + return new BackstageBackend(services); } /** diff --git a/packages/backend-defaults/src/CreateBackend.test.ts b/packages/backend-defaults/src/CreateBackend.test.ts new file mode 100644 index 0000000000..4b5e2e5848 --- /dev/null +++ b/packages/backend-defaults/src/CreateBackend.test.ts @@ -0,0 +1,58 @@ +/* + * 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. + */ + +import { + coreServices, + createServiceFactory, +} from '@backstage/backend-plugin-api'; +import { createBackend } from './CreateBackend'; + +describe('createBackend', () => { + it('should not throw when overriding a default service implementation', () => { + expect(() => + createBackend({ + services: [ + createServiceFactory({ + service: coreServices.rootLifecycle, + deps: {}, + factory: async () => ({ addShutdownHook: () => {} }), + }), + ], + }), + ).not.toThrow(); + }); + + it('should throw on duplicate service implementations', () => { + expect(() => + createBackend({ + services: [ + createServiceFactory({ + service: coreServices.rootLifecycle, + deps: {}, + factory: async () => ({ addShutdownHook: () => {} }), + }), + createServiceFactory({ + service: coreServices.rootLifecycle, + deps: {}, + factory: async () => ({ addShutdownHook: () => {} }), + }), + ], + }), + ).toThrow( + 'Duplicate service implementations provided for core.rootLifecycle', + ); + }); +}); diff --git a/packages/backend-defaults/src/CreateBackend.ts b/packages/backend-defaults/src/CreateBackend.ts index 20edfc85a8..6f9e8e39b6 100644 --- a/packages/backend-defaults/src/CreateBackend.ts +++ b/packages/backend-defaults/src/CreateBackend.ts @@ -35,20 +35,20 @@ import { import { ServiceFactory } from '@backstage/backend-plugin-api'; export const defaultServiceFactories = [ - cacheFactory, - configFactory, - databaseFactory, - discoveryFactory, - loggerFactory, - rootLoggerFactory, - permissionsFactory, - schedulerFactory, - tokenManagerFactory, - urlReaderFactory, - httpRouterFactory, - rootHttpRouterFactory, - lifecycleFactory, - rootLifecycleFactory, + cacheFactory(), + configFactory(), + databaseFactory(), + discoveryFactory(), + loggerFactory(), + rootLoggerFactory(), + permissionsFactory(), + schedulerFactory(), + tokenManagerFactory(), + urlReaderFactory(), + httpRouterFactory(), + rootHttpRouterFactory(), + lifecycleFactory(), + rootLifecycleFactory(), ]; /** @@ -62,7 +62,15 @@ export interface CreateBackendOptions { * @public */ export function createBackend(options?: CreateBackendOptions): Backend { + const providedServices = (options?.services ?? []).map(sf => + typeof sf === 'function' ? sf() : sf, + ); + const providedIds = new Set(providedServices.map(sf => sf.service.id)); + const neededDefaultFactories = defaultServiceFactories.filter( + sf => !providedIds.has(sf.service.id), + ); + return createSpecializedBackend({ - services: [...defaultServiceFactories, ...(options?.services ?? [])], + services: [...neededDefaultFactories, ...providedServices], }); } diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.test.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.test.ts index 3cc8507df3..5ea37e9106 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.test.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.test.ts @@ -64,33 +64,34 @@ describe('TestBackend', () => { const extensionPoint3 = createExtensionPoint({ id: 'b3' }); const extensionPoint4 = createExtensionPoint({ id: 'b4' }); const extensionPoint5 = createExtensionPoint({ id: 'b5' }); - await startTestBackend({ - services: [ - // @ts-expect-error - [extensionPoint1, { a: 'a' }], - [serviceRef, { a: 'a' }], - [serviceRef, { a: 'a', b: 'b' }], - // @ts-expect-error - [serviceRef, { c: 'c' }], - // @ts-expect-error - [serviceRef, { a: 'a', c: 'c' }], - // @ts-expect-error - [serviceRef, { a: 'a', b: 'b', c: 'c' }], - ], - extensionPoints: [ - // @ts-expect-error - [serviceRef, { a: 'a' }], - [extensionPoint1, { a: 'a' }], - [extensionPoint2, { a: 'a', b: 'b' }], - // @ts-expect-error - [extensionPoint3, { c: 'c' }], - // @ts-expect-error - [extensionPoint4, { a: 'a', c: 'c' }], - // @ts-expect-error - [extensionPoint5, { a: 'a', b: 'b', c: 'c' }], - ], - }); - expect(1).toBe(1); + await expect( + startTestBackend({ + services: [ + // @ts-expect-error + [extensionPoint1, { a: 'a' }], + [serviceRef, { a: 'a' }], + [serviceRef, { a: 'a', b: 'b' }], + // @ts-expect-error + [serviceRef, { c: 'c' }], + // @ts-expect-error + [serviceRef, { a: 'a', c: 'c' }], + // @ts-expect-error + [serviceRef, { a: 'a', b: 'b', c: 'c' }], + ], + extensionPoints: [ + // @ts-expect-error + [serviceRef, { a: 'a' }], + [extensionPoint1, { a: 'a' }], + [extensionPoint2, { a: 'a', b: 'b' }], + // @ts-expect-error + [extensionPoint3, { c: 'c' }], + // @ts-expect-error + [extensionPoint4, { a: 'a', c: 'c' }], + // @ts-expect-error + [extensionPoint5, { a: 'a', b: 'b', c: 'c' }], + ], + }), + ).rejects.toThrow(); }); it('should start the test backend', async () => { diff --git a/packages/backend-test-utils/src/next/wiring/TestBackend.ts b/packages/backend-test-utils/src/next/wiring/TestBackend.ts index 647ebc9dda..0295332855 100644 --- a/packages/backend-test-utils/src/next/wiring/TestBackend.ts +++ b/packages/backend-test-utils/src/next/wiring/TestBackend.ts @@ -93,11 +93,14 @@ export async function startTestBackend< factory: async () => impl, })(); } + if (typeof serviceDef === 'function') { + return serviceDef(); + } return serviceDef as ServiceFactory; }); for (const factory of defaultServiceFactories) { - if (!factories.some(f => f.service === factory.service)) { + if (!factories.some(f => f.service.id === factory.service.id)) { factories.push(factory); } }