From 519a522c03510a1a0ee70d0dc92472f4b9b0b640 Mon Sep 17 00:00:00 2001 From: Reuven Harrison Date: Sun, 9 Jul 2023 15:31:54 +0300 Subject: [PATCH] replace DeepEqual by getSchemaDiff (#312) --- data/x-of/anyof-base-openapi.yml | 24 ++++++++++++++++++++++ data/x-of/anyof-rev-openapi.yml | 28 ++++++++++++++++++++++++++ diff/breaking_property_test.go | 17 ---------------- diff/config.go | 3 ++- diff/config_test.go | 4 +--- diff/diff_x_of_test.go | 34 ++++++++++++++++++++++++++++++++ diff/schema_list_diff.go | 22 +++++++++++---------- internal/changelog_flags.go | 3 +-- internal/diff_flags.go | 3 +-- 9 files changed, 103 insertions(+), 35 deletions(-) create mode 100644 data/x-of/anyof-base-openapi.yml create mode 100644 data/x-of/anyof-rev-openapi.yml diff --git a/data/x-of/anyof-base-openapi.yml b/data/x-of/anyof-base-openapi.yml new file mode 100644 index 00000000..17a98f51 --- /dev/null +++ b/data/x-of/anyof-base-openapi.yml @@ -0,0 +1,24 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + summary: Test Get with anyOf #1 + responses: + '200': + description: Success + content: + 'application/json': + schema: + anyOf: + - type: object + properties: + prop1: + type: string + description: Some description + - type: object + properties: + prop2: + type: string \ No newline at end of file diff --git a/data/x-of/anyof-rev-openapi.yml b/data/x-of/anyof-rev-openapi.yml new file mode 100644 index 00000000..e2f924c0 --- /dev/null +++ b/data/x-of/anyof-rev-openapi.yml @@ -0,0 +1,28 @@ +openapi: 3.0.3 +info: + title: Test + version: "0.1" +paths: + '/test': + get: + summary: Test Get with anyOf + responses: + '200': + description: Successful + content: + 'application/json': + schema: + anyOf: + - type: object + properties: + prop1: + type: string + description: Other description + - type: object + properties: + prop2: + type: string + - type: object + properties: + prop3: + type: string \ No newline at end of file diff --git a/diff/breaking_property_test.go b/diff/breaking_property_test.go index 85c3f79b..188ef386 100644 --- a/diff/breaking_property_test.go +++ b/diff/breaking_property_test.go @@ -238,23 +238,6 @@ func TestBreaking_RespBodyDeleteRequiredProperty(t *testing.T) { require.NotEmpty(t, d) } -// BC: adding a new required property under AllOf in response body is not breaking -func TestBreaking_RespBodyNewAllOfRequiredProperty(t *testing.T) { - loader := openapi3.NewLoader() - - s1, err := loader.LoadFromFile(getReqPropFile("response-allof-base.json")) - require.NoError(t, err) - - s2, err := loader.LoadFromFile(getReqPropFile("response-allof-revision.json")) - require.NoError(t, err) - - d, err := diff.Get(&diff.Config{ - BreakingOnly: true, - }, s1, s2) - require.NoError(t, err) - require.Empty(t, d) -} - // BC: deleting a required property under AllOf in response body is breaking func TestBreaking_RespBodyDeleteAllOfRequiredProperty(t *testing.T) { loader := openapi3.NewLoader() diff --git a/diff/config.go b/diff/config.go index 071854a3..c40e74c2 100644 --- a/diff/config.go +++ b/diff/config.go @@ -44,8 +44,9 @@ func NewConfig() *Config { } } -func (config *Config) SetExcludeElements(excludeElements []string) { +func (config *Config) WithExcludeElements(excludeElements []string) *Config { config.ExcludeElements = utils.StringList(excludeElements).ToStringSet() + return config } func (config *Config) IsExcludeExamples() bool { diff --git a/diff/config_test.go b/diff/config_test.go index 59d3bd02..c9f98e30 100644 --- a/diff/config_test.go +++ b/diff/config_test.go @@ -9,7 +9,6 @@ import ( func TestConfig_Default(t *testing.T) { c := diff.NewConfig() - c.SetExcludeElements([]string{}) require.False(t, c.IsExcludeExamples()) require.False(t, c.IsExcludeDescription()) require.False(t, c.IsExcludeEndpoints()) @@ -18,8 +17,7 @@ func TestConfig_Default(t *testing.T) { } func TestConfig_ExcludeElements(t *testing.T) { - c := diff.NewConfig() - c.SetExcludeElements(diff.ExcludeDiffOptions) + c := diff.NewConfig().WithExcludeElements(diff.ExcludeDiffOptions) require.True(t, c.IsExcludeExamples()) require.True(t, c.IsExcludeDescription()) require.True(t, c.IsExcludeEndpoints()) diff --git a/diff/diff_x_of_test.go b/diff/diff_x_of_test.go index 685d4369..2103dc70 100644 --- a/diff/diff_x_of_test.go +++ b/diff/diff_x_of_test.go @@ -101,3 +101,37 @@ func TestOneOf_MultiRefs(t *testing.T) { 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]) } + +func TestAnyOf_IncludeDescriptions(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfFile("anyof-base-openapi.yml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfFile("anyof-rev-openapi.yml")) + require.NoError(t, err) + + dd, err := diff.Get(&diff.Config{}, s1, s2) + require.NoError(t, err) + anyOfDiff := dd.PathsDiff.Modified["/test"].OperationsDiff.Modified["GET"].ResponsesDiff.Modified["200"].ContentDiff.MediaTypeModified["application/json"].SchemaDiff.AnyOfDiff + require.Equal(t, 2, anyOfDiff.Added) + require.Equal(t, 1, anyOfDiff.Deleted) + require.Empty(t, anyOfDiff.Modified) +} + +func TestAnyOf_ExcludeDescriptions(t *testing.T) { + loader := openapi3.NewLoader() + + s1, err := loader.LoadFromFile(getXOfFile("anyof-base-openapi.yml")) + require.NoError(t, err) + + s2, err := loader.LoadFromFile(getXOfFile("anyof-rev-openapi.yml")) + require.NoError(t, err) + + 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.Equal(t, 1, anyOfDiff.Added) + require.Equal(t, 0, anyOfDiff.Deleted) + require.Empty(t, anyOfDiff.Modified) +} diff --git a/diff/schema_list_diff.go b/diff/schema_list_diff.go index 608d9ccf..1212a616 100644 --- a/diff/schema_list_diff.go +++ b/diff/schema_list_diff.go @@ -2,7 +2,6 @@ package diff import ( "fmt" - "reflect" "github.com/getkin/kin-openapi/openapi3" ) @@ -117,12 +116,12 @@ func getSchemaListsRefsDiff(config *Config, state *state, schemaRefs1, schemaRef // getSchemaListsRefsDiff compares schemas by their syntax func getSchemaListsInlineDiff(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter schemaRefsFilter) (SchemaListDiff, error) { - added, err := getGroupDifference(schemaRefs2, schemaRefs1, filter) + added, err := getGroupDifference(config, state, schemaRefs2, schemaRefs1, filter) if err != nil { return SchemaListDiff{}, err } - deleted, err := getGroupDifference(schemaRefs1, schemaRefs2, filter) + deleted, err := getGroupDifference(config, state, schemaRefs1, schemaRefs2, filter) if err != nil { return SchemaListDiff{}, err } @@ -148,7 +147,7 @@ func getSchemaListsInlineDiff(config *Config, state *state, schemaRefs1, schemaR }, nil } -func getGroupDifference(schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter schemaRefsFilter) ([]int, error) { +func getGroupDifference(config *Config, state *state, schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter schemaRefsFilter) ([]int, error) { notContained := []int{} matched := map[int]struct{}{} @@ -158,7 +157,9 @@ func getGroupDifference(schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter sch continue } - if found, index2 := findIndenticalSchema(schemaRef1, schemaRefs2, matched, filter); !found { + if found, index2, err := findIndenticalSchema(config, state, schemaRef1, schemaRefs2, matched, filter); err != nil { + return nil, err + } else if !found { notContained = append(notContained, index1) } else { matched[index2] = struct{}{} @@ -167,7 +168,7 @@ func getGroupDifference(schemaRefs1, schemaRefs2 openapi3.SchemaRefs, filter sch return notContained, nil } -func findIndenticalSchema(schemaRef1 *openapi3.SchemaRef, schemasRefs2 openapi3.SchemaRefs, matched map[int]struct{}, filter schemaRefsFilter) (bool, int) { +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 @@ -176,13 +177,14 @@ func findIndenticalSchema(schemaRef1 *openapi3.SchemaRef, schemasRefs2 openapi3. continue } - // compare with DeepEqual rather than SchemaDiff to ensure an exact syntactical match - if reflect.DeepEqual(schemaRef1, schemaRef2) { - return true, index2 + 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 + return false, 0, nil } func alreadyMatched(index int, matched map[int]struct{}) bool { diff --git a/internal/changelog_flags.go b/internal/changelog_flags.go index 1b76e6b3..f1c02036 100644 --- a/internal/changelog_flags.go +++ b/internal/changelog_flags.go @@ -27,7 +27,7 @@ type ChangelogFlags struct { } func (flags *ChangelogFlags) toConfig() *diff.Config { - config := diff.NewConfig().WithCheckBreaking() + config := diff.NewConfig().WithCheckBreaking().WithExcludeElements(flags.excludeElements) config.PathFilter = flags.matchPath config.FilterExtension = flags.filterExtension config.PathPrefixBase = flags.prefixBase @@ -35,7 +35,6 @@ func (flags *ChangelogFlags) toConfig() *diff.Config { config.PathStripPrefixBase = flags.stripPrefixBase config.PathStripPrefixRevision = flags.stripPrefixRevision config.IncludePathParams = flags.includePathParams - config.SetExcludeElements(flags.excludeElements) config.DeprecationDays = flags.deprecationDays return config diff --git a/internal/diff_flags.go b/internal/diff_flags.go index 88f2cade..f0667e0a 100644 --- a/internal/diff_flags.go +++ b/internal/diff_flags.go @@ -22,7 +22,7 @@ type DiffFlags struct { } func (flags *DiffFlags) toConfig() *diff.Config { - config := diff.NewConfig() + config := diff.NewConfig().WithExcludeElements(flags.excludeElements) config.PathFilter = flags.matchPath config.FilterExtension = flags.filterExtension config.PathPrefixBase = flags.prefixBase @@ -30,7 +30,6 @@ func (flags *DiffFlags) toConfig() *diff.Config { config.PathStripPrefixBase = flags.stripPrefixBase config.PathStripPrefixRevision = flags.stripPrefixRevision config.IncludePathParams = flags.includePathParams - config.SetExcludeElements(flags.excludeElements) return config }