Skip to content

Commit

Permalink
Merge pull request #8113 from Zclhlmgqzc/fix-middlewareConsumer-forRo…
Browse files Browse the repository at this point in the history
…utes-controller

fix(core): middlewareConsumer forRoutes controller
  • Loading branch information
kamilmysliwiec committed Oct 5, 2021
2 parents a2edf96 + 8289272 commit 2cdee25
Show file tree
Hide file tree
Showing 6 changed files with 220 additions and 20 deletions.
82 changes: 81 additions & 1 deletion integration/hello-world/e2e/middleware-class.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,17 @@ import {
Injectable,
MiddlewareConsumer,
Module,
Param,
RequestMethod,
} from '@nestjs/common';
import { Test } from '@nestjs/testing';
import * as request from 'supertest';
import { ApplicationModule } from '../src/app.module';
import { Response } from 'express';

const OPTIONAL_PARAM_VALUE = 'test_optional_param';
const FOR_ROUTE_CONTROLLER_VALUE = 'test_for_route_controller';
const FOR_ROUTE_PATH_VALUE = 'test_for_route_path';
const INCLUDED_VALUE = 'test_included';
const RETURN_VALUE = 'test';
const WILDCARD_VALUE = 'test_wildcard';
Expand All @@ -31,16 +35,44 @@ class TestController {
}
}

@Controller(OPTIONAL_PARAM_VALUE)
class TestParamController {
@Get(':test')
[OPTIONAL_PARAM_VALUE]() {
return RETURN_VALUE;
}
}

@Controller()
class ForRouteController {
@Get('for_route_controller')
forRouteController() {
return RETURN_VALUE;
}

@Get('for_route_controller/required_param/:param')
requiredParam() {
return RETURN_VALUE;
}
}

@Module({
imports: [ApplicationModule],
controllers: [TestController],
controllers: [TestController, TestParamController, ForRouteController],
})
class TestModule {
configure(consumer: MiddlewareConsumer) {
consumer
.apply((req, res: Response, next) => res.status(201).end(INCLUDED_VALUE))
.forRoutes({ path: 'tests/included', method: RequestMethod.POST })
.apply((req, res: Response, next) => res.end(FOR_ROUTE_PATH_VALUE))
.forRoutes({ path: 'for_route_path', method: RequestMethod.GET })
.apply((req, res: Response, next) => res.end(FOR_ROUTE_CONTROLLER_VALUE))
.forRoutes(ForRouteController)
.apply((req, res, next) => res.end(OPTIONAL_PARAM_VALUE))
.forRoutes({ path: `${OPTIONAL_PARAM_VALUE}/(:test)?`, method: RequestMethod.GET })
.apply(Middleware)
.exclude({ path: `${OPTIONAL_PARAM_VALUE}/(.*)`, method: RequestMethod.ALL })
.forRoutes('*');
}
}
Expand Down Expand Up @@ -82,6 +114,54 @@ describe('Middleware (class)', () => {
.expect(201, INCLUDED_VALUE);
});

it(`/for_route_path forRoutes(/for_route_path)`, () => {
return request(app.getHttpServer())
.get('/for_route_path')
.expect(200, FOR_ROUTE_PATH_VALUE);
});

it(`/for_route_path/test forRoutes(/for_route_path)`, () => {
return request(app.getHttpServer())
.get('/for_route_path/test')
.expect(200, FOR_ROUTE_PATH_VALUE);
});

it(`/for_route_controller forRoutes(ForRouteController)`, () => {
return request(app.getHttpServer())
.get('/for_route_controller')
.expect(200, FOR_ROUTE_CONTROLLER_VALUE);
});

it(`/for_route_controller/test forRoutes(ForRouteController)`, () => {
return request(app.getHttpServer())
.get('/for_route_controller/test')
.expect(200, WILDCARD_VALUE);
});

it(`/for_route_controller/required_param/ forRoutes(ForRouteController)`, () => {
return request(app.getHttpServer())
.get('/for_route_controller/required_param/')
.expect(200, WILDCARD_VALUE);
});

it(`/for_route_controller/required_param/test forRoutes(ForRouteController)`, () => {
return request(app.getHttpServer())
.get('/for_route_controller/required_param/test')
.expect(200, FOR_ROUTE_CONTROLLER_VALUE);
});

it(`/${OPTIONAL_PARAM_VALUE}/ forRoutes(${OPTIONAL_PARAM_VALUE}/(:test)?)`, () => {
return request(app.getHttpServer())
.get(`/${OPTIONAL_PARAM_VALUE}/`)
.expect(200, OPTIONAL_PARAM_VALUE);
});

