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

Feature/8844 api version in route info #10484

Merged
70 changes: 70 additions & 0 deletions integration/hello-world/e2e/middleware-with-versioning.spec.ts
@@ -0,0 +1,70 @@
import {
Controller,
Get,
INestApplication,
MiddlewareConsumer,
Module,
Version,
RequestMethod,
VersioningType,
VERSION_NEUTRAL,
} from '@nestjs/common';
import { Test } from '@nestjs/testing';
import * as request from 'supertest';
import { AppModule } from '../src/app.module';

const RETURN_VALUE = 'test';
const VERSIONED_VALUE = 'test_versioned';

@Controller()
class TestController {
@Version('1')
@Get('versioned')
versionedTest() {
return RETURN_VALUE;
}
}

@Module({
imports: [AppModule],
controllers: [TestController],
})
class TestModule {
configure(consumer: MiddlewareConsumer) {
consumer
.apply((req, res, next) => res.send(VERSIONED_VALUE))
.forRoutes({
path: '/versioned',
version: '1',
method: RequestMethod.ALL,
});
}
}

describe('Middleware', () => {
let app: INestApplication;

beforeEach(async () => {
app = (
await Test.createTestingModule({
imports: [TestModule],
}).compile()
).createNestApplication();

app.enableVersioning({
type: VersioningType.URI,
defaultVersion: VERSION_NEUTRAL,
});
await app.init();
});

it(`forRoutes({ path: 'tests/versioned', version: '1', method: RequestMethod.ALL })`, () => {
return request(app.getHttpServer())
.get('/v1/versioned')
.expect(200, VERSIONED_VALUE);
});

afterEach(async () => {
await app.close();
});
});
@@ -1,9 +1,11 @@
import { RequestMethod } from '../../enums';
import { Type } from '../type.interface';
import { VersionValue } from '../version-options.interface';

export interface RouteInfo {
path: string;
method: RequestMethod;
version?: VersionValue;
}

