diff --git a/client/taskenv/util.go b/client/taskenv/util.go index 561b03709..1686a100d 100644 --- a/client/taskenv/util.go +++ b/client/taskenv/util.go @@ -14,28 +14,11 @@ var ( ErrInvalidObjectPath = errors.New("invalid object path") ) -type ErrKeyExists struct { - msg string -} - -func NewErrKeyExists(newKey, oldKey string) *ErrKeyExists { - return &ErrKeyExists{ - msg: fmt.Sprintf( - "cannot add key %q because %q already exists with a different type", - newKey, oldKey, - ), - } -} - -func (e *ErrKeyExists) Error() string { - return e.msg -} - // addNestedKey expands keys into their nested form: // // k="foo.bar", v="quux" -> {"foo": {"bar": "quux"}} // -// Existing keys are overwritten. +// Existing keys are overwritten. Map values take precedence over primitives. // // If the key has dots but cannot be converted to a valid nested data structure // (eg "foo...bar", "foo.", or non-object value exists for key), an error is @@ -64,13 +47,17 @@ func addNestedKey(dst map[string]interface{}, k, v string) error { var target map[string]interface{} if existingI, ok := dst[newKey]; ok { - existing, ok := existingI.(map[string]interface{}) - if !ok { - // Existing value is not a map, unable to support this key - cleanup() - return NewErrKeyExists(k, newKey) + if existing, ok := existingI.(map[string]interface{}); ok { + // Target already exists + target = existing + } else { + // Existing value is not a map. Maps should + // take precedence over primitive values (eg + // overwrite attr.driver.qemu = "1" with + // attr.driver.qemu.version = "...") + target = make(map[string]interface{}) + dst[newKey] = target } - target = existing } else { // Does not exist, create target = make(map[string]interface{}) @@ -96,6 +83,13 @@ func addNestedKey(dst map[string]interface{}, k, v string) error { return ErrInvalidObjectPath } + if existingI, ok := dst[newKey]; ok { + if _, ok := existingI.(map[string]interface{}); ok { + // Existing value is a map which takes precedence over + // a primitive value. Drop primitive. + return nil + } + } dst[newKey] = v return nil } diff --git a/client/taskenv/util_test.go b/client/taskenv/util_test.go index c9246713a..e97cc5716 100644 --- a/client/taskenv/util_test.go +++ b/client/taskenv/util_test.go @@ -53,7 +53,7 @@ func TestAddNestedKey_Ok(t *testing.T) { }, }, { - // Nested object b should get overwritten with "x" + // Nested object b should take precedence over values M: map[string]interface{}{ "a": map[string]interface{}{ "b": map[string]interface{}{ @@ -64,7 +64,9 @@ func TestAddNestedKey_Ok(t *testing.T) { K: "a.b", Result: map[string]interface{}{ "a": map[string]interface{}{ - "b": "x", + "b": map[string]interface{}{ + "c": "c", + }, }, }, }, @@ -123,6 +125,81 @@ func TestAddNestedKey_Ok(t *testing.T) { }, }, }, + // Regardless of whether attr.driver.qemu = "1" is added first + // or second, attr.driver.qemu.version = "..." should take + // precedence (nested maps take precedence over values) + { + M: map[string]interface{}{ + "attr": map[string]interface{}{ + "driver": map[string]interface{}{ + "qemu": "1", + }, + }, + }, + K: "attr.driver.qemu.version", + Result: map[string]interface{}{ + "attr": map[string]interface{}{ + "driver": map[string]interface{}{ + "qemu": map[string]interface{}{ + "version": "x", + }, + }, + }, + }, + }, + { + M: map[string]interface{}{ + "attr": map[string]interface{}{ + "driver": map[string]interface{}{ + "qemu": map[string]interface{}{ + "version": "1.2.3", + }, + }, + }, + }, + K: "attr.driver.qemu", + Result: map[string]interface{}{ + "attr": map[string]interface{}{ + "driver": map[string]interface{}{ + "qemu": map[string]interface{}{ + "version": "1.2.3", + }, + }, + }, + }, + }, + { + M: map[string]interface{}{ + "a": "a", + }, + K: "a.b", + Result: map[string]interface{}{ + "a": map[string]interface{}{ + "b": "x", + }, + }, + }, + { + M: map[string]interface{}{ + "a": "a", + "foo": map[string]interface{}{ + "b": "b", + "bar": "quux", + }, + "c": map[string]interface{}{}, + }, + K: "foo.bar.quux", + Result: map[string]interface{}{ + "a": "a", + "foo": map[string]interface{}{ + "b": "b", + "bar": map[string]interface{}{ + "quux": "x", + }, + }, + "c": map[string]interface{}{}, + }, + }, } for i := range cases { @@ -234,40 +311,6 @@ func TestAddNestedKey_Bad(t *testing.T) { K: "foo.bar..quux", Result: ErrInvalidObjectPath, }, - { - M: func() map[string]interface{} { - return map[string]interface{}{ - "a": "a", - } - }, - K: "a.b", - Result: NewErrKeyExists("a.b", "a"), - }, - { - M: func() map[string]interface{} { - return map[string]interface{}{ - "a": "a", - "foo": map[string]interface{}{ - "b": "b", - "bar": "quux", - }, - "c": map[string]interface{}{}, - } - }, - K: "foo.bar.quux", - Result: NewErrKeyExists("foo.bar.quux", "bar"), - }, - { - M: func() map[string]interface{} { - return map[string]interface{}{ - "a": map[string]interface{}{ - "b": "b", - }, - } - }, - K: "a.b.c.", - Result: NewErrKeyExists("a.b.c.", "b"), - }, } for i := range cases {