From 9abdb29269df85e705364898193ac2dc5c987c8d Mon Sep 17 00:00:00 2001
From: Linh Vo <lvo@umbel.com>
Date: Thu, 6 Apr 2017 23:28:16 -0500
Subject: [PATCH] remove duplicate code, move ValidateName out of
 validateOptions

---
 db.go                   | 11 +++++++++++
 frame.go                |  5 +++++
 handle_internal_test.go |  4 +---
 handler.go              | 25 ++++++++-----------------
 4 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/db.go b/db.go
index 36fb38a94..965da17fa 100644
--- a/db.go
+++ b/db.go
@@ -86,6 +86,11 @@ func (db *DB) SetColumnLabel(v string) error {
 	db.mu.Lock()
 	defer db.mu.Unlock()
 
+	// Make sure columnLabel is valid name
+	err := ValidateName(v)
+	if err != nil {
+		return err
+	}
 	// Ignore if no change occurred.
 	if v == "" || db.columnLabel == v {
 		return nil
@@ -369,6 +374,12 @@ func (db *DB) createFrame(name string, opt FrameOptions) (*Frame, error) {
 	}
 
 	// Update options.
+	if opt.RowLabel != "" {
+		err := ValidateName(opt.RowLabel)
+		if err != nil {
+			return nil, err
+		}
+	}
 	f.SetRowLabel(opt.RowLabel)
 
 	// Add to database's frame lookup.
diff --git a/frame.go b/frame.go
index 9d0fccf50..21db2c24d 100644
--- a/frame.go
+++ b/frame.go
@@ -110,6 +110,11 @@ func (f *Frame) SetRowLabel(v string) error {
 	f.mu.Lock()
 	defer f.mu.Unlock()
 
+	// Make sure rowLabel is valid name
+	err := ValidateName(v)
+	if err != nil {
+		return err
+	}
 	// Ignore if no change occurred.
 	if v == "" || f.rowLabel == v {
 		return nil
diff --git a/handle_internal_test.go b/handle_internal_test.go
index 61790a950..b5ce8c161 100644
--- a/handle_internal_test.go
+++ b/handle_internal_test.go
@@ -19,7 +19,6 @@ func TestPostDBRequestUnmarshalJSON(t *testing.T) {
 		{json: `{"db": "d", "option": {}}`, err: "Unknown key: option:map[]"},
 		{json: `{"db": "d", "options": {"columnLabel": "test"}}`, expected: postDBRequest{DB: "d", Options: DBOptions{ColumnLabel: "test"}}},
 		{json: `{"db": "d", "options": {"columnLabl": "test"}}`, err: "invalid key for options {columnLabl:test}"},
-		{json: `{"db": "d", "options": {"columnLabel": "////"}}`, err: "invalid columnLabel value: ////"},
 	}
 	for _, test := range tests {
 		actual := &postDBRequest{}
@@ -52,12 +51,11 @@ func TestPostFrameRequestUnmarshalJSON(t *testing.T) {
 		err      string
 	}{
 		{json: `{"db": "d", "frame":"f",  "options": {}}`, expected: postFrameRequest{DB: "d", Frame: "f", Options: FrameOptions{}}},
-		{json: `{"db": "d", "options": {}}`, err: "db required and must be a string"},
+		{json: `{"db": "d", "options": {}}`, err: "frame required and must be a string"},
 		{json: `{"db": "d", "frame":"f", "options": 4}`, err: "options is not map[string]interface{}"},
 		{json: `{"db": "d", "frame":"f", "option": {}}`, err: "Unknown key: {option:map[]}"},
 		{json: `{"db": "d", "frame":"f", "options": {"rowLabel": "test"}}`, expected: postFrameRequest{DB: "d", Frame: "f", Options: FrameOptions{RowLabel: "test"}}},
 		{json: `{"db": "d", "frame":"f", "options": {"rowLabl": "test"}}`, err: "invalid key for options {rowLabl:test}"},
-		{json: `{"db": "d", "frame":"f", "options": {"rowLabel": "////"}}`, err: "invalid rowLabel value: ////"},
 	}
 	for _, test := range tests {
 		actual := &postFrameRequest{}
diff --git a/handler.go b/handler.go
index 19b2a7607..fc14fc22f 100644
--- a/handler.go
+++ b/handler.go
@@ -359,11 +359,7 @@ func (p *postDBRequest) UnmarshalJSON(b []byte) error {
 			}
 			p.DB = val
 		case "options":
-			options, ok := data["options"].(map[string]interface{})
-			if !ok {
-				return errors.New("options is not map[string]interface{}")
-			}
-			value, err := validateOptions(options, "columnLabel")
+			value, err := validateOptions(data, "columnLabel")
 			if err != nil {
 				return err
 			}
@@ -380,7 +376,11 @@ func (p *postDBRequest) UnmarshalJSON(b []byte) error {
 	return nil
 }
 
-func validateOptions(options map[string]interface{}, field string) (string, error) {
+func validateOptions(data map[string]interface{}, field string) (string, error) {
+	options, ok := data["options"].(map[string]interface{})
+	if !ok {
+		return "", errors.New("options is not map[string]interface{}")
+	}
 	var optionValue string
 	if len(options) == 0 {
 		optionValue = ""
@@ -392,12 +392,7 @@ func validateOptions(options map[string]interface{}, field string) (string, erro
 				if !ok {
 					return "", fmt.Errorf("invalid option %v: {%v:%v}", field, k, v)
 				}
-				err := ValidateName(val)
-				if err != nil {
-					return "", fmt.Errorf("invalid %v value: %v", field, v)
-
-				}
-				optionValue = options[field].(string)
+				optionValue = val
 			default:
 				return "", fmt.Errorf("invalid key for options {%v:%v}", k, v)
 			}
@@ -593,11 +588,7 @@ func (p *postFrameRequest) UnmarshalJSON(b []byte) error {
 			p.Frame = val
 
 		case "options":
-			options, ok := data["options"].(map[string]interface{})
-			if !ok {
-				return errors.New("options is not map[string]interface{}")
-			}
-			value, err := validateOptions(options, "rowLabel")
+			value, err := validateOptions(data, "rowLabel")
 			if err != nil {
 				return err
 			}