From 21d579225d973482d2c030af420fbb0ee16f1bc6 Mon Sep 17 00:00:00 2001 From: Nam Vu Date: Wed, 10 Jul 2024 17:27:57 +0200 Subject: [PATCH] feat: better handling of enum changes based on direction | Enum Change | In Request (path/query/body) | In Response (body) | In Both | |---------------|------------------------------|--------------------|----------| | Value Added | OK | Breaking | Breaking | | Value Removed | Breaking | OK | Breaking | --- .../OpenApiEnumDirectionTests.cs | 92 +++++++++++++++++++ .../OpenApiSpecificationsCompareTests.cs | 14 +-- .../Resource/enum_direction/body_add.yaml | 46 ++++++++++ .../Resource/enum_direction/body_remove.yaml | 44 +++++++++ .../Resource/enum_direction/both_ref_add.yaml | 49 ++++++++++ .../enum_direction/both_ref_remove.yaml | 47 ++++++++++ .../enum_direction/old_with_refs.json | 68 ++++++++++++++ .../enum_direction/old_without_refs.json | 60 ++++++++++++ .../Resource/enum_direction/path_add.yaml | 46 ++++++++++ .../Resource/enum_direction/path_remove.yaml | 44 +++++++++ .../Resource/enum_direction/query_add.yaml | 46 ++++++++++ .../Resource/enum_direction/query_remove.yaml | 44 +++++++++ .../enum_direction/request_ref_add.yaml | 49 ++++++++++ .../enum_direction/request_ref_remove.yaml | 47 ++++++++++ .../Resource/enum_direction/response_add.yaml | 46 ++++++++++ .../enum_direction/response_ref_add.yaml | 49 ++++++++++ .../enum_direction/response_ref_remove.yaml | 47 ++++++++++ .../enum_direction/response_remove.yaml | 44 +++++++++ .../Comparators/RequestBodyComparator.cs | 3 +- .../Comparators/SchemaComparator.cs | 36 +++----- .../ComparisonContext.cs | 12 +++ 21 files changed, 904 insertions(+), 29 deletions(-) create mode 100644 src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_remove.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_remove.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_with_refs.json create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_without_refs.json create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_remove.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_remove.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_remove.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_add.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_remove.yaml create mode 100644 src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_remove.yaml diff --git a/src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs b/src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs new file mode 100644 index 0000000..e970291 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/OpenApiEnumDirectionTests.cs @@ -0,0 +1,92 @@ +// Copyright (c) Criteo Technology. All rights reserved. +// Licensed under the Apache 2.0 License. See LICENSE in the project root for license information. + +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("old_without_refs", "path_add", "#/paths/~1order~1{path}/post/parameters/0/schema/enum")] + [TestCase("old_without_refs", "query_add", "#/paths/~1order~1{path}/post/parameters/1/schema/enum")] + [TestCase("old_without_refs", "body_add", "#/paths/~1order~1{path}/post/requestBody/content/application~1json/schema/properties/foo/enum")] + [TestCase("old_with_refs", "request_ref_add", "#/paths/~1order~1{path}/post/parameters/0/schema/enum")] + public void CompareOAS_ShouldReturn_NonBreaking_AddedEnumValueChange(string oldName, string newName, string jsonRef, int expectedDifferencesCount = 1) + { + var differences = CompareSpecifications(oldName, newName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.AddedEnumValue, + Severity = Severity.Warning, + NewJsonRef = jsonRef + }, expectedDifferencesCount); + } + + + [TestCase("old_without_refs", "response_add", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/bar/enum")] + [TestCase("old_with_refs", "response_ref_add", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/bar/enum")] + [TestCase("old_with_refs", "both_ref_add", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/foo/enum", 2)] + public void CompareOAS_ShouldReturn_Breaking_AddedEnumValueChange(string oldName, string newName, string jsonRef, int expectedDifferencesCount = 1) + { + var differences = CompareSpecifications(oldName, newName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.AddedEnumValue, + Severity = Severity.Error, + NewJsonRef = jsonRef + }, expectedDifferencesCount); + } + + [TestCase("old_without_refs", "response_remove", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/bar/enum")] + [TestCase("old_with_refs", "response_ref_remove", "#/paths/~1order~1{path}/post/responses/200/content/application~1json/schema/properties/bar/enum")] + public void CompareOAS_ShouldReturn_NonBreaking_RemovedEnumValueChange(string oldName, string newName, string jsonRef, int expectedDifferencesCount = 1) + { + var differences = CompareSpecifications(oldName, newName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.RemovedEnumValue, + Severity = Severity.Warning, + NewJsonRef = jsonRef + }, expectedDifferencesCount); + } + + [TestCase("old_without_refs", "path_remove", "#/paths/~1order~1{path}/post/parameters/0/schema/enum")] + [TestCase("old_without_refs", "query_remove", "#/paths/~1order~1{path}/post/parameters/1/schema/enum")] + [TestCase("old_without_refs", "body_remove", "#/paths/~1order~1{path}/post/requestBody/content/application~1json/schema/properties/foo/enum")] + [TestCase("old_with_refs", "request_ref_remove", "#/paths/~1order~1{path}/post/parameters/0/schema/enum")] + [TestCase("old_with_refs", "both_ref_remove", "#/paths/~1order~1{path}/post/requestBody/content/application~1json/schema/properties/foo/enum")] + public void CompareOAS_ShouldReturn_Breaking_RemovedEnumValueChange(string oldName, string newName, string jsonRef, int expectedDifferencesCount = 1) + { + var differences = CompareSpecifications(oldName, newName); + differences.AssertContains(new ExpectedDifference + { + Rule = ComparisonRules.RemovedEnumValue, + Severity = Severity.Error, + NewJsonRef = jsonRef, + }, expectedDifferencesCount); + } + + private static IList CompareSpecifications(string oldName, string newName) + { + var baseDirectory = Directory + .GetParent(typeof(OpenApiSpecificationsCompareTests).GetTypeInfo().Assembly.Location) + .ToString(); + + var oldFileName = Path.Combine(baseDirectory, "Resource", "enum_direction", oldName + ".json"); + var newFileName = Path.Combine(baseDirectory, "Resource", "enum_direction", newName + ".yaml"); + + var differences = OpenApiComparator + .Compare(File.ReadAllText(oldFileName), 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..97ad994 100644 --- a/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs +++ b/src/Criteo.OpenApi.Comparator.UTest/OpenApiSpecificationsCompareTests.cs @@ -315,12 +315,12 @@ public void CompareOAS_ShouldReturn_RemovedEnumValueDifferences() Rule = ComparisonRules.RemovedEnumValue, Severity = Severity.Warning, OldJsonRef = "#/paths/~1api~1Parameters/put/parameters/0/schema/enum" - }, 2); + }, 3); differences.AssertContains(new ExpectedDifference { Rule = ComparisonRules.ConstraintIsStronger, Severity = Severity.Info, - }, 2); + }, 3); } [Test] @@ -333,12 +333,12 @@ public void CompareOAS_ShouldReturn_AddedEnumValueDifferences() Rule = ComparisonRules.AddedEnumValue, Severity = Severity.Warning, NewJsonRef = "#/paths/~1api~1Parameters/put/responses/200/content/application~1json/schema/properties/petType/enum" - }, 1); + }, 2); differences.AssertContains(new ExpectedDifference { Rule = ComparisonRules.ConstraintIsWeaker, Severity = Severity.Info, - }, 2); + }, 4); } [Test] @@ -627,12 +627,12 @@ public void CompareOAS_ShouldReturn_OneMessagePerRule_When_RecursiveModel() Rule = ComparisonRules.RemovedProperty, Severity = Severity.Warning, OldJsonRef = "#/paths/~1api~1Operations/post/parameters/0/schema/properties/error/properties/target" - }, 1); + }, 2); differences.AssertContains(new ExpectedDifference { Rule = ComparisonRules.ReadonlyPropertyChanged, Severity = Severity.Warning, - }, 1); + }, 2); } [Test] @@ -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..9eeab82 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_add.yaml @@ -0,0 +1,46 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - def + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - zzzzzz + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + 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..2105211 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/body_remove.yaml @@ -0,0 +1,44 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - def + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_add.yaml new file mode 100644 index 0000000..5ba88d8 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_add.yaml @@ -0,0 +1,49 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + $ref: "#/components/schemas/RequestOnlyEnum" + requestBody: + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + bar: + $ref: "#/components/schemas/ResponseOnlyEnum" +components: + schemas: + RequestOnlyEnum: + type: string + enum: + - abc + - def + ResponseOnlyEnum: + type: string + enum: + - ghi + - jkl + RequestResponseEnum: + type: string + enum: + - zzzzzz + - mno + - pqr diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_remove.yaml new file mode 100644 index 0000000..ccd6408 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/both_ref_remove.yaml @@ -0,0 +1,47 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + $ref: "#/components/schemas/RequestOnlyEnum" + requestBody: + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + bar: + $ref: "#/components/schemas/ResponseOnlyEnum" +components: + schemas: + RequestOnlyEnum: + type: string + enum: + - abc + - def + ResponseOnlyEnum: + type: string + enum: + - ghi + - jkl + RequestResponseEnum: + type: string + enum: + - pqr diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_with_refs.json b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_with_refs.json new file mode 100644 index 0000000..04d2525 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_with_refs.json @@ -0,0 +1,68 @@ +{ + "openapi": "3.0.0", + "info": { "title": "My API", "version": "0.1.0" }, + "paths": { + "/order/{path}": { + "post": { + "parameters": [ + { + "name": "path", + "in": "path", + "required": true, + "schema": { + "$ref": "#/components/schemas/RequestOnlyEnum" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "foo": { + "$ref": "#/components/schemas/RequestResponseEnum" + } + } + } + } + } + }, + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "properties": { + "foo": { + "$ref": "#/components/schemas/RequestResponseEnum" + }, + "bar": { + "$ref": "#/components/schemas/ResponseOnlyEnum" + } + } + } + } + } + } + } + } + } + }, + "components": { + "schemas": { + "RequestOnlyEnum": { + "type": "string", + "enum": ["abc", "def"] + }, + "ResponseOnlyEnum": { + "type": "string", + "enum": ["ghi", "jkl"] + }, + "RequestResponseEnum": { + "type": "string", + "enum": ["mno", "pqr"] + } + } + } +} diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_without_refs.json b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_without_refs.json new file mode 100644 index 0000000..40a9662 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/old_without_refs.json @@ -0,0 +1,60 @@ +{ + "openapi": "3.0.0", + "info": { "title": "My API", "version": "0.1.0" }, + "paths": { + "/order/{path}": { + "post": { + "parameters": [ + { + "name": "path", + "in": "path", + "required": true, + "schema": { + "type": "string", + "enum": ["abc", "def"] + } + }, + { + "name": "query", + "in": "query", + "schema": { + "type": "string", + "enum": ["ghi", "jkl"] + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "properties": { + "foo": { + "type": "string", + "enum": ["mno", "pqr"] + } + } + } + } + } + }, + "responses": { + "200": { + "description": "Successful Response", + "content": { + "application/json": { + "schema": { + "properties": { + "bar": { + "type": "string", + "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..f79d219 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_add.yaml @@ -0,0 +1,46 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - zzzzzz + - def + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + 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..f2d60eb --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/path_remove.yaml @@ -0,0 +1,44 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + 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..35cc27e --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_add.yaml @@ -0,0 +1,46 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - def + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + - zzzzzz + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + 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..0e00cc7 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/query_remove.yaml @@ -0,0 +1,44 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - def + - name: query + in: query + schema: + type: string + enum: + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + enum: + - stu + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_add.yaml new file mode 100644 index 0000000..95d768b --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_add.yaml @@ -0,0 +1,49 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + $ref: "#/components/schemas/RequestOnlyEnum" + requestBody: + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + bar: + $ref: "#/components/schemas/ResponseOnlyEnum" +components: + schemas: + RequestOnlyEnum: + type: string + enum: + - abc + - zzzzzz + - def + ResponseOnlyEnum: + type: string + enum: + - ghi + - jkl + RequestResponseEnum: + type: string + enum: + - mno + - pqr diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_remove.yaml new file mode 100644 index 0000000..27e7b6c --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/request_ref_remove.yaml @@ -0,0 +1,47 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + $ref: "#/components/schemas/RequestOnlyEnum" + requestBody: + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + bar: + $ref: "#/components/schemas/ResponseOnlyEnum" +components: + schemas: + RequestOnlyEnum: + type: string + enum: + - abc + ResponseOnlyEnum: + type: string + enum: + - ghi + - jkl + RequestResponseEnum: + type: string + enum: + - mno + - pqr 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..507e865 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_add.yaml @@ -0,0 +1,46 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - def + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + enum: + - stu + - zzzzzz + - vwx diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_add.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_add.yaml new file mode 100644 index 0000000..bb90040 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_add.yaml @@ -0,0 +1,49 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + $ref: "#/components/schemas/RequestOnlyEnum" + requestBody: + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + bar: + $ref: "#/components/schemas/ResponseOnlyEnum" +components: + schemas: + RequestOnlyEnum: + type: string + enum: + - abc + - def + ResponseOnlyEnum: + type: string + enum: + - ghi + - jkl + - zzzzzz + RequestResponseEnum: + type: string + enum: + - mno + - pqr diff --git a/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_remove.yaml b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_remove.yaml new file mode 100644 index 0000000..0fdd3d4 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_ref_remove.yaml @@ -0,0 +1,47 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + $ref: "#/components/schemas/RequestOnlyEnum" + requestBody: + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + foo: + $ref: "#/components/schemas/RequestResponseEnum" + bar: + $ref: "#/components/schemas/ResponseOnlyEnum" +components: + schemas: + RequestOnlyEnum: + type: string + enum: + - abc + - def + ResponseOnlyEnum: + type: string + enum: + - jkl + RequestResponseEnum: + type: string + enum: + - mno + - pqr 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..5136c29 --- /dev/null +++ b/src/Criteo.OpenApi.Comparator.UTest/Resource/enum_direction/response_remove.yaml @@ -0,0 +1,44 @@ +openapi: 3.0.0 +info: + title: My API + version: 0.2.0 +paths: + /order/{path}: + post: + parameters: + - name: path + in: path + required: true + schema: + type: string + enum: + - abc + - def + - name: query + in: query + schema: + type: string + enum: + - ghi + - jkl + requestBody: + content: + application/json: + schema: + properties: + foo: + type: string + enum: + - mno + - pqr + responses: + "200": + description: Successful Response + content: + application/json: + schema: + properties: + bar: + type: string + enum: + - vwx diff --git a/src/Criteo.OpenApi.Comparator/Comparators/RequestBodyComparator.cs b/src/Criteo.OpenApi.Comparator/Comparators/RequestBodyComparator.cs index 520fdaf..f194669 100644 --- a/src/Criteo.OpenApi.Comparator/Comparators/RequestBodyComparator.cs +++ b/src/Criteo.OpenApi.Comparator/Comparators/RequestBodyComparator.cs @@ -1,7 +1,6 @@ // Copyright (c) Criteo Technology. All rights reserved. // Licensed under the Apache 2.0 License. See LICENSE in the project root for license information. -using System.Collections.Generic; using Criteo.OpenApi.Comparator.Comparators.Extensions; using Microsoft.OpenApi.Models; @@ -53,6 +52,8 @@ internal void Compare(ComparisonContext context, CompareRequired(context, oldRequestBody.Required, newRequestBody.Required); _contentComparator.Compare(context, oldRequestBody.Content, newRequestBody.Content); + + context.Direction = DataDirection.None; } private static void CompareRequired(ComparisonContext context, diff --git a/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs b/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs index 0442ebb..7ac5241 100644 --- a/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs +++ b/src/Criteo.OpenApi.Comparator/Comparators/SchemaComparator.cs @@ -65,24 +65,21 @@ internal void Compare(ComparisonContext context, return; } - // Avoid doing the comparison repeatedly by marking for which direction it's already been done. if (context.Direction != DataDirection.None) { - if (!_compareDirections.ContainsKey(newSchema)) - { - _compareDirections[newSchema] = DataDirection.None; - } - // Comparing two referenced schemas in the context of a parameter or response -- did we already do this? - if (_compareDirections[newSchema] == context.Direction - || _compareDirections[newSchema] == DataDirection.Both) + _compareDirections.TryGetValue(newSchema, out var savedDirection); + + // If this direction has already been checked, skip it + if (context.Direction == savedDirection || savedDirection == DataDirection.Both) return; - _compareDirections[newSchema] |= context.Direction; + context.Direction |= savedDirection; + _compareDirections[newSchema] = context.Direction; } if (areSchemasReferenced) { - if (_visitedSchemas.Contains(oldSchema)) + if (_visitedSchemas.Contains(oldSchema) && context.Direction != DataDirection.Both) return; _visitedSchemas.AddFirst(oldSchema); @@ -110,10 +107,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) @@ -337,19 +334,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.Response ? context.LogWarning : context.LogBreakingChange; + 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.Request ? context.LogWarning : context.LogBreakingChange; + 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,