diff --git a/docs/rules/1045.md b/docs/rules/1045.md index deeb07f2..eddacc7d 100644 --- a/docs/rules/1045.md +++ b/docs/rules/1045.md @@ -1,10 +1,12 @@ ### 1045 - AddedOptionalProperty +> **Note:** Previous versions of this tool incorrectly flagged this as a breaking change - whilst care _does_ need to be taken when reviewing these - new **Optional** fields MUST be optional in request payloads for this to not be a breaking change. + **Description**: Checks whether a property is added to the model from the previous specification. The model includes all the models that referenced by any request or response. -**Cause**: This is considered a breaking change. +**Cause**: A new optional field is added to the payload. This field is **Optional** therefore does not constitute a breaking change, in the event this field is **Required** in the new API version it should instead be marked as such. -**Example**: Property `c` is being added into model `Parameters` . +**Example**: Property `c` is being added into model `Parameters`. Old specification ```json5 diff --git a/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs index 5ac2f97e..3279407e 100644 --- a/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs +++ b/openapi-diff/src/modeler/AutoRest.Swagger.Tests/SwaggerModelerCompareTests.cs @@ -899,7 +899,7 @@ public void ChangedXmsLongRunningOperation() public void AddedOptionalProperty() { var messages = CompareSwagger("added_optional_property.json").ToArray(); - Assert.Equal(1, messages.Where(m => m.Id == ComparisonMessages.AddedOptionalProperty.Id).Count()); + Assert.Equal(1, messages.Where(m => m.Id == ComparisonMessages.AddedOptionalProperty.Id && m.Severity == Category.Warning).Count()); } diff --git a/openapi-diff/src/modeler/AutoRest.Swagger/ComparisonContext.cs b/openapi-diff/src/modeler/AutoRest.Swagger/ComparisonContext.cs index de033bc7..af24f395 100644 --- a/openapi-diff/src/modeler/AutoRest.Swagger/ComparisonContext.cs +++ b/openapi-diff/src/modeler/AutoRest.Swagger/ComparisonContext.cs @@ -71,6 +71,16 @@ public void LogInfo(MessageTemplate template, params object[] formatArguments) formatArguments )); + public void LogWarning(MessageTemplate template, params object[] formatArguments) + => _messages.Add(new ComparisonMessage( + template, + Path, + _PreviousRootDoc, + _CurrentRootDoc, + Category.Warning, + formatArguments + )); + public void LogError(MessageTemplate template, params object[] formatArguments) => _messages.Add(new ComparisonMessage( template, diff --git a/openapi-diff/src/modeler/AutoRest.Swagger/Model/Schema.cs b/openapi-diff/src/modeler/AutoRest.Swagger/Model/Schema.cs index 2aad27fb..fcff9c6a 100644 --- a/openapi-diff/src/modeler/AutoRest.Swagger/Model/Schema.cs +++ b/openapi-diff/src/modeler/AutoRest.Swagger/Model/Schema.cs @@ -290,7 +290,13 @@ private void CompareProperties(ComparisonContext context, Sch } else if(IsReferenced && property.Value != null) { - context.LogBreakingChange(ComparisonMessages.AddedOptionalProperty, property.Key); + // This is an Optional Property, therefore the minimal payload (e.g. just required fields) + // from the previous API version remains valid in the new API version - therefore this + // IS NOT a breaking change by definition. + // + // Whilst the additional field CAN be specified, it's OPTIONAL by design meaning this can + // be omitted - and is therefore NOT a breaking change. + context.LogWarning(ComparisonMessages.AddedOptionalProperty, property.Key); } } }