From b3f768cdad625df9856ad46993b08b544c10d549 Mon Sep 17 00:00:00 2001 From: Bruno Freudensprung Date: Mon, 3 May 2021 11:47:47 +0200 Subject: [PATCH] fix #229: Validation errors when paths have multiple path parameters --- .../openapi2.yml | 27 ++++++++ .../openapi3.yml | 33 +++++++++ .../openapi2.yml | 20 ++++++ .../openapi3.yml | 24 +++++++ ...LessGenericPathOverMoreGenericPath.test.ts | 67 +++++++++++++++++++ ...LessGenericPathOverMoreGenericPath.test.ts | 65 ++++++++++++++++++ .../lib/utils/common.utils.ts | 42 +++++++++++- 7 files changed, 277 insertions(+), 1 deletion(-) create mode 100644 commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi2.yml create mode 100644 commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi3.yml create mode 100644 commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi2.yml create mode 100644 commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi3.yml create mode 100644 packages/chai-openapi-response-validator/test/assertions/satisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts create mode 100644 packages/jest-openapi/__test__/matchers/toSatisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts diff --git a/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi2.yml b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi2.yml new file mode 100644 index 00000000..1105ef10 --- /dev/null +++ b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi2.yml @@ -0,0 +1,27 @@ +swagger: '2.0' +info: + title: Test OpenApi 2 spec + description: Test that our plugins prefer to match responses to less generic paths over more generic paths + version: 0.1.0 +paths: + '/preferLessGenericPathOverMoreGenericPath/{templatedPath}/{forCodeCoverage}': + get: + responses: + 200: + description: Response body should be a number + schema: + type: number + '/preferLessGenericPathOverMoreGenericPath/{templatedPath}/{templatedPath2}': + get: + responses: + 200: + description: Response body should be a number + schema: + type: number + '/preferLessGenericPathOverMoreGenericPath/{templatedPath}/nonTemplatedPath': + get: + responses: + 200: + description: Response body should be a string + schema: + type: string diff --git a/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi3.yml b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi3.yml new file mode 100644 index 00000000..f56a11cc --- /dev/null +++ b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathAfterMoreGenericPath/openapi3.yml @@ -0,0 +1,33 @@ +openapi: 3.0.0 +info: + title: Test OpenApi 3 spec + description: Test that our plugins prefer to match responses to less generic paths over more generic paths + version: 0.1.0 +paths: + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/{forCodeCoverage}: + get: + responses: + 200: + description: Response body should be a number + content: + application/json: + schema: + type: number + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/{templatedPath2}: + get: + responses: + 200: + description: Response body should be a number + content: + application/json: + schema: + type: number + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/nonTemplatedPath: + get: + responses: + 200: + description: Response body should be a string + content: + application/json: + schema: + type: string diff --git a/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi2.yml b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi2.yml new file mode 100644 index 00000000..df533302 --- /dev/null +++ b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi2.yml @@ -0,0 +1,20 @@ +swagger: '2.0' +info: + title: Test OpenApi 2 spec + description: Test that our plugins prefer to match responses to less generic paths over more generic paths + version: 0.1.0 +paths: + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/nonTemplatedPath: + get: + responses: + 200: + description: Response body should be a string + schema: + type: string + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/{templatedPath2}: + get: + responses: + 200: + description: Response body should be a number + schema: + type: number diff --git a/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi3.yml b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi3.yml new file mode 100644 index 00000000..3dc3e5f4 --- /dev/null +++ b/commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath/lessGenericPathBeforeMoreGenericPath/openapi3.yml @@ -0,0 +1,24 @@ +openapi: 3.0.0 +info: + title: Test OpenApi 3 spec + description: Test that our plugins prefer to match responses to less generic paths over more generic paths + version: 0.1.0 +paths: + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/nonTemplatedPath: + get: + responses: + 200: + description: Response body should be a string + content: + application/json: + schema: + type: string + /preferLessGenericPathOverMoreGenericPath/{templatedPath}/{templatedPath2}: + get: + responses: + 200: + description: Response body should be a number + content: + application/json: + schema: + type: number diff --git a/packages/chai-openapi-response-validator/test/assertions/satisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts b/packages/chai-openapi-response-validator/test/assertions/satisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts new file mode 100644 index 00000000..b44a84b5 --- /dev/null +++ b/packages/chai-openapi-response-validator/test/assertions/satisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts @@ -0,0 +1,67 @@ +import chai from 'chai'; +import path from 'path'; + +import chaiResponseValidator from '../../..'; + +const openApiSpecsDir = path.resolve( + '../../commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath', +); +const { expect } = chai; + +describe('expect(res).to.satisfyApiSpec (using an OpenAPI spec with similar less generic paths and more generic OpenAPI paths)', () => { + [2, 3].forEach((openApiVersion) => { + describe(`OpenAPI ${openApiVersion}`, () => { + const openApiSpecs = [ + { + isLessGenericPathFirst: true, + pathToApiSpec: path.join( + openApiSpecsDir, + 'lessGenericPathBeforeMoreGenericPath', + `openapi${openApiVersion}.yml`, + ), + }, + { + isLessGenericPathFirst: false, + pathToApiSpec: path.join( + openApiSpecsDir, + 'lessGenericPathAfterMoreGenericPath', + `openapi${openApiVersion}.yml`, + ), + }, + ]; + + openApiSpecs.forEach((spec) => { + const { pathToApiSpec, isLessGenericPathFirst } = spec; + + describe(`res.req.path matches a least-templated OpenAPI path ${ + isLessGenericPathFirst ? 'before' : 'after' + } a templated OpenAPI path`, () => { + const res = { + status: 200, + req: { + method: 'GET', + path: + '/preferLessGenericPathOverMoreGenericPath/templatedPath/nonTemplatedPath', + }, + body: 'valid body (string)', + }; + + before(() => { + chai.use(chaiResponseValidator(pathToApiSpec)); + }); + + it('passes', () => { + expect(res).to.satisfyApiSpec; + }); + + it('fails when using .not', () => { + const assertion = () => expect(res).to.not.satisfyApiSpec; + expect(assertion).to.throw( + "not to satisfy the '200' response defined for endpoint 'GET /preferLessGenericPathOverMoreGenericPath/{templatedPath}/nonTemplatedPath'", + ); + }); + }); + }); + }); + }); +}); diff --git a/packages/jest-openapi/__test__/matchers/toSatisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts b/packages/jest-openapi/__test__/matchers/toSatisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts new file mode 100644 index 00000000..09ff1d98 --- /dev/null +++ b/packages/jest-openapi/__test__/matchers/toSatisfyApiSpec/preferLessGenericPathOverMoreGenericPath.test.ts @@ -0,0 +1,65 @@ +import path from 'path'; + +import jestOpenAPI from '../../..'; + +const openApiSpecsDir = path.resolve( + '../../commonTestResources/exampleOpenApiFiles/valid/preferLessGenericPathOverMoreGenericPath', +); + +describe('expect(res).toSatisfyApiSpec() (using an OpenAPI spec with similar less generic paths and more generic OpenAPI paths)', () => { + [2, 3].forEach((openApiVersion) => { + describe(`OpenAPI ${openApiVersion}`, () => { + const openApiSpecs = [ + { + isLessGenericPathFirst: true, + pathToApiSpec: path.join( + openApiSpecsDir, + 'lessGenericPathBeforeMoreGenericPath', + `openapi${openApiVersion}.yml`, + ), + }, + { + isLessGenericPathFirst: false, + pathToApiSpec: path.join( + openApiSpecsDir, + 'lessGenericPathAfterMoreGenericPath', + `openapi${openApiVersion}.yml`, + ), + }, + ]; + + openApiSpecs.forEach((spec) => { + const { pathToApiSpec, isLessGenericPathFirst } = spec; + + describe(`res.req.path matches a non-templated OpenAPI path ${ + isLessGenericPathFirst ? 'before' : 'after' + } a templated OpenAPI path`, () => { + const res = { + status: 200, + req: { + method: 'GET', + path: + '/preferLessGenericPathOverMoreGenericPath/templatedPath/nonTemplatedPath', + }, + body: 'valid body (string)', + }; + + beforeAll(() => { + jestOpenAPI(pathToApiSpec); + }); + + it('passes', () => { + expect(res).toSatisfyApiSpec(); + }); + + it('fails when using .not', () => { + const assertion = () => expect(res).not.toSatisfyApiSpec(); + expect(assertion).toThrow( + "not to satisfy the '200' response defined for endpoint 'GET /preferLessGenericPathOverMoreGenericPath/{templatedPath}/nonTemplatedPath'", + ); + }); + }); + }); + }); + }); +}); diff --git a/packages/openapi-validator/lib/utils/common.utils.ts b/packages/openapi-validator/lib/utils/common.utils.ts index d789c7ec..d3db56dc 100644 --- a/packages/openapi-validator/lib/utils/common.utils.ts +++ b/packages/openapi-validator/lib/utils/common.utils.ts @@ -21,6 +21,40 @@ const doesOpenApiPathMatchPathname = (openApiPath, pathname) => { return doesColonPathMatchPathname(pathInColonForm, pathname); }; +const compareOpenApiPathsByGenericity = (openApiPath1, openApiPath2) => { + // genericity is a based on the idea that a path with a template parameters is more generic + // than a path without any template parameter + // simple examples: + // "/a" == "/b" + // "/{foo}" == "/{bar}" + // "/{foo}" > "/a" + // "/a" < "/{foo}" + // examples with templated prefix: + // "/{hello}/a" == "/{bye}/b" + // "/{hello}/{foo}" == "/{bye}/{bar}" + // "/{hello}/{foo}" > "/{bye}/a" + // "/{hello}/a" < "/{bye}/{foo}" + // examples with hardcoded prefix: + // "/hello/a" == "/bye/b" + // "/hello/{foo}" == "/bye/{bar}" + // "/hello/{foo}" > "/bye/a" + // "/hello/a" < "/bye/{foo}" + const pathElements1 = openApiPath1.substring(1).split(/\//); + const pathElements2 = openApiPath2.substring(1).split(/\//); + for (let i = 0; i < pathElements1.length && i < pathElements2.length; i++) { + const isTemplateElement1 = pathElements1[i][0] == '{'; + const isTemplateElement2 = pathElements2[i][0] == '{'; + if (isTemplateElement1 && !isTemplateElement2) { + return 1; + } else if (!isTemplateElement1 && isTemplateElement2) { + return -1; + } + } + // returning 0 is valid because this function is called with paths of the same length, + // so we don't have to compare "/{foo}/a" and "/{bar}" for instance. + return 0; +}; + export const findOpenApiPathMatchingPossiblePathnames = ( possiblePathnames, OAPaths, @@ -34,7 +68,13 @@ export const findOpenApiPathMatchingPossiblePathnames = ( return OAPath; } if (doesOpenApiPathMatchPathname(OAPath, pathname)) { - openApiPath = OAPath; + // favor OAPath if it is least generic than openApiPath + if ( + !openApiPath || + compareOpenApiPathsByGenericity(OAPath, openApiPath) < 0 + ) { + openApiPath = OAPath; + } } } }