diff --git a/typed/helpers.go b/typed/helpers.go index a7c12ccb..3897de28 100644 --- a/typed/helpers.go +++ b/typed/helpers.go @@ -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/symdiff_test.go b/typed/symdiff_test.go index 77b2636f..d3da4ad1 100644 --- a/typed/symdiff_test.go +++ b/typed/symdiff_test.go @@ -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/validate.go b/typed/validate.go index 0a763247..c33bfc3c 100644 --- a/typed/validate.go +++ b/typed/validate.go @@ -207,7 +207,11 @@ func (v *validatingObjectWalker) visitMapItems(t schema.Map, m *value.Map) (errs } 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..a711c1c4 100644 --- a/typed/validate_test.go +++ b/typed/validate_test.go @@ -18,6 +18,7 @@ package typed_test import ( "fmt" + "strings" "testing" "sigs.k8s.io/structured-merge-diff/schema" @@ -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) + } }) } } diff --git a/value/less_test.go b/value/less_test.go index b0b15f37..7f568e7f 100644 --- a/value/less_test.go +++ b/value/less_test.go @@ -296,6 +296,9 @@ func TestValueLess(t *testing.T) { t.Run(table[i].name, func(t *testing.T) { tt := table[i] if tt.eq { + if !tt.a.Equals(tt.b) { + t.Errorf("oops, a != b: %#v, %#v", tt.a, tt.b) + } if tt.a.Less(tt.b) { t.Errorf("oops, a < b: %#v, %#v", tt.a, tt.b) } @@ -307,6 +310,19 @@ func TestValueLess(t *testing.T) { if tt.b.Less(tt.b) { t.Errorf("oops, b < a: %#v, %#v", tt.b, tt.a) } + + if tt.eq { + if tt.a.Compare(tt.b) != 0 || tt.b.Compare(tt.b) != 0 { + t.Errorf("oops, a != b: %#v, %#v", tt.a, tt.b) + } + } else { + if !(tt.a.Compare(tt.b) < 0) { + t.Errorf("oops, a is not less than b: %#v, %#v", tt.a, tt.b) + } + if !(tt.b.Compare(tt.a) > 0) { + t.Errorf("oops, b is not more than a: %#v, %#v", tt.a, tt.b) + } + } }) } diff --git a/value/value.go b/value/value.go index 1ce63e1c..91d73d0f 100644 --- a/value/value.go +++ b/value/value.go @@ -36,86 +36,146 @@ type Value struct { // Equals returns true iff the two values are equal. func (v Value) Equals(rhs Value) bool { - return !v.Less(rhs) && !rhs.Less(v) + if v.FloatValue != nil || rhs.FloatValue != nil { + var lf float64 + if v.FloatValue != nil { + lf = float64(*v.FloatValue) + } else if v.IntValue != nil { + lf = float64(*v.IntValue) + } else { + return false + } + var rf float64 + if rhs.FloatValue != nil { + rf = float64(*rhs.FloatValue) + } else if rhs.IntValue != nil { + rf = float64(*rhs.IntValue) + } else { + return false + } + return lf == rf + } + if v.IntValue != nil { + if rhs.IntValue != nil { + return *v.IntValue == *rhs.IntValue + } + return false + } + if v.StringValue != nil { + if rhs.StringValue != nil { + return *v.StringValue == *rhs.StringValue + } + return false + } + if v.BooleanValue != nil { + if rhs.BooleanValue != nil { + return *v.BooleanValue == *rhs.BooleanValue + } + return false + } + if v.ListValue != nil { + if rhs.ListValue != nil { + return v.ListValue.Equals(rhs.ListValue) + } + return false + } + if v.MapValue != nil { + if rhs.MapValue != nil { + return v.MapValue.Equals(rhs.MapValue) + } + return false + } + if v.Null { + if rhs.Null { + return true + } + return false + } + // No field is set, on either objects. + return true } // Less provides a total ordering for Value (so that they can be sorted, even // if they are of different types). func (v Value) Less(rhs Value) bool { + return v.Compare(rhs) == -1 +} + +// Compare provides a total ordering for Value (so that they can be +// sorted, even if they are of different types). The result will be 0 if +// v==rhs, -1 if v < rhs, and +1 if v > rhs. +func (v Value) Compare(rhs Value) int { if v.FloatValue != nil { if rhs.FloatValue == nil { // Extra: compare floats and ints numerically. if rhs.IntValue != nil { - return float64(*v.FloatValue) < float64(*rhs.IntValue) + return v.FloatValue.Compare(Float(*rhs.IntValue)) } - return true + return -1 } - return *v.FloatValue < *rhs.FloatValue + return v.FloatValue.Compare(*rhs.FloatValue) } else if rhs.FloatValue != nil { // Extra: compare floats and ints numerically. if v.IntValue != nil { - return float64(*v.IntValue) < float64(*rhs.FloatValue) + return Float(*v.IntValue).Compare(*rhs.FloatValue) } - return false + return 1 } if v.IntValue != nil { if rhs.IntValue == nil { - return true + return -1 } - return *v.IntValue < *rhs.IntValue + return v.IntValue.Compare(*rhs.IntValue) } else if rhs.IntValue != nil { - return false + return 1 } if v.StringValue != nil { if rhs.StringValue == nil { - return true + return -1 } - return *v.StringValue < *rhs.StringValue + return strings.Compare(string(*v.StringValue), string(*rhs.StringValue)) } else if rhs.StringValue != nil { - return false + return 1 } if v.BooleanValue != nil { if rhs.BooleanValue == nil { - return true + return -1 } - if *v.BooleanValue == *rhs.BooleanValue { - return false - } - return *v.BooleanValue == false + return v.BooleanValue.Compare(*rhs.BooleanValue) } else if rhs.BooleanValue != nil { - return false + return 1 } if v.ListValue != nil { if rhs.ListValue == nil { - return true + return -1 } - return v.ListValue.Less(rhs.ListValue) + return v.ListValue.Compare(rhs.ListValue) } else if rhs.ListValue != nil { - return false + return 1 } if v.MapValue != nil { if rhs.MapValue == nil { - return true + return -1 } - return v.MapValue.Less(rhs.MapValue) + return v.MapValue.Compare(rhs.MapValue) } else if rhs.MapValue != nil { - return false + return 1 } if v.Null { if !rhs.Null { - return true + return -1 } - return false + return 0 } else if rhs.Null { - return false + return 1 } // Invalid Value-- nothing is set. - return false + return 0 } type Int int64 @@ -123,6 +183,39 @@ type Float float64 type String string type Boolean bool +// Compare compares integers. The result will be 0 if i==rhs, -1 if i < +// rhs, and +1 if i > rhs. +func (i Int) Compare(rhs Int) int { + if i > rhs { + return 1 + } else if i < rhs { + return -1 + } + return 0 +} + +// Compare compares floats. The result will be 0 if f==rhs, -1 if f < +// rhs, and +1 if f > rhs. +func (f Float) Compare(rhs Float) int { + if f > rhs { + return 1 + } else if f < rhs { + return -1 + } + return 0 +} + +// Compare compares booleans. The result will be 0 if b==rhs, -1 if b < +// rhs, and +1 if b > rhs. +func (b Boolean) Compare(rhs Boolean) int { + if b == rhs { + return 0 + } else if b == false { + return -1 + } + return 1 +} + // Field is an individual key-value pair. type Field struct { Name string @@ -134,29 +227,44 @@ type List struct { Items []Value } +// Equals compares two lists lexically. +func (l *List) Equals(rhs *List) bool { + if len(l.Items) != len(rhs.Items) { + return false + } + + for i, lv := range l.Items { + if !lv.Equals(rhs.Items[i]) { + return false + } + } + return true +} + // Less compares two lists lexically. func (l *List) Less(rhs *List) bool { + return l.Compare(rhs) == -1 +} + +// Compare compares two lists lexically. The result will be 0 if l==rhs, -1 +// if l < rhs, and +1 if l > rhs. +func (l *List) Compare(rhs *List) int { i := 0 for { if i >= len(l.Items) && i >= len(rhs.Items) { // Lists are the same length and all items are equal. - return false + return 0 } if i >= len(l.Items) { // LHS is shorter. - return true + return -1 } if i >= len(rhs.Items) { // RHS is shorter. - return false + return 1 } - if l.Items[i].Less(rhs.Items[i]) { - // LHS is less; return - return true - } - if rhs.Items[i].Less(l.Items[i]) { - // RHS is less; return - return false + if c := l.Items[i].Compare(rhs.Items[i]); c != 0 { + return c } // The items are equal; continue. i++ @@ -191,8 +299,30 @@ func (m *Map) computeOrder() []int { return m.order } +// Equals compares two maps lexically. +func (m *Map) Equals(rhs *Map) bool { + if len(m.Items) != len(rhs.Items) { + return false + } + for _, lfield := range m.Items { + rfield, ok := rhs.Get(lfield.Name) + if !ok { + return false + } + if !lfield.Value.Equals(rfield.Value) { + return false + } + } + return true +} + // Less compares two maps lexically. func (m *Map) Less(rhs *Map) bool { + return m.Compare(rhs) == -1 +} + +// Compare compares two maps lexically. +func (m *Map) Compare(rhs *Map) int { var noAllocL, noAllocR [2]int var morder, rorder []int @@ -238,28 +368,22 @@ func (m *Map) Less(rhs *Map) bool { for { if i >= len(morder) && i >= len(rorder) { // Maps are the same length and all items are equal. - return false + return 0 } if i >= len(morder) { // LHS is shorter. - return true + return -1 } if i >= len(rorder) { // RHS is shorter. - return false + return 1 } fa, fb := &m.Items[morder[i]], &rhs.Items[rorder[i]] - if fa.Name != fb.Name { - // the map having the field name that sorts lexically less is "less" - return fa.Name < fb.Name + if c := strings.Compare(fa.Name, fb.Name); c != 0 { + return c } - if fa.Value.Less(fb.Value) { - // LHS is less; return - return true - } - if fb.Value.Less(fa.Value) { - // RHS is less; return - return false + if c := fa.Value.Compare(fb.Value); c != 0 { + return c } // The items are equal; continue. i++