From 07e17a4ca9e5c76e13ea138535fed710e67824dd Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Mon, 16 May 2022 20:19:10 -0400 Subject: [PATCH 1/4] fix(core,platform-express): when version is an array with neutral on header and media type versioning, on the express adapter --- packages/core/router/router-explorer.ts | 23 +++++++++++++++---- .../adapters/express-adapter.ts | 21 ++++++++++++++--- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/packages/core/router/router-explorer.ts b/packages/core/router/router-explorer.ts index ee509718b7c..ab0517dc50b 100644 --- a/packages/core/router/router-explorer.ts +++ b/packages/core/router/router-explorer.ts @@ -338,8 +338,7 @@ export class RouterExplorer { return router.applyVersionFilter(handler, version, versioningOptions); } /** - * This can be removed in the next major release. - * Left for backward-compatibility. + * TODO(v9): This was left for backward-compatibility and can be removed. */ return = any, TResponse = any>( req: TRequest, @@ -403,9 +402,16 @@ export class RouterExplorer { const acceptHeaderVersionParameter = acceptHeaderValue ? acceptHeaderValue.split(';')[1] - : ''; + : undefined; - if (acceptHeaderVersionParameter) { + // No version was supplied + if (isUndefined(acceptHeaderVersionParameter)) { + if (Array.isArray(version)) { + if (version.includes(VERSION_NEUTRAL)) { + return handler(req, res, next); + } + } + } else { const headerVersion = acceptHeaderVersionParameter.split( versioningOptions.key, )[1]; @@ -427,7 +433,14 @@ export class RouterExplorer { req.headers?.[versioningOptions.header] || req.headers?.[versioningOptions.header.toLowerCase()]; - if (customHeaderVersionParameter) { + // No version was supplied + if (isUndefined(customHeaderVersionParameter)) { + if (Array.isArray(version)) { + if (version.includes(VERSION_NEUTRAL)) { + return handler(req, res, next); + } + } + } else { if (Array.isArray(version)) { if (version.includes(customHeaderVersionParameter)) { return handler(req, res, next); diff --git a/packages/platform-express/adapters/express-adapter.ts b/packages/platform-express/adapters/express-adapter.ts index 7a832e0ffc1..b8dcf93402e 100644 --- a/packages/platform-express/adapters/express-adapter.ts +++ b/packages/platform-express/adapters/express-adapter.ts @@ -19,6 +19,7 @@ import { isNil, isObject, isString, + isUndefined, } from '@nestjs/common/utils/shared.utils'; import { AbstractHttpAdapter } from '@nestjs/core/adapters/http-adapter'; import { RouterMethodFactory } from '@nestjs/core/helpers/router-method-factory'; @@ -256,9 +257,16 @@ export class ExpressAdapter extends AbstractHttpAdapter { const acceptHeaderVersionParameter = acceptHeaderValue ? acceptHeaderValue.split(';')[1] - : ''; + : undefined; - if (acceptHeaderVersionParameter) { + // No version was supplied + if (isUndefined(acceptHeaderVersionParameter)) { + if (Array.isArray(version)) { + if (version.includes(VERSION_NEUTRAL)) { + return handler(req, res, next); + } + } + } else { const headerVersion = acceptHeaderVersionParameter.split( versioningOptions.key, )[1]; @@ -280,7 +288,14 @@ export class ExpressAdapter extends AbstractHttpAdapter { req.headers?.[versioningOptions.header] || req.headers?.[versioningOptions.header.toLowerCase()]; - if (customHeaderVersionParameter) { + // No version was supplied + if (isUndefined(customHeaderVersionParameter)) { + if (Array.isArray(version)) { + if (version.includes(VERSION_NEUTRAL)) { + return handler(req, res, next); + } + } + } else { if (Array.isArray(version)) { if (version.includes(customHeaderVersionParameter)) { return handler(req, res, next); From 5467777e9b70d372c8d3f3c1d9eff23b3d3a83f7 Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Mon, 16 May 2022 21:27:06 -0400 Subject: [PATCH 2/4] fix(platform-fastify): when version is an array with neutral --- .../platform-fastify/adapters/fastify-adapter.ts | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/platform-fastify/adapters/fastify-adapter.ts b/packages/platform-fastify/adapters/fastify-adapter.ts index fe668b1c391..ebf64274ee3 100644 --- a/packages/platform-fastify/adapters/fastify-adapter.ts +++ b/packages/platform-fastify/adapters/fastify-adapter.ts @@ -154,12 +154,9 @@ export class FastifyAdapter< ? acceptHeaderValue.split(';')[1] : ''; - if (acceptHeaderVersionParameter) { - const headerVersion = acceptHeaderVersionParameter.split( - this.versioningOptions.key, - )[1]; - return headerVersion; - } + return isUndefined(acceptHeaderVersionParameter) + ? VERSION_NEUTRAL // No version was supplied + : acceptHeaderVersionParameter.split(this.versioningOptions.key)[1]; } // Header Versioning Handler else if (this.versioningOptions.type === VersioningType.HEADER) { @@ -167,9 +164,9 @@ export class FastifyAdapter< req.headers?.[this.versioningOptions.header] || req.headers?.[this.versioningOptions.header.toLowerCase()]; - if (customHeaderVersionParameter) { - return customHeaderVersionParameter; - } + return isUndefined(customHeaderVersionParameter) + ? VERSION_NEUTRAL // No version was supplied + : customHeaderVersionParameter; } // Custom Versioning Handler else if (this.versioningOptions.type === VersioningType.CUSTOM) { From 46ec9770b97968b2d0dfe7f395a5fa8a813383ea Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Mon, 16 May 2022 21:49:05 -0400 Subject: [PATCH 3/4] test(core): improve typings on `RouterExplorer` suite --- .../core/test/router/router-explorer.spec.ts | 72 +++++++++---------- 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/packages/core/test/router/router-explorer.spec.ts b/packages/core/test/router/router-explorer.spec.ts index e1977f36ef5..2bb94ee045b 100644 --- a/packages/core/test/router/router-explorer.spec.ts +++ b/packages/core/test/router/router-explorer.spec.ts @@ -365,38 +365,38 @@ describe('RouterExplorer', () => { }); describe('applyVersionFilter', () => { - describe('when the version is VERSION_NEUTRAL', () => { - it('should return the handler', () => { - const version = VERSION_NEUTRAL as VersionValue; - const versioningOptions: VersioningOptions = { - type: VersioningType.URI, - }; - const handler = sinon.stub(); + describe('when the versioning type is URI', () => { + describe('and the version is VERSION_NEUTRAL', () => { + it('should return the handler', () => { + const version: VersionValue = VERSION_NEUTRAL; + const versioningOptions: RoutePathMetadata['versioningOptions'] = { + type: VersioningType.URI, + }; + const handler = sinon.stub(); - const routePathMetadata: RoutePathMetadata = { - methodVersion: version, - versioningOptions, - }; - const versionFilter = (routerBuilder as any).applyVersionFilter( - null, - routePathMetadata, - handler, - ); + const routePathMetadata: RoutePathMetadata = { + methodVersion: version, + versioningOptions, + }; + const versionFilter = (routerBuilder as any).applyVersionFilter( + null, + routePathMetadata, + handler, + ); - const req = {}; - const res = {}; - const next = sinon.stub(); + const req = {}; + const res = {}; + const next = sinon.stub(); - versionFilter(req, res, next); + versionFilter(req, res, next); - expect(handler.calledWith(req, res, next)).to.be.true; + expect(handler.calledWith(req, res, next)).to.be.true; + }); }); - }); - describe('when the versioning type is URI', () => { it('should return the handler', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.URI, }; const handler = sinon.stub(); @@ -423,7 +423,7 @@ describe('RouterExplorer', () => { describe('when the versioning type is MEDIA_TYPE', () => { it('should return next if there is no Media Type header', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.MEDIA_TYPE, key: 'v=', }; @@ -450,7 +450,7 @@ describe('RouterExplorer', () => { it('should return next if there is no version in the Media Type header', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.MEDIA_TYPE, key: 'v=', }; @@ -478,7 +478,7 @@ describe('RouterExplorer', () => { describe('when the handler version is an array', () => { it('should return next if the version in the Media Type header does not match the handler version', () => { const version = ['1', '2']; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.MEDIA_TYPE, key: 'v=', }; @@ -505,7 +505,7 @@ describe('RouterExplorer', () => { it('should return the handler if the version in the Media Type header matches the handler version', () => { const version = ['1', '2']; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.MEDIA_TYPE, key: 'v=', }; @@ -534,7 +534,7 @@ describe('RouterExplorer', () => { describe('when the handler version is a string', () => { it('should return next if the version in the Media Type header does not match the handler version', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.MEDIA_TYPE, key: 'v=', }; @@ -561,7 +561,7 @@ describe('RouterExplorer', () => { it('should return the handler if the version in the Media Type header matches the handler version', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.MEDIA_TYPE, key: 'v=', }; @@ -767,7 +767,7 @@ describe('RouterExplorer', () => { describe('when the versioning type is HEADER', () => { it('should return next if there is no Custom Header', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.HEADER, header: 'X-API-Version', }; @@ -794,7 +794,7 @@ describe('RouterExplorer', () => { it('should return next if there is no version in the Custom Header', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.HEADER, header: 'X-API-Version', }; @@ -822,7 +822,7 @@ describe('RouterExplorer', () => { describe('when the handler version is an array', () => { it('should return next if the version in the Custom Header does not match the handler version', () => { const version = ['1', '2']; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.HEADER, header: 'X-API-Version', }; @@ -849,7 +849,7 @@ describe('RouterExplorer', () => { it('should return the handler if the version in the Custom Header matches the handler version', () => { const version = ['1', '2']; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.HEADER, header: 'X-API-Version', }; @@ -878,7 +878,7 @@ describe('RouterExplorer', () => { describe('when the handler version is a string', () => { it('should return next if the version in the Custom Header does not match the handler version', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.HEADER, header: 'X-API-Version', }; @@ -905,7 +905,7 @@ describe('RouterExplorer', () => { it('should return the handler if the version in the Custom Header matches the handler version', () => { const version = '1'; - const versioningOptions: VersioningOptions = { + const versioningOptions: RoutePathMetadata['versioningOptions'] = { type: VersioningType.HEADER, header: 'X-API-Version', }; From 15737e0622b1d2ee78b5a4e964136e54025cdf2d Mon Sep 17 00:00:00 2001 From: "Micael Levi (@micalevisk)" Date: Mon, 16 May 2022 22:07:53 -0400 Subject: [PATCH 4/4] test: add tests to cover version array with neutral --- .../core/test/router/router-explorer.spec.ts | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/packages/core/test/router/router-explorer.spec.ts b/packages/core/test/router/router-explorer.spec.ts index 2bb94ee045b..5f534321007 100644 --- a/packages/core/test/router/router-explorer.spec.ts +++ b/packages/core/test/router/router-explorer.spec.ts @@ -476,6 +476,61 @@ describe('RouterExplorer', () => { }); describe('when the handler version is an array', () => { + describe('and the version has VERSION_NEUTRAL', () => { + it('should return the handler if there is no version in the Media Type header', () => { + const version: VersionValue = [VERSION_NEUTRAL]; + const versioningOptions: RoutePathMetadata['versioningOptions'] = { + type: VersioningType.MEDIA_TYPE, + key: 'v=', + }; + const handler = sinon.stub(); + + const routePathMetadata: RoutePathMetadata = { + methodVersion: version, + versioningOptions, + }; + const versionFilter = (routerBuilder as any).applyVersionFilter( + null, + routePathMetadata, + handler, + ); + + const req = {}; + const res = {}; + const next = sinon.stub(); + + versionFilter(req, res, next); + + expect(handler.calledWith(req, res, next)).to.be.true; + }); + it('should return next if the version in the Media Type header does not match the handler version', () => { + const version: VersionValue = ['1', '2', VERSION_NEUTRAL]; + const versioningOptions: RoutePathMetadata['versioningOptions'] = { + type: VersioningType.MEDIA_TYPE, + key: 'v=', + }; + const handler = sinon.stub(); + + const routePathMetadata: RoutePathMetadata = { + methodVersion: version, + versioningOptions, + }; + const versionFilter = (routerBuilder as any).applyVersionFilter( + null, + routePathMetadata, + handler, + ); + + const req = { headers: { accept: 'application/json;v=3' } }; + const res = {}; + const next = sinon.stub(); + + versionFilter(req, res, next); + + expect(next.called).to.be.true; + }); + }); + it('should return next if the version in the Media Type header does not match the handler version', () => { const version = ['1', '2']; const versioningOptions: RoutePathMetadata['versioningOptions'] = { @@ -820,6 +875,61 @@ describe('RouterExplorer', () => { }); describe('when the handler version is an array', () => { + describe('and the version has VERSION_NEUTRAL', () => { + it('should return the handler if there is no version in the Custom Header', () => { + const version: VersionValue = [VERSION_NEUTRAL]; + const versioningOptions: RoutePathMetadata['versioningOptions'] = { + type: VersioningType.HEADER, + header: 'X-API-Version', + }; + const handler = sinon.stub(); + + const routePathMetadata: RoutePathMetadata = { + methodVersion: version, + versioningOptions, + }; + const versionFilter = (routerBuilder as any).applyVersionFilter( + null, + routePathMetadata, + handler, + ); + + const req = {}; + const res = {}; + const next = sinon.stub(); + + versionFilter(req, res, next); + + expect(handler.calledWith(req, res, next)).to.be.true; + }); + it('should return next if the version in the Custom Header does not match the handler version', () => { + const version: VersionValue = ['1', '2', VERSION_NEUTRAL]; + const versioningOptions: RoutePathMetadata['versioningOptions'] = { + type: VersioningType.HEADER, + header: 'X-API-Version', + }; + const handler = sinon.stub(); + + const routePathMetadata: RoutePathMetadata = { + methodVersion: version, + versioningOptions, + }; + const versionFilter = (routerBuilder as any).applyVersionFilter( + null, + routePathMetadata, + handler, + ); + + const req = { headers: { 'X-API-Version': '3' } }; + const res = {}; + const next = sinon.stub(); + + versionFilter(req, res, next); + + expect(next.called).to.be.true; + }); + }); + it('should return next if the version in the Custom Header does not match the handler version', () => { const version = ['1', '2']; const versioningOptions: RoutePathMetadata['versioningOptions'] = {