Repository: openconfig/ygot
Pull request #843 (2023-05-25 17:16:36)

Support a couple util/unmarshalling functions for ordered maps
Writing to an ordered list will error out if one already exists since it is expected to receive the whole of the ordered list elements as a whole instead of in chunks. If we later decide this is not the right behaviour we can relax the constraint.

coveralls: 
[![Coverage Status](https://coveralls.io/builds/60321064/badge)](https://coveralls.io/builds/60321064)

Coverage: 89.95% (-0.03%) from 89.982% when pulling **aee713da03183e0a54624a03d38861559acff9ff on util-orderedmaps1** into **e1762f5eb6828041448306889e018809f9d954d1 on gencode-to-testutil**.


Patch:
diff --git a/util/reflect.go b/util/reflect.go
index 3ad8493..e11a32f 100644
--- a/util/reflect.go
+++ b/util/reflect.go
@@ -188,7 +188,11 @@ func InsertIntoSlice(parentSlice interface{}, value interface{}) error {
 	return nil
 }
 
-// InsertIntoMap inserts value with key into parent which must be a map.
+// InsertIntoMap inserts value with key into parent which must be a map or a
+// GoOrderedList interface.
+//
+// If the type is a GoOrderedList, then there must not exist an existing
+// element.
 func InsertIntoMap(parentMap interface{}, key interface{}, value interface{}) error {
 	DbgPrint("InsertIntoMap into parent type %T with key %v(%T) value \n%s\n (%T)",
 		parentMap, ValueStrDebug(key), key, pretty.Sprint(value), value)
@@ -198,12 +202,27 @@ func InsertIntoMap(parentMap interface{}, key interface{}, value interface{}) er
 	kv := reflect.ValueOf(key)
 	vv := reflect.ValueOf(value)
 
-	if t.Kind() != reflect.Map {
+	_, isOrderedList := parentMap.(goOrderedList)
+
+	switch {
+	case t.Kind() == reflect.Map:
+		v.SetMapIndex(kv, vv)
+	case isOrderedList:
+		appendMethod := v.MethodByName("Append")
+		if !appendMethod.IsValid() || appendMethod.IsZero() {
+			return fmt.Errorf("did not find Append() method on type: %v", t.Name())
+		}
+		ret := appendMethod.Call([]reflect.Value{vv})
+		if got, wantReturnN := len(ret), 1; got != wantReturnN {
+			return fmt.Errorf("method Append() doesn't have expected number of return values, got %v, want %v", got, wantReturnN)
+		}
+		if err := ret[0].Interface(); err != nil {
+			return fmt.Errorf("unable to append new ordered map element (it is expected that YANG `ordered-by user` lists are always unmarshalled as a whole instead of individually: %v", err)
+		}
+	default:
 		return fmt.Errorf("InsertIntoMap parent type is %s, must be map", t)
 	}
 
-	v.SetMapIndex(kv, vv)
-
 	return nil
 }
 
diff --git a/util/types.go b/util/types.go
index 3d91427..746a19e 100644
--- a/util/types.go
+++ b/util/types.go
@@ -35,3 +35,11 @@ func YangIntTypeBits(t yang.TypeKind) (int, error) {
 	}
 	return 0, fmt.Errorf("type is not an int")
 }
+
+// goOrderedList is a convenience interface for ygot.GoOrderedList. It is here
+// to avoid a circular dependency.
+type goOrderedList interface {
+	// IsYANGOrderedList is a marker method that indicates that the struct
+	// implements the goOrderedList interface.
+	IsYANGOrderedList()
+}


Test patch:
diff --git a/util/reflect_exported_test.go b/util/reflect_exported_test.go
new file mode 100644
index 0000000..b7fdd6c
--- /dev/null
+++ b/util/reflect_exported_test.go
@@ -0,0 +1,260 @@
+// Copyright 2023 Google Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package util_test
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/openconfig/gnmi/errdiff"
+	"github.com/openconfig/ygot/integration_tests/schemaops/ctestschema"
+	"github.com/openconfig/ygot/util"
+)
+
+// to ptr conversion utility functions
+func toInt8Ptr(i int8) *int8 { return &i }
+
+func TestIsValueNil(t *testing.T) {
+	if !util.IsValueNil(nil) {
+		t.Error("got util.IsValueNil(nil) false, want true")
+	}
+	if !util.IsValueNil((*int)(nil)) {
+		t.Error("got util.IsValueNil(ptr) false, want true")
+	}
+	if !util.IsValueNil((map[int]int)(nil)) {
+		t.Error("got util.IsValueNil(map) false, want true")
+	}
+	if !util.IsValueNil(([]int)(nil)) {
+		t.Error("got util.IsValueNil(slice) false, want true")
+	}
+	if !util.IsValueNil((interface{})(nil)) {
+		t.Error("got util.IsValueNil(interface) false, want true")
+	}
+	if !util.IsValueNil((*ctestschema.OrderedList_OrderedMap)(nil)) {
+		t.Error("got util.IsValueNil(interface) false, want true")
+	}
+
+	if util.IsValueNil(toInt8Ptr(42)) {
+		t.Error("got util.IsValueNil(ptr) true, want false")
+	}
+	if util.IsValueNil(map[int]int{42: 42}) {
+		t.Error("got util.IsValueNil(map) true, want false")
+	}
+	if util.IsValueNil([]int{1, 2, 3}) {
+		t.Error("got util.IsValueNil(slice) true, want false")
+	}
+	if util.IsValueNil((interface{})(42)) {
+		t.Error("got util.IsValueNil(interface) true, want false")
+	}
+	if util.IsValueNil(ctestschema.GetOrderedMap(t)) {
+		t.Error("got util.IsValueNil(interface) true, want false")
+	}
+}
+
+func TestIsValueNilOrDefault(t *testing.T) {
+	// want true tests
+	if !util.IsValueNilOrDefault(nil) {
+		t.Error("got util.IsValueNilOrDefault(nil) false, want true")
+	}
+	if !util.IsValueNilOrDefault((*int)(nil)) {
+		t.Error("got util.IsValueNilOrDefault(ptr) false, want true")
+	}
+	if !util.IsValueNilOrDefault((map[int]int)(nil)) {
+		t.Error("got util.IsValueNilOrDefault(map) false, want true")
+	}
+	if !util.IsValueNilOrDefault(([]int)(nil)) {
+		t.Error("got util.IsValueNilOrDefault(slice) false, want true")
+	}
+	if !util.IsValueNilOrDefault((interface{})(nil)) {
+		t.Error("got util.IsValueNilOrDefault(interface) false, want true")
+	}
+	if !util.IsValueNilOrDefault(int(0)) {
+		t.Error("got util.IsValueNilOrDefault(int(0)) false, want true")
+	}
+	if !util.IsValueNilOrDefault("") {
+		t.Error("got util.IsValueNilOrDefault(\"\") false, want true")
+	}
+	if !util.IsValueNilOrDefault(false) {
+		t.Error("got util.IsValueNilOrDefault(false) false, want true")
+	}
+
+	// want false tests
+	i := 32
+	ip := &i
+	if util.IsValueNilOrDefault(&ip) {
+		t.Error("got util.IsValueNilOrDefault(ptr to ptr) true, want false")
+	}
+	if util.IsValueNilOrDefault([]int{}) {
+		t.Error("got util.IsValueNilOrDefault([]int{}) true, want false")
+	}
+	if util.IsValueNilOrDefault(ctestschema.GetOrderedMap(t)) {
+		t.Error("got util.IsValueNilOrDefault(false) false, want true")
+	}
+}
+
+func TestInsertIntoMap(t *testing.T) {
+	om := ctestschema.GetOrderedMap(t)
+	newKey := "new"
+	for om.Get(newKey) != nil {
+		newKey += "-repeat"
+	}
+	om2 := ctestschema.GetOrderedMap(t)
+	newOrderedListElement, err := om2.AppendNew(newKey)
+	if err != nil {
+		t.Fatalf("om2.AppendNew(newKey) failed, this is unexpected for testing: %v", err)
+	}
+	var stringval = "val"
+	newOrderedListElement.Value = &stringval
+
+	tests := []struct {
+		desc          string
+		inMap         interface{}
+		inKey         interface{}
+		inValue       interface{}
+		wantMap       interface{}
+		wantErrSubstr string
+	}{{
+		desc:    "regular map",
+		inMap:   map[int]string{42: "forty two", 43: "forty three"},
+		inKey:   44,
+		inValue: "forty four",
+		wantMap: map[int]string{42: "forty two", 43: "forty three", 44: "forty four"},
+	}, {
+		desc:    "ordered map",
+		inMap:   om,
+		inKey:   newKey,
+		inValue: newOrderedListElement,
+		wantMap: om2,
+	}, {
+		desc:          "bad map",
+		inMap:         &struct{}{},
+		inKey:         44,
+		inValue:       "forty four",
+		wantErrSubstr: `InsertIntoMap parent type is *struct {}, must be map`,
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			err := util.InsertIntoMap(tt.inMap, tt.inKey, tt.inValue)
+			if diff := errdiff.Substring(err, tt.wantErrSubstr); diff != "" {
+				t.Fatalf("InsertIntoMap: %s", diff)
+			}
+			if err != nil {
+				return
+			}
+
+			if diff := cmp.Diff(tt.wantMap, tt.inMap, cmp.AllowUnexported(ctestschema.OrderedList_OrderedMap{})); diff != "" {
+				t.Errorf("(-want, +got):\n%s", diff)
+			}
+		})
+	}
+}
+
+func TestInitializeStructField(t *testing.T) {
+	type testStruct struct {
+		// Following two fields exist to exercise
+		// initializing pointer fields
+		IPtr      *int
+		SPtr      *string
+		StructPtr *struct {
+			IPtr *int
+			SPtr *string
+		}
+		OMPtr *ctestschema.OrderedList_OrderedMap
+		// Following field exists to exercise
+		// initializing composite fields
+		MPtr map[string]int
+		// Following fields exist to exercise
+		// skipping initializing a slice and
+		// non pointer field
+		SlPtr []string
+		I     int
+	}
+
+	tests := []struct {
+		f          string
+		skip       bool
+		isLeafType bool
+	}{
+		{f: "IPtr", isLeafType: true},
+		{f: "SPtr", isLeafType: true},
+		{f: "StructPtr"},
+		{f: "OMPtr"},
+		{f: "MPtr"},
+		{f: "SlPtr", skip: true},
+		{f: "I", skip: true},
+	}
+
+	for _, initLeaf := range []bool{false, true} {
+		for _, tt := range tests {
+			i := &testStruct{}
+			v := reflect.ValueOf(i)
+			if util.IsValuePtr(v) {
+				v = v.Elem()
+			}
+			fv := v.FieldByName(tt.f)
+			err := util.InitializeStructField(i, tt.f, initLeaf)
+			if err != nil {
+				t.Errorf("got %v, want no error", err)
+			}
+			skip := tt.skip || (!initLeaf && tt.isLeafType)
+			switch {
+			case !skip && fv.IsNil():
+				t.Errorf("got nil, want initialized field value: %q", tt.f)
+			case skip && !util.IsValuePtr(fv) && !fv.IsZero():
+				t.Errorf("got initialized non-pointer field value %q, want zero value", tt.f)
+			case skip && util.IsValuePtr(fv) && !fv.IsNil():
+				t.Errorf("got initialized field value %q, want nil", tt.f)
+			}
+		}
+	}
+}
+
+func TestInsertIntoSlice(t *testing.T) {
+	tests := []struct {
+		desc          string
+		inSlice       any
+		inValue       any
+		wantSlice     any
+		wantErrSubstr string
+	}{{
+		desc:      "basic",
+		inSlice:   &[]int{42, 43},
+		inValue:   44,
+		wantSlice: &[]int{42, 43, 44},
+	}, {
+		desc:          "bad input slice",
+		inSlice:       &struct{}{},
+		inValue:       44,
+		wantErrSubstr: `InsertIntoSlice parent type is *struct {}, must be slice ptr`,
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			err := util.InsertIntoSlice(tt.inSlice, tt.inValue)
+			if diff := errdiff.Substring(err, tt.wantErrSubstr); diff != "" {
+				t.Fatalf("InsertIntoMap: %s", diff)
+			}
+			if err != nil {
+				return
+			}
+
+			if diff := cmp.Diff(tt.wantSlice, tt.inSlice); diff != "" {
+				t.Errorf("(-want, +got):\n%s", diff)
+			}
+		})
+	}
+}
diff --git a/util/reflect_test.go b/util/reflect_test.go
index 661ce92..31b962b 100644
--- a/util/reflect_test.go
+++ b/util/reflect_test.go
@@ -91,75 +91,6 @@ func toStringPtr(s string) *string { return &s }
 func toInt8Ptr(i int8) *int8       { return &i }
 func toInt32Ptr(i int32) *int32    { return &i }
 
