Merge pull request #1502 from Ace-Tang/config_empty_value

fix: support nullable bool value set in config
AliyunContainerService · Jun 11, 2018 · a0ef39d · a0ef39d
2 parents 5159116 + f240d6c
commit a0ef39d
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 9 deletions.
diff --git a/pkg/utils/utils.go b/pkg/utils/utils.go
@@ -196,16 +196,13 @@ func doMerge(src, dest reflect.Value) error {
 	return nil
 }
 
-// From src/pkg/encoding/json
+// From src/pkg/encoding/json,
+// we recognize nullable values like `false` `0` as not empty.
 func isEmptyValue(v reflect.Value) bool {
 	switch v.Kind() {
 	case reflect.Array, reflect.Map, reflect.Slice, reflect.String:
 		return v.Len() == 0
-	case reflect.Bool:
-		return !v.Bool()
-	case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64:
-		return v.Int() == 0
-	case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64, reflect.Uintptr:
+	case reflect.Uintptr:
 		return v.Uint() == 0
 	case reflect.Float32, reflect.Float64:
 		return v.Float() == 0

diff --git a/pkg/utils/utils_test.go b/pkg/utils/utils_test.go
@@ -187,12 +187,17 @@ func TestMerge(t *testing.T) {
 		}, {
 			src:      &simple{},
 			dest:     &simple{Sa: 1, Sb: "hello", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 22}},
-			expected: &simple{Sa: 1, Sb: "hello", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 22}},
+			expected: &simple{Sa: 0, Sb: "hello", Sc: false, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 0}},
 			ok:       true,
 		}, {
 			src:      &simple{Sa: 1, Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo"}},
-			dest:     &simple{Sa: 2, Sb: "world", Sc: false, Sd: map[string]string{"go": "old"}, Se: nestS{Na: 22}, Sf: []string{"foo"}},
-			expected: &simple{Sa: 1, Sb: "world", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo", "foo"}},
+			dest:     &simple{Sa: 2, Sb: "!", Sc: false, Sd: map[string]string{"go": "old"}, Se: nestS{Na: 22}, Sf: []string{"foo"}},
+			expected: &simple{Sa: 1, Sb: "!", Sc: true, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo", "foo"}},
+			ok:       true,
+		}, {
+			src:      &simple{Sa: 0, Sc: false, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo"}},
+			dest:     &simple{Sa: 2, Sb: "world", Sc: true, Sd: map[string]string{"go": "old"}, Se: nestS{Na: 22}, Sf: []string{"foo"}},
+			expected: &simple{Sa: 0, Sb: "world", Sc: false, Sd: map[string]string{"go": "gogo"}, Se: nestS{Na: 11}, Sf: []string{"foo", "foo"}},
 			ok:       true,
 		}, {
 			src:      &simple{Sd: map[string]string{"go": "gogo", "a": "b"}},
@@ -204,6 +209,18 @@ func TestMerge(t *testing.T) {
 			dest:     &simple{},
 			expected: &simple{Sd: map[string]string{"go": "gogo", "a": "b"}},
 			ok:       true,
+		}, {
+			// empty map should not overwrite
+			src:      &simple{Sd: map[string]string{}},
+			dest:     &simple{Sd: map[string]string{"a": "b"}},
+			expected: &simple{Sd: map[string]string{"a": "b"}},
+			ok:       true,
+		}, {
+			// empty slice should not overwrite
+			src:      &simple{Sf: []string{}},
+			dest:     &simple{Sf: []string{"c"}},
+			expected: &simple{Sf: []string{"c"}},
+			ok:       true,
 		},
 	} {
 		err := Merge(tm.src, tm.dest)