Added one more rule for response validation in model validation. (#436)

* Added one more rule for reponse validation. Fail the validation when there's schema defined for response body in spec but the example response doesn't provide the body in response. * Fix the exmaples for unit tests. * Added two more unit tests. Coverage for the new added rules about body/schema validation in response. * Apply snapshot patch. * Bump package version and update changeLog file. * Apply snapshot patch. * Update version number in changeLog. * Apply snapshots patch again. * Update per the review comment. * Update test snapshot.
Azure · Jul 8, 2019 · 6b4c9a8 · 6b4c9a8
1 parent 0223567
commit 6b4c9a8
Show file tree

Hide file tree

Showing 12 changed files with 10,820 additions and 5,337 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -1,8 +1,12 @@
 # Changelog
+### 07/02/2019 0.18.6
+-  Adding support for JSON path & schema path in errors.
+
+### 06/12/2019 0.18.5
+
 ### 05/30/2019 0.18.4
-- Better error reporting for spec reference pointing to invalid files
+- Better error reporting for spec reference pointing to invalid files.
 
-# Changelog
 ### 05/29/2019 0.18.3
 - Update cache initialization in live validator to `Promise.all()`.
 - Update package version of `yuml2svg` dependency (since it contains typescript definitions).

diff --git a/lib/util/constants.ts b/lib/util/constants.ts
@@ -96,6 +96,10 @@ export const ErrorCodes = {
   DoubleForwardSlashesInUrl: {
     name: "DOUBLE_FORWARD_SLASHES_IN_URL",
     id: "OAV129"
+  },
+  ResponseBodyNotInExample: {
+    name: "RESPONSE_BODY_NOT_IN_EXAMPLE",
+    id: "OAV130"
   }
 }
 

diff --git a/lib/validators/modelValidator.ts b/lib/validators/modelValidator.ts
@@ -558,7 +558,9 @@ export class ModelValidator extends SpecValidator<SpecValidationResult> {
 
       const exampleResponseHeaders = exampleResponseValue[exampleResponseStatusCode].headers || {}
       const exampleResponseBody = exampleResponseValue[exampleResponseStatusCode].body
-      if (exampleResponseBody && !response.schema) {
+
+      // Fail when example provides the response body but the swagger spec doesn't define the schema for the response.
+      if (exampleResponseBody !== undefined && !response.schema) {
         const msg =
           `Response statusCode "${exampleResponseStatusCode}" for operation ` +
           `"${operation.operationId}" has response body provided in the example, ` +
@@ -571,7 +573,22 @@ export class ModelValidator extends SpecValidator<SpecValidationResult> {
         validationResults.errors.push(e)
         log.error(e as any)
         continue
+      } else if (exampleResponseBody === undefined && response.schema) {
+        // Fail when example doesn't provide the response body but the swagger spec define the schema for the response.
+        const msg =
+          `Response statusCode "${exampleResponseStatusCode}" for operation ` +
+          `"${operation.operationId}" has no response body provided in the example, ` +
+          `however the response does have a "schema" defined in the swagger spec.`
+        const e = this.constructErrorObject<Sway.ValidationEntry>({
+          code: C.ErrorCodes.ResponseBodyNotInExample,
+          message: msg,
+          source: operation.definition
+        })
+        validationResults.errors.push(e)
+        log.error(e as any)
+        continue
       }
+
       // ensure content-type header is present
       if (!(exampleResponseHeaders["content-type"] || exampleResponseHeaders["Content-Type"])) {
         exampleResponseHeaders["content-type"] = utils.getJsonContentType(operation.produces)

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "oav",
-  "version": "0.18.5",
+  "version": "0.18.6",
   "author": {
     "name": "Microsoft Corporation",
     "email": "azsdkteam@microsoft.com",
@@ -36,7 +36,7 @@
     "vscode-jsonrpc": "^3.6.2",
     "winston": "^3.2.1",
     "yargs": "^6.6.0",
-    "yasway": "^1.8.1",
+    "yasway": "^1.8.3",
     "yuml2svg": "^4.2.1",
     "z-schema": "^4.1.0"
   },

diff --git a/regression/__snapshots__/validateExamplesRegression_latestOnly.test.ts.snap b/regression/__snapshots__/validateExamplesRegression_latestOnly.test.ts.snap
diff --git a/test/modelValidation/swaggers/specification/defaultIsErrorOnly/examples/fail.json b/test/modelValidation/swaggers/specification/defaultIsErrorOnly/examples/fail.json
@@ -1,7 +1,11 @@
 {
   "parameters":{},
   "responses":{
-    "200": {},
-    "203": {}
+    "200": {
+      "body": {}
+    },
+    "203": {
+      "body": {}
+    }
   }
 }
diff --git a/test/modelValidation/swaggers/specification/extraBodyInResponse/examples/fail.json b/test/modelValidation/swaggers/specification/extraBodyInResponse/examples/fail.json
@@ -0,0 +1,8 @@
+{
+  "parameters": {},
+  "responses": {
+    "200": {
+      "body": {}
+    }
+  }
+}
diff --git a/test/modelValidation/swaggers/specification/extraBodyInResponse/test.json b/test/modelValidation/swaggers/specification/extraBodyInResponse/test.json
@@ -0,0 +1,30 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "title": "sometitle",
+    "version": "2018-01-01"
+  },
+  "consumes": [
+    "application/json"
+  ],
+  "produces": [
+    "application/json"
+  ],
+  "paths": {
+    "/somepath": {
+      "get": {
+        "operationId": "op",
+        "responses": {
+          "200": {
+            "description": "somedescription"
+          }
+        },
+        "x-ms-examples": {
+          "Fail": {
+            "$ref": "./examples/fail.json"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/test/modelValidation/swaggers/specification/noBodyInResponse/examples/fail.json b/test/modelValidation/swaggers/specification/noBodyInResponse/examples/fail.json
@@ -0,0 +1,6 @@
+{
+  "parameters": {},
+  "responses": {
+    "200": {}
+  }
+}
diff --git a/test/modelValidation/swaggers/specification/noBodyInResponse/test.json b/test/modelValidation/swaggers/specification/noBodyInResponse/test.json
@@ -0,0 +1,31 @@
+{
+  "swagger": "2.0",
+  "info": {
+    "title": "sometitle",
+    "version": "2018-01-01"
+  },
+  "consumes": [
+    "application/json"
+  ],
+  "produces": [
+    "application/json"
+  ],
+  "paths": {
+    "/somepath": {
+      "get": {
+        "operationId": "op",
+        "responses": {
+          "200": {
+            "description": "somedescription",
+            "schema": {}
+          }
+        },
+        "x-ms-examples": {
+          "Fail": {
+            "$ref": "./examples/fail.json"
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/test/modelValidation/swaggers/specification/noDefault/examples/fail.json b/test/modelValidation/swaggers/specification/noDefault/examples/fail.json
@@ -1,7 +1,9 @@
 {
   "parameters":{},
   "responses":{
-    "200": {},
+    "200": {
+      "body": {}
+    },
     "404": {
       "body": {
         "nameAvailable": "string instead of bool"

diff --git a/test/modelValidatorTests.ts b/test/modelValidatorTests.ts
@@ -502,6 +502,28 @@ describe("Model Validation", () => {
     })
   })
 
+  describe("No body in response if schema is defined for response", () => {
+    it("should fail on example without body defined in response", async () => {
+      const specPath2 = `${testPath}/modelValidation/swaggers/specification/noBodyInResponse/test.json`
+      const result = await validate.validateExamples(specPath2, undefined, {
+        consoleLogLevel: "off"
+      })
+      assert(result.length === 1)
+      assert.strictEqual(result[0].code, "RESPONSE_BODY_NOT_IN_EXAMPLE")
+    })
+  })
+
+  describe("Extra body in response even it's not defined in schema", () => {
+    it("should fail on example when extra body defined in response", async () => {
+      const specPath2 = `${testPath}/modelValidation/swaggers/specification/extraBodyInResponse/test.json`
+      const result = await validate.validateExamples(specPath2, undefined, {
+        consoleLogLevel: "off"
+      })
+      assert(result.length === 1)
+      assert.strictEqual(result[0].code, "RESPONSE_SCHEMA_NOT_IN_SPEC")
+    })
+  })
+
   describe("Default doesn't cover non-error responses", () => {
     it("should fail on example with unrecognized status code", async () => {
       const specPath2 = `${testPath}/modelValidation/swaggers/specification/defaultIsErrorOnly/test.json`