Skip to content

Commit

Permalink
Fixed schema comparison for out of order keys. #170
Browse files Browse the repository at this point in the history
Keys were being compared incorrectly due to a combination of a swapped property, that sorts differently.

Signed-off-by: quobix <dave@quobix.com>
  • Loading branch information
daveshanley committed Sep 18, 2023
1 parent e58aeb7 commit dab6346
Show file tree
Hide file tree
Showing 2 changed files with 63 additions and 9 deletions.
27 changes: 18 additions & 9 deletions what-changed/model/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package model

import (
"fmt"
"golang.org/x/exp/slices"
"sort"
"sync"

Expand Down Expand Up @@ -480,17 +481,25 @@ func buildProperty(lProps, rProps []string, lEntities, rEntities map[string]*bas
go checkProperty(lProps[w], lp, rp, propChanges, doneChan)
}

// keys do not match, even after sorting, means a like for like replacement.
// keys do not match, even after sorting, check which one was added and which one was removed.
if lProps[w] != rProps[w] {

// old removed, new added.
CreateChange(changes, ObjectAdded, v3.PropertiesLabel,
nil, rKeyNodes[rProps[w]], false, nil, rEntities[rProps[w]])

CreateChange(changes, ObjectRemoved, v3.PropertiesLabel,
lKeyNodes[lProps[w]], nil, true, lEntities[lProps[w]], nil)
if !slices.Contains(lProps, rProps[w]) {
// new added.
CreateChange(changes, ObjectAdded, v3.PropertiesLabel,
nil, rKeyNodes[rProps[w]], false, nil, rEntities[rProps[w]])
}
if !slices.Contains(rProps, lProps[w]) {
CreateChange(changes, ObjectRemoved, v3.PropertiesLabel,
lKeyNodes[lProps[w]], nil, true, lEntities[lProps[w]], nil)
}
if slices.Contains(lProps, rProps[w]) {
h := slices.Index(lProps, rProps[w])
totalProperties++
lp = lEntities[lProps[h]]
rp = rEntities[rProps[w]]
go checkProperty(lProps[h], lp, rp, propChanges, doneChan)
}
}

}
}

Expand Down
45 changes: 45 additions & 0 deletions what-changed/model/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2663,3 +2663,48 @@ components:
assert.Equal(t, 0, changes.TotalBreakingChanges())

}

func TestCompareSchemas_CheckIssue_170(t *testing.T) {
left := `openapi: 3.0
components:
schemas:
OK:
type: object
properties:
id:
type: integer
format: int64
name:
type: string
tag:
type: string`

right := `openapi: 3.0
components:
schemas:
OK:
type: object
properties:
id:
type: integer
format: int64
name:
type: integer
format: int64
foo:
type: string`

leftDoc, rightDoc := test_BuildDoc(left, right)

// extract left reference schema and non reference schema.
lSchemaProxy := leftDoc.Components.Value.FindSchema("OK").Value
rSchemaProxy := rightDoc.Components.Value.FindSchema("OK").Value

changes := CompareSchemas(lSchemaProxy, rSchemaProxy)
assert.NotNil(t, changes)
assert.Equal(t, 4, changes.TotalChanges())
assert.Len(t, changes.GetAllChanges(), 4)
assert.Equal(t, 3, changes.TotalBreakingChanges())
assert.Len(t, changes.SchemaPropertyChanges["name"].PropertyChanges.Changes, 2)
assert.Len(t, changes.PropertyChanges.Changes, 2)
}

0 comments on commit dab6346

Please sign in to comment.