From 2713b986ad5a22464ae5e72e919be9f519466246 Mon Sep 17 00:00:00 2001 From: Wen Bo Li <50884368+wenovus@users.noreply.github.com> Date: Tue, 15 Jun 2021 15:32:09 -0700 Subject: [PATCH] Don't initialize leaf values when unmarshalling valid shadow paths. (#543) * Don't initialize leaf values when unmarshalling valid shadow paths. Currently when `SetNode` is called on a path, missing elements are initialized as the function traverses through the `GoStruct`. This happens even for paths leading to a leaf as well as shadow paths, causing the default value to always be populated into these elements. This change prevents `SetNode` from initializing a leaf value when traversing a shadow path. * Remove support for arbitrarily-deep shadow paths to reduce complexity * Add tests for checking that shadow paths cannot be non-leaves --- ytypes/node.go | 85 ++++--- ytypes/node_test.go | 560 +++++++++++++++++++------------------------- 2 files changed, 284 insertions(+), 361 deletions(-) diff --git a/ytypes/node.go b/ytypes/node.go index 9c76572a9..4b3681e29 100644 --- a/ytypes/node.go +++ b/ytypes/node.go @@ -53,12 +53,6 @@ type retrieveNodeArgs struct { // specifically to deal with uint values being streamed as positive int // values. tolerateJSONInconsistenciesForVal bool - // shadowPath is an implementation field indicating that the current - // path being traversed is a shadow path, and should result in a silent - // traversal, i.e. a normal traversal except any modifications to leaf - // values are skipped (non-existing GoStructs are still initialized), - // and any value retrieved is always nil. - shadowPath bool // reverseShadowPath reverses the meaning of the "path" and // "shadow-path" tags when both are present while processing a // GoStruct. @@ -80,12 +74,6 @@ func retrieveNode(schema *yang.Entry, root interface{}, path, traversedPath *gpb return nil, status.Errorf(codes.Unknown, "path %v points to a node with non-leaf schema %v", traversedPath, schema) } } - // Shadow traversal returns the node with nil schema and data. - if args.shadowPath { - return []*TreeNode{{ - Path: traversedPath, - }}, nil - } return []*TreeNode{{ Path: traversedPath, Schema: schema, @@ -128,6 +116,8 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, for i := 0; i < v.NumField(); i++ { fv, ft := v.Field(i), v.Type().Field(i) + // TODO(low priority): ChildSchema should return the shadow + // schema if we're traversing a shadow path. cschema, err := util.ChildSchema(schema, ft) if !util.IsYgotAnnotation(ft) { switch { @@ -138,12 +128,36 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, } } - checkPath := func(p []string, args retrieveNodeArgs) ([]*TreeNode, error) { + checkPath := func(p []string, args retrieveNodeArgs, shadowLeaf bool) ([]*TreeNode, error) { to := len(p) if util.IsTypeMap(ft.Type) { to-- } + np := &gpb.Path{} + if traversedPath != nil { + np = proto.Clone(traversedPath).(*gpb.Path) + } + for i := range p[0:to] { + np.Elem = append(np.Elem, path.GetElem()[i]) + } + + // If the current node is a shadow leaf, this means the input path is a shadow path + // that the GoStruct recognizes, but doesn't have space for. We will therefore + // silently ignore this path. + if shadowLeaf { + switch { + case cschema == nil: + return nil, status.Errorf(codes.InvalidArgument, "could not find schema for path %v", np) + case !cschema.IsLeaf(): + return nil, status.Errorf(codes.InvalidArgument, "shadow path traverses a non-leaf node, this is not allowed, path: %v", np) + default: + return []*TreeNode{{ + Path: np, + }}, nil + } + } + // If args.modifyRoot is true, then initialize the field before possibly searching further. if args.modifyRoot { if err := util.InitializeStructField(root, ft.Name); err != nil { return nil, status.Errorf(codes.Unknown, "failed to initialize struct field %s in %T, child schema %v, path %v", ft.Name, root, cschema, path) @@ -154,9 +168,7 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, // corresponding field to its zero value. The zero value is the unset value for // any node type, whether leaf or non-leaf. if args.delete && len(path.Elem) == to { - if !args.shadowPath { - fv.Set(reflect.Zero(ft.Type)) - } + fv.Set(reflect.Zero(ft.Type)) return nil, nil } @@ -164,7 +176,7 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, // may be dealing with a leaf or leaf list node. We should set the val // to the corresponding field in GoStruct. If the field is an annotation, // the field doesn't have a schema, so it is handled separately. - if !util.IsValueNil(args.val) && len(path.Elem) == to && !args.shadowPath { + if !util.IsValueNil(args.val) && len(path.Elem) == to { switch { case util.IsYgotAnnotation(ft): if err := util.UpdateField(root, ft.Name, args.val); err != nil { @@ -184,13 +196,6 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, } } - np := &gpb.Path{} - if traversedPath != nil { - np = proto.Clone(traversedPath).(*gpb.Path) - } - for i := range p[0:to] { - np.Elem = append(np.Elem, path.GetElem()[i]) - } return retrieveNode(cschema, fv.Interface(), util.TrimGNMIPathPrefix(path, p[0:to]), np, args) } @@ -200,26 +205,26 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, // Note that we first look through the non-shadow path, and if // no matches are found, we then look through the shadow path // to find matches. If the input path matches a shadow path, - // then the traversal continues, although the final operation - // is marked as a no-op. + // then we're guaranteed to have reached a leaf, since shadow + // paths can only occur for direct leaves under config/state. // // If the user has opted to reverse the "shadow-path" and // "path" tags, then the order of the look-ups is reversed. - args := args + var shadowLeaf bool if args.reverseShadowPath { // Look through shadow paths first instead. schPaths := util.ShadowSchemaPaths(ft) for _, p := range schPaths { if util.PathMatchesPrefix(path, p) { - return checkPath(p, args) + return checkPath(p, args, false) } } if len(schPaths) != 0 { - // Only if there exists shadow paths do we - // treat the non-shadow paths as no-ops. - // Otherwise, there is no reversal to do. - args.shadowPath = true + // Only if there exists shadow paths is there + // such thing as doing a reverse look-up, i.e. + // reversing non-shadow paths with shadow paths. + shadowLeaf = true } } schPaths, err := util.SchemaPaths(ft) @@ -228,16 +233,14 @@ func retrieveNodeContainer(schema *yang.Entry, root interface{}, path *gpb.Path, } for _, p := range schPaths { if util.PathMatchesPrefix(path, p) { - return checkPath(p, args) + return checkPath(p, args, shadowLeaf) } } if !args.reverseShadowPath { - // Look through shadow paths last, and mark operations - // as no-ops. - args.shadowPath = true + // Look through shadow paths last. for _, p := range util.ShadowSchemaPaths(ft) { if util.PathMatchesPrefix(path, p) { - return checkPath(p, args) + return checkPath(p, args, true) } } } @@ -303,7 +306,7 @@ func retrieveNodeList(schema *yang.Entry, root interface{}, path, traversedPath } if keyAsString == pathKey { remainingPath := util.PopGNMIPath(path) - if args.delete && len(remainingPath.GetElem()) == 0 && !args.shadowPath { + if args.delete && len(remainingPath.GetElem()) == 0 { rv.SetMapIndex(k, reflect.Value{}) return nil, nil } @@ -357,7 +360,7 @@ func retrieveNodeList(schema *yang.Entry, root interface{}, path, traversedPath return nil, status.Errorf(codes.Unknown, "could not extract keys from %v: %v", traversedPath, err) } remainingPath := util.PopGNMIPath(path) - if args.delete && len(remainingPath.GetElem()) == 0 && !args.shadowPath { + if args.delete && len(remainingPath.GetElem()) == 0 { rv.SetMapIndex(k, reflect.Value{}) return nil, nil } @@ -399,6 +402,10 @@ type GetOrCreateNodeOpt interface { // along the path if they are nil. // Function returns the value and schema of the node as well as error. // Note that this function may modify the supplied root even if the function fails. +// Note that this function may create containers or list entries even if the input path is a shadow path. +// TODO(wenbli): a traversal should remember what containers or list entries +// were created so that a failed call or a call to a shadow path can later undo +// this. This applies to SetNode as well. func GetOrCreateNode(schema *yang.Entry, root interface{}, path *gpb.Path, opts ...GetOrCreateNodeOpt) (interface{}, *yang.Entry, error) { nodes, err := retrieveNode(schema, root, path, nil, retrieveNodeArgs{ modifyRoot: true, diff --git a/ytypes/node_test.go b/ytypes/node_test.go index a98b564ff..e535f5885 100644 --- a/ytypes/node_test.go +++ b/ytypes/node_test.go @@ -31,9 +31,9 @@ import ( ) type InnerContainerType1 struct { - Int32LeafName *int32 `path:"int32-leaf-field"` + Int32LeafName *int32 `path:"int32-leaf-field" shadow-path:"state/int32-leaf-field"` Int32LeafListName []int32 `path:"int32-leaf-list"` - StringLeafName *string `path:"string-leaf-field"` + StringLeafName *string `path:"string-leaf-field" shadow-path:"state/string-leaf-field"` EnumLeafName EnumType `path:"enum-leaf-field"` Annotation []ygot.Annotation `path:"@annotation" ygotAnnotation:"true"` } @@ -61,7 +61,7 @@ func (l *ListElemStruct1) ΛListKeyMap() (map[string]interface{}, error) { func (*ListElemStruct1) IsYANGGoStruct() {} type ContainerStruct1 struct { - StructKeyList map[string]*ListElemStruct1 `path:"config/simple-key-list" shadow-path:"state/simple-key-list"` + StructKeyList map[string]*ListElemStruct1 `path:"config/simple-key-list"` } func (*ContainerStruct1) IsYANGGoStruct() {} @@ -168,6 +168,22 @@ var containerWithStringKey = &yang.Entry{ Kind: yang.LeafEntry, Type: &yang.YangType{Kind: yang.Yint32}, }, + "state": { + Name: "state", + Kind: yang.DirectoryEntry, + Dir: map[string]*yang.Entry{ + "int32-leaf-field": { + Name: "int32-leaf-field", + Kind: yang.LeafEntry, + Type: &yang.YangType{Kind: yang.Yint32}, + }, + "string-leaf-field": { + Name: "string-leaf-field", + Kind: yang.LeafEntry, + Type: &yang.YangType{Kind: yang.Ystring}, + }, + }, + }, "int32-leaf-list": { Name: "int32-leaf-list", Kind: yang.LeafEntry, @@ -414,25 +430,7 @@ func TestGetOrCreateNodeSimpleKey(t *testing.T) { inDesc: "success getting a nil shadow value", inParent: &ContainerStruct1{}, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), - want: nil, - }, - { - inDesc: "success getting a nil shadow container", - inParent: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{ - Inner: &InnerContainerType1{ - StringLeafName: ygot.String("forty_two"), - }, - }, - }, - }, - }, - inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/int32-leaf-field"), want: nil, }, { @@ -450,54 +448,26 @@ func TestGetOrCreateNodeSimpleKey(t *testing.T) { }, }, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/int32-leaf-field"), want: nil, }, { inDesc: "fail getting a shadow value whose container doesn't exist", inParent: &ContainerStruct1{}, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/INVALID_CONTAINER/int32-leaf-field"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/INVALID_CONTAINER/state/int32-leaf-field"), wantErrSubstring: "no match found in *ytypes.OuterContainerType1", }, { - inDesc: "fail getting a shadow value that doesn't exist", - inParent: &ContainerStruct1{}, - inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/INVALID_LEAF"), - wantErrSubstring: "no match found in *ytypes.InnerContainerType1", - }, - { - inDesc: "success getting a shadow value with reverseShadowPath=true", + inDesc: "success getting an initialized shadow value with reverseShadowPath=true", inParent: &ContainerStruct1{}, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/int32-leaf-field"), inOpts: []GetOrCreateNodeOpt{&ReverseShadowPaths{}}, want: ygot.Int32(0), }, { - inDesc: "success getting a shadow container with reverseShadowPath=true", - inParent: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{ - Inner: &InnerContainerType1{ - StringLeafName: ygot.String("forty_two"), - }, - }, - }, - }, - }, - inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner"), - inOpts: []GetOrCreateNodeOpt{&ReverseShadowPaths{}}, - want: &InnerContainerType1{ - StringLeafName: ygot.String("forty_two"), - }, - }, - { - inDesc: "success get shadow int32 leaf with an existing key with reverseShadowPath=true", + inDesc: "success getting a shadow int32 leaf with an existing key with reverseShadowPath=true", inParent: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ "forty-two": { @@ -511,7 +481,7 @@ func TestGetOrCreateNodeSimpleKey(t *testing.T) { }, }, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/int32-leaf-field"), inOpts: []GetOrCreateNodeOpt{&ReverseShadowPaths{}}, want: ygot.Int32(42), }, @@ -519,15 +489,15 @@ func TestGetOrCreateNodeSimpleKey(t *testing.T) { inDesc: "fail getting a shadow value whose container doesn't exist with reverseShadowPath=true", inParent: &ContainerStruct1{}, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/INVALID_CONTAINER/int32-leaf-field"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/INVALID_CONTAINER/state/int32-leaf-field"), inOpts: []GetOrCreateNodeOpt{&ReverseShadowPaths{}}, wantErrSubstring: "no match found in *ytypes.OuterContainerType1", }, { - inDesc: "fail getting a shadow value that doesn't exist with reverseShadowPath=true", + inDesc: "fail getting a value that doesn't exist with reverseShadowPath=true", inParent: &ContainerStruct1{}, inSchema: containerWithStringKey, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/INVALID_LEAF"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/INVALID_LEAF"), inOpts: []GetOrCreateNodeOpt{&ReverseShadowPaths{}}, wantErrSubstring: "no match found in *ytypes.InnerContainerType1", }, @@ -1009,7 +979,7 @@ type multiListKey struct { } type listChildContainer struct { - Value *string `path:"value"` + Value *string `path:"value" shadow-path:"shadow-value"` } type childList struct { @@ -1028,10 +998,10 @@ type grandchildContainer struct { type rootStruct struct { Leaf *string `path:"leaf"` LeafList []int32 `path:"int32-leaf-list"` - Container *childContainer `path:"container"` + Container *childContainer `path:"container" shadow-path:"shadow-container"` List map[string]*listEntry `path:"list"` Multilist map[multiListKey]*multiListEntry `path:"multilist"` - ChildList map[string]*childList `path:"state/childlist" shadow-path:"config/childlist"` + ChildList map[string]*childList `path:"state/childlist"` } func TestGetNode(t *testing.T) { @@ -1066,6 +1036,13 @@ func TestGetNode(t *testing.T) { Dir: map[string]*yang.Entry{}, } rootSchema.Dir["container"] = childContainerSchema + childShadowContainerSchema := &yang.Entry{ + Name: "shadow-container", + Kind: yang.DirectoryEntry, + Parent: rootSchema, + Dir: map[string]*yang.Entry{}, + } + rootSchema.Dir["shadow-container"] = childShadowContainerSchema grandchildContainerSchema := &yang.Entry{ Name: "grandchild", @@ -1168,10 +1145,16 @@ func TestGetNode(t *testing.T) { Type: &yang.YangType{Kind: yang.Ystring}, } childListContainerSchema.Dir["value"] = childListContainerValueSchema + childListContainerShadowValueSchema := &yang.Entry{ + Name: "shadow-value", + Kind: yang.LeafEntry, + Parent: childListContainerSchema, + Type: &yang.YangType{Kind: yang.Ystring}, + } + childListContainerSchema.Dir["shadow-value"] = childListContainerShadowValueSchema return configStateEntry } - rootSchema.Dir["config"] = newChildListSchema("config") rootSchema.Dir["state"] = newChildListSchema("state") tests := []struct { @@ -1532,50 +1515,30 @@ func TestGetNode(t *testing.T) { Path: mustPath("/multilist[keyone=1][keytwo=3]"), }}, }, { - desc: "deeper list non-shadow path", + desc: "shadow path that traverses a non-leaf node", inSchema: rootSchema, inData: &rootStruct{ - ChildList: map[string]*childList{ - "one": { - Key: ygot.String("one"), - ChildContainer: &listChildContainer{Value: ygot.String("1")}, - }, - "two": { - Key: ygot.String("two"), - ChildContainer: &listChildContainer{Value: ygot.String("2")}, + Container: &childContainer{ + Container: &grandchildContainer{ + Val: ygot.String("forty-two"), }, }, }, - inPath: mustPath("/state/childlist[key=one]"), - wantTreeNodes: []*TreeNode{{ - Data: &childList{ - Key: ygot.String("one"), - ChildContainer: &listChildContainer{Value: ygot.String("1")}, - }, - Schema: rootSchema.Dir["state"].Dir["childlist"], - Path: mustPath("/state/childlist[key=one]"), - }}, + inPath: mustPath("/shadow-container/grandchild/val"), + wantErrSubstring: "shadow path traverses a non-leaf node, this is not allowed", }, { - desc: "deeper list shadow path", + desc: "shadow path that traverses a non-leaf node with reverseShadowPath=true", inSchema: rootSchema, inData: &rootStruct{ - ChildList: map[string]*childList{ - "one": { - Key: ygot.String("one"), - ChildContainer: &listChildContainer{Value: ygot.String("1")}, - }, - "two": { - Key: ygot.String("two"), - ChildContainer: &listChildContainer{Value: ygot.String("2")}, + Container: &childContainer{ + Container: &grandchildContainer{ + Val: ygot.String("forty-two"), }, }, }, - inPath: mustPath("/config/childlist[key=one]"), - wantTreeNodes: []*TreeNode{{ - Data: nil, - Schema: nil, - Path: mustPath("/config/childlist[key=one]"), - }}, + inPath: mustPath("/container/grandchild/val"), + inArgs: []GetNodeOpt{&ReverseShadowPaths{}}, + wantErrSubstring: "shadow path traverses a non-leaf node, this is not allowed", }, { desc: "deeper list non-shadow leaf path", inSchema: rootSchema, @@ -1612,11 +1575,11 @@ func TestGetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/childlist[key=one]/child-container/value"), + inPath: mustPath("/state/childlist[key=one]/child-container/shadow-value"), wantTreeNodes: []*TreeNode{{ Data: nil, Schema: nil, - Path: mustPath("/config/childlist[key=one]/child-container/value"), + Path: mustPath("/state/childlist[key=one]/child-container/shadow-value"), }}, }, { desc: "deeper list shadow leaf path not found", @@ -1633,33 +1596,8 @@ func TestGetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/childlist[key=one]/child-container/valeur"), + inPath: mustPath("/state/childlist[key=one]/child-container/valeur"), wantErrSubstring: "no match found in *ytypes.listChildContainer", - }, { - desc: "deeper list shadow path with reverseShadowPath=true", - inSchema: rootSchema, - inData: &rootStruct{ - ChildList: map[string]*childList{ - "one": { - Key: ygot.String("one"), - ChildContainer: &listChildContainer{Value: ygot.String("1")}, - }, - "two": { - Key: ygot.String("two"), - ChildContainer: &listChildContainer{Value: ygot.String("2")}, - }, - }, - }, - inPath: mustPath("/config/childlist[key=one]"), - inArgs: []GetNodeOpt{&ReverseShadowPaths{}}, - wantTreeNodes: []*TreeNode{{ - Data: &childList{ - Key: ygot.String("one"), - ChildContainer: &listChildContainer{Value: ygot.String("1")}, - }, - Schema: rootSchema.Dir["state"].Dir["childlist"], - Path: mustPath("/config/childlist[key=one]"), - }}, }, { desc: "deeper list non-shadow leaf path with reverseShadowPath=true", inSchema: rootSchema, @@ -1697,15 +1635,15 @@ func TestGetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/childlist[key=one]/child-container/value"), + inPath: mustPath("/state/childlist[key=one]/child-container/shadow-value"), inArgs: []GetNodeOpt{&ReverseShadowPaths{}}, wantTreeNodes: []*TreeNode{{ Data: ygot.String("1"), Schema: rootSchema.Dir["state"].Dir["childlist"].Dir["child-container"].Dir["value"], - Path: mustPath("/config/childlist[key=one]/child-container/value"), + Path: mustPath("/state/childlist[key=one]/child-container/shadow-value"), }}, }, { - desc: "deeper list shadow leaf path not found with reverseShadowPath=true", + desc: "deeper list leaf path not found with reverseShadowPath=true", inSchema: rootSchema, inData: &rootStruct{ ChildList: map[string]*childList{ @@ -1719,7 +1657,7 @@ func TestGetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/childlist[key=one]/child-container/valeur"), + inPath: mustPath("/state/childlist[key=one]/child-container/valeur"), inArgs: []GetNodeOpt{&ReverseShadowPaths{}}, wantErrSubstring: "no match found in *ytypes.listChildContainer", }} @@ -1732,6 +1670,8 @@ func TestGetNode(t *testing.T) { } if err := treeNodesEqual(got, tt.wantTreeNodes); err != nil { + fmt.Println(got[0].Schema) + fmt.Println(tt.wantTreeNodes[0].Schema) t.Fatalf("did not get expected result, %v", err) } }) @@ -1763,25 +1703,27 @@ func TestSetNode(t *testing.T) { inVal interface{} inOpts []SetNodeOpt wantErrSubstring string - want interface{} + wantLeaf interface{} + wantParent interface{} }{ { - inDesc: "success setting string field in top node", - inSchema: simpleSchema, - inParent: &ListElemStruct1{}, - inPath: mustPath("/key1"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - inOpts: []SetNodeOpt{&ReverseShadowPaths{}}, - want: ygot.String("hello"), + inDesc: "success setting string field in top node", + inSchema: simpleSchema, + inParent: &ListElemStruct1{}, + inPath: mustPath("/key1"), + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, + wantLeaf: ygot.String("hello"), + wantParent: &ListElemStruct1{Key1: ygot.String("hello")}, }, { - inDesc: "success setting string field in top node with reverseShadowPath=true where shadow-path doesn't exist", - inSchema: simpleSchema, - inParent: &ListElemStruct1{}, - inPath: mustPath("/key1"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - inOpts: []SetNodeOpt{&ReverseShadowPaths{}}, - want: ygot.String("hello"), + inDesc: "success setting string field in top node with reverseShadowPath=true where shadow-path doesn't exist", + inSchema: simpleSchema, + inParent: &ListElemStruct1{}, + inPath: mustPath("/key1"), + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, + inOpts: []SetNodeOpt{&ReverseShadowPaths{}}, + wantLeaf: ygot.String("hello"), + wantParent: &ListElemStruct1{Key1: ygot.String("hello")}, }, { inDesc: "failure setting uint field in top node with int value", @@ -1792,22 +1734,24 @@ func TestSetNode(t *testing.T) { wantErrSubstring: "failed to unmarshal", }, { - inDesc: "success setting uint field in uint node with positive int value with JSON tolerance is set", - inSchema: listElemStruct4Schema, - inParent: &ListElemStruct4{}, - inPath: mustPath("/key1"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 42}}, - want: ygot.Uint32(42), - inOpts: []SetNodeOpt{&TolerateJSONInconsistencies{}}, + inDesc: "success setting uint field in uint node with positive int value with JSON tolerance is set", + inSchema: listElemStruct4Schema, + inParent: &ListElemStruct4{}, + inPath: mustPath("/key1"), + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 42}}, + inOpts: []SetNodeOpt{&TolerateJSONInconsistencies{}}, + wantLeaf: ygot.Uint32(42), + wantParent: &ListElemStruct4{Key1: ygot.Uint32(42)}, }, { - inDesc: "success setting uint field in uint node with 0 int value with JSON tolerance is set", - inSchema: listElemStruct4Schema, - inParent: &ListElemStruct4{}, - inPath: mustPath("/key1"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 0}}, - want: ygot.Uint32(0), - inOpts: []SetNodeOpt{&TolerateJSONInconsistencies{}}, + inDesc: "success setting uint field in uint node with 0 int value with JSON tolerance is set", + inSchema: listElemStruct4Schema, + inParent: &ListElemStruct4{}, + inPath: mustPath("/key1"), + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 0}}, + inOpts: []SetNodeOpt{&TolerateJSONInconsistencies{}}, + wantLeaf: ygot.Uint32(0), + wantParent: &ListElemStruct4{Key1: ygot.Uint32(0)}, }, { inDesc: "failure setting uint field in uint node with negative int value with JSON tolerance is set", @@ -1815,8 +1759,8 @@ func TestSetNode(t *testing.T) { inParent: &ListElemStruct4{}, inPath: mustPath("/key1"), inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: -42}}, - wantErrSubstring: "failed to unmarshal", inOpts: []SetNodeOpt{&TolerateJSONInconsistencies{}}, + wantErrSubstring: "failed to unmarshal", }, { inDesc: "fail setting value for node with non-leaf schema", @@ -1832,7 +1776,10 @@ func TestSetNode(t *testing.T) { inParent: &ListElemStruct1{}, inPath: mustPath("/@annotation"), inVal: &ExampleAnnotation{ConfigSource: "devicedemo"}, - want: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + wantLeaf: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + wantParent: &ListElemStruct1{ + Annotation: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + }, }, { inDesc: "success setting annotation in inner node", @@ -1841,7 +1788,14 @@ func TestSetNode(t *testing.T) { inPath: mustPath("/outer/inner/@annotation"), inVal: &ExampleAnnotation{ConfigSource: "devicedemo"}, inOpts: []SetNodeOpt{&InitMissingElements{}}, - want: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + wantLeaf: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + wantParent: &ListElemStruct1{ + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Annotation: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + }, + }, + }, }, { inDesc: "success setting int32 field in inner node", @@ -1850,7 +1804,14 @@ func TestSetNode(t *testing.T) { inPath: mustPath("/outer/inner/int32-leaf-field"), inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 42}}, inOpts: []SetNodeOpt{&InitMissingElements{}}, - want: ygot.Int32(42), + wantLeaf: ygot.Int32(42), + wantParent: &ListElemStruct1{ + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Int32LeafName: ygot.Int32(42), + }, + }, + }, }, { inDesc: "success setting int32 leaf list field", @@ -1862,8 +1823,16 @@ func TestSetNode(t *testing.T) { Element: []*gpb.TypedValue{ {Value: &gpb.TypedValue_IntVal{IntVal: 42}}, }}, - }}, inOpts: []SetNodeOpt{&InitMissingElements{}}, - want: []int32{42}, + }}, + inOpts: []SetNodeOpt{&InitMissingElements{}}, + wantLeaf: []int32{42}, + wantParent: &ListElemStruct1{ + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Int32LeafListName: []int32{42}, + }, + }, + }, }, { inDesc: "success setting int32 leaf list field for an existing leaf list", @@ -1882,8 +1851,15 @@ func TestSetNode(t *testing.T) { {Value: &gpb.TypedValue_IntVal{IntVal: 43}}, }}, }}, - inOpts: []SetNodeOpt{&InitMissingElements{}}, - want: []int32{42, 43}, + inOpts: []SetNodeOpt{&InitMissingElements{}}, + wantLeaf: []int32{42, 43}, + wantParent: &ListElemStruct1{ + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Int32LeafListName: []int32{42, 43}, + }, + }, + }, }, { inDesc: "success setting annotation in list element", @@ -1896,9 +1872,18 @@ func TestSetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/simple-key-list[key1=forty-two]/@annotation"), - inVal: &ExampleAnnotation{ConfigSource: "devicedemo"}, - want: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + inPath: mustPath("/config/simple-key-list[key1=forty-two]/@annotation"), + inVal: &ExampleAnnotation{ConfigSource: "devicedemo"}, + wantLeaf: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + wantParent: &ContainerStruct1{ + StructKeyList: map[string]*ListElemStruct1{ + "forty-two": { + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{}, + Annotation: []ygot.Annotation{&ExampleAnnotation{ConfigSource: "devicedemo"}}, + }, + }, + }, }, { inDesc: "failed to set annotation in invalid list element", @@ -1949,9 +1934,21 @@ func TestSetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 43}}, - want: ygot.Int32(43), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 43}}, + wantLeaf: ygot.Int32(43), + wantParent: &ContainerStruct1{ + StructKeyList: map[string]*ListElemStruct1{ + "forty-two": { + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Int32LeafName: ygot.Int32(43), + }, + }, + }, + }, + }, }, { inDesc: "success ignoring already-set shadow leaf", @@ -1968,9 +1965,21 @@ func TestSetNode(t *testing.T) { }, }, }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 43}}, - want: nil, + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/int32-leaf-field"), + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 43}}, + wantLeaf: nil, + wantParent: &ContainerStruct1{ + StructKeyList: map[string]*ListElemStruct1{ + "forty-two": { + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Int32LeafName: ygot.Int32(42), + }, + }, + }, + }, + }, }, { inDesc: "success setting non-shadow leaf", @@ -1983,42 +1992,41 @@ func TestSetNode(t *testing.T) { }, }, }, - inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/string-leaf-field"), - inOpts: []SetNodeOpt{&InitMissingElements{}}, - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - want: ygot.String("hello"), - }, - { - inDesc: "success ignore setting shadow leaf", - inSchema: containerWithStringKey, - inParent: &ContainerStruct1{ + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/string-leaf-field"), + inOpts: []SetNodeOpt{&InitMissingElements{}}, + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, + wantLeaf: ygot.String("hello"), + wantParent: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{}, + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + StringLeafName: ygot.String("hello"), + }, + }, }, }, }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/string-leaf-field"), - inOpts: []SetNodeOpt{&InitMissingElements{}}, - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - want: nil, }, { - inDesc: "fail setting shadow leaf that doesn't exist", + inDesc: "success ignore setting shadow leaf", inSchema: containerWithStringKey, - inParent: &ContainerStruct1{ + inParent: &ContainerStruct1{}, + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/string-leaf-field"), + inOpts: []SetNodeOpt{&InitMissingElements{}}, + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, + wantLeaf: nil, + wantParent: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{}, + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{}, + }, }, }, }, - inOpts: []SetNodeOpt{&InitMissingElements{}}, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/INVALID-LEAF"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - wantErrSubstring: "no match found in *ytypes.InnerContainerType1", }, { inDesc: "success setting already-set shadow leaf when reverseShadowPath=true", @@ -2035,45 +2043,44 @@ func TestSetNode(t *testing.T) { }, }, }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/int32-leaf-field"), - inOpts: []SetNodeOpt{&ReverseShadowPaths{}}, - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 43}}, - want: ygot.Int32(43), - }, - { - inDesc: "success ignoring non-shadow leaf when reverseShadowPath=true", - inSchema: containerWithStringKey, - inParent: &ContainerStruct1{ + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/state/int32-leaf-field"), + inOpts: []SetNodeOpt{&ReverseShadowPaths{}}, + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_IntVal{IntVal: 43}}, + wantLeaf: ygot.Int32(43), + wantParent: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{}, + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{ + Int32LeafName: ygot.Int32(43), + }, + }, }, }, }, - inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/string-leaf-field"), - inOpts: []SetNodeOpt{&InitMissingElements{}, &ReverseShadowPaths{}}, - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - want: nil, }, { - inDesc: "fail setting non-shadow leaf that doesn't exist when reverseShadowPath=true", + inDesc: "success ignoring non-shadow leaf when reverseShadowPath=true", inSchema: containerWithStringKey, - inParent: &ContainerStruct1{ + inParent: &ContainerStruct1{}, + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/inner/string-leaf-field"), + inOpts: []SetNodeOpt{&InitMissingElements{}, &ReverseShadowPaths{}}, + inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, + wantLeaf: nil, + wantParent: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{}, + Key1: ygot.String("forty-two"), + Outer: &OuterContainerType1{ + Inner: &InnerContainerType1{}, + }, }, }, }, - inOpts: []SetNodeOpt{&InitMissingElements{}, &ReverseShadowPaths{}}, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner/INVALID-LEAF"), - inVal: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "hello"}}, - wantErrSubstring: "no match found in *ytypes.InnerContainerType1", }, { - inDesc: "fail setting shadow leaf that doesn't exist when reverseShadowPath=true", + inDesc: "fail setting leaf that doesn't exist when reverseShadowPath=true", inSchema: containerWithStringKey, inParent: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ @@ -2099,6 +2106,10 @@ func TestSetNode(t *testing.T) { if err != nil { return } + if diff := cmp.Diff(tt.wantParent, tt.inParent); diff != "" { + t.Errorf("(-wantParent, +got):\n%s", diff) + } + var getNodeOpts []GetNodeOpt if hasSetNodeReverseShadowPaths(tt.inOpts) { getNodeOpts = append(getNodeOpts, &ReverseShadowPaths{}) @@ -2111,14 +2122,14 @@ func TestSetNode(t *testing.T) { case len(treeNode) == 1: // Expected case for most tests. break - case len(treeNode) == 0 && tt.want == nil: + case len(treeNode) == 0 && tt.wantLeaf == nil: return default: t.Fatalf("did not get exactly one tree node: %v", treeNode) } got := treeNode[0].Data - if diff := cmp.Diff(tt.want, got); diff != "" { - t.Errorf("(-want, +got):\n%s", diff) + if diff := cmp.Diff(tt.wantLeaf, got); diff != "" { + t.Errorf("(-wantLeaf, +got):\n%s", diff) } }) } @@ -2365,98 +2376,7 @@ func TestDeleteNode(t *testing.T) { }, }, }, { - name: "success ignore deleting an inner node in list", - inSchema: containerWithStringKey, - inRoot: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, - }, - }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/inner"), - want: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, - }, - }, - }, { - name: "success ignore deleting a shadow list entry", - inSchema: containerWithStringKey, - inRoot: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, - }, - }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]"), - want: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, - }, - }, - }, { - name: "failure deleting a non-existent shadow list inner node", - inSchema: containerWithStringKey, - inRoot: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, - }, - }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/INVALID"), - wantErrSubstring: "no match found in *ytypes.OuterContainerType1", - }, { - name: "success deleting a shadow list entry with reverseShadowPath=true", - inSchema: containerWithStringKey, - inRoot: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, - }, - }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]"), - inOpts: []DelNodeOpt{&ReverseShadowPaths{}}, - want: &ContainerStruct1{ - StructKeyList: map[string]*ListElemStruct1{ - "forty-one": { - Key1: ygot.String("forty-one"), - }, - }, - }, - }, { - name: "success ignore deleting a non-shadow list entry with reverseShadowPath=true", + name: "success deleting a list entry with reverseShadowPath=true", inSchema: containerWithStringKey, inRoot: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ @@ -2476,14 +2396,10 @@ func TestDeleteNode(t *testing.T) { "forty-one": { Key1: ygot.String("forty-one"), }, - "forty-two": { - Key1: ygot.String("forty-two"), - Outer: &OuterContainerType1{Inner: &InnerContainerType1{Int32LeafName: ygot.Int32(5)}}, - }, }, }, }, { - name: "failure deleting a non-existing shadow list inner node with reverseShadowPath=true", + name: "failure deleting a non-existing list inner node with reverseShadowPath=true", inSchema: containerWithStringKey, inRoot: &ContainerStruct1{ StructKeyList: map[string]*ListElemStruct1{ @@ -2496,7 +2412,7 @@ func TestDeleteNode(t *testing.T) { }, }, }, - inPath: mustPath("/state/simple-key-list[key1=forty-two]/outer/INVALID"), + inPath: mustPath("/config/simple-key-list[key1=forty-two]/outer/INVALID"), inOpts: []DelNodeOpt{&ReverseShadowPaths{}}, wantErrSubstring: "no match found in *ytypes.OuterContainerType1", }}