Fix Wildcard entries to return empty data instead of invalid path (#299)

* Fix Wildcard entries to return empty data instead of invalid path * Add existing table test * Add check for empty json in test
sonic-net · Nov 11, 2024 · e784b6f · e784b6f
1 parent e844925
commit e784b6f
Show file tree

Hide file tree

Showing 3 changed files with 196 additions and 8 deletions.
diff --git a/gnmi_server/server_test.go b/gnmi_server/server_test.go
@@ -1297,11 +1297,11 @@ func runGnmiTestGet(t *testing.T, namespace string) {
 	},
 		{
 			desc:       "Get valid but non-existing node",
-			pathTarget: "COUNTERS_DB",
+			pathTarget: stateDBPath,
 			textPbPath: `
-			elem: <name: "MyCounters" >
-		`,
-			wantRetCode: codes.NotFound,
+				elem: <name: "TRANSCEIVER_DOM_SENSOR" >
+			`,
+			wantRetCode: codes.OK,
 		}, {
 			desc:       "Get COUNTERS_PORT_NAME_MAP",
 			pathTarget: "COUNTERS_DB",
@@ -3487,6 +3487,191 @@ func TestClientConnections(t *testing.T) {
 	}
 }
 
+func TestWildcardTableNoError(t *testing.T) {
+	s := createServer(t, 8081)
+	go runServer(t, s)
+	defer s.ForceStop()
+
+	fileName := "../testdata/NEIGH_STATE_TABLE_MAP.txt"
+	neighStateTableByte, err := ioutil.ReadFile(fileName)
+	if err != nil {
+		t.Fatalf("read file %v err: %v", fileName, err)
+	}
+
+	var neighStateTableJson interface{}
+	json.Unmarshal(neighStateTableByte, &neighStateTableJson)
+
+	tests := []struct {
+		desc     string
+		q        client.Query
+		wantNoti []client.Notification
+		poll     int
+	}{
+		{
+			desc: "poll query for NEIGH_STATE_TABLE",
+			poll: 1,
+			q: client.Query{
+				Target:  "STATE_DB",
+				Type:    client.Poll,
+				Queries: []client.Path{{"NEIGH_STATE_TABLE"}},
+				TLS:     &tls.Config{InsecureSkipVerify: true},
+			},
+			wantNoti: []client.Notification{
+				client.Update{Path: []string{"NEIGH_STATE_TABLE"}, TS: time.Unix(0, 200), Val: neighStateTableJson},
+				client.Update{Path: []string{"NEIGH_STATE_TABLE"}, TS: time.Unix(0, 200), Val: neighStateTableJson},
+			},
+		},
+	}
+	namespace, _ := sdcfg.GetDbDefaultNamespace()
+	prepareStateDb(t, namespace)
+	var mutexNoti sync.Mutex
+	for _, tt := range tests {
+
+		t.Run(tt.desc, func(t *testing.T) {
+			q := tt.q
+			q.Addrs = []string{"127.0.0.1:8081"}
+			c := client.New()
+			var gotNoti []client.Notification
+			q.NotificationHandler = func(n client.Notification) error {
+				mutexNoti.Lock()
+				if nn, ok := n.(client.Update); ok {
+					nn.TS = time.Unix(0, 200)
+					gotNoti = append(gotNoti, nn)
+				}
+				mutexNoti.Unlock()
+				return nil
+			}
+
+			wg := new(sync.WaitGroup)
+			wg.Add(1)
+
+			go func() {
+				defer wg.Done()
+				if err := c.Subscribe(context.Background(), q); err != nil {
+					t.Errorf("c.Subscribe(): got error %v, expected nil", err)
+				}
+			}()
+
+			wg.Wait()
+
+			for i := 0; i < tt.poll; i++ {
+				if err := c.Poll(); err != nil {
+					t.Errorf("c.Poll(): got error %v, expected nil", err)
+				}
+			}
+
+			mutexNoti.Lock()
+
+			if len(gotNoti) == 0 {
+				t.Errorf("expected non zero notifications")
+			}
+
+			if diff := pretty.Compare(tt.wantNoti, gotNoti); diff != "" {
+				t.Log("\n Want: \n", tt.wantNoti)
+				t.Log("\n Got: \n", gotNoti)
+				t.Errorf("unexpected updates: \n%s", diff)
+			}
+
+			mutexNoti.Unlock()
+
+			c.Close()
+		})
+	}
+}
+
+func TestNonExistentTableNoError(t *testing.T) {
+	s := createServer(t, 8081)
+	go runServer(t, s)
+	defer s.ForceStop()
+
+	fileName := "../testdata/EMPTY_JSON.txt"
+	transceiverDomSensorTableByte, err := ioutil.ReadFile(fileName)
+	if err != nil {
+		t.Fatalf("read file %v err: %v", fileName, err)
+	}
+
+	var transceiverDomSensorTableJson interface{}
+	json.Unmarshal(transceiverDomSensorTableByte, &transceiverDomSensorTableJson)
+
+	tests := []struct {
+		desc     string
+		q        client.Query
+		wantNoti []client.Notification
+		poll     int
+	}{
+		{
+			desc: "poll query for TRANSCEIVER_DOM_SENSOR",
+			poll: 1,
+			q: client.Query{
+				Target:  "STATE_DB",
+				Type:    client.Poll,
+				Queries: []client.Path{{"TRANSCEIVER_DOM_SENSOR"}},
+				TLS:     &tls.Config{InsecureSkipVerify: true},
+			},
+			wantNoti: []client.Notification{
+				client.Update{Path: []string{"TRANSCEIVER_DOM_SENSOR"}, TS: time.Unix(0, 200), Val: transceiverDomSensorTableJson},
+				client.Update{Path: []string{"TRANSCEIVER_DOM_SENSOR"}, TS: time.Unix(0, 200), Val: transceiverDomSensorTableJson},
+			},
+		},
+	}
+	namespace, _ := sdcfg.GetDbDefaultNamespace()
+	prepareStateDb(t, namespace)
+	var mutexNoti sync.Mutex
+
+	for _, tt := range tests {
+		prepareStateDb(t, namespace)
+		t.Run(tt.desc, func(t *testing.T) {
+			q := tt.q
+			q.Addrs = []string{"127.0.0.1:8081"}
+			c := client.New()
+			var gotNoti []client.Notification
+			q.NotificationHandler = func(n client.Notification) error {
+				mutexNoti.Lock()
+				if nn, ok := n.(client.Update); ok {
+					nn.TS = time.Unix(0, 200)
+					gotNoti = append(gotNoti, nn)
+				}
+				mutexNoti.Unlock()
+				return nil
+			}
+
+			wg := new(sync.WaitGroup)
+			wg.Add(1)
+
+			go func() {
+				defer wg.Done()
+				if err := c.Subscribe(context.Background(), q); err != nil {
+					t.Errorf("c.Subscribe(): got error %v, expected nil", err)
+				}
+			}()
+
+			wg.Wait()
+
+			for i := 0; i < tt.poll; i++ {
+				if err := c.Poll(); err != nil {
+					t.Errorf("c.Poll(): got error %v, expected nil", err)
+				}
+			}
+
+			mutexNoti.Lock()
+
+			if len(gotNoti) == 0 {
+				t.Errorf("expected non zero notifications")
+			}
+
+			if diff := pretty.Compare(tt.wantNoti, gotNoti); diff != "" {
+				t.Log("\n Want: \n", tt.wantNoti)
+				t.Log("\n Got: \n", gotNoti)
+				t.Errorf("unexpected updates: \n%s", diff)
+			}
+
+			mutexNoti.Unlock()
+
+			c.Close()
+		})
+	}
+}
+
 func TestConnectionDataSet(t *testing.T) {
 	s := createServer(t, 8081)
 	go runServer(t, s)

diff --git a/sonic_data_client/db_client.go b/sonic_data_client/db_client.go
@@ -670,11 +670,13 @@ func populateDbtablePath(prefix, path *gnmipb.Path, pathG2S *map[*gnmipb.Path][]
 	// <5> DB Table Key Key Field
 	switch len(stringSlice) {
 	case 2: // only table name provided
-		res, err := redisDb.Keys(tblPath.tableName + "*").Result()
-		if err != nil || len(res) < 1 {
-			log.V(2).Infof("Invalid db table Path %v %v", target, dbPath)
-			return fmt.Errorf("Failed to find %v %v %v %v", target, dbPath, err, res)
+		wildcardTableName := tblPath.tableName + "*"
+		log.V(6).Infof("Fetching all keys for %v with table name %s", target, wildcardTableName)
+		res, err := redisDb.Keys(wildcardTableName).Result()
+		if err != nil {
+			return fmt.Errorf("redis Keys op failed for %v %v, got err %v %v", target, dbPath, err, res)
 		}
+		log.V(6).Infof("Result of keys operation for %v %v, got %v", target, dbPath, res)
 		tblPath.tableKey = ""
 	case 3: // Third element could be table key; or field name in which case table name itself is the key too
 		n, err := redisDb.Exists(tblPath.tableName + tblPath.delimitor + mappedKey).Result()

diff --git a/testdata/EMPTY_JSON.txt b/testdata/EMPTY_JSON.txt
@@ -0,0 +1 @@
+{}