From 3eec33d4acd5651687bad9865d0dd0f70ab8de8b Mon Sep 17 00:00:00 2001 From: Ke Yu <33389195+keryul@users.noreply.github.com> Date: Mon, 18 Apr 2022 17:19:14 +0800 Subject: [PATCH] fix bug about query parameter validation (#767) * Ignore INVALID_TYPE validation in case of query parameter in string format * update revalidate logic * align change log version * put all the revalidate logic to function ReValidateIfNeed() * set timeout to single test * update revalidate logic * update revalidate logic and test files * clean old codes --- ChangeLog.md | 2 + lib/swaggerValidator/ajvSchemaValidator.ts | 81 +++++- .../specification/query/examples/array.json | 11 - .../specification/query/examples/string.json | 8 - .../{bool.json => stringWithBoolean.json} | 2 +- .../query/examples/stringWithExtraError.json | 9 + .../{integer.json => stringWithNumber.json} | 2 +- .../swaggers/specification/query/test.json | 232 ++++-------------- test/modelValidatorTests.ts | 42 +++- 9 files changed, 170 insertions(+), 219 deletions(-) delete mode 100644 test/modelValidation/swaggers/specification/query/examples/array.json delete mode 100644 test/modelValidation/swaggers/specification/query/examples/string.json rename test/modelValidation/swaggers/specification/query/examples/{bool.json => stringWithBoolean.json} (69%) create mode 100644 test/modelValidation/swaggers/specification/query/examples/stringWithExtraError.json rename test/modelValidation/swaggers/specification/query/examples/{integer.json => stringWithNumber.json} (60%) diff --git a/ChangeLog.md b/ChangeLog.md index 5fdd0f4a..7d0ba943 100644 --- a/ChangeLog.md +++ b/ChangeLog.md @@ -1,6 +1,8 @@ # Change Log - oav ## 04/07/2022 2.12.0 + +- ModelValidator - Ignore INVALID_TYPE validation in case of query parameter in string format - Traffic Validation - Support validation report generation ## 04/06/2022 2.11.10 diff --git a/lib/swaggerValidator/ajvSchemaValidator.ts b/lib/swaggerValidator/ajvSchemaValidator.ts index 1d24e936..6ec0e072 100644 --- a/lib/swaggerValidator/ajvSchemaValidator.ts +++ b/lib/swaggerValidator/ajvSchemaValidator.ts @@ -1,3 +1,4 @@ +import * as lodash from "lodash"; import { ChildObjectInfo, getInfo, @@ -77,7 +78,15 @@ export class AjvSchemaValidator implements SchemaValidator { const result: SchemaValidateIssue[] = []; const isValid = validateSchema.validate.call(ctx, data); if (!isValid) { - ajvErrorListToSchemaValidateIssueList(validateSchema.validate.errors!, ctx, result); + const errors = ReValidateIfNeed( + validateSchema.validate.errors!, + ctx, + data, + validateSchema.validate + ); + if (errors.length > 0) { + ajvErrorListToSchemaValidateIssueList(errors, ctx, result); + } validateSchema.validate.errors = null; } return result; @@ -192,6 +201,76 @@ export const ajvErrorToSchemaValidateIssue = ( return result; }; +const ReValidateIfNeed = ( + originalErrors: ErrorObject[], + ctx: SchemaValidateContext, + data: any, + validate: ValidateFunction +): ErrorObject[] => { + const result: ErrorObject[] = []; + const newData = lodash.cloneDeep(data); + + for (const originalError of originalErrors) { + validate.errors = null; + const { schema, parentSchema: parentSch, keyword, data: errorData, dataPath } = originalError; + const parentSchema = parentSch as Schema; + + // If the value of query parameter is in string format, we can revalidate this error + if ( + !ctx.isResponse && + keyword === "type" && + schema === "array" && + typeof errorData === "string" && + (parentSchema as any)?.["in"] === "query" + ) { + const arrayData = errorData.split(",").map((item) => { + // when item is number + const numberRegex = /^[+-]?\d+(\.\d+)?([Ee]\+?\d+)?$/g; + if (numberRegex.test(item)) { + return parseFloat(item); + } + // when item is boolean + if (item === "true" || item === "false") { + return item === "true"; + } + return item; + }); + const position = dataPath.substr(1); + lodash.set(newData, position, arrayData); + const isValid = validate.call(ctx, newData); + if (!isValid) { + // if validate.errors have new errors, add them to result + for (const newError of validate.errors!) { + let [includedInResult, includedInOriginalErrors] = [false, false]; + for (const resultError of result) { + if (lodash.isEqual(newError, resultError)) { + // error is included in result + includedInResult = true; + break; + } + } + if (!includedInResult) { + for (const eachOriginalError of originalErrors) { + if (lodash.isEqual(newError, eachOriginalError)) { + // error is included in originalErrors + includedInOriginalErrors = true; + break; + } + } + if (!includedInOriginalErrors) { + result.push(newError); + } + } + } + } + continue; + } + result.push(originalError); + } + + return result; +}; + const shouldSkipError = (error: ErrorObject, cxt: SchemaValidateContext) => { const { schema, parentSchema: parentSch, params, keyword, data } = error; const parentSchema = parentSch as Schema; diff --git a/test/modelValidation/swaggers/specification/query/examples/array.json b/test/modelValidation/swaggers/specification/query/examples/array.json deleted file mode 100644 index b689234f..00000000 --- a/test/modelValidation/swaggers/specification/query/examples/array.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "parameters": { - "hello": [ - "Wake up at 3:14:16 AM", - "World" - ] - }, - "responses": { - "200": {} - } -} \ No newline at end of file diff --git a/test/modelValidation/swaggers/specification/query/examples/string.json b/test/modelValidation/swaggers/specification/query/examples/string.json deleted file mode 100644 index 5d2f26f8..00000000 --- a/test/modelValidation/swaggers/specification/query/examples/string.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "parameters": { - "hello": "Wake up at 3:14:16 AM" - }, - "responses": { - "200": {} - } -} \ No newline at end of file diff --git a/test/modelValidation/swaggers/specification/query/examples/bool.json b/test/modelValidation/swaggers/specification/query/examples/stringWithBoolean.json similarity index 69% rename from test/modelValidation/swaggers/specification/query/examples/bool.json rename to test/modelValidation/swaggers/specification/query/examples/stringWithBoolean.json index 71ae5ac9..72a4903e 100644 --- a/test/modelValidation/swaggers/specification/query/examples/bool.json +++ b/test/modelValidation/swaggers/specification/query/examples/stringWithBoolean.json @@ -1,6 +1,6 @@ { "parameters": { - "hello": true + "helloArray": "true" }, "responses": { "200": {} diff --git a/test/modelValidation/swaggers/specification/query/examples/stringWithExtraError.json b/test/modelValidation/swaggers/specification/query/examples/stringWithExtraError.json new file mode 100644 index 00000000..d37b00ac --- /dev/null +++ b/test/modelValidation/swaggers/specification/query/examples/stringWithExtraError.json @@ -0,0 +1,9 @@ +{ + "parameters": { + "helloArray": "127.0.0.1,1.2E+20", + "name": "hello,hello2" + }, + "responses": { + "200": {} + } +} \ No newline at end of file diff --git a/test/modelValidation/swaggers/specification/query/examples/integer.json b/test/modelValidation/swaggers/specification/query/examples/stringWithNumber.json similarity index 60% rename from test/modelValidation/swaggers/specification/query/examples/integer.json rename to test/modelValidation/swaggers/specification/query/examples/stringWithNumber.json index 580ab85f..832c570c 100644 --- a/test/modelValidation/swaggers/specification/query/examples/integer.json +++ b/test/modelValidation/swaggers/specification/query/examples/stringWithNumber.json @@ -1,6 +1,6 @@ { "parameters": { - "hello": 31416 + "helloArray": "050.974520,1.2E+2" }, "responses": { "200": {} diff --git a/test/modelValidation/swaggers/specification/query/test.json b/test/modelValidation/swaggers/specification/query/test.json index 998414c5..cfc1cd69 100644 --- a/test/modelValidation/swaggers/specification/query/test.json +++ b/test/modelValidation/swaggers/specification/query/test.json @@ -11,154 +11,30 @@ "consumes": [], "produces": [], "paths": { - "/query/string/encoded": { + "/query/string/definedAsArray/number": { "post": { "tags": [ "Query" ], - "operationId": "Query_StringEncoded", - "description": "The parameter is a query, an encoded string", + "operationId": "Query_StringButDefinedAsArray_numberItem", + "description": "The parameter is a query, a string but defined as an array", "x-ms-examples": { - "queryStringEncoded": { - "$ref": "./examples/string.json" + "queryStringButDefinedAsArray": { + "$ref": "./examples/stringWithNumber.json" } }, "parameters": [ { - "name": "hello", - "in": "query", - "required": false, - "type": "string", - "description": "Encoded query string", - "x-ms-skip-url-encoding": false - } - ], - "responses": { - "200": { - "description": "OK" - } - } - } - }, - "/query/string/notEncoded": { - "post": { - "tags": [ - "Query" - ], - "operationId": "Query_StringNotEncoded", - "description": "The parameter is a query, a not encoded string", - "x-ms-examples": { - "queryStringNotEncoded": { - "$ref": "./examples/string.json" - } - }, - "parameters": [ - { - "name": "hello", - "in": "query", - "required": false, - "type": "string", - "description": "Not encoded query string", - "x-ms-skip-url-encoding": true - } - ], - "responses": { - "200": { - "description": "OK" - } - } - } - }, - "/query/bool/encoded": { - "post": { - "tags": [ - "Query" - ], - "operationId": "Query_BoolEncoded", - "description": "The parameter is a query, an encoded bool", - "x-ms-examples": { - "queryBoolEncoded": { - "$ref": "./examples/bool.json" - } - }, - "parameters": [ - { - "name": "hello", - "in": "query", - "required": false, - "type": "boolean", - "description": "Encoded query bool", - "x-ms-skip-url-encoding": false - } - ], - "responses": { - "200": { - "description": "OK" - } - } - } - }, - "/query/bool/notEncoded": { - "post": { - "tags": [ - "Query" - ], - "operationId": "Query_BoolNotEncoded", - "description": "The parameter is a query, a not encoded bool", - "x-ms-examples": { - "queryBoolNotEncoded": { - "$ref": "./examples/bool.json" - } - }, - "parameters": [ - { - "name": "hello", - "in": "query", - "required": false, - "type": "boolean", - "description": "Not encoded query bool", - "x-ms-skip-url-encoding": true - } - ], - "responses": { - "200": { - "description": "OK" - } - } - } - }, - "/query/array/encoded": { - "post": { - "tags": [ - "Query" - ], - "operationId": "Query_ArrayEncoded", - "description": "The parameter is a query, an encoded array", - "x-ms-examples": { - "queryArrayEncoded": { - "$ref": "./examples/array.json" - } - }, - "parameters": [ - { - "name": "hello", + "name": "helloArray", "in": "query", "required": false, "type": "array", - "description": "Encoded query array", - "x-ms-skip-url-encoding": false, - "collectionFormat": "csv", + "description": "array items are numbers", + "minItems": 2, + "maxItems": 2, "items": { - "type": "string", - "x-nullable": false, - "x-ms-enum": { - "name": "Hello", - "modelAsString": true - }, - "enum": [ - "Wake up at 3:14:16 AM", - "World" - ] + "type": "number", + "format": "double" } } ], @@ -169,38 +45,27 @@ } } }, - "/query/array/notEncoded": { + "/query/string/definedAsArray/boolean": { "post": { "tags": [ "Query" ], - "operationId": "Query_ArrayNotEncoded", - "description": "The parameter is a query, a not encoded array", + "operationId": "Query_StringButDefinedAsArray_booleanItem", + "description": "The parameter is a query, a string but defined as an array", "x-ms-examples": { - "queryArrayNotEncoded": { - "$ref": "./examples/array.json" + "queryStringButDefinedAsArray": { + "$ref": "./examples/stringWithBoolean.json" } }, "parameters": [ { - "name": "hello", + "name": "helloArray", "in": "query", "required": false, "type": "array", - "description": "Encoded query array", - "x-ms-skip-url-encoding": true, - "collectionFormat": "csv", + "description": "array item is boolean", "items": { - "type": "string", - "x-nullable": false, - "x-ms-enum": { - "name": "Hello", - "modelAsString": true - }, - "enum": [ - "Wake up at 3:14:16 AM", - "World" - ] + "type": "boolean" } } ], @@ -211,55 +76,44 @@ } } }, - "/query/integer/encoded": { + "/query/string/definedAsArray/extraError": { "post": { "tags": [ "Query" ], - "operationId": "Query_IntegerEncoded", - "description": "The parameter is a query, an encoded integer", + "operationId": "Query_StringButDefinedAsArray_extraError", + "description": "The parameter is a query, a string but defined as an array", "x-ms-examples": { - "queryIntegerEncoded": { - "$ref": "./examples/integer.json" + "queryStringButDefinedAsArray": { + "$ref": "./examples/stringWithExtraError.json" } }, "parameters": [ { - "name": "hello", + "name": "helloArray", "in": "query", "required": false, - "type": "integer", - "description": "Encoded query integer", - "x-ms-skip-url-encoding": false - } - ], - "responses": { - "200": { - "description": "OK" - } - } - } - }, - "/query/integer/notEncoded": { - "post": { - "tags": [ - "Query" - ], - "operationId": "Query_IntegerNotEncoded", - "description": "The parameter is a query, a not encoded integer", - "x-ms-examples": { - "queryIntegerNotEncoded": { - "$ref": "./examples/integer.json" - } - }, - "parameters": [ + "type": "array", + "description": "array items are numbers", + "minItems": 2, + "maxItems": 2, + "items": { + "type": "number", + "format": "double" + } + }, { - "name": "hello", + "name": "name", "in": "query", "required": false, - "type": "integer", - "description": "Not encoded query integer", - "x-ms-skip-url-encoding": true + "type": "array", + "description": "array items are numbers", + "minItems": 2, + "maxItems": 2, + "items": { + "type": "string", + "maxLength": 5 + } } ], "responses": { diff --git a/test/modelValidatorTests.ts b/test/modelValidatorTests.ts index 29c98611..eed59361 100644 --- a/test/modelValidatorTests.ts +++ b/test/modelValidatorTests.ts @@ -461,14 +461,40 @@ describe("Model Validation", () => { }); describe("Queries - ", () => { - it("should pass for various query parameters", async () => { - const specPath2 = `${testPath}/modelValidation/swaggers/specification/query/test.json`; - const result = await validate.validateExamples(specPath2, undefined, { - consoleLogLevel: "off", + describe("Should revalidate query parameters in string format which be defined as array", () => { + it("array items are numbers", async () => { + const specPath2 = `${testPath}/modelValidation/swaggers/specification/query/test.json`; + const result = await validate.validateExamples( + specPath2, + "Query_StringButDefinedAsArray_numberItem", + { consoleLogLevel: "off" } + ); + assert(result.length === 0, `swagger "${specPath2}" contains model validation errors.`); + }); + it("array item is boolean", async () => { + const specPath2 = `${testPath}/modelValidation/swaggers/specification/query/test.json`; + const result = await validate.validateExamples( + specPath2, + "Query_StringButDefinedAsArray_booleanItem", + { consoleLogLevel: "off" } + ); + assert(result.length === 0, `swagger "${specPath2}" contains model validation errors.`); + }); + it("should report other error and skip INVALID_TYPE error about query parameter", async () => { + const specPath2 = `${testPath}/modelValidation/swaggers/specification/query/test.json`; + const result = await validate.validateExamples( + specPath2, + "Query_StringButDefinedAsArray_extraError", + { consoleLogLevel: "off" } + ); + assert(result.length === 2); + assert.strictEqual(result[0].code, "INVALID_TYPE"); + assert.strictEqual(result[0].message, "Expected type number but found type string"); + assert.strictEqual(result[0].schemaJsonPath, "helloArray/items/type"); + assert.strictEqual(result[1].code, "MAX_LENGTH"); + assert.strictEqual(result[1].message, "String is too long (6 chars), maximum 5"); + assert.strictEqual(result[1].schemaJsonPath, "name/items/maxLength"); }); - // console.dir(result, { depth: null }) - assert(result.length === 0, `swagger "${specPath2}" contains model validation errors.`); - // console.log(result) }); }); @@ -771,6 +797,6 @@ describe("Model Validation", () => { const result = await validate.validateExamples(specPath2, undefined); assert.strictEqual(result.length, 1); assert.strictEqual(result[0].code, "XMS_EXAMPLE_NOTFOUND_ERROR"); - }); + }, 10000); }); });