From d99ceddf7925f02867ae1d2d4118f46ef3a7a81f Mon Sep 17 00:00:00 2001 From: Daniel Grau Date: Tue, 10 Jan 2023 13:40:19 -0800 Subject: [PATCH] Allow extra value when unmarshaling using gnmi.Get (#81) * Allow extra value when unmarshaling using gnmi.Get * don't automatically change the year --- exampleoc/gen.sh | 2 +- ygnmi/unmarshal.go | 10 +- ygnmi/unmarshal_test.go | 4 +- ygnmi/ygnmi.go | 20 ++-- ygnmi/ygnmi_test.go | 198 ++++++++++++++++++++++++++++------------ 5 files changed, 161 insertions(+), 73 deletions(-) diff --git a/exampleoc/gen.sh b/exampleoc/gen.sh index ae7d2c4..e4e1f07 100755 --- a/exampleoc/gen.sh +++ b/exampleoc/gen.sh @@ -27,4 +27,4 @@ go run ../app/ygnmi generator \ go install golang.org/x/tools/cmd/goimports@latest go install github.com/google/addlicense@latest goimports -w . -addlicense -c "Google LLC" -l apache . +addlicense -c "Google LLC" -y "2022" -l apache . diff --git a/ygnmi/unmarshal.go b/ygnmi/unmarshal.go index 82a01ad..82fb364 100644 --- a/ygnmi/unmarshal.go +++ b/ygnmi/unmarshal.go @@ -118,7 +118,7 @@ func (c *ComplianceErrors) String() string { // NOTE: The datapoints are applied in order as they are in the input slice, // *NOT* in order of their timestamps. As such, in order to correctly support // Collect calls, the input data must be sorted in order of timestamps. -func unmarshalAndExtract[T any](data []*DataPoint, q AnyQuery[T], goStruct ygot.ValidatedGoStruct) (*Value[T], error) { +func unmarshalAndExtract[T any](data []*DataPoint, q AnyQuery[T], goStruct ygot.ValidatedGoStruct, opts *opt) (*Value[T], error) { queryPath, err := resolvePath(q.PathStruct()) if err != nil { return nil, err @@ -149,7 +149,7 @@ func unmarshalAndExtract[T any](data []*DataPoint, q AnyQuery[T], goStruct ygot. return ret, nil } - unmarshalledData, complianceErrs, err := unmarshal(data, q.schema().SchemaTree[q.dirName()], goStruct, queryPath, q.schema(), q.isLeaf(), !q.IsState()) + unmarshalledData, complianceErrs, err := unmarshal(data, q.schema().SchemaTree[q.dirName()], goStruct, queryPath, q.schema(), q.isLeaf(), !q.IsState(), opts) ret.ComplianceErrors = complianceErrs if err != nil { return ret, err @@ -264,7 +264,7 @@ func unmarshalSchemaless(data []*DataPoint, val any) (bool, error) { // NOTE: The subset of datapoints includes datapoints that are value restriction noncompliant. // The second error slice are internal errors, while the returned // *ComplianceError stores the compliance errors. -func unmarshal(data []*DataPoint, structSchema *yang.Entry, structPtr ygot.ValidatedGoStruct, queryPath *gpb.Path, schema *ytypes.Schema, isLeaf, isConfig bool) ([]*DataPoint, *ComplianceErrors, error) { +func unmarshal(data []*DataPoint, structSchema *yang.Entry, structPtr ygot.ValidatedGoStruct, queryPath *gpb.Path, schema *ytypes.Schema, isLeaf, isConfig bool, opts *opt) ([]*DataPoint, *ComplianceErrors, error) { queryPathStr := pathToString(queryPath) if isLeaf { switch { @@ -346,6 +346,10 @@ func unmarshal(data []*DataPoint, structSchema *yang.Entry, structPtr ygot.Valid if isConfig { sopts = append(sopts, &ytypes.PreferShadowPath{}) } + // TODO: Fully support partial unmarshaling of JSON blobs. + if opts.useGet { + sopts = append(sopts, &ytypes.IgnoreExtraFields{}) + } // 2. Check for type compliance (since path should already be compliant). if err := ytypes.SetNode(structSchema, structPtr, relPath, dp.Value, sopts...); err == nil { unmarshalledDatapoints = append(unmarshalledDatapoints, dp) diff --git a/ygnmi/unmarshal_test.go b/ygnmi/unmarshal_test.go index 0e89b6c..8c8cd9c 100644 --- a/ygnmi/unmarshal_test.go +++ b/ygnmi/unmarshal_test.go @@ -718,7 +718,7 @@ func TestUnmarshal(t *testing.T) { for _, tt := range passingTests { t.Run(tt.name, func(t *testing.T) { - unmarshalledData, complianceErrs, err := unmarshal(tt.inData, tt.inStructSchema, tt.inStruct, tt.inQueryPath, schemaStruct(), tt.inLeaf, tt.inPreferShadowPath) + unmarshalledData, complianceErrs, err := unmarshal(tt.inData, tt.inStructSchema, tt.inStruct, tt.inQueryPath, schemaStruct(), tt.inLeaf, tt.inPreferShadowPath, &opt{}) if err != nil { t.Fatalf("unmarshal: got error, want none: %v", err) } @@ -938,7 +938,7 @@ func TestUnmarshal(t *testing.T) { for _, tt := range failingTests { t.Run(tt.name, func(t *testing.T) { - unmarshalledData, complianceErrs, errs := unmarshal(tt.inData, tt.inStructSchema, tt.inStruct, tt.inQueryPath, schemaStruct(), tt.inLeaf, tt.inPreferShadowPath) + unmarshalledData, complianceErrs, errs := unmarshal(tt.inData, tt.inStructSchema, tt.inStruct, tt.inQueryPath, schemaStruct(), tt.inLeaf, tt.inPreferShadowPath, &opt{}) if errs != nil { t.Fatalf("unmarshal: got more than one error: %v", errs) } diff --git a/ygnmi/ygnmi.go b/ygnmi/ygnmi.go index 63665bd..ebf112c 100644 --- a/ygnmi/ygnmi.go +++ b/ygnmi/ygnmi.go @@ -231,7 +231,8 @@ func WithSetFallbackEncoding() Option { // Lookup fetches the value of a SingletonQuery with a ONCE subscription. func Lookup[T any](ctx context.Context, c *Client, q SingletonQuery[T], opts ...Option) (*Value[T], error) { - sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_ONCE, resolveOpts(opts)) + resolvedOpts := resolveOpts(opts) + sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_ONCE, resolvedOpts) if err != nil { return nil, fmt.Errorf("failed to subscribe to path: %w", err) } @@ -239,7 +240,7 @@ func Lookup[T any](ctx context.Context, c *Client, q SingletonQuery[T], opts ... if err != nil { return nil, fmt.Errorf("failed to receive to data: %w", err) } - val, err := unmarshalAndExtract[T](data, q, q.goStruct()) + val, err := unmarshalAndExtract[T](data, q, q.goStruct(), resolvedOpts) if err != nil { return val, fmt.Errorf("failed to unmarshal data: %w", err) } @@ -303,7 +304,8 @@ func Watch[T any](ctx context.Context, c *Client, q SingletonQuery[T], pred func errCh: make(chan error, 1), } - sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_STREAM, resolveOpts(opts)) + resolvedOpts := resolveOpts(opts) + sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_STREAM, resolvedOpts) if err != nil { w.errCh <- err return w @@ -316,7 +318,7 @@ func Watch[T any](ctx context.Context, c *Client, q SingletonQuery[T], pred func for { select { case data := <-dataCh: - val, err := unmarshalAndExtract[T](data, q, gs) + val, err := unmarshalAndExtract[T](data, q, gs, resolvedOpts) if err != nil { w.errCh <- err return @@ -385,7 +387,8 @@ func Collect[T any](ctx context.Context, c *Client, q SingletonQuery[T], opts .. // LookupAll fetches the values of a WildcardQuery with a ONCE subscription. // It returns an empty list if no values are present at the path. func LookupAll[T any](ctx context.Context, c *Client, q WildcardQuery[T], opts ...Option) ([]*Value[T], error) { - sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_ONCE, resolveOpts(opts)) + resolvedOpts := resolveOpts(opts) + sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_ONCE, resolvedOpts) if err != nil { return nil, fmt.Errorf("failed to subscribe to path: %w", err) } @@ -405,7 +408,7 @@ func LookupAll[T any](ctx context.Context, c *Client, q WildcardQuery[T], opts . var vals []*Value[T] for _, prefix := range sortedPrefixes { goStruct := q.goStruct() - v, err := unmarshalAndExtract[T](datapointGroups[prefix], q, goStruct) + v, err := unmarshalAndExtract[T](datapointGroups[prefix], q, goStruct, resolvedOpts) if err != nil { return nil, fmt.Errorf("failed to unmarshal data: %w", err) } @@ -454,7 +457,8 @@ func WatchAll[T any](ctx context.Context, c *Client, q WildcardQuery[T], pred fu w.errCh <- err return w } - sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_STREAM, resolveOpts(opts)) + resolvedOpts := resolveOpts(opts) + sub, err := subscribe[T](ctx, c, q, gpb.SubscriptionList_STREAM, resolvedOpts) if err != nil { w.errCh <- err return w @@ -479,7 +483,7 @@ func WatchAll[T any](ctx context.Context, c *Client, q WildcardQuery[T], pred fu if _, ok := structs[pre]; !ok { structs[pre] = q.goStruct() } - val, err := unmarshalAndExtract[T](datapointGroups[pre], q, structs[pre]) + val, err := unmarshalAndExtract[T](datapointGroups[pre], q, structs[pre], resolvedOpts) if err != nil { w.errCh <- err return diff --git a/ygnmi/ygnmi_test.go b/ygnmi/ygnmi_test.go index 055051e..94fa445 100644 --- a/ygnmi/ygnmi_test.go +++ b/ygnmi/ygnmi_test.go @@ -428,67 +428,147 @@ func TestLookup(t *testing.T) { }) } - t.Run("use get", func(t *testing.T) { - tests := []struct { - desc string - stub func(s *testutil.Stubber) - wantVal *ygnmi.Value[string] - wantRequest *gpb.GetRequest - wantErr string - }{{ - desc: "success", - stub: func(s *testutil.Stubber) { - s.GetResponse(&gpb.GetResponse{ - Notification: []*gpb.Notification{{ - Timestamp: 100, - Update: []*gpb.Update{{ - Path: leafPath, - Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{JsonIetfVal: []byte(`"foo"`)}}, - }}, +} + +func TestLookupWithGet(t *testing.T) { + fakeGNMI, c := newClient(t) + leafPath := testutil.GNMIPath(t, "/remote-container/state/a-leaf") + + tests := []struct { + desc string + stub func(s *testutil.Stubber) + wantVal *ygnmi.Value[string] + wantRequest *gpb.GetRequest + wantErr string + }{{ + desc: "success", + stub: func(s *testutil.Stubber) { + s.GetResponse(&gpb.GetResponse{ + Notification: []*gpb.Notification{{ + Timestamp: 100, + Update: []*gpb.Update{{ + Path: leafPath, + Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{JsonIetfVal: []byte(`"foo"`)}}, }}, - }, nil) - }, - wantVal: (&ygnmi.Value[string]{ - Path: leafPath, - Timestamp: time.Unix(0, 100), - }).SetVal("foo"), - wantRequest: &gpb.GetRequest{ - Encoding: gpb.Encoding_JSON_IETF, - Type: gpb.GetRequest_STATE, - Prefix: &gpb.Path{}, - Path: []*gpb.Path{leafPath}, - }, - }, { - desc: "not found error", - stub: func(s *testutil.Stubber) { - s.GetResponse(nil, status.Error(codes.NotFound, "test")) - }, - wantVal: (&ygnmi.Value[string]{ - Path: leafPath, - }), - wantRequest: &gpb.GetRequest{ - Encoding: gpb.Encoding_JSON_IETF, - Type: gpb.GetRequest_STATE, - Prefix: &gpb.Path{}, - Path: []*gpb.Path{leafPath}, + }}, + }, nil) + }, + wantVal: (&ygnmi.Value[string]{ + Path: leafPath, + Timestamp: time.Unix(0, 100), + }).SetVal("foo"), + wantRequest: &gpb.GetRequest{ + Encoding: gpb.Encoding_JSON_IETF, + Type: gpb.GetRequest_STATE, + Prefix: &gpb.Path{}, + Path: []*gpb.Path{leafPath}, + }, + }, { + desc: "not found error", + stub: func(s *testutil.Stubber) { + s.GetResponse(nil, status.Error(codes.NotFound, "test")) + }, + wantVal: (&ygnmi.Value[string]{ + Path: leafPath, + }), + wantRequest: &gpb.GetRequest{ + Encoding: gpb.Encoding_JSON_IETF, + Type: gpb.GetRequest_STATE, + Prefix: &gpb.Path{}, + Path: []*gpb.Path{leafPath}, + }, + }} + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + tt.stub(fakeGNMI.Stub()) + got, err := ygnmi.Lookup(context.Background(), c, exampleocpath.Root().RemoteContainer().ALeaf().State(), ygnmi.WithUseGet()) + if err != nil { + t.Fatalf("Lookup() returned unexpected error: %v", err) + } + if diff := cmp.Diff(tt.wantVal, got, cmp.AllowUnexported(ygnmi.Value[string]{}), cmpopts.IgnoreFields(ygnmi.Value[string]{}, "RecvTimestamp"), protocmp.Transform()); diff != "" { + t.Errorf("Lookup() returned unexpected diff: %s", diff) + } + if diff := cmp.Diff(tt.wantRequest, fakeGNMI.GetRequests()[0], protocmp.Transform()); diff != "" { + t.Errorf("Lookup() GetRequest different from expected: %s", diff) + } + }) + } + + nonLeafPath := testutil.GNMIPath(t, "/parent/child") + nonLeafTests := []struct { + desc string + stub func(s *testutil.Stubber) + wantVal *ygnmi.Value[*exampleoc.Parent_Child] + wantErr string + }{{ + desc: "single leaf", + stub: func(s *testutil.Stubber) { + s.GetResponse(&gpb.GetResponse{ + Notification: []*gpb.Notification{{ + Timestamp: 100, + Update: []*gpb.Update{{ + Path: nonLeafPath, + Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{JsonIetfVal: []byte(`{"config": {"three": "ONE" }}`)}}, + }}, + }}, + }, nil) + }, + wantVal: (&ygnmi.Value[*exampleoc.Parent_Child]{ + Path: nonLeafPath, + Timestamp: time.Unix(0, 100), + }).SetVal(&exampleoc.Parent_Child{Three: exampleoc.Child_Three_ONE}), + }, { + desc: "with extra value", + stub: func(s *testutil.Stubber) { + s.GetResponse(&gpb.GetResponse{ + Notification: []*gpb.Notification{{ + Timestamp: 100, + Update: []*gpb.Update{{ + Path: nonLeafPath, + Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{JsonIetfVal: []byte(`{"config": {"three": "ONE", "ten": "ten" }}`)}}, + }}, + }}, + }, nil) + }, + wantVal: (&ygnmi.Value[*exampleoc.Parent_Child]{ + Path: nonLeafPath, + Timestamp: time.Unix(0, 100), + }).SetVal(&exampleoc.Parent_Child{Three: exampleoc.Child_Three_ONE}), + }, { + desc: "with invalid type", // TODO: When partial unmarshaling of JSON is supports, this test case should have a value. + stub: func(s *testutil.Stubber) { + s.GetResponse(&gpb.GetResponse{ + Notification: []*gpb.Notification{{ + Timestamp: 100, + Update: []*gpb.Update{{ + Path: nonLeafPath, + Val: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{JsonIetfVal: []byte(`{"config": {"three": "ONE", "one": 10 }}`)}}, + }}, + }}, + }, nil) + }, + wantVal: &ygnmi.Value[*exampleoc.Parent_Child]{ + Path: nonLeafPath, + ComplianceErrors: &ygnmi.ComplianceErrors{ + TypeErrors: []*ygnmi.TelemetryError{{ + Path: nonLeafPath, + Value: &gpb.TypedValue{Value: &gpb.TypedValue_JsonIetfVal{JsonIetfVal: []byte(`{"config": {"three": "ONE", "one": 10 }}`)}}, + }}, }, - }} - for _, tt := range tests { - t.Run(tt.desc, func(t *testing.T) { - tt.stub(fakeGNMI.Stub()) - got, err := ygnmi.Lookup(context.Background(), c, exampleocpath.Root().RemoteContainer().ALeaf().State(), ygnmi.WithUseGet()) - if err != nil { - t.Fatalf("Lookup() returned unexpected error: %v", err) - } - if diff := cmp.Diff(tt.wantVal, got, cmp.AllowUnexported(ygnmi.Value[string]{}), cmpopts.IgnoreFields(ygnmi.Value[string]{}, "RecvTimestamp"), protocmp.Transform()); diff != "" { - t.Errorf("Lookup() returned unexpected diff: %s", diff) - } - if diff := cmp.Diff(tt.wantRequest, fakeGNMI.GetRequests()[0], protocmp.Transform()); diff != "" { - t.Errorf("Lookup() GetRequest different from expected: %s", diff) - } - }) - } - }) + }, + }} + for _, tt := range nonLeafTests { + t.Run(tt.desc, func(t *testing.T) { + tt.stub(fakeGNMI.Stub()) + got, err := ygnmi.Lookup[*exampleoc.Parent_Child](context.Background(), c, exampleocpath.Root().Parent().Child().Config(), ygnmi.WithUseGet()) + if err != nil { + t.Fatalf("Lookup() returned unexpected error: %v", err) + } + if diff := cmp.Diff(tt.wantVal, got, cmp.AllowUnexported(ygnmi.Value[*exampleoc.Parent_Child]{}), cmpopts.IgnoreFields(ygnmi.TelemetryError{}, "Err"), cmpopts.IgnoreFields(ygnmi.Value[*exampleoc.Parent_Child]{}, "RecvTimestamp"), protocmp.Transform()); diff != "" { + t.Errorf("Lookup() returned unexpected diff: %s", diff) + } + }) + } } func TestGet(t *testing.T) {