paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 49 Packages Projects Releases Wiki Activity

Merge pull request #15520 from backstage/rugvip/nodup

Browse Source 
backend-app-api: throw errors on conflicting service implementations
This commit is contained in:
Patrik Oldsberg
2023-01-09 11:14:43 +01:00
committed by GitHub
parent 5a399da8f2 dd9f7b8b24
commit cfe22d7ee7
14 changed files with 273 additions and 82 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.changeset/eleven-fans-love.md
+6
View File
@@ -0,0 +1,6 @@
---
'@backstage/backend-test-utils': patch
'@backstage/backend-defaults': patch
---
Updated to make sure that service implementations replace default service implementations.
.changeset/great-forks-lay.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/backend-app-api': patch
---
An error will now be thrown if attempting to override the plugin metadata service.
.changeset/nine-falcons-appear.md
+5
View File
@@ -0,0 +1,5 @@
---
'@backstage/backend-app-api': patch
---
The `createSpecializedBackend` function will now throw an error if duplicate service implementations are provided.
packages/backend-app-api/src/wiring/BackendInitializer.test.ts
+2 -2
View File
@@ -40,12 +40,12 @@ describe('BackendInitializer', () => {
service: rootRef,
deps: {},
factory: rootFactory,
}),
})(),
createServiceFactory({
service: pluginRef,
deps: {},
factory: pluginFactory,
}),
})(),
]);
const init = new BackendInitializer(registry);
packages/backend-app-api/src/wiring/ServiceRegistry.test.ts
+11 -11
View File
@@ -33,7 +33,7 @@ const sf1 = createServiceFactory({
return { x: 1 };
};
},
});
})();
const ref2 = createServiceRef<{ x: number }>({
scope: 'root',
@@ -45,7 +45,7 @@ const sf2 = createServiceFactory({
async factory() {
return { x: 2 };
},
});
})();
const sf2b = createServiceFactory({
service: ref2,
deps: {},
@@ -133,7 +133,7 @@ describe('ServiceRegistry', () => {
return { x: 2 };
},
});
const registry = new ServiceRegistry([factory, sf1]);
const registry = new ServiceRegistry([factory(), sf1]);
await expect(registry.get(ref2, 'catalog')).rejects.toThrow(
"Failed to instantiate 'root' scoped service '2' because it depends on 'plugin' scoped service '1'.",
);
@@ -147,7 +147,7 @@ describe('ServiceRegistry', () => {
return async () => ({ x: rootDep.x });
},
});
const registry = new ServiceRegistry([factory, sf2]);
const registry = new ServiceRegistry([factory(), sf2]);
await expect(registry.get(ref1, 'catalog')).resolves.toEqual({
x: 2,
});
@@ -162,7 +162,7 @@ describe('ServiceRegistry', () => {
return { x: rootDep.x };
},
});
const registry = new ServiceRegistry([factory, sf2]);
const registry = new ServiceRegistry([factory(), sf2]);
await expect(registry.get(ref, 'catalog')).resolves.toEqual({
x: 2,
});
@@ -177,7 +177,7 @@ describe('ServiceRegistry', () => {
return async ({ meta }) => ({ pluginId: meta.getId() });
},
});
const registry = new ServiceRegistry([factory]);
const registry = new ServiceRegistry([factory()]);
await expect(registry.get(ref, 'catalog')).resolves.toEqual({
pluginId: 'catalog',
});
@@ -257,7 +257,7 @@ describe('ServiceRegistry', () => {
factory,
});
const registry = new ServiceRegistry([myFactory]);
const registry = new ServiceRegistry([myFactory()]);
await Promise.all([
registry.get(ref1, 'catalog')!,
@@ -280,7 +280,7 @@ describe('ServiceRegistry', () => {
},
});
const registry = new ServiceRegistry([myFactory]);
const registry = new ServiceRegistry([myFactory()]);
await expect(registry.get(ref1, 'catalog')).rejects.toThrow(
"Failed to instantiate service '1' for 'catalog' because the following dependent services are missing: '2'",
@@ -309,7 +309,7 @@ describe('ServiceRegistry', () => {
},
});
const registry = new ServiceRegistry([factoryA, factoryB]);
const registry = new ServiceRegistry([factoryA(), factoryB()]);
await expect(registry.get(refA, 'catalog')).rejects.toThrow(
"Failed to instantiate service 'a' for 'catalog' because the factory function threw an error, Error: Failed to instantiate service 'b' for 'catalog' because the following dependent services are missing: 'c', 'd'",
@@ -325,7 +325,7 @@ describe('ServiceRegistry', () => {
},
});
const registry = new ServiceRegistry([myFactory]);
const registry = new ServiceRegistry([myFactory()]);
await expect(registry.get(ref1, 'catalog')).rejects.toThrow(
"Failed to instantiate service '1' because the top-level factory function threw an error, Error: top-level error",
@@ -343,7 +343,7 @@ describe('ServiceRegistry', () => {
},
});
const registry = new ServiceRegistry([myFactory]);
const registry = new ServiceRegistry([myFactory()]);
await expect(registry.get(ref1, 'catalog')).rejects.toThrow(
"Failed to instantiate service '1' for 'catalog' because the factory function threw an error, Error: error in plugin",
packages/backend-app-api/src/wiring/ServiceRegistry.ts
+2 -12
View File
@@ -44,18 +44,8 @@ export class ServiceRegistry implements EnumerableServiceHolder {
}
>;
constructor(
factories: Array<ServiceFactory<unknown> | (() => ServiceFactory<unknown>)>,
) {
this.#providedFactories = new Map(
factories.map(f => {
if (typeof f === 'function') {
const cf = f();
return [cf.service.id, cf];
}
return [f.service.id, f];
}),
);
constructor(factories: Array<ServiceFactory<unknown>>) {
this.#providedFactories = new Map(factories.map(f => [f.service.id, f]));
this.#loadedDefaultFactories = new Map();
this.#implementations = new Map();
}
packages/backend-app-api/src/wiring/createSpecializedBackend.test.ts
+62
View File
@@ -0,0 +1,62 @@
/*
* 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 { createSpecializedBackend } from './createSpecializedBackend';
describe('createSpecializedBackend', () => {
it('should create a backend without services', () => {
expect(() => createSpecializedBackend({ services: [] })).not.toThrow();
});
it('should throw on duplicate service implementations', () => {
expect(() =>
createSpecializedBackend({
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',
);
});
it('should throw when providing a plugin metadata service implementation', () => {
expect(() =>
createSpecializedBackend({
services: [
createServiceFactory({
service: coreServices.pluginMetadata,
deps: {},
factory: async () => async () => ({ getId: () => 'test' }),
}),
],
}),
).toThrow('The core.pluginMetadata service cannot be overridden');
});
});
packages/backend-app-api/src/wiring/createSpecializedBackend.ts
+51
View File
@@ -0,0 +1,51 @@
/*
* Copyright 2022 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 } from '@backstage/backend-plugin-api';
import { BackstageBackend } from './BackstageBackend';
import { Backend, CreateSpecializedBackendOptions } from './types';
/**
* @public
*/
export function createSpecializedBackend(
options: CreateSpecializedBackendOptions,
): Backend {
const services = options.services.map(sf =>
typeof sf === 'function' ? sf() : sf,
);
const exists = new Set<string>();
const duplicates = new Set<string>();
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}`);
}
if (exists.has(coreServices.pluginMetadata.id)) {
throw new Error(
`The ${coreServices.pluginMetadata.id} service cannot be overridden`,
);
}
return new BackstageBackend(services);
}
packages/backend-app-api/src/wiring/index.ts
+1 -1
View File
@@ -19,4 +19,4 @@ export type {
CreateSpecializedBackendOptions,
ServiceOrExtensionPoint,
} from './types';
export { createSpecializedBackend } from './types';
export { createSpecializedBackend } from './createSpecializedBackend';
packages/backend-app-api/src/wiring/types.ts
+1 -13
View File
@@ -15,12 +15,11 @@
*/
import {
ServiceFactory,
BackendFeature,
ExtensionPoint,
ServiceFactory,
ServiceRef,
} from '@backstage/backend-plugin-api';
import { BackstageBackend } from './BackstageBackend';
/**
* @public
@@ -57,17 +56,6 @@ export interface EnumerableServiceHolder extends ServiceHolder {
getServiceRefs(): ServiceRef<unknown>[];
}
/**
* @public
*/
export function createSpecializedBackend(
options: CreateSpecializedBackendOptions,
): Backend {
return new BackstageBackend(
options.services.map(s => (typeof s === 'function' ? s() : s)),
);
}
/**
* @public
*/
packages/backend-defaults/src/CreateBackend.test.ts
+72
View File
@@ -0,0 +1,72 @@
/*
* 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',
);
});
it('should throw when providing a plugin metadata service implementation', () => {
expect(() =>
createBackend({
services: [
createServiceFactory({
service: coreServices.pluginMetadata,
deps: {},
factory: async () => async () => ({ getId: () => 'test' }),
}),
],
}),
).toThrow('The core.pluginMetadata service cannot be overridden');
});
});
packages/backend-defaults/src/CreateBackend.ts
+23 -15
View File
@@ -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],
});
}
packages/backend-test-utils/src/next/wiring/TestBackend.test.ts
+28 -27
View File
@@ -64,33 +64,34 @@ describe('TestBackend', () => {
const extensionPoint3 = createExtensionPoint<Obj>({ id: 'b3' });
const extensionPoint4 = createExtensionPoint<Obj>({ id: 'b4' });
const extensionPoint5 = createExtensionPoint<Obj>({ 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 () => {
packages/backend-test-utils/src/next/wiring/TestBackend.ts
+4 -1
View File
@@ -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);
}
}