diff --git a/ygot/struct_validation_map.go b/ygot/struct_validation_map.go index 96f897b6e..d3e54b978 100644 --- a/ygot/struct_validation_map.go +++ b/ygot/struct_validation_map.go @@ -502,6 +502,9 @@ func MergeStructInto(dst, src ValidatedGoStruct) error { // DeepCopy returns a deep copy of the supplied GoStruct. A new copy // of the GoStruct is created, along with any underlying values. func DeepCopy(s GoStruct) (GoStruct, error) { + if util.IsNilOrInvalidValue(reflect.ValueOf(s)) { + return nil, fmt.Errorf("invalid input to DeepCopy, got nil value: %v", s) + } n := reflect.New(reflect.TypeOf(s).Elem()) if err := copyStruct(n.Elem(), reflect.ValueOf(s).Elem()); err != nil { return nil, fmt.Errorf("cannot DeepCopy struct: %v", err) @@ -540,6 +543,17 @@ func copyStruct(dstVal, srcVal reflect.Value) error { if err := copySliceField(dstField, srcField); err != nil { return err } + case reflect.Int64: + // In the case of an int64 field, which represents a YANG enumeration + // we should only set the value in the destination if it is not set + // to the default value in the source. + vSrc, vDst := srcField.Int(), dstField.Int() + switch { + case vSrc != 0 && vDst != 0 && vSrc != vDst: + return fmt.Errorf("destination and source values were set when merging enum field, dst: %d, src: %d", vSrc, vDst) + case vSrc != 0 && vDst == 0: + dstField.Set(srcField) + } default: dstField.Set(srcField) } @@ -600,7 +614,7 @@ func copyPtrField(dstField, srcField reflect.Value) error { // copyInterfaceField copies srcField into dstField. Both srcField and dstField // are reflect.Value structs which contain an interface value. func copyInterfaceField(dstField, srcField reflect.Value) error { - if srcField.IsNil() || !srcField.IsValid() { + if util.IsNilOrInvalidValue(srcField) { return nil } @@ -608,7 +622,14 @@ func copyInterfaceField(dstField, srcField reflect.Value) error { return fmt.Errorf("invalid interface type received: %T", srcField.Interface()) } - s := srcField.Elem().Elem() // Dereference to a struct. + s := srcField.Elem().Elem() // Dereference src to a struct. + if !util.IsNilOrInvalidValue(dstField) { + dV := dstField.Elem().Elem() // Dereference dst to a struct. + if !reflect.DeepEqual(s.Interface(), dV.Interface()) { + return fmt.Errorf("interface field was set in both src and dst and was not equal, src: %v, dst: %v", s.Interface(), dV.Interface()) + } + } + var d reflect.Value d = reflect.New(s.Type()) if err := copyStruct(d.Elem(), s); err != nil { diff --git a/ygot/struct_validation_map_test.go b/ygot/struct_validation_map_test.go index 50e8cb7cb..f188eda83 100644 --- a/ygot/struct_validation_map_test.go +++ b/ygot/struct_validation_map_test.go @@ -1152,7 +1152,8 @@ func TestMergeStructJSON(t *testing.T) { type enumType int64 const ( - EnumTypeValue enumType = 1 + EnumTypeValue enumType = 1 + EnumTypeValueTwo enumType = 2 ) type copyUnion interface { @@ -1165,6 +1166,12 @@ type copyUnionS struct { func (*copyUnionS) IsUnion() {} +type copyUnionI struct { + I int64 +} + +func (*copyUnionI) IsUnion() {} + type copyMapKey struct { A string } @@ -1545,6 +1552,8 @@ type validatedMergeTest struct { String *string StringTwo *string Uint32Field *uint32 + EnumValue enumType + UnionField copyUnion } func (*validatedMergeTest) Validate(...ValidationOption) error { return nil } @@ -1618,6 +1627,44 @@ var mergeStructTests = []struct { StringTwo: String("new-belgium-lips-of-faith-la-folie"), Uint32Field: Uint32(42), }, +}, { + name: "enum merge: set in a, and not b", + inA: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, + inB: &validatedMergeTest{}, + want: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, +}, { + name: "enum merge: set in b and not a", + inA: &validatedMergeTest{}, + inB: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, + want: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, +}, { + name: "enum merge: set to same value in both", + inA: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, + inB: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, + want: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, +}, { + name: "enum merge: set to different values in both", + inA: &validatedMergeTest{ + EnumValue: EnumTypeValueTwo, + }, + inB: &validatedMergeTest{ + EnumValue: EnumTypeValue, + }, + wantErr: "destination and source values were set when merging enum field", }, { name: "error, differing types", inA: &validatedMergeTest{String: String("great-divide-yeti")}, @@ -1677,6 +1724,53 @@ var mergeStructTests = []struct { SliceField: []*validatedMergeTestSliceField{{String("chinook-single-hop")}}, }, wantErr: "source and destination lists must be unique", +}, { + name: "merge union: values not equal", + inA: &validatedMergeTest{ + UnionField: ©UnionS{"glutenberg-ipa"}, + }, + inB: &validatedMergeTest{ + UnionField: ©UnionS{"mikkeler-pale-peter-and-mary"}, + }, + wantErr: "interface field was set in both src and dst and was not equal", +}, { + name: "merge union: values equal", + inA: &validatedMergeTest{ + UnionField: ©UnionS{"ipswich-ale-celia-saison"}, + }, + inB: &validatedMergeTest{ + UnionField: ©UnionS{"ipswich-ale-celia-saison"}, + }, + want: &validatedMergeTest{ + UnionField: ©UnionS{"ipswich-ale-celia-saison"}, + }, +}, { + name: "merge union: set in src and not dst", + inA: &validatedMergeTest{ + UnionField: ©UnionS{"estrella-damn-daura"}, + }, + inB: &validatedMergeTest{}, + want: &validatedMergeTest{ + UnionField: ©UnionS{"estrella-damn-daura"}, + }, +}, { + name: "merge union: set in dst and not src", + inA: &validatedMergeTest{}, + inB: &validatedMergeTest{ + UnionField: ©UnionS{"two-brothers-prairie-path-golden-ale"}, + }, + want: &validatedMergeTest{ + UnionField: ©UnionS{"two-brothers-prairie-path-golden-ale"}, + }, +}, { + name: "merge union: values not equal, and different types", + inA: &validatedMergeTest{ + UnionField: ©UnionS{"greens-amber"}, + }, + inB: &validatedMergeTest{ + UnionField: ©UnionI{42}, + }, + wantErr: "interface field was set in both src and dst and was not equal", }} func TestMergeStructs(t *testing.T) { @@ -1822,10 +1916,10 @@ func TestCopyErrorCases(t *testing.T) { func TestDeepCopy(t *testing.T) { tests := []struct { - name string - in *copyTest - inKey string - wantErr bool + name string + in *copyTest + inKey string + wantErrSubstring string }{{ name: "simple copy", in: ©Test{StringField: String("zaphod")}, @@ -1842,13 +1936,21 @@ func TestDeepCopy(t *testing.T) { in: ©Test{ StringSlice: []string{"one"}, }, + }, { + name: "nil inputs", + wantErrSubstring: "got nil value", }} for _, tt := range tests { got, err := DeepCopy(tt.in) - if (err != nil) != tt.wantErr { - t.Errorf("%s: DeepCopy(%#v): did not get expected error, got: %v, wantErr: %v", tt.name, tt.in, err, tt.wantErr) + + if err != nil { + if diff := errdiff.Substring(err, tt.wantErrSubstring); diff != "" { + t.Errorf("%s: DeepCopy(%#v): did not get expected error, %s", tt.name, tt.in, diff) + } + continue } + if diff := pretty.Compare(got, tt.in); diff != "" { t.Errorf("%s: DeepCopy(%#v): did not get identical copy, diff(-got,+want):\n%s", tt.name, tt.in, diff) }