feat(tests): validate that ConfigSet and ConfigGet work with Modules (#3258)

* Add tests for unified config in Redis 8

* WIP: fix reading FT.CONFIG with RESP3

* add more tests

* use search-timeout

* move deprecated warnings on the bottom

.gitignore

@@ -4,3 +4,4 @@ testdata/*
 .DS_Store
 *.tar.gz
 *.dic
+redis8tests.sh

command.go

@@ -3862,30 +3862,48 @@ func (cmd *MapMapStringInterfaceCmd) Val() map[string]interface{} {
 	return cmd.val
 }
 
+// readReply will try to parse the reply from the proto.Reader for both resp2 and resp3
 func (cmd *MapMapStringInterfaceCmd) readReply(rd *proto.Reader) (err error) {
-	n, err := rd.ReadArrayLen()
+	data, err := rd.ReadReply()
 	if err != nil {
 		return err
 	}
+	resultMap := map[string]interface{}{}
 
-	data := make(map[string]interface{}, n/2)
-	for i := 0; i < n; i += 2 {
-		_, err := rd.ReadArrayLen()
-		if err != nil {
-			cmd.err = err
+	switch midResponse := data.(type) {
+	case map[interface{}]interface{}: // resp3 will return map
+		for k, v := range midResponse {
+			stringKey, ok := k.(string)
+			if !ok {
+				return fmt.Errorf("redis: invalid map key %#v", k)
+			}
+			resultMap[stringKey] = v
 		}
-		key, err := rd.ReadString()
-		if err != nil {
-			cmd.err = err
+	case []interface{}: // resp2 will return array of arrays
+		n := len(midResponse)
+		for i := 0; i < n; i++ {
+			finalArr, ok := midResponse[i].([]interface{}) // final array that we need to transform to map
+			if !ok {
+				return fmt.Errorf("redis: unexpected response %#v", data)
+			}
+			m := len(finalArr)
+			if m%2 != 0 { // since this should be map, keys should be even number
+				return fmt.Errorf("redis: unexpected response %#v", data)
+			}
+
+			for j := 0; j < m; j += 2 {
+				stringKey, ok := finalArr[j].(string) // the first one
+				if !ok {
+					return fmt.Errorf("redis: invalid map key %#v", finalArr[i])
+				}
+				resultMap[stringKey] = finalArr[j+1] // second one is value
+			}
 		}
-		value, err := rd.ReadString()
-		if err != nil {
-			cmd.err = err
-		}
-		data[key] = value
+	default:
+		return fmt.Errorf("redis: unexpected response %#v", data)
 	}
 
-	cmd.val = data
+	cmd.val = resultMap
 	return nil
 }
 

commands_test.go

@@ -344,6 +344,23 @@ var _ = Describe("Commands", func() {
 			Expect(val).NotTo(BeEmpty())
 		})
 
+		It("should ConfigGet Modules", func() {
+			SkipBeforeRedisMajor(8, "Config doesn't include modules before Redis 8")
+			expected := map[string]string{
+				"search-*": "search-timeout",
+				"ts-*":     "ts-retention-policy",
+				"bf-*":     "bf-error-rate",
+				"cf-*":     "cf-initial-size",
+			}
+
+			for prefix, lookup := range expected {
+				val, err := client.ConfigGet(ctx, prefix).Result()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(val).NotTo(BeEmpty())
+				Expect(val[lookup]).NotTo(BeEmpty())
+			}
+		})
+
 		It("should ConfigResetStat", Label("NonRedisEnterprise"), func() {
 			r := client.ConfigResetStat(ctx)
 			Expect(r.Err()).NotTo(HaveOccurred())
@@ -362,6 +379,127 @@ var _ = Describe("Commands", func() {
 			Expect(configSet.Val()).To(Equal("OK"))
 		})
 
+		It("should ConfigGet with Modules", Label("NonRedisEnterprise"), func() {
+			SkipBeforeRedisMajor(8, "config get won't return modules configs before redis 8")
+			configGet := client.ConfigGet(ctx, "*")
+			Expect(configGet.Err()).NotTo(HaveOccurred())
+			Expect(configGet.Val()).To(HaveKey("maxmemory"))
+			Expect(configGet.Val()).To(HaveKey("search-timeout"))
+			Expect(configGet.Val()).To(HaveKey("ts-retention-policy"))
+			Expect(configGet.Val()).To(HaveKey("bf-error-rate"))
+			Expect(configGet.Val()).To(HaveKey("cf-initial-size"))
+		})
+
+		It("should ConfigSet FT DIALECT", func() {
+			SkipBeforeRedisMajor(8, "config doesn't include modules before Redis 8")
+			defaultState, err := client.ConfigGet(ctx, "search-default-dialect").Result()
+			Expect(err).NotTo(HaveOccurred())
+
+			// set to 3
+			res, err := client.ConfigSet(ctx, "search-default-dialect", "3").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(res).To(BeEquivalentTo("OK"))
+
+			defDialect, err := client.FTConfigGet(ctx, "DEFAULT_DIALECT").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(defDialect).To(BeEquivalentTo(map[string]interface{}{"DEFAULT_DIALECT": "3"}))
+
+			resGet, err := client.ConfigGet(ctx, "search-default-dialect").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(resGet).To(BeEquivalentTo(map[string]string{"search-default-dialect": "3"}))
+
+			// set to 2
+			res, err = client.ConfigSet(ctx, "search-default-dialect", "2").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(res).To(BeEquivalentTo("OK"))
+
+			defDialect, err = client.FTConfigGet(ctx, "DEFAULT_DIALECT").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(defDialect).To(BeEquivalentTo(map[string]interface{}{"DEFAULT_DIALECT": "2"}))
+
+			// set to 1
+			res, err = client.ConfigSet(ctx, "search-default-dialect", "1").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(res).To(BeEquivalentTo("OK"))
+
+			defDialect, err = client.FTConfigGet(ctx, "DEFAULT_DIALECT").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(defDialect).To(BeEquivalentTo(map[string]interface{}{"DEFAULT_DIALECT": "1"}))
+
+			resGet, err = client.ConfigGet(ctx, "search-default-dialect").Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(resGet).To(BeEquivalentTo(map[string]string{"search-default-dialect": "1"}))
+
+			// set to default
+			res, err = client.ConfigSet(ctx, "search-default-dialect", defaultState["search-default-dialect"]).Result()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(res).To(BeEquivalentTo("OK"))
+		})
+
+		It("should ConfigSet fail for ReadOnly", func() {
+			SkipBeforeRedisMajor(8, "Config doesn't include modules before Redis 8")
+			_, err := client.ConfigSet(ctx, "search-max-doctablesize", "100000").Result()
+			Expect(err).To(HaveOccurred())
+		})
+
+		It("should ConfigSet Modules", func() {
+			SkipBeforeRedisMajor(8, "Config doesn't include modules before Redis 8")
+			defaults := map[string]string{}
+			expected := map[string]string{
+				"search-timeout":      "100",
+				"ts-retention-policy": "2",
+				"bf-error-rate":       "0.13",
+				"cf-initial-size":     "64",
+			}
+
+			// read the defaults to set them back later
+			for setting, _ := range expected {
+				val, err := client.ConfigGet(ctx, setting).Result()
+				Expect(err).NotTo(HaveOccurred())
+				defaults[setting] = val[setting]
+			}
+
+			// check if new values can be set
+			for setting, value := range expected {
+				val, err := client.ConfigSet(ctx, setting, value).Result()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(val).NotTo(BeEmpty())
+				Expect(val).To(Equal("OK"))
+			}
+
+			for setting, value := range expected {
+				val, err := client.ConfigGet(ctx, setting).Result()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(val).NotTo(BeEmpty())
+				Expect(val[setting]).To(Equal(value))
+			}
+
+			// set back to the defaults
+			for setting, value := range defaults {
+				val, err := client.ConfigSet(ctx, setting, value).Result()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(val).NotTo(BeEmpty())
+				Expect(val).To(Equal("OK"))
+			}
+		})
+
+		It("should Fail ConfigSet Modules", func() {
+			SkipBeforeRedisMajor(8, "Config doesn't include modules before Redis 8")
+			expected := map[string]string{
+				"search-timeout":      "-100",
+				"ts-retention-policy": "-10",
+				"bf-error-rate":       "1.5",
+				"cf-initial-size":     "-10",
+			}
+
+			for setting, value := range expected {
+				val, err := client.ConfigSet(ctx, setting, value).Result()
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(MatchError(ContainSubstring(setting)))
+				Expect(val).To(BeEmpty())
+			}
+		})
+
 		It("should ConfigRewrite", Label("NonRedisEnterprise"), func() {
 			configRewrite := client.ConfigRewrite(ctx)
 			Expect(configRewrite.Err()).NotTo(HaveOccurred())

main_test.go

@@ -73,7 +73,19 @@ var RCEDocker = false
 // Notes the major version of redis we are executing tests.
 // This can be used before we change the bsm fork of ginkgo for one,
 // which have support for label sets, so we can filter tests per redis major version.
-var REDIS_MAJOR_VERSION = 7
+var RedisMajorVersion = 7
+
+func SkipBeforeRedisMajor(version int, msg string) {
+	if RedisMajorVersion < version {
+		Skip(fmt.Sprintf("(redis major version < %d) %s", version, msg))
+	}
+}
+
+func SkipAfterRedisMajor(version int, msg string) {
+	if RedisMajorVersion > version {
+		Skip(fmt.Sprintf("(redis major version > %d) %s", version, msg))
+	}
+}
 
 func registerProcess(port string, p *redisProcess) {
 	if processes == nil {
@@ -92,16 +104,20 @@ var _ = BeforeSuite(func() {
 	RECluster, _ = strconv.ParseBool(os.Getenv("RE_CLUSTER"))
 	RCEDocker, _ = strconv.ParseBool(os.Getenv("RCE_DOCKER"))
 
-	REDIS_MAJOR_VERSION, _ = strconv.Atoi(os.Getenv("REDIS_MAJOR_VERSION"))
-	if REDIS_MAJOR_VERSION == 0 {
-		REDIS_MAJOR_VERSION = 7
+	RedisMajorVersion, _ = strconv.Atoi(os.Getenv("REDIS_MAJOR_VERSION"))
+
+	if RedisMajorVersion == 0 {
+		RedisMajorVersion = 7
 	}
-	Expect(REDIS_MAJOR_VERSION).To(BeNumerically(">=", 6))
-	Expect(REDIS_MAJOR_VERSION).To(BeNumerically("<=", 8))
 
 	fmt.Printf("RECluster: %v\n", RECluster)
 	fmt.Printf("RCEDocker: %v\n", RCEDocker)
-	fmt.Printf("REDIS_MAJOR_VERSION: %v\n", REDIS_MAJOR_VERSION)
+	fmt.Printf("REDIS_MAJOR_VERSION: %v\n", RedisMajorVersion)
+
+	if RedisMajorVersion < 6 || RedisMajorVersion > 8 {
+		panic("incorrect or not supported redis major version")
+	}
+
 	if !RECluster && !RCEDocker {
 
 		redisMain, err = startRedis(redisPort)

search_commands.go

@@ -831,20 +831,32 @@ func (c cmdable) FTAlter(ctx context.Context, index string, skipInitialScan bool
 	return cmd
 }
 
-// FTConfigGet - Retrieves the value of a RediSearch configuration parameter.
+// Retrieves the value of a RediSearch configuration parameter.
 // The 'option' parameter specifies the configuration parameter to retrieve.
-// For more information, please refer to the Redis documentation:
-// [FT.CONFIG GET]: (https://redis.io/commands/ft.config-get/)
+// For more information, please refer to the Redis [FT.CONFIG GET] documentation.
+//
+// Deprecated: FTConfigGet is deprecated in Redis 8.
+// All configuration will be done with the CONFIG GET command.
+// For more information check [Client.ConfigGet] and [CONFIG GET Documentation]
+//
+// [CONFIG GET Documentation]: https://redis.io/commands/config-get/
+// [FT.CONFIG GET]: https://redis.io/commands/ft.config-get/
 func (c cmdable) FTConfigGet(ctx context.Context, option string) *MapMapStringInterfaceCmd {
 	cmd := NewMapMapStringInterfaceCmd(ctx, "FT.CONFIG", "GET", option)
 	_ = c(ctx, cmd)
 	return cmd
 }
 
-// FTConfigSet - Sets the value of a RediSearch configuration parameter.
+// Sets the value of a RediSearch configuration parameter.
 // The 'option' parameter specifies the configuration parameter to set, and the 'value' parameter specifies the new value.
-// For more information, please refer to the Redis documentation:
-// [FT.CONFIG SET]: (https://redis.io/commands/ft.config-set/)
+// For more information, please refer to the Redis [FT.CONFIG SET] documentation.
+//
+// Deprecated: FTConfigSet is deprecated in Redis 8.
+// All configuration will be done with the CONFIG SET command.
+// For more information check [Client.ConfigSet] and [CONFIG SET Documentation]
+//
+// [CONFIG SET Documentation]: https://redis.io/commands/config-set/
+// [FT.CONFIG SET]: https://redis.io/commands/ft.config-set/
 func (c cmdable) FTConfigSet(ctx context.Context, option string, value interface{}) *StatusCmd {
 	cmd := NewStatusCmd(ctx, "FT.CONFIG", "SET", option, value)
 	_ = c(ctx, cmd)

search_test.go

@@ -374,9 +374,8 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
 	// up until redis 8 the default scorer was TFIDF, in redis 8 it is BM25
 	// this test expect redis major version >= 8
 	It("should FTSearch WithScores", Label("search", "ftsearch"), func() {
-		if REDIS_MAJOR_VERSION < 8 {
-			Skip("(redis major version < 8) default scorer is not BM25")
-		}
+		SkipBeforeRedisMajor(8, "default scorer is not BM25")
+
 		text1 := &redis.FieldSchema{FieldName: "description", FieldType: redis.SearchFieldTypeText}
 		val, err := client.FTCreate(ctx, "idx1", &redis.FTCreateOptions{}, text1).Result()
 		Expect(err).NotTo(HaveOccurred())
@@ -418,9 +417,7 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
 	// up until redis 8 the default scorer was TFIDF, in redis 8 it is BM25
 	// this test expect redis major version <=7
 	It("should FTSearch WithScores", Label("search", "ftsearch"), func() {
-		if REDIS_MAJOR_VERSION > 7 {
-			Skip("(redis major version > 7) default scorer is not TFIDF")
-		}
+		SkipAfterRedisMajor(7, "default scorer is not TFIDF")
 		text1 := &redis.FieldSchema{FieldName: "description", FieldType: redis.SearchFieldTypeText}
 		val, err := client.FTCreate(ctx, "idx1", &redis.FTCreateOptions{}, text1).Result()
 		Expect(err).NotTo(HaveOccurred())
@@ -1015,6 +1012,24 @@ var _ = Describe("RediSearch commands Resp 2", Label("search"), func() {
 
 	})
 
+	It("should FTConfigGet return multiple fields", Label("search", "NonRedisEnterprise"), func() {
+		res, err := client.FTConfigSet(ctx, "DEFAULT_DIALECT", "1").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(res).To(BeEquivalentTo("OK"))
+
+		defDialect, err := client.FTConfigGet(ctx, "DEFAULT_DIALECT").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(defDialect).To(BeEquivalentTo(map[string]interface{}{"DEFAULT_DIALECT": "1"}))
+
+		res, err = client.FTConfigSet(ctx, "DEFAULT_DIALECT", "2").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(res).To(BeEquivalentTo("OK"))
+
+		defDialect, err = client.FTConfigGet(ctx, "DEFAULT_DIALECT").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(defDialect).To(BeEquivalentTo(map[string]interface{}{"DEFAULT_DIALECT": "2"}))
+	})
+
 	It("should FTConfigSet and FTConfigGet dialect", Label("search", "ftconfigget", "ftconfigset", "NonRedisEnterprise"), func() {
 		res, err := client.FTConfigSet(ctx, "DEFAULT_DIALECT", "1").Result()
 		Expect(err).NotTo(HaveOccurred())
@@ -1471,6 +1486,46 @@ func _assert_geosearch_result(result *redis.FTSearchResult, expectedDocIDs []str
 // 		Expect(results0["extra_attributes"].(map[interface{}]interface{})["__v_score"]).To(BeEquivalentTo("0"))
 // 	})
 
+var _ = Describe("RediSearch FT.Config with Resp2 and Resp3", Label("search", "NonRedisEnterprise"), func() {
+
+	var clientResp2 *redis.Client
+	var clientResp3 *redis.Client
+	BeforeEach(func() {
+		clientResp2 = redis.NewClient(&redis.Options{Addr: ":6379", Protocol: 2})
+		clientResp3 = redis.NewClient(&redis.Options{Addr: ":6379", Protocol: 3, UnstableResp3: true})
+		Expect(clientResp3.FlushDB(ctx).Err()).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		Expect(clientResp2.Close()).NotTo(HaveOccurred())
+		Expect(clientResp3.Close()).NotTo(HaveOccurred())
+	})
+
+	It("should FTConfigSet and FTConfigGet ", Label("search", "ftconfigget", "ftconfigset", "NonRedisEnterprise"), func() {
+		val, err := clientResp3.FTConfigSet(ctx, "TIMEOUT", "100").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(val).To(BeEquivalentTo("OK"))
+
+		res2, err := clientResp2.FTConfigGet(ctx, "TIMEOUT").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(res2).To(BeEquivalentTo(map[string]interface{}{"TIMEOUT": "100"}))
+
+		res3, err := clientResp3.FTConfigGet(ctx, "TIMEOUT").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(res3).To(BeEquivalentTo(map[string]interface{}{"TIMEOUT": "100"}))
+	})
+
+	It("should FTConfigGet all resp2 and resp3", Label("search", "NonRedisEnterprise"), func() {
+		res2, err := clientResp2.FTConfigGet(ctx, "*").Result()
+		Expect(err).NotTo(HaveOccurred())
+
+		res3, err := clientResp3.FTConfigGet(ctx, "*").Result()
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(res3)).To(BeEquivalentTo(len(res2)))
+		Expect(res2["DEFAULT_DIALECT"]).To(BeEquivalentTo(res2["DEFAULT_DIALECT"]))
+	})
+})
+
 var _ = Describe("RediSearch commands Resp 3", Label("search"), func() {
 	ctx := context.TODO()
 	var client *redis.Client