Skip to content

Commit

Permalink
Merge pull request #13337 from nestjs/fix/global-prefix-midleware-issue
Browse files Browse the repository at this point in the history
fix(core): middleware is not executed for root route when global prefix is set
  • Loading branch information
kamilmysliwiec committed Mar 18, 2024
2 parents 9dda2cd + 3321f6c commit 40800d3
Show file tree
Hide file tree
Showing 7 changed files with 29 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,9 @@ describe('Global prefix', () => {
server = app.getHttpServer();
await app.init();

await request(server).get('/').expect(200, '1');
await request(server)
.get('/')
.expect(200, 'Extras: Data attached in middleware, Count: 1');
await request(server).get('/api/count').expect(200, '2');
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export class AppController {

@Get()
getHome(@Req() req) {
return req.count;
return 'Extras: ' + req.extras?.data + ', Count: ' + req.count;
}

@Get('count')
Expand Down
2 changes: 2 additions & 0 deletions packages/core/middleware/middleware-module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -190,12 +190,14 @@ export class MiddlewareModule<
for (const metatype of middlewareCollection) {
const collection = middlewareContainer.getMiddlewareCollection(moduleKey);
const instanceWrapper = collection.get(metatype);

if (isUndefined(instanceWrapper)) {
throw new RuntimeException();
}
if (instanceWrapper.isTransient) {
return;
}

this.graphInspector.insertClassNode(
moduleRef,
instanceWrapper,
Expand Down
12 changes: 8 additions & 4 deletions packages/core/middleware/route-info-path-extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ export class RouteInfoPathExtractor {
if (this.isAWildcard(path)) {
const entries =
versionPaths.length > 0
? versionPaths.map(
versionPath =>
? versionPaths
.map(versionPath => [
this.prefixPath + versionPath + '$',
this.prefixPath + versionPath + addLeadingSlash(path),
)
: [this.prefixPath + addLeadingSlash(path)];
])
.flat()
: this.prefixPath
? [this.prefixPath + '$', this.prefixPath + addLeadingSlash(path)]
: [addLeadingSlash(path)];

return Array.isArray(this.excludedGlobalPrefixRoutes)
? [
Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/middleware/builder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ describe('MiddlewareBuilder', () => {
expect(proxy.getExcludedRoutes()).to.be.eql([
{
path,
method: -1 as any,
method: -1,
},
]);
});
Expand Down
10 changes: 5 additions & 5 deletions packages/core/test/middleware/route-info-path-extractor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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`, () => {
Expand All @@ -42,15 +42,15 @@ describe('RouteInfoPathExtractor', () => {
path: '*',
method: RequestMethod.ALL,
}),
).to.eql(['/api/*']);
).to.eql(['/api$', '/api/*']);

expect(
routeInfoPathExtractor.extractPathsFrom({
path: '*',
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`, () => {
Expand All @@ -66,15 +66,15 @@ describe('RouteInfoPathExtractor', () => {
path: '*',
method: RequestMethod.ALL,
}),
).to.eql(['/api/*', '/foo']);
).to.eql(['/api$', '/api/*', '/foo']);

expect(
routeInfoPathExtractor.extractPathsFrom({
path: '*',
method: RequestMethod.ALL,
version: '1',
}),
).to.eql(['/api/v1/*', '/v1/foo']);
).to.eql(['/api/v1$', '/api/v1/*', '/v1/foo']);

expect(
routeInfoPathExtractor.extractPathsFrom({
Expand Down
14 changes: 9 additions & 5 deletions packages/platform-fastify/adapters/fastify-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,15 @@ import {
import * as pathToRegexp from 'path-to-regexp';
// `querystring` is used internally in fastify for registering urlencoded body parser.
import { parse as querystringParse } from 'querystring';
import {
FASTIFY_ROUTE_CONFIG_METADATA,
FASTIFY_ROUTE_CONSTRAINTS_METADATA,
} from '../constants';
import { NestFastifyBodyParserOptions } from '../interfaces';
import {
FastifyStaticOptions,
FastifyViewOptions,
} from '../interfaces/external';
import {
FASTIFY_ROUTE_CONFIG_METADATA,
FASTIFY_ROUTE_CONSTRAINTS_METADATA,
} from '../constants';

type FastifyHttp2SecureOptions<
Server extends http2.Http2SecureServer,
Expand Down Expand Up @@ -554,14 +554,18 @@ export class FastifyAdapter<
await this.registerMiddie();
}
return (path: string, callback: Function) => {
const hasEndOfStringCharacter = path.endsWith('$');
path = hasEndOfStringCharacter ? path.slice(0, -1) : path;

let normalizedPath = path.endsWith('/*')
? `${path.slice(0, -1)}(.*)`
: path;

// Fallback to "(.*)" to support plugins like GraphQL
normalizedPath = normalizedPath === '/(.*)' ? '(.*)' : normalizedPath;

const re = pathToRegexp(normalizedPath);
let re = pathToRegexp(normalizedPath);
re = hasEndOfStringCharacter ? new RegExp(re.source + '$', re.flags) : re;

// The following type assertion is valid as we use import('@fastify/middie') rather than require('@fastify/middie')
// ref https://github.com/fastify/middie/pull/55
Expand Down

0 comments on commit 40800d3

Please sign in to comment.