it(`/${OPTIONAL_PARAM_VALUE}/test forRoutes(${OPTIONAL_PARAM_VALUE}/(:test)?)`, () => {
return request(app.getHttpServer())
.get(`/${OPTIONAL_PARAM_VALUE}/test`)
.expect(200, OPTIONAL_PARAM_VALUE);
});

afterEach(async () => {
await app.close();
});
Expand Down
124 changes: 120 additions & 4 deletions integration/hello-world/e2e/middleware-fastify.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
Get,
MiddlewareConsumer,
Module,
Param,
Query,
RequestMethod,
} from '@nestjs/common';
Expand All @@ -14,12 +15,16 @@ import { Test } from '@nestjs/testing';
import { expect } from 'chai';
import { ApplicationModule } from '../src/app.module';

const OPTIONAL_PARAM_VALUE = 'test_optional_param';
const FOR_ROUTE_CONTROLLER_VALUE = 'test_for_route_controller';
const FOR_ROUTE_PATH_VALUE = 'test_for_route_path';
const INCLUDED_VALUE = 'test_included';
const QUERY_VALUE = 'test_query';
const REQ_URL_VALUE = 'test_req_url';
const RETURN_VALUE = 'test';
const SCOPED_VALUE = 'test_scoped';
const WILDCARD_VALUE = 'test_wildcard';
const CATCH_ALL_VALUE = 'test_catch_all';

@Controller()
class TestController {
Expand Down Expand Up @@ -57,9 +62,35 @@ class TestQueryController {
}
}

@Controller(OPTIONAL_PARAM_VALUE)
class TestParamController {
@Get(':test')
[OPTIONAL_PARAM_VALUE]() {
return RETURN_VALUE;
}
}

@Controller()
class ForRouteController {
@Get('for_route_controller')
forRouteController() {
return RETURN_VALUE;
}

@Get('for_route_controller/required_param/:param')
requiredParam() {
return RETURN_VALUE;
}
}

@Module({
imports: [ApplicationModule],
controllers: [TestController, TestQueryController],
controllers: [
TestController,
TestQueryController,
TestParamController,
ForRouteController,
],
})
class TestModule {
configure(consumer: MiddlewareConsumer) {
Expand All @@ -76,8 +107,17 @@ class TestModule {
.forRoutes(TestQueryController)
.apply((req, res, next) => res.end(SCOPED_VALUE))
.forRoutes(TestController)
.apply((req, res, next) => res.end(RETURN_VALUE))
.exclude({ path: QUERY_VALUE, method: -1 })
.apply((req, res, next) => res.end(FOR_ROUTE_PATH_VALUE))
.forRoutes({ path: 'for_route_path', method: RequestMethod.GET })
.apply((req, res, next) => res.end(FOR_ROUTE_CONTROLLER_VALUE))
.forRoutes(ForRouteController)
.apply((req, res, next) => res.end(OPTIONAL_PARAM_VALUE))
.forRoutes({ path: `${OPTIONAL_PARAM_VALUE}/:test?`, method: RequestMethod.GET })
.apply((req, res, next) => res.end(CATCH_ALL_VALUE))
.exclude(
{ path: QUERY_VALUE, method: RequestMethod.ALL },
{ path: `${OPTIONAL_PARAM_VALUE}/(.*)`, method: RequestMethod.ALL },
)
.forRoutes('(.*)');
}
}
Expand All @@ -101,7 +141,7 @@ describe('Middleware (FastifyAdapter)', () => {
method: 'GET',
url: '/hello',
})
.then(({ payload }) => expect(payload).to.be.eql(RETURN_VALUE));
.then(({ payload }) => expect(payload).to.be.eql(CATCH_ALL_VALUE));
});

