Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added UnmarshalOpt to allow a best effort unmarshal #863

Merged
merged 17 commits into from
Jun 26, 2023
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 54 additions & 9 deletions ytypes/gnmi.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,29 +65,48 @@ func UnmarshalNotifications(schema *Schema, ns []*gpb.Notification, opts ...Unma
func UnmarshalSetRequest(schema *Schema, req *gpb.SetRequest, opts ...UnmarshalOpt) error {
preferShadowPath := hasPreferShadowPath(opts)
ignoreExtraFields := hasIgnoreExtraFields(opts)
bestEffortUnmarshal := hasBestEffortUnmarshal(opts)
if req == nil {
return nil
}
root := schema.Root
rootName := reflect.TypeOf(root).Elem().Name()

var complianceErrs *ComplianceErrors

// Process deletes, then replace, then updates.
if err := deletePaths(schema.SchemaTree[rootName], root, req.Prefix, req.Delete, preferShadowPath); err != nil {
return err
if err := deletePaths(schema.SchemaTree[rootName], root, req.Prefix, req.Delete, preferShadowPath, bestEffortUnmarshal); err != nil {
if bestEffortUnmarshal {
complianceErrs = complianceErrs.append(err.(*ComplianceErrors).Errors...)
} else {
return err
}
}
if err := replacePaths(schema.SchemaTree[rootName], root, req.Prefix, req.Replace, preferShadowPath, ignoreExtraFields); err != nil {
return err
if err := replacePaths(schema.SchemaTree[rootName], root, req.Prefix, req.Replace, preferShadowPath, ignoreExtraFields, bestEffortUnmarshal); err != nil {
if bestEffortUnmarshal{
complianceErrs = complianceErrs.append(err.(*ComplianceErrors).Errors...)
} else {
return err
}
}
if err := updatePaths(schema.SchemaTree[rootName], root, req.Prefix, req.Update, preferShadowPath, ignoreExtraFields); err != nil {
return err
if err := updatePaths(schema.SchemaTree[rootName], root, req.Prefix, req.Update, preferShadowPath, ignoreExtraFields, bestEffortUnmarshal); err != nil {
if bestEffortUnmarshal {
complianceErrs = complianceErrs.append(err.(*ComplianceErrors).Errors...)
} else {
return err
}
}

if bestEffortUnmarshal && complianceErrs != nil {
return complianceErrs
}
return nil
}

// deletePaths deletes a slice of paths from the given GoStruct.
func deletePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, paths []*gpb.Path, preferShadowPath bool) error {
func deletePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, paths []*gpb.Path, preferShadowPath, bestEffortUnmarshal bool) error {
var dopts []DelNodeOpt
var ce *ComplianceErrors
if preferShadowPath {
dopts = append(dopts, &PreferShadowPath{})
}
Expand All @@ -100,9 +119,17 @@ func deletePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, p
}
}
if err := DeleteNode(schema, goStruct, path, dopts...); err != nil {
if bestEffortUnmarshal {
ce = ce.append(err)
continue
}
return err
}
}

if bestEffortUnmarshal && ce != nil {
return ce
}
return nil
}

Expand Down Expand Up @@ -130,8 +157,9 @@ func joinPrefixToUpdate(prefix *gpb.Path, update *gpb.Update) (*gpb.Update, erro
// replacePaths unmarshals a slice of updates into the given GoStruct. It
// deletes the values at these paths before unmarshalling them. These updates
// can either by JSON-encoded or gNMI-encoded values (scalars).
func replacePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, updates []*gpb.Update, preferShadowPath, ignoreExtraFields bool) error {
func replacePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, updates []*gpb.Update, preferShadowPath, ignoreExtraFields, bestEffortUnmarshal bool) error {
var dopts []DelNodeOpt
var ce *ComplianceErrors
if preferShadowPath {
dopts = append(dopts, &PreferShadowPath{})
}
Expand All @@ -145,24 +173,41 @@ func replacePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path,
return err
}
if err := setNode(schema, goStruct, update, preferShadowPath, ignoreExtraFields); err != nil {
if bestEffortUnmarshal {
ce = ce.append(err)
continue
}
return err
}
}
if bestEffortUnmarshal && ce != nil {
return ce
}
return nil
}

