Skip to content

Commit

Permalink
Merge pull request #9585 from micalevisk/fix-issue-9581
Browse files Browse the repository at this point in the history 
fix: when version is an array with neutral version
  • Loading branch information
@kamilmysliwiec
kamilmysliwiec committed May 17, 2022
2 parents 20870b1 + 15737e0 commit bad85d9
Show file tree
Hide file tree
Showing 4 changed files with 188 additions and 53 deletions.
23 changes: 18 additions & 5 deletions packages/core/router/router-explorer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 <TRequest extends Record<string, any> = any, TResponse = any>(
req: TRequest,
Expand Down Expand Up @@ -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];
Expand All @@ -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);
Expand Down
182 changes: 146 additions & 36 deletions packages/core/test/router/router-explorer.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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=',
};
Expand All @@ -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=',
};
Expand All @@ -476,9 +476,64 @@ 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: VersioningOptions = {
const versioningOptions: RoutePathMetadata['versioningOptions'] = {
type: VersioningType.MEDIA_TYPE,
key: 'v=',
};
Expand All @@ -505,7 +560,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=',
};
Expand Down Expand Up @@ -534,7 +589,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=',
};
Expand All @@ -561,7 +616,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=',
};
Expand Down Expand Up @@ -767,7 +822,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',
};
Expand All @@ -794,7 +849,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',
};
Expand All @@ -820,9 +875,64 @@ 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: VersioningOptions = {
const versioningOptions: RoutePathMetadata['versioningOptions'] = {
type: VersioningType.HEADER,
header: 'X-API-Version',
};
Expand All @@ -849,7 +959,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',
};
Expand Down Expand Up @@ -878,7 +988,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',
};
Expand All @@ -905,7 +1015,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',
};
Expand Down
21 changes: 18 additions & 3 deletions packages/platform-express/adapters/express-adapter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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];
Expand All @@ -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);
Expand Down

0 comments on commit bad85d9

Please sign in to comment.