Honor origin set in gNMI path in addition to gNMI prefix

PiperOrigin-RevId: 224601096

path/path.go

@@ -19,6 +19,7 @@ limitations under the License.
 package path
 
 import (
+	"errors"
 	"sort"
 
 	gpb "github.com/openconfig/gnmi/proto/gnmi"
@@ -82,3 +83,33 @@ func sortedVals(m map[string]string) []string {
 	}
 	return vs
 }
+
+// CompletePath joins provided prefix and subscription paths. Also, verifies
+// whether origin is set properly according to:
+// https://github.com/openconfig/reference/blob/master/rpc/gnmi/mixed-schema.md
+// Note that this function doesn't add "openconfig" default origin if neither
+// prefix nor path specifies the origin. Also, target field isn't prepended in
+// the returned path.
+func CompletePath(prefix, path *gpb.Path) ([]string, error) {
+	oPre, oPath := prefix.GetOrigin(), path.GetOrigin()
+
+	var fullPrefix []string
+	indexedPrefix := ToStrings(prefix, false)
+	switch {
+	case oPre != "" && oPath != "":
+		return nil, errors.New("origin is set both in prefix and path")
+	case oPre != "":
+		fullPrefix = append(fullPrefix, oPre)
+		fullPrefix = append(fullPrefix, indexedPrefix...)
+	case oPath != "":
+		if len(indexedPrefix) > 0 {
+			return nil, errors.New("path elements in prefix are set even though origin is set in path")
+		}
+		fullPrefix = append(fullPrefix, oPath)
+	default:
+		// Neither prefix nor path specified an origin. Include the path elements in prefix.
+		fullPrefix = append(fullPrefix, indexedPrefix...)
+	}
+
+	return append(fullPrefix, ToStrings(path, false)...), nil
+}

path/path_test.go

@@ -20,6 +20,7 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/openconfig/gnmi/errdiff"
 	gpb "github.com/openconfig/gnmi/proto/gnmi"
 )
 
@@ -149,3 +150,50 @@ func TestToStrings(t *testing.T) {
 		}
 	}
 }
+
+func TestCompletePath(t *testing.T) {
+	tests := []struct {
+		desc          string
+		inPrefix      *gpb.Path
+		inPath        *gpb.Path
+		wantSlice     []string
+		wantErrSubstr string
+	}{
+		{
+			desc:      "origin is just set in prefix",
+			inPrefix:  &gpb.Path{Target: "t", Origin: "o"},
+			wantSlice: []string{"o"},
+		},
+		{
+			desc:          "origin is set both in prefix and path",
+			inPrefix:      &gpb.Path{Target: "t", Origin: "o"},
+			inPath:        &gpb.Path{Origin: "o"},
+			wantErrSubstr: "origin is set both in prefix and path",
+		},
+		{
+			desc:          "origin is set in path, but prefix has path elements",
+			inPrefix:      &gpb.Path{Target: "t", Elem: []*gpb.PathElem{{Name: "e"}}},
+			inPath:        &gpb.Path{Origin: "o"},
+			wantErrSubstr: "path elements in prefix are set even though origin is set in path",
+		},
+		{
+			desc:      "origin is set in path properly",
+			inPrefix:  &gpb.Path{Target: "t"},
+			inPath:    &gpb.Path{Origin: "o", Elem: []*gpb.PathElem{{Name: "e"}}},
+			wantSlice: []string{"o", "e"},
+		},
+	}
+
+	for _, tt := range tests {
+		gotSlice, err := CompletePath(tt.inPrefix, tt.inPath)
+		if diff := errdiff.Substring(err, tt.wantErrSubstr); diff != "" {
+			t.Errorf("%s: %v", tt.desc, diff)
+		}
+		if err != nil {
+			continue
+		}
+		if !reflect.DeepEqual(tt.wantSlice, gotSlice) {
+			t.Errorf("%s: got %v, want %v", tt.desc, gotSlice, tt.wantSlice)
+		}
+	}
+}

subscribe/subscribe.go

