diff --git a/fieldpath/element.go b/fieldpath/element.go index 841558a6..c471241a 100644 --- a/fieldpath/element.go +++ b/fieldpath/element.go @@ -21,7 +21,7 @@ import ( "sort" "strings" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // PathElement describes how to select a child field given a containing object. diff --git a/fieldpath/element_test.go b/fieldpath/element_test.go index 34554817..9231136a 100644 --- a/fieldpath/element_test.go +++ b/fieldpath/element_test.go @@ -19,7 +19,7 @@ package fieldpath import ( "testing" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) func TestPathElementSet(t *testing.T) { diff --git a/fieldpath/fromvalue.go b/fieldpath/fromvalue.go index 3f9eaf63..c0e76fa4 100644 --- a/fieldpath/fromvalue.go +++ b/fieldpath/fromvalue.go @@ -17,7 +17,7 @@ limitations under the License. package fieldpath import ( - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // SetFromValue creates a set containing every leaf field mentioned in v. diff --git a/fieldpath/fromvalue_test.go b/fieldpath/fromvalue_test.go index 0c705e75..b89f1007 100644 --- a/fieldpath/fromvalue_test.go +++ b/fieldpath/fromvalue_test.go @@ -19,7 +19,7 @@ package fieldpath import ( "testing" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) func TestFromValue(t *testing.T) { diff --git a/fieldpath/managers.go b/fieldpath/managers.go index 25d5f0ad..95438e06 100644 --- a/fieldpath/managers.go +++ b/fieldpath/managers.go @@ -53,6 +53,37 @@ func (v versionedSet) Applied() bool { // what version). type ManagedFields map[string]VersionedSet +// Equals returns true if the two managedfields are the same, false +// otherwise. +func (lhs ManagedFields) Equals(rhs ManagedFields) bool { + if len(lhs) != len(rhs) { + return false + } + + for manager, left := range lhs { + right, ok := rhs[manager] + if !ok { + return false + } + if left.APIVersion() != right.APIVersion() || left.Applied() != right.Applied() { + return false + } + if !left.Set().Equals(right.Set()) { + return false + } + } + return true +} + +// Copy the list, this is mostly a shallow copy. +func (lhs ManagedFields) Copy() ManagedFields { + copy := ManagedFields{} + for manager, set := range lhs { + copy[manager] = set + } + return copy +} + // Difference returns a symmetric difference between two Managers. If a // given user's entry has version X in lhs and version Y in rhs, then // the return value for that user will be from rhs. If the difference for diff --git a/fieldpath/managers_test.go b/fieldpath/managers_test.go index 22619465..1884576d 100644 --- a/fieldpath/managers_test.go +++ b/fieldpath/managers_test.go @@ -21,7 +21,7 @@ import ( "reflect" "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" ) var ( @@ -30,7 +30,7 @@ var ( _P = fieldpath.MakePathOrDie ) -func TestManagersDifference(t *testing.T) { +func TestManagersEquals(t *testing.T) { tests := []struct { name string lhs fieldpath.ManagedFields @@ -164,3 +164,123 @@ func TestManagersDifference(t *testing.T) { }) } } + +func TestManagersDifference(t *testing.T) { + tests := []struct { + name string + lhs fieldpath.ManagedFields + rhs fieldpath.ManagedFields + equal bool + }{ + { + name: "Empty sets", + equal: true, + }, + { + name: "Same everything", + lhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v1", + false, + ), + }, + rhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v1", + false, + ), + }, + equal: true, + }, + { + name: "Empty RHS", + lhs: fieldpath.ManagedFields{ + "default": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v1", + false, + ), + }, + equal: false, + }, + { + name: "Empty LHS", + rhs: fieldpath.ManagedFields{ + "default": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v1", + false, + ), + }, + equal: false, + }, + { + name: "Different managers", + lhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v1", + false, + ), + }, + rhs: fieldpath.ManagedFields{ + "two": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v1", + false, + ), + }, + equal: false, + }, + { + name: "Same manager, different version", + lhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("integer")), + "v1", + false, + ), + }, + rhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string"), _P("bool")), + "v2", + false, + ), + }, + equal: false, + }, + { + name: "Set difference", + lhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("numeric"), _P("string")), + "v1", + false, + ), + }, + rhs: fieldpath.ManagedFields{ + "one": fieldpath.NewVersionedSet( + _NS(_P("string"), _P("bool")), + "v1", + false, + ), + }, + equal: false, + }, + } + + for _, test := range tests { + t.Run(fmt.Sprintf(test.name), func(t *testing.T) { + equal := test.lhs.Equals(test.rhs) + if test.equal && !equal { + difference := test.lhs.Difference(test.rhs) + t.Errorf("should be equal, but are different: %v", difference) + } else if !test.equal && equal { + t.Errorf("should not be equal, but they are") + } + }) + } +} diff --git a/fieldpath/path.go b/fieldpath/path.go index bdf6ce90..4ccf2a5e 100644 --- a/fieldpath/path.go +++ b/fieldpath/path.go @@ -20,7 +20,7 @@ import ( "fmt" "strings" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // Path describes how to select a potentially deeply-nested child field given a diff --git a/fieldpath/path_test.go b/fieldpath/path_test.go index 877ce7e6..5e7ffc29 100644 --- a/fieldpath/path_test.go +++ b/fieldpath/path_test.go @@ -19,7 +19,7 @@ package fieldpath import ( "testing" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) func TestPathString(t *testing.T) { diff --git a/fieldpath/pathelementmap.go b/fieldpath/pathelementmap.go index f35089a1..4625b274 100644 --- a/fieldpath/pathelementmap.go +++ b/fieldpath/pathelementmap.go @@ -19,7 +19,7 @@ package fieldpath import ( "sort" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // PathElementValueMap is a map from PathElement to value.Value. diff --git a/fieldpath/pathelementmap_test.go b/fieldpath/pathelementmap_test.go index 51fa72f3..b90e183b 100644 --- a/fieldpath/pathelementmap_test.go +++ b/fieldpath/pathelementmap_test.go @@ -19,7 +19,7 @@ package fieldpath import ( "testing" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) func TestPathElementValueMap(t *testing.T) { diff --git a/fieldpath/serialize-pe.go b/fieldpath/serialize-pe.go index e491fce1..3095177d 100644 --- a/fieldpath/serialize-pe.go +++ b/fieldpath/serialize-pe.go @@ -24,7 +24,7 @@ import ( "strings" jsoniter "github.com/json-iterator/go" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) var ErrUnknownPathElementType = errors.New("unknown path element type") diff --git a/fieldpath/set_test.go b/fieldpath/set_test.go index f5605063..0d1336d9 100644 --- a/fieldpath/set_test.go +++ b/fieldpath/set_test.go @@ -22,7 +22,7 @@ import ( "math/rand" "testing" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/value" ) type randomPathAlphabet []PathElement diff --git a/go.mod b/go.mod index 28f977d3..00ac0307 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module sigs.k8s.io/structured-merge-diff +module sigs.k8s.io/structured-merge-diff/v2 require gopkg.in/yaml.v2 v2.2.1 @@ -9,3 +9,5 @@ require ( github.com/modern-go/reflect2 v1.0.1 // indirect github.com/stretchr/testify v1.3.0 // indirect ) + +go 1.13 diff --git a/internal/cli/operation.go b/internal/cli/operation.go index 9f8052cb..f0049f5c 100644 --- a/internal/cli/operation.go +++ b/internal/cli/operation.go @@ -21,7 +21,7 @@ import ( "io" "io/ioutil" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) type Operation interface { diff --git a/internal/cli/options.go b/internal/cli/options.go index 3b2e9201..d371f86b 100644 --- a/internal/cli/options.go +++ b/internal/cli/options.go @@ -24,7 +24,7 @@ import ( "io/ioutil" "os" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var ( diff --git a/internal/fixture/state.go b/internal/fixture/state.go index 4bc96f31..0b1b0f35 100644 --- a/internal/fixture/state.go +++ b/internal/fixture/state.go @@ -20,9 +20,9 @@ import ( "bytes" "fmt" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) // State of the current test in terms of live object. One can check at @@ -121,9 +121,10 @@ func (s *State) ApplyObject(tv *typed.TypedValue, version fieldpath.APIVersion, if err != nil { return err } - s.Live = new s.Managers = managers - + if new != nil { + s.Live = new + } return nil } diff --git a/internal/fixture/state_test.go b/internal/fixture/state_test.go index 67323343..16a4889f 100644 --- a/internal/fixture/state_test.go +++ b/internal/fixture/state_test.go @@ -20,7 +20,7 @@ import ( "fmt" "testing" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) func TestFixTabs(t *testing.T) { diff --git a/merge/conflict.go b/merge/conflict.go index 34477f7d..5c271601 100644 --- a/merge/conflict.go +++ b/merge/conflict.go @@ -21,7 +21,7 @@ import ( "sort" "strings" - "sigs.k8s.io/structured-merge-diff/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" ) // Conflict is a conflict on a specific field with the current manager of diff --git a/merge/conflict_test.go b/merge/conflict_test.go index 925a8f06..c7f258db 100644 --- a/merge/conflict_test.go +++ b/merge/conflict_test.go @@ -19,9 +19,9 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/value" ) var ( diff --git a/merge/deduced_test.go b/merge/deduced_test.go index 0be622d9..5c8620f2 100644 --- a/merge/deduced_test.go +++ b/merge/deduced_test.go @@ -19,10 +19,10 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) func TestDeduced(t *testing.T) { diff --git a/merge/key_test.go b/merge/key_test.go index cef9e296..3f6a1b66 100644 --- a/merge/key_test.go +++ b/merge/key_test.go @@ -19,9 +19,9 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var associativeListParser = func() typed.ParseableType { diff --git a/merge/leaf_test.go b/merge/leaf_test.go index fc502b7f..134291ad 100644 --- a/merge/leaf_test.go +++ b/merge/leaf_test.go @@ -19,10 +19,10 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var leafFieldsParser = func() typed.ParseableType { diff --git a/merge/multiple_appliers_test.go b/merge/multiple_appliers_test.go index 588a650f..677d70a8 100644 --- a/merge/multiple_appliers_test.go +++ b/merge/multiple_appliers_test.go @@ -22,10 +22,10 @@ import ( "strings" "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) func TestMultipleAppliersSet(t *testing.T) { diff --git a/merge/nested_test.go b/merge/nested_test.go index 7a952dfe..f1f89660 100644 --- a/merge/nested_test.go +++ b/merge/nested_test.go @@ -19,9 +19,9 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var nestedTypeParser = func() typed.ParseableType { diff --git a/merge/obsolete_versions_test.go b/merge/obsolete_versions_test.go index 40e222d5..16101ec5 100644 --- a/merge/obsolete_versions_test.go +++ b/merge/obsolete_versions_test.go @@ -20,10 +20,10 @@ import ( "fmt" "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) // specificVersionConverter doesn't convert and return the exact same diff --git a/merge/preserve_unknown_test.go b/merge/preserve_unknown_test.go index 6259c6d0..ff7ac930 100644 --- a/merge/preserve_unknown_test.go +++ b/merge/preserve_unknown_test.go @@ -19,9 +19,9 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var preserveUnknownParser = func() typed.ParseableType { diff --git a/merge/real_test.go b/merge/real_test.go index 1124c223..7480cb4d 100644 --- a/merge/real_test.go +++ b/merge/real_test.go @@ -22,8 +22,8 @@ import ( "strings" "testing" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/typed" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) func testdata(file string) string { diff --git a/merge/set_test.go b/merge/set_test.go index d44b5645..de326997 100644 --- a/merge/set_test.go +++ b/merge/set_test.go @@ -19,9 +19,9 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var setFieldsParser = func() typed.ParseableType { diff --git a/merge/union_test.go b/merge/union_test.go index 1c933cc7..ce299865 100644 --- a/merge/union_test.go +++ b/merge/union_test.go @@ -19,10 +19,10 @@ package merge_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - . "sigs.k8s.io/structured-merge-diff/internal/fixture" - "sigs.k8s.io/structured-merge-diff/merge" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + . "sigs.k8s.io/structured-merge-diff/v2/internal/fixture" + "sigs.k8s.io/structured-merge-diff/v2/merge" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var unionFieldsParser = func() typed.ParseableType { diff --git a/merge/update.go b/merge/update.go index 75d59907..7065aeec 100644 --- a/merge/update.go +++ b/merge/update.go @@ -16,8 +16,8 @@ package merge import ( "fmt" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) // Converter is an interface to the conversion logic. The converter @@ -148,7 +148,8 @@ func (s *Updater) Update(liveObject, newObject *typed.TypedValue, version fieldp // Apply should be called when Apply is run, given the current object as // well as the configuration that is applied. This will merge the object -// and return it. +// and return it. If the object hasn't changed, nil is returned (the +// managers can still have changed though). func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fieldpath.APIVersion, managers fieldpath.ManagedFields, manager string, force bool) (*typed.TypedValue, fieldpath.ManagedFields, error) { managers = shallowCopyManagers(managers) var err error @@ -178,10 +179,13 @@ func (s *Updater) Apply(liveObject, configObject *typed.TypedValue, version fiel if err != nil { return nil, fieldpath.ManagedFields{}, fmt.Errorf("failed to prune fields: %v", err) } - managers, _, err = s.update(liveObject, newObject, version, managers, manager, force) + managers, compare, err := s.update(liveObject, newObject, version, managers, manager, force) if err != nil { return nil, fieldpath.ManagedFields{}, err } + if compare.IsSame() { + newObject = nil + } return newObject, managers, nil } diff --git a/schema/elements_test.go b/schema/elements_test.go index d6c46ed1..c4eca6ef 100644 --- a/schema/elements_test.go +++ b/schema/elements_test.go @@ -40,7 +40,7 @@ func TestFindNamedType(t *testing.T) { Types: tt.defs, } td, exist := s.FindNamedType(tt.namedType) - if !td.Equals(tt.expectTypeDef) { + if !td.Equals(&tt.expectTypeDef) { t.Errorf("expected TypeDef %v, got %v", tt.expectTypeDef, td) } if exist != tt.expectExist { diff --git a/schema/equals.go b/schema/equals.go index 271aed3c..464d47de 100644 --- a/schema/equals.go +++ b/schema/equals.go @@ -17,12 +17,16 @@ limitations under the License. package schema // Equals returns true iff the two Schemas are equal. -func (a Schema) Equals(b Schema) bool { +func (a *Schema) Equals(b *Schema) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + if len(a.Types) != len(b.Types) { return false } for i := range a.Types { - if !a.Types[i].Equals(b.Types[i]) { + if !a.Types[i].Equals(&b.Types[i]) { return false } } @@ -33,7 +37,10 @@ func (a Schema) Equals(b Schema) bool { // // Note that two typerefs that have an equivalent type but where one is // inlined and the other is named, are not considered equal. -func (a TypeRef) Equals(b TypeRef) bool { +func (a *TypeRef) Equals(b *TypeRef) bool { + if a == nil || b == nil { + return a == nil && b == nil + } if (a.NamedType == nil) != (b.NamedType == nil) { return false } @@ -43,19 +50,25 @@ func (a TypeRef) Equals(b TypeRef) bool { } //return true } - return a.Inlined.Equals(b.Inlined) + return a.Inlined.Equals(&b.Inlined) } // Equals returns true iff the two TypeDefs are equal. -func (a TypeDef) Equals(b TypeDef) bool { +func (a *TypeDef) Equals(b *TypeDef) bool { + if a == nil || b == nil { + return a == nil && b == nil + } if a.Name != b.Name { return false } - return a.Atom.Equals(b.Atom) + return a.Atom.Equals(&b.Atom) } // Equals returns true iff the two Atoms are equal. -func (a Atom) Equals(b Atom) bool { +func (a *Atom) Equals(b *Atom) bool { + if a == nil || b == nil { + return a == nil && b == nil + } if (a.Scalar == nil) != (b.Scalar == nil) { return false } @@ -69,16 +82,19 @@ func (a Atom) Equals(b Atom) bool { case a.Scalar != nil: return *a.Scalar == *b.Scalar case a.List != nil: - return a.List.Equals(*b.List) + return a.List.Equals(b.List) case a.Map != nil: - return a.Map.Equals(*b.Map) + return a.Map.Equals(b.Map) } return true } // Equals returns true iff the two Maps are equal. -func (a Map) Equals(b Map) bool { - if !a.ElementType.Equals(b.ElementType) { +func (a *Map) Equals(b *Map) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + if !a.ElementType.Equals(&b.ElementType) { return false } if a.ElementRelationship != b.ElementRelationship { @@ -88,7 +104,7 @@ func (a Map) Equals(b Map) bool { return false } for i := range a.Fields { - if !a.Fields[i].Equals(b.Fields[i]) { + if !a.Fields[i].Equals(&b.Fields[i]) { return false } } @@ -96,7 +112,7 @@ func (a Map) Equals(b Map) bool { return false } for i := range a.Unions { - if !a.Unions[i].Equals(b.Unions[i]) { + if !a.Unions[i].Equals(&b.Unions[i]) { return false } } @@ -104,7 +120,10 @@ func (a Map) Equals(b Map) bool { } // Equals returns true iff the two Unions are equal. -func (a Union) Equals(b Union) bool { +func (a *Union) Equals(b *Union) bool { + if a == nil || b == nil { + return a == nil && b == nil + } if (a.Discriminator == nil) != (b.Discriminator == nil) { return false } @@ -120,7 +139,7 @@ func (a Union) Equals(b Union) bool { return false } for i := range a.Fields { - if !a.Fields[i].Equals(b.Fields[i]) { + if !a.Fields[i].Equals(&b.Fields[i]) { return false } } @@ -128,7 +147,10 @@ func (a Union) Equals(b Union) bool { } // Equals returns true iff the two UnionFields are equal. -func (a UnionField) Equals(b UnionField) bool { +func (a *UnionField) Equals(b *UnionField) bool { + if a == nil || b == nil { + return a == nil && b == nil + } if a.FieldName != b.FieldName { return false } @@ -139,16 +161,22 @@ func (a UnionField) Equals(b UnionField) bool { } // Equals returns true iff the two StructFields are equal. -func (a StructField) Equals(b StructField) bool { +func (a *StructField) Equals(b *StructField) bool { + if a == nil || b == nil { + return a == nil && b == nil + } if a.Name != b.Name { return false } - return a.Type.Equals(b.Type) + return a.Type.Equals(&b.Type) } // Equals returns true iff the two Lists are equal. -func (a List) Equals(b List) bool { - if !a.ElementType.Equals(b.ElementType) { +func (a *List) Equals(b *List) bool { + if a == nil || b == nil { + return a == nil && b == nil + } + if !a.ElementType.Equals(&b.ElementType) { return false } if a.ElementRelationship != b.ElementRelationship { diff --git a/schema/equals_test.go b/schema/equals_test.go index 49f3fff6..a496ea3b 100644 --- a/schema/equals_test.go +++ b/schema/equals_test.go @@ -61,43 +61,43 @@ func TestEquals(t *testing.T) { // add new fields without fixing the Equals function and this test. funcs := []interface{}{ func(x Schema) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y Schema y.Types = x.Types - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(&x, &y) }, func(x TypeDef) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y TypeDef y.Name = x.Name y.Atom = x.Atom - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x TypeRef) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y TypeRef y.NamedType = x.NamedType y.Inlined = x.Inlined - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x Atom) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y Atom y.Scalar = x.Scalar y.List = x.List y.Map = x.Map - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x Map) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y Map @@ -105,45 +105,45 @@ func TestEquals(t *testing.T) { y.ElementRelationship = x.ElementRelationship y.Fields = x.Fields y.Unions = x.Unions - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(&x, &y) }, func(x Union) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y Union y.Discriminator = x.Discriminator y.DeduceInvalidDiscriminator = x.DeduceInvalidDiscriminator y.Fields = x.Fields - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x UnionField) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y UnionField y.DiscriminatorValue = x.DiscriminatorValue y.FieldName = x.FieldName - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x StructField) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y StructField y.Name = x.Name y.Type = x.Type - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, func(x List) bool { - if !x.Equals(x) { + if !x.Equals(&x) { return false } var y List y.ElementType = x.ElementType y.ElementRelationship = x.ElementRelationship y.Keys = x.Keys - return x.Equals(y) == reflect.DeepEqual(x, y) + return x.Equals(&y) == reflect.DeepEqual(x, y) }, } for i, f := range funcs { diff --git a/smd/main.go b/smd/main.go index 611dbeb9..2d2dd257 100644 --- a/smd/main.go +++ b/smd/main.go @@ -22,7 +22,7 @@ import ( "flag" "log" - "sigs.k8s.io/structured-merge-diff/internal/cli" + "sigs.k8s.io/structured-merge-diff/v2/internal/cli" ) func main() { diff --git a/typed/deduced_test.go b/typed/deduced_test.go index 1c06d92b..77b44349 100644 --- a/typed/deduced_test.go +++ b/typed/deduced_test.go @@ -21,7 +21,7 @@ import ( "reflect" "testing" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) func TestValidateDeducedType(t *testing.T) { diff --git a/typed/helpers.go b/typed/helpers.go index 0911ce0b..8388135b 100644 --- a/typed/helpers.go +++ b/typed/helpers.go @@ -21,9 +21,9 @@ import ( "fmt" "strings" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // ValidationError reports an error about a particular field @@ -114,17 +114,27 @@ func resolveSchema(s *schema.Schema, tr schema.TypeRef, v *value.Value, ah atomH return handleAtom(a, tr, ah) } -func deduceAtom(a schema.Atom, v *value.Value) schema.Atom { +// deduceAtom determines which of the possible types in atom 'atom' applies to value 'val'. +// If val is of a type allowed by atom, return a copy of atom with all other types set to nil. +// if val is nil, or is not of a type allowed by atom, just return the original atom, +// and validation will fail at a later stage. (with a more useful error) +func deduceAtom(atom schema.Atom, val *value.Value) schema.Atom { switch { - case v == nil: - case v.FloatValue != nil, v.IntValue != nil, v.StringValue != nil, v.BooleanValue != nil: - return schema.Atom{Scalar: a.Scalar} - case v.ListValue != nil: - return schema.Atom{List: a.List} - case v.MapValue != nil: - return schema.Atom{Map: a.Map} + case val == nil: + case val.FloatValue != nil, val.IntValue != nil, val.StringValue != nil, val.BooleanValue != nil: + if atom.Scalar != nil { + return schema.Atom{Scalar: atom.Scalar} + } + case val.ListValue != nil: + if atom.List != nil { + return schema.Atom{List: atom.List} + } + case val.MapValue != nil: + if atom.Map != nil { + return schema.Atom{Map: atom.Map} + } } - return a + return atom } func handleAtom(a schema.Atom, tr schema.TypeRef, ah atomHandler) ValidationErrors { diff --git a/typed/merge.go b/typed/merge.go index bb92f84b..2584b4f3 100644 --- a/typed/merge.go +++ b/typed/merge.go @@ -17,9 +17,9 @@ limitations under the License. package typed import ( - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) type mergingWalker struct { @@ -76,7 +76,7 @@ func (w *mergingWalker) merge() (errs ValidationErrors) { alhs := deduceAtom(a, w.lhs) arhs := deduceAtom(a, w.rhs) - if alhs.Equals(arhs) { + if alhs.Equals(&arhs) { errs = append(errs, handleAtom(arhs, w.typeRef, w)...) } else { w2 := *w diff --git a/typed/merge_test.go b/typed/merge_test.go index c83c5619..2745c58e 100644 --- a/typed/merge_test.go +++ b/typed/merge_test.go @@ -21,7 +21,7 @@ import ( "reflect" "testing" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) type mergeTestCase struct { diff --git a/typed/parser.go b/typed/parser.go index 2e36857b..5ade977e 100644 --- a/typed/parser.go +++ b/typed/parser.go @@ -20,8 +20,8 @@ import ( "fmt" yaml "gopkg.in/yaml.v2" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // YAMLObject is an object encoded in YAML. diff --git a/typed/parser_test.go b/typed/parser_test.go index 89fbd183..f8ba90e3 100644 --- a/typed/parser_test.go +++ b/typed/parser_test.go @@ -23,7 +23,7 @@ import ( "testing" yaml "gopkg.in/yaml.v2" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) func testdata(file string) string { diff --git a/typed/remove.go b/typed/remove.go index a6b74841..64fe8648 100644 --- a/typed/remove.go +++ b/typed/remove.go @@ -14,9 +14,9 @@ limitations under the License. package typed import ( - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) type removingWalker struct { diff --git a/typed/symdiff_test.go b/typed/symdiff_test.go index 4a7841fe..5d3fd48a 100644 --- a/typed/symdiff_test.go +++ b/typed/symdiff_test.go @@ -20,8 +20,8 @@ import ( "fmt" "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) type symdiffTestCase struct { @@ -299,6 +299,244 @@ var symdiffCases = []symdiffTestCase{{ modified: _NS(), added: _NS(_P("a", "b")), }}, +}, { + name: "untyped deduced", + rootTypeName: "__untyped_deduced_", + schema: `types: +- name: __untyped_atomic_ + scalar: untyped + list: + elementType: + namedType: __untyped_atomic_ + elementRelationship: atomic + map: + elementType: + namedType: __untyped_atomic_ + elementRelationship: atomic +- name: __untyped_deduced_ + scalar: untyped + list: + elementType: + namedType: __untyped_atomic_ + elementRelationship: atomic + map: + elementType: + namedType: __untyped_deduced_ + elementRelationship: separable +`, + quints: []symdiffQuint{{ + lhs: `{"a":{}}}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":null}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":{}}}`, + removed: _NS(_P("a", "b")), + modified: _NS(), + added: _NS(), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":null}`, + removed: _NS(_P("a", "b")), + modified: _NS(), + added: _NS(), + }, { + lhs: `{"a":[]}`, + rhs: `{"a":["b"]}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":null}`, + rhs: `{"a":["b"]}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":["b"]}`, + rhs: `{"a":[]}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":["b"]}`, + rhs: `{"a":null}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":null}`, + rhs: `{"a":"b"}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":"b"}`, + rhs: `{"a":null}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":["b"]}}`, + removed: _NS(_P("a", "b")), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":["b"]}}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":"b"}`, + removed: _NS(_P("a", "b")), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":"b"}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":["b"]}}`, + rhs: `{"a":"b"}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":"b"}`, + rhs: `{"a":["b"]}}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }}, +}, { + name: "untyped separable", + rootTypeName: "__untyped_separable_", + schema: `types: +- name: __untyped_separable_ + scalar: untyped + list: + elementType: + namedType: __untyped_separable_ + elementRelationship: associative + map: + elementType: + namedType: __untyped_separable_ + elementRelationship: separable +`, + quints: []symdiffQuint{{ + lhs: `{"a":{}}}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":null}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":{}}}`, + removed: _NS(_P("a", "b")), + modified: _NS(), + added: _NS(), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":null}`, + removed: _NS(_P("a", "b")), + modified: _NS(), + added: _NS(), + }, { + lhs: `{"a":[]}`, + rhs: `{"a":["b"]}`, + removed: _NS(), + modified: _NS(), + added: _NS(_P("a", _SV("b"))), + }, { + lhs: `{"a":null}`, + rhs: `{"a":["b"]}`, + removed: _NS(), + // TODO: result should be the same as the previous case + // nothing shoule be modified here. + modified: _NS(_P("a")), + added: _NS(_P("a", _SV("b"))), + }, { + lhs: `{"a":["b"]}`, + rhs: `{"a":[]}`, + removed: _NS(_P("a", _SV("b"))), + modified: _NS(), + added: _NS(), + }, { + lhs: `{"a":["b"]}`, + rhs: `{"a":null}`, + removed: _NS(_P("a", _SV("b"))), + // TODO: result should be the same as the previous case + // nothing shoule be modified here. + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":null}`, + rhs: `{"a":"b"}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":"b"}`, + rhs: `{"a":null}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":["b"]}}`, + removed: _NS(_P("a", "b")), + modified: _NS(), + added: _NS(_P("a", _SV("b"))), + }, { + lhs: `{"a":["b"]}}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(_P("a", _SV("b"))), + modified: _NS(), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":{"b":{}}}`, + rhs: `{"a":"b"}`, + removed: _NS(_P("a", "b")), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":"b"}`, + rhs: `{"a":{"b":{}}}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(_P("a", "b")), + }, { + lhs: `{"a":["b"]}}`, + rhs: `{"a":"b"}`, + removed: _NS(_P("a", _SV("b"))), + modified: _NS(_P("a")), + added: _NS(), + }, { + lhs: `{"a":"b"}`, + rhs: `{"a":["b"]}}`, + removed: _NS(), + modified: _NS(_P("a")), + added: _NS(_P("a", _SV("b"))), + }}, }, { name: "struct grab bag", rootTypeName: "myStruct", diff --git a/typed/toset_test.go b/typed/toset_test.go index d565b323..2222485f 100644 --- a/typed/toset_test.go +++ b/typed/toset_test.go @@ -20,9 +20,9 @@ import ( "fmt" "testing" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/typed" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/typed" + "sigs.k8s.io/structured-merge-diff/v2/value" ) type objSetPair struct { diff --git a/typed/typed.go b/typed/typed.go index 9c61b845..51af1e9a 100644 --- a/typed/typed.go +++ b/typed/typed.go @@ -21,9 +21,9 @@ import ( "strings" "sync" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) // AsTyped accepts a value and a type and returns a TypedValue. 'v' must have @@ -219,7 +219,7 @@ func merge(lhs, rhs *TypedValue, rule, postRule mergeRule) (*TypedValue, error) return nil, errorFormatter{}. errorf("expected objects with types from the same schema") } - if !lhs.typeRef.Equals(rhs.typeRef) { + if !lhs.typeRef.Equals(&rhs.typeRef) { return nil, errorFormatter{}. errorf("expected objects of the same type, but got %v and %v", lhs.typeRef, rhs.typeRef) } diff --git a/typed/union.go b/typed/union.go index c4a012ee..838b39f6 100644 --- a/typed/union.go +++ b/typed/union.go @@ -20,8 +20,8 @@ import ( "fmt" "strings" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) func normalizeUnions(w *mergingWalker) error { diff --git a/typed/union_test.go b/typed/union_test.go index dc621b6c..e862dff6 100644 --- a/typed/union_test.go +++ b/typed/union_test.go @@ -19,7 +19,7 @@ package typed_test import ( "testing" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) var unionParser = func() typed.ParseableType { diff --git a/typed/validate.go b/typed/validate.go index 338c0583..a581fc7e 100644 --- a/typed/validate.go +++ b/typed/validate.go @@ -19,9 +19,9 @@ package typed import ( "sync" - "sigs.k8s.io/structured-merge-diff/fieldpath" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/value" + "sigs.k8s.io/structured-merge-diff/v2/fieldpath" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/value" ) var vPool = sync.Pool{ @@ -198,7 +198,11 @@ func (v *validatingObjectWalker) visitMapItems(t *schema.Map, m *value.Map) (err } else { v2 := v.prepareDescent(pe, t.ElementType) v2.value = item.Value - errs = append(errs, v2.validate()...) + if (t.ElementType == schema.TypeRef{}) { + errs = append(errs, v2.errorf("field not declared in schema")...) + } else { + errs = append(errs, v2.validate()...) + } v2.doNode() v.finishDescent(v2) } diff --git a/typed/validate_test.go b/typed/validate_test.go index d720ebd7..d7a21b8c 100644 --- a/typed/validate_test.go +++ b/typed/validate_test.go @@ -18,10 +18,11 @@ package typed_test import ( "fmt" + "strings" "testing" - "sigs.k8s.io/structured-merge-diff/schema" - "sigs.k8s.io/structured-merge-diff/typed" + "sigs.k8s.io/structured-merge-diff/v2/schema" + "sigs.k8s.io/structured-merge-diff/v2/typed" ) type validationTestCase struct { @@ -216,6 +217,8 @@ var validationCases = []validationTestCase{{ invalidObjects: []typed.YAMLObject{ `{"key":true,"value":1}`, `{"list":{"key":true,"value":1}}`, + `{"list":true}`, + `true`, `{"list":[{"key":true,"value":1}]}`, `{"list":[{"key":[],"value":1}]}`, `{"list":[{"key":{},"value":1}]}`, @@ -261,6 +264,9 @@ func (tt validationTestCase) test(t *testing.T) { if err == nil { t.Errorf("Object should fail: %v\n%v", err, iv) } + if strings.Contains(err.Error(), "invalid atom") { + t.Errorf("Error should be useful, but got: %v\n%v", err, iv) + } }) } }