From 79ee021a0eb9f674709ae19bae4cacf096349ce8 Mon Sep 17 00:00:00 2001 From: codytseng Date: Wed, 14 Aug 2024 12:32:57 +0800 Subject: [PATCH 1/5] fix: resolve middlewares not executed when root path is excluded --- .../e2e/middleware-fastify.spec.ts | 109 ++++++++++++++++++ packages/core/middleware/middleware-module.ts | 6 +- .../middleware/route-info-path-extractor.ts | 2 +- 3 files changed, 111 insertions(+), 6 deletions(-) diff --git a/integration/hello-world/e2e/middleware-fastify.spec.ts b/integration/hello-world/e2e/middleware-fastify.spec.ts index 36a6d64e024..63d8b4c145b 100644 --- a/integration/hello-world/e2e/middleware-fastify.spec.ts +++ b/integration/hello-world/e2e/middleware-fastify.spec.ts @@ -398,4 +398,113 @@ describe('Middleware (FastifyAdapter)', () => { await app.close(); }); }); + + describe('should work properly when globalPrefix is set', () => { + class Middleware implements NestMiddleware { + use(request: any, reply: any, next: () => void) { + if (request.middlewareExecutionCount === undefined) { + request.middlewareExecutionCount = 1; + } else { + request.middlewareExecutionCount++; + } + next(); + } + } + + @Controller() + class AbcController { + @Get('/a') + async a(@Req() request: any) { + return this.validateExecutionCount({ + request, + expected: 1, + }); + } + + @Get('/') + async root(@Req() request: any) { + return this.validateExecutionCount({ + request, + expected: 1, + }); + } + + private validateExecutionCount({ + request, + expected, + }: { + request: any; + expected: number; + }) { + let actual: number | undefined; + actual = request.raw.middlewareExecutionCount; + actual ??= 0; + + return { + success: actual === expected, + actual, + expected, + }; + } + } + + @Module({ + controllers: [AbcController], + }) + class TestModule implements NestModule { + configure(consumer: MiddlewareConsumer) { + consumer.apply(Middleware).forRoutes(AbcController); + } + } + + beforeEach(async () => { + app = ( + await Test.createTestingModule({ + imports: [TestModule], + }).compile() + ).createNestApplication(new FastifyAdapter()); + + app.setGlobalPrefix('api', { exclude: ['/'] }); + + await app.init(); + }); + + it(`GET forRoutes(/api/a)`, () => { + return app + .inject({ + method: 'GET', + url: '/api/a', + }) + .then(({ payload }) => + expect(payload).to.be.eql( + JSON.stringify({ + success: true, + actual: 1, + expected: 1, + }), + ), + ); + }); + + it(`GET forRoutes(/)`, () => { + return app + .inject({ + method: 'GET', + url: '/', + }) + .then(({ payload }) => + expect(payload).to.be.eql( + JSON.stringify({ + success: true, + actual: 1, + expected: 1, + }), + ), + ); + }); + + afterEach(async () => { + await app.close(); + }); + }); }); diff --git a/packages/core/middleware/middleware-module.ts b/packages/core/middleware/middleware-module.ts index b956a23850f..fd226bbf53f 100644 --- a/packages/core/middleware/middleware-module.ts +++ b/packages/core/middleware/middleware-module.ts @@ -329,11 +329,7 @@ export class MiddlewareModule< } return next(); }; - const pathsToApplyMiddleware = []; - paths.some(path => path.match(/^\/?$/)) - ? pathsToApplyMiddleware.push('/') - : pathsToApplyMiddleware.push(...paths); - pathsToApplyMiddleware.forEach(path => router(path, middlewareFunction)); + paths.forEach(path => router(path, middlewareFunction)); } private getContextId(request: unknown, isTreeDurable: boolean): ContextId { diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index 263b12679db..17bdc3b34c3 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -49,7 +49,7 @@ export class RouteInfoPathExtractor { ? [ ...entries, ...this.excludedGlobalPrefixRoutes.map( - route => versionPaths + addLeadingSlash(route.path), + route => versionPaths + addLeadingSlash(route.path) + '$', ), ] : entries; From ffa4f65ef1ce45cc5e9681da120bae6a76183588 Mon Sep 17 00:00:00 2001 From: codytseng Date: Wed, 14 Aug 2024 15:21:28 +0800 Subject: [PATCH 2/5] fix: `extractPathsForm` result when there are multiple `versionPaths` --- .../core/middleware/route-info-path-extractor.ts | 13 ++++++++++--- .../middleware/route-info-path-extractor.spec.ts | 4 ++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index 17bdc3b34c3..94fd5d4c4b8 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -48,9 +48,16 @@ export class RouteInfoPathExtractor { return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ ...entries, - ...this.excludedGlobalPrefixRoutes.map( - route => versionPaths + addLeadingSlash(route.path) + '$', - ), + ...this.excludedGlobalPrefixRoutes + .map(route => + versionPaths.length > 0 + ? versionPaths.map( + versionPath => + versionPath + addLeadingSlash(route.path) + '$', + ) + : addLeadingSlash(route.path) + '$', + ) + .flat(), ] : entries; } diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index 62bfb5fc8d7..76369a6501b 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api$', '/api/*', '/foo']); + ).to.eql(['/api$', '/api/*', '/foo$']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -74,7 +74,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1$', '/api/v1/*', '/v1/foo']); + ).to.eql(['/api/v1$', '/api/v1/*', '/v1/foo$']); expect( routeInfoPathExtractor.extractPathsFrom({ From efb30c1d6707a8d0564dc2dd3ed4cb85f5a7abb0 Mon Sep 17 00:00:00 2001 From: codytseng Date: Wed, 14 Aug 2024 17:00:33 +0800 Subject: [PATCH 3/5] test: add test case for middleware param retrieval --- .../global-prefix/e2e/global-prefix.spec.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts index 76acf712137..2f5ec4237cd 100644 --- a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts +++ b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts @@ -130,6 +130,17 @@ describe('Global prefix', () => { .expect(200, { '0': 'params', tenantId: 'test' }); }); + it(`should get the params in the global prefix with exclude option`, async () => { + app.setGlobalPrefix('/api/:tenantId', { exclude: ['/'] }); + + server = app.getHttpServer(); + await app.init(); + + await request(server) + .get('/api/test/params') + .expect(200, { '0': 'params', tenantId: 'test' }); + }); + it(`should execute middleware only once`, async () => { app.setGlobalPrefix('/api', { exclude: ['/'] }); From 526c4d953e353f835236d49b5abfabac080d6fdc Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 15 Aug 2024 16:49:17 +0800 Subject: [PATCH 4/5] refactor: improve readability of `extractPathsFrom` method --- .../middleware/route-info-path-extractor.ts | 59 ++++++++++--------- 1 file changed, 30 insertions(+), 29 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index 94fd5d4c4b8..ac3362dd54c 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -30,39 +30,30 @@ export class RouteInfoPathExtractor { } public extractPathsFrom({ path, method, version }: RouteInfo): string[] { + if (!this.isAWildcard(path)) { + return this.extractNonWildcardPathsFrom({ path, method, version }); + } + const versionPaths = this.extractVersionPathFrom(version); + const prefixes = this.combinePaths(this.prefixPath, versionPaths); + const entries = [ + ...prefixes.filter(Boolean).map(prefix => prefix + '$'), + ...this.combinePaths(prefixes, addLeadingSlash(path)), + ]; - if (this.isAWildcard(path)) { - const entries = - versionPaths.length > 0 - ? versionPaths - .map(versionPath => [ - this.prefixPath + versionPath + '$', - this.prefixPath + versionPath + addLeadingSlash(path), - ]) - .flat() - : this.prefixPath - ? [this.prefixPath + '$', this.prefixPath + addLeadingSlash(path)] - : [addLeadingSlash(path)]; - - return Array.isArray(this.excludedGlobalPrefixRoutes) - ? [ - ...entries, - ...this.excludedGlobalPrefixRoutes - .map(route => - versionPaths.length > 0 - ? versionPaths.map( - versionPath => - versionPath + addLeadingSlash(route.path) + '$', - ) - : addLeadingSlash(route.path) + '$', - ) - .flat(), - ] - : entries; + if ( + Array.isArray(this.excludedGlobalPrefixRoutes) && + this.excludedGlobalPrefixRoutes.length + ) { + const excludedGlobalPrefixPaths = this.excludedGlobalPrefixRoutes + .map(route => + this.combinePaths(versionPaths, addLeadingSlash(route.path + '$')), + ) + .flat(); + entries.push(...excludedGlobalPrefixPaths); } - return this.extractNonWildcardPathsFrom({ path, method, version }); + return entries; } public extractPathFrom(route: RouteInfo): string[] { @@ -120,4 +111,14 @@ export class RouteInfoPathExtractor { } return [addLeadingSlash(versionPrefix + versionValue.toString())]; } + + public combinePaths(a: string | string[], b: string | string[]): string[] { + const formatter = (path: string | string[]) => { + return Array.isArray(path) ? (path.length > 0 ? path : ['']) : [path]; + }; + + const aArr = formatter(a); + const bArr = formatter(b); + return aArr.map(a => bArr.map(b => a + b)).flat(); + } } From 683ef1f93a49996765d7d4f84c772d5dfa78a96e Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 15 Aug 2024 18:15:26 +0800 Subject: [PATCH 5/5] feat: special extract paths logic only when prefix contains `:param` --- .../middleware/route-info-path-extractor.ts | 10 +++ .../route-info-path-extractor.spec.ts | 69 +++++++++++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index ac3362dd54c..7a63ed6b3ca 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -34,6 +34,12 @@ export class RouteInfoPathExtractor { return this.extractNonWildcardPathsFrom({ path, method, version }); } + if (!this.hasParamInPrefixPath()) { + return [addLeadingSlash(path)]; + } + + // The following logic is for cases where `prefixPath` contains `:param`. + // To resolve https://github.com/nestjs/nest/issues/9776 const versionPaths = this.extractVersionPathFrom(version); const prefixes = this.combinePaths(this.prefixPath, versionPaths); const entries = [ @@ -64,6 +70,10 @@ export class RouteInfoPathExtractor { return this.extractNonWildcardPathsFrom(route); } + private hasParamInPrefixPath(): boolean { + return this.prefixPath.includes(':'); + } + private isAWildcard(path: string): boolean { return ['*', '/*', '/*/', '(.*)', '/(.*)'].includes(path); } diff --git a/packages/core/test/middleware/route-info-path-extractor.spec.ts b/packages/core/test/middleware/route-info-path-extractor.spec.ts index 76369a6501b..dbcace8ade5 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1$', '/v1/*']); + ).to.eql(['/*']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,26 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api$', '/api/*']); + ).to.eql(['/*']); + + expect( + routeInfoPathExtractor.extractPathsFrom({ + path: '*', + method: RequestMethod.ALL, + version: '1', + }), + ).to.eql(['/*']); + }); + + it(`should return correct paths when set global prefix (has :param)`, () => { + Reflect.set(routeInfoPathExtractor, 'prefixPath', '/:param'); + + expect( + routeInfoPathExtractor.extractPathsFrom({ + path: '*', + method: RequestMethod.ALL, + }), + ).to.eql(['/:param$', '/:param/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +69,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1$', '/api/v1/*']); + ).to.eql(['/:param/v1$', '/:param/v1/*']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +85,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api$', '/api/*', '/foo$']); + ).to.eql(['/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -74,7 +93,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1$', '/api/v1/*', '/v1/foo$']); + ).to.eql(['/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -92,6 +111,46 @@ describe('RouteInfoPathExtractor', () => { }), ).to.eql(['/api/v1/bar']); }); + + it(`should return correct paths when set global prefix (has :param) and global prefix options`, () => { + Reflect.set(routeInfoPathExtractor, 'prefixPath', '/:param'); + Reflect.set( + routeInfoPathExtractor, + 'excludedGlobalPrefixRoutes', + mapToExcludeRoute(['foo']), + ); + + expect( + routeInfoPathExtractor.extractPathsFrom({ + path: '*', + method: RequestMethod.ALL, + }), + ).to.eql(['/:param$', '/:param/*', '/foo$']); + + expect( + routeInfoPathExtractor.extractPathsFrom({ + path: '*', + method: RequestMethod.ALL, + version: '1', + }), + ).to.eql(['/:param/v1$', '/:param/v1/*', '/v1/foo$']); + + expect( + routeInfoPathExtractor.extractPathsFrom({ + path: 'foo', + method: RequestMethod.ALL, + version: '1', + }), + ).to.eql(['/v1/foo']); + + expect( + routeInfoPathExtractor.extractPathsFrom({ + path: 'bar', + method: RequestMethod.ALL, + version: '1', + }), + ).to.eql(['/:param/v1/bar']); + }); }); describe('extractPathFrom', () => {