From d9256e401c6dcead11d3bec75bbdc5291d3b87d1 Mon Sep 17 00:00:00 2001
From: Steven Zou <szou@vmware.com>
Date: Thu, 23 Aug 2018 13:59:13 +0800
Subject: [PATCH] Fix the issue of parsing redis connection address in chart
 API - update logic in func parseRedisConfig

Signed-off-by: Steven Zou <szou@vmware.com>

- Add UT case for redis addr parsing func
---
 src/chartserver/utils.go      | 36 +++++++++++++++++++-----------
 src/chartserver/utils_test.go | 42 +++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 13 deletions(-)
 create mode 100644 src/chartserver/utils_test.go

diff --git a/src/chartserver/utils.go b/src/chartserver/utils.go
index 029e5e793..78aa0d919 100644
--- a/src/chartserver/utils.go
+++ b/src/chartserver/utils.go
@@ -68,22 +68,32 @@ func parseRedisConfig(redisConfigV string) (string, error) {
 	redisConfig := make(map[string]string)
 	redisConfig["key"] = cacheCollectionName
 
-	//The full pattern
-	if strings.Index(redisConfigV, ",") != -1 {
-		//Read only the previous 4 segments
-		configSegments := strings.SplitN(redisConfigV, ",", 4)
-		if len(configSegments) != 4 {
-			return "", errors.New("invalid redis config, it should be address:port[,weight,password,db_index]")
+	//Try best to parse the configuration segments.
+	//If the related parts are missing, assign default value.
+	//The default database index for UI process is 0.
+	configSegments := strings.Split(redisConfigV, ",")
+	for i, segment := range configSegments {
+		if i > 3 {
+			//ignore useless segments
+			break
 		}
 
-		redisConfig["conn"] = configSegments[0]
-		redisConfig["password"] = configSegments[2]
-		redisConfig["dbNum"] = configSegments[3]
-	} else {
-		//The short pattern
-		redisConfig["conn"] = redisConfigV
+		switch i {
+		//address:port
+		case 0:
+			redisConfig["conn"] = segment
+		//password, may not exist
+		case 2:
+			redisConfig["password"] = segment
+		//database index, may not exist
+		case 3:
+			redisConfig["dbNum"] = segment
+		}
+	}
+
+	//Assign default value
+	if len(redisConfig["dbNum"]) == 0 {
 		redisConfig["dbNum"] = "0"
-		redisConfig["password"] = ""
 	}
 
 	//Try to validate the connection address
diff --git a/src/chartserver/utils_test.go b/src/chartserver/utils_test.go
new file mode 100644
index 000000000..c678853ea
--- /dev/null
+++ b/src/chartserver/utils_test.go
@@ -0,0 +1,42 @@
+package chartserver
+
+import (
+	"strings"
+	"testing"
+)
+
+//Test the utility function parseRedisConfig
+func TestParseRedisConfig(t *testing.T) {
+	//Case 1: empty addr
+	redisAddr := ""
+	if _, err := parseRedisConfig(redisAddr); err == nil {
+		t.Fatal("expect non nil error but got nil one if addr is empty")
+	}
+
+	//Case 2: short pattern, addr:port
+	redisAddr = "redis:6379"
+	if parsedConnStr, err := parseRedisConfig(redisAddr); err != nil {
+		t.Fatalf("expect nil error but got non nil one if addr is short pattern: %s\n", parsedConnStr)
+	}
+
+	//Case 3: long pattern but miss some parts
+	redisAddr = "redis:6379,100"
+	if parsedConnStr, err := parseRedisConfig(redisAddr); err != nil {
+		t.Fatalf("expect nil error but got non nil one if addr is long pattern with some parts missing: %s\n", parsedConnStr)
+	} else {
+		if strings.Index(parsedConnStr, `"dbNum":"0"`) == -1 {
+			t.Fatalf("expect 'dbNum:0' in the parsed conn str but got nothing: %s\n", parsedConnStr)
+		}
+	}
+
+	//Case 4: long pattern
+	redisAddr = "redis:6379,100,Passw0rd,1"
+	if parsedConnStr, err := parseRedisConfig(redisAddr); err != nil {
+		t.Fatal("expect nil error but got non nil one if addr is long pattern")
+	} else {
+		if strings.Index(parsedConnStr, `"dbNum":"1"`) == -1 ||
+			strings.Index(parsedConnStr, `"password":"Passw0rd"`) == -1 {
+			t.Fatalf("expect 'dbNum:0' and 'password:Passw0rd' in the parsed conn str but got nothing: %s", parsedConnStr)
+		}
+	}
+}