Skip to content

Commit

Permalink
Support shadow-paths for ordered map Diff (#911)
Browse files Browse the repository at this point in the history
  • Loading branch information
@wenovus
wenovus authored Aug 29, 2023
1 parent 8c1381e commit e206a8d
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 15 deletions.
12 changes: 7 additions & 5 deletions ygot/diff.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -535,7 +535,7 @@ func DiffWithAtomic(original, modified GoStruct, opts ...DiffOpt) ([]*gnmipb.Not
// list. This must not be empty.
// - The second return value is the prefix path that must be used in the
// atomic Notification representing the ordered list.
func orderedMapLeaves(orderedMap GoOrderedMap, parent *gnmiPath) ([]*pathval, *gnmiPath, error) {
func orderedMapLeaves(orderedMap GoOrderedMap, parent *gnmiPath, preferShadowPath bool) ([]*pathval, *gnmiPath, error) {
var errs errlist.List
var atomicLeaves []*pathval

Expand All @@ -551,7 +551,7 @@ func orderedMapLeaves(orderedMap GoOrderedMap, parent *gnmiPath) ([]*pathval, *g
errs.Add(fmt.Errorf("%v: was not a valid GoStruct", parent))
return true
}
errs.Add(findUpdatedLeaves(&atomicLeaves, goStruct, childPath))
errs.Add(findUpdatedLeaves(&atomicLeaves, goStruct, childPath, preferShadowPath))
return true
}); err != nil {
errs.Add(err)
Expand Down Expand Up @@ -579,8 +579,8 @@ func orderedMapLeaves(orderedMap GoOrderedMap, parent *gnmiPath) ([]*pathval, *g
// - If empty, then nil is returned.
// - parent is the gNMI path representing the absolute path to the ordered
// list. This must not be empty.
func orderedMapNotif(orderedMap GoOrderedMap, parent *gnmiPath, ts int64) (*gnmipb.Notification, error) {
atomicLeaves, subtreePath, err := orderedMapLeaves(orderedMap, parent)
func orderedMapNotif(orderedMap GoOrderedMap, parent *gnmiPath, ts int64, preferShadowPath bool) (*gnmipb.Notification, error) {
atomicLeaves, subtreePath, err := orderedMapLeaves(orderedMap, parent, preferShadowPath)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -644,7 +644,9 @@ func diff(original, modified GoStruct, withAtomic bool, opts ...DiffOpt) ([]*gnm
n := &gnmipb.Notification{}
processUpdate := func(path string, modVal *pathInfo) error {
if orderedMap, isOrderedMap := modVal.val.(GoOrderedMap); isOrderedMap {
notif, err := orderedMapNotif(orderedMap, newPathElemGNMIPath(modVal.path.GetElem()), 0)
diffopts := hasDiffPathOpt(opts)
preferShadowPath := diffopts != nil && diffopts.PreferShadowPath
notif, err := orderedMapNotif(orderedMap, newPathElemGNMIPath(modVal.path.GetElem()), 0, preferShadowPath)
if err != nil {
return err
}
Expand Down
76 changes: 75 additions & 1 deletion ygot/diff_exported_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,69 @@ func TestDiffOrderedMap(t *testing.T) {
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar-val"}},
}},
}},
}, {
name: "disjoint-ordered-lists-shadow-paths",
inOrig: &ctestschema.Device{
OrderedList: ctestschema.GetOrderedMap2(t),
},
inMod: &ctestschema.Device{
OrderedList: ctestschema.GetOrderedMap(t),
},
inOpts: []ygot.DiffOpt{
&ygot.DiffPathOpt{PreferShadowPath: true},
},
want: &gnmipb.Notification{
Update: []*gnmipb.Update{{
Path: mustPath(`/ordered-lists/ordered-list[key=foo]/state/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "foo"}},
}, {
Path: mustPath(`/ordered-lists/ordered-list[key=foo]/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "foo"}},
}, {
Path: mustPath(`/ordered-lists/ordered-list[key=foo]/state/value`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "foo-val"}},
}, {
Path: mustPath(`/ordered-lists/ordered-list[key=bar]/state/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar"}},
}, {
Path: mustPath(`/ordered-lists/ordered-list[key=bar]/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar"}},
}, {
Path: mustPath(`/ordered-lists/ordered-list[key=bar]/state/value`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar-val"}},
}},
Delete: []*gnmipb.Path{
mustPath(`/ordered-lists/ordered-list[key=wee]/state/key`),
mustPath(`/ordered-lists/ordered-list[key=wee]/key`),
mustPath(`/ordered-lists/ordered-list[key=wee]/state/value`),
mustPath(`/ordered-lists/ordered-list[key=woo]/state/key`),
mustPath(`/ordered-lists/ordered-list[key=woo]/key`),
mustPath(`/ordered-lists/ordered-list[key=woo]/state/value`),
},
},
wantAtomic: []*gnmipb.Notification{{
Prefix: mustPath(`ordered-lists`),
Atomic: true,
Update: []*gnmipb.Update{{
Path: mustPath(`ordered-list[key=foo]/state/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "foo"}},
}, {
Path: mustPath(`ordered-list[key=foo]/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "foo"}},
}, {
Path: mustPath(`ordered-list[key=foo]/state/value`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "foo-val"}},
}, {
Path: mustPath(`ordered-list[key=bar]/state/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar"}},
}, {
Path: mustPath(`ordered-list[key=bar]/key`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar"}},
}, {
Path: mustPath(`ordered-list[key=bar]/state/value`),
Val: &gnmipb.TypedValue{Value: &gnmipb.TypedValue_StringVal{StringVal: "bar-val"}},
}},
}},
}, {
name: "disjoint-ordered-lists-with-ignore-additions",
inOrig: &ctestschema.Device{
Expand Down Expand Up @@ -712,7 +775,18 @@ func TestDiffOrderedMap(t *testing.T) {
t.Fatalf("Unexpected type: %T", tt.inOrig)
}
schema.Root = tt.inOrig
if err := ytypes.UnmarshalNotifications(schema, got); err != nil {
var preferShadowPath bool
for _, o := range tt.inOpts {
switch v := o.(type) {
case *ygot.DiffPathOpt:
preferShadowPath = v.PreferShadowPath
}
}
var unmopts []ytypes.UnmarshalOpt
if preferShadowPath {
unmopts = append(unmopts, &ytypes.PreferShadowPath{})
}
if err := ytypes.UnmarshalNotifications(schema, got, unmopts...); err != nil {
t.Fatal(err)
}
if diff := cmp.Diff(tt.inOrig, tt.inMod, ytestutil.OrderedMapCmpOptions...); diff != "" {
Expand Down
16 changes: 8 additions & 8 deletions ygot/render.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func TogNMINotifications(s GoStruct, ts int64, cfg GNMINotificationsConfig) ([]*
}

leaves := map[*path]any{}
if err := findUpdatedLeaves(leaves, s, pfx); err != nil {
if err := findUpdatedLeaves(leaves, s, pfx, false); err != nil {
return nil, err
}

Expand All @@ -404,7 +404,7 @@ func TogNMINotifications(s GoStruct, ts int64, cfg GNMINotificationsConfig) ([]*
// Note: the returned paths use a shallow copy of the parentPath.
//
// Limitation: nested ordered lists are not supported.
func findUpdatedOrderedListLeaves(leaves any, s GoOrderedMap, parent *gnmiPath) error {
func findUpdatedOrderedListLeaves(leaves any, s GoOrderedMap, parent *gnmiPath, preferShadowPath bool) error {
var errs errlist.List

var leavesMap map[*path]any
Expand All @@ -419,7 +419,7 @@ func findUpdatedOrderedListLeaves(leaves any, s GoOrderedMap, parent *gnmiPath)
return fmt.Errorf("internal ygot error: leaves is not an expected type: %T", leaves)
}

atomicLeaves, subtreePath, err := orderedMapLeaves(s, parent)
atomicLeaves, subtreePath, err := orderedMapLeaves(s, parent, preferShadowPath)
if err != nil {
errs.Add(err)
return errs.Err()
Expand All @@ -439,7 +439,7 @@ func findUpdatedOrderedListLeaves(leaves any, s GoOrderedMap, parent *gnmiPath)
// is called recursively on them.
//
// Note: the returned paths use a shallow copy of the parentPath.
func findUpdatedLeaves(leaves any, s GoStruct, parent *gnmiPath) error {
func findUpdatedLeaves(leaves any, s GoStruct, parent *gnmiPath, preferShadowPath bool) error {
// addLeaf is the function that must be used to add a single leaf or
// atomic update to the input cache of leaves. The reason this is
// different is because atomic values must be added in a different way
Expand Down Expand Up @@ -492,7 +492,7 @@ func findUpdatedLeaves(leaves any, s GoStruct, parent *gnmiPath) error {
}
}

mapPaths, err := structTagToLibPaths(ftype, parent, false)
mapPaths, err := structTagToLibPaths(ftype, parent, preferShadowPath)
if err != nil {
errs.Add(fmt.Errorf("%v->%s: %v", parent, ftype.Name, err))
continue
Expand All @@ -513,12 +513,12 @@ func findUpdatedLeaves(leaves any, s GoStruct, parent *gnmiPath) error {
errs.Add(fmt.Errorf("%v: was not a valid GoStruct", mapPaths[0]))
continue
}
errs.Add(findUpdatedLeaves(leaves, goStruct, childPath))
errs.Add(findUpdatedLeaves(leaves, goStruct, childPath, preferShadowPath))
}
case reflect.Ptr:
if ol, ok := fval.Interface().(GoOrderedMap); ok {
// This is an ordered-map for YANG "ordered-by user" lists.
errs.Add(findUpdatedOrderedListLeaves(leaves, ol, mapPaths[0]))
errs.Add(findUpdatedOrderedListLeaves(leaves, ol, mapPaths[0], preferShadowPath))
} else {
// Otherwise this is a pointer to a struct (another YANG container), or a leaf.
switch fval.Elem().Kind() {
Expand All @@ -528,7 +528,7 @@ func findUpdatedLeaves(leaves any, s GoStruct, parent *gnmiPath) error {
errs.Add(fmt.Errorf("%v: was not a valid GoStruct", mapPaths[0]))
continue
}
errs.Add(findUpdatedLeaves(leaves, goStruct, mapPaths[0]))
errs.Add(findUpdatedLeaves(leaves, goStruct, mapPaths[0], preferShadowPath))
default:
for _, p := range mapPaths {
addLeaf(&path{p}, fval.Interface())
Expand Down
2 changes: 1 addition & 1 deletion ygot/render_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4146,7 +4146,7 @@ func TestFindUpdatedLeaves(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
gotLeaves := map[*path]any{}
if err := findUpdatedLeaves(gotLeaves, tt.in, tt.inParent); err != nil {
if err := findUpdatedLeaves(gotLeaves, tt.in, tt.inParent, false); err != nil {
if diff := errdiff.Substring(err, tt.wantErrSubstring); diff != "" {
t.Fatalf("did not get expected error, %v", err)
}
Expand Down

0 comments on commit e206a8d

Please sign in to comment.