it(`forRoutes(TestController)`, () => {
Expand Down Expand Up @@ -185,6 +225,82 @@ describe('Middleware (FastifyAdapter)', () => {
.then(({ payload }) => expect(payload).to.be.eql(INCLUDED_VALUE));
});

it(`/for_route_path forRoutes(/for_route_path)`, () => {
return app
.inject({
method: 'GET',
url: '/for_route_path',
})
.then(({ payload }) => expect(payload).to.be.eql(FOR_ROUTE_PATH_VALUE));
});

it(`/for_route_path/test forRoutes(/for_route_path)`, () => {
return app
.inject({
method: 'GET',
url: '/for_route_path/test',
})
.then(({ payload }) => expect(payload).to.be.eql(FOR_ROUTE_PATH_VALUE));
});

it(`/for_route_controller forRoutes(ForRouteController)`, () => {
return app
.inject({
method: 'GET',
url: '/for_route_controller',
})
.then(({ payload }) =>
expect(payload).to.be.eql(FOR_ROUTE_CONTROLLER_VALUE),
);
});

it(`/for_route_controller/test forRoutes(ForRouteController)`, () => {
return app
.inject({
method: 'GET',
url: '/for_route_controller/test',
})
.then(({ payload }) => expect(payload).to.be.eql(CATCH_ALL_VALUE));
});

it(`/for_route_controller/required_param/ forRoutes(ForRouteController)`, () => {
return app
.inject({
method: 'GET',
url: '/for_route_controller/required_param/',
})
.then(({ payload }) => expect(payload).to.be.eql(CATCH_ALL_VALUE));
});

it(`/for_route_controller/required_param/test forRoutes(ForRouteController)`, () => {
return app
.inject({
method: 'GET',
url: '/for_route_controller/required_param/test',
})
.then(({ payload }) =>
expect(payload).to.be.eql(FOR_ROUTE_CONTROLLER_VALUE),
);
});

it(`/${OPTIONAL_PARAM_VALUE}/ forRoutes(${OPTIONAL_PARAM_VALUE}/:test?)`, () => {
return app
.inject({
method: 'GET',
url: `/${OPTIONAL_PARAM_VALUE}/`,
})
.then(({ payload }) => expect(payload).to.be.eql(OPTIONAL_PARAM_VALUE));
});

it(`/${OPTIONAL_PARAM_VALUE}/test forRoutes(${OPTIONAL_PARAM_VALUE}/:test?)`, () => {
return app
.inject({
method: 'GET',
url: `/${OPTIONAL_PARAM_VALUE}/test`,
})
.then(({ payload }) => expect(payload).to.be.eql(OPTIONAL_PARAM_VALUE));
});

afterEach(async () => {
await app.close();
});
Expand Down
3 changes: 2 additions & 1 deletion packages/core/middleware/routes-mapper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,8 @@ export class RoutesMapper {
.map(item =>
item.path?.map(p => {
let path = modulePath ?? '';
path += this.normalizeGlobalPath(routePath) + addLeadingSlash(p);
path +=
this.normalizeGlobalPath(routePath) + addLeadingSlash(p) + '$';

return {
path,
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 @@ -45,7 +45,7 @@ describe('MiddlewareBuilder', () => {
},
{
method: 0,
path: '/path/route',
path: '/path/route$',
},
],
},
Expand Down
12 changes: 6 additions & 6 deletions packages/core/test/middleware/routes-mapper.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ describe('RoutesMapper', () => {
{ path: '/test', method: RequestMethod.GET },
]);
expect(mapper.mapRouteToRouteInfo(config.forRoutes[1])).to.deep.equal([
{ path: '/test/test', method: RequestMethod.GET },
{ path: '/test/another', method: RequestMethod.DELETE },
{ path: '/test/test$', method: RequestMethod.GET },
{ path: '/test/another$', method: RequestMethod.DELETE },
]);
});
@Controller(['test', 'test2'])
Expand All @@ -56,10 +56,10 @@ describe('RoutesMapper', () => {
{ path: '/test', method: RequestMethod.GET },
]);
expect(mapper.mapRouteToRouteInfo(config.forRoutes[1])).to.deep.equal([
{ path: '/test/test', method: RequestMethod.GET },
{ path: '/test/another', method: RequestMethod.DELETE },
{ path: '/test2/test', method: RequestMethod.GET },
{ path: '/test2/another', method: RequestMethod.DELETE },
{ path: '/test/test$', method: RequestMethod.GET },
{ path: '/test/another$', method: RequestMethod.DELETE },
{ path: '/test2/test$', method: RequestMethod.GET },
{ path: '/test2/another$', method: RequestMethod.DELETE },
]);
});
});
17 changes: 10 additions & 7 deletions packages/platform-fastify/adapters/fastify-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
InjectOptions,
Response as LightMyRequestResponse,
} from 'light-my-request';
import * as pathToRegexp from 'path-to-regexp';
import {
FastifyStaticOptions,
PointOfViewOptions,
Expand Down Expand Up @@ -414,16 +415,18 @@ export class FastifyAdapter<
await this.registerMiddie();
}
return (path: string, callback: Function) => {
const normalizedPath = path.endsWith('/*')
? `${path.slice(0, -1)}(.*)`
: path;
if (path.endsWith('/*')) {
path = `${path.slice(0, -1)}(.*)`
} else if (path.endsWith('$')) {
path = pathToRegexp(path.slice(0, -1), [], {
end: true,
strict: false
}) as any
}

// The following type assertion is valid as we use import('middie') rather than require('middie')
// ref https://github.com/fastify/middie/pull/55
this.instance.use(
normalizedPath,
callback as Parameters<TInstance['use']>['1'],
);
this.instance.use(path, callback as Parameters<TInstance['use']>['1']);
};
}

Expand Down

0 comments on commit 2cdee25

Please sign in to comment.