From eed385b56311a799e7fc5c119b93ec5e8654ce94 Mon Sep 17 00:00:00 2001 From: adatzer Date: Tue, 26 Mar 2024 07:12:57 +0200 Subject: [PATCH] Address pull request review comments --- pkg/models/message.go | 5 +- pkg/models/message_test.go | 2 +- pkg/target/http.go | 10 +- pkg/target/http_test.go | 43 +-- pkg/transform/engine/engine.go | 2 +- pkg/transform/engine/engine_javascript.go | 4 +- .../engine/engine_javascript_test.go | 360 ++++++++++-------- pkg/transform/gtmss_preview.go | 35 +- pkg/transform/gtmss_preview_test.go | 106 +++--- 9 files changed, 298 insertions(+), 269 deletions(-) diff --git a/pkg/models/message.go b/pkg/models/message.go index 806ebf73..8ad85693 100644 --- a/pkg/models/message.go +++ b/pkg/models/message.go @@ -16,7 +16,7 @@ import ( type Message struct { PartitionKey string Data []byte - HTTPHeaders map[string][]string + HTTPHeaders map[string]string // TimeCreated is when the message was created originally TimeCreated time.Time @@ -54,12 +54,11 @@ func (m *Message) GetError() error { func (m *Message) String() string { return fmt.Sprintf( - "PartitionKey:%s,TimeCreated:%v,TimePulled:%v,TimeTransformed:%v,HttpHeaders:%v,Data:%s", + "PartitionKey:%s,TimeCreated:%v,TimePulled:%v,TimeTransformed:%v,Data:%s", m.PartitionKey, m.TimeCreated, m.TimePulled, m.TimeTransformed, - m.HTTPHeaders, string(m.Data), ) } diff --git a/pkg/models/message_test.go b/pkg/models/message_test.go index b40acc41..51253d3a 100644 --- a/pkg/models/message_test.go +++ b/pkg/models/message_test.go @@ -22,7 +22,7 @@ func TestMessageString(t *testing.T) { PartitionKey: "some-key", } - assert.Equal("PartitionKey:some-key,TimeCreated:0001-01-01 00:00:00 +0000 UTC,TimePulled:0001-01-01 00:00:00 +0000 UTC,TimeTransformed:0001-01-01 00:00:00 +0000 UTC,HttpHeaders:map[],Data:Hello World!", msg.String()) + assert.Equal("PartitionKey:some-key,TimeCreated:0001-01-01 00:00:00 +0000 UTC,TimePulled:0001-01-01 00:00:00 +0000 UTC,TimeTransformed:0001-01-01 00:00:00 +0000 UTC,Data:Hello World!", msg.String()) assert.Nil(msg.GetError()) assert.Nil(msg.HTTPHeaders) diff --git a/pkg/target/http.go b/pkg/target/http.go index 7cb66806..30c293e8 100644 --- a/pkg/target/http.go +++ b/pkg/target/http.go @@ -78,15 +78,13 @@ func getHeaders(headers string) (map[string]string, error) { return parsed, nil } -func addHeadersToRequest(request *http.Request, headers map[string]string, dynamicHeaders map[string][]string) { +func addHeadersToRequest(request *http.Request, headers map[string]string, dynamicHeaders map[string]string) { for key, value := range headers { request.Header.Add(key, value) } - for key, values := range dynamicHeaders { - for _, val := range values { - request.Header.Add(key, val) - } + for key, value := range dynamicHeaders { + request.Header.Add(key, value) } } @@ -258,7 +256,7 @@ func (ht *HTTPTarget) GetID() string { return ht.httpURL } -func (ht *HTTPTarget) retrieveHeaders(msg *models.Message) map[string][]string { +func (ht *HTTPTarget) retrieveHeaders(msg *models.Message) map[string]string { if !ht.dynamicHeaders { return nil } diff --git a/pkg/target/http_test.go b/pkg/target/http_test.go index 58122a18..cde9afb3 100644 --- a/pkg/target/http_test.go +++ b/pkg/target/http_test.go @@ -93,7 +93,7 @@ func TestRetrieveHeaders(t *testing.T) { Name string Msg *models.Message Dynamic bool - Expected map[string][]string + Expected map[string]string }{ { Name: "message_headers_nil_dynamic_false", @@ -110,7 +110,7 @@ func TestRetrieveHeaders(t *testing.T) { { Name: "message_headers_empty_dynamic_false", Msg: &models.Message{ - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{}, }, Dynamic: false, Expected: nil, @@ -118,16 +118,16 @@ func TestRetrieveHeaders(t *testing.T) { { Name: "message_headers_empty_dynamic_true", Msg: &models.Message{ - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{}, }, Dynamic: true, - Expected: map[string][]string{}, + Expected: map[string]string{}, }, { Name: "message_headers_non_empty_dynamic_false", Msg: &models.Message{ - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, Dynamic: false, @@ -136,12 +136,12 @@ func TestRetrieveHeaders(t *testing.T) { { Name: "message_headers_non_empty_dynamic_true", Msg: &models.Message{ - HTTPHeaders: map[string][]string{ - "foo": {"bar", "baz"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, Dynamic: true, - Expected: map[string][]string{"foo": {"bar", "baz"}}, + Expected: map[string]string{"foo": "bar"}, }, } @@ -203,7 +203,7 @@ func TestAddHeadersToRequest_WithDynamicHeaders(t *testing.T) { testCases := []struct { Name string ConfigHeaders map[string]string - DynamicHeaders map[string][]string + DynamicHeaders map[string]string ExpectedHeader http.Header }{ { @@ -215,17 +215,17 @@ func TestAddHeadersToRequest_WithDynamicHeaders(t *testing.T) { { Name: "config_nil_dynamic_empty", ConfigHeaders: nil, - DynamicHeaders: map[string][]string{}, + DynamicHeaders: map[string]string{}, ExpectedHeader: http.Header{}, }, { Name: "config_nil_dynamic_yes", ConfigHeaders: nil, - DynamicHeaders: map[string][]string{ - "Content-Length": {"0", "1"}, + DynamicHeaders: map[string]string{ + "Content-Length": "0", }, ExpectedHeader: http.Header{ - "Content-Length": {"0", "1"}, + "Content-Length": {"0"}, }, }, { @@ -243,7 +243,7 @@ func TestAddHeadersToRequest_WithDynamicHeaders(t *testing.T) { ConfigHeaders: map[string]string{ "Max Forwards": "10", }, - DynamicHeaders: map[string][]string{}, + DynamicHeaders: map[string]string{}, ExpectedHeader: http.Header{ "Max Forwards": {"10"}, }, @@ -253,13 +253,12 @@ func TestAddHeadersToRequest_WithDynamicHeaders(t *testing.T) { ConfigHeaders: map[string]string{ "Max Forwards": "10", }, - DynamicHeaders: map[string][]string{ - "Content-Length": {"0", "1"}, - "Empty": {}, + DynamicHeaders: map[string]string{ + "Content-Length": "0", }, ExpectedHeader: http.Header{ "Max Forwards": {"10"}, - "Content-Length": {"0", "1"}, + "Content-Length": {"0"}, }, }, { @@ -268,9 +267,9 @@ func TestAddHeadersToRequest_WithDynamicHeaders(t *testing.T) { "Max Forwards": "10", "Content-Length": "0", }, - DynamicHeaders: map[string][]string{ - "Content-Length": {"1"}, - "Test-Header": {"test"}, + DynamicHeaders: map[string]string{ + "Content-Length": "1", + "Test-Header": "test", }, ExpectedHeader: http.Header{ "Max Forwards": {"10"}, diff --git a/pkg/transform/engine/engine.go b/pkg/transform/engine/engine.go index c30f4294..6578adcc 100644 --- a/pkg/transform/engine/engine.go +++ b/pkg/transform/engine/engine.go @@ -41,5 +41,5 @@ type engineProtocol struct { FilterOut bool PartitionKey string Data interface{} - HTTPHeaders map[string][]string + HTTPHeaders map[string]string } diff --git a/pkg/transform/engine/engine_javascript.go b/pkg/transform/engine/engine_javascript.go index 2106e521..3cc61847 100644 --- a/pkg/transform/engine/engine_javascript.go +++ b/pkg/transform/engine/engine_javascript.go @@ -193,7 +193,9 @@ func (e *JSEngine) MakeFunction(funcName string) transform.TransformationFunctio } // HTTPHeaders - message.HTTPHeaders = protocol.HTTPHeaders + if len(protocol.HTTPHeaders) > 0 { + message.HTTPHeaders = protocol.HTTPHeaders + } return message, nil, nil, protocol } diff --git a/pkg/transform/engine/engine_javascript_test.go b/pkg/transform/engine/engine_javascript_test.go index 7a5c0b7c..191e6efd 100644 --- a/pkg/transform/engine/engine_javascript_test.go +++ b/pkg/transform/engine/engine_javascript_test.go @@ -1157,7 +1157,7 @@ func TestJSEngineMakeFunction_HTTPHeaders(t *testing.T) { Src: ` function main(x) { var httpHeaders = { - foo: ['bar'] + foo: 'bar' }; x.HTTPHeaders = httpHeaders; return x; @@ -1173,8 +1173,8 @@ function main(x) { "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, "filtered": nil, @@ -1185,8 +1185,8 @@ function main(x) { FilterOut: false, PartitionKey: "", Data: testJSMap, - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, Error: nil, @@ -1196,7 +1196,7 @@ function main(x) { Src: ` function main(x) { x.HTTPHeaders = { - foo: ['bar', 'baz'] + foo: 'bar' }; return x; } @@ -1205,8 +1205,8 @@ function main(x) { InputMsg: &models.Message{ Data: testJsTsv, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, InputInterState: nil, @@ -1214,8 +1214,8 @@ function main(x) { "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar", "baz"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, "filtered": nil, @@ -1226,8 +1226,8 @@ function main(x) { FilterOut: false, PartitionKey: "", Data: testJSMap, - HTTPHeaders: map[string][]string{ - "foo": {"bar", "baz"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, Error: nil, @@ -1237,7 +1237,7 @@ function main(x) { Src: ` function main(x) { var headers = x.HTTPHeaders || {}; - headers.foo = ['bar', 'baz']; + headers.foo = 'bar'; x.HTTPHeaders = headers; return x; } @@ -1246,8 +1246,8 @@ function main(x) { InputMsg: &models.Message{ Data: testJsTsv, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, InputInterState: nil, @@ -1255,9 +1255,9 @@ function main(x) { "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - "foo": {"bar", "baz"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + "foo": "bar", }, }, "filtered": nil, @@ -1268,9 +1268,9 @@ function main(x) { FilterOut: false, PartitionKey: "", Data: testJSMap, - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - "foo": {"bar", "baz"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + "foo": "bar", }, }, Error: nil, @@ -1280,7 +1280,7 @@ function main(x) { Src: ` function main(x) { var httpHeaders = { - foo: ['bar'] + foo: 'bar' }; x.HTTPHeaders = httpHeaders; return x; @@ -1290,22 +1290,22 @@ function main(x) { InputMsg: &models.Message{ Data: testJsTsv, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, InputInterState: &engineProtocol{ Data: testJSMap, - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, Expected: map[string]*models.Message{ "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, "filtered": nil, @@ -1316,8 +1316,8 @@ function main(x) { FilterOut: false, PartitionKey: "", Data: testJSMap, - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, Error: nil, @@ -1327,11 +1327,11 @@ function main(x) { Src: ` function main(x) { var headers = x.HTTPHeaders || {}; - var foo = ['bar']; - var old = Array.isArray(headers.oldKey) ? headers.oldKey : []; + var foo = 'bar'; + var old = headers.oldKey || ''; headers.foo = foo; - headers.oldKey = old.concat(['newVal']); + headers.oldKey = old.concat('newVal'); x.HTTPHeaders = headers; return x; } @@ -1340,23 +1340,23 @@ function main(x) { InputMsg: &models.Message{ Data: testJsTsv, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, InputInterState: &engineProtocol{ Data: testJSMap, - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, Expected: map[string]*models.Message{ "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal", "newVal"}, - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldValnewVal", + "foo": "bar", }, }, "filtered": nil, @@ -1367,15 +1367,15 @@ function main(x) { FilterOut: false, PartitionKey: "", Data: testJSMap, - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal", "newVal"}, - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldValnewVal", + "foo": "bar", }, }, Error: nil, }, { - Scenario: "with_initial_headers_set_to_null", + Scenario: "with_initial_headers_set_to_null_no_effect", Src: ` function main(x) { x.HTTPHeaders = null; @@ -1386,8 +1386,8 @@ function main(x) { InputMsg: &models.Message{ Data: testJsTsv, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, InputInterState: nil, @@ -1395,7 +1395,9 @@ function main(x) { "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: nil, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, "filtered": nil, "failed": nil, @@ -1410,7 +1412,7 @@ function main(x) { Error: nil, }, { - Scenario: "without_initial_headers_set_to_null", + Scenario: "without_initial_headers_set_to_null_no_effect", Src: ` function main(x) { x.HTTPHeaders = null; @@ -1442,7 +1444,7 @@ function main(x) { Error: nil, }, { - Scenario: "with_initial_headers_set_to_undefined", + Scenario: "with_initial_headers_set_to_undefined_no_effect", Src: ` function main(x) { x.HTTPHeaders = undefined; @@ -1453,8 +1455,8 @@ function main(x) { InputMsg: &models.Message{ Data: testJsTsv, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, InputInterState: nil, @@ -1462,7 +1464,9 @@ function main(x) { "success": { Data: testJsJSON, PartitionKey: "pk", - HTTPHeaders: nil, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, "filtered": nil, "failed": nil, @@ -1477,7 +1481,7 @@ function main(x) { Error: nil, }, { - Scenario: "without_initial_headers_set_to_undefined", + Scenario: "without_initial_headers_set_to_undefined_no_effect", Src: ` function main(x) { x.HTTPHeaders = undefined; @@ -1509,108 +1513,106 @@ function main(x) { Error: nil, }, { - Scenario: "with_initial_headers_set_to_invalid_primitive_mutate", + Scenario: "with_initial_headers_set_to_empty_object_no_effect", Src: ` function main(x) { - x.HTTPHeaders = 'invalid'; + var newHeaders = {}; + x.HTTPHeaders = newHeaders; return x; } -`, + `, SpMode: false, InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - }, - }, - InputInterState: &engineProtocol{ - Data: []byte("asdf"), - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, + InputInterState: nil, Expected: map[string]*models.Message{ - "success": nil, - "filtered": nil, - "failed": { + "success": { Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, + "filtered": nil, + "failed": nil, }, - IsJSON: false, - ExpInterState: nil, - Error: fmt.Errorf("could not convert"), + IsJSON: false, + ExpInterState: &engineProtocol{ + FilterOut: false, + Data: "asdf", + PartitionKey: "", + HTTPHeaders: map[string]string{}, + }, + Error: nil, }, { - Scenario: "with_initial_headers_set_to_invalid_primitive_replace", + Scenario: "without_initial_headers_set_to_empty_object", Src: ` function main(x) { - return { - Data: x.Data, - PrimaryKey: x.PrimaryKey, - HTTPHeaders: 'invalid' - }; + var newHeaders = {}; + x.HTTPHeaders = newHeaders; + return x; } -`, + `, SpMode: false, InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - }, - }, - InputInterState: &engineProtocol{ - Data: []byte("asdf"), - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - }, }, + InputInterState: nil, Expected: map[string]*models.Message{ - "success": nil, - "filtered": nil, - "failed": { + "success": { Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - }, + HTTPHeaders: nil, }, + "filtered": nil, + "failed": nil, }, - IsJSON: false, - ExpInterState: nil, - Error: fmt.Errorf("protocol violation"), + IsJSON: false, + ExpInterState: &engineProtocol{ + FilterOut: false, + Data: "asdf", + PartitionKey: "", + HTTPHeaders: map[string]string{}, + }, + Error: nil, }, { - Scenario: "with_initial_headers_set_to_invalid_header_value", + Scenario: "with_initial_headers_mutate_set_headers_to_invalid_primitive", Src: ` function main(x) { - var newHeaders = {invalid: 'not_array'}; - x.HTTPHeaders = newHeaders; + x.HTTPHeaders = 'invalid'; return x; } - `, +`, SpMode: false, InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, + }, + InputInterState: &engineProtocol{ + Data: []byte("asdf"), + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, - InputInterState: nil, Expected: map[string]*models.Message{ "success": nil, "filtered": nil, "failed": { Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", }, }, }, @@ -1619,34 +1621,47 @@ function main(x) { Error: fmt.Errorf("could not convert"), }, { - Scenario: "without_initial_headers_set_to_invalid_header_value", + Scenario: "with_initial_headers_replace_set_headers_to_invalid_primitive", Src: ` function main(x) { - var newHeaders = {invalid: {}}; - x.HTTPHeaders = newHeaders; - return x; + return { + Data: x.Data, + PrimaryKey: x.PrimaryKey, + HTTPHeaders: 'invalid' + }; } - `, +`, SpMode: false, InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, + }, + InputInterState: &engineProtocol{ + Data: []byte("asdf"), + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, - InputInterState: nil, Expected: map[string]*models.Message{ "success": nil, "filtered": nil, "failed": { Data: []byte("asdf"), PartitionKey: "pk", + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, }, IsJSON: false, ExpInterState: nil, - Error: fmt.Errorf("could not convert"), + Error: fmt.Errorf("protocol violation"), }, { - Scenario: "without_initial_headers_set_to_invalid_object(e.g.function)_mutate", + Scenario: "with_initial_headers_mutate_set_headers_to_invalid_object_function_as_empty_object", Src: ` function main(x) { var newHeaders = function(y) {return y;}; @@ -1658,13 +1673,18 @@ function main(x) { InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, InputInterState: nil, Expected: map[string]*models.Message{ "success": { Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, "filtered": nil, "failed": nil, @@ -1674,48 +1694,57 @@ function main(x) { FilterOut: false, Data: "asdf", PartitionKey: "", - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{}, }, Error: nil, }, { - Scenario: "without_initial_headers_set_to_invalid_object(e.g.array)_replace", + Scenario: "with_initial_headers_mutate_set_headers_to_invalid_object_array_as_object", Src: ` function main(x) { - var newHeaders = []; - var ret = { - FilterOut: x.FilterOut, - Data: x.Data, - PartitionKey: x.PartitionKey, - HTTPHeaders: newHeaders - }; - - return ret; + var newHeaders = [10, 11]; + x.HTTPHeaders = newHeaders; + return x; } `, SpMode: false, InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", + HTTPHeaders: map[string]string{ + "oldKey": "oldVal", + }, }, InputInterState: nil, Expected: map[string]*models.Message{ - "success": nil, - "filtered": nil, - "failed": { + "success": { Data: []byte("asdf"), PartitionKey: "pk", + HTTPHeaders: map[string]string{ + "0": "10", + "1": "11", + }, }, + "filtered": nil, + "failed": nil, }, - IsJSON: false, - ExpInterState: nil, - Error: fmt.Errorf("protocol violation"), + IsJSON: false, + ExpInterState: &engineProtocol{ + FilterOut: false, + Data: "asdf", + PartitionKey: "", + HTTPHeaders: map[string]string{ + "0": "10", + "1": "11", + }, + }, + Error: nil, }, { - Scenario: "with_initial_headers_set_to_empty_object", + Scenario: "without_initial_headers_set_invalid_header_value(object)_calls_toString", Src: ` function main(x) { - var newHeaders = {}; + var newHeaders = {invalid: {}}; x.HTTPHeaders = newHeaders; return x; } @@ -1724,36 +1753,42 @@ function main(x) { InputMsg: &models.Message{ Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "oldKey": {"oldVal"}, - }, }, InputInterState: nil, Expected: map[string]*models.Message{ "success": { Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{ + "invalid": "[object Object]", + }, }, "filtered": nil, "failed": nil, }, IsJSON: false, ExpInterState: &engineProtocol{ - FilterOut: false, Data: "asdf", PartitionKey: "", - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{ + "invalid": "[object Object]", + }, }, Error: nil, }, { - Scenario: "without_initial_headers_set_to_empty_object", + Scenario: "without_initial_headers_set_to_invalid_object(e.g.array)_replace", Src: ` function main(x) { - var newHeaders = {}; - x.HTTPHeaders = newHeaders; - return x; + var newHeaders = []; + var ret = { + FilterOut: x.FilterOut, + Data: x.Data, + PartitionKey: x.PartitionKey, + HTTPHeaders: newHeaders + }; + + return ret; } `, SpMode: false, @@ -1763,29 +1798,24 @@ function main(x) { }, InputInterState: nil, Expected: map[string]*models.Message{ - "success": { + "success": nil, + "filtered": nil, + "failed": { Data: []byte("asdf"), PartitionKey: "pk", - HTTPHeaders: map[string][]string{}, }, - "filtered": nil, - "failed": nil, - }, - IsJSON: false, - ExpInterState: &engineProtocol{ - FilterOut: false, - Data: "asdf", - PartitionKey: "", - HTTPHeaders: map[string][]string{}, }, - Error: nil, + IsJSON: false, + ExpInterState: nil, + Error: fmt.Errorf("protocol violation"), }, + { Scenario: "filterOut_ignores_headers", Src: ` function main(x) { x.HTTPHeaders = { - foo: ['bar'] + foo: 'bar' }; x.FilterOut = true; return x; @@ -2076,6 +2106,7 @@ func assertMessagesCompareJs(t *testing.T, act, exp *models.Message, isJSON bool t.Helper() ok := false + headersOk := false switch { case act == nil: ok = exp == nil @@ -2092,7 +2123,7 @@ func assertMessagesCompareJs(t *testing.T, act, exp *models.Message, isJSON bool pTimeOk := reflect.DeepEqual(act.TimePulled, exp.TimePulled) tTimeOk := reflect.DeepEqual(act.TimeTransformed, exp.TimeTransformed) ackOk := reflect.DeepEqual(act.AckFunc, exp.AckFunc) - headersOk := reflect.DeepEqual(act.HTTPHeaders, exp.HTTPHeaders) + headersOk = reflect.DeepEqual(act.HTTPHeaders, exp.HTTPHeaders) if pkOk && dataOk && cTimeOk && pTimeOk && tTimeOk && ackOk && headersOk { ok = true @@ -2100,9 +2131,16 @@ func assertMessagesCompareJs(t *testing.T, act, exp *models.Message, isJSON bool } if !ok { - t.Errorf("\nGOT:\n%s\nEXPECTED:\n%s\n", - spew.Sdump(act), - spew.Sdump(exp)) + // message.HTTPHeaders are not printed + if headersOk == false { + t.Errorf("\nUnexpected HTTPHeaders:\nGOT:\n%s\nEXPECTED:\n%s\n", + spew.Sdump(act.HTTPHeaders), + spew.Sdump(exp.HTTPHeaders)) + } else { + t.Errorf("\nGOT:\n%s\nEXPECTED:\n%s\n", + spew.Sdump(act), + spew.Sdump(exp)) + } } } diff --git a/pkg/transform/gtmss_preview.go b/pkg/transform/gtmss_preview.go index 42101607..462cbb87 100644 --- a/pkg/transform/gtmss_preview.go +++ b/pkg/transform/gtmss_preview.go @@ -63,9 +63,9 @@ func gtmssPreviewTransformation(ctx, property, headerKey string) TransformationF } if headerVal != nil { if message.HTTPHeaders == nil { - message.HTTPHeaders = make(map[string][]string) + message.HTTPHeaders = make(map[string]string) } - message.HTTPHeaders[headerKey] = append(message.HTTPHeaders[headerKey], *headerVal) + message.HTTPHeaders[headerKey] = *headerVal return message, nil, nil, parsedEvent } @@ -74,36 +74,20 @@ func gtmssPreviewTransformation(ctx, property, headerKey string) TransformationF } func extractHeaderValue(parsedEvent analytics.ParsedEvent, ctx, prop string) (*string, error) { - spMap, err := parsedEvent.ToMap() + values, err := parsedEvent.GetContextValue(ctx, prop) if err != nil { return nil, err } - gtmssPreview, ok := spMap[ctx] - if !ok { - // not for preview mode, so do nothing - return nil, nil - } - - gtmssPreviewData, ok := gtmssPreview.([]interface{}) + headerVals, ok := values.([]interface{}) if !ok { // this is generally not expected to happen - return nil, errors.New("invalid gtmss preview context") + return nil, errors.New("invalid return type encountered") } - if len(gtmssPreviewData) > 0 { - previewMode, ok := gtmssPreviewData[0].(map[string]interface{}) - if !ok { - // this is generally not expected to happen - return nil, errors.New("invalid gtmss preview context data") - } - - previewHeader, ok := previewMode[prop] - if !ok { - return nil, errors.New("missing header property") - } - - headerVal, ok := previewHeader.(string) + if len(headerVals) > 0 { + // use only first value found + headerVal, ok := headerVals[0].(string) if !ok { return nil, errors.New("invalid header value") } @@ -111,5 +95,6 @@ func extractHeaderValue(parsedEvent analytics.ParsedEvent, ctx, prop string) (*s return &headerVal, nil } - return nil, errors.New("empty gtmss preview context") + // no value found + return nil, nil } diff --git a/pkg/transform/gtmss_preview_test.go b/pkg/transform/gtmss_preview_test.go index 91876538..76d78bd8 100644 --- a/pkg/transform/gtmss_preview_test.go +++ b/pkg/transform/gtmss_preview_test.go @@ -39,10 +39,8 @@ func TestGTMSSPreview(t *testing.T) { "success": { Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "x-gtm-server-preview": { - "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", - }, + HTTPHeaders: map[string]string{ + "x-gtm-server-preview": "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", }, }, "filtered": nil, @@ -103,8 +101,8 @@ func TestGTMSSPreview(t *testing.T) { InputMsg: &models.Message{ Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, InputInterState: nil, @@ -112,11 +110,9 @@ func TestGTMSSPreview(t *testing.T) { "success": { Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, - "x-gtm-server-preview": { - "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", - }, + HTTPHeaders: map[string]string{ + "foo": "bar", + "x-gtm-server-preview": "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", }, }, "filtered": nil, @@ -133,9 +129,9 @@ func TestGTMSSPreview(t *testing.T) { InputMsg: &models.Message{ Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, - "x-gtm-server-preview": {"existing"}, + HTTPHeaders: map[string]string{ + "foo": "bar", + "x-gtm-server-preview": "existing", }, }, InputInterState: nil, @@ -143,12 +139,9 @@ func TestGTMSSPreview(t *testing.T) { "success": { Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, - "x-gtm-server-preview": { - "existing", - "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", - }, + HTTPHeaders: map[string]string{ + "foo": "bar", + "x-gtm-server-preview": "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", }, }, "filtered": nil, @@ -165,17 +158,15 @@ func TestGTMSSPreview(t *testing.T) { InputMsg: &models.Message{ Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{}, + HTTPHeaders: map[string]string{}, }, InputInterState: nil, Expected: map[string]*models.Message{ "success": { Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "x-gtm-server-preview": { - "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", - }, + HTTPHeaders: map[string]string{ + "x-gtm-server-preview": "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", }, }, "filtered": nil, @@ -185,29 +176,29 @@ func TestGTMSSPreview(t *testing.T) { Error: nil, }, { - Scenario: "extract_fails_with_existing_headers", + Scenario: "not_found_with_existing_headers", Ctx: "app_id", Property: "x-gtm-server-preview", HeaderKey: "x-gtm-server-preview", InputMsg: &models.Message{ Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{"foo": {"bar"}}, + HTTPHeaders: map[string]string{"foo": "bar"}, }, InputInterState: nil, Expected: map[string]*models.Message{ - "success": nil, - "filtered": nil, - "failed": { + "success": { Data: spTsvWithGtmss, PartitionKey: "pk", - HTTPHeaders: map[string][]string{ - "foo": {"bar"}, + HTTPHeaders: map[string]string{ + "foo": "bar", }, }, + "filtered": nil, + "failed": nil, }, - ExpInterState: nil, - Error: errors.New("invalid gtmss preview context"), + ExpInterState: spTsvWithGtmssParsed, + Error: nil, }, } @@ -248,9 +239,7 @@ func TestGTMSSPreview(t *testing.T) { } func TestExtractHeaderValue(t *testing.T) { - expectedValues := []string{ - "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==", - } + expectedValue := "ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw==" testCases := []struct { Scenario string Event analytics.ParsedEvent @@ -264,7 +253,7 @@ func TestExtractHeaderValue(t *testing.T) { Event: spTsvWithGtmssParsed, Ctx: "contexts_com_google_tag-manager_server-side_preview_mode_1", Prop: "x-gtm-server-preview", - Expected: &expectedValues[0], + Expected: &expectedValue, Error: nil, }, { @@ -281,15 +270,15 @@ func TestExtractHeaderValue(t *testing.T) { Ctx: "foo", Prop: "bar", Expected: nil, - Error: errors.New("Cannot transform event"), + Error: errors.New("wrong number of fields"), }, { - Scenario: "not_a_context", + Scenario: "not_a_context_same_as_no_such_context", Event: spTsvNoGtmssParsed, Ctx: "app_id", Prop: "foobar", Expected: nil, - Error: errors.New("invalid gtmss preview context"), + Error: nil, }, { Scenario: "invalid_header_value", @@ -300,12 +289,20 @@ func TestExtractHeaderValue(t *testing.T) { Error: errors.New("invalid header value"), }, { - Scenario: "missing_header_property", + Scenario: "event_without_contexts", + Event: spTsvNoCtxParsed, + Ctx: "contexts_com_snowplowanalytics_snowplow_web_page_1", + Prop: "id", + Expected: nil, + Error: nil, + }, + { + Scenario: "missing_property", Event: fakeSpTsvParsed, Ctx: "contexts_com_snowplowanalytics_snowplow_web_page_1", Prop: "doesNotExist", Expected: nil, - Error: errors.New("missing header property"), + Error: nil, }, } @@ -313,7 +310,6 @@ func TestExtractHeaderValue(t *testing.T) { t.Run(tt.Scenario, func(t *testing.T) { assert := assert.New(t) result, err := extractHeaderValue(tt.Event, tt.Ctx, tt.Prop) - if err == nil && tt.Error != nil { t.Fatalf("missed expected error") } @@ -413,6 +409,7 @@ func Benchmark_GTMSSPreview_No_Preview_Ctx_With_intermediate(b *testing.B) { func assertMessagesCompareGtmss(t *testing.T, act, exp *models.Message, hint string) { t.Helper() ok := false + headersOk := false switch { case act == nil: ok = exp == nil @@ -424,7 +421,7 @@ func assertMessagesCompareGtmss(t *testing.T, act, exp *models.Message, hint str pTimeOk := reflect.DeepEqual(act.TimePulled, exp.TimePulled) tTimeOk := reflect.DeepEqual(act.TimeTransformed, exp.TimeTransformed) ackOk := reflect.DeepEqual(act.AckFunc, exp.AckFunc) - headersOk := reflect.DeepEqual(act.HTTPHeaders, exp.HTTPHeaders) + headersOk = reflect.DeepEqual(act.HTTPHeaders, exp.HTTPHeaders) if pkOk && dataOk && cTimeOk && pTimeOk && tTimeOk && ackOk && headersOk { ok = true @@ -432,10 +429,17 @@ func assertMessagesCompareGtmss(t *testing.T, act, exp *models.Message, hint str } if !ok { - t.Errorf("MESSAGES DIFFER\nGOT:\n%s\nEXPECTED[%s]:\n%s\n", - spew.Sdump(act), - hint, - spew.Sdump(exp)) + // message.HTTPHeaders are not printed + if headersOk == false { + t.Errorf("\nHTTPHeaders DIFFER:\nGOT:\n%s\nEXPECTED:\n%s\n", + spew.Sdump(act.HTTPHeaders), + spew.Sdump(exp.HTTPHeaders)) + } else { + t.Errorf("MESSAGES DIFFER\nGOT:\n%s\nEXPECTED[%s]:\n%s\n", + spew.Sdump(act), + hint, + spew.Sdump(exp)) + } } } @@ -447,3 +451,7 @@ var spTsvWithGtmssParsed, _ = analytics.ParseEvent(string(spTsvWithGtmss)) var fakeSpTsv = []byte(`media-test web 2024-03-12 04:25:40.277 2024-03-12 04:25:40.272 2024-03-12 04:25:36.685 page_view 1313411b-282f-4aa9-b37c-c60d4723cf47 spTest js-3.17.0 snowplow-micro-2.0.0-stdout$ snowplow-micro-2.0.0 media_tester 172.17.0.1 23a0eb65-83f6-4957-839e-f3044bfefb99 1 a2f53212-26a3-4781-81d6-f14aa8d4552b http://localhost:8000/ Test Media Tracking http localhost 8000 / {"schema":"iglu:com.snowplowanalytics.snowplow/contexts/jsonschema/1-0-0","data":[{"schema":"iglu:com.snowplowanalytics.snowplow/web_page/jsonschema/1-0-0","data":{"id":["FAILS"]}},{"schema":"iglu:com.google.tag-manager.server-side/user_data/jsonschema/1-0-0","data":{"email_address":"foo@example.com","phone_number":"+15551234567","address":{"first_name":"Jane","last_name":"Doe","street":"123 Fake St","city":"San Francisco","region":"CA","postal_code":"94016","country":"US"}}},{"schema":"iglu:com.snowplowanalytics.snowplow/mobile_context/jsonschema/1-0-2","data":{"osType":"testOsType","osVersion":"testOsVersion","deviceManufacturer":"testDevMan","deviceModel":"testDevModel"}},{"schema":"iglu:com.snowplowanalytics.snowplow/client_session/jsonschema/1-0-2","data":{"userId":"23a0eb65-83f6-4957-839e-f3044bfefb99","sessionId":"73fcdaa3-0164-41ce-a336-fb00c4ebf68c","eventIndex":2,"sessionIndex":1,"previousSessionId":null,"storageMechanism":"COOKIE_1","firstEventId":"327b9ff9-ed5f-40cf-918a-1b1a775ae347","firstEventTimestamp":"2024-03-12T04:25:36.684Z"}}]} Mozilla/5.0 (X11; Linux x86_64; rv:123.0) Gecko/20100101 Firefox/123.0 en-US 1 24 1920 935 Europe/Athens 1920 1080 windows-1252 1920 935 2024-03-12 04:25:40.268 73fcdaa3-0164-41ce-a336-fb00c4ebf68c 2024-03-12 04:25:36.689 com.snowplowanalytics.snowplow page_view jsonschema 1-0-0 `) var fakeSpTsvParsed, _ = analytics.ParseEvent(string(fakeSpTsv)) + +var spTsvNoCtx = []byte(`media-test web 2024-03-12 04:27:01.760 2024-03-12 04:27:01.755 2024-03-12 04:27:01.743 unstruct 9be3afe8-8a62-41ac-93db-12f425d82ac9 spTest js-3.17.0 snowplow-micro-2.0.0-stdout$ snowplow-micro-2.0.0 media_tester 172.17.0.1 23a0eb65-83f6-4957-839e-f3044bfefb99 1 a2f53212-26a3-4781-81d6-f14aa8d4552b http://localhost:8000/?sgtm-preview-header=ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw== http localhost 8000 / sgtm-preview-header=ZW52LTcyN3wtMkMwR084ekptbWxiZmpkcHNIRENBfDE4ZTJkYzgxMDc2NDg1MjVmMzI2Mw== {"schema":"iglu:com.snowplowanalytics.snowplow/unstruct_event/jsonschema/1-0-0","data":{"schema":"iglu:com.snowplowanalytics.snowplow/media_player_event/jsonschema/1-0-0","data":{"type":"play"}}} Mozilla/5.0 (X11; Linux x86_64; rv:123.0) Gecko/20100101 Firefox/123.0 en-US 1 24 1920 935 Europe/Athens 1920 1080 windows-1252 1920 935 2024-03-12 04:27:01.745 73fcdaa3-0164-41ce-a336-fb00c4ebf68c 2024-03-12 04:27:01.753 com.snowplowanalytics.snowplow media_player_event jsonschema 1-0-0 `) + +var spTsvNoCtxParsed, _ = analytics.ParseEvent(string(spTsvNoCtx))