Skip to content

Commit

Permalink
fix(core): middleware is not executed for root route with prefix
Browse files Browse the repository at this point in the history
  • Loading branch information
kamilmysliwiec committed Mar 18, 2024
1 parent 7d8822c commit 3321f6c
Show file tree
Hide file tree
Showing 8 changed files with 24 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,6 @@ 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 () => {
Expand Down Expand Up @@ -140,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 @@ -2,11 +2,6 @@ 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;
Expand Down Expand Up @@ -34,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
4 changes: 2 additions & 2 deletions packages/core/middleware/route-info-path-extractor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,12 +37,12 @@ export class RouteInfoPathExtractor {
versionPaths.length > 0
? versionPaths
.map(versionPath => [
this.prefixPath + versionPath,
this.prefixPath + versionPath + '$',
this.prefixPath + versionPath + addLeadingSlash(path),
])
.flat()
: this.prefixPath
? [this.prefixPath, this.prefixPath + addLeadingSlash(path)]
? [this.prefixPath + '$', this.prefixPath + addLeadingSlash(path)]
: [addLeadingSlash(path)];

return Array.isArray(this.excludedGlobalPrefixRoutes)
Expand Down
3 changes: 1 addition & 2 deletions packages/core/middleware/routes-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ 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;
Expand All @@ -47,7 +46,7 @@ export class RoutesMapper {
}

private getRouteInfoFromPath(routePath: string): RouteInfo[] {
const defaultRequestMethod = RequestMethod.ALL;
const defaultRequestMethod = -1;
return [
{
path: addLeadingSlash(routePath),
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: RequestMethod.ALL,
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', '/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', '/api/*']);
).to.eql(['/api$', '/api/*']);

expect(
routeInfoPathExtractor.extractPathsFrom({
path: '*',
method: RequestMethod.ALL,
version: '1',
}),
).to.eql(['/api/v1', '/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', '/api/*', '/foo']);
).to.eql(['/api$', '/api/*', '/foo']);

expect(
routeInfoPathExtractor.extractPathsFrom({
path: '*',
method: RequestMethod.ALL,
version: '1',
}),
).to.eql(['/api/v1', '/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 3321f6c

Please sign in to comment.