Skip to content

Commit

Permalink
fix: merge should preserve zero value in dest (#34)
Browse files Browse the repository at this point in the history
## Description
Backport fix of sprig into sprout, initially provided by @huww98

## Fixes Masterminds/sprig#255
Masterminds/sprig#399

## Checklist
- [x] I have read the **CONTRIBUTING.md** document.
- [x] My code follows the code style of this project.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [x] I have updated the documentation accordingly.
- [ ] This change requires a change to the documentation on the website.

Co-authored-by: 胡玮文 <[email protected]>
  • Loading branch information
42atomys and huww98 authored May 15, 2024
1 parent 9d4a544 commit ec86e0b
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 7 deletions.
4 changes: 2 additions & 2 deletions maps_functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ func (fh *FunctionHandler) MergeOverwrite(dest map[string]any, srcs ...map[strin
// {{ mustMerge {}, {"a": 1, "b": 2}, {"b": 3, "c": 4} }} // Output: {"a": 1, "b": 2, "c": 4}, nil
func (fh *FunctionHandler) MustMerge(dest map[string]any, srcs ...map[string]any) (any, error) {
for _, src := range srcs {
if err := mergo.Merge(&dest, src); err != nil {
if err := mergo.Merge(&dest, src, mergo.WithoutDereference); err != nil {
// This error is not expected to occur, as we ensure types are correct in
// the function signature. If it does, it is a bug in the function implementation.
return nil, err
Expand Down Expand Up @@ -426,7 +426,7 @@ func (fh *FunctionHandler) MustMerge(dest map[string]any, srcs ...map[string]any
// {{ mustMergeOverwrite {}, {"a": 1, "b": 2}, {"b": 3, "c": 4} }} // Output: {"a": 1, "b": 3, "c": 4}, nil
func (fh *FunctionHandler) MustMergeOverwrite(dest map[string]any, srcs ...map[string]any) (any, error) {
for _, src := range srcs {
if err := mergo.Merge(&dest, src, mergo.WithOverride); err != nil {
if err := mergo.Merge(&dest, src, mergo.WithOverride, mergo.WithoutDereference); err != nil {
// This error is not expected to occur, as we ensure types are correct in
// the function signature. If it does, it is a bug in the function implementation.
return nil, err
Expand Down
12 changes: 7 additions & 5 deletions maps_functions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,8 @@ func TestMerge(t *testing.T) {
var tests = testCases{
{"TestEmpty", `{{merge .}}`, "map[]", nil},
{"TestWithOneMap", `{{merge .}}`, "map[a:1 b:2]", map[string]any{"a": 1, "b": 2}},
{"TestWithTwoMaps", `{{merge .A .B}}`, "map[a:1 b:2 c:3 d:4]", map[string]any{"A": map[string]any{"a": 1, "b": 2}, "B": map[string]any{"c": 3, "d": 4}}},
{"TestWithTwoMaps", `{{merge .Dest .Src1}}`, "map[a:1 b:2 c:3 d:4]", map[string]any{"Dest": map[string]any{"a": 1, "b": 2}, "Src1": map[string]any{"c": 3, "d": 4}}},
{"TestWithZeroValues", `{{merge .Dest .Src1}}`, "map[a:0 b:false c:3 d:4]", map[string]any{"Dest": map[string]any{"a": 0, "b": false}, "Src1": map[string]any{"a": 2, "b": true, "c": 3, "d": 4}}},
}

runTestCases(t, tests)
Expand All @@ -141,8 +142,9 @@ func TestMergeOverwrite(t *testing.T) {
var tests = testCases{
{"TestEmpty", `{{mergeOverwrite .}}`, "map[]", nil},
{"TestWithOneMap", `{{mergeOverwrite .}}`, "map[a:1 b:2]", map[string]any{"a": 1, "b": 2}},
{"TestWithTwoMaps", `{{mergeOverwrite .A .B}}`, "map[a:1 b:2 c:3 d:4]", map[string]any{"A": map[string]any{"a": 1, "b": 2}, "B": map[string]any{"c": 3, "d": 4}}},
{"TestWithOverwrite", `{{mergeOverwrite .A .B}}`, "map[a:3 b:2 d:4]", map[string]any{"A": map[string]any{"a": 1, "b": 2}, "B": map[string]any{"a": 3, "d": 4}}},
{"TestWithTwoMaps", `{{mergeOverwrite .Dest .Src1}}`, "map[a:1 b:2 c:3 d:4]", map[string]any{"Dest": map[string]any{"a": 1, "b": 2}, "Src1": map[string]any{"c": 3, "d": 4}}},
{"TestWithOverwrite", `{{mergeOverwrite .Dest .Src1}}`, "map[a:3 b:2 d:4]", map[string]any{"Dest": map[string]any{"a": 1, "b": 2}, "Src1": map[string]any{"a": 3, "d": 4}}},
{"TestWithZeroValues", `{{mergeOverwrite .Dest .Src1}}`, "map[a:2 b:true c:3 d:4]", map[string]any{"Dest": map[string]any{"a": 0, "b": false}, "Src1": map[string]any{"a": 2, "b": true, "c": 3, "d": 4}}},
}

runTestCases(t, tests)
Expand All @@ -152,8 +154,8 @@ func TestMustMerge(t *testing.T) {
var dest map[string]any
var tests = mustTestCases{
{testCase{"TestWithNotEnoughArgs", `{{mustMerge .}}`, "map[a:1]", map[string]any{"a": 1}}, ""},
{testCase{"TestWithDestNonInitialized", `{{mustMerge .A .B}}`, "map[b:2]", map[string]any{"A": dest, "B": map[string]any{"b": 2}}}, ""},
{testCase{"TestWithDestNotMap", `{{mustMerge .A .B}}`, "", map[string]any{"A": 1, "B": map[string]any{"b": 2}}}, "wrong type for value"},
{testCase{"TestWithDestNonInitialized", `{{mustMerge .Dest .Src1}}`, "map[b:2]", map[string]any{"Dest": dest, "Src1": map[string]any{"b": 2}}}, ""},
{testCase{"TestWithDestNotMap", `{{mustMerge .Dest .Src1}}`, "", map[string]any{"Dest": 1, "Src1": map[string]any{"b": 2}}}, "wrong type for value"},
}

runMustTestCases(t, tests)
Expand Down

0 comments on commit ec86e0b

Please sign in to comment.