fix validation for nullable array types and fix a discrepancy in specResolver (#440)

* fix validation for nullable array types and fix a discrepancy in specResolver * update test snapshot * apply patch
2019-07-30 15:17:13 -07:00 · 2019-07-30 15:17:13 -07:00 · 4123d46bee
--- a/ChangeLog.md
+++ b/ChangeLog.md
@ -1,5 +1,10 @@
 # Changelog

+## 07/30/2019 0.19.3
+
+- Fixed validation of nullable array types if they were defined as tope level models or transformed into top level models in the `"definitions"` object. Fixes #437.
+- Fixed an issue where `"properties": {}` was being incorrectly added to a schema with `"$ref"` in the response or a top level `array`. 
+
 ## 07/12/2019 0.19.2

 - Add support for validating discriminator is required and also the support for suppression.[Issue#386](https://github.com/Azure/oav/issues/386).
@ -13,13 +18,15 @@

 - Modify path in payload for MISSING_REQUIRED_PROPERTIES and for OBJECT_ADDITIONAL_PROPERTIES to point to the actual property.

-
 ## 07/02/2019 0.18.6

 - Adding support for JSON path & schema path in errors.
+- Improved model validation when example provides the response body however the spec doesn't have a schema for the response and vice-versa.

 ## 06/12/2019 0.18.5

+-  Added support for JSON path & schema path in errors.
+
 ## 05/30/2019 0.18.4

 - Better error reporting for spec reference pointing to invalid files.
--- a/lib/util/utils.ts
+++ b/lib/util/utils.ts
@ -576,9 +576,9 @@ export function allowNullableTypes(model: SchemaObject): SchemaObject {
          ? allowNullableTypes(model.items)
          : allowNullType(model.items)
      }
-      // tslint:disable-next-line:max-line-length
-      // if we have a top level "object" with x-nullable set, we need to relax the model at that level
-    } else if (model.type === "object" && model["x-nullable"]) {
+    }
+    // if we have a top level entity with x-nullable set, we need to relax the model at that level
+    if (model["x-nullable"]) {
      model = allowNullType(model)
    }
  }
