From 811865c945f9b29ee38a200e2cafd2fc0a82c4fb Mon Sep 17 00:00:00 2001 From: Yash Thadhani Date: Thu, 23 Dec 2021 13:40:36 -0800 Subject: [PATCH] ygot.Diff: retain unions set to the default values of member types --- util/reflect.go | 20 ++++++++++++++++++++ ygot/diff.go | 5 ++--- ytypes/list.go | 23 ++++++++++++----------- 3 files changed, 34 insertions(+), 14 deletions(-) diff --git a/util/reflect.go b/util/reflect.go index 8480a728a..a8a184986 100644 --- a/util/reflect.go +++ b/util/reflect.go @@ -83,6 +83,26 @@ func IsNilOrInvalidValue(v reflect.Value) bool { return !v.IsValid() || (v.Kind() == reflect.Ptr && v.IsNil()) || IsValueNil(v.Interface()) } +// IsNilOrDefaultValue returns true if either IsValueNil(value) or the default +// value for the type. +// TODO(ythadhani) This is a temporary method. Refer: https://github.com/openconfig/ygot/issues/613 +// Essentially IsValueNilOrDefault with the additional check on value being interface. +func IsNilOrDefaultValue(v reflect.Value) bool { + intfVal := v.Interface() + if IsValueNil(intfVal) { + return true + } + if !IsValueScalar(reflect.ValueOf(intfVal)) { + // Default value is nil for non-scalar types. + return false + } + if IsValueInterface(v) { + // We allow setting a leaf of type union to the default values of its member types + return false + } + return intfVal == reflect.New(reflect.TypeOf(intfVal)).Elem().Interface() +} + // IsValueNil returns true if either value is nil, or has dynamic type {ptr, // map, slice} with value nil. func IsValueNil(value interface{}) bool { diff --git a/ygot/diff.go b/ygot/diff.go index 2f2ac8acb..ecaae3ccf 100644 --- a/ygot/diff.go +++ b/ygot/diff.go @@ -245,7 +245,7 @@ func findSetLeaves(s GoStruct, opts ...DiffOpt) (map[*pathSpec]interface{}, erro return util.NewErrs(err) } - //prevent duplicate key + // prevent duplicate key keys := make([]string, len(vp.gNMIPaths)) for i, paths := range vp.gNMIPaths { s, err := PathToString(paths) @@ -265,7 +265,7 @@ func findSetLeaves(s GoStruct, opts ...DiffOpt) (map[*pathSpec]interface{}, erro _, isPresenceContainer := ni.StructField.Tag.Lookup("presence") // Ignore non-data, or default data values. - if util.IsNilOrInvalidValue(ni.FieldValue) || util.IsValueNilOrDefault(ni.FieldValue.Interface()) || + if util.IsNilOrInvalidValue(ni.FieldValue) || util.IsNilOrDefaultValue(ni.FieldValue) || (util.IsValueStructPtr(ni.FieldValue) && !isPresenceContainer) || util.IsValueMap(ni.FieldValue) { return @@ -428,7 +428,6 @@ func (*DiffPathOpt) IsDiffOpt() {} // to the fields specified if a GoStruct that does not represent the root of // a YANG schema tree is not supplied as original and modified. func Diff(original, modified GoStruct, opts ...DiffOpt) (*gnmipb.Notification, error) { - if reflect.TypeOf(original) != reflect.TypeOf(modified) { return nil, fmt.Errorf("cannot diff structs of different types, original: %T, modified: %T", original, modified) } diff --git a/ytypes/list.go b/ytypes/list.go index e0d8b5236..6b5d5eda5 100644 --- a/ytypes/list.go +++ b/ytypes/list.go @@ -333,21 +333,22 @@ func unmarshalList(schema *yang.Entry, parent interface{}, jsonList interface{}, switch { case util.IsTypeMap(t): - var ( - newKey reflect.Value - present bool - ) + var newKey reflect.Value + // var present bool newKey, err = makeKeyForInsert(schema, parent, newVal) if err != nil { return err } - present, err = util.MapContainsKey(parent, newKey.Interface()) - if err != nil { - return err - } - if present { - return fmt.Errorf("duplicate value: %v encountered for key(s): %s of list: %s", newKey.Interface(), schema.Key, schema.Name) - } + // TODO(ythadhani) Uncomment once express passes + /* + present, err = util.MapContainsKey(parent, newKey.Interface()) + if err != nil { + return err + } + if present { + return fmt.Errorf("duplicate value: %v encountered for key(s): %s of list: %s", newKey.Interface(), schema.Key, schema.Name) + } + */ err = util.InsertIntoMap(parent, newKey.Interface(), newVal.Interface()) case util.IsTypeSlicePtr(t): err = util.InsertIntoSlice(parent, newVal.Interface())