diff --git a/what-changed/comparison_functions.go b/what-changed/comparison_functions.go index b159a00..1078753 100644 --- a/what-changed/comparison_functions.go +++ b/what-changed/comparison_functions.go @@ -105,12 +105,10 @@ func CheckObjectAdded[T any](l, r map[string]*T) (bool, string) { // for running checks on the following methods in order: // CheckPropertyAdditionOrRemoval // CheckForModification -// CheckForMove func CheckProperties[T any](properties []*PropertyCheck[T]) { for _, n := range properties { CheckPropertyAdditionOrRemoval(n.LeftNode, n.RightNode, n.Label, n.Changes, n.Breaking, n.Original, n.New) CheckForModification(n.LeftNode, n.RightNode, n.Label, n.Changes, n.Breaking, n.Original, n.New) - CheckForMove(n.LeftNode, n.RightNode, n.Label, n.Changes, n.Breaking, n.Original, n.New) } } @@ -146,32 +144,14 @@ func CheckForAddition[T any](l, r *yaml.Node, label string, changes *[]*Change[T } // CheckForModification will check left and right yaml.Node instances for changes. Anything that is found in both -// sides, but vary in value is considered a modification. CheckForModification will also check if the position of the -// value has changed in the source documents. +// sides, but vary in value is considered a modification. // -// If there is no change to position, but in value the function adds a change type of Modified, if there is both a change -// in value and a change in position, then it will be set to ModifiedAndMoved +// If there is a change in value the function adds a change type of Modified. // // The Change is then added to the slice of []Change[T] instances provided as a pointer. func CheckForModification[T any](l, r *yaml.Node, label string, changes *[]*Change[T], breaking bool, orig, new T) { if l != nil && l.Value != "" && r != nil && r.Value != "" && r.Value != l.Value { - changeType := Modified - ctx := CreateContext(l, r) - if ctx.HasChanged() { - changeType = ModifiedAndMoved - } - CreateChange[T](changes, changeType, label, l, r, breaking, orig, new) - } -} - -// CheckForMove will check if left and right yaml.Node instances have moved position, but not changed in value. A change -// of type Moved is created and added to changes. -func CheckForMove[T any](l, r *yaml.Node, label string, changes *[]*Change[T], breaking bool, orig, new T) { - if l != nil && l.Value != "" && r != nil && r.Value != "" && r.Value == l.Value { // everything is equal - ctx := CreateContext(l, r) - if ctx.HasChanged() { - CreateChange[T](changes, Moved, label, l, r, breaking, orig, new) - } + CreateChange[T](changes, Modified, label, l, r, breaking, orig, new) } } diff --git a/what-changed/contact_test.go b/what-changed/contact_test.go index 494972f..b3702c1 100644 --- a/what-changed/contact_test.go +++ b/what-changed/contact_test.go @@ -213,9 +213,7 @@ email: dave@quobix.com` // compare. extChanges := CompareContact(&lDoc, &rDoc) - assert.Equal(t, 2, extChanges.TotalChanges()) - assert.Equal(t, Moved, extChanges.Changes[0].ChangeType) - assert.Equal(t, ModifiedAndMoved, extChanges.Changes[1].ChangeType) + assert.Equal(t, 1, extChanges.TotalChanges()) } func TestCompareContact_Identical(t *testing.T) { diff --git a/what-changed/extensions_test.go b/what-changed/extensions_test.go index a0fa289..9adec6e 100644 --- a/what-changed/extensions_test.go +++ b/what-changed/extensions_test.go @@ -31,54 +31,6 @@ func TestCompareExtensions(t *testing.T) { assert.False(t, extChanges.Changes[0].Context.HasChanged()) } -func TestCompareExtensions_Moved(t *testing.T) { - - left := `pizza: pie -x-test: 1` - - right := `x-test: 1` - - var lNode, rNode yaml.Node - _ = yaml.Unmarshal([]byte(left), &lNode) - _ = yaml.Unmarshal([]byte(right), &rNode) - - lExt := low.ExtractExtensions(lNode.Content[0]) - rExt := low.ExtractExtensions(rNode.Content[0]) - - extChanges := CompareExtensions(lExt, rExt) - - assert.Len(t, extChanges.Changes, 1) - assert.Equal(t, Moved, extChanges.Changes[0].ChangeType) - assert.Equal(t, 2, extChanges.Changes[0].Context.OriginalLine) - assert.Equal(t, 1, extChanges.Changes[0].Context.NewLine) - assert.True(t, extChanges.Changes[0].Context.HasChanged()) -} - -func TestCompareExtensions_ModifiedAndMoved(t *testing.T) { - - left := `pizza: pie -x-test: 1` - - right := `x-test: 2` - - var lNode, rNode yaml.Node - _ = yaml.Unmarshal([]byte(left), &lNode) - _ = yaml.Unmarshal([]byte(right), &rNode) - - lExt := low.ExtractExtensions(lNode.Content[0]) - rExt := low.ExtractExtensions(rNode.Content[0]) - - extChanges := CompareExtensions(lExt, rExt) - - assert.Len(t, extChanges.Changes, 1) - assert.Equal(t, ModifiedAndMoved, extChanges.Changes[0].ChangeType) - assert.Equal(t, 2, extChanges.Changes[0].Context.OriginalLine) - assert.Equal(t, 1, extChanges.Changes[0].Context.NewLine) - assert.Equal(t, "1", extChanges.Changes[0].Original) - assert.Equal(t, "2", extChanges.Changes[0].New) - assert.True(t, extChanges.Changes[0].Context.HasChanged()) -} - func TestCompareExtensions_Removed(t *testing.T) { left := `pizza: pie diff --git a/what-changed/external_docs_test.go b/what-changed/external_docs_test.go index abed83e..40999dd 100644 --- a/what-changed/external_docs_test.go +++ b/what-changed/external_docs_test.go @@ -97,34 +97,24 @@ url: https://quobix.com` // validate property changes urlChange := extChanges.Changes[0] - assert.Equal(t, ModifiedAndMoved, urlChange.ChangeType) + assert.Equal(t, Modified, urlChange.ChangeType) assert.True(t, urlChange.Context.HasChanged()) assert.Equal(t, "https://pb33f.io", urlChange.Original) assert.Equal(t, "https://quobix.com", urlChange.New) - assert.Equal(t, 1, urlChange.Context.OriginalLine) - assert.Equal(t, 3, urlChange.Context.NewLine) assert.Equal(t, lowv3.URLLabel, urlChange.Property) descChange := extChanges.Changes[1] - assert.Equal(t, ModifiedAndMoved, descChange.ChangeType) + assert.Equal(t, Modified, descChange.ChangeType) assert.True(t, descChange.Context.HasChanged()) assert.Equal(t, "this is another test", descChange.New) assert.Equal(t, "this is a test", descChange.Original) - assert.Equal(t, 2, descChange.Context.OriginalLine) - assert.Equal(t, 14, descChange.Context.OriginalColumn) - assert.Equal(t, 1, descChange.Context.NewLine) - assert.Equal(t, 14, descChange.Context.NewColumn) // validate extensions extChange := extChanges.ExtensionChanges.Changes[0] - assert.Equal(t, ModifiedAndMoved, extChange.ChangeType) + assert.Equal(t, Modified, extChange.ChangeType) assert.True(t, extChange.Context.HasChanged()) assert.Equal(t, "hiya!", extChange.New) assert.Equal(t, "hello", extChange.Original) - assert.Equal(t, 3, extChange.Context.OriginalLine) - assert.Equal(t, 12, extChange.Context.OriginalColumn) - assert.Equal(t, 2, extChange.Context.NewLine) - assert.Equal(t, 12, extChange.Context.NewColumn) } func TestCompareExternalDocs_Identical(t *testing.T) { @@ -177,7 +167,7 @@ x-testing: hello` // compare. extChanges := CompareExternalDocs(&lDoc, &rDoc) - assert.Equal(t, 2, extChanges.TotalChanges()) + assert.Equal(t, 1, extChanges.TotalChanges()) assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType) } diff --git a/what-changed/license_test.go b/what-changed/license_test.go index 57866a9..00def51 100644 --- a/what-changed/license_test.go +++ b/what-changed/license_test.go @@ -162,7 +162,7 @@ url: https://pb33f.io` // compare. extChanges := CompareLicense(&lDoc, &rDoc) assert.Equal(t, 2, extChanges.TotalChanges()) - assert.Equal(t, ModifiedAndMoved, extChanges.Changes[0].ChangeType) + assert.Equal(t, Modified, extChanges.Changes[0].ChangeType) assert.Equal(t, PropertyRemoved, extChanges.Changes[1].ChangeType) } diff --git a/what-changed/models.go b/what-changed/models.go index cd1ece1..eb30e6e 100644 --- a/what-changed/models.go +++ b/what-changed/models.go @@ -22,12 +22,6 @@ const ( // PropertyRemoved means that a property of an object was removed PropertyRemoved - - // Moved means that a property or an object was moved, but unchanged (line/columns changed) - Moved - - // ModifiedAndMoved means that a property was modified, and it was also moved. - ModifiedAndMoved ) // WhatChanged is a summary object that contains a high level summary of everything changed. @@ -50,6 +44,9 @@ type ChangeContext struct { } // HasChanged determines if the line and column numbers of the original and new values have changed. +// +// It's worth noting that there is no guarantee to the positions of anything in either left or right, so +// considering these values as 'changes' is going to add a considerable amount of noise to results. func (c *ChangeContext) HasChanged() bool { return c.NewLine != c.OriginalLine || c.NewColumn != c.OriginalColumn } @@ -57,7 +54,9 @@ func (c *ChangeContext) HasChanged() bool { // Change represents a change between two different elements inside an OpenAPI specification. type Change[T any] struct { - // Context represents the lines and column numbers of the original and new changes. + // Context represents the lines and column numbers of the original and new values + // It's worth noting that these values may frequently be different and are not used to calculate + // a change. If the positions change, but values do not, then no change is recorded. Context *ChangeContext // ChangeType represents the type of change that occurred. stored as an integer, defined by constants above. diff --git a/what-changed/tags_test.go b/what-changed/tags_test.go index 372189b..dfec172 100644 --- a/what-changed/tags_test.go +++ b/what-changed/tags_test.go @@ -4,15 +4,15 @@ package what_changed import ( - "github.com/pb33f/libopenapi/datamodel" - lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" - "github.com/stretchr/testify/assert" - "testing" + "github.com/pb33f/libopenapi/datamodel" + lowv3 "github.com/pb33f/libopenapi/datamodel/low/v3" + "github.com/stretchr/testify/assert" + "testing" ) func TestCompareTags(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - name: a tag description: a lovely tag @@ -21,7 +21,7 @@ tags: url: https://quobix.com description: cool` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - name: a tag description: a lovelier tag description @@ -30,31 +30,31 @@ tags: url: https://pb33f.io description: cooler` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Len(t, changes.Changes, 1) - assert.Len(t, changes.ExternalDocs.Changes, 2) - assert.Len(t, changes.ExtensionChanges.Changes, 1) - assert.Equal(t, 4, changes.TotalChanges()) + // evaluate. + assert.Len(t, changes.Changes, 1) + assert.Len(t, changes.ExternalDocs.Changes, 2) + assert.Len(t, changes.ExtensionChanges.Changes, 1) + assert.Equal(t, 4, changes.TotalChanges()) - descChange := changes.Changes[0] - assert.Equal(t, "a lovelier tag description", descChange.New) - assert.Equal(t, "a lovely tag", descChange.Original) - assert.Equal(t, Modified, descChange.ChangeType) - assert.False(t, descChange.Context.HasChanged()) + descChange := changes.Changes[0] + assert.Equal(t, "a lovelier tag description", descChange.New) + assert.Equal(t, "a lovely tag", descChange.Original) + assert.Equal(t, Modified, descChange.ChangeType) + assert.False(t, descChange.Context.HasChanged()) } func TestCompareTags_AddNewTag(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - name: a tag description: a lovelier tag description @@ -63,7 +63,7 @@ tags: url: https://pb33f.io description: cooler` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - name: a tag description: a lovelier tag description @@ -74,26 +74,26 @@ tags: - name: a new tag description: a cool new tag` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Len(t, changes.Changes, 1) - assert.Equal(t, 1, changes.TotalChanges()) + // evaluate. + assert.Len(t, changes.Changes, 1) + assert.Equal(t, 1, changes.TotalChanges()) - descChange := changes.Changes[0] - assert.Equal(t, ObjectAdded, descChange.ChangeType) + descChange := changes.Changes[0] + assert.Equal(t, ObjectAdded, descChange.ChangeType) } func TestCompareTags_AddDeleteTag(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - name: a tag description: a lovelier tag description @@ -102,31 +102,31 @@ tags: url: https://pb33f.io description: cooler` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - name: a new tag description: a cool new tag` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Len(t, changes.Changes, 2) - assert.Equal(t, 2, changes.TotalChanges()) + // evaluate. + assert.Len(t, changes.Changes, 2) + assert.Equal(t, 2, changes.TotalChanges()) - assert.Equal(t, ObjectRemoved, changes.Changes[0].ChangeType) - assert.Equal(t, ObjectAdded, changes.Changes[1].ChangeType) + assert.Equal(t, ObjectRemoved, changes.Changes[0].ChangeType) + assert.Equal(t, ObjectAdded, changes.Changes[1].ChangeType) } func TestCompareTags_DescriptionMoved(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - description: a lovelier tag description name: a tag @@ -135,7 +135,7 @@ tags: url: https://pb33f.io description: cooler` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - name: a tag x-tag: something else @@ -144,36 +144,23 @@ tags: url: https://pb33f.io description: cooler` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Len(t, changes.Changes, 2) - assert.Equal(t, 3, changes.TotalChanges()) - - nameChange := changes.Changes[0] - assert.Equal(t, Moved, nameChange.ChangeType) - assert.Equal(t, 4, nameChange.Context.OriginalLine) - assert.Equal(t, 3, nameChange.Context.NewLine) - assert.True(t, nameChange.Context.HasChanged()) - - descChange := changes.Changes[1] - assert.Equal(t, Moved, descChange.ChangeType) - assert.Equal(t, 3, descChange.Context.OriginalLine) - assert.Equal(t, 5, descChange.Context.NewLine) - assert.True(t, descChange.Context.HasChanged()) + // evaluate. + assert.Nil(t, changes) } func TestCompareTags_NameMoved(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - description: a lovelier tag description name: a tag @@ -182,7 +169,7 @@ tags: url: https://pb33f.io description: cooler` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - description: a lovelier tag description x-tag: something else @@ -191,29 +178,22 @@ tags: description: cooler name: a tag` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Len(t, changes.Changes, 1) - assert.Equal(t, 4, changes.TotalChanges()) - - nameChange := changes.Changes[0] - assert.Equal(t, Moved, nameChange.ChangeType) - assert.Equal(t, 4, nameChange.Context.OriginalLine) - assert.Equal(t, 8, nameChange.Context.NewLine) - assert.True(t, nameChange.Context.HasChanged()) + // evaluate. + assert.Nil(t, changes) } func TestCompareTags_ModifiedAndMoved(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - description: a lovelier tag description name: a tag @@ -222,7 +202,7 @@ tags: url: https://pb33f.io description: cooler` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - name: a tag x-tag: something else @@ -231,31 +211,29 @@ tags: url: https://pb33f.io description: cooler` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Len(t, changes.Changes, 2) - assert.Equal(t, 3, changes.TotalChanges()) + // evaluate. + assert.Len(t, changes.Changes, 1) + assert.Equal(t, 1, changes.TotalChanges()) - descChange := changes.Changes[1] - assert.Equal(t, ModifiedAndMoved, descChange.ChangeType) - assert.Equal(t, 3, descChange.Context.OriginalLine) - assert.Equal(t, 5, descChange.Context.NewLine) - assert.Equal(t, "a lovelier tag description", descChange.Original) - assert.Equal(t, "a different tag description", descChange.New) - assert.True(t, descChange.Context.HasChanged()) + descChange := changes.Changes[0] + assert.Equal(t, Modified, descChange.ChangeType) + assert.Equal(t, "a lovelier tag description", descChange.Original) + assert.Equal(t, "a different tag description", descChange.New) + assert.True(t, descChange.Context.HasChanged()) } func TestCompareTags_Identical(t *testing.T) { - left := `openapi: 3.0.1 + left := `openapi: 3.0.1 tags: - description: a lovelier tag description name: a tag @@ -264,7 +242,7 @@ tags: url: https://pb33f.io description: cooler` - right := `openapi: 3.0.1 + right := `openapi: 3.0.1 tags: - description: a lovelier tag description name: a tag @@ -273,16 +251,16 @@ tags: url: https://pb33f.io description: cooler` - // create document (which will create our correct tags low level structures) - lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) - rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) - lDoc, _ := lowv3.CreateDocument(lInfo) - rDoc, _ := lowv3.CreateDocument(rInfo) + // create document (which will create our correct tags low level structures) + lInfo, _ := datamodel.ExtractSpecInfo([]byte(left)) + rInfo, _ := datamodel.ExtractSpecInfo([]byte(right)) + lDoc, _ := lowv3.CreateDocument(lInfo) + rDoc, _ := lowv3.CreateDocument(rInfo) - // compare. - changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) + // compare. + changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) - // evaluate. - assert.Nil(t, changes) + // evaluate. + assert.Nil(t, changes) }