@@ -318,11 +318,14 @@ func (s *Server) processSubscription(c *streamClient) {
 		}()
 	}
 	if !c.sr.GetSubscribe().GetUpdatesOnly() {
-		// remove the target name from the index string
-		prefix := path.ToStrings(c.sr.GetSubscribe().Prefix, true)[1:]
 		for _, subscription := range c.sr.GetSubscribe().Subscription {
-			path := append(prefix, path.ToStrings(subscription.Path, false)...)
-			s.c.Query(c.target, path, func(_ []string, l *ctree.Leaf, _ interface{}) {
+			var fullPath []string
+			fullPath, err = path.CompletePath(c.sr.GetSubscribe().GetPrefix(), subscription.GetPath())
+			if err != nil {
+				return
+			}
+			// Note that fullPath doesn't contain target name as the first element.
+			s.c.Query(c.target, fullPath, func(_ []string, l *ctree.Leaf, _ interface{}) {
 				// Stop processing query results on error.
 				if err != nil {
 					return

subscribe/subscribe_test.go

@@ -143,6 +143,123 @@ func TestOnce(t *testing.T) {
 	}
 }
 
+func TestOriginInSubscribeRequest(t *testing.T) {
+	cache.Type = cache.GnmiNoti
+	defer func() {
+		cache.Type = cache.ClientLeaf
+	}()
+	addr, cache, teardown, err := startServer(client.Path{"dev1"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer teardown()
+
+	paths := []client.Path{
+		{"dev1", "a", "b", "c", "d"},
+		{"dev1", "a", "b", "d", "e"},
+		{"dev1", "a", "c", "d", "e"},
+	}
+	var timestamp time.Time
+	sendUpdates(t, cache, paths, &timestamp)
+
+	tests := []struct {
+		desc      string
+		inPrefix  *pb.Path
+		inPath    *pb.Path
+		wantCount int
+		wantErr   bool
+	}{
+		{
+			desc:      "no origin set",
+			inPrefix:  &pb.Path{Target: "dev1", Elem: []*pb.PathElem{{Name: "a"}}},
+			inPath:    &pb.Path{Elem: []*pb.PathElem{}},
+			wantCount: 3,
+		},
+		{
+			desc:      "origin set in prefix",
+			inPrefix:  &pb.Path{Target: "dev1", Origin: "a", Elem: []*pb.PathElem{{Name: "b"}}},
+			inPath:    &pb.Path{Elem: []*pb.PathElem{}},
+			wantCount: 2,
+		},
+		{
+			desc:     "origin set in path with path elements in prefix",
+			inPrefix: &pb.Path{Target: "dev1", Elem: []*pb.PathElem{{Name: "a"}}},
+			inPath:   &pb.Path{Origin: "b"},
+			wantErr:  true,
+		},
+		{
+			desc:      "origin set in path",
+			inPrefix:  &pb.Path{Target: "dev1"},
+			inPath:    &pb.Path{Origin: "a", Elem: []*pb.PathElem{{Name: "b"}}},
+			wantCount: 2,
+		},
+		{
+			desc:     "origin set in path and prefix",
+			inPrefix: &pb.Path{Target: "dev1", Origin: "a", Elem: []*pb.PathElem{{Name: "b"}}},
+			inPath:   &pb.Path{Origin: "c"},
+			wantErr:  true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(fmt.Sprintf("%s: prefix %v and path %v", tt.desc, tt.inPrefix, tt.inPath), func(t *testing.T) {
+			sync := 0
+			count := 0
+			q, err := client.NewQuery(&pb.SubscribeRequest{
+				Request: &pb.SubscribeRequest_Subscribe{
+					Subscribe: &pb.SubscriptionList{
+						Prefix:       tt.inPrefix,
+						Subscription: []*pb.Subscription{{Path: tt.inPath}},
+						Mode:         pb.SubscriptionList_ONCE,
+					},
+				},
+			})
+			if err != nil {
+				t.Fatalf("failed to initialize a client.Query: %v", err)
+			}
+			q.ProtoHandler = func(msg proto.Message) error {
+				resp, ok := msg.(*pb.SubscribeResponse)
+				if !ok {
+					return fmt.Errorf("failed to type assert message %#v", msg)
+				}
+				switch v := resp.Response.(type) {
+				case *pb.SubscribeResponse_Update:
+					count++
+				case *pb.SubscribeResponse_Error:
+					return fmt.Errorf("error in response: %s", v)
+				case *pb.SubscribeResponse_SyncResponse:
+					sync++
+				default:
+					return fmt.Errorf("unknown response %T: %s", v, v)
+				}
+
+				return nil
+			}
+			q.TLS = &tls.Config{InsecureSkipVerify: true}
+			q.Addrs = []string{addr}
+
+			c := client.BaseClient{}
+			err = c.Subscribe(context.Background(), q, gnmiclient.Type)
+			defer c.Close()
+			if tt.wantErr {
+				if err == nil {
+					t.Fatal("got nil, want err")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("got %v, want no error", err)
+			}
+			if sync != 1 {
+				t.Errorf("got %d sync messages, want 1", sync)
+			}
+			if count != tt.wantCount {
+				t.Errorf("got %d updates, want %d", count, tt.wantCount)
+			}
+		})
+	}
+}
+
 func TestGNMIOnce(t *testing.T) {
 	cache.Type = cache.GnmiNoti
 	defer func() {