From 682db9378f44cd4954324d7d5d312363a40c0c39 Mon Sep 17 00:00:00 2001
From: pnkp
Date: Mon, 9 Jan 2023 10:13:02 +0100
Subject: [PATCH 1/2] fix(core): middleware does not apply to a controller that
has versioning
---
.../e2e/middleware-versioning.spec.ts | 47 +++++++++++++
.../versioning/src/app-v3.controller.ts | 17 +++++
.../src/app-with-middleware.module.ts | 29 ++++++++
.../versioning/src/versioning-middleware.ts | 15 +++++
packages/core/middleware/routes-mapper.ts | 5 +-
packages/core/router/paths-explorer.ts | 20 ++++--
packages/core/router/router-explorer.ts | 5 +-
.../test/middleware/routes-mapper.spec.ts | 67 ++++++++++++++++++-
.../core/test/router/routes-resolver.spec.ts | 2 +-
9 files changed, 199 insertions(+), 8 deletions(-)
create mode 100644 integration/versioning/e2e/middleware-versioning.spec.ts
create mode 100644 integration/versioning/src/app-v3.controller.ts
create mode 100644 integration/versioning/src/app-with-middleware.module.ts
create mode 100644 integration/versioning/src/versioning-middleware.ts
diff --git a/integration/versioning/e2e/middleware-versioning.spec.ts b/integration/versioning/e2e/middleware-versioning.spec.ts
new file mode 100644
index 00000000000..dc4d20cc533
--- /dev/null
+++ b/integration/versioning/e2e/middleware-versioning.spec.ts
@@ -0,0 +1,47 @@
+import { INestApplication, VersioningType } from '@nestjs/common';
+import { Test } from '@nestjs/testing';
+import * as request from 'supertest';
+import { expect } from 'chai';
+import { AppWithMiddlewareModule } from '../src/app-with-middleware.module';
+import * as sinon from 'sinon';
+import { SpyInjectToken } from '../src/versioning-middleware';
+
+describe('MiddlewareVersioning', () => {
+ let app: INestApplication;
+ let spy = sinon.spy();
+
+ before(async () => {
+ const moduleRef = await Test.createTestingModule({
+ imports: [AppWithMiddlewareModule],
+ }).compile();
+
+ app = moduleRef.createNestApplication();
+ app.enableVersioning({
+ type: VersioningType.URI,
+ defaultVersion: '-default-version',
+ });
+ await app.init();
+
+ spy = app.get(SpyInjectToken);
+ });
+
+ beforeEach(() => spy.resetHistory());
+
+ ['/v-default-version/foo/bar', '/v1/', '/v3', '/v4'].forEach(path => {
+ it('should call middleware for route with version', async () => {
+ const response = await request(app.getHttpServer()).get(path);
+
+ expect(response.status).to.eq(200);
+ expect(spy.called).to.be.true;
+ });
+ });
+
+ it('should not call middleware if the controller route is not defined', async () => {
+ const response = await request(app.getHttpServer()).get('/v2/');
+
+ expect(response.status).to.eq(200);
+ expect(spy.called).to.be.false;
+ });
+
+ after(() => app.close());
+});
diff --git a/integration/versioning/src/app-v3.controller.ts b/integration/versioning/src/app-v3.controller.ts
new file mode 100644
index 00000000000..15849e9fcde
--- /dev/null
+++ b/integration/versioning/src/app-v3.controller.ts
@@ -0,0 +1,17 @@
+import { Controller, Get, Version } from '@nestjs/common';
+
+@Controller({
+ version: '3',
+})
+export class AppV3Controller {
+ @Get()
+ paramV3() {
+ return 'V3';
+ }
+
+ @Version('4')
+ @Get()
+ paramV4() {
+ return 'V4';
+ }
+}
diff --git a/integration/versioning/src/app-with-middleware.module.ts b/integration/versioning/src/app-with-middleware.module.ts
new file mode 100644
index 00000000000..5e5c8debb12
--- /dev/null
+++ b/integration/versioning/src/app-with-middleware.module.ts
@@ -0,0 +1,29 @@
+import { MiddlewareConsumer, Module, NestModule } from '@nestjs/common';
+import { SpyInjectToken, VersioningMiddleware } from './versioning-middleware';
+import { AppV1Controller } from './app-v1.controller';
+import * as sinon from 'sinon';
+import { AppV3Controller } from './app-v3.controller';
+import { NoVersioningController } from './no-versioning.controller';
+import { AppV2Controller } from './app-v2.controller';
+
+@Module({
+ providers: [
+ {
+ provide: SpyInjectToken,
+ useValue: sinon.spy(),
+ },
+ ],
+ controllers: [
+ NoVersioningController,
+ AppV1Controller,
+ AppV2Controller,
+ AppV3Controller,
+ ],
+})
+export class AppWithMiddlewareModule implements NestModule {
+ configure(consumer: MiddlewareConsumer) {
+ consumer
+ .apply(VersioningMiddleware)
+ .forRoutes(NoVersioningController, AppV1Controller, AppV3Controller);
+ }
+}
diff --git a/integration/versioning/src/versioning-middleware.ts b/integration/versioning/src/versioning-middleware.ts
new file mode 100644
index 00000000000..1eab470c5b9
--- /dev/null
+++ b/integration/versioning/src/versioning-middleware.ts
@@ -0,0 +1,15 @@
+import { Inject, Injectable, NestMiddleware } from '@nestjs/common';
+import * as sinon from 'sinon';
+import { NextFunction } from 'express';
+
+export const SpyInjectToken = 'SpyInjectToken';
+
+@Injectable()
+export class VersioningMiddleware implements NestMiddleware {
+ constructor(@Inject(SpyInjectToken) private readonly spy: sinon.SinonSpy) {}
+
+ use(req: any, res: any, next: NextFunction): any {
+ this.spy(req, res);
+ next();
+ }
+}
diff --git a/packages/core/middleware/routes-mapper.ts b/packages/core/middleware/routes-mapper.ts
index 5a803f49995..f246ac4cec5 100644
--- a/packages/core/middleware/routes-mapper.ts
+++ b/packages/core/middleware/routes-mapper.ts
@@ -15,7 +15,10 @@ export class RoutesMapper {
private readonly pathsExplorer: PathsExplorer;
constructor(private readonly container: NestContainer) {
- this.pathsExplorer = new PathsExplorer(new MetadataScanner());
+ this.pathsExplorer = new PathsExplorer(
+ new MetadataScanner(),
+ container.applicationConfig?.getVersioning(),
+ );
}
public mapRouteToRouteInfo(
diff --git a/packages/core/router/paths-explorer.ts b/packages/core/router/paths-explorer.ts
index 67ba7d6b4a2..16af303bfcf 100644
--- a/packages/core/router/paths-explorer.ts
+++ b/packages/core/router/paths-explorer.ts
@@ -5,7 +5,10 @@ import {
} from '@nestjs/common/constants';
import { RequestMethod } from '@nestjs/common/enums';
import { Controller } from '@nestjs/common/interfaces/controllers/controller.interface';
-import { VersionValue } from '@nestjs/common/interfaces/version-options.interface';
+import {
+ VersioningOptions,
+ VersionValue,
+} from '@nestjs/common/interfaces/version-options.interface';
import {
addLeadingSlash,
isString,
@@ -23,7 +26,10 @@ export interface RouteDefinition {
}
export class PathsExplorer {
- constructor(private readonly metadataScanner: MetadataScanner) {}
+ constructor(
+ private readonly metadataScanner: MetadataScanner,
+ private readonly versioningOptions?: VersioningOptions,
+ ) {}
public scanForPaths(
instance: Controller,
@@ -55,20 +61,26 @@ export class PathsExplorer {
METHOD_METADATA,
prototypeCallback,
);
- const version: VersionValue | undefined = Reflect.getMetadata(
+ const methodVersion: VersionValue | undefined = Reflect.getMetadata(
VERSION_METADATA,
prototypeCallback,
);
+ const controllerVersion: VersionValue | undefined = Reflect.getMetadata(
+ VERSION_METADATA,
+ prototype.constructor,
+ );
const path = isString(routePath)
? [addLeadingSlash(routePath)]
: routePath.map((p: string) => addLeadingSlash(p));
+ const globalVersion = this.versioningOptions?.defaultVersion;
+
return {
path,
requestMethod,
targetCallback: instanceCallback,
methodName,
- version,
+ version: methodVersion || controllerVersion || globalVersion,
};
}
}
diff --git a/packages/core/router/router-explorer.ts b/packages/core/router/router-explorer.ts
index 843ca5d2d1a..55f90d3851e 100644
--- a/packages/core/router/router-explorer.ts
+++ b/packages/core/router/router-explorer.ts
@@ -73,7 +73,10 @@ export class RouterExplorer {
private readonly routePathFactory: RoutePathFactory,
private readonly graphInspector: GraphInspector,
) {
- this.pathsExplorer = new PathsExplorer(metadataScanner);
+ this.pathsExplorer = new PathsExplorer(
+ metadataScanner,
+ config.getVersioning(),
+ );
const routeParamsFactory = new RouteParamsFactory();
const pipesContextCreator = new PipesContextCreator(container, config);
diff --git a/packages/core/test/middleware/routes-mapper.spec.ts b/packages/core/test/middleware/routes-mapper.spec.ts
index f030a676caf..a36ac91cfcf 100644
--- a/packages/core/test/middleware/routes-mapper.spec.ts
+++ b/packages/core/test/middleware/routes-mapper.spec.ts
@@ -1,4 +1,4 @@
-import { Version } from '../../../common';
+import { Version, VersioningType } from '../../../common';
import { MiddlewareConfiguration } from '../../../common/interfaces';
import { expect } from 'chai';
import { Controller } from '../../../common/decorators/core/controller.decorator';
@@ -9,6 +9,7 @@ import {
import { RequestMethod } from '../../../common/enums/request-method.enum';
import { NestContainer } from '../../injector/container';
import { RoutesMapper } from '../../middleware/routes-mapper';
+import { ApplicationConfig } from '../../../core/application-config';
describe('RoutesMapper', () => {
@Controller('test')
@@ -25,8 +26,17 @@ describe('RoutesMapper', () => {
}
let mapper: RoutesMapper;
+ let mapperWithGlobalVersioning: RoutesMapper;
beforeEach(() => {
mapper = new RoutesMapper(new NestContainer());
+
+ const applicationConfig = new ApplicationConfig();
+ applicationConfig.enableVersioning({
+ defaultVersion: 'defaultVersion1',
+ type: VersioningType.URI,
+ });
+ const container = new NestContainer(applicationConfig);
+ mapperWithGlobalVersioning = new RoutesMapper(container);
});
it('should map @Controller() to "ControllerMetadata" in forRoutes', () => {
@@ -81,4 +91,59 @@ describe('RoutesMapper', () => {
{ path: '/test2/another', method: RequestMethod.DELETE },
]);
});
+
+ @Controller({
+ path: 'test',
+ version: '1',
+ })
+ class TestRouteWithControllerVersion {
+ @RequestMapping({ path: 'test' })
+ public getTest() {}
+
+ @Version('2')
+ @RequestMapping({ path: 'another', method: RequestMethod.DELETE })
+ public getAnother() {}
+ }
+
+ it('should map @Controller with controller version to "ControllerMetadata"', () => {
+ const config = {
+ middleware: 'Test',
+ forRoutes: [TestRouteWithControllerVersion],
+ };
+
+ expect(mapper.mapRouteToRouteInfo(config.forRoutes[0])).to.deep.equal([
+ { path: '/test/test', method: RequestMethod.GET, version: '1' },
+ { path: '/test/another', method: RequestMethod.DELETE, version: '2' },
+ ]);
+ });
+
+ it('should create ControllerMetadata from version defined in @Controller rather than global default version', () => {
+ const config = {
+ middleware: 'Test',
+ forRoutes: [TestRoute, TestRouteWithControllerVersion],
+ };
+
+ expect(
+ mapperWithGlobalVersioning.mapRouteToRouteInfo(config.forRoutes[0]),
+ ).to.deep.equal([
+ {
+ path: '/test/test',
+ method: RequestMethod.GET,
+ version: 'defaultVersion1',
+ },
+ {
+ path: '/test/another',
+ method: RequestMethod.DELETE,
+ version: 'defaultVersion1',
+ },
+ { path: '/test/versioned', method: RequestMethod.GET, version: '1' },
+ ]);
+
+ expect(
+ mapperWithGlobalVersioning.mapRouteToRouteInfo(config.forRoutes[1]),
+ ).to.deep.equal([
+ { path: '/test/test', method: RequestMethod.GET, version: '1' },
+ { path: '/test/another', method: RequestMethod.DELETE, version: '2' },
+ ]);
+ });
});
diff --git a/packages/core/test/router/routes-resolver.spec.ts b/packages/core/test/router/routes-resolver.spec.ts
index 9819f4aaee0..d877fdd7a7d 100644
--- a/packages/core/test/router/routes-resolver.spec.ts
+++ b/packages/core/test/router/routes-resolver.spec.ts
@@ -210,7 +210,7 @@ describe('RoutesResolver', () => {
versioningOptions: {
type: VersioningType.URI,
},
- methodVersion: undefined,
+ methodVersion: '1',
methodPath: '/',
};
From 44eb5b561fcd316cfb3e242d4f3c9d68c3c1776e Mon Sep 17 00:00:00 2001
From: Kamil Mysliwiec
Date: Fri, 3 Feb 2023 10:30:26 +0100
Subject: [PATCH 2/2] Update
packages/core/test/middleware/routes-mapper.spec.ts
---
packages/core/test/middleware/routes-mapper.spec.ts | 1 +
1 file changed, 1 insertion(+)
diff --git a/packages/core/test/middleware/routes-mapper.spec.ts b/packages/core/test/middleware/routes-mapper.spec.ts
index a36ac91cfcf..f34743297ab 100644
--- a/packages/core/test/middleware/routes-mapper.spec.ts
+++ b/packages/core/test/middleware/routes-mapper.spec.ts
@@ -27,6 +27,7 @@ describe('RoutesMapper', () => {
let mapper: RoutesMapper;
let mapperWithGlobalVersioning: RoutesMapper;
+
beforeEach(() => {
mapper = new RoutesMapper(new NestContainer());