Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(core): middleware is not executed for root route when global prefix is set #13337

Merged
merged 8 commits into from
Mar 18, 2024
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
Loading