From 9c98f8b110078ab35882fece44f45fde34a4feeb Mon Sep 17 00:00:00 2001 From: "Marc J. Schmidt" Date: Tue, 12 Mar 2024 21:29:28 +0100 Subject: [PATCH] fix(http): parameter service injection into route methods with encapsulated modules Also rework parameter injection uses a prebuilt resolver instead of creating always a new one. This increases performance. --- packages/http/src/request-parser.ts | 24 +++++++++----- packages/http/tests/module.spec.ts | 49 ++++++++++++++++++++++++++--- 2 files changed, 62 insertions(+), 11 deletions(-) diff --git a/packages/http/src/request-parser.ts b/packages/http/src/request-parser.ts index 34320f692..66efd63c6 100644 --- a/packages/http/src/request-parser.ts +++ b/packages/http/src/request-parser.ts @@ -1,5 +1,5 @@ import { asyncOperation, ClassType, CompilerContext, getClassName, isObject } from '@deepkit/core'; -import { InjectorModule } from '@deepkit/injector'; +import { DependenciesUnmetError, InjectorModule } from '@deepkit/injector'; import { assertType, findMember, @@ -225,6 +225,7 @@ export function getRequestParserCodeForParameters( routeConfig?: RouteConfig, }, ) { + compiler.set({ DependenciesUnmetError }); let enableParseBody = false; let requiresAsyncParameters = false; const setParameters: string[] = []; @@ -338,7 +339,8 @@ export function getRequestParserCodeForParameters( } let injector = '_injector'; - const moduleVar = config.module ? ', ' + compiler.reserveConst(config.module, 'module') : ''; + const moduleRawVar = config.module ? compiler.reserveConst(config.module, 'module') : 'undefined'; + const moduleVar = config.module ? `, ${moduleRawVar}` : ''; if (resolverType) { requiresAsyncParameters = true; @@ -374,12 +376,20 @@ export function getRequestParserCodeForParameters( } if (!parameter.isPartOfPath()) { - //todo: if injectorToken is a Type, then this will be very slow since Injector.createResolver is used all the time. - let injectorGet = `parameters.${parameter.parameter.name} = ${injector}.get(${injectorTokenVar});`; - if (parameter.parameter.isOptional()) { - injectorGet = `try {parameters.${parameter.parameter.name} = ${injector}.get(${injectorTokenVar}); } catch (e) {}`; + const resolverVar = compiler.reserveVariable('resolver'); + let injectorGet = ` + if (!${resolverVar}) ${resolverVar} = ${injector}.resolve(${moduleRawVar}, ${injectorTokenVar}); + parameters.${parameter.parameter.name} = ${resolverVar}(${injector}.scope); + `; + if (!parameter.parameter.isOptional()) { + injectorGet += ` + if (!parameters.${parameter.parameter.name}) { + throw new DependenciesUnmetError( + \`Parameter \${${JSON.stringify(parameter.parameter.name)}} is required but provider returned undefined.\`, + ); + }`; } - setParameters.push(`if (!${parameterResolverFoundVar}) ${injectorGet}`); + setParameters.push(`if (!${parameterResolverFoundVar}) { ${injectorGet} }`); } } } diff --git a/packages/http/tests/module.spec.ts b/packages/http/tests/module.spec.ts index f6202e477..7b9c28d42 100644 --- a/packages/http/tests/module.spec.ts +++ b/packages/http/tests/module.spec.ts @@ -1,11 +1,12 @@ -import { App } from '@deepkit/app'; +import { App, AppModule } from '@deepkit/app'; import { expect, test } from '@jest/globals'; import { HttpModule } from '../src/module.js'; import { HttpKernel } from '../src/kernel.js'; import { HttpRequest } from '../src/model.js'; import { http } from '../src/decorator.js'; -import { httpWorkflow } from '../src/http.js'; +import { HttpUnauthorizedError, httpWorkflow } from '../src/http.js'; import { HttpRouterRegistry, RouteConfig } from '../src/router.js'; +import { provide } from '@deepkit/injector'; test('module basic functionality', async () => { class Controller { @@ -116,12 +117,12 @@ test('dynamic route', async () => { app.configureProvider(router => { router.addRoute(new RouteConfig('name', ['GET'], '/users/:id', { type: 'function', fn: (id: number) => { - return {id}; + return { id }; }, })); router.get('/users', () => { - return [{id: 1}, {id: 2}]; + return [{ id: 1 }, { id: 2 }]; }); }); @@ -135,3 +136,43 @@ test('dynamic route', async () => { expect(response2.statusCode).toBe(200); expect(response2.json).toEqual([{ id: 1 }, { id: 2 }]); }); + +test('encapsulated service in router methods', async () => { + class User { + constructor(public username: string) { + } + } + + class MyController { + @http.GET('/me') + me(user: User) { + return user; + } + } + + const myModule = new AppModule(); + myModule.addController(MyController); + myModule.addProvider(provide({ scope: 'http', useFactory: (req: HttpRequest) => req.store.user })); + + myModule.addListener(httpWorkflow.onAuth.listen((event, request: HttpRequest) => { + if (!request.headers.authorization) throw new HttpUnauthorizedError(); + request.store.user = new User('Peter:' + request.headers.authorization); + })); + + const app = new App({ + imports: [new HttpModule(), myModule], + }); + + const httpKernel = app.get(HttpKernel); + + { + const response = await httpKernel.request(HttpRequest.GET('/me')); + expect(response.statusCode).toBe(401); + } + + { + const response = await httpKernel.request(HttpRequest.GET('/me').header('authorization', '123')); + expect(response.statusCode).toBe(200); + expect(response.json).toEqual({ username: 'Peter:123' }); + } +});