Support subset merge for ordered maps (#891)

* Support subset merge for ordered maps

when merging A <- B, and B is subset of A and the ordering of elements
don't contradict, allow merging to occur.

* slight readability improvement

internal/yreflect/reflect_orderedmap.go

@@ -141,3 +141,27 @@ func OrderedMapKeys(om goOrderedMap) ([]reflect.Value, error) {
 
 	return keySlice, nil
 }
+
+// GetOrderedMapElement calls the given ordered map's Get function given the
+// key value.
+//
+// - reflect.Value is the retrieved value at the key.
+// - bool is whether the value exists.
+// - error is whether an unexpected condition was detected.
+func GetOrderedMapElement(om goOrderedMap, k reflect.Value) (reflect.Value, bool, error) {
+	getMethod, err := MethodByName(reflect.ValueOf(om), "Get")
+	if err != nil {
+		return reflect.Value{}, false, err
+	}
+
+	ret := getMethod.Call([]reflect.Value{k})
+	if got, wantReturnN := len(ret), 1; got != wantReturnN {
+		return reflect.Value{}, false, fmt.Errorf("method Get() doesn't have expected number of return values, got %v, want %v", got, wantReturnN)
+	}
+	v := ret[0]
+	if gotKind := v.Type().Kind(); gotKind != reflect.Ptr {
+		return reflect.Value{}, false, fmt.Errorf("method Keys() did not return a ptr value, got %v", gotKind)
+	}
+
+	return v, !v.IsZero(), nil
+}

internal/yreflect/reflect_orderedmap_test.go

@@ -15,6 +15,7 @@
 package yreflect_test
 
 import (
+	"reflect"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -41,7 +42,7 @@ func TestAppendIntoOrderedMap(t *testing.T) {
 	tests := []struct {
 		desc          string
 		inMap         ygot.GoOrderedMap
-		inValue       interface{}
+		inValue       any
 		wantMap       ygot.GoOrderedMap
 		wantErrSubstr string
 	}{{
@@ -67,3 +68,56 @@ func TestAppendIntoOrderedMap(t *testing.T) {
 		})
 	}
 }
+
+type invalidOrderedMapType struct {
+	ygot.GoOrderedMap
+}
+
+func TestGetOrderedMapElement(t *testing.T) {
+	tests := []struct {
+		desc          string
+		inMap         ygot.GoOrderedMap
+		inKey         any
+		wantVal       any
+		wantOk        bool
+		wantErrSubstr string
+	}{{
+		desc:  "has",
+		inMap: ctestschema.GetOrderedMap(t),
+		inKey: "foo",
+		wantVal: &ctestschema.OrderedList{
+			Key:   ygot.String("foo"),
+			Value: ygot.String("foo-val"),
+		},
+		wantOk: true,
+	}, {
+		desc:    "has-not",
+		inMap:   ctestschema.GetOrderedMap(t),
+		inKey:   "fooo",
+		wantVal: (*ctestschema.OrderedList)(nil),
+		wantOk:  false,
+	}, {
+		desc:          "invalid",
+		inMap:         &invalidOrderedMapType{},
+		wantErrSubstr: "did not find Get() method on type",
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			got, gotOk, err := yreflect.GetOrderedMapElement(tt.inMap, reflect.ValueOf(tt.inKey))
+			if diff := errdiff.Substring(err, tt.wantErrSubstr); diff != "" {
+				t.Fatalf("InsertIntoMap: %s", diff)
+			}
+			if err != nil {
+				return
+			}
+
+			if diff := cmp.Diff(tt.wantVal, got.Interface()); diff != "" {
+				t.Errorf("(-want, +got):\n%s", diff)
+			}
+			if gotOk != tt.wantOk {
+				t.Errorf("gotOk: %v, wantOk: %v", gotOk, tt.wantOk)
+			}
+		})
+	}
+}

ygot/struct_validation_map.go

@@ -912,11 +912,60 @@ func validateMap(srcField, dstField reflect.Value) (*mapType, error) {
 	return &mapType{key: st.Key(), value: st.Elem()}, nil
 }
 
+func srcKeysIsSubset(dstKeys, srcKeys []reflect.Value) bool {
+	dstKeyMap := map[any]struct{}{}
+	for _, k := range dstKeys {
+		dstKeyMap[k.Interface()] = struct{}{}
+	}
+	for _, k := range srcKeys {
+		if _, ok := dstKeyMap[k.Interface()]; !ok {
+			return false
+		}
+	}
+	return true
+}
+
+// orderedMapKeysMergeable determines whether the src ordered map is mergeable
+// into dst.
+//
+// Mergeability criteria:
+// * Maps are disjoint; or,
+// * src map is a subset of dst, and the ordering does not contradict.
+func orderedMapKeysMergeable(dstOrderedMap, srcOrderedMap GoOrderedMap) error {
+	srcKeys, err := yreflect.OrderedMapKeys(srcOrderedMap)
+	if err != nil {
+		return err
+	}
+	dstKeys, err := yreflect.OrderedMapKeys(dstOrderedMap)
+	if err != nil {
+		return err
+	}
+
+	si := 0
+	for di := 0; si != len(srcKeys) && di != len(dstKeys); di++ {
+		// Map keys must be comparable
+		if srcKeys[si].Interface() == dstKeys[di].Interface() {
+			si++
+		}
+	}
+
+	switch {
+	case si == len(srcKeys), si == 0:
+		// Either all srcKeys are matched in order, or the two maps are disjoint.
+		return nil
+	case srcKeysIsSubset(dstKeys, srcKeys):
+		return fmt.Errorf("ordered map keys have different ordering -- merge behaviour is not well defined: (src order: %v) (dst order: %v)", srcKeys, dstKeys)
+	default:
+		return fmt.Errorf("src ordered map partially overlaps with dst ordered map -- merge behaviour is not well defined: (src order: %v) (dst order: %v)", srcKeys, dstKeys)
+	}
+}
+
 // copyOrderedMap copies srcOrderedMap into dstField. Both srcOrderedMap and
 // dstField (this is a reflect.Value for convenience) are ordered list values.
 // If both srcOrderedMap and dstField are populated, and have non-overlapping
 // keys, then the keys in the src are appended to the dst. If there are
-// overlapping values, then an ereror is returned since the behaviour is not
+// overlapping values, then if src is a subset of dst and is in the same order,
+// merge is done; otherwise an error is returned since the behaviour is not
 // well-defined.
 func copyOrderedMap(dstField reflect.Value, srcOrderedMap GoOrderedMap, accessPath string, opts ...MergeOpt) error {
 	dstOrderedMap, dstIsOrderedMap := dstField.Interface().(GoOrderedMap)
@@ -938,23 +987,9 @@ func copyOrderedMap(dstField reflect.Value, srcOrderedMap GoOrderedMap, accessPa
 		dstOrderedMap = dstField.Interface().(GoOrderedMap)
 	}
 
-	srcKeys := map[any]struct{}{}
-	keys, err := yreflect.OrderedMapKeys(srcOrderedMap)
-	if err != nil {
+	if err := orderedMapKeysMergeable(dstOrderedMap, srcOrderedMap); err != nil {
 		return err
 	}
-	for _, k := range keys {
-		srcKeys[k.Interface()] = struct{}{}
-	}
-	keys, err = yreflect.OrderedMapKeys(dstOrderedMap)
-	if err != nil {
-		return err
-	}
-	for _, k := range keys {
-		if _, ok := srcKeys[k.Interface()]; ok {
-			return fmt.Errorf("ordered map keys overlap at %v -- merge behaviour is not well defined", k)
-		}
-	}
 
 	elemType, err := yreflect.OrderedMapElementType(dstOrderedMap)
 	if err != nil {
@@ -964,13 +999,26 @@ func copyOrderedMap(dstField reflect.Value, srcOrderedMap GoOrderedMap, accessPa
 	errs := &errlist.Error{}
 	errs.Separator = "\n"
 	if err := yreflect.RangeOrderedMap(srcOrderedMap, func(k, v reflect.Value) bool {
-		d := reflect.New(elemType.Elem())
-		if err := copyStruct(d.Elem(), v.Elem(), fmt.Sprintf("%s[%#v]", accessPath, k.Interface()), opts...); err != nil {
+		d, ok, err := yreflect.GetOrderedMapElement(dstOrderedMap, k)
+		if err != nil {
 			errs.Add(err)
 			return true
 		}
-		if err := yreflect.AppendIntoOrderedMap(dstOrderedMap, d.Interface()); err != nil {
+		switch {
+		case !ok:
+			d = reflect.New(elemType.Elem())
+		case d.IsZero():
+			errs.Add(fmt.Errorf("dst ordered map has a key whose value is nil: %v", k.Interface()))
+			return true
+		}
+		if err := copyStruct(d.Elem(), v.Elem(), fmt.Sprintf("%s[%#v]", accessPath, k.Interface()), opts...); err != nil {
 			errs.Add(err)
+			return true
+		}
+		if !ok {
+			if err := yreflect.AppendIntoOrderedMap(dstOrderedMap, d.Interface()); err != nil {
+				errs.Add(err)
+			}
 		}
 		return true
 	}); err != nil {