From 341d77d9e296c9f1a077ed061c4a2d6cdc8a71e4 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Wed, 24 Apr 2024 17:01:39 +0300 Subject: [PATCH] Compare inline schemas under AllOf/AnyOf/OneOf by title (#527) --- BREAKING-CHANGES-EXAMPLES.md | 1 + ...ck-request-property-all-of-updated_test.go | 8 +- ...ck-request-property-any-of-updated_test.go | 8 +- ...ck-request-property-one-of-updated_test.go | 8 +- ...k-response-property-all-of-updated_test.go | 8 +- ...k-response-property-any-of-updated_test.go | 8 +- ...k-response-property-one-of-updated_test.go | 12 +- ...eck-response-property-type-changed_test.go | 42 +++ checker/checker_breaking_test.go | 16 +- checker/checks-utils.go | 36 +- ...response_property_any_of_complex_base.yaml | 50 +++ ...onse_property_any_of_complex_revision.yaml | 57 ++++ .../response-enum-one-of-inline-2.yaml | 83 +++++ .../response-enum-one-of-inline.yaml | 76 +++++ data/x-of-titles/spec1.yaml | 18 + data/x-of-titles/spec2.yaml | 18 + data/x-of-titles/spec3.yaml | 18 + data/x-of-titles/spec4.yaml | 20 ++ data/x-of-titles/spec5.yaml | 20 ++ data/x-of-titles/spec6.yaml | 22 ++ data/x-of-titles/spec7.yaml | 22 ++ diff/diff_x_of_test.go | 126 +++++-- diff/diff_x_of_titles_test.go | 223 ++++++++++++ diff/modified_schemas.go | 33 -- diff/modified_subschemas.go | 104 ++++++ diff/modified_subschemas_test.go | 90 +++++ diff/schema_diff.go | 12 +- diff/schema_list_diff.go | 217 ------------ diff/schema_ref_map.go | 40 ++- diff/schemas_diff.go | 30 +- diff/subschemas_diff.go | 323 ++++++++++++++++++ report/report.go | 8 +- 32 files changed, 1396 insertions(+), 361 deletions(-) create mode 100644 data/checker/response_property_any_of_complex_base.yaml create mode 100644 data/checker/response_property_any_of_complex_revision.yaml create mode 100644 data/x-of-titles/response-enum-one-of-inline-2.yaml create mode 100644 data/x-of-titles/response-enum-one-of-inline.yaml create mode 100644 data/x-of-titles/spec1.yaml create mode 100644 data/x-of-titles/spec2.yaml create mode 100644 data/x-of-titles/spec3.yaml create mode 100644 data/x-of-titles/spec4.yaml create mode 100644 data/x-of-titles/spec5.yaml create mode 100644 data/x-of-titles/spec6.yaml create mode 100644 data/x-of-titles/spec7.yaml create mode 100644 diff/diff_x_of_titles_test.go delete mode 100644 diff/modified_schemas.go create mode 100644 diff/modified_subschemas.go create mode 100644 diff/modified_subschemas_test.go delete mode 100644 diff/schema_list_diff.go create mode 100644 diff/subschemas_diff.go diff --git a/BREAKING-CHANGES-EXAMPLES.md b/BREAKING-CHANGES-EXAMPLES.md index 93f47891..767635ca 100644 --- a/BREAKING-CHANGES-EXAMPLES.md +++ b/BREAKING-CHANGES-EXAMPLES.md @@ -201,6 +201,7 @@ These examples are automatically generated from unit tests. [changing optional response property to write-only](checker/check-response-optional-property-write-only-read-only_test.go?plain=1#L12) [changing optional response write-only property to required](checker/check-response-property-became-required_test.go?plain=1#L33) [changing pattern of request parameters](checker/check-request-parameter-pattern-added-or-changed_test.go?plain=1#L12) +[changing properties of subschemas under allOf](checker/check-response-property-type-changed_test.go?plain=1#L82) [changing request body and property types from array to object](checker/check-request-property-type-changed_test.go?plain=1#L85) [changing request body and property types from object to array](checker/check-request-property-type-changed_test.go?plain=1#L117) [changing request body default value](checker/check-request-property-default-value-changed_test.go?plain=1#L12) diff --git a/checker/check-request-property-all-of-updated_test.go b/checker/check-request-property-all-of-updated_test.go index cd3064a2..6b4d6ae1 100644 --- a/checker/check-request-property-all-of-updated_test.go +++ b/checker/check-request-property-all-of-updated_test.go @@ -25,7 +25,7 @@ func TestRequestPropertyAllOfAdded(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.RequestBodyAllOfAddedId, - Args: []any{"Rabbit"}, + Args: []any{"#/components/schemas/Rabbit"}, Level: checker.ERR, Operation: "POST", Path: "/pets", @@ -34,7 +34,7 @@ func TestRequestPropertyAllOfAdded(t *testing.T) { }, { Id: checker.RequestPropertyAllOfAddedId, - Args: []any{"Breed3", "/allOf[#/components/schemas/Dog]/breed"}, + Args: []any{"#/components/schemas/Breed3", "/allOf[#/components/schemas/Dog]/breed"}, Level: checker.ERR, Operation: "POST", Path: "/pets", @@ -59,7 +59,7 @@ func TestRequestPropertyAllOfRemoved(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.RequestBodyAllOfRemovedId, - Args: []any{"Rabbit"}, + Args: []any{"#/components/schemas/Rabbit"}, Level: checker.WARN, Operation: "POST", Path: "/pets", @@ -68,7 +68,7 @@ func TestRequestPropertyAllOfRemoved(t *testing.T) { }, { Id: checker.RequestPropertyAllOfRemovedId, - Args: []any{"Breed3", "/allOf[#/components/schemas/Dog]/breed"}, + Args: []any{"#/components/schemas/Breed3", "/allOf[#/components/schemas/Dog]/breed"}, Level: checker.WARN, Operation: "POST", Path: "/pets", diff --git a/checker/check-request-property-any-of-updated_test.go b/checker/check-request-property-any-of-updated_test.go index 5960ec8e..7d852fec 100644 --- a/checker/check-request-property-any-of-updated_test.go +++ b/checker/check-request-property-any-of-updated_test.go @@ -25,7 +25,7 @@ func TestRequestPropertyAnyOfAdded(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.RequestBodyAnyOfAddedId, - Args: []any{"Rabbit"}, + Args: []any{"#/components/schemas/Rabbit"}, Level: checker.INFO, Operation: "POST", Path: "/pets", @@ -34,7 +34,7 @@ func TestRequestPropertyAnyOfAdded(t *testing.T) { }, { Id: checker.RequestPropertyAnyOfAddedId, - Args: []any{"Breed3", "/anyOf[#/components/schemas/Dog]/breed"}, + Args: []any{"#/components/schemas/Breed3", "/anyOf[#/components/schemas/Dog]/breed"}, Level: checker.INFO, Operation: "POST", Path: "/pets", @@ -59,7 +59,7 @@ func TestRequestPropertyAnyOfRemoved(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.RequestBodyAnyOfRemovedId, - Args: []any{"Rabbit"}, + Args: []any{"#/components/schemas/Rabbit"}, Level: checker.ERR, Operation: "POST", Path: "/pets", @@ -68,7 +68,7 @@ func TestRequestPropertyAnyOfRemoved(t *testing.T) { }, { Id: checker.RequestPropertyAnyOfRemovedId, - Args: []any{"Breed3", "/anyOf[#/components/schemas/Dog]/breed"}, + Args: []any{"#/components/schemas/Breed3", "/anyOf[#/components/schemas/Dog]/breed"}, Level: checker.ERR, Operation: "POST", Path: "/pets", diff --git a/checker/check-request-property-one-of-updated_test.go b/checker/check-request-property-one-of-updated_test.go index 0a5a924e..91bee3f9 100644 --- a/checker/check-request-property-one-of-updated_test.go +++ b/checker/check-request-property-one-of-updated_test.go @@ -25,7 +25,7 @@ func TestRequestPropertyOneOfAdded(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.RequestBodyOneOfAddedId, - Args: []any{"Rabbit"}, + Args: []any{"#/components/schemas/Rabbit"}, Level: checker.INFO, Operation: "POST", Path: "/pets", @@ -34,7 +34,7 @@ func TestRequestPropertyOneOfAdded(t *testing.T) { }, { Id: checker.RequestPropertyOneOfAddedId, - Args: []any{"Breed3", "/oneOf[#/components/schemas/Dog]/breed"}, + Args: []any{"#/components/schemas/Breed3", "/oneOf[#/components/schemas/Dog]/breed"}, Level: checker.INFO, Operation: "POST", Path: "/pets", @@ -59,7 +59,7 @@ func TestRequestPropertyOneOfRemoved(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.RequestBodyOneOfRemovedId, - Args: []any{"Rabbit"}, + Args: []any{"#/components/schemas/Rabbit"}, Level: checker.ERR, Operation: "POST", Path: "/pets", @@ -68,7 +68,7 @@ func TestRequestPropertyOneOfRemoved(t *testing.T) { }, { Id: checker.RequestPropertyOneOfRemovedId, - Args: []any{"Breed3", "/oneOf[#/components/schemas/Dog]/breed"}, + Args: []any{"#/components/schemas/Breed3", "/oneOf[#/components/schemas/Dog]/breed"}, Level: checker.ERR, Operation: "POST", Path: "/pets", diff --git a/checker/check-response-property-all-of-updated_test.go b/checker/check-response-property-all-of-updated_test.go index 1012aebd..4a8b1e7c 100644 --- a/checker/check-response-property-all-of-updated_test.go +++ b/checker/check-response-property-all-of-updated_test.go @@ -25,7 +25,7 @@ func TestResponsePropertyAllOfAdded(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.ResponseBodyAllOfAddedId, - Args: []any{"Rabbit", "200"}, + Args: []any{"#/components/schemas/Rabbit", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -34,7 +34,7 @@ func TestResponsePropertyAllOfAdded(t *testing.T) { }, { Id: checker.ResponsePropertyAllOfAddedId, - Args: []any{"Breed3", "/allOf[#/components/schemas/Dog]/breed", "200"}, + Args: []any{"#/components/schemas/Breed3", "/allOf[#/components/schemas/Dog]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -59,7 +59,7 @@ func TestResponsePropertyAllOfRemoved(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.ResponseBodyAllOfRemovedId, - Args: []any{"Rabbit", "200"}, + Args: []any{"#/components/schemas/Rabbit", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -68,7 +68,7 @@ func TestResponsePropertyAllOfRemoved(t *testing.T) { }, { Id: checker.ResponsePropertyAllOfRemovedId, - Args: []any{"Breed3", "/allOf[#/components/schemas/Dog]/breed", "200"}, + Args: []any{"#/components/schemas/Breed3", "/allOf[#/components/schemas/Dog]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", diff --git a/checker/check-response-property-any-of-updated_test.go b/checker/check-response-property-any-of-updated_test.go index 7c70a408..63a6bfc6 100644 --- a/checker/check-response-property-any-of-updated_test.go +++ b/checker/check-response-property-any-of-updated_test.go @@ -25,7 +25,7 @@ func TestResponsePropertyAnyOfAdded(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.ResponseBodyAnyOfAddedId, - Args: []any{"Rabbit", "200"}, + Args: []any{"#/components/schemas/Rabbit", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -34,7 +34,7 @@ func TestResponsePropertyAnyOfAdded(t *testing.T) { }, { Id: checker.ResponsePropertyAnyOfAddedId, - Args: []any{"Breed3", "/anyOf[#/components/schemas/Dog]/breed", "200"}, + Args: []any{"#/components/schemas/Breed3", "/anyOf[#/components/schemas/Dog]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -59,7 +59,7 @@ func TestResponsePropertyAnyOfRemoved(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.ResponseBodyAnyOfRemovedId, - Args: []any{"Rabbit", "200"}, + Args: []any{"#/components/schemas/Rabbit", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -68,7 +68,7 @@ func TestResponsePropertyAnyOfRemoved(t *testing.T) { }, { Id: checker.ResponsePropertyAnyOfRemovedId, - Args: []any{"Breed3", "/anyOf[#/components/schemas/Dog]/breed", "200"}, + Args: []any{"#/components/schemas/Breed3", "/anyOf[#/components/schemas/Dog]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", diff --git a/checker/check-response-property-one-of-updated_test.go b/checker/check-response-property-one-of-updated_test.go index 48535739..a354717c 100644 --- a/checker/check-response-property-one-of-updated_test.go +++ b/checker/check-response-property-one-of-updated_test.go @@ -25,7 +25,7 @@ func TestResponsePropertyOneOfAdded(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.ResponseBodyOneOfAddedId, - Args: []any{"Rabbit", "200"}, + Args: []any{"#/components/schemas/Rabbit", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -34,7 +34,7 @@ func TestResponsePropertyOneOfAdded(t *testing.T) { }, { Id: checker.ResponsePropertyOneOfAddedId, - Args: []any{"Breed3", "/oneOf[#/components/schemas/Dog]/breed", "200"}, + Args: []any{"#/components/schemas/Breed3", "/oneOf[#/components/schemas/Dog]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -43,7 +43,7 @@ func TestResponsePropertyOneOfAdded(t *testing.T) { }, { Id: checker.ResponsePropertyOneOfAddedId, - Args: []any{"RevisionSchema[1]:Dark brown types", "/oneOf[#/components/schemas/Fox]/breed", "200"}, + Args: []any{"subschema #2: Dark brown types", "/oneOf[#/components/schemas/Fox]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -68,7 +68,7 @@ func TestResponsePropertyOneOfRemoved(t *testing.T) { require.ElementsMatch(t, []checker.ApiChange{ { Id: checker.ResponseBodyOneOfRemovedId, - Args: []any{"Rabbit", "200"}, + Args: []any{"#/components/schemas/Rabbit", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -77,7 +77,7 @@ func TestResponsePropertyOneOfRemoved(t *testing.T) { }, { Id: checker.ResponsePropertyOneOfRemovedId, - Args: []any{"Breed3", "/oneOf[#/components/schemas/Dog]/breed", "200"}, + Args: []any{"#/components/schemas/Breed3", "/oneOf[#/components/schemas/Dog]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", @@ -86,7 +86,7 @@ func TestResponsePropertyOneOfRemoved(t *testing.T) { }, { Id: checker.ResponsePropertyOneOfRemovedId, - Args: []any{"BaseSchema[1]:Dark brown types", "/oneOf[#/components/schemas/Fox]/breed", "200"}, + Args: []any{"subschema #2: Dark brown types", "/oneOf[#/components/schemas/Fox]/breed", "200"}, Level: checker.INFO, Operation: "GET", Path: "/pets", diff --git a/checker/check-response-property-type-changed_test.go b/checker/check-response-property-type-changed_test.go index ae3ef16b..baa76fa9 100644 --- a/checker/check-response-property-type-changed_test.go +++ b/checker/check-response-property-type-changed_test.go @@ -78,3 +78,45 @@ func TestResponsePropertyFormatChangedCheck(t *testing.T) { OperationId: "createOneGroup", }, errs[0]) } + +// CL: changing properties of subschemas under allOf +func TestResponsePropertyAnyOfModified(t *testing.T) { + s1, err := open("../data/checker/response_property_any_of_complex_base.yaml") + require.NoError(t, err) + s2, err := open("../data/checker/response_property_any_of_complex_revision.yaml") + require.NoError(t, err) + + d, osm, err := diff.GetWithOperationsSourcesMap(diff.NewConfig(), s1, s2) + require.NoError(t, err) + errs := checker.CheckBackwardCompatibilityUntilLevel(singleCheckConfig(checker.ResponsePropertyTypeChangedCheck), d, osm, checker.INFO) + + require.Len(t, errs, 3) + require.ElementsMatch(t, []checker.ApiChange{ + { + Id: checker.ResponsePropertyTypeChangedId, + Args: []any{"/anyOf[#/components/schemas/Dog]/breed/anyOf[#/components/schemas/Breed2]/name", "string", "", "number", "", "200"}, + Level: checker.ERR, + Operation: "GET", + Path: "/pets", + Source: load.NewSource("../data/checker/response_property_any_of_complex_revision.yaml"), + OperationId: "listPets", + }, + { + Id: checker.ResponsePropertyTypeChangedId, + Args: []any{"/anyOf[subschema #3: Rabbit]/", "string", "", "number", "", "200"}, + Level: checker.ERR, + Operation: "GET", + Path: "/pets", + Source: load.NewSource("../data/checker/response_property_any_of_complex_revision.yaml"), + OperationId: "listPets", + }, + { + Id: checker.ResponsePropertyTypeChangedId, + Args: []any{"/anyOf[subschema #4 -> subschema #5]/", "string", "", "number", "", "200"}, + Level: checker.ERR, + Operation: "GET", + Path: "/pets", + Source: load.NewSource("../data/checker/response_property_any_of_complex_revision.yaml"), + OperationId: "listPets", + }}, errs) +} diff --git a/checker/checker_breaking_test.go b/checker/checker_breaking_test.go index cdab30b0..c0bac4cf 100644 --- a/checker/checker_breaking_test.go +++ b/checker/checker_breaking_test.go @@ -682,11 +682,11 @@ func TestBreaking_RequestPropertyAnyOfRemoved(t *testing.T) { require.Equal(t, checker.RequestBodyAnyOfRemovedId, errs[0].GetId()) require.Equal(t, checker.ERR, errs[0].GetLevel()) - require.Equal(t, "removed 'Rabbit' from the request body 'anyOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "removed '#/components/schemas/Rabbit' from the request body 'anyOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) require.Equal(t, checker.RequestPropertyAnyOfRemovedId, errs[1].GetId()) require.Equal(t, checker.ERR, errs[1].GetLevel()) - require.Equal(t, "removed 'Breed3' from the '/anyOf[#/components/schemas/Dog]/breed' request property 'anyOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "removed '#/components/schemas/Breed3' from the '/anyOf[#/components/schemas/Dog]/breed' request property 'anyOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) } // BC: removing 'oneOf' schema from the request body or request body property is breaking @@ -703,11 +703,11 @@ func TestBreaking_RequestPropertyOneOfRemoved(t *testing.T) { require.Len(t, errs, 2) require.Equal(t, checker.RequestBodyOneOfRemovedId, errs[0].GetId()) require.Equal(t, checker.ERR, errs[0].GetLevel()) - require.Equal(t, "removed 'Rabbit' from the request body 'oneOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "removed '#/components/schemas/Rabbit' from the request body 'oneOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) require.Equal(t, checker.RequestPropertyOneOfRemovedId, errs[1].GetId()) require.Equal(t, checker.ERR, errs[1].GetLevel()) - require.Equal(t, "removed 'Breed3' from the '/oneOf[#/components/schemas/Dog]/breed' request property 'oneOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "removed '#/components/schemas/Breed3' from the '/oneOf[#/components/schemas/Dog]/breed' request property 'oneOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) } // BC: adding 'allOf' subschema to the request body or request body property is breaking @@ -725,11 +725,11 @@ func TestBreaking_RequestPropertyAllOfAdded(t *testing.T) { require.Equal(t, checker.RequestBodyAllOfAddedId, errs[0].GetId()) require.Equal(t, checker.ERR, errs[0].GetLevel()) - require.Equal(t, "added 'Rabbit' to the request body 'allOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "added '#/components/schemas/Rabbit' to the request body 'allOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) require.Equal(t, checker.RequestPropertyAllOfAddedId, errs[1].GetId()) require.Equal(t, checker.ERR, errs[1].GetLevel()) - require.Equal(t, "added 'Breed3' to the '/allOf[#/components/schemas/Dog]/breed' request property 'allOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "added '#/components/schemas/Breed3' to the '/allOf[#/components/schemas/Dog]/breed' request property 'allOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) } // BC: removing 'allOf' subschema from the request body or request body property is breaking with warn @@ -747,9 +747,9 @@ func TestBreaking_RequestPropertyAllOfRemoved(t *testing.T) { require.Equal(t, checker.RequestBodyAllOfRemovedId, errs[0].GetId()) require.Equal(t, checker.WARN, errs[0].GetLevel()) - require.Equal(t, "removed 'Rabbit' from the request body 'allOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "removed '#/components/schemas/Rabbit' from the request body 'allOf' list", errs[0].GetUncolorizedText(checker.NewDefaultLocalizer())) require.Equal(t, checker.RequestPropertyAllOfRemovedId, errs[1].GetId()) require.Equal(t, checker.WARN, errs[1].GetLevel()) - require.Equal(t, "removed 'Breed3' from the '/allOf[#/components/schemas/Dog]/breed' request property 'allOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) + require.Equal(t, "removed '#/components/schemas/Breed3' from the '/allOf[#/components/schemas/Dog]/breed' request property 'allOf' list", errs[1].GetUncolorizedText(checker.NewDefaultLocalizer())) } diff --git a/checker/checks-utils.go b/checker/checks-utils.go index 1e59e252..24393c8f 100644 --- a/checker/checks-utils.go +++ b/checker/checks-utils.go @@ -74,20 +74,20 @@ func processModifiedPropertiesDiff(propertyPath string, propertyName string, sch } if schemaDiff.AllOfDiff != nil { - for k, v := range schemaDiff.AllOfDiff.Modified { - processModifiedPropertiesDiff(fmt.Sprintf("%s/allOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.AllOfDiff.Modified { + processModifiedPropertiesDiff(fmt.Sprintf("%s/allOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } if schemaDiff.AnyOfDiff != nil { - for k, v := range schemaDiff.AnyOfDiff.Modified { - processModifiedPropertiesDiff(fmt.Sprintf("%s/anyOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.AnyOfDiff.Modified { + processModifiedPropertiesDiff(fmt.Sprintf("%s/anyOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } if schemaDiff.OneOfDiff != nil { - for k, v := range schemaDiff.OneOfDiff.Modified { - processModifiedPropertiesDiff(fmt.Sprintf("%s/oneOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.OneOfDiff.Modified { + processModifiedPropertiesDiff(fmt.Sprintf("%s/oneOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } @@ -119,20 +119,20 @@ func processAddedPropertiesDiff(propertyPath string, propertyName string, schema } if schemaDiff.AllOfDiff != nil { - for k, v := range schemaDiff.AllOfDiff.Modified { - processAddedPropertiesDiff(fmt.Sprintf("%s/allOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.AllOfDiff.Modified { + processAddedPropertiesDiff(fmt.Sprintf("%s/allOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } if schemaDiff.AnyOfDiff != nil { - for k, v := range schemaDiff.AnyOfDiff.Modified { - processAddedPropertiesDiff(fmt.Sprintf("%s/anyOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.AnyOfDiff.Modified { + processAddedPropertiesDiff(fmt.Sprintf("%s/anyOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } if schemaDiff.OneOfDiff != nil { - for k, v := range schemaDiff.OneOfDiff.Modified { - processAddedPropertiesDiff(fmt.Sprintf("%s/oneOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.OneOfDiff.Modified { + processAddedPropertiesDiff(fmt.Sprintf("%s/oneOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } @@ -168,19 +168,19 @@ func processDeletedPropertiesDiff(propertyPath string, propertyName string, sche } if schemaDiff.AllOfDiff != nil { - for k, v := range schemaDiff.AllOfDiff.Modified { - processDeletedPropertiesDiff(fmt.Sprintf("%s/allOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.AllOfDiff.Modified { + processDeletedPropertiesDiff(fmt.Sprintf("%s/allOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } if schemaDiff.AnyOfDiff != nil { - for k, v := range schemaDiff.AnyOfDiff.Modified { - processDeletedPropertiesDiff(fmt.Sprintf("%s/anyOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.AnyOfDiff.Modified { + processDeletedPropertiesDiff(fmt.Sprintf("%s/anyOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } if schemaDiff.OneOfDiff != nil { - for k, v := range schemaDiff.OneOfDiff.Modified { - processDeletedPropertiesDiff(fmt.Sprintf("%s/oneOf[%s]", propertyPath, k), "", v, schemaDiff, processor) + for _, v := range schemaDiff.OneOfDiff.Modified { + processDeletedPropertiesDiff(fmt.Sprintf("%s/oneOf[%s]", propertyPath, v), "", v.Diff, schemaDiff, processor) } } diff --git a/data/checker/response_property_any_of_complex_base.yaml b/data/checker/response_property_any_of_complex_base.yaml new file mode 100644 index 00000000..e51eb868 --- /dev/null +++ b/data/checker/response_property_any_of_complex_base.yaml @@ -0,0 +1,50 @@ +openapi: 3.0.0 +info: + title: ACME + version: 1.0.0 + +paths: + /pets: + get: + operationId: listPets + responses: + "200": + content: + application/json: + schema: + anyOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + - title: Rabbit + type: string + - title: + type: string +components: + schemas: + Dog: + type: object + properties: + name: + type: string + breed: + type: object + anyOf: + - $ref: "#/components/schemas/Breed1" + - $ref: "#/components/schemas/Breed2" + + Breed1: + type: object + properties: + name: + type: string + Breed2: + type: object + properties: + name: + type: string + + Cat: + type: object + properties: + name: + type: string diff --git a/data/checker/response_property_any_of_complex_revision.yaml b/data/checker/response_property_any_of_complex_revision.yaml new file mode 100644 index 00000000..e6e02720 --- /dev/null +++ b/data/checker/response_property_any_of_complex_revision.yaml @@ -0,0 +1,57 @@ +openapi: 3.0.0 +info: + title: ACME + version: 1.0.0 + +paths: + /pets: + get: + operationId: listPets + responses: + "200": + content: + application/json: + schema: + anyOf: + - $ref: "#/components/schemas/Dog" + - $ref: "#/components/schemas/Cat" + - title: Rabbit + type: number + - $ref: "#/components/schemas/Fox" + - title: + type: number +components: + schemas: + Dog: + type: object + properties: + name: + type: string + breed: + type: object + anyOf: + - $ref: "#/components/schemas/Breed2" + - $ref: "#/components/schemas/Breed1" + + Breed1: + type: object + properties: + name: + type: string + Breed2: + type: object + properties: + name: + type: number + + Cat: + type: object + properties: + name: + type: string + Fox: + type: object + properties: + name: + type: string + \ No newline at end of file diff --git a/data/x-of-titles/response-enum-one-of-inline-2.yaml b/data/x-of-titles/response-enum-one-of-inline-2.yaml new file mode 100644 index 00000000..52521917 --- /dev/null +++ b/data/x-of-titles/response-enum-one-of-inline-2.yaml @@ -0,0 +1,83 @@ +openapi: 3.0.1 +info: + title: Test API + version: "2.0" +tags: +- name: Tests + description: Test tag. +paths: + /api/v2/changeOfResponseFieldValueTiedToEnumTest: + get: + tags: + - Tests + summary: This is a test + description: Test description. + operationId: getTest + requestBody: + description: Test. + content: + application/json: + schema: + $ref: '#/components/schemas/GroupOfRequestObjects' + required: true + responses: + "200": + description: OK + security: + - DigestAuth: [] +components: + schemas: + GroupOfRequestObjects: + type: object + description: Enum values + oneOf: + - $ref: "#/components/schemas/ResponseEnumInline" + - $ref: "#/components/schemas/ResponseEnumInline2" + ResponseEnumInline: + type: object + description: Enum values + properties: + eventTypeName: + description: Incident that triggered this alert. + type: object + oneOf: + - title: Billing Event Types + type: string + enum: + - CREDIT_CARD_ABOUT_TO_EXPIRE + - title: Cps Backup Event Types + type: string + enum: + - CPS_SNAPSHOT_STARTED + - CPS_SNAPSHOT_SUCCESSFUL + - CPS_SNAPSHOT_FAILED + - CPS_SNAPSHOT_FALLBACK_SUCCESSFUL + - CPS_SNAPSHOT_FALLBACK_FAILED + - CPS_RESTORE_SUCCESSFUL + - CPS_NEW_EVENT_1 + - CPS_NEW_EVENT_2 + - CPS_EXPORT_SUCCESSFUL + - CPS_RESTORE_FAILED + - CPS_EXPORT_FAILED + - CPS_SNAPSHOT_DOWNLOAD_REQUEST_FAILED + - CPS_OPLOG_CAUGHT_UP + - title: New Events + type: string + enum: + - NEW_EVENT_1 + - NEW_EVENT_2 + ResponseEnumInline2: + type: object + description: Enum values + properties: + eventTypeName2: + description: Incident that triggered this alert. + type: object + oneOf: + - enum: + - CREDIT_CARD_ABOUT_TO_EXPIRE + title: Billing Event Types + type: string + DigestAuth: + type: http + scheme: digest diff --git a/data/x-of-titles/response-enum-one-of-inline.yaml b/data/x-of-titles/response-enum-one-of-inline.yaml new file mode 100644 index 00000000..b4982997 --- /dev/null +++ b/data/x-of-titles/response-enum-one-of-inline.yaml @@ -0,0 +1,76 @@ +openapi: 3.0.1 +info: + title: Test API + version: "2.0" +tags: +- name: Tests + description: Test tag. +paths: + /api/v2/changeOfResponseFieldValueTiedToEnumTest: + get: + tags: + - Tests + summary: This is a test + description: Test description. + operationId: getTest + requestBody: + description: Test. + content: + application/json: + schema: + $ref: '#/components/schemas/GroupOfRequestObjects' + required: true + responses: + "200": + description: OK + security: + - DigestAuth: [] +components: + schemas: + GroupOfRequestObjects: + type: object + description: Enum values + oneOf: + - $ref: "#/components/schemas/ResponseEnumInline" + - $ref: "#/components/schemas/ResponseEnumInline2" + ResponseEnumInline: + type: object + description: Enum values + properties: + eventTypeName: + description: Incident that triggered this alert. + type: object + oneOf: + - enum: + - CREDIT_CARD_ABOUT_TO_EXPIRE + title: Billing Event Types + type: string + - enum: + - CPS_SNAPSHOT_STARTED + - CPS_SNAPSHOT_SUCCESSFUL + - CPS_SNAPSHOT_FAILED + - CPS_SNAPSHOT_FALLBACK_SUCCESSFUL + - CPS_SNAPSHOT_FALLBACK_FAILED + - CPS_RESTORE_SUCCESSFUL + - CPS_EXPORT_SUCCESSFUL + - CPS_RESTORE_FAILED + - CPS_EXPORT_FAILED + - CPS_SNAPSHOT_DOWNLOAD_REQUEST_FAILED + - CPS_OPLOG_CAUGHT_UP + title: Cps Backup Event Types + type: string + ResponseEnumInline2: + type: object + description: Enum values + properties: + eventTypeName2: + description: Incident that triggered this alert. + type: object + oneOf: + - enum: + - CREDIT_CARD_ABOUT_TO_EXPIRE + title: Billing Event Types + type: string + DigestAuth: + type: http + scheme: digest diff --git a/data/x-of-titles/spec1.yaml b/data/x-of-titles/spec1.yaml new file mode 100644 index 00000000..98e5e6a4 --- /dev/null +++ b/data/x-of-titles/spec1.yaml @@ -0,0 +1,18 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 1 + type: string + - title: Title 2 + type: string diff --git a/data/x-of-titles/spec2.yaml b/data/x-of-titles/spec2.yaml new file mode 100644 index 00000000..acc64406 --- /dev/null +++ b/data/x-of-titles/spec2.yaml @@ -0,0 +1,18 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 1 + type: string + - title: Title 3 + type: string diff --git a/data/x-of-titles/spec3.yaml b/data/x-of-titles/spec3.yaml new file mode 100644 index 00000000..4585854d --- /dev/null +++ b/data/x-of-titles/spec3.yaml @@ -0,0 +1,18 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 1 + type: boolean + - title: Title 2 + type: number diff --git a/data/x-of-titles/spec4.yaml b/data/x-of-titles/spec4.yaml new file mode 100644 index 00000000..03756a04 --- /dev/null +++ b/data/x-of-titles/spec4.yaml @@ -0,0 +1,20 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 1 + type: string + - title: Title 2 + type: number + - title: Title 3 + type: boolean diff --git a/data/x-of-titles/spec5.yaml b/data/x-of-titles/spec5.yaml new file mode 100644 index 00000000..2b13866c --- /dev/null +++ b/data/x-of-titles/spec5.yaml @@ -0,0 +1,20 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 2 + type: string + - title: Title 2 + type: number + - title: Title 3 + type: boolean diff --git a/data/x-of-titles/spec6.yaml b/data/x-of-titles/spec6.yaml new file mode 100644 index 00000000..6a63375b --- /dev/null +++ b/data/x-of-titles/spec6.yaml @@ -0,0 +1,22 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 2 + type: string + - title: Title 2 + type: number + - title: Title 3 + type: boolean + - title: + type: string diff --git a/data/x-of-titles/spec7.yaml b/data/x-of-titles/spec7.yaml new file mode 100644 index 00000000..af96b719 --- /dev/null +++ b/data/x-of-titles/spec7.yaml @@ -0,0 +1,22 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - title: Title 2 + type: string + - title: Title 2 + type: number + - title: Title 3 + type: boolean + - title: + type: number diff --git a/diff/diff_x_of_test.go b/diff/diff_x_of_test.go index fc22e092..f0a4d924 100644 --- a/diff/diff_x_of_test.go +++ b/diff/diff_x_of_test.go @@ -23,9 +23,19 @@ func TestAllOf_SingleRef(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("single-ref-revision.yaml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) - require.NoError(t, err) - require.Equal(t, utils.StringList{"sku"}, dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AllOfDiff.Modified["#/components/schemas/ProductDto"].PropertiesDiff.Added) + dd, err := diff.Get(diff.NewConfig(), s1, s2) + require.NoError(t, err) + allOfDiff := dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AllOfDiff + require.Len(t, allOfDiff.Modified, 1) + require.Equal(t, diff.Subschema{ + Index: 0, + Component: "ProductDto", + }, allOfDiff.Modified[0].Base) + require.Equal(t, diff.Subschema{ + Index: 0, + Component: "ProductDto", + }, allOfDiff.Modified[0].Revision) + require.Equal(t, utils.StringList{"sku"}, allOfDiff.Modified[0].Diff.PropertiesDiff.Added) } func TestOneOf_TwoRefs(t *testing.T) { @@ -37,9 +47,20 @@ func TestOneOf_TwoRefs(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("two-refs-revision.yaml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) - require.NoError(t, err) - require.Equal(t, utils.StringList{"guard"}, dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["PATCH"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#/components/schemas/Dog"].AllOfDiff.Modified["#2"].PropertiesDiff.Added) + dd, err := diff.Get(diff.NewConfig(), s1, s2) + require.NoError(t, err) + oneOfDiff := dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["PATCH"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff + require.Len(t, oneOfDiff.Modified, 1) + require.Equal(t, diff.Subschema{ + Index: 1, + Component: "Dog", + }, oneOfDiff.Modified[0].Base) + require.Equal(t, diff.Subschema{ + Index: 1, + Component: "Dog", + }, oneOfDiff.Modified[0].Revision) + require.Equal(t, 1, oneOfDiff.Modified[0].Diff.AllOfDiff.Modified[0].Base.Index) + require.Equal(t, utils.StringList{"guard"}, oneOfDiff.Modified[0].Diff.AllOfDiff.Modified[0].Diff.PropertiesDiff.Added) } func TestOneOf_ChangeBoth(t *testing.T) { @@ -51,10 +72,32 @@ func TestOneOf_ChangeBoth(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("two-refs-both-changed-revision.yaml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) - require.NoError(t, err) - require.Equal(t, utils.StringList{"miao"}, dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["PATCH"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#/components/schemas/Cat"].AllOfDiff.Modified["#2"].PropertiesDiff.Added) - require.Equal(t, utils.StringList{"guard"}, dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["PATCH"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#/components/schemas/Dog"].AllOfDiff.Modified["#2"].PropertiesDiff.Added) + dd, err := diff.Get(diff.NewConfig(), s1, s2) + require.NoError(t, err) + oneOfDiff := dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["PATCH"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff + require.Len(t, oneOfDiff.Modified, 2) + + require.Equal(t, diff.Subschema{ + Index: 0, + Component: "Cat", + }, oneOfDiff.Modified[0].Base) + require.Equal(t, diff.Subschema{ + Index: 0, + Component: "Cat", + }, oneOfDiff.Modified[0].Revision) + require.Equal(t, 1, oneOfDiff.Modified[0].Diff.AllOfDiff.Modified[0].Base.Index) + require.Equal(t, utils.StringList{"miao"}, oneOfDiff.Modified[0].Diff.AllOfDiff.Modified[0].Diff.PropertiesDiff.Added) + + require.Equal(t, diff.Subschema{ + Index: 1, + Component: "Dog", + }, oneOfDiff.Modified[1].Base) + require.Equal(t, diff.Subschema{ + Index: 1, + Component: "Dog", + }, oneOfDiff.Modified[1].Revision) + require.Equal(t, 1, oneOfDiff.Modified[1].Diff.AllOfDiff.Modified[0].Base.Index) + require.Equal(t, utils.StringList{"guard"}, oneOfDiff.Modified[1].Diff.AllOfDiff.Modified[0].Diff.PropertiesDiff.Added) } func TestOneOf_TwoInlineDuplicate(t *testing.T) { @@ -66,10 +109,14 @@ func TestOneOf_TwoInlineDuplicate(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("two-inline-revision-duplicate.yaml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) + dd, err := diff.Get(diff.NewConfig(), s1, s2) require.NoError(t, err) - require.Equal(t, "name2", dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#1"].PropertiesDiff.Added[0]) - require.Equal(t, "name1", dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#1"].PropertiesDiff.Deleted[0]) + oneOfDiff := dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff + require.Len(t, oneOfDiff.Modified, 1) + require.Equal(t, 0, oneOfDiff.Modified[0].Base.Index) + require.Equal(t, 1, oneOfDiff.Modified[0].Revision.Index) + require.Equal(t, "name2", oneOfDiff.Modified[0].Diff.PropertiesDiff.Added[0]) + require.Equal(t, "name1", oneOfDiff.Modified[0].Diff.PropertiesDiff.Deleted[0]) } func TestOneOf_TwoInlineOneModified(t *testing.T) { @@ -81,10 +128,14 @@ func TestOneOf_TwoInlineOneModified(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("two-inline-revision-one-modified.yaml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) + dd, err := diff.Get(diff.NewConfig(), s1, s2) require.NoError(t, err) - require.Equal(t, "name4", dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#1"].PropertiesDiff.Added[0]) - require.Equal(t, "name1", dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#1"].PropertiesDiff.Deleted[0]) + oneOfDiff := dd.PathsDiff.Modified["/api"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff + require.Len(t, oneOfDiff.Modified, 1) + require.Equal(t, 0, oneOfDiff.Modified[0].Base.Index) + require.Equal(t, 1, oneOfDiff.Modified[0].Revision.Index) + require.Equal(t, "name4", oneOfDiff.Modified[0].Diff.PropertiesDiff.Added[0]) + require.Equal(t, "name1", oneOfDiff.Modified[0].Diff.PropertiesDiff.Deleted[0]) } func TestOneOf_MultiRefs(t *testing.T) { @@ -96,10 +147,20 @@ func TestOneOf_MultiRefs(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("multi-refs-revision.yaml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) - require.NoError(t, err) - require.Equal(t, "bark", dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["GET"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#/components/schemas/Dog"].PropertiesDiff.Added[0]) - require.Equal(t, "name", dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["GET"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff.Modified["#/components/schemas/Dog"].PropertiesDiff.Deleted[0]) + dd, err := diff.Get(diff.NewConfig(), s1, s2) + require.NoError(t, err) + oneOfDiff := dd.PathsDiff.Modified["/pets"].OperationsDiff.Modified["GET"].RequestBodyDiff.ContentDiff.MediaTypeModified["application/json"].SchemaDiff.OneOfDiff + require.Len(t, oneOfDiff.Modified, 1) + require.Equal(t, diff.Subschema{ + Index: 2, + Component: "Dog", + }, oneOfDiff.Modified[0].Base) + require.Equal(t, diff.Subschema{ + Index: 1, + Component: "Dog", + }, oneOfDiff.Modified[0].Revision) + require.Equal(t, "bark", oneOfDiff.Modified[0].Diff.PropertiesDiff.Added[0]) + require.Equal(t, "name", oneOfDiff.Modified[0].Diff.PropertiesDiff.Deleted[0]) } func TestAnyOf_IncludeDescriptions(t *testing.T) { @@ -111,11 +172,22 @@ func TestAnyOf_IncludeDescriptions(t *testing.T) { s2, err := loader.LoadFromFile(getXOfFile("anyof-rev-openapi.yml")) require.NoError(t, err) - dd, err := diff.Get(&diff.Config{}, s1, s2) + dd, err := diff.Get(diff.NewConfig(), s1, s2) require.NoError(t, err) anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff - require.ElementsMatch(t, []string{"RevisionSchema[0]", "RevisionSchema[2]"}, anyOfDiff.Added) - require.ElementsMatch(t, []string{"BaseSchema[0]"}, anyOfDiff.Deleted) + require.ElementsMatch(t, diff.Subschemas{ + { + Index: 0, + }, + { + Index: 2, + }, + }, anyOfDiff.Added) + require.ElementsMatch(t, diff.Subschemas{ + { + Index: 0, + }, + }, anyOfDiff.Deleted) require.Empty(t, anyOfDiff.Modified) } @@ -131,7 +203,11 @@ func TestAnyOf_ExcludeDescriptions(t *testing.T) { dd, err := diff.Get(diff.NewConfig().WithExcludeElements([]string{diff.ExcludeDescriptionOption}), s1, s2) require.NoError(t, err) anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff - require.ElementsMatch(t, []string{"RevisionSchema[2]"}, anyOfDiff.Added) - require.ElementsMatch(t, []string{}, anyOfDiff.Deleted) + require.ElementsMatch(t, diff.Subschemas{ + { + Index: 2, + }, + }, anyOfDiff.Added) + require.Empty(t, anyOfDiff.Deleted) require.Empty(t, anyOfDiff.Modified) } diff --git a/diff/diff_x_of_titles_test.go b/diff/diff_x_of_titles_test.go new file mode 100644 index 00000000..30156401 --- /dev/null +++ b/diff/diff_x_of_titles_test.go @@ -0,0 +1,223 @@ +package diff_test + +import ( + "fmt" + "testing" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/stretchr/testify/require" + "github.com/tufin/oasdiff/diff" +) + +func getXOfTitlesFile(file string) string { + return fmt.Sprintf("../data/x-of-titles/%s", file) +} + +func TestXOfTitles_Identical(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfTitlesFile("spec1.yaml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfTitlesFile("spec1.yaml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + require.Empty(t, dd) +} + +func TestXOfTitles_TitleNameChanged(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfTitlesFile("spec1.yaml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfTitlesFile("spec2.yaml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + require.NotEmpty(t, dd) + anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff + require.ElementsMatch(t, + diff.Subschemas{ + diff.Subschema{ + Index: 1, + Title: "Title 3", + }, + }, + anyOfDiff.Added) + require.ElementsMatch(t, + diff.Subschemas{ + diff.Subschema{ + Index: 1, + Title: "Title 2", + }, + }, + anyOfDiff.Deleted) + require.Empty(t, anyOfDiff.Modified) +} + +func TestXOfTitles_TitlesModified(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfTitlesFile("spec1.yaml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfTitlesFile("spec3.yaml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + require.NotEmpty(t, dd) + anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff + require.Empty(t, anyOfDiff.Added) + require.Empty(t, anyOfDiff.Deleted) + + require.Len(t, anyOfDiff.Modified, 2) + + require.Equal(t, + diff.Subschema{ + Index: 0, + Title: "Title 1", + }, + anyOfDiff.Modified[0].Base, + ) + require.Equal(t, + diff.Subschema{ + Index: 0, + Title: "Title 1", + }, + anyOfDiff.Modified[0].Revision, + ) + require.Equal(t, diff.ValueDiff{From: "string", To: "boolean"}, *anyOfDiff.Modified[0].Diff.TypeDiff) + + require.Equal(t, + diff.Subschema{ + Index: 1, + Title: "Title 2", + }, + anyOfDiff.Modified[1].Base, + ) + require.Equal(t, + diff.Subschema{ + Index: 1, + Title: "Title 2", + }, + anyOfDiff.Modified[1].Revision, + ) + require.Equal(t, diff.ValueDiff{From: "string", To: "number"}, *anyOfDiff.Modified[1].Diff.TypeDiff) +} + +func TestXOfTitles_TitlesModifiedAndAdded(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfTitlesFile("spec3.yaml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfTitlesFile("spec4.yaml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + require.NotEmpty(t, dd) + anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff + require.Len(t, anyOfDiff.Added, 1) + require.ElementsMatch(t, diff.Subschemas{ + { + Index: 2, + Title: "Title 3", + }, + }, anyOfDiff.Added) + require.Empty(t, anyOfDiff.Deleted) + require.Len(t, anyOfDiff.Modified, 1) + require.Equal(t, + diff.Subschema{ + Index: 0, + Title: "Title 1", + }, + anyOfDiff.Modified[0].Base, + ) + require.Equal(t, + diff.Subschema{ + Index: 0, + Title: "Title 1", + }, + anyOfDiff.Modified[0].Revision, + ) + require.Equal(t, + diff.ValueDiff{From: "boolean", To: "string"}, + *anyOfDiff.Modified[0].Diff.TypeDiff, + ) +} + +func TestXOfTitles_DuplicateTitles(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfTitlesFile("spec3.yaml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfTitlesFile("spec5.yaml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + require.NotEmpty(t, dd) + anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff + require.Len(t, anyOfDiff.Added, 2) + require.ElementsMatch(t, diff.Subschemas{ + { + Index: 0, + Title: "Title 2", + }, + { + Index: 2, + Title: "Title 3", + }, + }, anyOfDiff.Added) + + require.Len(t, anyOfDiff.Deleted, 1) + require.ElementsMatch(t, diff.Subschemas{ + { + Index: 0, + Title: "Title 1", + }, + }, anyOfDiff.Deleted) + + require.Empty(t, anyOfDiff.Modified, 0) +} + +func TestXOfTitles_EmptyTitle(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfTitlesFile("spec6.yaml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfTitlesFile("spec7.yaml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + require.NotEmpty(t, dd) + anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff + require.Empty(t, anyOfDiff.Added) + require.Empty(t, anyOfDiff.Deleted) + require.Len(t, anyOfDiff.Modified, 1) + require.Equal(t, + diff.Subschema{ + Index: 3, + }, + anyOfDiff.Modified[0].Base, + ) + require.Equal(t, + diff.Subschema{ + Index: 3, + }, + anyOfDiff.Modified[0].Revision, + ) + require.Equal(t, + diff.ValueDiff{From: "string", To: "number"}, + *anyOfDiff.Modified[0].Diff.TypeDiff, + ) +} diff --git a/diff/modified_schemas.go b/diff/modified_schemas.go deleted file mode 100644 index ba076131..00000000 --- a/diff/modified_schemas.go +++ /dev/null @@ -1,33 +0,0 @@ -package diff - -import ( - "github.com/getkin/kin-openapi/openapi3" -) - -// ModifiedSchemas is map of schema names to their respective diffs -type ModifiedSchemas map[string]*SchemaDiff - -func (modifiedSchemas ModifiedSchemas) addSchemaDiff(config *Config, state *state, schemaName string, schemaRef1, schemaRef2 *openapi3.SchemaRef) error { - - diff, err := getSchemaDiff(config, state, schemaRef1, schemaRef2) - if err != nil { - return err - } - if !diff.Empty() { - modifiedSchemas[schemaName] = diff - } - - return nil -} - -func (modifiedSchemas ModifiedSchemas) combine(other ModifiedSchemas) ModifiedSchemas { - result := ModifiedSchemas{} - - for ref, d := range modifiedSchemas { - result[ref] = d - } - for ref, d := range other { - result[ref] = d - } - return result -} diff --git a/diff/modified_subschemas.go b/diff/modified_subschemas.go new file mode 100644 index 00000000..55ebdfbe --- /dev/null +++ b/diff/modified_subschemas.go @@ -0,0 +1,104 @@ +package diff + +import ( + "fmt" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/tufin/oasdiff/utils" +) + +// ModifiedSubschemas is list of modified subschemas with their diffs +// Unlike other Modiefied types which are modeled as maps, this one is modeled as a slice to avoid complex mapping keys +type ModifiedSubschemas []*ModifiedSubschema + +// ModifiedSubschema represents a modified subschema with its indentifiers in base and revision, and the schema diff +type ModifiedSubschema struct { + Base Subschema `json:"base" yaml:"base"` + Revision Subschema `json:"revision" yaml:"revision"` + Diff *SchemaDiff `json:"diff" yaml:"diff"` +} + +// String returns a string representation of the modified subschema +func (modifiedSchema *ModifiedSubschema) String() string { + baseName := modifiedSchema.Base.String() + revisonName := modifiedSchema.Revision.String() + + if baseName == revisonName { + return baseName + } + + return fmt.Sprintf("%s -> %s", baseName, revisonName) +} + +// Subschemas is a list of subschemas +type Subschemas []Subschema + +// Subschema uniquely identifies a subschema by its index, component and title +type Subschema struct { + Index int `json:"index" yaml:"index"` // zero-based index in the schema's subschemas + Component string `json:"component,omitempty" yaml:"component,omitempty"` // component name if the subschema is a reference to components/schemas + Title string `json:"title,omitempty" yaml:"title,omitempty"` // title of the subschema +} + +// String returns a string representation of the subschema +// Note that we convert the index to 1-based index +func (subschema Subschema) String() string { + const prefix = "subschema" + + if subschema.Title != "" { + return fmt.Sprintf("%s #%d: %s", prefix, subschema.Index+1, subschema.Title) + } + + // note: we may want to ad the index to the component name in the future + if subschema.Component != "" { + return fmt.Sprintf("#/components/schemas/%s", subschema.Component) + } + + return fmt.Sprintf("%s #%d", prefix, subschema.Index+1) +} + +func getSubschemas(indexes []int, schemaRefs openapi3.SchemaRefs) Subschemas { + result := Subschemas{} + for _, index := range indexes { + result = append(result, Subschema{ + Index: index, + Title: schemaRefs[index].Value.Title, + }) + } + return result +} + +// String returns a string representation of the subschemas +func (schemas Subschemas) String() string { + names := make([]string, len(schemas)) + for i, schema := range schemas { + names[i] = schema.String() + } + list := utils.StringList(names) + return list.String() +} + +func (modifiedSchemas ModifiedSubschemas) addSchemaDiff(config *Config, state *state, schemaRef1, schemaRef2 *openapi3.SchemaRef, index1, index2 int) (ModifiedSubschemas, error) { + + diff, err := getSchemaDiff(config, state, schemaRef1, schemaRef2) + if err != nil { + return nil, err + } + if !diff.Empty() { + modifiedSchemas = append(modifiedSchemas, &ModifiedSubschema{ + Base: Subschema{ + Index: index1, + Component: getComponentName(schemaRef1), + Title: schemaRef1.Value.Title, + }, + Revision: Subschema{ + Index: index2, + Component: getComponentName(schemaRef2), + Title: schemaRef2.Value.Title, + }, + Diff: diff, + }) + } + + return modifiedSchemas, nil +} diff --git a/diff/modified_subschemas_test.go b/diff/modified_subschemas_test.go new file mode 100644 index 00000000..e6c1be8b --- /dev/null +++ b/diff/modified_subschemas_test.go @@ -0,0 +1,90 @@ +package diff_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + "github.com/tufin/oasdiff/diff" +) + +func TestModifiedSubschemaString_TitlesSame(t *testing.T) { + + ms := diff.ModifiedSubschema{ + Base: diff.Subschema{ + Title: "Title 1", + }, + Revision: diff.Subschema{ + Title: "Title 1", + }, + } + + require.Equal(t, "subschema #1: Title 1", ms.String()) +} + +func TestModifiedSubschemaString_TitlesDiff(t *testing.T) { + + ms := diff.ModifiedSubschema{ + Base: diff.Subschema{ + Title: "Title 1", + }, + Revision: diff.Subschema{ + Title: "Title 2", + }, + } + + require.Equal(t, "subschema #1: Title 1 -> subschema #1: Title 2", ms.String()) +} + +func TestModifiedSubschemaString_ComponentsSame(t *testing.T) { + + ms := diff.ModifiedSubschema{ + Base: diff.Subschema{ + Component: "component1", + }, + Revision: diff.Subschema{ + Component: "component1", + }, + } + + require.Equal(t, "#/components/schemas/component1", ms.String()) +} + +func TestModifiedSubschemaString_ComponentsDiff(t *testing.T) { + + ms := diff.ModifiedSubschema{ + Base: diff.Subschema{ + Component: "component1", + }, + Revision: diff.Subschema{ + Component: "component2", + }, + } + + require.Equal(t, "#/components/schemas/component1 -> #/components/schemas/component2", ms.String()) +} + +func TestSubschemaString(t *testing.T) { + ms := diff.Subschemas{ + diff.Subschema{ + Index: 0, + Title: "Title 0", + }, + diff.Subschema{ + Index: 1, + Title: "Title 1", + }, + } + require.Equal(t, "subschema #1: Title 0, subschema #2: Title 1", ms.String()) +} + +func TestSubschemaString_Empty(t *testing.T) { + ms := diff.Subschemas{ + diff.Subschema{ + Index: 0, + }, + diff.Subschema{ + Index: 1, + }, + } + require.Equal(t, "subschema #1, subschema #2", ms.String()) +} diff --git a/diff/schema_diff.go b/diff/schema_diff.go index b0eb31f9..511ebd28 100644 --- a/diff/schema_diff.go +++ b/diff/schema_diff.go @@ -12,9 +12,9 @@ type SchemaDiff struct { SchemaDeleted bool `json:"schemaDeleted,omitempty" yaml:"schemaDeleted,omitempty"` CircularRefDiff bool `json:"circularRef,omitempty" yaml:"circularRef,omitempty"` ExtensionsDiff *ExtensionsDiff `json:"extensions,omitempty" yaml:"extensions,omitempty"` - OneOfDiff *SchemaListDiff `json:"oneOf,omitempty" yaml:"oneOf,omitempty"` - AnyOfDiff *SchemaListDiff `json:"anyOf,omitempty" yaml:"anyOf,omitempty"` - AllOfDiff *SchemaListDiff `json:"allOf,omitempty" yaml:"allOf,omitempty"` + OneOfDiff *SubschemasDiff `json:"oneOf,omitempty" yaml:"oneOf,omitempty"` + AnyOfDiff *SubschemasDiff `json:"anyOf,omitempty" yaml:"anyOf,omitempty"` + AllOfDiff *SubschemasDiff `json:"allOf,omitempty" yaml:"allOf,omitempty"` NotDiff *SchemaDiff `json:"not,omitempty" yaml:"not,omitempty"` TypeDiff *ValueDiff `json:"type,omitempty" yaml:"type,omitempty"` TitleDiff *ValueDiff `json:"title,omitempty" yaml:"title,omitempty"` @@ -129,15 +129,15 @@ func getSchemaDiffInternal(config *Config, state *state, schema1, schema2 *opena if err != nil { return nil, err } - result.OneOfDiff, err = getSchemaListsDiff(config, state, value1.OneOf, value2.OneOf) + result.OneOfDiff, err = getSubschemasDiff(config, state, value1.OneOf, value2.OneOf) if err != nil { return nil, err } - result.AnyOfDiff, err = getSchemaListsDiff(config, state, value1.AnyOf, value2.AnyOf) + result.AnyOfDiff, err = getSubschemasDiff(config, state, value1.AnyOf, value2.AnyOf) if err != nil { return nil, err } - result.AllOfDiff, err = getSchemaListsDiff(config, state, value1.AllOf, value2.AllOf) + result.AllOfDiff, err = getSubschemasDiff(config, state, value1.AllOf, value2.AllOf) if err != nil { return nil, err } diff --git a/diff/schema_list_diff.go b/diff/schema_list_diff.go deleted file mode 100644 index 7c961c4a..00000000 --- a/diff/schema_list_diff.go +++ /dev/null @@ -1,217 +0,0 @@ -package diff - -import ( - "fmt" - "strings" - - "github.com/getkin/kin-openapi/openapi3" - "github.com/tufin/oasdiff/utils" -) - -/* -SchemaListDiff describes the changes between a pair of lists of schema objects: https://swagger.io/specification/#schema-object -The result is a combination of two diffs: -1. Diff of schemas with a $ref: added/deleted schema names; modified=diff of schemas with the same $ref -2. Diff of schemas without a $ref (inline schemas): added/deleted schemas (base/revision + index in the list of schemas); modified=only if exactly one schema was added and one deleted, the Modified field will show a diff between them -*/ -type SchemaListDiff struct { - Added utils.StringList `json:"added,omitempty" yaml:"added,omitempty"` - Deleted utils.StringList `json:"deleted,omitempty" yaml:"deleted,omitempty"` - Modified ModifiedSchemas `json:"modified,omitempty" yaml:"modified,omitempty"` -} - -// Empty indicates whether a change was found in this element -func (diff *SchemaListDiff) Empty() bool { - if diff == nil { - return true - } - - return len(diff.Added) == 0 && - len(diff.Deleted) == 0 && - len(diff.Modified) == 0 -} - -func getSchemaListsDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (*SchemaListDiff, error) { - diff, err := getSchemaListsDiffInternal(config, state, schemaRefs1, schemaRefs2) - if err != nil { - return nil, err - } - - if diff.Empty() { - return nil, nil - } - - return diff, nil -} - -func (diff SchemaListDiff) combine(other SchemaListDiff) (*SchemaListDiff, error) { - - return &SchemaListDiff{ - Added: append(diff.Added, other.Added...), - Deleted: append(diff.Deleted, other.Deleted...), - Modified: diff.Modified.combine(other.Modified), - }, nil -} - -func getSchemaListsDiffInternal(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (*SchemaListDiff, error) { - - if len(schemaRefs1) == 0 && len(schemaRefs2) == 0 { - return nil, nil - } - - diffRefs, err := getSchemaListsRefsDiff(config, state, schemaRefs1, schemaRefs2, isSchemaRef) - if err != nil { - return nil, err - } - - diffInline, err := getSchemaListsInlineDiff(config, state, schemaRefs1, schemaRefs2, isSchemaInline) - if err != nil { - return nil, err - } - - return diffRefs.combine(diffInline) -} - -type schemaRefsFilter func(schemaRef *openapi3.SchemaRef) bool - -// getSchemaListsRefsDiff compares schemas by $ref name -func getSchemaListsRefsDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter schemaRefsFilter) (SchemaListDiff, error) { - added := utils.StringList{} - deleted := utils.StringList{} - modified := ModifiedSchemas{} - - schemaMap2 := toSchemaRefMap(schemaRefs2, filter) - for _, schema1 := range schemaRefs1 { - if !filter(schema1) { - continue - } - ref := schema1.Ref - if schema2, found := schemaMap2[ref]; found { - schemaMap2.delete(ref) - if err := modified.addSchemaDiff(config, state, ref, schema1, schema2.schemaRef); err != nil { - return SchemaListDiff{}, err - } - } else { - schemaName := ref[strings.LastIndex(ref, "/")+1:] - deleted = append(deleted, schemaName) - } - } - - schemaMap1 := toSchemaRefMap(schemaRefs1, filter) - for _, schema2 := range schemaRefs2 { - if !filter(schema2) { - continue - } - ref := schema2.Ref - if _, found := schemaMap1[ref]; found { - schemaMap1.delete(ref) - } else { - schemaName := ref[strings.LastIndex(ref, "/")+1:] - added = append(added, schemaName) - } - } - return SchemaListDiff{ - Added: added, - Deleted: deleted, - Modified: modified, - }, nil -} - -// getSchemaListsRefsDiff compares schemas by their syntax -func getSchemaListsInlineDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter schemaRefsFilter) (SchemaListDiff, error) { - - addedIdx, addedSchemas, err := getGroupDiffForInlineSchemas(config, state, schemaRefs2, schemaRefs1, filter, "RevisionSchema") - if err != nil { - return SchemaListDiff{}, err - } - - deletedIdx, deletedSchemas, err := getGroupDiffForInlineSchemas(config, state, schemaRefs1, schemaRefs2, filter, "BaseSchema") - if err != nil { - return SchemaListDiff{}, err - } - - if len(addedIdx) == 1 && len(deletedIdx) == 1 { - d, err := getSchemaDiff(config, state, schemaRefs1[deletedIdx[0]], schemaRefs2[addedIdx[0]]) - if err != nil { - return SchemaListDiff{}, err - } - - if d.Empty() { - return SchemaListDiff{}, err - } - - return SchemaListDiff{ - Modified: ModifiedSchemas{fmt.Sprintf("#%d", 1+deletedIdx[0]): d}, - }, nil - } - - return SchemaListDiff{ - Added: addedSchemas, - Deleted: deletedSchemas, - }, nil -} - -func getGroupDiffForInlineSchemas(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter schemaRefsFilter, inlineSchemaPrefix string) ([]int, []string, error) { - - notContainedIdx := []int{} - notContainedSchemas := []string{} - matched := map[int]struct{}{} - - for index1, schemaRef1 := range schemaRefs1 { - if !filter(schemaRef1) { - continue - } - - if found, index2, err := findIndenticalSchema(config, state, schemaRef1, schemaRefs2, matched, filter); err != nil { - return nil, nil, err - } else if !found { - notContainedIdx = append(notContainedIdx, index1) - schemaName := fmt.Sprintf("%s[%d]", inlineSchemaPrefix, index1) - if schemaRef1 != nil && schemaRef1.Value != nil && schemaRef1.Value.Title != "" { - schemaName = fmt.Sprintf("%s:%s", schemaName, schemaRef1.Value.Title) - } - notContainedSchemas = append(notContainedSchemas, schemaName) - } else { - matched[index2] = struct{}{} - } - } - return notContainedIdx, notContainedSchemas, nil -} - -func findIndenticalSchema(config *Config, state *state, schemaRef1 *openapi3.SchemaRef, schemasRefs2 openapi3.SchemaRefs, matched map[int]struct{}, filter schemaRefsFilter) (bool, int, error) { - for index2, schemaRef2 := range schemasRefs2 { - if !filter(schemaRef1) { - continue - } - if alreadyMatched(index2, matched) { - continue - } - - if schemaDiff, err := getSchemaDiff(config, state, schemaRef1, schemaRef2); err != nil { - return false, 0, err - } else if schemaDiff.Empty() { - return true, index2, nil - } - } - - return false, 0, nil -} - -func alreadyMatched(index int, matched map[int]struct{}) bool { - _, found := matched[index] - return found -} - -func isSchemaInline(schemaRef *openapi3.SchemaRef) bool { - if schemaRef == nil { - return false - } - return schemaRef.Ref == "" -} - -func isSchemaRef(schemaRef *openapi3.SchemaRef) bool { - if schemaRef == nil { - return false - } - return schemaRef.Ref != "" -} diff --git a/diff/schema_ref_map.go b/diff/schema_ref_map.go index f54426f7..8000bd84 100644 --- a/diff/schema_ref_map.go +++ b/diff/schema_ref_map.go @@ -4,38 +4,44 @@ import ( "github.com/getkin/kin-openapi/openapi3" ) -type refWithCount struct { +type ref struct { schemaRef *openapi3.SchemaRef - count int + indices []int } -type schemaRefMap map[string]*refWithCount +// refMap is used to match referenced subschemas across base and revision +type refMap map[string]*ref -func (schemaRefMap schemaRefMap) add(schemaRef *openapi3.SchemaRef) { - if val, found := schemaRefMap[schemaRef.Ref]; found { - val.count++ +// push adds a schema reference to the map or, if exists, adds an index to the existing reference +func (m refMap) push(schemaRef *openapi3.SchemaRef, index int) { + if val, found := m[schemaRef.Ref]; found { + val.indices = append(val.indices, index) } else { - schemaRefMap[schemaRef.Ref] = &refWithCount{ + m[schemaRef.Ref] = &ref{ schemaRef: schemaRef, - count: 1, + indices: []int{index}, } } } -func (schemaRefMap schemaRefMap) delete(ref string) { - if val, found := schemaRefMap[ref]; found { - val.count-- - if val.count == 0 { - delete(schemaRefMap, ref) +// pop checks if the reference exists in the map, returns the schema reference and an index if exists, and removes the index from the map +func (m refMap) pop(ref string) (*openapi3.SchemaRef, int, bool) { + if val, found := m[ref]; found { + if len(val.indices) > 0 { + index := val.indices[0] + val.indices = val.indices[1:] + return val.schemaRef, index, true } + delete(m, ref) } + return nil, 0, false } -func toSchemaRefMap(schemaRefs openapi3.SchemaRefs, filter schemaRefsFilter) schemaRefMap { - result := schemaRefMap{} - for _, schemaRef := range schemaRefs { +func toRefMap(schemaRefs openapi3.SchemaRefs, filter schemaRefsFilter) refMap { + result := refMap{} + for index, schemaRef := range schemaRefs { if filter(schemaRef) { - result.add(schemaRef) + result.push(schemaRef, index) } } return result diff --git a/diff/schemas_diff.go b/diff/schemas_diff.go index 3750657b..68807d21 100644 --- a/diff/schemas_diff.go +++ b/diff/schemas_diff.go @@ -5,13 +5,13 @@ import ( "github.com/tufin/oasdiff/utils" ) -// SchemasDiff describes the changes between a pair of sets of schema objects: https://swagger.io/specification/#schema-object +// SchemasDiff describes the changes between a pair of maps of schema objects like the components.schemas object type SchemasDiff struct { - Added utils.StringList `json:"added,omitempty" yaml:"added,omitempty"` - Deleted utils.StringList `json:"deleted,omitempty" yaml:"deleted,omitempty"` - Modified ModifiedSchemas `json:"modified,omitempty" yaml:"modified,omitempty"` - Base openapi3.Schemas `json:"-" yaml:"-"` - Revision openapi3.Schemas `json:"-" yaml:"-"` + Added utils.StringList `json:"added,omitempty" yaml:"added,omitempty"` + Deleted utils.StringList `json:"deleted,omitempty" yaml:"deleted,omitempty"` + Modified ModifiedSchemasMap `json:"modified,omitempty" yaml:"modified,omitempty"` + Base openapi3.Schemas `json:"-" yaml:"-"` + Revision openapi3.Schemas `json:"-" yaml:"-"` } // Empty indicates whether a change was found in this element @@ -29,7 +29,7 @@ func newSchemasDiff() *SchemasDiff { return &SchemasDiff{ Added: utils.StringList{}, Deleted: utils.StringList{}, - Modified: ModifiedSchemas{}, + Modified: ModifiedSchemasMap{}, } } @@ -128,3 +128,19 @@ func (schemasDiff *SchemasDiff) getSummary() *SummaryDetails { Modified: len(schemasDiff.Modified), } } + +// ModifiedSchemasMap is map of schema names to their respective diffs +type ModifiedSchemasMap map[string]*SchemaDiff + +func (modifiedSchemas ModifiedSchemasMap) addSchemaDiff(config *Config, state *state, schemaName string, schemaRef1, schemaRef2 *openapi3.SchemaRef) error { + + diff, err := getSchemaDiff(config, state, schemaRef1, schemaRef2) + if err != nil { + return err + } + if !diff.Empty() { + modifiedSchemas[schemaName] = diff + } + + return nil +} diff --git a/diff/subschemas_diff.go b/diff/subschemas_diff.go new file mode 100644 index 00000000..ee2d0862 --- /dev/null +++ b/diff/subschemas_diff.go @@ -0,0 +1,323 @@ +package diff + +import ( + "strings" + + "github.com/getkin/kin-openapi/openapi3" + "github.com/tufin/oasdiff/utils" +) + +/* +SubschemasDiff describes the changes between a pair of subschemas under AllOf, AnyOf or OneOf +[oneOf, anyOf, allOf]: https://swagger.io/docs/specification/data-models/oneof-anyof-allof-not/ +[Schema Objects]: https://swagger.io/specification/#schema-object +SubschemasDiff is a combination of two diffs: + + 1. Diff of referenced schemas: subschemas under AllOf, AnyOf or OneOf defined as references to schemas under components/schemas + - schemas with the same $ref across base and revision are compared to each other and based on the result are considered as modified or unmodified + - other schemas are considered added/deleted + + 2. Diff of inline schemas: subschemas defined directly under AllOf, AnyOf or OneOf, without a reference to components/schemas + Unlike referenced schemas, inline schemas cannot be matched by a unique name and are therefor compared by their content. + - syntactically identical schemas across base and revision are considered unmodified + - schemas with the same title across base and revision are compared to each other and based on the result are considered as modified or unmodified + - other schemas are considered added/deleted + +Special case: +If there remains exactly one added schema and one deleted schema without a reference and without a title, they will be be compared to eachother and considered as modified or unmodified +*/ +type SubschemasDiff struct { + Added Subschemas `json:"added,omitempty" yaml:"added,omitempty"` + Deleted Subschemas `json:"deleted,omitempty" yaml:"deleted,omitempty"` + Modified ModifiedSubschemas `json:"modified,omitempty" yaml:"modified,omitempty"` +} + +// NewSubschemasDiff creates a new SubschemasDiff +func NewSubschemasDiff() *SubschemasDiff { + return &SubschemasDiff{ + Added: Subschemas{}, + Deleted: Subschemas{}, + Modified: ModifiedSubschemas{}, + } +} + +func (diff *SubschemasDiff) appendAdded(index int, schemaRef *openapi3.SchemaRef, title string) { + diff.Added = append(diff.Added, + Subschema{ + Index: index, + Component: getComponentName(schemaRef), + Title: title, + }, + ) +} + +func (diff *SubschemasDiff) appendDeleted(index int, schemaRef *openapi3.SchemaRef, title string) { + diff.Deleted = append(diff.Deleted, + Subschema{ + Index: index, + Component: getComponentName(schemaRef), + Title: title, + }, + ) +} + +func (diff *SubschemasDiff) appendModified(config *Config, state *state, schemaRef1, schemaRef2 *openapi3.SchemaRef, index1, index2 int) error { + var err error + diff.Modified, err = diff.Modified.addSchemaDiff(config, state, schemaRef1, schemaRef2, index1, index2) + if err != nil { + return err + } + return nil +} + +func getComponentName(schemaRef *openapi3.SchemaRef) string { + return schemaRef.Ref[strings.LastIndex(schemaRef.Ref, "/")+1:] +} + +// Empty indicates whether a change was found in this element +func (diff *SubschemasDiff) Empty() bool { + if diff == nil { + return true + } + + return len(diff.Added) == 0 && + len(diff.Deleted) == 0 && + len(diff.Modified) == 0 +} + +func getSubschemasDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (*SubschemasDiff, error) { + diff, err := getSubschemasDiffInternal(config, state, schemaRefs1, schemaRefs2) + if err != nil { + return nil, err + } + + if diff.Empty() { + return nil, nil + } + + return diff, nil +} + +func (diff SubschemasDiff) combine(other SubschemasDiff) (*SubschemasDiff, error) { + + return &SubschemasDiff{ + Added: append(diff.Added, other.Added...), + Deleted: append(diff.Deleted, other.Deleted...), + Modified: append(diff.Modified, other.Modified...), + }, nil +} + +func getSubschemasDiffInternal(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (*SubschemasDiff, error) { + + if len(schemaRefs1) == 0 && len(schemaRefs2) == 0 { + return nil, nil + } + + diffRefs, err := getSubschemasRefDiff(config, state, schemaRefs1, schemaRefs2) + if err != nil { + return nil, err + } + + diffInline, err := getSubschemasInlineDiff(config, state, schemaRefs1, schemaRefs2) + if err != nil { + return nil, err + } + + return diffRefs.combine(diffInline) +} + +type schemaRefsFilter func(schemaRef *openapi3.SchemaRef) bool + +// getSubschemasRefDiff compares subschemas by $ref name +func getSubschemasRefDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (*SubschemasDiff, error) { + + result := NewSubschemasDiff() + + refMap2 := toRefMap(schemaRefs2, isSchemaRef) + for index1, schemaRef1 := range schemaRefs1 { + if !isSchemaRef(schemaRef1) { + continue + } + if schemaRef2, index2, found := refMap2.pop(schemaRef1.Ref); found { + if err := result.appendModified(config, state, schemaRef1, schemaRef2, index1, index2); err != nil { + return result, err + } + continue + } + result.appendDeleted(index1, schemaRef1, "") + } + + refMap1 := toRefMap(schemaRefs1, isSchemaRef) + for index2, schemaRef2 := range schemaRefs2 { + if !isSchemaRef(schemaRef2) { + continue + } + if _, _, found := refMap1.pop(schemaRef2.Ref); !found { + result.appendAdded(index2, schemaRef2, "") + } + } + return result, nil +} + +// getSubschemasInlineDiff compares inline subschemas +func getSubschemasInlineDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (SubschemasDiff, error) { + + // find schemas in revision that have no matching schema in the base + addedIdx, err := getNonContainedInlineSchemas(config, state, schemaRefs2, schemaRefs1) + if err != nil { + return SubschemasDiff{}, err + } + + // find schemas in base that have no matching schema in the revision + deletedIdx, err := getNonContainedInlineSchemas(config, state, schemaRefs1, schemaRefs2) + if err != nil { + return SubschemasDiff{}, err + } + + // match schemas by title + addedIdx, deletedIdx, modifiedSchemas, err := compareByTitle(config, state, addedIdx, deletedIdx, schemaRefs1, schemaRefs2) + if err != nil { + return SubschemasDiff{}, err + } + + // special case: single modified schema with no title + if isSingleModifiedCase(schemaRefs1, schemaRefs2, addedIdx, deletedIdx) { + var err error + modifiedSchemas, err = modifiedSchemas.addSchemaDiff(config, state, schemaRefs1[deletedIdx[0]], schemaRefs2[addedIdx[0]], deletedIdx[0], addedIdx[0]) + if err != nil { + return SubschemasDiff{}, err + } + addedIdx = []int{} + deletedIdx = []int{} + } + + return SubschemasDiff{ + Added: getSubschemas(addedIdx, schemaRefs2), + Deleted: getSubschemas(deletedIdx, schemaRefs1), + Modified: modifiedSchemas, + }, nil +} + +func isSingleModifiedCase(schemaRefs1, schemaRefs2 openapi3.SchemaRefs, addedIdx, deletedIdx []int) bool { + return len(addedIdx) == 1 && + len(deletedIdx) == 1 && + schemaRefs1[deletedIdx[0]].Value.Title == "" && + schemaRefs2[addedIdx[0]].Value.Title == "" +} + +func compareByTitle(config *Config, state *state, addedIdx, deletedIdx []int, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) ([]int, []int, ModifiedSubschemas, error) { + + addedMatched, deletedMatched := matchByTitle(config, state, addedIdx, deletedIdx, schemaRefs1, schemaRefs2) + + modifiedSchemas := ModifiedSubschemas{} + for _, addedId := range addedIdx { + deletedId, found := addedMatched[addedId] + if !found { + continue + } + + var err error + modifiedSchemas, err = modifiedSchemas.addSchemaDiff(config, state, schemaRefs1[deletedId], schemaRefs2[addedId], deletedId, addedId) + if err != nil { + return nil, nil, nil, err + } + } + + return deleteMatched(addedIdx, addedMatched), deleteMatched(deletedIdx, deletedMatched), modifiedSchemas, nil +} + +func deleteMatched(idx []int, addedMatched map[int]int) []int { + addedIdxRemaining := []int{} + for _, addedId := range idx { + if _, found := addedMatched[addedId]; !found { + addedIdxRemaining = append(addedIdxRemaining, addedId) + } + } + return addedIdxRemaining +} + +func matchByTitle(config *Config, state *state, addedIdx, deletedIdx []int, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) (map[int]int, map[int]int) { + + addedMatched := map[int]int{} + deletedMatched := map[int]int{} + + matchedTitles := utils.StringSet{} + matchedTitles.Add("") // empty title is not allowed + + for _, addedId := range addedIdx { + title := schemaRefs2[addedId].Value.Title + if matchedTitles.Contains(title) { + // title already matched, skip + continue + } + for _, deletedId := range deletedIdx { + if title == schemaRefs1[deletedId].Value.Title { + addedMatched[addedId] = deletedId + deletedMatched[deletedId] = addedId + matchedTitles.Add(title) + break + } + } + } + + return addedMatched, deletedMatched +} + +func getNonContainedInlineSchemas(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs) ([]int, error) { + + notContainedIdx := []int{} + matched := map[int]struct{}{} + + for index1, schemaRef1 := range schemaRefs1 { + if !isSchemaInline(schemaRef1) { + continue + } + + if found, index2, err := findIndenticalSchema(config, state, schemaRef1, schemaRefs2, matched, isSchemaInline); err != nil { + return nil, err + } else if !found { + notContainedIdx = append(notContainedIdx, index1) + } else { + matched[index2] = struct{}{} + } + } + return notContainedIdx, nil +} + +func findIndenticalSchema(config *Config, state *state, schemaRef1 *openapi3.SchemaRef, schemasRefs2 openapi3.SchemaRefs, matched map[int]struct{}, filter schemaRefsFilter) (bool, int, error) { + for index2, schemaRef2 := range schemasRefs2 { + if !filter(schemaRef1) { + continue + } + if alreadyMatched(index2, matched) { + continue + } + + if schemaDiff, err := getSchemaDiff(config, state, schemaRef1, schemaRef2); err != nil { + return false, 0, err + } else if schemaDiff.Empty() { + return true, index2, nil + } + } + + return false, 0, nil +} + +func alreadyMatched(index int, matched map[int]struct{}) bool { + _, found := matched[index] + return found +} + +func isSchemaInline(schemaRef *openapi3.SchemaRef) bool { + if schemaRef == nil { + return false + } + return schemaRef.Ref == "" +} + +func isSchemaRef(schemaRef *openapi3.SchemaRef) bool { + if schemaRef == nil { + return false + } + return schemaRef.Ref != "" +} diff --git a/report/report.go b/report/report.go index 19a91cb7..59bcb040 100644 --- a/report/report.go +++ b/report/report.go @@ -387,7 +387,7 @@ func (r *report) printSchema(d *diff.SchemaDiff) { r.printMessage(d.DiscriminatorDiff, "Discriminator changed") } -func (r *report) printSchemaListDiff(d *diff.SchemaListDiff) { +func (r *report) printSchemaListDiff(d *diff.SubschemasDiff) { if d.Empty() { return } @@ -396,9 +396,9 @@ func (r *report) printSchemaListDiff(d *diff.SchemaListDiff) { r.printConditional(len(d.Deleted) > 0, "Schemas deleted:", d.Deleted) if len(d.Modified) > 0 { - for schemaRef, schemaDiff := range d.Modified { - r.print("Schema", schemaRef, "modified") - r.indent().printSchema(schemaDiff) + for _, schemaDiff := range d.Modified { + r.print("Modified schema:", schemaDiff.String()) + r.indent().printSchema(schemaDiff.Diff) } } }