cache/redis: use redisConfig to receive incoming JSON (previously using a map) (#5268)

* refactor cache/redis: Use redisConfig to receive incoming JSON (previously using a map).

* refactor cache/redis: Use the string type to receive JSON parameters.

---------

Co-authored-by: Tan <tanqianheng@gmail.com>

CHANGELOG.md

@@ -9,6 +9,7 @@
 - [cache/redis: support skipEmptyPrefix option ](https://github.com/beego/beego/pull/5264)
 - [fix: refactor InsertValue method](https://github.com/beego/beego/pull/5267)
 - [fix: modify InsertOrUpdate method, Remove the isMulti variable and its associated code](https://github.com/beego/beego/pull/5269)
+- [refactor cache/redis: Use redisConfig to receive incoming JSON (previously using a map)](https://github.com/beego/beego/pull/5268)
 
 ## ORM refactoring
 - [introducing internal/models pkg](https://github.com/beego/beego/pull/5238)

client/cache/redis/redis.go

@@ -43,8 +43,14 @@ import (
 	"github.com/beego/beego/v2/core/berror"
 )
 
-// DefaultKey defines the collection name of redis for the cache adapter.
+const (
-var DefaultKey = "beecacheRedis"
+	// DefaultKey defines the collection name of redis for the cache adapter.
+	DefaultKey = "beecacheRedis"
+	// defaultMaxIdle defines the default max idle connection number.
+	defaultMaxIdle = 3
+	// defaultTimeout defines the default timeout .
+	defaultTimeout = time.Second * 180
+)
 
 // Cache is Redis cache adapter.
 type Cache struct {
@@ -61,7 +67,8 @@ type Cache struct {
 	// see https://github.com/beego/beego/issues/5248
 	skipEmptyPrefix bool
 
-	// Timeout value (less than the redis server's timeout value)
+	// Timeout value (less than the redis server's timeout value).
+	// Timeout used for idle connection
 	timeout time.Duration
 }
 
@@ -204,46 +211,9 @@ func (rc *Cache) Scan(pattern string) (keys []string, err error) {
 // config: must be in this format {"key":"collection key","conn":"connection info","dbNum":"0", "skipEmptyPrefix":"true"}
 // Cached items in redis are stored forever, no garbage collection happens
 func (rc *Cache) StartAndGC(config string) error {
-	var cf map[string]string
+	err := rc.parseConf(config)
-	err := json.Unmarshal([]byte(config), &cf)
 	if err != nil {
-		return berror.Wrapf(err, cache.InvalidRedisCacheCfg, "could not unmarshal the config: %s", config)
+		return err
-	}
-
-	if _, ok := cf["key"]; !ok {
-		cf["key"] = DefaultKey
-	}
-	if _, ok := cf["conn"]; !ok {
-		return berror.Wrapf(err, cache.InvalidRedisCacheCfg, "config missing conn field: %s", config)
-	}
-
-	// Format redis://<password>@<host>:<port>
-	cf["conn"] = strings.Replace(cf["conn"], "redis://", "", 1)
-	if i := strings.Index(cf["conn"], "@"); i > -1 {
-		cf["password"] = cf["conn"][0:i]
-		cf["conn"] = cf["conn"][i+1:]
-	}
-
-	if v, ok := cf["dbNum"]; ok {
-		rc.dbNum, _ = strconv.Atoi(v)
-	}
-	if _, ok := cf["maxIdle"]; !ok {
-		cf["maxIdle"] = "3"
-	}
-
-	if v, ok := cf["skipEmptyPrefix"]; ok {
-		rc.skipEmptyPrefix, _ = strconv.ParseBool(v)
-	}
-
-	rc.key = cf["key"]
-	rc.conninfo = cf["conn"]
-	rc.password = cf["password"]
-	rc.maxIdle, _ = strconv.Atoi(cf["maxIdle"])
-
-	if v, err := time.ParseDuration(cf["timeout"]); err == nil {
-		rc.timeout = v
-	} else {
-		rc.timeout = 180 * time.Second
 	}
 
 	rc.connectInit()
@@ -261,6 +231,89 @@ func (rc *Cache) StartAndGC(config string) error {
 	return nil
 }
 
+func (rc *Cache) parseConf(config string) error {
+	var cf redisConfig
+	err := json.Unmarshal([]byte(config), &cf)
+	if err != nil {
+		return berror.Wrapf(err, cache.InvalidRedisCacheCfg, "could not unmarshal the config: %s", config)
+	}
+
+	err = cf.parse()
+	if err != nil {
+		return err
+	}
+
+	rc.dbNum = cf.dbNum
+	rc.key = cf.Key
+	rc.conninfo = cf.Conn
+	rc.password = cf.password
+	rc.maxIdle = cf.maxIdle
+	rc.timeout = cf.timeout
+	rc.skipEmptyPrefix = cf.skipEmptyPrefix
+
+	return nil
+}
+
+type redisConfig struct {
+	DbNum           string `json:"dbNum"`
+	SkipEmptyPrefix string `json:"skipEmptyPrefix"`
+	Key             string `json:"key"`
+	// Format redis://<password>@<host>:<port>
+	Conn       string `json:"conn"`
+	MaxIdle    string `json:"maxIdle"`
+	TimeoutStr string `json:"timeout"`
+
+	dbNum           int
+	skipEmptyPrefix bool
+	maxIdle         int
+	// parse from Conn
+	password string
+	// timeout used for idle connection, default is 180 seconds.
+	timeout time.Duration
+}
+
+// parse parses the config.
+// If the necessary settings have not been set, it will return an error.
+// It will fill the default values if some fields are missing.
+func (cf *redisConfig) parse() error {
+	if cf.Conn == "" {
+		return berror.Error(cache.InvalidRedisCacheCfg, "config missing conn field")
+	}
+
+	// Format redis://<password>@<host>:<port>
+	cf.Conn = strings.Replace(cf.Conn, "redis://", "", 1)
+	if i := strings.Index(cf.Conn, "@"); i > -1 {
+		cf.password = cf.Conn[0:i]
+		cf.Conn = cf.Conn[i+1:]
+	}
+
+	if cf.Key == "" {
+		cf.Key = DefaultKey
+	}
+
+	if cf.DbNum != "" {
+		cf.dbNum, _ = strconv.Atoi(cf.DbNum)
+	}
+
+	if cf.SkipEmptyPrefix != "" {
+		cf.skipEmptyPrefix, _ = strconv.ParseBool(cf.SkipEmptyPrefix)
+	}
+
+	if cf.MaxIdle == "" {
+		cf.maxIdle = defaultMaxIdle
+	} else {
+		cf.maxIdle, _ = strconv.Atoi(cf.MaxIdle)
+	}
+
+	if v, err := time.ParseDuration(cf.TimeoutStr); err == nil {
+		cf.timeout = v
+	} else {
+		cf.timeout = defaultTimeout
+	}
+
+	return nil
+}
+
 // connect to redis.
 func (rc *Cache) connectInit() {
 	dialFunc := func() (c redis.Conn, err error) {

client/cache/redis/redis_test.go

@@ -296,3 +296,66 @@ func TestCache_associate(t *testing.T) {
 		})
 	}
 }
+
+func TestCache_parseConf(t *testing.T) {
+	tests := []struct {
+		name string
+
+		configStr string
+
+		wantCache Cache
+		wantErr   error
+	}{
+		{
+			name: "just conn",
+			configStr: `{
+  "conn": "127.0.0.1:6379"
+}`,
+
+			wantCache: Cache{
+				conninfo:        "127.0.0.1:6379",
+				dbNum:           0,
+				key:             DefaultKey,
+				password:        "",
+				maxIdle:         defaultMaxIdle,
+				skipEmptyPrefix: false,
+				timeout:         defaultTimeout,
+			},
+			wantErr: nil,
+		},
+
+		{
+			name: "all",
+			configStr: `{
+  "dbNum": "2",
+  "skipEmptyPrefix": "true",
+  "key": "mykey",
+  "conn": "redis://mypwd@127.0.0.1:6379",
+  "maxIdle": "10",
+  "timeout": "30s"
+}`,
+
+			wantCache: Cache{
+				conninfo:        "127.0.0.1:6379",
+				dbNum:           2,
+				key:             "mykey",
+				password:        "mypwd",
+				maxIdle:         10,
+				skipEmptyPrefix: true,
+				timeout:         time.Second * 30,
+			},
+			wantErr: nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			c := Cache{}
+			err := c.parseConf(tt.configStr)
+			assert.Equal(t, tt.wantErr, err)
+			if err != nil {
+				return
+			}
+			assert.Equal(t, tt.wantCache, c)
+		})
+	}
+}