From 1e27f7c6df315f9e423a26dc5b16bc82627ee69d Mon Sep 17 00:00:00 2001 From: "Marc J. Schmidt" Date: Wed, 8 Oct 2025 00:04:32 +0200 Subject: [PATCH] fix: wrong InjectorContext scope set in HTTP, and wrong received in RPC Removed RpcInjectorContext due to highly misleading behaviour. It's now possible to inject InjectorContext in the different scopes with correct expectations (to get the scoped injector context). --- packages/app/tests/service-container.spec.ts | 7 +- packages/framework/src/module.ts | 6 +- packages/framework/src/rpc.ts | 3 - packages/framework/src/testing.ts | 5 ++ packages/framework/src/worker.ts | 5 +- packages/framework/tests/rpc.spec.ts | 53 +++++++++++++- packages/http/src/http.ts | 2 +- packages/http/src/module.ts | 6 +- packages/injector/src/injector.ts | 74 ++++++-------------- packages/injector/tests/injector3.spec.ts | 39 +++++++++-- packages/injector/tests/nominal.spec.ts | 21 ++---- packages/rpc/src/server/kernel.ts | 2 + packages/rpc/tests/controller.spec.ts | 1 - 13 files changed, 129 insertions(+), 95 deletions(-) diff --git a/packages/app/tests/service-container.spec.ts b/packages/app/tests/service-container.spec.ts index 90a0615aa..4f32751bf 100644 --- a/packages/app/tests/service-container.spec.ts +++ b/packages/app/tests/service-container.spec.ts @@ -275,9 +275,6 @@ test('exported module', () => { }); test('scoped InjectorContext access', () => { - class RpcInjectorContext extends InjectorContext { - } - class MyService { constructor(public injectorContext: InjectorContext) { } @@ -286,7 +283,7 @@ test('scoped InjectorContext access', () => { const myModule = new AppModule({}, { providers: [ { provide: MyService }, - { provide: RpcInjectorContext, scope: 'rpc', useValue: undefined }, + { provide: InjectorContext, scope: 'rpc', useValue: undefined }, ] }); @@ -299,7 +296,7 @@ test('scoped InjectorContext access', () => { { const injector = serviceContainer.getInjectorContext().createChildScope('rpc'); - injector.set(RpcInjectorContext, injector); + injector.set(InjectorContext, injector); const myService = injector.get(MyService); expect(myService.injectorContext).toBeInstanceOf(InjectorContext); diff --git a/packages/framework/src/module.ts b/packages/framework/src/module.ts index d477a1ab8..52726f12d 100644 --- a/packages/framework/src/module.ts +++ b/packages/framework/src/module.ts @@ -35,7 +35,7 @@ import { Zone } from './zone.js'; import { DebugBrokerBus } from './debug/broker.js'; import { ApiConsoleModule } from '@deepkit/api-console-module'; import { AppModule, ControllerConfig, createModuleClass, DeepPartial, onAppShutdown } from '@deepkit/app'; -import { RpcControllers, RpcInjectorContext, RpcKernelWithStopwatch } from './rpc.js'; +import { RpcControllers, RpcKernelWithStopwatch } from './rpc.js'; import { normalizeDirectory } from './utils.js'; import { FilesystemRegistry, PublicFilesystem } from './filesystem.js'; import { Filesystem } from '@deepkit/filesystem'; @@ -92,7 +92,7 @@ export class FrameworkModule extends createModuleClass({ //all of these will be set on scope creation { provide: HttpRequest, scope: 'rpc', useValue: undefined }, - { provide: RpcInjectorContext, scope: 'rpc', useValue: undefined }, + { provide: InjectorContext, scope: 'rpc', useValue: undefined }, { provide: SessionState, scope: 'rpc', useValue: undefined }, { provide: RpcKernelBaseConnection, scope: 'rpc', useValue: undefined }, { provide: RpcKernelConnection, scope: 'rpc', useValue: undefined }, @@ -125,7 +125,7 @@ export class FrameworkModule extends createModuleClass({ DatabaseRegistry, HttpRequest, - RpcInjectorContext, + InjectorContext, SessionState, RpcKernelConnection, RpcKernelBaseConnection, diff --git a/packages/framework/src/rpc.ts b/packages/framework/src/rpc.ts index 642170898..3ec991718 100644 --- a/packages/framework/src/rpc.ts +++ b/packages/framework/src/rpc.ts @@ -18,9 +18,6 @@ export class RpcControllers { public readonly controllers = new Map }>(); } -export class RpcInjectorContext extends InjectorContext { -} - export class RpcServerActionWithStopwatch extends RpcServerAction { stopwatch?: Stopwatch; diff --git a/packages/framework/src/testing.ts b/packages/framework/src/testing.ts index 2dbbcc9a7..f84de9bda 100644 --- a/packages/framework/src/testing.ts +++ b/packages/framework/src/testing.ts @@ -43,6 +43,11 @@ export class TestingFacade> { await this.app.get(ApplicationServer).close(graceful); } + /** + * ```typescript + * const response = await testing.request(HttpRequest.GET('/test').header('x-test', '123')); + * ``` + */ public async request(requestBuilder: RequestBuilder): Promise { const request = requestBuilder.build(); const response = new MemoryHttpResponse(request); diff --git a/packages/framework/src/worker.ts b/packages/framework/src/worker.ts index 4abf4d2e0..861e665b0 100644 --- a/packages/framework/src/worker.ts +++ b/packages/framework/src/worker.ts @@ -17,7 +17,7 @@ import selfsigned from 'selfsigned'; import { HttpConfig, HttpKernel, HttpRequest, HttpResponse } from '@deepkit/http'; import { InjectorContext } from '@deepkit/injector'; -import { RpcControllers, RpcInjectorContext } from './rpc.js'; +import { RpcControllers } from './rpc.js'; import { SecureContextOptions, TlsOptions } from 'tls'; // @ts-ignore @@ -174,8 +174,7 @@ export class WebMemoryWorkerFactory extends WebWorkerFactory { export function createRpcConnection(rootScopedContext: InjectorContext, rpcKernel: RpcKernel, transport: TransportConnection, request?: HttpRequest) { const injector = rootScopedContext.createChildScope('rpc'); injector.set(HttpRequest, request); - injector.set(RpcInjectorContext, injector); - + injector.set(InjectorContext, injector); const connection = rpcKernel.createConnection(transport, injector); injector.set(SessionState, connection.sessionState); injector.set(RpcKernelConnection, connection); diff --git a/packages/framework/tests/rpc.spec.ts b/packages/framework/tests/rpc.spec.ts index 4b7af5935..9ff94460e 100644 --- a/packages/framework/tests/rpc.spec.ts +++ b/packages/framework/tests/rpc.spec.ts @@ -2,6 +2,8 @@ import { expect, test } from '@jest/globals'; import { ControllerSymbol, rpc, RpcKernelConnection, RpcKernelSecurity, Session, SessionState } from '@deepkit/rpc'; import { createTestingApp } from '../src/testing.js'; import { AppModule } from '@deepkit/app'; +import { InjectorContext } from '@deepkit/injector'; +import { http, HttpQuery, HttpRequest } from '@deepkit/http'; test('di', async () => { class MyService { @@ -100,8 +102,8 @@ test('module provides RpcKernelSecurity', async () => { name: 'module', controllers: [Controller], providers: [{ - provide: RpcKernelSecurity, useClass: MyRpcKernelSecurity, scope: 'rpc' - }] + provide: RpcKernelSecurity, useClass: MyRpcKernelSecurity, scope: 'rpc', + }], }).forRoot(); const testing = createTestingApp({ imports: [module] }); await testing.startServer(); @@ -143,7 +145,7 @@ test('rpc controller access unscoped provider', async () => { const testing = createTestingApp({ controllers: [Controller], - providers: [ModelRegistryService] + providers: [ModelRegistryService], }); const client = testing.createRpcClient(); @@ -153,3 +155,48 @@ test('rpc controller access unscoped provider', async () => { const registry = testing.app.get(ModelRegistryService); expect(registry.models).toEqual(['a']); }); + +test('InjectorContext', async () => { + @rpc.controller('main') + class RpcController { + constructor(private injectorContext: InjectorContext) { + } + + @rpc.action() + test() { + expect(this.injectorContext.scope?.name).toBe('rpc'); + expect(this.injectorContext.getOrUndefined(HttpRequest)?.url).toBe(undefined); + } + } + + class HttpController { + constructor(private injectorContext: InjectorContext) { + } + + @http.GET('/test') + test(q: HttpQuery) { + expect(this.injectorContext.scope?.name).toBe('http'); + expect(this.injectorContext.get(HttpRequest)?.url).toBe('/test?q=' + q); + } + } + + const testing = createTestingApp({ + controllers: [RpcController, HttpController], + }); + + await testing.startServer(); + + const request1 = await testing.request(HttpRequest.GET('/test?q=1')); + expect(request1.statusCode).toBe(200); + const request2 = await testing.request(HttpRequest.GET('/test?q=2')); + expect(request2.statusCode).toBe(200); + + const client = testing.createRpcClient(); + const controller = client.controller('main'); + await controller.test(); + + const request3 = await testing.request(HttpRequest.GET('/test?q=3')); + expect(request3.statusCode).toBe(200); + + await testing.stopServer(); +}); diff --git a/packages/http/src/http.ts b/packages/http/src/http.ts index 0643b3f89..5ced5bc0f 100644 --- a/packages/http/src/http.ts +++ b/packages/http/src/http.ts @@ -543,7 +543,7 @@ export class HttpListener { protected injector: Injector, protected stopwatch: Stopwatch, ) { - this.setRouteConfig = injector.createSetter(RouteConfig, {name: 'http'}); + this.setRouteConfig = injector.createSetter(RouteConfig, 'http'); } @eventDispatcher.listen(httpWorkflow.onRequest, 100) diff --git a/packages/http/src/module.ts b/packages/http/src/module.ts index e4dc02ada..00736a02d 100644 --- a/packages/http/src/module.ts +++ b/packages/http/src/module.ts @@ -32,8 +32,9 @@ export class HttpModule extends createModuleClass({ HttpResultFormatter, HttpRouterRegistry, HttpRouterFilterResolver, - { provide: HttpResponse, scope: 'http' }, - { provide: HttpRequest, scope: 'http' }, + { provide: InjectorContext, useValue: undefined, scope: 'http' }, + { provide: HttpResponse, useValue: undefined, scope: 'http' }, + { provide: HttpRequest, useValue: undefined, scope: 'http' }, { provide: RouteConfig, useValue: undefined, scope: 'http' }, { provide: Logger, useValue: new Logger([new ConsoleTransport()]) }, ], @@ -49,6 +50,7 @@ export class HttpModule extends createModuleClass({ HttpKernel, HttpResultFormatter, HttpRouterFilterResolver, + InjectorContext, HttpResponse, HttpRequest, HttpControllers, diff --git a/packages/injector/src/injector.ts b/packages/injector/src/injector.ts index 6e21c161f..f23a2f060 100644 --- a/packages/injector/src/injector.ts +++ b/packages/injector/src/injector.ts @@ -1,42 +1,7 @@ -import { - isClassProvider, - isExistingProvider, - isFactoryProvider, - isValueProvider, - NormalizedProvider, - ProviderWithScope, - Tag, - TagProvider, - TagRegistry, - Token, -} from './provider.js'; -import { - AbstractClassType, - ClassType, - CompilerContext, - CustomError, - getClassName, - getPathValue, - isArray, - isClass, - isFunction, - isPrototypeOfBase, -} from '@deepkit/core'; +import { isClassProvider, isExistingProvider, isFactoryProvider, isValueProvider, NormalizedProvider, ProviderWithScope, Tag, TagProvider, TagRegistry, Token } from './provider.js'; +import { AbstractClassType, ClassType, CompilerContext, CustomError, getClassName, getPathValue, isArray, isClass, isFunction, isPrototypeOfBase } from '@deepkit/core'; import { ConfigurationProviderRegistry, ConfigureProviderEntry, findModuleForConfig, getScope, InjectorModule, PreparedProvider } from './module.js'; -import { - isOptional, - isWithAnnotations, - Packed, - ReceiveType, - reflect, - ReflectionClass, - ReflectionFunction, - ReflectionKind, - resolveReceiveType, - stringifyType, - Type, - typeAnnotation, -} from '@deepkit/type'; +import { isOptional, isWithAnnotations, Packed, ReceiveType, reflect, ReflectionClass, ReflectionFunction, ReflectionKind, resolveReceiveType, stringifyType, Type, typeAnnotation } from '@deepkit/type'; export class InjectorError extends CustomError { @@ -316,7 +281,7 @@ interface BuiltInjector { resolver(token: Token, scope?: Scope, optional?: boolean): Resolver; - setter(token: Token, scope?: Scope): Setter; + setter(token: Token, scopeName?: string): Setter; collectStack(stack: StackEntry[]): void; } @@ -368,11 +333,11 @@ export class Injector { return this.built.instantiationCount(token, scope); } - getSetter(token: ReceiveType | Token): Setter { + getSetter(token: ReceiveType | Token, scopeName?: string): Setter { let setter = this.setterMap.get(token); if (!setter) { - setter = this.createSetter(token as ReceiveType | Token); - this.setterMap.set(token, setter); + setter = this.createSetter(token as ReceiveType | Token, scopeName); + if (!scopeName) this.setterMap.set(token, setter); } return setter; } @@ -645,9 +610,8 @@ export class Injector { } } - function setter_${routerName}(scope) { - const name = scope?.name || ''; - switch (name) { + function setter_${routerName}(scopeName) { + switch (scopeName) { ${setterNames.map(v => `case ${JSON.stringify(v.scope)}: return ${v.function};`).join('\n')} default: return ${routerName}_setter; // no scope given, so return route for value itself } @@ -773,17 +737,17 @@ export class Injector { throw serviceNotFoundError(tokenLabel(token)); } - // setter(token: Token, scope?: Scope): Setter; - function setter(token, scope) { + // setter(token: Token, scopeName?: string): Setter; + function setter(token, scopeName) { const containerToken = getContainerToken(token); const fn = containerToken[symbolSetter] || lookupSetter[containerToken]; - if (fn) return fn(scope); + if (fn) return fn(scopeName); throw serviceNotFoundError(tokenLabel(token)); } // set(token: Token, value: any, scope?: Scope): void; function set(token, value, scope) { - setter(token)(value, scope); + setter(token, scope?.name)(value, scope); } // get(token: Token, scope?: Scope, optional?: boolean): unknown; @@ -1194,7 +1158,7 @@ export class Injector { return foundPreparedProvider; } - createSetter(token: ReceiveType | Token, scope?: Scope, label?: string): Setter { + createSetter(token: ReceiveType | Token, scopeName?: string, label?: string): Setter { if (token instanceof TagProvider) token = token.provider.provide; // todo remove isClass since it's slow @@ -1215,7 +1179,7 @@ export class Injector { const containerToken = getContainerToken(foundPreparedProvider.token); const resolveFromModule = foundPreparedProvider.resolveFrom || foundPreparedProvider.modules[0]; - return resolveFromModule.injector!.built!.setter(containerToken, scope); + return resolveFromModule.injector!.built!.setter(containerToken, scopeName); } createResolver(token: ReceiveType | Token, scope?: Scope, label?: string): Resolver { @@ -1391,11 +1355,13 @@ export class InjectorContext { * be executed to resolve the token. This increases performance in hot paths. */ resolve(module?: InjectorModule, type?: ReceiveType | Token): Resolver { - return this.getInjector(module || this.rootModule).getResolver(type) as Resolver; + const injector = this.getInjector(module || this.rootModule); + return injector.getResolver(type) as Resolver; } - setter(module?: InjectorModule, type?: ReceiveType | Token): Setter { - return this.getInjector(module || this.rootModule).getSetter(type) as Setter; + setter(module?: InjectorModule, type?: ReceiveType | Token, scopeName?: string): Setter { + const injector = this.getInjector(module || this.rootModule); + return injector.getSetter(type, scopeName || this.scope?.name) as Setter; } /** diff --git a/packages/injector/tests/injector3.spec.ts b/packages/injector/tests/injector3.spec.ts index c72703494..955c6c49c 100644 --- a/packages/injector/tests/injector3.spec.ts +++ b/packages/injector/tests/injector3.spec.ts @@ -156,9 +156,6 @@ test('optional forwarded to external module', () => { }); test('scoped InjectorContext', () => { - class RpcInjectorContext extends InjectorContext { - } - class HttpListener { constructor(public injector: InjectorContext) { } @@ -169,10 +166,10 @@ test('scoped InjectorContext', () => { ]); const frameworkModule = new InjectorModule([ - { provide: RpcInjectorContext, scope: 'rpc', useValue: undefined }, + { provide: InjectorContext, scope: 'rpc', useValue: undefined }, ]) .addImport(httpModule) - .addExport(RpcInjectorContext, httpModule); + .addExport(InjectorContext, httpModule); const rootModule = new InjectorModule([ { provide: InjectorContext, useFactory: () => injector }, @@ -277,3 +274,35 @@ test('constructor properties only handled once', () => { const a1 = injector.get(ServiceA); expect(created).toBe(1); }); + +test('define a second provider in different scope in child module', () => { + class ServiceA { + constructor(public injector: InjectorContext) { + } + } + + const childModule = new InjectorModule([ + { provide: ServiceA, scope: 'rpc' }, + { provide: InjectorContext, scope: 'rpc' }, + ]).addExport(ServiceA, InjectorContext); + + const root = new InjectorModule([ + { provide: InjectorContext, useFactory: () => injectorContext }, + ]).addImport(childModule); + + const injectorContext = new InjectorContext(root); + + { + const injector = injectorContext.createChildScope('rpc'); + injector.set(InjectorContext, injector); + const a = injector.get(ServiceA); + expect(a.injector).toBe(injector); + } + { + const setter = injectorContext.setter(undefined, InjectorContext); + const injector = injectorContext.createChildScope('rpc'); + setter(injector, injector.scope); + const a = injector.get(ServiceA); + expect(a.injector).toBe(injector); + } +}); diff --git a/packages/injector/tests/nominal.spec.ts b/packages/injector/tests/nominal.spec.ts index 7effeaf86..76843bf48 100644 --- a/packages/injector/tests/nominal.spec.ts +++ b/packages/injector/tests/nominal.spec.ts @@ -100,15 +100,12 @@ test('child implementation from imported module encapsulated', () => { } } - class RpcInjectorContext extends InjectorContext { - } - let injectorContext: InjectorContext | undefined; const module = new InjectorModule([ User, - // => since User is not exported, it should not access overrides from outside, thus it should be RpcInjectorContext - { provide: RpcInjectorContext }, + // => since User is not exported, it should not access overrides from outside, thus it should be InjectorContext + { provide: InjectorContext }, ]); const rootModule = new InjectorModule([ @@ -117,7 +114,7 @@ test('child implementation from imported module encapsulated', () => { injectorContext = new InjectorContext(rootModule); const user = injectorContext.get(User, module); - expect(user.context.constructor).toBe(RpcInjectorContext); + expect(user.context.constructor).toBe(InjectorContext); }); test('child implementation from imported module partly exported', () => { @@ -126,15 +123,12 @@ test('child implementation from imported module partly exported', () => { } } - class RpcInjectorContext extends InjectorContext { - } - let injectorContext: InjectorContext | undefined; const module = new InjectorModule([ User, // => since User is exported, it should access overrides from outside, thus it should be InjectorContext - { provide: RpcInjectorContext }, + { provide: InjectorContext }, ]).addExport(User); const rootModule = new InjectorModule([ @@ -152,16 +146,13 @@ test('child implementation from imported module exported', () => { } } - class RpcInjectorContext extends InjectorContext { - } - let injectorContext: InjectorContext | undefined; const module = new InjectorModule([ User, // => since User is exported, it should access overrides from outside, thus it should be InjectorContext - { provide: RpcInjectorContext }, - ]).addExport(User, RpcInjectorContext); + { provide: InjectorContext }, + ]).addExport(User, InjectorContext); const rootModule = new InjectorModule([ { provide: InjectorContext, useFactory: () => injectorContext! }, diff --git a/packages/rpc/src/server/kernel.ts b/packages/rpc/src/server/kernel.ts index cfba260cd..577bc3eee 100644 --- a/packages/rpc/src/server/kernel.ts +++ b/packages/rpc/src/server/kernel.ts @@ -667,6 +667,7 @@ export class RpcKernel { EventDispatcher, { provide: SessionState, scope: 'rpc' }, { provide: RpcKernelSecurity, scope: 'rpc' }, + { provide: InjectorContext, scope: 'rpc' }, //will be provided when scope is created { provide: RpcKernelConnection, scope: 'rpc', useValue: undefined }, @@ -739,6 +740,7 @@ export class RpcKernel { injector.get(RpcKernelSecurity), this.peerExchange, ); + injector.set(InjectorContext, injector); injector.set(RpcKernelConnection, connection); injector.set(RpcKernelBaseConnection, connection); for (const on of this.onConnectionListeners) on(connection, injector, this.logger); diff --git a/packages/rpc/tests/controller.spec.ts b/packages/rpc/tests/controller.spec.ts index f1c6735ba..75b711441 100644 --- a/packages/rpc/tests/controller.spec.ts +++ b/packages/rpc/tests/controller.spec.ts @@ -993,4 +993,3 @@ test('connection disconnect back-controller', async () => { await expect(promise).rejects.toThrow('Connection closed'); }); -