From a39be3727325dae492684209207d4822fd0b87df Mon Sep 17 00:00:00 2001 From: Nedyalko Dyakov Date: Mon, 3 Feb 2025 19:10:54 +0200 Subject: [PATCH] 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 | 1 + command.go | 48 +++++++++++----- commands_test.go | 138 +++++++++++++++++++++++++++++++++++++++++++++ main_test.go | 30 +++++++--- search_commands.go | 24 ++++++-- search_test.go | 67 ++++++++++++++++++++-- 6 files changed, 274 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index 63b21b0b..f1883206 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ testdata/* .DS_Store *.tar.gz *.dic +redis8tests.sh diff --git a/command.go b/command.go index f5aad914..2623a239 100644 --- a/command.go +++ b/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 } diff --git a/commands_test.go b/commands_test.go index 901e96e3..dacc7f3d 100644 --- a/commands_test.go +++ b/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()) diff --git a/main_test.go b/main_test.go index 6b3b563a..a326960a 100644 --- a/main_test.go +++ b/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) diff --git a/search_commands.go b/search_commands.go index 9e592801..1312a78f 100644 --- a/search_commands.go +++ b/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) diff --git a/search_test.go b/search_test.go index a48f45bf..0a06ffef 100644 --- a/search_test.go +++ b/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