-func TestIsValueNil(t *testing.T) {
-	if !IsValueNil(nil) {
-		t.Error("got IsValueNil(nil) false, want true")
-	}
-	if !IsValueNil((*int)(nil)) {
-		t.Error("got IsValueNil(ptr) false, want true")
-	}
-	if !IsValueNil((map[int]int)(nil)) {
-		t.Error("got IsValueNil(map) false, want true")
-	}
-	if !IsValueNil(([]int)(nil)) {
-		t.Error("got IsValueNil(slice) false, want true")
-	}
-	if !IsValueNil((interface{})(nil)) {
-		t.Error("got IsValueNil(interface) false, want true")
-	}
-
-	if IsValueNil(toInt8Ptr(42)) {
-		t.Error("got IsValueNil(ptr) true, want false")
-	}
-	if IsValueNil(map[int]int{42: 42}) {
-		t.Error("got IsValueNil(map) true, want false")
-	}
-	if IsValueNil([]int{1, 2, 3}) {
-		t.Error("got IsValueNil(slice) true, want false")
-	}
-	if IsValueNil((interface{})(42)) {
-		t.Error("got IsValueNil(interface) true, want false")
-	}
-}
-
-func TestIsValueNilOrDefault(t *testing.T) {
-	// want true tests
-	if !IsValueNilOrDefault(nil) {
-		t.Error("got IsValueNilOrDefault(nil) false, want true")
-	}
-	if !IsValueNilOrDefault((*int)(nil)) {
-		t.Error("got IsValueNilOrDefault(ptr) false, want true")
-	}
-	if !IsValueNilOrDefault((map[int]int)(nil)) {
-		t.Error("got IsValueNilOrDefault(map) false, want true")
-	}
-	if !IsValueNilOrDefault(([]int)(nil)) {
-		t.Error("got IsValueNilOrDefault(slice) false, want true")
-	}
-	if !IsValueNilOrDefault((interface{})(nil)) {
-		t.Error("got IsValueNilOrDefault(interface) false, want true")
-	}
-	if !IsValueNilOrDefault(int(0)) {
-		t.Error("got IsValueNilOrDefault(int(0)) false, want true")
-	}
-	if !IsValueNilOrDefault("") {
-		t.Error("got IsValueNilOrDefault(\"\") false, want true")
-	}
-	if !IsValueNilOrDefault(false) {
-		t.Error("got IsValueNilOrDefault(false) false, want true")
-	}
-
-	// want false tests
-	i := 32
-	ip := &i
-	if IsValueNilOrDefault(&ip) {
-		t.Error("got IsValueNilOrDefault(ptr to ptr) true, want false")
-	}
-	if IsValueNilOrDefault([]int{}) {
-		t.Error("got IsValueNilOrDefault([]int{}) true, want false")
-	}
-}
-
 func TestIsValueFuncs(t *testing.T) {
 	testInt := int(42)
 	testStruct := struct{}{}
@@ -862,103 +793,6 @@ func TestInsertIntoMapStructField(t *testing.T) {
 	}
 }
 
