diff --git a/framework/src/App.ts b/framework/src/App.ts index 3ea5c310ae..7ba191ffe6 100644 --- a/framework/src/App.ts +++ b/framework/src/App.ts @@ -1,10 +1,4 @@ -import { - AnyObject, - ArrayElement, - Constructor, - JovoLoggerConfig, - UnknownObject, -} from '@jovotech/common'; +import { AnyObject, ArrayElement, JovoLoggerConfig, UnknownObject } from '@jovotech/common'; import _merge from 'lodash.merge'; import { AppData, @@ -75,6 +69,7 @@ export interface AppConfig extends ExtensibleConfig { export type AppInitConfig = ExtensibleInitConfig & { components?: Array; + // eslint-disable-next-line @typescript-eslint/no-explicit-any providers?: Provider[]; }; diff --git a/framework/src/BaseComponent.ts b/framework/src/BaseComponent.ts index 6458eaf113..cc441ef192 100644 --- a/framework/src/BaseComponent.ts +++ b/framework/src/BaseComponent.ts @@ -12,9 +12,10 @@ export type ComponentConfig = Exclude< undefined >; -// eslint-disable-next-line @typescript-eslint/no-explicit-any export type ComponentConstructor< + // eslint-disable-next-line @typescript-eslint/no-explicit-any COMPONENT extends BaseComponent = any, + // eslint-disable-next-line @typescript-eslint/no-explicit-any ARGS extends unknown[] = any[], > = new ( jovo: Jovo, diff --git a/framework/src/ComponentTreeNode.ts b/framework/src/ComponentTreeNode.ts index 338a8c797a..a73114302f 100644 --- a/framework/src/ComponentTreeNode.ts +++ b/framework/src/ComponentTreeNode.ts @@ -69,7 +69,7 @@ export class ComponentTreeNode handler = BuiltInHandler.Start, callArgs, }: ExecuteHandlerOptions): Promise { - const componentInstance = this.instantiateComponent(jovo); + const componentInstance = await this.instantiateComponent(jovo); try { if (!componentInstance[handler as keyof COMPONENT]) { throw new HandlerNotFoundError(componentInstance.constructor.name, handler.toString()); @@ -88,8 +88,8 @@ export class ComponentTreeNode } } - private instantiateComponent(jovo: Jovo): COMPONENT { - return DependencyInjector.instantiateClass( + private async instantiateComponent(jovo: Jovo): Promise { + return await DependencyInjector.instantiateClass( jovo, this.metadata.target as ComponentConstructor, jovo, diff --git a/framework/src/DependencyInjector.ts b/framework/src/DependencyInjector.ts index e80f8d377b..c02c5af4a8 100644 --- a/framework/src/DependencyInjector.ts +++ b/framework/src/DependencyInjector.ts @@ -1,13 +1,30 @@ import { MetadataStorage } from './metadata/MetadataStorage'; import { Jovo } from './Jovo'; -import { AnyObject, Constructor } from '@jovotech/common'; +import { AnyObject, Constructor, ArrayElement } from '@jovotech/common'; import { InjectionToken, Provider } from './metadata/InjectableMetadata'; +import { CircularDependencyError } from './errors/CircularDependencyError'; + +const INSTANTIATE_DEPENDENCY_MIDDLEWARE = 'event.DependencyInjector.instantiateDependency'; + +export interface DependencyTree { + token: InjectionToken; + resolvedValue: Node; + children: DependencyTree< + // eslint-disable-next-line @typescript-eslint/no-explicit-any + Node extends Constructor ? ArrayElement : unknown + >[]; +} export class DependencyInjector { private static resolveInjectionToken( jovo: Jovo, token: InjectionToken, - ): TYPE | undefined { + dependencyPath: InjectionToken[], + ): DependencyTree { + if (dependencyPath.includes(token)) { + throw new CircularDependencyError(dependencyPath); + } + const updatedPath = [...dependencyPath, token]; // eslint-disable-next-line @typescript-eslint/no-explicit-any const providers: Provider[] = [ ...jovo.$app.providers, @@ -24,30 +41,70 @@ export class DependencyInjector { } }) as Provider | undefined; if (!injection) { - return undefined; + return { + token, + resolvedValue: undefined, + children: [], + }; } + if (typeof injection === 'function') { - return DependencyInjector.instantiateClass(jovo, injection); + return DependencyInjector.instantiateClassWithTracing(jovo, injection, updatedPath); } else if ('useValue' in injection) { - return injection.useValue; + return { + token, + resolvedValue: injection.useValue, + children: [], + }; } else if ('useFactory' in injection) { - return injection.useFactory(jovo); + const value = injection.useFactory(jovo); + return { + token, + resolvedValue: value, + children: [], + }; } else if ('useClass' in injection) { - return DependencyInjector.instantiateClass(jovo, injection.useClass); + const tree = DependencyInjector.instantiateClassWithTracing( + jovo, + injection.useClass, + updatedPath, + ); + // insert proper token + return { + ...tree, + token, + }; } else if ('useExisting' in injection) { - return DependencyInjector.resolveInjectionToken(jovo, injection.useExisting); + const tree = DependencyInjector.resolveInjectionToken( + jovo, + injection.useExisting, + updatedPath, + ); + return { + token, + resolvedValue: tree?.resolvedValue as TYPE, + children: tree?.children ?? [], + }; + } else { + return { + token, + resolvedValue: undefined, + children: [], + }; } } - static instantiateClass( + private static instantiateClassWithTracing( jovo: Jovo, clazz: Constructor, + dependencyPath: InjectionToken[], ...predefinedArgs: ARGS - ): TYPE { + ): DependencyTree { const injectedArgs = [...predefinedArgs]; const storage = MetadataStorage.getInstance(); const injectMetadata = storage.getMergedInjectMetadata(clazz); const argTypes = Reflect.getMetadata('design:paramtypes', clazz) ?? []; + const children: DependencyTree[] = []; for (let i = predefinedArgs.length; i < argTypes.length; i++) { const injectMetadataForArg = injectMetadata.find((metadata) => metadata.index === i); let injectionToken: InjectionToken; @@ -60,10 +117,34 @@ export class DependencyInjector { injectedArgs.push(undefined); continue; } - const injectedValue = DependencyInjector.resolveInjectionToken(jovo, injectionToken); - injectedArgs.push(injectedValue); + const childNode = DependencyInjector.resolveInjectionToken( + jovo, + injectionToken, + dependencyPath, + ); + injectedArgs.push(childNode?.resolvedValue); + children.push(childNode); } - return new clazz(...injectedArgs); + const instance = new clazz(...injectedArgs); + return { + token: clazz, + resolvedValue: instance, + children, + }; + } + + static async instantiateClass( + jovo: Jovo, + clazz: Constructor, + ...predefinedArgs: ARGS + ): Promise { + const tree = this.instantiateClassWithTracing(jovo, clazz, [], ...predefinedArgs); + await jovo.$handleRequest.middlewareCollection.run( + INSTANTIATE_DEPENDENCY_MIDDLEWARE, + jovo, + tree, + ); + return tree.resolvedValue; } } diff --git a/framework/src/Jovo.ts b/framework/src/Jovo.ts index 81e08aae02..e2734aea06 100644 --- a/framework/src/Jovo.ts +++ b/framework/src/Jovo.ts @@ -264,7 +264,7 @@ export abstract class Jovo< ): Promise { let newOutput: OutputTemplate | OutputTemplate[]; if (typeof outputConstructorOrTemplateOrMessage === 'function') { - const outputInstance = DependencyInjector.instantiateClass( + const outputInstance = await DependencyInjector.instantiateClass( this, outputConstructorOrTemplateOrMessage, this, diff --git a/framework/src/errors/CircularDependencyError.ts b/framework/src/errors/CircularDependencyError.ts new file mode 100644 index 0000000000..07ee69193f --- /dev/null +++ b/framework/src/errors/CircularDependencyError.ts @@ -0,0 +1,12 @@ +import { InjectionToken } from '../metadata/InjectableMetadata'; +import { Constructor, JovoError } from '@jovotech/common'; + +export class CircularDependencyError extends JovoError { + constructor(readonly dependencyPath: InjectionToken[]) { + super({ + message: `Circular dependency detected: ${dependencyPath + .map((x) => String((x as Constructor).name ?? x)) + .join(' -> ')}.`, + }); + } +} diff --git a/framework/src/index.ts b/framework/src/index.ts index e4f732cb95..6edd98b9ed 100644 --- a/framework/src/index.ts +++ b/framework/src/index.ts @@ -51,6 +51,7 @@ export * from './BaseOutput'; export * from './ComponentPlugin'; export * from './ComponentTree'; export * from './ComponentTreeNode'; +export * from './DependencyInjector'; export * from './Extensible'; export * from './HandleRequest'; export * from './I18Next'; @@ -86,6 +87,7 @@ export * from './decorators/PrioritizedOverUnhandled'; export * from './decorators/SubState'; export * from './decorators/Types'; +export * from './errors/CircularDependencyError'; export * from './errors/ComponentNotFoundError'; export * from './errors/DuplicateChildComponentsError'; export * from './errors/DuplicateGlobalIntentsError'; diff --git a/framework/src/metadata/MetadataStorage.ts b/framework/src/metadata/MetadataStorage.ts index ba0bb93f26..e4ebdf0691 100644 --- a/framework/src/metadata/MetadataStorage.ts +++ b/framework/src/metadata/MetadataStorage.ts @@ -7,7 +7,7 @@ import { HandlerMetadata } from './HandlerMetadata'; import { HandlerOptionMetadata } from './HandlerOptionMetadata'; import { MethodDecoratorMetadata } from './MethodDecoratorMetadata'; import { OutputMetadata } from './OutputMetadata'; -import { InjectableMetadata, InjectionToken } from './InjectableMetadata'; +import { InjectableMetadata } from './InjectableMetadata'; import { Constructor } from '@jovotech/common'; import { InjectMetadata } from './InjectMetadata'; diff --git a/framework/test/DependencyInjection.test.ts b/framework/test/DependencyInjection.test.ts index 232850f4fa..bd5b014d2a 100644 --- a/framework/test/DependencyInjection.test.ts +++ b/framework/test/DependencyInjection.test.ts @@ -15,7 +15,8 @@ import { DeepPartial, OutputOptions, Inject, - isSameProvide, + CircularDependencyError, + DependencyTree, } from '../src'; import { ExamplePlatform, ExampleServer } from './utilities'; @@ -322,3 +323,113 @@ describe('dependency overrides', () => { ]); }); }); + +describe('circular dependency detection', () => { + test('circular dependency', async () => { + interface SecondServiceInterface {} + const SecondServiceToken = Symbol('SecondService'); + + @Injectable() + class FirstService { + constructor(@Inject(SecondServiceToken) readonly secondService: SecondServiceInterface) {} + } + + @Injectable() + class SecondService { + constructor(readonly firstService: FirstService) {} + } + + @Global() + @Component() + class ComponentA extends BaseComponent { + constructor( + jovo: Jovo, + options: ComponentOptions | undefined, + readonly firstService: FirstService, + ) { + super(jovo, options); + } + + @Intents('IntentA') + handleIntentA() { + return this.$send('IntentA'); + } + } + + const app = new App({ + plugins: [new ExamplePlatform()], + providers: [FirstService, { provide: SecondServiceToken, useClass: SecondService }], + components: [ComponentA], + }); + + const onError = jest.fn(); + app.onError(onError); + + await app.initialize(); + + const server = new ExampleServer({ + input: { + type: InputType.Intent, + intent: 'IntentA', + }, + }); + await app.handle(server); + expect(server.response.output).toEqual([]); + expect(onError).toHaveBeenCalledTimes(1); + expect(onError.mock.calls[0][0]).toBeInstanceOf(CircularDependencyError); + }); +}); + +describe('dependency injection middleware', () => { + test('middleware arguments', async () => { + @Injectable() + class ExampleService {} + + @Global() + @Component() + class ComponentA extends BaseComponent { + constructor( + jovo: Jovo, + options: ComponentOptions | undefined, + readonly exampleService: ExampleService, + ) { + super(jovo, options); + } + + @Intents('IntentA') + handleIntentA() { + return this.$send('IntentA'); + } + } + + const app = new App({ + plugins: [new ExamplePlatform()], + providers: [ExampleService], + components: [ComponentA], + }); + + const middlewareFunction = jest.fn(); + app.middlewareCollection.use( + 'event.DependencyInjector.instantiateDependency', + middlewareFunction, + ); + await app.initialize(); + + const server = new ExampleServer({ + input: { + type: InputType.Intent, + intent: 'IntentA', + }, + }); + await app.handle(server); + expect(middlewareFunction).toHaveBeenCalledTimes(1); + const dependencyTree: DependencyTree = middlewareFunction.mock.calls[0][1]; + + expect(dependencyTree.token).toEqual(ComponentA); + expect(dependencyTree.resolvedValue).toBeInstanceOf(ComponentA); + expect(dependencyTree.children.length).toEqual(1); + expect(dependencyTree.children[0].token).toEqual(ExampleService); + expect(dependencyTree.children[0].resolvedValue).toBeInstanceOf(ExampleService); + expect(dependencyTree.children[0].children.length).toEqual(0); + }); +});