// updatePaths unmarshals a slice of updates into the given GoStruct. These
// updates can either by JSON-encoded or gNMI-encoded values (scalars).
func updatePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, updates []*gpb.Update, preferShadowPath, ignoreExtraFields bool) error {
func updatePaths(schema *yang.Entry, goStruct ygot.GoStruct, prefix *gpb.Path, updates []*gpb.Update, preferShadowPath, ignoreExtraFields, bestEffortUnmarshal bool) error {
var ce *ComplianceErrors

for _, update := range updates {
var err error
if update, err = joinPrefixToUpdate(prefix, update); err != nil {
return err
}
if err := setNode(schema, goStruct, update, preferShadowPath, ignoreExtraFields); err != nil {
if bestEffortUnmarshal {
ce = ce.append(err)
continue
}
return err
}
}

if bestEffortUnmarshal && ce != nil {
return ce
}
return nil
}

Expand Down
147 changes: 147 additions & 0 deletions ytypes/gnmi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
package ytypes

import (
"errors"
"testing"

"github.com/google/go-cmp/cmp"
Expand All @@ -32,6 +33,7 @@ func TestUnmarshalSetRequest(t *testing.T) {
inUnmarshalOpts []UnmarshalOpt
want ygot.GoStruct
wantErr bool
numErrs int
}{{
desc: "nil input",
inSchema: &Schema{
Expand Down Expand Up @@ -496,6 +498,141 @@ func TestUnmarshalSetRequest(t *testing.T) {
}},
},
wantErr: true,
}, {
desc: "mix of an error and a non-error update with best-effort flag",
lgomez9 marked this conversation as resolved.
Show resolved Hide resolved
wenovus marked this conversation as resolved.
Show resolved Hide resolved
inSchema: &Schema{
Root: &ListElemStruct1{
Key1: ygot.String("mixedupdate"),
Outer: &OuterContainerType1{
Inner: &InnerContainerType1{
Int32LeafName: ygot.Int32(43),
Int32LeafListName: []int32{100},
StringLeafName: ygot.String("bear"),
},
},
},
SchemaTree: map[string]*yang.Entry{
"ListElemStruct1": simpleSchema(),
},
},
inReq: &gpb.SetRequest{
Prefix: &gpb.Path{},
Update: []*gpb.Update{{
Path: mustPath("/key1"),
Val: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "non-error"}},
}, {
Path: mustPath("/outer/error"),
Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{
JsonIetfVal: []byte(`
{
"int32-leaf-list": [42]
}
`),
}},
}, {
Path: mustPath("/outer/error2"),
Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{
JsonIetfVal: []byte(`
{
"int32-leaf-list": [42]
}
`),
}},
}},
},
inUnmarshalOpts: []UnmarshalOpt{&BestEffortUnmarshal{}},
want: &ListElemStruct1{
Key1: ygot.String("non-error"),
Outer: &OuterContainerType1{
Inner: &InnerContainerType1{
Int32LeafName: ygot.Int32(43),
Int32LeafListName: []int32{100},
StringLeafName: ygot.String("bear"),
},
},
},
wantErr: true,
numErrs: 2,
}, {
desc: "mix of an error and a non-error replace with best-effort flag",
inSchema: &Schema{
Root: &ListElemStruct1{
Key1: ygot.String("mixedreplace"),
Outer: &OuterContainerType1{
Inner: &InnerContainerType1{
Int32LeafName: ygot.Int32(43),
Int32LeafListName: []int32{100},
StringLeafName: ygot.String("bear"),
},
},
},
SchemaTree: map[string]*yang.Entry{
"ListElemStruct1": simpleSchema(),
},
},
inReq: &gpb.SetRequest{
Prefix: &gpb.Path{},
Replace: []*gpb.Update{{
Path: mustPath("/key1"),
Val: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "success"}},
}, {
Path: mustPath("/key2"),
Val: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "failure"}},
}, {
Path: mustPath("/key3"),
Val: &gpb.TypedValue{Value: &gpb.TypedValue_StringVal{StringVal: "failure"}},
}},
},
inUnmarshalOpts: []UnmarshalOpt{&BestEffortUnmarshal{}},
want: &ListElemStruct1{
Key1: ygot.String("success"),
Outer: &OuterContainerType1{
Inner: &InnerContainerType1{
Int32LeafName: ygot.Int32(43),
Int32LeafListName: []int32{100},
StringLeafName: ygot.String("bear"),
},
},
},
wantErr: true,
numErrs: 2,
}, {
desc: "mix of an error and a non-error delete with best-effort flag",
inSchema: &Schema{
Root: &ListElemStruct1{
Key1: ygot.String("mixeddelete"),
Outer: &OuterContainerType1{
Inner: &InnerContainerType1{
Int32LeafName: ygot.Int32(43),
Int32LeafListName: []int32{100},
StringLeafName: ygot.String("bear"),
},
},
},
SchemaTree: map[string]*yang.Entry{
"ListElemStruct1": simpleSchema(),
},
},
inReq: &gpb.SetRequest{
Prefix: &gpb.Path{},
Delete: []*gpb.Path{
mustPath("/outer/inner/config/int32-leaf-field"),
mustPath("/outer/error"),
mustPath("/outer/error2"),
},
},
inUnmarshalOpts: []UnmarshalOpt{&BestEffortUnmarshal{}},
want: &ListElemStruct1{
Key1: ygot.String("mixeddelete"),
Outer: &OuterContainerType1{
Inner: &InnerContainerType1{
Int32LeafListName: []int32{100},
StringLeafName: ygot.String("bear"),
},
},
},
wantErr: true,
numErrs: 2,
}}

