From 62955a2b1623cbbb3f24d7e6543b81c2fa4903b4 Mon Sep 17 00:00:00 2001
From: Krzysztof Kiewicz <krzysztof@quesma.com>
Date: Thu, 9 May 2024 07:14:19 +0200
Subject: [PATCH] Add context to mergeMaps function (#66)

It's very needed, as it's one of the most fragile parts of aggregation
handling.
When something goes wrong (e.g. for me during testing), it's very often
there, and so far we didn't catch any errors in UI because of lack of
context.
---
 quesma/queryparser/query_translator.go | 2 +-
 quesma/util/utils.go                   | 9 +++++----
 quesma/util/utils_test.go              | 3 ++-
 3 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/quesma/queryparser/query_translator.go b/quesma/queryparser/query_translator.go
index f0fe66f4f..c18fe0065 100644
--- a/quesma/queryparser/query_translator.go
+++ b/quesma/queryparser/query_translator.go
@@ -459,7 +459,7 @@ func (cw *ClickhouseQueryTranslator) MakeAggregationPartOfResponse(queries []mod
 		}
 		aggregation := cw.makeResponseAggregationRecursive(query, ResultSets[i+1], 0, 0)
 		if len(aggregation) != 0 {
-			aggregations = util.MergeMaps(aggregations, aggregation[0]) // result of root node is always a single map, thus [0]
+			aggregations = util.MergeMaps(cw.Ctx, aggregations, aggregation[0]) // result of root node is always a single map, thus [0]
 		}
 	}
 	return aggregations
diff --git a/quesma/util/utils.go b/quesma/util/utils.go
index 796ab227d..40725c98a 100644
--- a/quesma/util/utils.go
+++ b/quesma/util/utils.go
@@ -2,6 +2,7 @@ package util
 
 import (
 	"bytes"
+	"context"
 	"encoding/json"
 	"fmt"
 	"io"
@@ -255,7 +256,7 @@ func JsonDifference(jsonActual, jsonExpected string) (JsonMap, JsonMap, error) {
 // but none of them works for nested maps, so needed to write our own.
 // * mActual - uses JsonMap fully: values are []JsonMap, or JsonMap, or base types
 // * mExpected - value can also be []any, because it's generated from Golang's json.Unmarshal
-func MergeMaps(mActual, mExpected JsonMap) JsonMap {
+func MergeMaps(ctx context.Context, mActual, mExpected JsonMap) JsonMap {
 	var mergeMapsRec func(m1, m2 JsonMap) JsonMap
 	// merges 'i1' and 'i2' in 3 cases: both are JsonMap, both are []JsonMap, or both are some base type
 	mergeAny := func(i1, i2 any) any {
@@ -263,7 +264,7 @@ func MergeMaps(mActual, mExpected JsonMap) JsonMap {
 		case JsonMap:
 			i2Typed, ok := i2.(JsonMap)
 			if !ok {
-				logger.Error().Msgf("mergeAny: i1 is map, i2 is not. i1: %v, i2: %v", i1, i2)
+				logger.ErrorWithCtx(ctx).Msgf("mergeAny: i1 is map, i2 is not. i1: %v, i2: %v", i1, i2)
 				return i1
 			}
 			return mergeMapsRec(i1Typed, i2Typed)
@@ -277,13 +278,13 @@ func MergeMaps(mActual, mExpected JsonMap) JsonMap {
 						i2Typed = append(i2Typed, val)
 					}
 				} else {
-					logger.Error().Msgf("mergeAny: i1 is []JsonMap, i2 is not an array. i1: %v, i2: %v", i1Typed, i2)
+					logger.ErrorWithCtx(ctx).Msgf("mergeAny: i1 is []JsonMap, i2 is not an array. i1: %v, i2: %v", i1Typed, i2)
 				}
 			}
 
 			// lengths should be always equal in our usage of this function, maybe that'll change
 			if len(i1Typed) != len(i2Typed) {
-				logger.Error().Msgf("mergeAny: i1 and i2 are slices, but have different lengths. i1: %v, i2: %v", i1, i2)
+				logger.ErrorWithCtx(ctx).Msgf("mergeAny: i1 and i2 are slices, but have different lengths. len(i1): %v, len(i2): %v, i1: %v, i2: %v", len(i1Typed), len(i2Typed), i1, i2)
 				return []JsonMap{}
 			}
 			mergedArray := make([]JsonMap, len(i1Typed))
diff --git a/quesma/util/utils_test.go b/quesma/util/utils_test.go
index c597118ee..d90c3b966 100644
--- a/quesma/util/utils_test.go
+++ b/quesma/util/utils_test.go
@@ -1,6 +1,7 @@
 package util
 
 import (
+	"context"
 	"encoding/json"
 	"github.com/stretchr/testify/assert"
 	"reflect"
@@ -602,7 +603,7 @@ func TestMergeMaps(t *testing.T) {
 	for i, tt := range cases {
 		t.Run("TestMergeMaps_"+strconv.Itoa(i), func(t *testing.T) {
 			// simple == or Equal doesn't work on nested maps => need DeepEqual
-			assert.True(t, reflect.DeepEqual(tt.wanted, MergeMaps(tt.m1, tt.m2)))
+			assert.True(t, reflect.DeepEqual(tt.wanted, MergeMaps(context.Background(), tt.m1, tt.m2)))
 		})
 	}
 }