export interface MiddlewareConfiguration<T = any> {
Expand Down
19 changes: 10 additions & 9 deletions packages/core/middleware/middleware-module.ts
Expand Up @@ -174,8 +174,7 @@ export class MiddlewareModule {
await this.bindHandler(
instanceWrapper,
applicationRef,
routeInfo.method,
routeInfo.path,
routeInfo,
moduleRef,
collection,
);
Expand All @@ -185,8 +184,7 @@ export class MiddlewareModule {
private async bindHandler(
wrapper: InstanceWrapper<NestMiddleware>,
applicationRef: HttpServer,
method: RequestMethod,
path: string,
routeInfo: RouteInfo,
moduleRef: Module,
collection: Map<InstanceToken, InstanceWrapper>,
) {
Expand All @@ -197,12 +195,11 @@ export class MiddlewareModule {
const isStatic = wrapper.isDependencyTreeStatic();
if (isStatic) {
const proxy = await this.createProxy(instance);
return this.registerHandler(applicationRef, method, path, proxy);
return this.registerHandler(applicationRef, routeInfo, proxy);
}
await this.registerHandler(
applicationRef,
method,
path,
routeInfo,
async <TRequest, TResponse>(
req: TRequest,
res: TResponse,
Expand Down Expand Up @@ -266,8 +263,7 @@ export class MiddlewareModule {

private async registerHandler(
applicationRef: HttpServer,
method: RequestMethod,
path: string,
{ path, method, version }: RouteInfo,
proxy: <TRequest, TResponse>(
req: TRequest,
res: TResponse,
Expand All @@ -291,6 +287,11 @@ export class MiddlewareModule {
}
path = basePath + path;
}

if (version) {
path = `/v${version.toString()}${path}`;
}

const isMethodAll = isRequestMethodAll(method);
const requestMethod = RequestMethod[method];
const router = await applicationRef.createMiddlewareFactory(method);
Expand Down
63 changes: 44 additions & 19 deletions packages/core/middleware/routes-mapper.ts
@@ -1,5 +1,5 @@
import { MODULE_PATH, PATH_METADATA } from '@nestjs/common/constants';
import { RouteInfo, Type } from '@nestjs/common/interfaces';
import { RouteInfo, Type, VersionValue } from '@nestjs/common/interfaces';
import {
addLeadingSlash,
isString,
Expand All @@ -22,46 +22,71 @@ export class RoutesMapper {
route: Type<any> | RouteInfo | string,
): RouteInfo[] {
if (isString(route)) {
const defaultRequestMethod = -1;
return [
{
path: addLeadingSlash(route),
method: defaultRequestMethod,
},
];
return this.getRouteInfoFromPath(route);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I extracted many logic branches here to private methods to improve the public method's readability.

}
const routePathOrPaths = this.getRoutePath(route);
if (this.isRouteInfo(routePathOrPaths, route)) {
return [
{
path: addLeadingSlash(route.path),
method: route.method,
},
];
return this.getRouteInfoFromObject(route);
}

return this.getRouteInfoFromController(route, routePathOrPaths);
}

private getRouteInfoFromPath(routePath: string): RouteInfo[] {
const defaultRequestMethod = -1;
return [
{
path: addLeadingSlash(routePath),
method: defaultRequestMethod,
},
];
}

private getRouteInfoFromObject(routeInfoObject: RouteInfo): RouteInfo[] {
const routeInfo: RouteInfo = {
path: addLeadingSlash(routeInfoObject.path),
method: routeInfoObject.method,
};

if (routeInfoObject.version) {
routeInfo.version = routeInfoObject.version;
}
return [routeInfo];
}

private getRouteInfoFromController(
controller: Type<any>,
routePath: string,
): RouteInfo[] {
const controllerPaths = this.routerExplorer.scanForPaths(
Object.create(route),
route.prototype,
Object.create(controller),
controller.prototype,
);
const moduleRef = this.getHostModuleOfController(route);
const moduleRef = this.getHostModuleOfController(controller);
const modulePath = this.getModulePath(moduleRef?.metatype);

const concatPaths = <T>(acc: T[], currentValue: T[]) =>
acc.concat(currentValue);

return []
.concat(routePathOrPaths)
.concat(routePath)
.map(routePath =>
controllerPaths
.map(item =>
item.path?.map(p => {
let path = modulePath ?? '';
path += this.normalizeGlobalPath(routePath) + addLeadingSlash(p);

return {
const routeInfo: RouteInfo = {
path,
method: item.requestMethod,
};

if (item.version) {
routeInfo.version = item.version;
}

return routeInfo;
}),
)
.reduce(concatPaths, []),
Expand Down
17 changes: 13 additions & 4 deletions packages/core/test/middleware/builder.spec.ts
@@ -1,5 +1,5 @@
import { expect } from 'chai';
import { Controller, Get } from '../../../common';
import { Controller, Get, RequestMethod, Version } from '../../../common';
import { NestContainer } from '../../injector/container';
import { MiddlewareBuilder } from '../../middleware/builder';
import { RoutesMapper } from '../../middleware/routes-mapper';
Expand Down Expand Up @@ -30,8 +30,12 @@ describe('MiddlewareBuilder', () => {
class Test {
@Get('route')
public getAll() {}

@Version('1')
@Get('versioned')
public getAllVersioned() {}
}
const route = { path: '/test', method: 0 };
const route = { path: '/test', method: RequestMethod.GET };
it('should store configuration passed as argument', () => {
configProxy.forRoutes(route, Test);

Expand All @@ -40,13 +44,18 @@ describe('MiddlewareBuilder', () => {
middleware: [],
forRoutes: [
{
method: 0,
method: RequestMethod.GET,
path: route.path,
},
{
method: 0,
method: RequestMethod.GET,
path: '/path/route',
},
{
method: RequestMethod.GET,
path: '/path/versioned',
version: '1',
},
],
},
]);
Expand Down
25 changes: 22 additions & 3 deletions packages/core/test/middleware/routes-mapper.spec.ts
@@ -1,6 +1,11 @@
import { Version } from '../../../common';
import { MiddlewareConfiguration } from '../../../common/interfaces';
import { expect } from 'chai';
import { Controller } from '../../../common/decorators/core/controller.decorator';
import { RequestMapping } from '../../../common/decorators/http/request-mapping.decorator';
import {
Get,
RequestMapping,
} from '../../../common/decorators/http/request-mapping.decorator';
import { RequestMethod } from '../../../common/enums/request-method.enum';
import { NestContainer } from '../../injector/container';
import { RoutesMapper } from '../../middleware/routes-mapper';
Expand All @@ -13,6 +18,10 @@ describe('RoutesMapper', () => {

@RequestMapping({ path: 'another', method: RequestMethod.DELETE })
public getAnother() {}

@Version('1')
@Get('versioned')
public getVersioned() {}
}

let mapper: RoutesMapper;
Expand All @@ -21,17 +30,27 @@ describe('RoutesMapper', () => {
});

it('should map @Controller() to "ControllerMetadata" in forRoutes', () => {
const config = {
const config: MiddlewareConfiguration = {
middleware: 'Test',
forRoutes: [{ path: 'test', method: RequestMethod.GET }, TestRoute],
forRoutes: [
{ path: 'test', method: RequestMethod.GET },
{ path: 'versioned', version: '1', method: RequestMethod.GET },
TestRoute,
],
};

expect(mapper.mapRouteToRouteInfo(config.forRoutes[0])).to.deep.equal([
{ path: '/test', method: RequestMethod.GET },
]);

expect(mapper.mapRouteToRouteInfo(config.forRoutes[1])).to.deep.equal([
{ path: '/versioned', version: '1', method: RequestMethod.GET },
]);

expect(mapper.mapRouteToRouteInfo(config.forRoutes[2])).to.deep.equal([
{ path: '/test/test', method: RequestMethod.GET },
{ path: '/test/another', method: RequestMethod.DELETE },
{ path: '/test/versioned', method: RequestMethod.GET, version: '1' },
]);
});
@Controller(['test', 'test2'])
Expand Down