diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 2e05b36cf..5c44880b7 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -14,7 +14,7 @@ jobs: strategy: fail-fast: false matrix: - go: ['1.13', '1.14', '1.15', '1.16', '1.x'] + go: ['1.13', '1.14', '1.15', '1.16', '1.17', '1.18', '1.x'] steps: - name: Install protobuf @@ -33,14 +33,12 @@ jobs: run: | GO111MODULE=on go get google.golang.org/protobuf/cmd/protoc-gen-go@latest GO111MODULE=on go get golang.org/x/tools/cmd/goimports@latest - GO111MODULE=on go get honnef.co/go/tools/cmd/staticcheck@latest - if: ${{ !(matrix.go == '1.13' || matrix.go == '1.14' || matrix.go == '1.15') }} name: Install protoc-gen-go, goimports, Staticcheck run: | go install google.golang.org/protobuf/cmd/protoc-gen-go@latest go install golang.org/x/tools/cmd/goimports@latest - go install honnef.co/go/tools/cmd/staticcheck@latest - name: Check out code uses: actions/checkout@v2 diff --git a/util/reflect.go b/util/reflect.go index 1d29089ce..a91de7117 100644 --- a/util/reflect.go +++ b/util/reflect.go @@ -1038,11 +1038,12 @@ func getNodesList(schema *yang.Entry, root interface{}, path *gpb.Path) ([]inter if !fv.IsValid() { return nil, nil, fmt.Errorf("element struct type %s does not contain key field %s", k.Type(), kfn) } - nv := fv - if fv.Type().Kind() == reflect.Ptr { - // Ptr values are deferenced in key struct. - nv = nv.Elem() - } + // FIXME(wenbli): This block was here but is not doing anything. We need to ensure that no functionality would be missing by removing it. + //nv := fv + //if fv.Type().Kind() == reflect.Ptr { + // // Ptr values are deferenced in key struct. + // nv = nv.Elem() + //} kf, ok := listElementType.FieldByName(kfn) if !ok { return nil, nil, fmt.Errorf("element struct type %s does not contain key field %s", k.Type(), kfn) diff --git a/ytypes/leaf_list.go b/ytypes/leaf_list.go index 36a549be0..3a57f2690 100644 --- a/ytypes/leaf_list.go +++ b/ytypes/leaf_list.go @@ -100,6 +100,11 @@ func unmarshalLeafList(schema *yang.Entry, parent interface{}, value interface{} return err } + fieldName, _, err := schemaToStructFieldName(schema, parent) + if err != nil { + return err + } + util.DbgPrint("unmarshalLeafList value %v, type %T, into parent type %T, schema name %s", util.ValueStrDebug(value), value, parent, schema.Name) // The leaf schema is just the leaf-list schema without the list attrs. @@ -119,6 +124,8 @@ func unmarshalLeafList(schema *yang.Entry, parent interface{}, value interface{} if len(sa.LeaflistVal.GetElement()) == 0 { return fmt.Errorf("unmarshalLeafList for schema %s: value %v: got empty leaf list, expect non-empty leaf list", schema.Name, util.ValueStr(value)) } + // A new leaf-list update specifies the entire leaf-list, so we should clear its contents if it is non-nil. + clearSliceField(parent, fieldName) for _, v := range sa.LeaflistVal.GetElement() { if err := unmarshalGeneric(&leafSchema, parent, v, enc); err != nil { return err @@ -130,6 +137,8 @@ func unmarshalLeafList(schema *yang.Entry, parent interface{}, value interface{} return fmt.Errorf("unmarshalLeafList for schema %s: value %v: got type %T, expect []interface{}", schema.Name, util.ValueStr(value), value) } + // A new leaf-list update specifies the entire leaf-list, so we should clear its contents if it is non-nil. + clearSliceField(parent, fieldName) for _, leaf := range leafList { if err := unmarshalGeneric(&leafSchema, parent, leaf, enc); err != nil { return err @@ -141,3 +150,30 @@ func unmarshalLeafList(schema *yang.Entry, parent interface{}, value interface{} return nil } + +// clearSliceField sets updates a field called fieldName (which must exist, but may be +// nil) in parentStruct, with value nil. +func clearSliceField(parentStruct interface{}, fieldName string) error { + util.DbgPrint("clearSliceField field %s of parent type %T with value %v", fieldName, parentStruct) + + if util.IsValueNil(parentStruct) { + return fmt.Errorf("parent is nil in clearSliceField for field %s", fieldName) + } + + pt, pv := reflect.TypeOf(parentStruct), reflect.ValueOf(parentStruct) + + if !util.IsTypeStructPtr(pt) { + return fmt.Errorf("parent type %T must be a struct ptr", parentStruct) + } + ft, ok := pt.Elem().FieldByName(fieldName) + if !ok { + return fmt.Errorf("parent type %T does not have a field name %s", parentStruct, fieldName) + } + + if ft.Type.Kind() != reflect.Slice { + return fmt.Errorf("field %s of parent type %T must be Slice type (%v)", fieldName, parentStruct, ft.Type.Kind()) + } + + pv.Elem().FieldByName(fieldName).Set(reflect.Zero(ft.Type)) + return nil +} diff --git a/ytypes/leaf_list_test.go b/ytypes/leaf_list_test.go index 26b78cb59..c783432a7 100644 --- a/ytypes/leaf_list_test.go +++ b/ytypes/leaf_list_test.go @@ -271,6 +271,7 @@ func TestUnmarshalLeafListGNMIEncoding(t *testing.T) { desc string sch *yang.Entry val interface{} + in LeafListContainer want LeafListContainer wantErr string }{ @@ -293,6 +294,21 @@ func TestUnmarshalLeafListGNMIEncoding(t *testing.T) { }}, want: LeafListContainer{Int32LeafList: []*int32{ygot.Int32(-42), ygot.Int32(0), ygot.Int32(42)}}, }, + { + desc: "int32 success with existing values", + sch: int32LeafListSchema, + val: &gpb.TypedValue{Value: &gpb.TypedValue_LeaflistVal{ + LeaflistVal: &gpb.ScalarArray{ + Element: []*gpb.TypedValue{ + {Value: &gpb.TypedValue_IntVal{IntVal: -42}}, + {Value: &gpb.TypedValue_IntVal{IntVal: 0}}, + {Value: &gpb.TypedValue_IntVal{IntVal: 42}}, + }, + }, + }}, + in: LeafListContainer{Int32LeafList: []*int32{ygot.Int32(-41), ygot.Int32(41)}}, + want: LeafListContainer{Int32LeafList: []*int32{ygot.Int32(-42), ygot.Int32(0), ygot.Int32(42)}}, + }, { desc: "int32 fail with nil TypedValue", sch: int32LeafListSchema, @@ -434,15 +450,14 @@ func TestUnmarshalLeafListGNMIEncoding(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - var parent LeafListContainer - err := unmarshalGeneric(tt.sch, &parent, tt.val, GNMIEncoding) + err := unmarshalGeneric(tt.sch, &tt.in, tt.val, GNMIEncoding) if diff := errdiff.Substring(err, tt.wantErr); diff != "" { - t.Errorf("unmarshalGeneric(%v, %v, %v): diff(-got,+want):\n%s", tt.sch, parent, tt.val, diff) + t.Errorf("unmarshalGeneric(%v, %v, %v): diff(-got,+want):\n%s", tt.sch, tt.in, tt.val, diff) } if err != nil { return } - got, want := parent, tt.want + got, want := tt.in, tt.want if diff := cmp.Diff(want, got); diff != "" { t.Errorf("unmarshalGeneric (-want, +got):\n%s", diff) } @@ -477,6 +492,7 @@ func TestUnmarshalLeafListJSONEncoding(t *testing.T) { tests := []struct { desc string json string + in ContainerStruct want ContainerStruct wantErr string }{ @@ -490,6 +506,12 @@ func TestUnmarshalLeafListJSONEncoding(t *testing.T) { json: `{ "int32-leaf-list" : [-42, 0, 42] }`, want: ContainerStruct{Int32LeafList: []*int32{ygot.Int32(-42), ygot.Int32(0), ygot.Int32(42)}}, }, + { + desc: "int32 success with existing values", + json: `{ "int32-leaf-list" : [-42, 0, 42] }`, + in: ContainerStruct{Int32LeafList: []*int32{ygot.Int32(-41), ygot.Int32(41)}}, + want: ContainerStruct{Int32LeafList: []*int32{ygot.Int32(-42), ygot.Int32(0), ygot.Int32(42)}}, + }, { desc: "enum success", json: `{ "enum-leaf-list" : ["E_VALUE_FORTY_TWO"] }`, @@ -510,21 +532,19 @@ func TestUnmarshalLeafListJSONEncoding(t *testing.T) { var jsonTree interface{} for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { - var parent ContainerStruct - if tt.json != "" { if err := json.Unmarshal([]byte(tt.json), &jsonTree); err != nil { t.Fatalf("%s : %s", tt.desc, err) } } - err := Unmarshal(containerWithLeafListSchema, &parent, jsonTree) + err := Unmarshal(containerWithLeafListSchema, &tt.in, jsonTree) if got, want := errToString(err), tt.wantErr; got != want { t.Errorf("%s: Unmarshal got error: %v, want error: %v", tt.desc, got, want) } testErrLog(t, tt.desc, err) if err == nil { - got, want := parent, tt.want + got, want := tt.in, tt.want if diff := cmp.Diff(want, got); diff != "" { t.Errorf("%s: unmarshal (-want, +got):\n%s", tt.desc, diff) } @@ -555,7 +575,9 @@ func TestUnmarshalLeafListJSONEncoding(t *testing.T) { // bad value type wantErr = `unmarshalLeafList for schema valid-leaf-list-schema: value 42 (int): got type int, expect []interface{}` - if got, want := errToString(unmarshalLeafList(validLeafListSchema, &struct{}{}, int(42), JSONEncoding)), wantErr; got != want { + if got, want := errToString(unmarshalLeafList(validLeafListSchema, &struct { + Field []int32 `path:"valid-leaf-list-schema"` + }{}, int(42), JSONEncoding)), wantErr; got != want { t.Errorf("nil schema: Unmarshal got error: %v, want error: %v", got, want) } } diff --git a/ytypes/node_test.go b/ytypes/node_test.go index e6d9d3252..1f9803820 100644 --- a/ytypes/node_test.go +++ b/ytypes/node_test.go @@ -2011,11 +2011,11 @@ func TestSetNode(t *testing.T) { }}, }}, inOpts: []SetNodeOpt{&InitMissingElements{}}, - wantLeaf: []int32{42, 43}, + wantLeaf: []int32{43}, wantParent: &ListElemStruct1{ Outer: &OuterContainerType1{ Inner: &InnerContainerType1{ - Int32LeafListName: []int32{42, 43}, + Int32LeafListName: []int32{43}, }, }, },