Fix defects in merging structs for enums and unions. (#367)

* Fix defects in merging structs for enums and unions.

 * (M) ygot/struct_validation_map(_test)?.go
  - Resolve bug with merging enums where an unset value would overwrite
    a set value.
  - Resolve bug with merging enums where an error would not be thrown
    when the values were explicitly set to different values.
  - Resolve bug with merging unions where two unequal values would not
    result in an error being thrown.
  - Resolve bug with merging unions where an unset value would overwrite
    a set value.

* run gofmt against ygot.

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,15 +614,22 @@ 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
 	}
 
 	if !util.IsValueInterface(srcField) || !util.IsValueStructPtr(srcField.Elem()) {
 		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 {

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: &copyUnionS{"glutenberg-ipa"},
+	},
+	inB: &validatedMergeTest{
+		UnionField: &copyUnionS{"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: &copyUnionS{"ipswich-ale-celia-saison"},
+	},
+	inB: &validatedMergeTest{
+		UnionField: &copyUnionS{"ipswich-ale-celia-saison"},
+	},
+	want: &validatedMergeTest{
+		UnionField: &copyUnionS{"ipswich-ale-celia-saison"},
+	},
+}, {
+	name: "merge union: set in src and not dst",
+	inA: &validatedMergeTest{
+		UnionField: &copyUnionS{"estrella-damn-daura"},
+	},
+	inB: &validatedMergeTest{},
+	want: &validatedMergeTest{
+		UnionField: &copyUnionS{"estrella-damn-daura"},
+	},
+}, {
+	name: "merge union: set in dst and not src",
+	inA:  &validatedMergeTest{},
+	inB: &validatedMergeTest{
+		UnionField: &copyUnionS{"two-brothers-prairie-path-golden-ale"},
+	},
+	want: &validatedMergeTest{
+		UnionField: &copyUnionS{"two-brothers-prairie-path-golden-ale"},
+	},
+}, {
+	name: "merge union: values not equal, and different types",
+	inA: &validatedMergeTest{
+		UnionField: &copyUnionS{"greens-amber"},
+	},
+	inB: &validatedMergeTest{
+		UnionField: &copyUnionI{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:   &copyTest{StringField: String("zaphod")},
@@ -1842,13 +1936,21 @@ func TestDeepCopy(t *testing.T) {
 		in: &copyTest{
 			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)
 		}