Skip to content

Commit

Permalink
replace DeepEqual by getSchemaDiff (#312)
Browse files Browse the repository at this point in the history
  • Loading branch information
Reuven Harrison authored Jul 9, 2023
1 parent 5dc9383 commit 519a522
Show file tree
Hide file tree
Showing 9 changed files with 103 additions and 35 deletions.
24 changes: 24 additions & 0 deletions data/x-of/anyof-base-openapi.yml
Original file line number Diff line number Diff line change
@@ -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
28 changes: 28 additions & 0 deletions data/x-of/anyof-rev-openapi.yml
Original file line number Diff line number Diff line change
@@ -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
17 changes: 0 additions & 17 deletions diff/breaking_property_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion diff/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
4 changes: 1 addition & 3 deletions diff/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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())
Expand Down
34 changes: 34 additions & 0 deletions diff/diff_x_of_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
22 changes: 12 additions & 10 deletions diff/schema_list_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package diff

import (
"fmt"
"reflect"

"github.com/getkin/kin-openapi/openapi3"
)
Expand Down Expand Up @@ -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
}
Expand All @@ -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{}{}
Expand All @@ -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{}{}
Expand All @@ -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
Expand All @@ -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 {
Expand Down
3 changes: 1 addition & 2 deletions internal/changelog_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,14 @@ 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
config.PathPrefixRevision = flags.prefixRevision
config.PathStripPrefixBase = flags.stripPrefixBase
config.PathStripPrefixRevision = flags.stripPrefixRevision
config.IncludePathParams = flags.includePathParams
config.SetExcludeElements(flags.excludeElements)
config.DeprecationDays = flags.deprecationDays

return config
Expand Down
3 changes: 1 addition & 2 deletions internal/diff_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,14 @@ 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
config.PathPrefixRevision = flags.prefixRevision
config.PathStripPrefixBase = flags.stripPrefixBase
config.PathStripPrefixRevision = flags.stripPrefixRevision
config.IncludePathParams = flags.includePathParams
config.SetExcludeElements(flags.excludeElements)

return config
}
Expand Down

0 comments on commit 519a522

Please sign in to comment.