Removed the ModifiedAndRemoved and Moved change types from the library.

They are not useful and will create a lot of noise, The context will always be available, but changes in context will not result in a recorded change.

Signed-off-by: Dave Shanley <dave@quobix.com>
This commit is contained in:
Dave Shanley
2022-10-02 12:38:28 -04:00
parent 60da35e8b5
commit 9775c384f7
7 changed files with 116 additions and 219 deletions

View File

@@ -105,12 +105,10 @@ func CheckObjectAdded[T any](l, r map[string]*T) (bool, string) {
// for running checks on the following methods in order: // for running checks on the following methods in order:
// CheckPropertyAdditionOrRemoval // CheckPropertyAdditionOrRemoval
// CheckForModification // CheckForModification
// CheckForMove
func CheckProperties[T any](properties []*PropertyCheck[T]) { func CheckProperties[T any](properties []*PropertyCheck[T]) {
for _, n := range properties { for _, n := range properties {
CheckPropertyAdditionOrRemoval(n.LeftNode, n.RightNode, n.Label, n.Changes, n.Breaking, n.Original, n.New) 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) 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 // 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 // sides, but vary in value is considered a modification.
// value has changed in the source documents.
// //
// If there is no change to position, but in value the function adds a change type of Modified, if there is both a change // If there is a change in value the function adds a change type of Modified.
// in value and a change in position, then it will be set to ModifiedAndMoved
// //
// The Change is then added to the slice of []Change[T] instances provided as a pointer. // 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) { 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 { if l != nil && l.Value != "" && r != nil && r.Value != "" && r.Value != l.Value {
changeType := Modified CreateChange[T](changes, Modified, label, l, r, breaking, orig, new)
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)
}
} }
} }

View File

@@ -213,9 +213,7 @@ email: dave@quobix.com`
// compare. // compare.
extChanges := CompareContact(&lDoc, &rDoc) extChanges := CompareContact(&lDoc, &rDoc)
assert.Equal(t, 2, extChanges.TotalChanges()) assert.Equal(t, 1, extChanges.TotalChanges())
assert.Equal(t, Moved, extChanges.Changes[0].ChangeType)
assert.Equal(t, ModifiedAndMoved, extChanges.Changes[1].ChangeType)
} }
func TestCompareContact_Identical(t *testing.T) { func TestCompareContact_Identical(t *testing.T) {

View File

@@ -31,54 +31,6 @@ func TestCompareExtensions(t *testing.T) {
assert.False(t, extChanges.Changes[0].Context.HasChanged()) 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) { func TestCompareExtensions_Removed(t *testing.T) {
left := `pizza: pie left := `pizza: pie

View File

@@ -97,34 +97,24 @@ url: https://quobix.com`
// validate property changes // validate property changes
urlChange := extChanges.Changes[0] urlChange := extChanges.Changes[0]
assert.Equal(t, ModifiedAndMoved, urlChange.ChangeType) assert.Equal(t, Modified, urlChange.ChangeType)
assert.True(t, urlChange.Context.HasChanged()) assert.True(t, urlChange.Context.HasChanged())
assert.Equal(t, "https://pb33f.io", urlChange.Original) assert.Equal(t, "https://pb33f.io", urlChange.Original)
assert.Equal(t, "https://quobix.com", urlChange.New) 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) assert.Equal(t, lowv3.URLLabel, urlChange.Property)
descChange := extChanges.Changes[1] descChange := extChanges.Changes[1]
assert.Equal(t, ModifiedAndMoved, descChange.ChangeType) assert.Equal(t, Modified, descChange.ChangeType)
assert.True(t, descChange.Context.HasChanged()) assert.True(t, descChange.Context.HasChanged())
assert.Equal(t, "this is another test", descChange.New) assert.Equal(t, "this is another test", descChange.New)
assert.Equal(t, "this is a test", descChange.Original) 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 // validate extensions
extChange := extChanges.ExtensionChanges.Changes[0] extChange := extChanges.ExtensionChanges.Changes[0]
assert.Equal(t, ModifiedAndMoved, extChange.ChangeType) assert.Equal(t, Modified, extChange.ChangeType)
assert.True(t, extChange.Context.HasChanged()) assert.True(t, extChange.Context.HasChanged())
assert.Equal(t, "hiya!", extChange.New) assert.Equal(t, "hiya!", extChange.New)
assert.Equal(t, "hello", extChange.Original) 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) { func TestCompareExternalDocs_Identical(t *testing.T) {
@@ -177,7 +167,7 @@ x-testing: hello`
// compare. // compare.
extChanges := CompareExternalDocs(&lDoc, &rDoc) extChanges := CompareExternalDocs(&lDoc, &rDoc)
assert.Equal(t, 2, extChanges.TotalChanges()) assert.Equal(t, 1, extChanges.TotalChanges())
assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType) assert.Equal(t, PropertyAdded, extChanges.Changes[0].ChangeType)
} }

View File

@@ -162,7 +162,7 @@ url: https://pb33f.io`
// compare. // compare.
extChanges := CompareLicense(&lDoc, &rDoc) extChanges := CompareLicense(&lDoc, &rDoc)
assert.Equal(t, 2, extChanges.TotalChanges()) 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) assert.Equal(t, PropertyRemoved, extChanges.Changes[1].ChangeType)
} }

View File

@@ -22,12 +22,6 @@ const (
// PropertyRemoved means that a property of an object was removed // PropertyRemoved means that a property of an object was removed
PropertyRemoved 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. // 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. // 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 { func (c *ChangeContext) HasChanged() bool {
return c.NewLine != c.OriginalLine || c.NewColumn != c.OriginalColumn 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. // Change represents a change between two different elements inside an OpenAPI specification.
type Change[T any] struct { 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 Context *ChangeContext
// ChangeType represents the type of change that occurred. stored as an integer, defined by constants above. // ChangeType represents the type of change that occurred. stored as an integer, defined by constants above.

View File

@@ -154,20 +154,7 @@ tags:
changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
// evaluate. // evaluate.
assert.Len(t, changes.Changes, 2) assert.Nil(t, changes)
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())
} }
@@ -201,14 +188,7 @@ tags:
changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
// evaluate. // evaluate.
assert.Len(t, changes.Changes, 1) assert.Nil(t, changes)
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())
} }
func TestCompareTags_ModifiedAndMoved(t *testing.T) { func TestCompareTags_ModifiedAndMoved(t *testing.T) {
@@ -241,13 +221,11 @@ tags:
changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value) changes := CompareTags(lDoc.Tags.Value, rDoc.Tags.Value)
// evaluate. // evaluate.
assert.Len(t, changes.Changes, 2) assert.Len(t, changes.Changes, 1)
assert.Equal(t, 3, changes.TotalChanges()) assert.Equal(t, 1, changes.TotalChanges())
descChange := changes.Changes[1] descChange := changes.Changes[0]
assert.Equal(t, ModifiedAndMoved, descChange.ChangeType) assert.Equal(t, Modified, 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 lovelier tag description", descChange.Original)
assert.Equal(t, "a different tag description", descChange.New) assert.Equal(t, "a different tag description", descChange.New)
assert.True(t, descChange.Context.HasChanged()) assert.True(t, descChange.Context.HasChanged())