From e0ea27f2c11b0849b439754f6a01dd97746fc50c Mon Sep 17 00:00:00 2001
From: Laurence <45508533+LaurenceLiZhixin@users.noreply.github.com>
Date: Sun, 8 May 2022 05:15:07 +0800
Subject: [PATCH] Fix 4529: Ignore Subscribe/Get wrong redis configuration type
 keys. (#1693)

* fix: 4529

Signed-off-by: LaurenceLiZhixin <382673304@qq.com>

* Fix: add test does not throw error for wrong type during get all test case of redis configuration

Signed-off-by: LaurenceLiZhixin <382673304@qq.com>

Co-authored-by: Yaron Schneider <schneider.yaron@live.com>
Co-authored-by: Ian Luo <ian.luo@gmail.com>
---
 configuration/redis/redis.go      | 39 +++++++++++++++++--------------
 configuration/redis/redis_test.go | 39 +++++++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 17 deletions(-)

diff --git a/configuration/redis/redis.go b/configuration/redis/redis.go
index a3b98d4eb..68bec7180 100644
--- a/configuration/redis/redis.go
+++ b/configuration/redis/redis.go
@@ -33,23 +33,24 @@ import (
 )
 
 const (
-	connectedSlavesReplicas  = "connected_slaves:"
-	infoReplicationDelimiter = "\r\n"
-	host                     = "redisHost"
-	password                 = "redisPassword"
-	enableTLS                = "enableTLS"
-	maxRetries               = "maxRetries"
-	maxRetryBackoff          = "maxRetryBackoff"
-	failover                 = "failover"
-	sentinelMasterName       = "sentinelMasterName"
-	defaultBase              = 10
-	defaultBitSize           = 0
-	defaultDB                = 0
-	defaultMaxRetries        = 3
-	defaultMaxRetryBackoff   = time.Second * 2
-	defaultEnableTLS         = false
-	keySpacePrefix           = "__keyspace@0__:"
-	keySpaceAny              = "__keyspace@0__:*"
+	connectedSlavesReplicas   = "connected_slaves:"
+	infoReplicationDelimiter  = "\r\n"
+	host                      = "redisHost"
+	password                  = "redisPassword"
+	enableTLS                 = "enableTLS"
+	maxRetries                = "maxRetries"
+	maxRetryBackoff           = "maxRetryBackoff"
+	failover                  = "failover"
+	sentinelMasterName        = "sentinelMasterName"
+	defaultBase               = 10
+	defaultBitSize            = 0
+	defaultDB                 = 0
+	defaultMaxRetries         = 3
+	defaultMaxRetryBackoff    = time.Second * 2
+	defaultEnableTLS          = false
+	keySpacePrefix            = "__keyspace@0__:"
+	keySpaceAny               = "__keyspace@0__:*"
+	redisWrongTypeIdentifyStr = "WRONGTYPE"
 )
 
 // ConfigurationStore is a Redis configuration store.
@@ -242,6 +243,10 @@ func (r *ConfigurationStore) Get(ctx context.Context, req *configuration.GetRequ
 
 		redisValue, err := r.client.Get(ctx, redisKey).Result()
 		if err != nil {
+			if strings.Contains(err.Error(), redisWrongTypeIdentifyStr) {
+				r.logger.Warnf("redis key %s 's type is not supported, ignore it\n", redisKey)
+				continue
+			}
 			return &configuration.GetResponse{}, fmt.Errorf("fail to get configuration for redis key=%s, error is %s", redisKey, err)
 		}
 		val, version := internal.GetRedisValueAndVersion(redisValue)
diff --git a/configuration/redis/redis_test.go b/configuration/redis/redis_test.go
index 1a793f705..8785ea955 100644
--- a/configuration/redis/redis_test.go
+++ b/configuration/redis/redis_test.go
@@ -47,6 +47,8 @@ func TestConfigurationStore_Get(t *testing.T) {
 	}
 	tests := []struct {
 		name    string
+		prepare func(*redis.Client)
+		restore func(*redis.Client)
 		fields  fields
 		args    args
 		want    *configuration.GetResponse
@@ -118,9 +120,43 @@ func TestConfigurationStore_Get(t *testing.T) {
 			},
 			wantErr: true,
 		},
+		{
+			name: "test does not throw error for wrong type during get all",
+			prepare: func(client *redis.Client) {
+				client.HSet(context.Background(), "notSupportedType", []string{"key1", "value1", "key2", "value2"})
+			},
+			fields: fields{
+				client: c,
+				json:   jsoniter.ConfigFastest,
+				logger: logger.NewLogger("test"),
+			},
+			args: args{
+				req: &configuration.GetRequest{},
+				ctx: context.Background(),
+			},
+			want: &configuration.GetResponse{
+				Items: []*configuration.Item{
+					{
+						Key:      "testKey",
+						Value:    "testValue",
+						Metadata: make(map[string]string),
+					}, {
+						Key:      "testKey2",
+						Value:    "testValue2",
+						Metadata: make(map[string]string),
+					},
+				},
+			},
+			restore: func(client *redis.Client) {
+				client.HDel(context.Background(), "notSupportedType")
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			if tt.prepare != nil {
+				tt.prepare(tt.fields.client)
+			}
 			r := &ConfigurationStore{
 				client:   tt.fields.client,
 				json:     tt.fields.json,
@@ -151,6 +187,9 @@ func TestConfigurationStore_Get(t *testing.T) {
 			for k := range got.Items {
 				assert.Equal(t, tt.want.Items[k], got.Items[k])
 			}
+			if tt.restore != nil {
+				tt.restore(tt.fields.client)
+			}
 		})
 	}
 }