Allow extra value when unmarshaling using gnmi.Get (#81)

* Allow extra value when unmarshaling using gnmi.Get * don't automatically change the year
openconfig · Jan 10, 2023 · d99cedd · d99cedd
1 parent f87058c
commit d99cedd
Show file tree

Hide file tree

Showing 5 changed files with 161 additions and 73 deletions.
diff --git 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
@@ -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
@@ -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
@@ -231,15 +231,16 @@ 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)
 	}
 	data, err := receiveAll(sub, false)
 	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
@@ -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) {