Adding mechanism to compare models's required set

openapi-diff/src/modeler/AutoRest.Swagger.Tests/AutoRest.Swagger.Tests.csproj

@@ -11,6 +11,9 @@
 
   <ItemGroup>
     <Content Include="Resource\Swagger\new\added_path.json" />
+    <Content Include="Resource\Swagger\new\added_required_property.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </Content>
     <Content Include="Resource\Swagger\new\changed_operation_id.json" />
     <Content Include="Resource\Swagger\new\enum_values_changed.json" />
     <Content Include="Resource\Swagger\new\misc_checks_01.json" />
@@ -44,6 +47,9 @@
     <Content Include="Resource\Swagger\new\version_check_02.json" />
     <Content Include="Resource\Swagger\new\version_check_03.json" />
     <Content Include="Resource\Swagger\new\version_check_04.json" />
+    <Content Include="Resource\Swagger\old\added_required_property.json">
+      <CopyToOutputDirectory>Always</CopyToOutputDirectory>
+    </Content>
     <Content Include="Resource\Swagger\old\recursive_model.json" />
     <Content Include="Resource\Swagger\old\changed_operation_id.json" />
     <Content Include="Resource\Swagger\old\added_path.json" />

openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/new/added_required_property.json

@@ -0,0 +1,49 @@
+{
+  "swagger": 2.0,
+  "info": {
+    "title": "added_required_property",
+    "version": "1.0"
+  },
+  "host": "localhost:8000",
+  "schemes": [ "http", "https" ],
+  "consumes": [ "text/plain", "text/json" ],
+  "produces": [ "text/plain" ],
+  "paths": {
+    "/api/Parameters": {
+      "put": {
+        "tag": [ "Parameters" ],
+        "operationId": "Parameters_Put",
+        "produces": [
+          "text/plain"
+        ],
+        "parameters": [
+          {
+            "name": "database",
+            "in": "body",
+            "required": true,
+            "type": "object",
+            "schema": { "$ref": "#/definitions/Database" }
+          }
+        ]
+      }
+    }
+  },
+  "definitions": {
+    "Database": {
+      "properties": {
+        "a": {
+          "type": "string",
+          "description": "Enum.",
+          "enum": [ "a1", "a2", "a3" ]
+
+        },
+        "b": {
+          "type": "string",
+          "description": "Enum.",
+          "enum": [ "b1" ]
+        }
+      },
+      "required": [ "b" ]
+    }
+  }
+}

openapi-diff/src/modeler/AutoRest.Swagger.Tests/Resource/Swagger/old/added_required_property.json

@@ -0,0 +1,48 @@
+{
+  "swagger": 2.0,
+  "info": {
+    "title": "added_required_property",
+    "version": "1.0"
+  },
+  "host": "localhost:8000",
+  "schemes": [ "http", "https" ],
+  "consumes": [ "text/plain", "text/json" ],
+  "produces": [ "text/plain" ],
+  "paths": {
+    "/api/Parameters": {
+      "put": {
+        "tag": [ "Parameters" ],
+        "operationId": "Parameters_Put",
+        "produces": [
+          "text/plain"
+        ],
+        "parameters": [
+          {
+            "name": "database",
+            "in": "body",
+            "required": true,
+            "type": "object",
+            "schema": { "$ref": "#/definitions/Database" }
+          }
+        ]
+      }
+    }
+  },
+  "definitions": {
+    "Database": {
+      "properties": {
+        "a": {
+          "type": "string",
+          "description": "Enum.",
+          "enum": [ "a1", "a2", "a3" ]
+
+        },
+        "b": {
+          "type": "string",
+          "description": "Enum.",
+          "enum": [ "b1" ]
+        }
+      }
+    }
+  }
+}

openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs

@@ -217,6 +217,19 @@ public void RequiredParameterRemoved()
             Assert.NotEmpty(missing.Where(m => m.Severity == Category.Error && m.Path.ReadablePath == "#/paths/api/Parameters/{a}/get/f"));
         }
 
+        /// <summary>
+        /// Verifies that if you add a required property in the model, it's found.
+        /// </summary>
+        [Fact]
+        public void AddedRequiredProperty()
+        {
+            var messages = CompareSwagger("added_required_property.json").ToArray();
+            var missing = messages.Where(m => m.Severity == Category.Error && m.Id == ComparisonMessages.AddedRequiredProperty.Id);
+            Assert.Equal(2, missing.Count());
+            var error = missing.First();
+            Assert.Equal("#/paths/api/Parameters/put/database", error.Path.ReadablePath);
+        }
+
         /// <summary>
         /// Verifies that if you remove a required request header, it's found.
         /// </summary>

openapi-diff/src/modeler/AutoRest.Swagger/ComparisonMessages.cs

@@ -13,7 +13,7 @@ public static class ComparisonMessages
         {
             Id = 1034,
             Code = nameof(ComparisonMessages.AddedRequiredProperty),
-            Message = "The new version has a new required property '{0}' that was not found in the old version."
+            Message = "The new version has new required property '{0}' that was not found in the old version."
         };
 
         public static MessageTemplate AddedReadOnlyPropertyInResponse = new MessageTemplate

openapi-diff/src/modeler/AutoRest.Swagger/Model/Schema.cs

@@ -160,13 +160,41 @@ public override IEnumerable<ComparisonMessage> Compare(ComparisonContext context
                 CompareAllOfs(context, priorSchema);
             }
 
+            // Compare each properties of the model
             context.PushProperty("properties");
             CompareProperties(context, priorSchema);
             context.Pop();
 
+            // Compare `required` list of properties of the model
+            CompareRequired(context, priorSchema);
+
             return context.Messages;
         }
 
+        /// <summary>
+        /// Comapares list of required properties of this model
+        /// </summary>
+        /// <param name="context">Comaprision Context</param>
+        /// <param name="priorSchema">Schema of the old model</param>
+        private void CompareRequired(ComparisonContext context, Schema priorSchema)
+        {
+            if (Required == null && priorSchema.Required == null)
+            {
+                return;
+            }
+
+            if (Required != null && priorSchema.Required == null)
+            {
+                context.LogBreakingChange(ComparisonMessages.AddedRequiredProperty, String.Join(", ", Required));
+                return;
+            }
+
+            List<string> addedRequiredProperties = Required.Except(priorSchema.Required).ToList();
+            if (addedRequiredProperties.Count > 0)
+            {
+                context.LogBreakingChange(ComparisonMessages.AddedRequiredProperty, String.Join(", ", addedRequiredProperties));
+            }
+        }
 
         private void CompareAllOfs(ComparisonContext context, Schema priorSchema)
         {

openapi-diff/src/modeler/AutoRest.Swagger/Model/ServiceDefinition.cs

@@ -528,6 +528,12 @@ private static void ReferenceTrackSchemas(ServiceDefinition service)
 
                 foreach (var schema in service.Definitions.Values.Where(d => d.IsReferenced))
                 {
+                    // If schema does not have properties then do not recurse
+                    if (schema.Properties == null)
+                    {
+                        continue;
+                    }
+
                     foreach (var property in schema.Properties.Values)
                     {
                         if (!string.IsNullOrWhiteSpace(property.Reference))