--- a/lib/validators/resolveNestedDefinitions.ts
+++ b/lib/validators/resolveNestedDefinitions.ts
@ -76,7 +76,9 @@ export function resolveNestedDefinitions(spec: SwaggerObject): SwaggerObject {
  // a function to resolve SchemaObject (top-level and nested)
  const resolveSchemaObject = (schemaObject: SchemaObject): SchemaObject => {
    const result = propertySetMap<SchemaObject>(schemaObject, {
-      properties: properties => stringMapMap(properties, resolveNestedSchemaObject),
+      properties: skipIfUndefined(properties =>
+        stringMapMap(properties, resolveNestedSchemaObject)
+      ),
      additionalProperties: additionalProperties =>
        additionalProperties === undefined || typeof additionalProperties !== "object"
          ? additionalProperties
--- a/package.json
+++ b/package.json
@ -1,6 +1,6 @@
 {
  "name": "oav",
-  "version": "0.19.2",
+  "version": "0.19.3",
  "author": {
    "name": "Microsoft Corporation",
    "email": "azsdkteam@microsoft.com",
--- a/regression/snapshots/validateExamplesRegression_latestOnly.test.ts.snap
+++ b/regression/snapshots/validateExamplesRegression_latestOnly.test.ts.snap
@ -8742,6 +8742,110 @@ Array [
    "severity": 0,
    "source": "request",
  },
+  Object {
+    "code": "INVALID_TYPE",
+    "details": Object {
+      "code": "INVALID_TYPE",
+      "description": "App properties",
+      "directives": Object {},
+      "jsonPath": "$.message.appProperties",
+      "message": "Expected type object but found type string",
+      "params": Array [
+        "object",
+        "string",
+      ],
+      "path": "message/appProperties",
+      "position": Object {
+        "column": 26,
+        "line": 3158,
+      },
+      "title": "#/definitions/RoutingMessage/properties/appProperties",
+      "url": "/home/vsts/work/1/s/regression/azure-rest-api-specs/specification/iothub/resource-manager/Microsoft.Devices/stable/2018-04-01/iothub.json",
+    },
+    "operationId": "IotHubResource_TestAllRoutes",
+    "responseCode": "ALL",
+    "scenario": "IotHubResource_TestAllRoutes",
+    "severity": 0,
+    "source": "request",
+  },
+  Object {
+    "code": "INVALID_TYPE",
+    "details": Object {
+      "code": "INVALID_TYPE",
+      "description": "System properties",
+      "directives": Object {},
+      "jsonPath": "$.message.systemProperties",
+      "message": "Expected type object but found type string",
+      "params": Array [
+        "object",
+        "string",
+      ],
+      "path": "message/systemProperties",
+      "position": Object {
+        "column": 29,
+        "line": 3165,
+      },
+      "title": "#/definitions/RoutingMessage/properties/systemProperties",
+      "url": "/home/vsts/work/1/s/regression/azure-rest-api-specs/specification/iothub/resource-manager/Microsoft.Devices/stable/2018-04-01/iothub.json",
+    },
+    "operationId": "IotHubResource_TestAllRoutes",
+    "responseCode": "ALL",
+    "scenario": "IotHubResource_TestAllRoutes",
+    "severity": 0,
+    "source": "request",
+  },
+  Object {
+    "code": "INVALID_TYPE",
+    "details": Object {
+      "code": "INVALID_TYPE",
+      "description": "App properties",
+      "directives": Object {},
+      "jsonPath": "$.message.appProperties",
+      "message": "Expected type object but found type string",
+      "params": Array [
+        "object",
+        "string",
+      ],
+      "path": "message/appProperties",
+      "position": Object {
+        "column": 26,
+        "line": 3158,
+      },
+      "title": "#/definitions/RoutingMessage/properties/appProperties",
+      "url": "/home/vsts/work/1/s/regression/azure-rest-api-specs/specification/iothub/resource-manager/Microsoft.Devices/stable/2018-04-01/iothub.json",
+    },
+    "operationId": "IotHubResource_TestRoute",
+    "responseCode": "ALL",
+    "scenario": "IotHubResource_TestRoute",
+    "severity": 0,
+    "source": "request",
+  },
+  Object {
+    "code": "INVALID_TYPE",
+    "details": Object {
+      "code": "INVALID_TYPE",
+      "description": "System properties",
+      "directives": Object {},
+      "jsonPath": "$.message.systemProperties",
+      "message": "Expected type object but found type string",
+      "params": Array [
+        "object",
+        "string",
+      ],
+      "path": "message/systemProperties",
+      "position": Object {
+        "column": 29,
+        "line": 3165,
+      },
+      "title": "#/definitions/RoutingMessage/properties/systemProperties",
+      "url": "/home/vsts/work/1/s/regression/azure-rest-api-specs/specification/iothub/resource-manager/Microsoft.Devices/stable/2018-04-01/iothub.json",
+    },
+    "operationId": "IotHubResource_TestRoute",
+    "responseCode": "ALL",
+    "scenario": "IotHubResource_TestRoute",
+    "severity": 0,
+    "source": "request",
+  },
 ]
 `;

--- a/test/modelValidation/swaggers/specification/nullableTypes/array_nullable.json
+++ b/test/modelValidation/swaggers/specification/nullableTypes/array_nullable.json
@ -0,0 +1,83 @@
+{
+  "swagger": "2.0",
+  "schemes": [
+    "https"
+  ],
+  "consumes": [
+    "application/json"
+  ],
+  "produces": [
+    "application/json"
+  ],
+  "paths": {
+    "/x": {
+      "patch": {
+        "x-ms-examples": {
+          "Update": {
+            "$ref": "./examples/myExample.json"
+          }
+        },
+        "operationId": "Models_Update",
+        "parameters": [
+          {
+            "name": "data",
+            "in": "body",
+            "required": true,
+            "x-ms-parameter-location": "method",
+            "type": "object",
+            "schema": {
+              "type": "object",
+              "properties": {
+                "myObject": {
+                  "$ref": "#/definitions/compositeProps"
+                }
+              }
+            }
+          }
+        ],
+        "responses": {
+          "200": {
+            "description": "Updated",
+            "schema": {
+              "$ref": "#/definitions/model"
+            }
+          }
+        }
+      }
+    }
+  },
+  "definitions": {
+    "model": {
+      "type": "object",
+      "properties": {
+        "properties": {
+          "x-ms-client-flatten": true,
+          "$ref": "#/definitions/myProperties"
+        }
+      }
+    },
+    "myProperties": {
+      "type": "object",
+      "properties": {
+        "myObject": {
+          "$ref": "#/definitions/compositeProps"
+        }
+      }
+    },
+    "compositeProps": {
+      "type": "object",
+      "properties": {
+        "myArray": {
+          "type": "array",
+          "x-nullable": true,
+          "items": {
+            "type": "string"
+          }
+        }
+      },
+      "required": [
+        "myArray"
+      ]
+    }
+  }
+}
--- a/test/modelValidation/swaggers/specification/nullableTypes/examples/myExample.json
+++ b/test/modelValidation/swaggers/specification/nullableTypes/examples/myExample.json
@ -0,0 +1,22 @@
+{
+  "parameters": {
+    "data": {
+      "properties": {
+        "myObject": {
+          "myArray": null
+        }
+      }
+    }
+  },
+  "responses": {
+    "200": {
+      "body": {
+        "properties": {
+          "myObject": {
+            "myArray": null
+          }
+        }
+      }
+    }
+  }
+}
--- a/test/modelValidatorTests.ts
+++ b/test/modelValidatorTests.ts
@ -451,6 +451,19 @@ describe("Model Validation", () => {
      )
      // console.log(result)
    })
+
+    it("should pass for nullable array types", async () => {
+      const specPath2 = `${testPath}/modelValidation/swaggers/specification/nullableTypes/array_nullable.json`
+      const operationIds = "Models_Update"
+      const result = await validate.validateExamples(specPath2, operationIds, {
+        consoleLogLevel: "off"
+      })
+      assert(
+        result.length === 0,
+        `swagger "${specPath2}" with operation "${operationIds}" contains model validation errors.`
+      )
+      // console.log(result)
+    })
  })

  describe("Content type - ", () => {