diff --git a/documentation/rules/2001.md b/documentation/rules/2001.md new file mode 100644 index 0000000..1f7dfbe --- /dev/null +++ b/documentation/rules/2001.md @@ -0,0 +1,64 @@ +### 2001 - RemovedRequiredProperty + +**Description**: Checks whether an existing required property is removed from the previous specification. + +**Cause**: This is considered a breaking change when the property is used in responses. + +**Example**: Property `name` of schema `Person` is removed in the new version. + +Old specification +```json5 +{ + "Person": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "format": "int32", + "xml": { + "attribute": true + } + }, + "name": { + "type": "string", + "xml": { + "namespace": "http://example.com/schema/sample", + "prefix": "sample" + } + } + }, + "required": [ + "id", + "name" + ] + } +} +``` + +New specification +```json5 +{ + "Person": { + "type": "object", + "properties": { + "id": { + "type": "integer", + "format": "int32", + "xml": { + "attribute": true + } + }, + "name": { + "type": "string", + "xml": { + "namespace": "http://example.com/schema/sample", + "prefix": "sample" + } + } + }, + "required": [ + "id" + ] + } +} +``` diff --git a/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs b/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs index 307d81f..bda882d 100644 --- a/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs +++ b/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs @@ -810,6 +810,21 @@ public void Compare_OAS_Should_Return_Nullable_Property_Changed_When_Nullable_Pr }); } + [Test] + public void CompareOAS_ShouldReturn_RemovedRequiredPropertyDifferences() + { + var differences = CompareSpecifications("removed_required_property.json"); + + var expectedDifference = new ExpectedDifference + { + Rule = ComparisonRules.RemovedRequiredProperty, + Severity = Severity.Warning, + NewJsonRef = "#/paths/~1pets/put/responses/200/content/text~1plain/schema" + }; + differences.AssertContains(expectedDifference, 1); + differences.AssertContainsOnly(expectedDifference); + } + /// /// Helper method -- load two Open Api Specification documents and invoke the comparison logic. /// diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/new/removed_required_property.json b/src/Criteo.OpenApi.Comparator.UTest/Resource/new/removed_required_property.json new file mode 100644 index 0000000..28164bb --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/new/removed_required_property.json @@ -0,0 +1,75 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Removed_required_property", + "version": "2.0" + }, + "paths": { + "/pets": { + "put": { + "description": "Update a pet", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PetWrite" + } + } + } + }, + "responses": { + "200": { + "description": "Request successful", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/PetRead" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "PetRead": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "petType": { + "type": "string", + "enum": [ + "cat", + "dog" + ] + } + }, + "required": [ + "petType" + ] + }, + "PetWrite": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "petType": { + "type": "string", + "enum": [ + "cat", + "dog" + ] + } + }, + "required": [ + "petType" + ] + } + } + } +} \ No newline at end of file diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/old/different_discriminator.json b/src/Criteo.OpenApi.Comparator.UTest/Resource/old/different_discriminator.json index 943a522..3f1d717 100644 --- a/src/Criteo.OpenApi.Comparator.UTest/Resource/old/different_discriminator.json +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/old/different_discriminator.json @@ -70,9 +70,6 @@ "schemas": { "Pet": { "type": "object", - "required": [ - "name" - ], "discriminator": { "propertyName": "petType" }, diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/old/removed_required_property.json b/src/Criteo.OpenApi.Comparator.UTest/Resource/old/removed_required_property.json new file mode 100644 index 0000000..2e80fb3 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/old/removed_required_property.json @@ -0,0 +1,77 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Removed_required_property", + "version": "1.0" + }, + "paths": { + "/pets": { + "put": { + "description": "Update a pet", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PetWrite" + } + } + } + }, + "responses": { + "200": { + "description": "Request successful", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/PetRead" + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "PetRead": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "petType": { + "type": "string", + "enum": [ + "cat", + "dog" + ] + } + }, + "required": [ + "name", + "petType" + ] + }, + "PetWrite": { + "type": "object", + "properties": { + "name": { + "type": "string" + }, + "petType": { + "type": "string", + "enum": [ + "cat", + "dog" + ] + } + }, + "required": [ + "name", + "petType" + ] + } + } + } +} \ No newline at end of file diff --git a/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs b/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs index 0442ebb..51ab4a0 100644 --- a/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs +++ b/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs @@ -547,7 +547,7 @@ private static void CompareRequired(ComparisonContext context, ISet oldRequired, ISet newRequired) { - if (newRequired == null) + if (newRequired == null && (oldRequired == null || context.Direction == DataDirection.Request)) return; if (oldRequired == null) @@ -556,12 +556,29 @@ private static void CompareRequired(ComparisonContext context, return; } + if (newRequired == null) + { + context.LogBreakingChange(ComparisonRules.RemovedRequiredProperty, string.Join(", ", oldRequired)); + return; + } + List addedRequiredProperties = newRequired.Except(oldRequired).ToList(); if (addedRequiredProperties.Any()) { context.LogBreakingChange(ComparisonRules.AddedRequiredProperty, string.Join(", ", addedRequiredProperties)); } + + if (context.Direction == DataDirection.Request) + { + return; + } + List removedRequiredProperties = oldRequired.Except(newRequired).ToList(); + if (removedRequiredProperties.Any()) + { + context.LogBreakingChange(ComparisonRules.RemovedRequiredProperty, + string.Join(", ", addedRequiredProperties)); + } } } } diff --git a/src/Criteo.OpenApi.Comparator/ComparisonRules.cs b/src/Criteo.OpenApi.Comparator/ComparisonRules.cs index 4d4c4cb..398378c 100644 --- a/src/Criteo.OpenApi.Comparator/ComparisonRules.cs +++ b/src/Criteo.OpenApi.Comparator/ComparisonRules.cs @@ -609,5 +609,16 @@ public static class ComparisonRules Message = "The nullable property has changed from '{0}' to '{1}'.", Type = MessageType.Update }; + + /// + /// OpenApi Specification version 3 specific + /// + public static ComparisonRule RemovedRequiredProperty = new ComparisonRule() + { + Id = 2001, + Code = nameof(RemovedRequiredProperty), + Message = "The required property '{0}' was removed in the new version.", + Type = MessageType.Removal + }; } }