Repository: avenga/couper
Pull request #518 (2022-06-08 07:14:13)

Fix merge function with tombstone removal
`null` value properties should not be removed from the resulting object.

Introduced in #141 

---
<details>
    <summary>Reviewer checklist</summary>
    <ul>
        <li>Read PR description: a summary about the changes is required</li>
        <li>Changelog updated</li>
        <li>Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage</li>
        <li>Pulled branch, manually tested</li>
        <li>Verified requirements are met</li>
        <li>Reviewed the code</li>
        <li>Reviewed the related tests</li>
    </ul>
</details>



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 51dc5320..96a8fb2e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -37,6 +37,7 @@ Unreleased changes are available as `avenga/couper:edge` container.
   * Spurious `duplicate endpoint /**` error for APIs sharing the same base path ([#507](https://github.com/avenga/couper/pull/507))
   * Invalid (by [OpenAPI validation](./docs/REFERENCE.md#openapi-block)) backend response missing in [`backend_responses`](./docs/REFERENCE.md#backend_responses) ([#501](https://github.com/avenga/couper/pull/501))
   * Ignore the `expected_status` check for a request configured via a [`proxy`](./docs/REFERENCE.md#proxy-block) or [`request`](./docs/REFERENCE.md#request-block) block if a [`backend` error](./docs/ERRORS.md#endpoint-error-types) occured ([#505](https://github.com/avenga/couper/pull/505))
+  * `merge()` function removes key with `null` value. ([#518](https://github.com/avenga/couper/pull/518))
 
 * **Removed**
   * support for `beta_oidc` block (use [`oidc` block](./docs/REFERENCE.md#oidc-block) instead) ([#475](https://github.com/avenga/couper/pull/475))
diff --git a/eval/lib/merge.go b/eval/lib/merge.go
index 328833b1..8086590c 100644
--- a/eval/lib/merge.go
+++ b/eval/lib/merge.go
@@ -7,9 +7,7 @@ import (
 	"github.com/zclconf/go-cty/cty/function"
 )
 
-var (
-	MergeFunc = newMergeFunction()
-)
+var MergeFunc = newMergeFunction()
 
 func newMergeFunction() function.Function {
 	return function.New(&function.Spec{
@@ -76,9 +74,7 @@ func mergeObjects(args []cty.Value) cty.Value {
 
 		for it := arg.ElementIterator(); it.Next(); {
 			k, v := it.Element()
-			if v.IsNull() {
-				delete(outputMap, k.AsString())
-			} else if existingVal, ok := outputMap[k.AsString()]; !ok {
+			if existingVal, ok := outputMap[k.AsString()]; !ok {
 				// key not set
 				outputMap[k.AsString()] = v
 			} else if vType := v.Type(); vType.IsPrimitiveType() {
@@ -97,7 +93,7 @@ func mergeObjects(args []cty.Value) cty.Value {
 }
 
 func mergeTuples(args []cty.Value) cty.Value {
-	outputList := []cty.Value{}
+	var outputList []cty.Value
 	for _, arg := range args {
 		if arg.IsNull() {
 			continue


Test patch:
diff --git a/eval/lib/merge_test.go b/eval/lib/merge_test.go
index b32b444f..ef91c2df 100644
--- a/eval/lib/merge_test.go
+++ b/eval/lib/merge_test.go
@@ -17,6 +17,9 @@ func TestMerge(t *testing.T) {
 	cf, err := configload.LoadBytes([]byte(`server "test" {}`), "couper.hcl")
 	helper.Must(err)
 
+	hclContext := cf.Context.Value(request.ContextType).(*eval.Context).HCLContext()
+	mergeFn := hclContext.Functions["merge"]
+
 	tests := []struct {
 		name string
 		args []cty.Value
@@ -144,11 +147,12 @@ func TestMerge(t *testing.T) {
 			/*
 				merge(
 					{"k2": {"k2.2": 2}},
-					{"k2": null},
-					{"k2": {"k4.2": 4}}
+					{"k0": null},
+					{"k2": {"k4.2": 4}},
+					{"k2": {"k4.0": null}}
 				)
 			*/
-			"merge objects with tombstone null",
+			"merge objects with null",
 			[]cty.Value{
 				cty.ObjectVal(map[string]cty.Value{
 					"k2": cty.ObjectVal(map[string]cty.Value{
@@ -156,22 +160,31 @@ func TestMerge(t *testing.T) {
 					}),
 				}),
 				cty.ObjectVal(map[string]cty.Value{
-					"k2": cty.NullVal(cty.Bool),
+					"k0": cty.NullVal(cty.Bool),
 				}),
 				cty.ObjectVal(map[string]cty.Value{
 					"k2": cty.MapVal(map[string]cty.Value{
 						"k4.2": cty.NumberIntVal(4),
 					}),
 				}),
+				cty.ObjectVal(map[string]cty.Value{
+					"k2": cty.MapVal(map[string]cty.Value{
+						"k4.0": cty.NullVal(cty.Bool),
+					}),
+				}),
 			},
 			/*
 				{
+				  "k0": null,
 				  "k2": {"k4.2": 4}
 				}
 			*/
 			cty.ObjectVal(map[string]cty.Value{
-				"k2": cty.MapVal(map[string]cty.Value{
+				"k0": cty.NullVal(cty.Bool),
+				"k2": cty.ObjectVal(map[string]cty.Value{
+					"k2.2": cty.NumberIntVal(2),
 					"k4.2": cty.NumberIntVal(4),
+					"k4.0": cty.NullVal(cty.Bool),
 				}),
 			}),
 		},
@@ -264,17 +277,14 @@ func TestMerge(t *testing.T) {
 		},
 	}
 
-	hclContext := cf.Context.Value(request.ContextType).(*eval.Context).HCLContext()
-
 	for _, tt := range tests {
 		t.Run(tt.name, func(subT *testing.T) {
-			helper := test.New(subT)
-
-			mergedV, err := hclContext.Functions["merge"].Call(tt.args)
-			helper.Must(err)
+			h := test.New(subT)
 
+			mergedV, merr := mergeFn.Call(tt.args)
+			h.Must(merr)
 			if !mergedV.RawEquals(tt.want) {
-				subT.Errorf("Wrong return value; expected %#v, got: %#v", tt.want, mergedV)
+				subT.Errorf("Wrong return value:\nwant:\t%#v\ngot:\t%#v\n", tt.want, mergedV)
 			}
 		})
 	}
diff --git a/server/http_integration_test.go b/server/http_integration_test.go
index 0875c578..21850a37 100644
--- a/server/http_integration_test.go
+++ b/server/http_integration_test.go
@@ -5208,7 +5208,7 @@ func TestEndpoint_ResponseNilEvaluation(t *testing.T) {
 		{"/conditional/nested", true, ""},
 		{"/conditional/nested/true", true, ""},
 		{"/conditional/nested/false", true, ""},
-		{"/functions/arg-items", true, `{"foo":"bar","obj":{"key":"val"}}`},
+		{"/functions/arg-items", true, `{"foo":"bar","obj":{"key":"val"},"xxxx":null}`},
 		{"/functions/tuple-expr", true, `{"array":["a","b"]}`},
 		{"/rte1", true, "2"},
 		{"/rte2", true, "2"},