From 9fa3d930db7236e2235e6c09cfd3ca98f0b27eb3 Mon Sep 17 00:00:00 2001 From: codytseng Date: Tue, 1 Aug 2023 21:33:00 +0800 Subject: [PATCH 1/5] fix: the root path middleware not applied when setting the global prefix --- .../global-prefix/e2e/global-prefix.spec.ts | 4 ++++ .../global-prefix/src/app.controller.ts | 5 +++++ packages/core/middleware/route-info-path-extractor.ts | 8 ++++++-- .../test/middleware/route-info-path-extractor.spec.ts | 10 +++++----- 4 files changed, 20 insertions(+), 7 deletions(-) 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 74f8e9ad6fc..01973ed17fa 100644 --- a/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts +++ b/integration/nest-application/global-prefix/e2e/global-prefix.spec.ts @@ -28,6 +28,10 @@ describe('Global prefix', () => { await request(server).get('/health').expect(404); await request(server).get('/api/v1/health').expect(200); + + await request(server) + .get('/api/v1') + .expect(200, 'Root: Data attached in middleware'); }); it(`should exclude the path as string`, async () => { diff --git a/integration/nest-application/global-prefix/src/app.controller.ts b/integration/nest-application/global-prefix/src/app.controller.ts index d826cd0dbc2..35444679426 100644 --- a/integration/nest-application/global-prefix/src/app.controller.ts +++ b/integration/nest-application/global-prefix/src/app.controller.ts @@ -2,6 +2,11 @@ import { Controller, Get, Post, Req } from '@nestjs/common'; @Controller() export class AppController { + @Get() + root(@Req() req): string { + return 'Root: ' + req.extras?.data; + } + @Get('hello/:name') getHello(@Req() req): string { return 'Hello: ' + req.extras?.data; diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index c3d2bf8bfb6..c75b9ca29e5 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -33,14 +33,18 @@ export class RouteInfoPathExtractor { const versionPath = this.extractVersionPathFrom(version); if (this.isAWildcard(path)) { + const prefix = addLeadingSlash(this.prefixPath + versionPath); + const basePaths = prefix + ? [prefix, prefix + addLeadingSlash(path)] + : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ - this.prefixPath + versionPath + addLeadingSlash(path), + ...basePaths, ...this.excludedGlobalPrefixRoutes.map( route => versionPath + addLeadingSlash(route.path), ), ] - : [this.prefixPath + versionPath + addLeadingSlash(path)]; + : basePaths; } return [this.extractNonWildcardPathFrom({ path, method, version })]; 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 967b26243bd..e3f258bb2ec 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/*']); + ).to.eql(['/v1', '/v1/*']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api/*']); + ).to.eql(['/api', '/api/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1/*']); + ).to.eql(['/api/v1', '/api/v1/*']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/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/*', '/v1/foo']); + ).to.eql(['/api/v1', '/api/v1/*', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ From edd5eaf7186c47ecfb2b7cb3d58d294c57a8c176 Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 22:17:40 +0800 Subject: [PATCH 2/5] fix: middleware applied twice --- .../core/middleware/route-info-path-extractor.ts | 7 ++----- .../middleware/route-info-path-extractor.spec.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index c75b9ca29e5..2d24625b101 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -34,17 +34,14 @@ export class RouteInfoPathExtractor { if (this.isAWildcard(path)) { const prefix = addLeadingSlash(this.prefixPath + versionPath); - const basePaths = prefix - ? [prefix, prefix + addLeadingSlash(path)] - : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ - ...basePaths, + prefix, ...this.excludedGlobalPrefixRoutes.map( route => versionPath + addLeadingSlash(route.path), ), ] - : basePaths; + : [prefix]; } return [this.extractNonWildcardPathFrom({ path, method, version })]; 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 e3f258bb2ec..c4791e255c3 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -23,7 +23,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/*']); + ).to.eql(['']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1', '/v1/*']); + ).to.eql(['/v1']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/api/*']); + ).to.eql(['/api']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1', '/api/v1/*']); + ).to.eql(['/api/v1']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api', '/api/*', '/foo']); + ).to.eql(['/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', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ From 73ff3ac81dac8dc83f714492d8bb9e21fe2528af Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 23:30:35 +0800 Subject: [PATCH 3/5] Revert "fix: middleware applied twice" This reverts commit edd5eaf7186c47ecfb2b7cb3d58d294c57a8c176. --- .../core/middleware/route-info-path-extractor.ts | 7 +++++-- .../middleware/route-info-path-extractor.spec.ts | 12 ++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/core/middleware/route-info-path-extractor.ts b/packages/core/middleware/route-info-path-extractor.ts index 2d24625b101..c75b9ca29e5 100644 --- a/packages/core/middleware/route-info-path-extractor.ts +++ b/packages/core/middleware/route-info-path-extractor.ts @@ -34,14 +34,17 @@ export class RouteInfoPathExtractor { if (this.isAWildcard(path)) { const prefix = addLeadingSlash(this.prefixPath + versionPath); + const basePaths = prefix + ? [prefix, prefix + addLeadingSlash(path)] + : [addLeadingSlash(path)]; return Array.isArray(this.excludedGlobalPrefixRoutes) ? [ - prefix, + ...basePaths, ...this.excludedGlobalPrefixRoutes.map( route => versionPath + addLeadingSlash(route.path), ), ] - : [prefix]; + : basePaths; } return [this.extractNonWildcardPathFrom({ path, method, version })]; 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 c4791e255c3..e3f258bb2ec 100644 --- a/packages/core/test/middleware/route-info-path-extractor.spec.ts +++ b/packages/core/test/middleware/route-info-path-extractor.spec.ts @@ -23,7 +23,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['']); + ).to.eql(['/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -31,7 +31,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/v1']); + ).to.eql(['/v1', '/v1/*']); }); it(`should return correct paths when set global prefix`, () => { @@ -42,7 +42,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/api']); + ).to.eql(['/api', '/api/*']); expect( routeInfoPathExtractor.extractPathsFrom({ @@ -50,7 +50,7 @@ describe('RouteInfoPathExtractor', () => { method: RequestMethod.ALL, version: '1', }), - ).to.eql(['/api/v1']); + ).to.eql(['/api/v1', '/api/v1/*']); }); it(`should return correct paths when set global prefix and global prefix options`, () => { @@ -66,7 +66,7 @@ describe('RouteInfoPathExtractor', () => { path: '*', method: RequestMethod.ALL, }), - ).to.eql(['/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', '/v1/foo']); + ).to.eql(['/api/v1', '/api/v1/*', '/v1/foo']); expect( routeInfoPathExtractor.extractPathsFrom({ From 3b100cc97142066b2f6f6cdbdb891099511486d5 Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 23:32:24 +0800 Subject: [PATCH 4/5] feat: change middleware default request method --- packages/core/middleware/routes-mapper.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/core/middleware/routes-mapper.ts b/packages/core/middleware/routes-mapper.ts index 90193250a35..0bfa8ade8bb 100644 --- a/packages/core/middleware/routes-mapper.ts +++ b/packages/core/middleware/routes-mapper.ts @@ -20,6 +20,7 @@ import { Module } from '../injector/module'; import { MetadataScanner } from '../metadata-scanner'; import { PathsExplorer, RouteDefinition } from '../router/paths-explorer'; import { targetModulesByContainer } from '../router/router-module'; +import { RequestMethod } from '@nestjs/common'; export class RoutesMapper { private readonly pathsExplorer: PathsExplorer; @@ -46,7 +47,7 @@ export class RoutesMapper { } private getRouteInfoFromPath(routePath: string): RouteInfo[] { - const defaultRequestMethod = -1; + const defaultRequestMethod = RequestMethod.ALL; return [ { path: addLeadingSlash(routePath), From 16106e084c06656409c393fd897f222ce0c43670 Mon Sep 17 00:00:00 2001 From: codytseng Date: Thu, 24 Aug 2023 23:56:53 +0800 Subject: [PATCH 5/5] test: default request method --- packages/core/test/middleware/builder.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/core/test/middleware/builder.spec.ts b/packages/core/test/middleware/builder.spec.ts index fa73acae66a..f3854a5bf1d 100644 --- a/packages/core/test/middleware/builder.spec.ts +++ b/packages/core/test/middleware/builder.spec.ts @@ -193,7 +193,7 @@ describe('MiddlewareBuilder', () => { expect(proxy.getExcludedRoutes()).to.be.eql([ { path, - method: -1 as any, + method: RequestMethod.ALL, }, ]); });