-func TestInsertIntoSlice(t *testing.T) {
-	parentSlice := []int{42, 43}
-	value := 44
-	if err := InsertIntoSlice(&parentSlice, value); err != nil {
-		t.Fatalf("got error: %s, want error: nil", err)
-	}
-	wantSlice := []int{42, 43, value}
-	got, want := parentSlice, wantSlice
-	if diff := cmp.Diff(want, got); diff != "" {
-		t.Errorf("(-want, +got):\n%s", diff)
-	}
-
-	badParent := struct{}{}
-	wantErr := `InsertIntoSlice parent type is *struct {}, must be slice ptr`
-	if got, want := errToString(InsertIntoSlice(&badParent, value)), wantErr; got != want {
-		t.Fatalf("got error: %s, want error: %s", got, want)
-	}
-}
-
-func TestInsertIntoMap(t *testing.T) {
-	parentMap := map[int]string{42: "forty two", 43: "forty three"}
-	key := 44
-	value := "forty four"
-	if err := InsertIntoMap(parentMap, key, value); err != nil {
-		t.Fatalf("got error: %s, want error: nil", err)
-	}
-	wantMap := map[int]string{42: "forty two", 43: "forty three", 44: "forty four"}
-	got, want := parentMap, wantMap
-	if diff := cmp.Diff(want, got); diff != "" {
-		t.Errorf("(-want, +got):\n%s", diff)
-	}
-
-	badParent := struct{}{}
-	wantErr := `InsertIntoMap parent type is *struct {}, must be map`
-	if got, want := errToString(InsertIntoMap(&badParent, key, value)), wantErr; got != want {
-		t.Fatalf("got error: %s, want error: %s", got, want)
-	}
-}
-
-func TestInitializeStructField(t *testing.T) {
-	type testStruct struct {
-		// Following two fields exist to exercise
-		// initializing pointer fields
-		IPtr      *int
-		SPtr      *string
-		StructPtr *struct {
-			IPtr *int
-			SPtr *string
-		}
-		// Following field exists to exercise
-		// initializing composite fields
-		MPtr map[string]int
-		// Following fields exist to exercise
-		// skipping initializing a slice and
-		// non pointer field
-		SlPtr []string
-		I     int
-	}
-
-	tests := []struct {
-		f          string
-		skip       bool
-		isLeafType bool
-	}{
-		{f: "IPtr", isLeafType: true},
-		{f: "SPtr", isLeafType: true},
-		{f: "StructPtr"},
-		{f: "MPtr"},
-		{f: "SlPtr", skip: true},
-		{f: "I", skip: true},
-	}
-
-	for _, initLeaf := range []bool{false, true} {
-		for _, tt := range tests {
-			i := &testStruct{}
-			v := reflect.ValueOf(i)
-			if IsValuePtr(v) {
-				v = v.Elem()
-			}
-			fv := v.FieldByName(tt.f)
-			err := InitializeStructField(i, tt.f, initLeaf)
-			if err != nil {
-				t.Errorf("got %v, want no error", err)
-			}
-			skip := tt.skip || (!initLeaf && tt.isLeafType)
-			switch {
-			case !skip && fv.IsNil():
-				t.Errorf("got nil, want initialized field value: %q", tt.f)
-			case skip && !IsValuePtr(fv) && !fv.IsZero():
-				t.Errorf("got initialized non-pointer field value %q, want zero value", tt.f)
-			case skip && IsValuePtr(fv) && !fv.IsNil():
-				t.Errorf("got initialized field value %q, want nil", tt.f)
-			}
-		}
-	}
-}
-
 func TestInitializeStructFieldForSameField(t *testing.T) {
 	type testStruct struct {
 		MPtr map[string]string