for _, tt := range tests {
Expand All @@ -504,6 +641,16 @@ func TestUnmarshalSetRequest(t *testing.T) {
if gotErr := err != nil; gotErr != tt.wantErr {
t.Fatalf("got error: %v, want: %v", err, tt.wantErr)
}
if gotErr := err != nil; gotErr && hasBestEffortUnmarshal(tt.inUnmarshalOpts) {
wenovus marked this conversation as resolved.
Show resolved Hide resolved
var ce *ComplianceErrors
if errors.As(err, &ce) {
if len(ce.Errors) != tt.numErrs {
t.Fatalf("Got the incorrect number of errors: want %v, got %v", tt.numErrs, len(ce.Errors))
}
} else {
t.Fatalf("Error casting BestEffortUnmarshal result to compliance errors struct")
}
}
if !tt.wantErr {
if diff := cmp.Diff(tt.inSchema.Root, tt.want); diff != "" {
t.Errorf("(-got, +want):\n%s", diff)
Expand Down
56 changes: 56 additions & 0 deletions ytypes/unmarshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package ytypes
import (
"errors"
"fmt"
"strings"

"github.com/openconfig/goyang/pkg/yang"
"github.com/openconfig/ygot/util"
Expand All @@ -29,6 +30,49 @@ type UnmarshalOpt interface {
IsUnmarshalOpt()
}

// ComplianceErrors contains the compliance errors encountered from an Unmarshal operation.
type ComplianceErrors struct {
// Errors represent generic errors for now, until we make a decision on what specific types
// of errors should be returned.
Errors []error
}

func (c *ComplianceErrors) Error() string {
if c == nil {
return ""
}

var b strings.Builder
b.WriteString("Noncompliance errors:")
if len(c.Errors) != 0 {
for _, e := range c.Errors {
b.WriteString("\n\t")
b.WriteString(e.Error())
}
} else {
b.WriteString(" None")
}
b.WriteString("\n")
return b.String()
}
lgomez9 marked this conversation as resolved.
Show resolved Hide resolved

func (c *ComplianceErrors) append(errs... error) *ComplianceErrors {
if c == nil {
return &ComplianceErrors{Errors: errs}
}

c.Errors = append(c.Errors, errs...)
return c
}

// BestEffortUnmarshal is an unmarshal option that accumulates errors while unmarshalling,
// and continues the unmarshaling process. An unmarshal now return a ComplianceErrors struct,
// instead of a single error.
type BestEffortUnmarshal struct{}
wenovus marked this conversation as resolved.
Show resolved Hide resolved

// IsUnmarshalOpt marks BestEffortUnmarshal as a valid UnmarshalOpt.
func (*BestEffortUnmarshal) IsUnmarshalOpt() {}

// IgnoreExtraFields is an unmarshal option that controls the
// behaviour of the Unmarshal function when additional fields are
// found in the input JSON. By default, an error will be returned,
Expand Down Expand Up @@ -123,3 +167,15 @@ func hasPreferShadowPath(opts []UnmarshalOpt) bool {
}
return false
}

// hasBestEffortUnmarshal determines whether the supplied slice of UnmarshalOpts
// contains the BestEffortUnmarshal option.
func hasBestEffortUnmarshal(opts []UnmarshalOpt) bool {
for _, o := range opts {
if _, ok := o.(*BestEffortUnmarshal); ok {
return true
}
}

return false
}