diff --git a/src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs b/src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs new file mode 100644 index 0000000..eb9228c --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs @@ -0,0 +1,82 @@ +using System.Collections.Generic; +using System.IO; +using System.Linq; +using System.Reflection; +using Criteo.OpenApi.Comparator.Logging; +using NUnit.Framework; + +namespace Criteo.OpenApi.Comparator.UTest; + +[TestFixture] +public class OpenApiEnumDirectionTests +{ + [TestCase("path_add", "#/paths/~1order~1{path}/post/parameters/0/schema/enum")] + [TestCase("query_add", "#/paths/~1order~1{path}/post/parameters/1/schema/enum")] + [TestCase("body_add", "#/paths/~1order~1{path}/post/requestBody/content/application~1json/schema/properties/foo/enum")] + public void CompareOAS_ShouldReturn_NonBreakingEnumChange_InRequest(string oasName, string jsonRef) + { + var differences = CompareSpecifications(oasName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.AddedEnumValue, + Severity = Severity.Warning, + NewJsonRef = jsonRef + }, 1); + } + + [TestCase("response_remove", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/bar/enum")] + public void CompareOAS_ShouldReturn_NonBreakingEnumChange_InResponse(string oasName, string jsonRef) + { + var differences = CompareSpecifications(oasName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.RemovedEnumValue, + Severity = Severity.Warning, + NewJsonRef = jsonRef + }, 1); + } + + [TestCase("path_remove", "#/paths/~1order~1{path}/post/parameters/0/schema/enum")] + [TestCase("query_remove", "#/paths/~1order~1{path}/post/parameters/1/schema/enum")] + [TestCase("body_remove", "#/paths/~1order~1{path}/post/requestBody/content/application~1json/schema/properties/foo/enum")] + public void CompareOAS_ShouldReturn_BreakingEnumChange_InRequest(string oasName, string jsonRef) + { + var differences = CompareSpecifications(oasName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.RemovedEnumValue, + Severity = Severity.Error, + NewJsonRef = jsonRef, + }, 1); + } + + [TestCase("response_add", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/bar/enum")] + public void CompareOAS_ShouldReturn_BreakingEnumChange_InResponse(string oasName, string jsonRef) + { + var differences = CompareSpecifications(oasName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.AddedEnumValue, + Severity = Severity.Error, + NewJsonRef = jsonRef + }, 1); + } + + private static IList CompareSpecifications(string oasName) + { + var baseDirectory = Directory + .GetParent(typeof(OpenApiSpecificationsCompareTests).GetTypeInfo().Assembly.Location) + .ToString(); + + var referenceFileName = Path.Combine(baseDirectory, "Resource", "enum_direction", "reference.json"); + var newFileName = Path.Combine(baseDirectory, "Resource", "enum_direction", oasName + ".yaml"); + + var differences = OpenApiComparator + .Compare(File.ReadAllText(referenceFileName), File.ReadAllText(newFileName), strict: true) + .ToList(); + + OpenApiSpecificationsCompareTests.ValidateDifferences(differences); + + return differences; + } +} diff --git a/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs b/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs index 7c265d8..5b48070 100644 --- a/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs +++ b/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs @@ -859,7 +859,7 @@ private static void ValidateMessage(ComparisonMessage message) } } - private static void ValidateDifferences(IEnumerable differences) + internal static void ValidateDifferences(IEnumerable differences) { foreach (var message in differences) { diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_add.yaml new file mode 100644 index 0000000..6563a60 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_add.yaml @@ -0,0 +1,42 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - def + - name: query + in: query + schema: + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - zzzzzz + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_remove.yaml new file mode 100644 index 0000000..51c84a4 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_remove.yaml @@ -0,0 +1,40 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - def + - name: query + in: query + schema: + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_add.yaml new file mode 100644 index 0000000..fa9d6c0 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_add.yaml @@ -0,0 +1,42 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - zzzzzz + - def + - name: query + in: query + schema: + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_remove.yaml new file mode 100644 index 0000000..7b48365 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_remove.yaml @@ -0,0 +1,40 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - name: query + in: query + schema: + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_add.yaml new file mode 100644 index 0000000..b712163 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_add.yaml @@ -0,0 +1,42 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - def + - name: query + in: query + schema: + enum: + - ghi + - jkl + - zzzzzz + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_remove.yaml new file mode 100644 index 0000000..bb2e419 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_remove.yaml @@ -0,0 +1,40 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - def + - name: query + in: query + schema: + enum: + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/reference.json b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/reference.json new file mode 100644 index 0000000..be3456d --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/reference.json @@ -0,0 +1,48 @@ +{ + "openapi": "3.0.0", + "info": { "title": "My API", "version": "0.1.0" }, + "paths": { + "/order/{path}": { + "post": { + "parameters": [ + { + "name": "path", + "in": "path", + "required": true, + "schema": { "enum": ["abc", "def"] } + }, + { + "name": "query", + "in": "query", + "schema": { "enum": ["ghi", "jkl"] } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "foo": { "enum": ["mno", "pqr"] } + } + } + } + } + }, + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "properties": { + "bar": { "enum": ["stu", "vwx"] } + } + } + } + } + } + } + } + } + } +} diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_add.yaml new file mode 100644 index 0000000..86f96e9 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_add.yaml @@ -0,0 +1,42 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - def + - name: query + in: query + schema: + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - stu + - zzzzzz + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_remove.yaml new file mode 100644 index 0000000..c9ba6f3 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_remove.yaml @@ -0,0 +1,40 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + enum: + - abc + - def + - name: query + in: query + schema: + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + enum: + - vwx diff --git a/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs b/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs index 0442ebb..06c55d6 100644 --- a/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs +++ b/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs @@ -110,10 +110,10 @@ internal void Compare(ComparisonContext context, CompareProperties(context, oldSchema, newSchema, isSchemaReferenced); CompareRequired(context, oldSchema.Required, newSchema.Required); - + CompareNullable(context, oldSchema.Nullable, newSchema.Nullable); } - + private static void CompareNullable(ComparisonContext context, bool oldNullable, bool newNullable) @@ -171,8 +171,8 @@ private static void CompareDefault(ComparisonContext context, context.Pop(); } - private static void CompareConstraints(ComparisonContext context, - OpenApiSchema oldSchema, OpenApiSchema newSchema) + private static void CompareConstraints(ComparisonContext context, + OpenApiSchema oldSchema, OpenApiSchema newSchema) { if (oldSchema.Maximum.DifferFrom(newSchema.Maximum) || oldSchema.ExclusiveMaximum != newSchema.ExclusiveMaximum) @@ -230,30 +230,30 @@ private static void CompareConstraints(ComparisonContext context, } } - private static void CompareConstraint(ComparisonContext context, decimal? oldConstraint, - decimal? newConstraint, string attributeName, bool isLowerBound, bool additionalCondition = false) - { - context.PushProperty(attributeName); - if (additionalCondition) - { - context.LogBreakingChange(ComparisonRules.ConstraintChanged, attributeName); - } - else if (Narrows(oldConstraint, newConstraint, isLowerBound)) - { - if (context.Direction == DataDirection.Request) + private static void CompareConstraint(ComparisonContext context, decimal? oldConstraint, + decimal? newConstraint, string attributeName, bool isLowerBound, bool additionalCondition = false) + { + context.PushProperty(attributeName); + if (additionalCondition) + { + context.LogBreakingChange(ComparisonRules.ConstraintChanged, attributeName); + } + else if (Narrows(oldConstraint, newConstraint, isLowerBound)) + { + if (context.Direction == DataDirection.Request) context.LogBreakingChange(ComparisonRules.ConstraintIsStronger, attributeName); - else + else context.LogInfo(ComparisonRules.ConstraintIsStronger, attributeName); - } - else if (Widens(oldConstraint, newConstraint, isLowerBound)) - { - if (context.Direction == DataDirection.Response) + } + else if (Widens(oldConstraint, newConstraint, isLowerBound)) + { + if (context.Direction == DataDirection.Response) context.LogBreakingChange(ComparisonRules.ConstraintIsWeaker, attributeName); - else + else context.LogInfo(ComparisonRules.ConstraintIsWeaker, attributeName); - } - context.Pop(); - } + } + context.Pop(); + } private static bool Narrows(decimal? oldConstraint, decimal? newConstraint, bool isLowerBound) { @@ -337,19 +337,16 @@ private static void CompareEnum(ComparisonContext context, var addedEnums = newEnum.Where(newEnumElement => oldEnum.All(newEnumElement.DifferFrom)).ToList(); relaxes = addedEnums.Any(); - if (context.Direction == DataDirection.Request && constrains) + if (constrains) { - context.LogBreakingChange(ComparisonRules.RemovedEnumValue, - string.Join(", ", removedEnums)); + LogAction logger = context.Direction == DataDirection.Request ? context.LogBreakingChange : context.LogWarning; + logger(ComparisonRules.RemovedEnumValue, string.Join(", ", removedEnums)); } - if (context.Direction == DataDirection.Response && relaxes) + if (relaxes && !IsEnumModelAsString(enumExtension)) { - if (!IsEnumModelAsString(enumExtension)) - { - context.LogBreakingChange(ComparisonRules.AddedEnumValue, - string.Join(", ", addedEnums)); - } + LogAction logger = context.Direction == DataDirection.Response ? context.LogBreakingChange : context.LogWarning; + logger(ComparisonRules.AddedEnumValue, string.Join(", ", addedEnums)); } } diff --git a/src/Criteo.OpenApi.Comparator/ComparisonContext.cs b/src/Criteo.OpenApi.Comparator/ComparisonContext.cs index 619ce1b..7ad0f72 100644 --- a/src/Criteo.OpenApi.Comparator/ComparisonContext.cs +++ b/src/Criteo.OpenApi.Comparator/ComparisonContext.cs @@ -9,6 +9,8 @@ namespace Criteo.OpenApi.Comparator { + internal delegate void LogAction(ComparisonRule rule, params object[] formatArguments); + /// /// Provides context for a comparison, such as the ancestors in the validation tree, the root object /// and information about the key or index that locate this object in the parent's list or dictionary @@ -69,6 +71,16 @@ internal void LogInfo(ComparisonRule rule, params object[] formatArguments) => formatArguments )); + internal void LogWarning(ComparisonRule rule, params object[] formatArguments) => + _messages.Add(new ComparisonMessage( + rule, + Path, + _oldOpenApiDocument, + _newOpenApiDocument, + Severity.Warning, + formatArguments + )); + internal void LogError(ComparisonRule rule, params object[] formatArguments) => _messages.Add(new ComparisonMessage( rule,