From 229a1fa92716a384f67850cfe887bbfa91668f01 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 8 Sep 2022 15:46:16 +0200 Subject: [PATCH 01/10] backend-{plugin,app}-api: introduce service scopes and update registry to support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../src/wiring/ServiceRegistry.test.ts | 155 +++++++-------- .../src/wiring/ServiceRegistry.ts | 185 ++++++++++++------ packages/backend-app-api/src/wiring/types.ts | 3 +- .../src/services/system/types.ts | 135 ++++++++----- 4 files changed, 284 insertions(+), 194 deletions(-) diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts index a00f6c0b4b..ab709dcb77 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts @@ -21,28 +21,28 @@ import { } from '@backstage/backend-plugin-api'; import { ServiceRegistry } from './ServiceRegistry'; -const ref1 = createServiceRef<{ x: number; pluginId: string }>({ +const ref1 = createServiceRef<{ x: number }>({ id: '1', }); const sf1 = createServiceFactory({ service: ref1, deps: {}, factory: async () => { - return async pluginId => { - return { x: 1, pluginId }; + return async () => { + return { x: 1 }; }; }, }); -const ref2 = createServiceRef<{ x: number; pluginId: string }>({ +const ref2 = createServiceRef<{ x: number }>({ id: '2', }); const sf2 = createServiceFactory({ service: ref2, deps: {}, factory: async () => { - return async pluginId => { - return { x: 2, pluginId }; + return async () => { + return { x: 2 }; }; }, }); @@ -50,126 +50,115 @@ const sf2b = createServiceFactory({ service: ref2, deps: {}, factory: async () => { - return async pluginId => { - return { x: 22, pluginId }; + return async () => { + return { x: 22 }; }; }, }); -const refDefault1 = createServiceRef<{ x: number; pluginId: string }>({ +const refDefault1 = createServiceRef<{ x: number }>({ id: '1', defaultFactory: async service => createServiceFactory({ service, deps: {}, - factory: async () => async pluginId => ({ x: 10, pluginId }), + factory: async () => async () => ({ x: 10 }), }), }); -const refDefault2a = createServiceRef<{ x: number; pluginId: string }>({ +const refDefault2a = createServiceRef<{ x: number }>({ id: '2a', defaultFactory: async service => createServiceFactory({ service, deps: {}, - factory: async () => async pluginId => ({ x: 20, pluginId }), + factory: async () => async () => ({ x: 20 }), }), }); -const refDefault2b = createServiceRef<{ x: number; pluginId: string }>({ +const refDefault2b = createServiceRef<{ x: number }>({ id: '2b', defaultFactory: async service => createServiceFactory({ service, deps: {}, - factory: async () => async pluginId => ({ x: 220, pluginId }), + factory: async () => async () => ({ x: 220 }), }), }); describe('ServiceRegistry', () => { it('should return undefined if there is no factory defined', async () => { const registry = new ServiceRegistry([]); - expect(registry.get(ref1)).toBe(undefined); + expect(registry.get(ref1, 'catalog')).toBe(undefined); }); - it('should return a factory for a registered ref', async () => { + it('should return an implementation for a registered ref', async () => { const registry = new ServiceRegistry([sf1]); - const factory = registry.get(ref1)!; - expect(factory).toEqual(expect.any(Function)); - await expect(factory('catalog')).resolves.toEqual({ - x: 1, - pluginId: 'catalog', - }); - await expect(factory('scaffolder')).resolves.toEqual({ - x: 1, - pluginId: 'scaffolder', - }); - expect(await factory('catalog')).toBe(await factory('catalog')); + await expect(registry.get(ref1, 'catalog')).resolves.toEqual({ x: 1 }); + await expect(registry.get(ref1, 'scaffolder')).resolves.toEqual({ x: 1 }); + expect(await registry.get(ref1, 'catalog')).toBe( + await registry.get(ref1, 'catalog'), + ); + expect(await registry.get(ref1, 'scaffolder')).toBe( + await registry.get(ref1, 'scaffolder'), + ); + expect(await registry.get(ref1, 'catalog')).not.toBe( + await registry.get(ref1, 'scaffolder'), + ); }); it('should handle multiple factories with different serviceRefs', async () => { const registry = new ServiceRegistry([sf1, sf2]); - const factory1 = registry.get(ref1)!; - const factory2 = registry.get(ref2)!; - expect(factory1).toEqual(expect.any(Function)); - expect(factory2).toEqual(expect.any(Function)); - await expect(factory1('catalog')).resolves.toEqual({ + + await expect(registry.get(ref1, 'catalog')).resolves.toEqual({ x: 1, - pluginId: 'catalog', }); - await expect(factory2('catalog')).resolves.toEqual({ + await expect(registry.get(ref2, 'catalog')).resolves.toEqual({ x: 2, - pluginId: 'catalog', }); - expect(await factory1('catalog')).not.toBe(await factory2('catalog')); + expect(await registry.get(ref1, 'catalog')).not.toBe( + await registry.get(ref2, 'catalog'), + ); }); it('should use the last factory for each ref', async () => { const registry = new ServiceRegistry([sf2, sf2b]); - const factory2 = registry.get(ref2)!; - await expect(factory2('catalog')).resolves.toEqual({ + await expect(registry.get(ref2, 'catalog')).resolves.toEqual({ x: 22, - pluginId: 'catalog', }); }); - it('should return the defaultFactory from the ref if not provided to the registry', async () => { + it('should use the defaultFactory from the ref if not provided to the registry', async () => { const registry = new ServiceRegistry([]); - const factory = registry.get(refDefault1)!; - expect(factory).toEqual(expect.any(Function)); - await expect(factory('catalog')).resolves.toEqual({ + await expect(registry.get(refDefault1, 'catalog')).resolves.toEqual({ x: 10, - pluginId: 'catalog', }); }); - it('should not return the defaultFactory from the ref if provided to the registry', async () => { + it('should not use the defaultFactory from the ref if provided to the registry', async () => { const registry = new ServiceRegistry([sf1]); - const factory = registry.get(refDefault1)!; - expect(factory).toEqual(expect.any(Function)); - await expect(factory('catalog')).resolves.toEqual({ + await expect(registry.get(refDefault1, 'catalog')).resolves.toEqual({ x: 1, - pluginId: 'catalog', }); }); it('should handle duplicate defaultFactories by duplicating the implementations', async () => { const registry = new ServiceRegistry([]); - const factoryA = registry.get(refDefault2a)!; - const factoryB = registry.get(refDefault2b)!; - expect(factoryA).toEqual(expect.any(Function)); - expect(factoryB).toEqual(expect.any(Function)); - await expect(factoryA('catalog')).resolves.toEqual({ + await expect(registry.get(refDefault2a, 'catalog')).resolves.toEqual({ x: 20, - pluginId: 'catalog', }); - await expect(factoryB('catalog')).resolves.toEqual({ + await expect(registry.get(refDefault2b, 'catalog')).resolves.toEqual({ x: 220, - pluginId: 'catalog', }); - expect(await factoryA('catalog')).toBe(await factoryA('catalog')); - expect(await factoryB('catalog')).toBe(await factoryB('catalog')); - expect(await factoryA('catalog')).not.toBe(await factoryB('catalog')); + expect(await registry.get(refDefault2a, 'catalog')).toBe( + await registry.get(refDefault2a, 'catalog'), + ); + expect(await registry.get(refDefault2b, 'catalog')).toBe( + await registry.get(refDefault2b, 'catalog'), + ); + expect(await registry.get(refDefault2a, 'catalog')).not.toBe( + await registry.get(refDefault2b, 'catalog'), + ); }); it('should only call each default factory loader once', async () => { @@ -186,17 +175,16 @@ describe('ServiceRegistry', () => { }); const registry = new ServiceRegistry([]); - const factory = registry.get(ref)!; await Promise.all([ - expect(factory('catalog')).resolves.toBeUndefined(), - expect(factory('catalog')).resolves.toBeUndefined(), + expect(registry.get(ref, 'catalog')).resolves.toBeUndefined(), + expect(registry.get(ref, 'catalog')).resolves.toBeUndefined(), ]); expect(factoryLoader).toHaveBeenCalledTimes(1); }); it('should not call factory functions more than once', async () => { - const innerFactory = jest.fn(async (pluginId: string) => { - return { x: 1, pluginId }; + const innerFactory = jest.fn(async () => { + return { x: 1 }; }); const factory = jest.fn(async () => innerFactory); const myFactory = createServiceFactory({ @@ -208,17 +196,15 @@ describe('ServiceRegistry', () => { const registry = new ServiceRegistry([myFactory]); await Promise.all([ - registry.get(ref1)!('catalog')!, - registry.get(ref1)!('catalog')!, - registry.get(ref1)!('catalog')!, - registry.get(ref1)!('scaffolder')!, - registry.get(ref1)!('scaffolder')!, + registry.get(ref1, 'catalog')!, + registry.get(ref1, 'catalog')!, + registry.get(ref1, 'catalog')!, + registry.get(ref1, 'scaffolder')!, + registry.get(ref1, 'scaffolder')!, ]); expect(factory).toHaveBeenCalledTimes(1); expect(innerFactory).toHaveBeenCalledTimes(2); - expect(innerFactory).toHaveBeenCalledWith('catalog'); - expect(innerFactory).toHaveBeenCalledWith('scaffolder'); }); it('should throw if dependencies are not available', async () => { @@ -231,9 +217,8 @@ describe('ServiceRegistry', () => { }); const registry = new ServiceRegistry([myFactory]); - const factory = registry.get(ref1)!; - await expect(factory('catalog')).rejects.toThrow( + await expect(registry.get(ref1, 'catalog')).rejects.toThrow( "Failed to instantiate service '1' for 'catalog' because the following dependent services are missing: '2'", ); }); @@ -247,8 +232,8 @@ describe('ServiceRegistry', () => { const factoryA = createServiceFactory({ service: refA, deps: { b: refB }, - async factory({ b }) { - return async pluginId => b(pluginId); + async factory() { + return async ({ b }) => b; }, }); @@ -261,9 +246,8 @@ describe('ServiceRegistry', () => { }); const registry = new ServiceRegistry([factoryA, factoryB]); - const factory = registry.get(refA)!; - await expect(factory('catalog')).rejects.toThrow( + 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'", ); }); @@ -278,9 +262,8 @@ describe('ServiceRegistry', () => { }); const registry = new ServiceRegistry([myFactory]); - const factory = registry.get(ref1)!; - await expect(factory('catalog')).rejects.toThrow( + 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", ); }); @@ -290,17 +273,16 @@ describe('ServiceRegistry', () => { service: ref1, deps: {}, async factory() { - return pluginId => { - throw new Error(`error in plugin ${pluginId}`); + return () => { + throw new Error(`error in plugin`); }; }, }); const registry = new ServiceRegistry([myFactory]); - const factory = registry.get(ref1)!; - await expect(factory('catalog')).rejects.toThrow( - "Failed to instantiate service '1' for 'catalog' because the factory function threw an error, Error: error in plugin catalog", + 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", ); }); @@ -313,9 +295,8 @@ describe('ServiceRegistry', () => { }); const registry = new ServiceRegistry([]); - const factory = registry.get(ref)!; - await expect(factory('catalog')).rejects.toThrow( + await expect(registry.get(ref, 'catalog')).rejects.toThrow( "Failed to instantiate service '1' because the default factory loader threw an error, Error: default factory error", ); }); diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.ts index e0509aecde..007ca00c88 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.ts @@ -16,8 +16,8 @@ import { ServiceFactory, - FactoryFunc, ServiceRef, + pluginMetadataServiceRef, } from '@backstage/backend-plugin-api'; import { stringifyError } from '@backstage/errors'; @@ -37,7 +37,9 @@ export class ServiceRegistry { readonly #implementations: Map< ServiceFactory, { - factoryFunc: Promise>; + factoryFunc: Promise< + (deps: { [name in string]: unknown }) => Promise + >; byPlugin: Map>; } >; @@ -58,67 +60,123 @@ export class ServiceRegistry { this.#implementations = new Map(); } - get(ref: ServiceRef): FactoryFunc | undefined { - let factory = this.#providedFactories.get(ref.id); - const { __defaultFactory: defaultFactory } = ref as InternalServiceRef; - if (!factory && !defaultFactory) { + #resolveFactory( + ref: ServiceRef, + pluginId: string, + ): Promise | undefined { + // Special case handling of the plugin metadata service, generating a custom factory for it each time + if (ref.id === pluginMetadataServiceRef.id) { + return Promise.resolve({ + scope: 'plugin', + service: pluginMetadataServiceRef, + deps: {}, + factory: async () => async () => ({ + getId() { + return pluginId; + }, + }), + }); + } + + let resolvedFactory: Promise | ServiceFactory | undefined = + this.#providedFactories.get(ref.id); + const { __defaultFactory: defaultFactory } = + ref as InternalServiceRef; + if (!resolvedFactory && !defaultFactory) { return undefined; } - return async (pluginId: string): Promise => { - if (!factory) { - let loadedFactory = this.#loadedDefaultFactories.get(defaultFactory!); - if (!loadedFactory) { - loadedFactory = Promise.resolve() - .then(() => defaultFactory!(ref)) - .then(f => - typeof f === 'function' ? f() : f, - ) as Promise; - this.#loadedDefaultFactories.set(defaultFactory!, loadedFactory); - } - // NOTE: This await is safe as long as #providedFactories is not mutated. - factory = await loadedFactory.catch(error => { - throw new Error( - `Failed to instantiate service '${ - ref.id - }' because the default factory loader threw an error, ${stringifyError( - error, - )}`, + if (!resolvedFactory) { + let loadedFactory = this.#loadedDefaultFactories.get(defaultFactory!); + if (!loadedFactory) { + loadedFactory = Promise.resolve() + .then(() => defaultFactory!(ref)) + .then(f => + typeof f === 'function' ? f() : f, + ) as Promise; + this.#loadedDefaultFactories.set(defaultFactory!, loadedFactory); + } + resolvedFactory = loadedFactory.catch(error => { + throw new Error( + `Failed to instantiate service '${ + ref.id + }' because the default factory loader threw an error, ${stringifyError( + error, + )}`, + ); + }); + } + + return Promise.resolve(resolvedFactory); + } + + #separateMapForTheRootService = new Map>(); + + #checkForMissingDeps(factory: ServiceFactory, pluginId: string) { + const missingDeps = Object.values(factory.deps).filter(ref => { + if (ref.id === pluginMetadataServiceRef.id) { + return false; + } + if (this.#providedFactories.get(ref.id)) { + return false; + } + + return !(ref as InternalServiceRef).__defaultFactory; + }); + + if (missingDeps.length) { + const missing = missingDeps.map(r => `'${r.id}'`).join(', '); + throw new Error( + `Failed to instantiate service '${factory.service.id}' for '${pluginId}' because the following dependent services are missing: ${missing}`, + ); + } + } + + get(ref: ServiceRef, pluginId: string): Promise | undefined { + return this.#resolveFactory(ref, pluginId)?.then(factory => { + if (factory.scope === 'root') { + let existing = this.#separateMapForTheRootService.get(factory); + if (!existing) { + this.#checkForMissingDeps(factory, pluginId); + const rootDeps = new Array>(); + + for (const [name, serviceRef] of Object.entries(factory.deps)) { + if (serviceRef.scope !== 'root') { + throw new Error( + `Failed to instantiate 'root' scoped service '${ref.id}' because it depends on '${serviceRef.scope}' scoped service '${serviceRef.id}'.`, + ); + } + const target = this.get(serviceRef, pluginId)!; + rootDeps.push(target.then(impl => [name, impl])); + } + + existing = Promise.all(rootDeps).then(entries => + factory.factory(Object.fromEntries(entries)), ); - }); + this.#separateMapForTheRootService.set(factory, existing); + } + return existing as Promise; } let implementation = this.#implementations.get(factory); if (!implementation) { - const missingRefs = new Array>(); - const factoryDeps: { [name in string]: FactoryFunc } = {}; + this.#checkForMissingDeps(factory, pluginId); + const rootDeps = new Array>(); for (const [name, serviceRef] of Object.entries(factory.deps)) { - const target = this.get(serviceRef); - if (!target) { - missingRefs.push(serviceRef); - } else { - factoryDeps[name] = target; + if (serviceRef.scope === 'root') { + const target = this.get(serviceRef, pluginId)!; + rootDeps.push(target.then(impl => [name, impl])); } } - if (missingRefs.length) { - const missing = missingRefs.map(r => `'${r.id}'`).join(', '); - throw new Error( - `Failed to instantiate service '${ref.id}' for '${pluginId}' because the following dependent services are missing: ${missing}`, - ); - } - implementation = { - factoryFunc: Promise.resolve() - .then(() => factory!.factory(factoryDeps)) + factoryFunc: Promise.all(rootDeps) + .then(entries => factory.factory(Object.fromEntries(entries))) .catch(error => { + const cause = stringifyError(error); throw new Error( - `Failed to instantiate service '${ - ref.id - }' because the top-level factory function threw an error, ${stringifyError( - error, - )}`, + `Failed to instantiate service '${ref.id}' because the top-level factory function threw an error, ${cause}`, ); }), byPlugin: new Map(), @@ -129,24 +187,29 @@ export class ServiceRegistry { let result = implementation.byPlugin.get(pluginId) as Promise; if (!result) { - result = implementation.factoryFunc.then(func => - Promise.resolve() - .then(() => func(pluginId)) - .catch(error => { - throw new Error( - `Failed to instantiate service '${ - ref.id - }' for '${pluginId}' because the factory function threw an error, ${stringifyError( - error, - )}`, - ); - }), - ); + const allDeps = new Array>(); + for (const [name, serviceRef] of Object.entries(factory.deps)) { + const target = this.get(serviceRef, pluginId)!; + allDeps.push(target.then(impl => [name, impl])); + } + + result = implementation.factoryFunc + .then(func => + Promise.all(allDeps).then(entries => + func(Object.fromEntries(entries)), + ), + ) + .catch(error => { + const cause = stringifyError(error); + throw new Error( + `Failed to instantiate service '${ref.id}' for '${pluginId}' because the factory function threw an error, ${cause}`, + ); + }); implementation.byPlugin.set(pluginId, result); } return result; - }; + }); } } diff --git a/packages/backend-app-api/src/wiring/types.ts b/packages/backend-app-api/src/wiring/types.ts index febc6830c7..40ce6aa1ae 100644 --- a/packages/backend-app-api/src/wiring/types.ts +++ b/packages/backend-app-api/src/wiring/types.ts @@ -18,7 +18,6 @@ import { ServiceFactory, BackendFeature, ExtensionPoint, - FactoryFunc, ServiceRef, } from '@backstage/backend-plugin-api'; import { BackstageBackend } from './BackstageBackend'; @@ -47,7 +46,7 @@ export interface CreateSpecializedBackendOptions { } export type ServiceHolder = { - get(api: ServiceRef): FactoryFunc | undefined; + get(api: ServiceRef, pluginId: string): Promise | undefined; }; /** diff --git a/packages/backend-plugin-api/src/services/system/types.ts b/packages/backend-plugin-api/src/services/system/types.ts index a19c17bb9c..4a9f91541a 100644 --- a/packages/backend-plugin-api/src/services/system/types.ts +++ b/packages/backend-plugin-api/src/services/system/types.ts @@ -19,63 +19,84 @@ * * @public */ -export type ServiceRef = { +export type ServiceRef< + TService, + TScope extends 'root' | 'plugin' = 'root' | 'plugin', +> = { id: string; + /** + * This determines the scope at which this service is available. + * + * Root scoped services are available to all other services but + * may only depend on other root scoped services. + * + * Plugin scoped services are only available to other plugin scoped + * services but may depend on all other services. + */ + scope: TScope; + /** * Utility for getting the type of the service, using `typeof serviceRef.T`. * Attempting to actually read this value will result in an exception. */ - T: T; + T: TService; toString(): string; $$ref: 'service'; }; -/** - * @internal - */ -export type InternalServiceRef = ServiceRef & { - /** - * The default factory that will be used to create service - * instances if no other factory is provided. - */ - __defaultFactory?: ( - service: ServiceRef, - ) => Promise | (() => ServiceFactory)>; -}; - /** @public */ export type TypesToServiceRef = { [key in keyof T]: ServiceRef }; /** @public */ -export type DepsToDepFactories = { - [key in keyof T]: (pluginId: string) => Promise; -}; - -/** @public */ -export type FactoryFunc = (pluginId: string) => Promise; - -/** @public */ -export type ServiceFactory = { - service: ServiceRef; - deps: { [key in string]: ServiceRef }; - factory(deps: { [key in string]: unknown }): Promise>; -}; +export type ServiceFactory = + | { + // This scope prop is needed in addition to the service ref, as TypeScript + // can't properly discriminate the two factory types otherwise. + scope: 'root'; + service: ServiceRef; + deps: { [key in string]: ServiceRef }; + factory(deps: { [key in string]: unknown }): Promise; + } + | { + scope: 'plugin'; + service: ServiceRef; + deps: { [key in string]: ServiceRef }; + factory(deps: { [key in string]: unknown }): Promise< + (deps: { [key in string]: unknown }) => Promise + >; + }; /** * @public */ export function createServiceRef(options: { id: string; + scope?: 'plugin'; defaultFactory?: ( service: ServiceRef, ) => Promise | (() => ServiceFactory)>; -}): ServiceRef { - const { id, defaultFactory } = options; +}): ServiceRef; +export function createServiceRef(options: { + id: string; + scope: 'root'; + defaultFactory?: ( + service: ServiceRef, + ) => Promise | (() => ServiceFactory)>; +}): ServiceRef; +export function createServiceRef(options: { + id: string; + scope?: 'plugin' | 'root'; + defaultFactory?: ( + service: ServiceRef, + ) => Promise | (() => ServiceFactory)>; +}): ServiceRef { + const { id, scope = 'plugin', defaultFactory } = options; return { id, + scope, get T(): T { throw new Error(`tried to read ServiceRef.T of ${this}`); }, @@ -84,32 +105,58 @@ export function createServiceRef(options: { }, $$ref: 'service', // TODO: declare __defaultFactory: defaultFactory, - } as InternalServiceRef; + } as ServiceRef & { + __defaultFactory?: ( + service: ServiceRef, + ) => Promise | (() => ServiceFactory)>; + }; } +type OnlyRootScopeDependencies< + TDeps extends { [key in string]: ServiceRef }, +> = Pick< + TDeps, + { + [name in keyof TDeps]: TDeps[name]['scope'] extends 'root' ? name : never; + }[keyof TDeps] +>; + +type DependencyRefsToInstances< + T extends { [key in string]: ServiceRef }, +> = { + [key in keyof T]: T[key] extends ServiceRef ? TImpl : never; +}; + /** * @public */ export function createServiceFactory< TService, + TScope extends 'root' | 'plugin', TImpl extends TService, - TDeps extends { [name in string]: unknown }, + TDeps extends { [name in string]: ServiceRef }, TOpts extends { [name in string]: unknown } | undefined = undefined, ->(factory: { - service: ServiceRef; - deps: TypesToServiceRef; +>(config: { + service: ServiceRef; + deps: TDeps; factory( - deps: DepsToDepFactories, + deps: DependencyRefsToInstances>, options: TOpts, - ): Promise>; + ): TScope extends 'root' + ? Promise + : Promise<(deps: DependencyRefsToInstances) => Promise>; }): undefined extends TOpts ? (options?: TOpts) => ServiceFactory : (options: TOpts) => ServiceFactory { - return (options?: TOpts) => ({ - service: factory.service, - deps: factory.deps, - factory(deps: DepsToDepFactories) { - return factory.factory(deps, options!); - }, - }); + return (options?: TOpts) => + ({ + scope: config.service.scope, + service: config.service, + deps: config.deps, + factory( + deps: DependencyRefsToInstances>, + ) { + return config.factory(deps, options!); + }, + } as ServiceFactory); } From 4b324c83f8bbc62f191139735cf9e4d43a100e54 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Thu, 8 Sep 2022 16:09:42 +0200 Subject: [PATCH 02/10] backend-app-api: 100% coverage of ServiceRegistry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../src/wiring/ServiceRegistry.test.ts | 72 ++++++++++++++++--- 1 file changed, 64 insertions(+), 8 deletions(-) diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts index ab709dcb77..211f9ac31e 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts @@ -18,6 +18,7 @@ import { createServiceRef, createServiceFactory, ServiceRef, + pluginMetadataServiceRef, } from '@backstage/backend-plugin-api'; import { ServiceRegistry } from './ServiceRegistry'; @@ -35,26 +36,23 @@ const sf1 = createServiceFactory({ }); const ref2 = createServiceRef<{ x: number }>({ + scope: 'root', id: '2', }); const sf2 = createServiceFactory({ service: ref2, deps: {}, factory: async () => { - return async () => { - return { x: 2 }; - }; + return { x: 2 }; }, }); const sf2b = createServiceFactory({ service: ref2, deps: {}, factory: async () => { - return async () => { - return { x: 22 }; - }; + return { x: 22 }; }, -}); +})(); const refDefault1 = createServiceRef<{ x: number }>({ id: '1', @@ -63,7 +61,7 @@ const refDefault1 = createServiceRef<{ x: number }>({ service, deps: {}, factory: async () => async () => ({ x: 10 }), - }), + })(), }); const refDefault2a = createServiceRef<{ x: number }>({ @@ -121,6 +119,64 @@ describe('ServiceRegistry', () => { ); }); + it('should not be possible for root scoped services to depend on plugin scoped services', async () => { + const factory = createServiceFactory({ + service: ref2, + deps: { pluginDep: ref1 }, + factory: async () => { + return { x: 2 }; + }, + }); + 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'.", + ); + }); + + it('should be possible for plugin scoped services to depend on root scoped services', async () => { + const factory = createServiceFactory({ + service: ref1, + deps: { rootDep: ref2 }, + factory: async ({ rootDep }) => { + return async () => ({ x: rootDep.x }); + }, + }); + const registry = new ServiceRegistry([factory, sf2]); + await expect(registry.get(ref1, 'catalog')).resolves.toEqual({ + x: 2, + }); + }); + + it('should be possible for root scoped services to depend on root scoped services', async () => { + const ref = createServiceRef<{ x: number }>({ id: 'x', scope: 'root' }); + const factory = createServiceFactory({ + service: ref, + deps: { rootDep: ref2 }, + factory: async ({ rootDep }) => { + return { x: rootDep.x }; + }, + }); + const registry = new ServiceRegistry([factory, sf2]); + await expect(registry.get(ref, 'catalog')).resolves.toEqual({ + x: 2, + }); + }); + + it('should return the pluginId from the pluginMetadata service', async () => { + const ref = createServiceRef<{ pluginId: string }>({ id: 'x' }); + const factory = createServiceFactory({ + service: ref, + deps: { meta: pluginMetadataServiceRef }, + factory: async ({}) => { + return async ({ meta }) => ({ pluginId: meta.getId() }); + }, + }); + const registry = new ServiceRegistry([factory]); + await expect(registry.get(ref, 'catalog')).resolves.toEqual({ + pluginId: 'catalog', + }); + }); + it('should use the last factory for each ref', async () => { const registry = new ServiceRegistry([sf2, sf2b]); await expect(registry.get(ref2, 'catalog')).resolves.toEqual({ From dc74ebd10d57f8f290db50c172aad3435e46e6bf Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 15:29:44 +0200 Subject: [PATCH 03/10] backend-plugin-api: refactor createServiceFactory helper types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../src/services/system/types.ts | 24 +++++++------------ 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/packages/backend-plugin-api/src/services/system/types.ts b/packages/backend-plugin-api/src/services/system/types.ts index 4a9f91541a..579d7abaab 100644 --- a/packages/backend-plugin-api/src/services/system/types.ts +++ b/packages/backend-plugin-api/src/services/system/types.ts @@ -112,19 +112,13 @@ export function createServiceRef(options: { }; } -type OnlyRootScopeDependencies< - TDeps extends { [key in string]: ServiceRef }, -> = Pick< - TDeps, - { - [name in keyof TDeps]: TDeps[name]['scope'] extends 'root' ? name : never; - }[keyof TDeps] ->; - -type DependencyRefsToInstances< +type ServiceRefsToInstances< T extends { [key in string]: ServiceRef }, + TScope extends 'root' | 'plugin' = 'root' | 'plugin', > = { - [key in keyof T]: T[key] extends ServiceRef ? TImpl : never; + [key in keyof T]: T[key] extends ServiceRef + ? TImpl + : never; }; /** @@ -140,11 +134,11 @@ export function createServiceFactory< service: ServiceRef; deps: TDeps; factory( - deps: DependencyRefsToInstances>, + deps: ServiceRefsToInstances, options: TOpts, ): TScope extends 'root' ? Promise - : Promise<(deps: DependencyRefsToInstances) => Promise>; + : Promise<(deps: ServiceRefsToInstances) => Promise>; }): undefined extends TOpts ? (options?: TOpts) => ServiceFactory : (options: TOpts) => ServiceFactory { @@ -153,9 +147,7 @@ export function createServiceFactory< scope: config.service.scope, service: config.service, deps: config.deps, - factory( - deps: DependencyRefsToInstances>, - ) { + factory(deps: ServiceRefsToInstances) { return config.factory(deps, options!); }, } as ServiceFactory); From 2bf8855c1dc37ceb21dda11c1590795a96e8fa1e Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 15:30:08 +0200 Subject: [PATCH 04/10] core-app-api: switch to non-broken way of defining service factories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: blam Co-authored-by: Fredrik Adelöw Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../src/wiring/ServiceRegistry.test.ts | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts index 211f9ac31e..897ed4e3cd 100644 --- a/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts +++ b/packages/backend-app-api/src/wiring/ServiceRegistry.test.ts @@ -28,7 +28,7 @@ const ref1 = createServiceRef<{ x: number }>({ const sf1 = createServiceFactory({ service: ref1, deps: {}, - factory: async () => { + async factory() { return async () => { return { x: 1 }; }; @@ -42,14 +42,14 @@ const ref2 = createServiceRef<{ x: number }>({ const sf2 = createServiceFactory({ service: ref2, deps: {}, - factory: async () => { + async factory() { return { x: 2 }; }, }); const sf2b = createServiceFactory({ service: ref2, deps: {}, - factory: async () => { + async factory() { return { x: 22 }; }, })(); @@ -60,7 +60,9 @@ const refDefault1 = createServiceRef<{ x: number }>({ createServiceFactory({ service, deps: {}, - factory: async () => async () => ({ x: 10 }), + async factory() { + return async () => ({ x: 10 }); + }, })(), }); @@ -70,7 +72,9 @@ const refDefault2a = createServiceRef<{ x: number }>({ createServiceFactory({ service, deps: {}, - factory: async () => async () => ({ x: 20 }), + async factory() { + return async () => ({ x: 20 }); + }, }), }); @@ -80,7 +84,9 @@ const refDefault2b = createServiceRef<{ x: number }>({ createServiceFactory({ service, deps: {}, - factory: async () => async () => ({ x: 220 }), + async factory() { + return async () => ({ x: 220 }); + }, }), }); @@ -123,7 +129,7 @@ describe('ServiceRegistry', () => { const factory = createServiceFactory({ service: ref2, deps: { pluginDep: ref1 }, - factory: async () => { + async factory() { return { x: 2 }; }, }); @@ -137,7 +143,7 @@ describe('ServiceRegistry', () => { const factory = createServiceFactory({ service: ref1, deps: { rootDep: ref2 }, - factory: async ({ rootDep }) => { + async factory({ rootDep }) { return async () => ({ x: rootDep.x }); }, }); @@ -152,7 +158,7 @@ describe('ServiceRegistry', () => { const factory = createServiceFactory({ service: ref, deps: { rootDep: ref2 }, - factory: async ({ rootDep }) => { + async factory({ rootDep }) { return { x: rootDep.x }; }, }); @@ -167,7 +173,7 @@ describe('ServiceRegistry', () => { const factory = createServiceFactory({ service: ref, deps: { meta: pluginMetadataServiceRef }, - factory: async ({}) => { + async factory() { return async ({ meta }) => ({ pluginId: meta.getId() }); }, }); @@ -222,7 +228,9 @@ describe('ServiceRegistry', () => { createServiceFactory({ service, deps: {}, - factory: async () => async () => {}, + async factory() { + return async () => {}; + }, }), ); const ref = createServiceRef({ From f55c11f29d55cf2639b3d75976167f1506274cfa Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 15:34:29 +0200 Subject: [PATCH 05/10] core-plugin-api: narrow scope type of service refs passed to default factory loader Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- .../src/services/system/types.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/backend-plugin-api/src/services/system/types.ts b/packages/backend-plugin-api/src/services/system/types.ts index 579d7abaab..e52ef5cc5a 100644 --- a/packages/backend-plugin-api/src/services/system/types.ts +++ b/packages/backend-plugin-api/src/services/system/types.ts @@ -76,23 +76,27 @@ export function createServiceRef(options: { id: string; scope?: 'plugin'; defaultFactory?: ( - service: ServiceRef, + service: ServiceRef, ) => Promise | (() => ServiceFactory)>; }): ServiceRef; export function createServiceRef(options: { id: string; scope: 'root'; defaultFactory?: ( - service: ServiceRef, + service: ServiceRef, ) => Promise | (() => ServiceFactory)>; }): ServiceRef; export function createServiceRef(options: { id: string; scope?: 'plugin' | 'root'; - defaultFactory?: ( - service: ServiceRef, - ) => Promise | (() => ServiceFactory)>; -}): ServiceRef { + defaultFactory?: + | (( + service: ServiceRef, + ) => Promise | (() => ServiceFactory)>) + | (( + service: ServiceRef, + ) => Promise | (() => ServiceFactory)>); +}): ServiceRef { const { id, scope = 'plugin', defaultFactory } = options; return { id, From 06c744d142d7d9e8112dfe897ae0c3028560437a Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 15:35:21 +0200 Subject: [PATCH 06/10] backend-plugin-api: add initial PluginMetadataService definition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Fredrik Adelöw Co-authored-by: blam Co-authored-by: Johan Haals Signed-off-by: Patrik Oldsberg --- .../src/services/definitions/index.ts | 1 + .../definitions/pluginMetadataServiceRef.ts | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 packages/backend-plugin-api/src/services/definitions/pluginMetadataServiceRef.ts diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index 797cfb5a76..fb204df495 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -26,3 +26,4 @@ export { discoveryServiceRef } from './discoveryServiceRef'; export { tokenManagerServiceRef } from './tokenManagerServiceRef'; export { permissionsServiceRef } from './permissionsServiceRef'; export { schedulerServiceRef } from './schedulerServiceRef'; +export { pluginMetadataServiceRef } from './pluginMetadataServiceRef'; diff --git a/packages/backend-plugin-api/src/services/definitions/pluginMetadataServiceRef.ts b/packages/backend-plugin-api/src/services/definitions/pluginMetadataServiceRef.ts new file mode 100644 index 0000000000..3af6e54900 --- /dev/null +++ b/packages/backend-plugin-api/src/services/definitions/pluginMetadataServiceRef.ts @@ -0,0 +1,31 @@ +/* + * 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 { createServiceRef } from '../system/types'; + +/** + * @public + */ +export interface PluginMetadata { + getId(): string; +} + +/** + * @public + */ +export const pluginMetadataServiceRef = createServiceRef({ + id: 'core.plugin-metadata', +}); From fb93ecad9cf98f0410fb7341eff7d427347b1ab2 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 15:52:03 +0200 Subject: [PATCH 07/10] backend-plugin-api: remove services with scope mismatch from deps type Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- packages/backend-plugin-api/src/services/system/types.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/backend-plugin-api/src/services/system/types.ts b/packages/backend-plugin-api/src/services/system/types.ts index e52ef5cc5a..3edee2ec0f 100644 --- a/packages/backend-plugin-api/src/services/system/types.ts +++ b/packages/backend-plugin-api/src/services/system/types.ts @@ -120,9 +120,9 @@ type ServiceRefsToInstances< T extends { [key in string]: ServiceRef }, TScope extends 'root' | 'plugin' = 'root' | 'plugin', > = { - [key in keyof T]: T[key] extends ServiceRef - ? TImpl - : never; + [name in { + [key in keyof T]: T[key] extends ServiceRef ? key : never; + }[keyof T]]: T[name] extends ServiceRef ? TImpl : never; }; /** From 2a29d24519cd5ae64e6787eb55beaa4ff7665ca5 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 16:10:59 +0200 Subject: [PATCH 08/10] backend-app-api,backend-plugin-api: refactor existing service factories and fix type issues Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- .../services/implementations/cacheService.ts | 11 +++-- .../services/implementations/configService.ts | 11 ++--- .../implementations/databaseService.ts | 11 +++-- .../implementations/discoveryService.ts | 5 +- .../implementations/httpRouterService.ts | 11 +++-- .../services/implementations/loggerService.ts | 33 ++++--------- .../implementations/permissionsService.ts | 22 ++++----- .../implementations/rootLoggerService.ts | 48 +++++++++++++++++++ .../implementations/schedulerService.ts | 11 +++-- .../implementations/tokenManagerService.ts | 29 ++--------- .../implementations/urlReaderService.ts | 11 ++--- .../src/wiring/BackendInitializer.ts | 7 +-- .../services/definitions/configServiceRef.ts | 3 +- .../src/services/definitions/index.ts | 1 + .../definitions/rootLoggerServiceRef.ts | 26 ++++++++++ .../src/services/system/index.ts | 8 +--- .../src/services/system/types.ts | 1 + plugins/catalog-node/src/catalogService.ts | 10 ++-- 18 files changed, 145 insertions(+), 114 deletions(-) create mode 100644 packages/backend-app-api/src/services/implementations/rootLoggerService.ts create mode 100644 packages/backend-plugin-api/src/services/definitions/rootLoggerServiceRef.ts diff --git a/packages/backend-app-api/src/services/implementations/cacheService.ts b/packages/backend-app-api/src/services/implementations/cacheService.ts index c5e58b4a41..7e15f031c6 100644 --- a/packages/backend-app-api/src/services/implementations/cacheService.ts +++ b/packages/backend-app-api/src/services/implementations/cacheService.ts @@ -18,6 +18,7 @@ import { CacheManager } from '@backstage/backend-common'; import { configServiceRef, createServiceFactory, + pluginMetadataServiceRef, cacheServiceRef, } from '@backstage/backend-plugin-api'; @@ -25,13 +26,13 @@ import { export const cacheFactory = createServiceFactory({ service: cacheServiceRef, deps: { - configFactory: configServiceRef, + config: configServiceRef, + plugin: pluginMetadataServiceRef, }, - factory: async ({ configFactory }) => { - const config = await configFactory('root'); + async factory({ config }) { const cacheManager = CacheManager.fromConfig(config); - return async (pluginId: string) => { - return cacheManager.forPlugin(pluginId); + return async ({ plugin }) => { + return cacheManager.forPlugin(plugin.getId()); }; }, }); diff --git a/packages/backend-app-api/src/services/implementations/configService.ts b/packages/backend-app-api/src/services/implementations/configService.ts index c4aa641f72..91327289df 100644 --- a/packages/backend-app-api/src/services/implementations/configService.ts +++ b/packages/backend-app-api/src/services/implementations/configService.ts @@ -19,23 +19,20 @@ import { configServiceRef, createServiceFactory, loggerToWinstonLogger, - loggerServiceRef, + rootLoggerServiceRef, } from '@backstage/backend-plugin-api'; /** @public */ export const configFactory = createServiceFactory({ service: configServiceRef, deps: { - loggerFactory: loggerServiceRef, + logger: rootLoggerServiceRef, }, - factory: async ({ loggerFactory }) => { - const logger = await loggerFactory('root'); + async factory({ logger }) { const config = await loadBackendConfig({ argv: process.argv, logger: loggerToWinstonLogger(logger), }); - return async () => { - return config; - }; + return config; }, }); diff --git a/packages/backend-app-api/src/services/implementations/databaseService.ts b/packages/backend-app-api/src/services/implementations/databaseService.ts index b2bc19de84..f6401528e6 100644 --- a/packages/backend-app-api/src/services/implementations/databaseService.ts +++ b/packages/backend-app-api/src/services/implementations/databaseService.ts @@ -19,19 +19,20 @@ import { configServiceRef, createServiceFactory, databaseServiceRef, + pluginMetadataServiceRef, } from '@backstage/backend-plugin-api'; /** @public */ export const databaseFactory = createServiceFactory({ service: databaseServiceRef, deps: { - configFactory: configServiceRef, + config: configServiceRef, + plugin: pluginMetadataServiceRef, }, - factory: async ({ configFactory }) => { - const config = await configFactory('root'); + async factory({ config }) { const databaseManager = DatabaseManager.fromConfig(config); - return async (pluginId: string) => { - return databaseManager.forPlugin(pluginId); + return async ({ plugin }) => { + return databaseManager.forPlugin(plugin.getId()); }; }, }); diff --git a/packages/backend-app-api/src/services/implementations/discoveryService.ts b/packages/backend-app-api/src/services/implementations/discoveryService.ts index 3f1a584c61..7f35bf2447 100644 --- a/packages/backend-app-api/src/services/implementations/discoveryService.ts +++ b/packages/backend-app-api/src/services/implementations/discoveryService.ts @@ -25,10 +25,9 @@ import { export const discoveryFactory = createServiceFactory({ service: discoveryServiceRef, deps: { - configFactory: configServiceRef, + config: configServiceRef, }, - factory: async ({ configFactory }) => { - const config = await configFactory('root'); + async factory({ config }) { const discovery = SingleHostDiscovery.fromConfig(config); return async () => { return discovery; diff --git a/packages/backend-app-api/src/services/implementations/httpRouterService.ts b/packages/backend-app-api/src/services/implementations/httpRouterService.ts index 6460a77eaf..be4af664c4 100644 --- a/packages/backend-app-api/src/services/implementations/httpRouterService.ts +++ b/packages/backend-app-api/src/services/implementations/httpRouterService.ts @@ -18,6 +18,7 @@ import { createServiceFactory, httpRouterServiceRef, configServiceRef, + pluginMetadataServiceRef, } from '@backstage/backend-plugin-api'; import Router from 'express-promise-router'; import { Handler } from 'express'; @@ -27,18 +28,20 @@ import { createServiceBuilder } from '@backstage/backend-common'; export const httpRouterFactory = createServiceFactory({ service: httpRouterServiceRef, deps: { - configFactory: configServiceRef, + config: configServiceRef, + plugin: pluginMetadataServiceRef, }, - factory: async ({ configFactory }) => { + async factory({ config }) { const rootRouter = Router(); const service = createServiceBuilder(module) - .loadConfig(await configFactory('root')) + .loadConfig(config) .addRouter('', rootRouter); await service.start(); - return async (pluginId?: string) => { + return async ({ plugin }) => { + const pluginId = plugin.getId(); const path = pluginId ? `/api/${pluginId}` : ''; return { use(handler: Handler) { diff --git a/packages/backend-app-api/src/services/implementations/loggerService.ts b/packages/backend-app-api/src/services/implementations/loggerService.ts index e90b591302..ff72020140 100644 --- a/packages/backend-app-api/src/services/implementations/loggerService.ts +++ b/packages/backend-app-api/src/services/implementations/loggerService.ts @@ -14,38 +14,23 @@ * limitations under the License. */ -import { createRootLogger } from '@backstage/backend-common'; import { createServiceFactory, - Logger, loggerServiceRef, + pluginMetadataServiceRef, + rootLoggerServiceRef, } from '@backstage/backend-plugin-api'; -import { Logger as WinstonLogger } from 'winston'; - -class BackstageLogger implements Logger { - static fromWinston(logger: WinstonLogger): BackstageLogger { - return new BackstageLogger(logger); - } - - private constructor(private readonly winston: WinstonLogger) {} - - info(message: string, ...meta: any[]): void { - this.winston.info(message, ...meta); - } - - child(fields: { [name: string]: string }): Logger { - return new BackstageLogger(this.winston.child(fields)); - } -} /** @public */ export const loggerFactory = createServiceFactory({ service: loggerServiceRef, - deps: {}, - factory: async () => { - const root = BackstageLogger.fromWinston(createRootLogger()); - return async (pluginId: string) => { - return root.child({ pluginId }); + deps: { + rootLogger: rootLoggerServiceRef, + plugin: pluginMetadataServiceRef, + }, + async factory({ rootLogger }) { + return async ({ plugin }) => { + return rootLogger.child({ pluginId: plugin.getId() }); }; }, }); diff --git a/packages/backend-app-api/src/services/implementations/permissionsService.ts b/packages/backend-app-api/src/services/implementations/permissionsService.ts index 26fe012a20..32e8a1a9f9 100644 --- a/packages/backend-app-api/src/services/implementations/permissionsService.ts +++ b/packages/backend-app-api/src/services/implementations/permissionsService.ts @@ -27,20 +27,16 @@ import { ServerPermissionClient } from '@backstage/plugin-permission-node'; export const permissionsFactory = createServiceFactory({ service: permissionsServiceRef, deps: { - configFactory: configServiceRef, - discoveryFactory: discoveryServiceRef, - tokenManagerFactory: tokenManagerServiceRef, + config: configServiceRef, + discovery: discoveryServiceRef, + tokenManager: tokenManagerServiceRef, }, - factory: async ({ configFactory, discoveryFactory, tokenManagerFactory }) => { - const config = await configFactory('root'); - const discovery = await discoveryFactory('root'); - const tokenManager = await tokenManagerFactory('root'); - const permissions = ServerPermissionClient.fromConfig(config, { - discovery, - tokenManager, - }); - return async (_pluginId: string) => { - return permissions; + async factory({ config }) { + return async ({ discovery, tokenManager }) => { + return ServerPermissionClient.fromConfig(config, { + discovery, + tokenManager, + }); }; }, }); diff --git a/packages/backend-app-api/src/services/implementations/rootLoggerService.ts b/packages/backend-app-api/src/services/implementations/rootLoggerService.ts new file mode 100644 index 0000000000..d7da11723e --- /dev/null +++ b/packages/backend-app-api/src/services/implementations/rootLoggerService.ts @@ -0,0 +1,48 @@ +/* + * 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 { createRootLogger } from '@backstage/backend-common'; +import { + createServiceFactory, + Logger, + rootLoggerServiceRef, +} from '@backstage/backend-plugin-api'; +import { Logger as WinstonLogger } from 'winston'; + +class BackstageLogger implements Logger { + static fromWinston(logger: WinstonLogger): BackstageLogger { + return new BackstageLogger(logger); + } + + private constructor(private readonly winston: WinstonLogger) {} + + info(message: string, ...meta: any[]): void { + this.winston.info(message, ...meta); + } + + child(fields: { [name: string]: string }): Logger { + return new BackstageLogger(this.winston.child(fields)); + } +} + +/** @public */ +export const loggerFactory = createServiceFactory({ + service: rootLoggerServiceRef, + deps: {}, + async factory() { + return BackstageLogger.fromWinston(createRootLogger()); + }, +}); diff --git a/packages/backend-app-api/src/services/implementations/schedulerService.ts b/packages/backend-app-api/src/services/implementations/schedulerService.ts index 39dbf26ba9..40676344ec 100644 --- a/packages/backend-app-api/src/services/implementations/schedulerService.ts +++ b/packages/backend-app-api/src/services/implementations/schedulerService.ts @@ -17,6 +17,7 @@ import { configServiceRef, createServiceFactory, + pluginMetadataServiceRef, schedulerServiceRef, } from '@backstage/backend-plugin-api'; import { TaskScheduler } from '@backstage/backend-tasks'; @@ -25,13 +26,13 @@ import { TaskScheduler } from '@backstage/backend-tasks'; export const schedulerFactory = createServiceFactory({ service: schedulerServiceRef, deps: { - configFactory: configServiceRef, + config: configServiceRef, + plugin: pluginMetadataServiceRef, }, - factory: async ({ configFactory }) => { - const config = await configFactory('root'); + async factory({ config }) { const taskScheduler = TaskScheduler.fromConfig(config); - return async (pluginId: string) => { - return taskScheduler.forPlugin(pluginId); + return async ({ plugin }) => { + return taskScheduler.forPlugin(plugin.getId()); }; }, }); diff --git a/packages/backend-app-api/src/services/implementations/tokenManagerService.ts b/packages/backend-app-api/src/services/implementations/tokenManagerService.ts index 7767c17944..92f42c10db 100644 --- a/packages/backend-app-api/src/services/implementations/tokenManagerService.ts +++ b/packages/backend-app-api/src/services/implementations/tokenManagerService.ts @@ -27,32 +27,11 @@ import { ServerTokenManager } from '@backstage/backend-common'; export const tokenManagerFactory = createServiceFactory({ service: tokenManagerServiceRef, deps: { - configFactory: configServiceRef, - loggerFactory: loggerServiceRef, + config: configServiceRef, + logger: loggerServiceRef, }, - factory: async ({ configFactory, loggerFactory }) => { - const logger = await loggerFactory('root'); - const config = await configFactory('root'); - return async (_pluginId: string) => { - // doesn't the logger want to be inferred from the plugin tho here? - // maybe ... also why do we recreate it every time otherwise - // we should memoize on a per plugin right? so I think it's should be fine to re-use the plugin one - // we shouldn't recreate on a per plugin basis. - // hm - on the other hand, is this really ever called more than once? - // not this function right. should only be called when the plugin requests this serviceRef - // yeah so no need to worry about memo probably - // but we still want to scope the logger to the ServrTokenmanagfer>? - // mm sure maybe - // maybe in this case it doesn't provide so much value b - // oh hang on - isn't it up to THE MANAGER to make a child internally if it wants to do that - // so that it becomes a property intrinsic to that class, no matter how it's constructed - // or is that too much responsibility for it - making the constructor complex so to speak, making it harder to tweak that behavior - // this is not ultra efficient :) - - // I think the naming here is wrong to be gonest - // this isn't like the cache manager or the database manager - // the manager name is confusuion i think - // aye perhaps + async factory() { + return async ({ config, logger }) => { return ServerTokenManager.fromConfig(config, { logger: loggerToWinstonLogger(logger), }); diff --git a/packages/backend-app-api/src/services/implementations/urlReaderService.ts b/packages/backend-app-api/src/services/implementations/urlReaderService.ts index df353a52d2..d7d7502a08 100644 --- a/packages/backend-app-api/src/services/implementations/urlReaderService.ts +++ b/packages/backend-app-api/src/services/implementations/urlReaderService.ts @@ -27,15 +27,14 @@ import { export const urlReaderFactory = createServiceFactory({ service: urlReaderServiceRef, deps: { - configFactory: configServiceRef, - loggerFactory: loggerServiceRef, + config: configServiceRef, + logger: loggerServiceRef, }, - factory: async ({ configFactory, loggerFactory }) => { - return async (pluginId: string) => { - const logger = await loggerFactory(pluginId); + async factory() { + return async ({ config, logger }) => { return UrlReaders.default({ + config, logger: loggerToWinstonLogger(logger), - config: await configFactory(pluginId), }); }; }, diff --git a/packages/backend-app-api/src/wiring/BackendInitializer.ts b/packages/backend-app-api/src/wiring/BackendInitializer.ts index afb5250504..c86cd8382d 100644 --- a/packages/backend-app-api/src/wiring/BackendInitializer.ts +++ b/packages/backend-app-api/src/wiring/BackendInitializer.ts @@ -50,11 +50,12 @@ export class BackendInitializer { if (extensionPoint) { result.set(name, extensionPoint); } else { - const factory = await this.#serviceHolder.get( + const impl = await this.#serviceHolder.get( ref as ServiceRef, + pluginId, ); - if (factory) { - result.set(name, await factory(pluginId)); + if (impl) { + result.set(name, impl); } else { missingRefs.add(ref); } diff --git a/packages/backend-plugin-api/src/services/definitions/configServiceRef.ts b/packages/backend-plugin-api/src/services/definitions/configServiceRef.ts index ba5dcacdc8..f17c5f57bc 100644 --- a/packages/backend-plugin-api/src/services/definitions/configServiceRef.ts +++ b/packages/backend-plugin-api/src/services/definitions/configServiceRef.ts @@ -21,5 +21,6 @@ import { createServiceRef } from '../system/types'; * @public */ export const configServiceRef = createServiceRef({ - id: 'core.config', + id: 'core.root.config', + scope: 'root', }); diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index fb204df495..44ccb62978 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -26,4 +26,5 @@ export { discoveryServiceRef } from './discoveryServiceRef'; export { tokenManagerServiceRef } from './tokenManagerServiceRef'; export { permissionsServiceRef } from './permissionsServiceRef'; export { schedulerServiceRef } from './schedulerServiceRef'; +export { rootLoggerServiceRef } from './rootLoggerServiceRef'; export { pluginMetadataServiceRef } from './pluginMetadataServiceRef'; diff --git a/packages/backend-plugin-api/src/services/definitions/rootLoggerServiceRef.ts b/packages/backend-plugin-api/src/services/definitions/rootLoggerServiceRef.ts new file mode 100644 index 0000000000..62e22c53d9 --- /dev/null +++ b/packages/backend-plugin-api/src/services/definitions/rootLoggerServiceRef.ts @@ -0,0 +1,26 @@ +/* + * 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 { createServiceRef } from '../system/types'; +import { Logger } from './loggerServiceRef'; + +/** + * @public + */ +export const rootLoggerServiceRef = createServiceRef({ + id: 'core.root.logger', + scope: 'root', +}); diff --git a/packages/backend-plugin-api/src/services/system/index.ts b/packages/backend-plugin-api/src/services/system/index.ts index 817b0a590f..8c666af42e 100644 --- a/packages/backend-plugin-api/src/services/system/index.ts +++ b/packages/backend-plugin-api/src/services/system/index.ts @@ -14,11 +14,5 @@ * limitations under the License. */ -export type { - ServiceRef, - TypesToServiceRef, - DepsToDepFactories, - FactoryFunc, - ServiceFactory, -} from './types'; +export type { ServiceRef, TypesToServiceRef, ServiceFactory } from './types'; export { createServiceRef, createServiceFactory } from './types'; diff --git a/packages/backend-plugin-api/src/services/system/types.ts b/packages/backend-plugin-api/src/services/system/types.ts index 3edee2ec0f..a7bce3d220 100644 --- a/packages/backend-plugin-api/src/services/system/types.ts +++ b/packages/backend-plugin-api/src/services/system/types.ts @@ -116,6 +116,7 @@ export function createServiceRef(options: { }; } +/** @ignore */ type ServiceRefsToInstances< T extends { [key in string]: ServiceRef }, TScope extends 'root' | 'plugin' = 'root' | 'plugin', diff --git a/plugins/catalog-node/src/catalogService.ts b/plugins/catalog-node/src/catalogService.ts index ddfff75462..c6a8fb44f5 100644 --- a/plugins/catalog-node/src/catalogService.ts +++ b/plugins/catalog-node/src/catalogService.ts @@ -31,13 +31,11 @@ export const catalogServiceRef = createServiceRef({ createServiceFactory({ service, deps: { - discoveryFactory: discoveryServiceRef, + discoveryApi: discoveryServiceRef, }, - factory: async ({ discoveryFactory }) => { - const discoveryApi = await discoveryFactory('root'); - const catalogClient = new CatalogClient({ discoveryApi }); - return async _pluginId => { - return catalogClient; + async factory() { + return async ({ discoveryApi }) => { + return new CatalogClient({ discoveryApi }); }; }, }), From 06ad1b16de01f67d529e202e0aed997fd901b964 Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 16:15:09 +0200 Subject: [PATCH 09/10] update API reports + fixes Co-authored-by: blam Signed-off-by: Patrik Oldsberg --- packages/backend-plugin-api/api-report.md | 108 ++++++++++++------ .../src/services/definitions/index.ts | 1 + .../src/services/system/types.ts | 5 +- plugins/catalog-node/api-report.md | 2 +- 4 files changed, 77 insertions(+), 39 deletions(-) diff --git a/packages/backend-plugin-api/api-report.md b/packages/backend-plugin-api/api-report.md index 30e58237af..3344576db8 100644 --- a/packages/backend-plugin-api/api-report.md +++ b/packages/backend-plugin-api/api-report.md @@ -66,10 +66,10 @@ export interface BackendRegistrationPoints { } // @public (undocumented) -export const cacheServiceRef: ServiceRef; +export const cacheServiceRef: ServiceRef; // @public (undocumented) -export const configServiceRef: ServiceRef; +export const configServiceRef: ServiceRef; // @public (undocumented) export function createBackendModule< @@ -105,22 +105,25 @@ export function createExtensionPoint(options: { // @public (undocumented) export function createServiceFactory< TService, + TScope extends 'root' | 'plugin', TImpl extends TService, TDeps extends { - [name in string]: unknown; + [name in string]: ServiceRef; }, TOpts extends | { [name in string]: unknown; } | undefined = undefined, ->(factory: { - service: ServiceRef; - deps: TypesToServiceRef; +>(config: { + service: ServiceRef; + deps: TDeps; factory( - deps: DepsToDepFactories, + deps: ServiceRefsToInstances, options: TOpts, - ): Promise>; + ): TScope extends 'root' + ? Promise + : Promise<(deps: ServiceRefsToInstances) => Promise>; }): undefined extends TOpts ? (options?: TOpts) => ServiceFactory : (options: TOpts) => ServiceFactory; @@ -128,21 +131,26 @@ export function createServiceFactory< // @public (undocumented) export function createServiceRef(options: { id: string; + scope?: 'plugin'; defaultFactory?: ( - service: ServiceRef, + service: ServiceRef, ) => Promise | (() => ServiceFactory)>; -}): ServiceRef; +}): ServiceRef; // @public (undocumented) -export const databaseServiceRef: ServiceRef; +export function createServiceRef(options: { + id: string; + scope: 'root'; + defaultFactory?: ( + service: ServiceRef, + ) => Promise | (() => ServiceFactory)>; +}): ServiceRef; // @public (undocumented) -export type DepsToDepFactories = { - [key in keyof T]: (pluginId: string) => Promise; -}; +export const databaseServiceRef: ServiceRef; // @public (undocumented) -export const discoveryServiceRef: ServiceRef; +export const discoveryServiceRef: ServiceRef; // @public export type ExtensionPoint = { @@ -152,9 +160,6 @@ export type ExtensionPoint = { $$ref: 'extension-point'; }; -// @public (undocumented) -export type FactoryFunc = (pluginId: string) => Promise; - // @public (undocumented) export interface HttpRouterService { // (undocumented) @@ -162,7 +167,7 @@ export interface HttpRouterService { } // @public (undocumented) -export const httpRouterServiceRef: ServiceRef; +export const httpRouterServiceRef: ServiceRef; // @public (undocumented) export interface Logger { @@ -173,7 +178,7 @@ export interface Logger { } // @public (undocumented) -export const loggerServiceRef: ServiceRef; +export const loggerServiceRef: ServiceRef; // @public (undocumented) export function loggerToWinstonLogger( @@ -183,33 +188,66 @@ export function loggerToWinstonLogger( // @public (undocumented) export const permissionsServiceRef: ServiceRef< - PermissionAuthorizer | PermissionEvaluator + PermissionAuthorizer | PermissionEvaluator, + 'plugin' >; // @public (undocumented) -export const schedulerServiceRef: ServiceRef; +export interface PluginMetadata { + // (undocumented) + getId(): string; +} // @public (undocumented) -export type ServiceFactory = { - service: ServiceRef; - deps: { - [key in string]: ServiceRef; - }; - factory(deps: { - [key in string]: unknown; - }): Promise>; -}; +export const pluginMetadataServiceRef: ServiceRef; + +// @public (undocumented) +export const rootLoggerServiceRef: ServiceRef; + +// @public (undocumented) +export const schedulerServiceRef: ServiceRef; + +// @public (undocumented) +export type ServiceFactory = + | { + scope: 'root'; + service: ServiceRef; + deps: { + [key in string]: ServiceRef; + }; + factory(deps: { + [key in string]: unknown; + }): Promise; + } + | { + scope: 'plugin'; + service: ServiceRef; + deps: { + [key in string]: ServiceRef; + }; + factory(deps: { + [key in string]: unknown; + }): Promise< + (deps: { + [key in string]: unknown; + }) => Promise + >; + }; // @public -export type ServiceRef = { +export type ServiceRef< + TService, + TScope extends 'root' | 'plugin' = 'root' | 'plugin', +> = { id: string; - T: T; + scope: TScope; + T: TService; toString(): string; $$ref: 'service'; }; // @public (undocumented) -export const tokenManagerServiceRef: ServiceRef; +export const tokenManagerServiceRef: ServiceRef; // @public (undocumented) export type TypesToServiceRef = { @@ -217,5 +255,5 @@ export type TypesToServiceRef = { }; // @public (undocumented) -export const urlReaderServiceRef: ServiceRef; +export const urlReaderServiceRef: ServiceRef; ``` diff --git a/packages/backend-plugin-api/src/services/definitions/index.ts b/packages/backend-plugin-api/src/services/definitions/index.ts index 44ccb62978..e5f032ef60 100644 --- a/packages/backend-plugin-api/src/services/definitions/index.ts +++ b/packages/backend-plugin-api/src/services/definitions/index.ts @@ -28,3 +28,4 @@ export { permissionsServiceRef } from './permissionsServiceRef'; export { schedulerServiceRef } from './schedulerServiceRef'; export { rootLoggerServiceRef } from './rootLoggerServiceRef'; export { pluginMetadataServiceRef } from './pluginMetadataServiceRef'; +export type { PluginMetadata } from './pluginMetadataServiceRef'; diff --git a/packages/backend-plugin-api/src/services/system/types.ts b/packages/backend-plugin-api/src/services/system/types.ts index a7bce3d220..d20fd6fd8f 100644 --- a/packages/backend-plugin-api/src/services/system/types.ts +++ b/packages/backend-plugin-api/src/services/system/types.ts @@ -69,9 +69,7 @@ export type ServiceFactory = >; }; -/** - * @public - */ +/** @public */ export function createServiceRef(options: { id: string; scope?: 'plugin'; @@ -79,6 +77,7 @@ export function createServiceRef(options: { service: ServiceRef, ) => Promise | (() => ServiceFactory)>; }): ServiceRef; +/** @public */ export function createServiceRef(options: { id: string; scope: 'root'; diff --git a/plugins/catalog-node/api-report.md b/plugins/catalog-node/api-report.md index 3873e3376d..525516be54 100644 --- a/plugins/catalog-node/api-report.md +++ b/plugins/catalog-node/api-report.md @@ -109,7 +109,7 @@ export type CatalogProcessorResult = | CatalogProcessorRefreshKeysResult; // @alpha -export const catalogServiceRef: ServiceRef; +export const catalogServiceRef: ServiceRef; // @public export type DeferredEntity = { From 409ed984e8e506abc9327cb8381c2a12aa59b5ce Mon Sep 17 00:00:00 2001 From: Patrik Oldsberg Date: Fri, 9 Sep 2022 18:26:34 +0200 Subject: [PATCH 10/10] changesets: added changesets for scoped services Signed-off-by: Patrik Oldsberg --- .changeset/flat-humans-dance.md | 5 +++++ .changeset/quick-items-invite.md | 5 +++++ .changeset/slow-phones-count.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/flat-humans-dance.md create mode 100644 .changeset/quick-items-invite.md create mode 100644 .changeset/slow-phones-count.md diff --git a/.changeset/flat-humans-dance.md b/.changeset/flat-humans-dance.md new file mode 100644 index 0000000000..ea7571f5bc --- /dev/null +++ b/.changeset/flat-humans-dance.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-plugin-api': patch +--- + +Service are now scoped to either `'plugin'` or `'root'` scope. Service factories have been updated to provide dependency instances directly rather than factory functions. diff --git a/.changeset/quick-items-invite.md b/.changeset/quick-items-invite.md new file mode 100644 index 0000000000..33f90cb662 --- /dev/null +++ b/.changeset/quick-items-invite.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-catalog-node': patch +--- + +Updated usage of experimental backend service APIs. diff --git a/.changeset/slow-phones-count.md b/.changeset/slow-phones-count.md new file mode 100644 index 0000000000..ce2f9c8432 --- /dev/null +++ b/.changeset/slow-phones-count.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-app-api': patch +--- + +Updated service implementations and backend wiring to support scoped service.