Skip to content

Commit

Permalink
Copy the value from slice to avoid the original value being modified (#…
Browse files Browse the repository at this point in the history
…572)

It has code to deep copy map but not slice. This PR also copies value
for slice
  • Loading branch information
ktong authored Nov 21, 2024
1 parent 411ab81 commit 1f54b69
Show file tree
Hide file tree
Showing 4 changed files with 56 additions and 25 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
- Removes the chronological order between Config.Load and Config.Watch,
so they can be called in different goroutines (#569).

### Fixed

- Copy the value from slice to avoid the original value being modified (#572).

## [1.3.1] - 2024-09-09

### Added
Expand Down
15 changes: 13 additions & 2 deletions internal/convert/converter.go
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,7 @@ func (c Converter) convertStruct(name string, fromVal, toVal reflect.Value) erro
}

func (c Converter) convertInterface(name string, fromVal, toVal reflect.Value) error {
// Copy the value from map and slice to avoid the original value being modified.
switch fromVal.Kind() {
case reflect.Map:
if fromVal.IsNil() {
Expand All @@ -564,11 +565,21 @@ func (c Converter) convertInterface(name string, fromVal, toVal reflect.Value) e
toVal.Set(reflect.MakeMapWithSize(fromVal.Type(), fromVal.Len()))

return c.convertMap(name, fromVal, toVal.Elem())
case reflect.Slice:
if fromVal.IsNil() {
toVal.SetZero()

return nil
}

newSlice := reflect.MakeSlice(fromVal.Type(), fromVal.Len(), fromVal.Len())
reflect.Copy(newSlice, fromVal)
toVal.Set(newSlice)
default:
toVal.Set(fromVal)

return nil
}

return nil
}

func pointer(val reflect.Value) reflect.Value {
Expand Down
58 changes: 37 additions & 21 deletions internal/convert/converter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -839,15 +839,15 @@ func TestConverter(t *testing.T) { //nolint:maintidx
"Enum": "sky",
"OuterField": "outer",
"PrivateField": "private",
"InterfaceField": "interface{}",
"InterfaceField": "any",
"InnerField": "squash",
"Inner": map[string]any{"InnerField": "inner"},
},
to: pointer(OuterStruct{}),
expected: pointer(OuterStruct{
Enum: Sky,
OuterField: "outer",
InterfaceField: "interface{}",
InterfaceField: "any",
InnerStruct: InnerStruct{InnerField: "squash"},
Inner: &InnerStruct{InnerField: "inner"},
}),
Expand All @@ -857,18 +857,18 @@ func TestConverter(t *testing.T) { //nolint:maintidx
opts: []convert.Option{
convert.WithKeyMapper(strings.ToLower),
},
from: map[string]string{"innerfield": "inner", "interfacefield": "interface{}"},
from: map[string]string{"innerfield": "inner", "interfacefield": "any"},
to: pointer(struct {
InnerField string
InterfaceField interface{}
InterfaceField any
}{}),
expected: pointer(
struct {
InnerField string
InterfaceField interface{}
InterfaceField any
}{
InnerField: "inner",
InterfaceField: "interface{}",
InterfaceField: "any",
}),
},
{
Expand Down Expand Up @@ -908,6 +908,12 @@ func TestConverter(t *testing.T) { //nolint:maintidx
to: pointer(any(nil)),
expected: pointer(any(42)),
},
{
description: "zero int to interface",
from: 0,
to: pointer(any(nil)),
expected: pointer(any(0)),
},
{
description: "string to interface",
from: "str",
Expand Down Expand Up @@ -936,16 +942,16 @@ func TestConverter(t *testing.T) { //nolint:maintidx
expected: pointer(any(map[string]int{"key": 42, "keysensitive": 43})),
},
{
description: "packed KV and field to map[string]interface{}",
from: map[string]interface{}{
description: "packed KV and field to map[string]any",
from: map[string]any{
"key1": maps.KeyValue{
Key: "key1",
Value: "value1",
},
"key2": "value2",
},
to: pointer(map[string]interface{}{}),
expected: pointer(map[string]interface{}{
to: pointer(map[string]any{}),
expected: pointer(map[string]any{
"key1": "value1",
"key2": "value2",
}),
Expand All @@ -955,43 +961,47 @@ func TestConverter(t *testing.T) { //nolint:maintidx
opts: []convert.Option{
convert.WithKeyMapper(strings.ToLower),
},
from: map[string]interface{}{
from: map[string]any{
"key1": maps.KeyValue{
Key: "key1",
Value: "value1",
},
"key2": "value2",
"key3": []int{1, 2},
},
to: pointer(struct {
Key1 interface{}
Key2 interface{}
Key1 any
Key2 any
Key3 any
}{}),
expected: pointer(struct {
Key1 interface{}
Key2 interface{}
Key1 any
Key2 any
Key3 any
}{
Key1: "value1",
Key2: "value2",
Key3: []int{1, 2},
}),
},
{
description: "packed KV and field to interface{}",
from: map[string]interface{}{
description: "packed KV and field to any",
from: map[string]any{
"key1": maps.KeyValue{
Key: "key1",
Value: "value1",
},
"key2": "value2",
},
to: pointer(any(nil)),
expected: pointer(any(map[string]interface{}{
expected: pointer(any(map[string]any{
"key1": "value1",
"key2": "value2",
})),
},
{
description: "nil map to interface{}",
from: map[string]interface{}(nil),
description: "nil map to any",
from: map[string]any(nil),
to: pointer(any(nil)),
expected: pointer(any(nil)),
},
Expand All @@ -1001,6 +1011,12 @@ func TestConverter(t *testing.T) { //nolint:maintidx
to: pointer(any(nil)),
expected: pointer(any([]int{1, 2, 3})),
},
{
description: "nil slice to interface",
from: []int(nil),
to: pointer(any(nil)),
expected: pointer(any(nil)),
},
// unsupported.
{
description: "to func (unsupported)",
Expand Down Expand Up @@ -1065,7 +1081,7 @@ type (
Enum Enum
OuterField string
privateField string //nolint:unused
InterfaceField interface{}
InterfaceField any

InnerStruct `konf:",squash"`
Inner *InnerStruct
Expand Down
4 changes: 2 additions & 2 deletions internal/maps/transform.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@

package maps

func TransformKeys(src map[string]interface{}, keyMap func(string) string, mapKeyCaseSensitive bool) {
func TransformKeys(src map[string]any, keyMap func(string) string, mapKeyCaseSensitive bool) {
if src == nil || keyMap == nil {
return
}
for key, value := range src {
if m, ok := value.(map[string]interface{}); ok {
if m, ok := value.(map[string]any); ok {
TransformKeys(m, keyMap, mapKeyCaseSensitive)
}
newKey := keyMap(key)
Expand Down

0 comments on commit 1f54b69

Please sign in to comment.