Merge pull request #27 from DEXPRO-Solutions-GmbH/fix/unmarshaling-du…

…plicate-keys

Fix unmarshaling of JSON keys which differ only in case

go.mod

@@ -3,6 +3,7 @@ module github.com/DEXPRO-Solutions-GmbH/easclient
 go 1.21.4
 
 require (
+	github.com/go-json-experiment/json v0.0.0-20231102232822-2e55bd4e08b0
 	github.com/google/uuid v1.5.0
 	github.com/joho/godotenv v1.5.1
 	github.com/stretchr/testify v1.8.4

go.sum

@@ -1,5 +1,7 @@
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
+github.com/go-json-experiment/json v0.0.0-20231102232822-2e55bd4e08b0 h1:ymLjT4f35nQbASLnvxEde4XOBL+Sn7rFuV+FOJqkljg=
+github.com/go-json-experiment/json v0.0.0-20231102232822-2e55bd4e08b0/go.mod h1:6daplAwHHGbUGib4990V3Il26O0OC4aRyvewaaAihaA=
 github.com/google/uuid v1.5.0 h1:1p67kYwdtXjb0gL0BPiP1Av9wiZPo5A8z2cWkTZ+eyU=
 github.com/google/uuid v1.5.0/go.mod h1:TIyPZe4MgqvfeYDBFedMoGGpEw/LqOeaOT+nhxU+yHo=
 github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0=

json.go

@@ -0,0 +1,11 @@
+package easclient
+
+import "github.com/go-json-experiment/json"
+
+// unmarshalJSON is a wrapper around the json library we want to use for unmarshaling.
+//
+// Since the std library does not handle our edge cases, a specialized library is used.
+// Have a look at the tests for details.
+func unmarshalJSON(data []byte, v any, opts ...json.Options) error {
+	return json.Unmarshal(data, v, opts...)
+}

json_test.go

@@ -0,0 +1,96 @@
+package easclient
+
+import (
+	stdjson "encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestUnmarshalOfSimilarKeys contains tests which show a challenge when dealing with the
+// JSON representation of EAS responses:
+//
+// Standard fields are sometimes mixed with custom fields in the same
+// object. The std lib's json package does not strictly enforce that only the field
+// with a matching case is being used. Have a look at the tests and these links for details:
+//
+// - https://github.com/golang/go/issues/14750
+// - https://github.com/go-json-experiment/json
+func TestUnmarshalOfSimilarKeys(t *testing.T) {
+	t.Run("std lib does unmarshal from exact key", func(t *testing.T) {
+		t.Run("uses last key 1", func(t *testing.T) {
+			attachmentStr := `{
+				"id": "0dd018f8-bf23-455d-8214-44e76b24e5db",
+				"Id": "00000000-0000-0000-0000-000000000000"
+			}`
+
+			attachment := RecordAttachment{}
+
+			err := stdjson.Unmarshal([]byte(attachmentStr), &attachment)
+			require.NoError(t, err)
+
+			// This test asserts that the std lib behaves "weired" - given that the Id struct field
+			// should be unmarshalled from the JSON field "id" and not "Id".
+			//
+			// In practice however, the std lib will use the last key it finds, accepting all case-variations.
+			assert.Equal(t, "00000000-0000-0000-0000-000000000000", attachment.Id.String())
+		})
+
+		t.Run("uses last key 2", func(t *testing.T) {
+			attachmentStr := `{
+				"Id": "00000000-0000-0000-0000-000000000000",
+				"id": "0dd018f8-bf23-455d-8214-44e76b24e5db"
+			}`
+
+			attachment := RecordAttachment{}
+
+			err := stdjson.Unmarshal([]byte(attachmentStr), &attachment)
+			require.NoError(t, err)
+
+			// This test asserts that the std lib behaves "weired" - given that the Id struct field
+			// should be unmarshalled from the JSON field "id" and not "Id".
+			//
+			// In practice however, the std lib will use the last key it finds, accepting all case-variations.
+			assert.Equal(t, "0dd018f8-bf23-455d-8214-44e76b24e5db", attachment.Id.String())
+		})
+	})
+
+	t.Run("v2 json lib does unmarshal from exact key", func(t *testing.T) {
+		t.Run("uses correct key 1", func(t *testing.T) {
+			attachmentStr := `{
+				"id": "0dd018f8-bf23-455d-8214-44e76b24e5db",
+				"Id": "00000000-0000-0000-0000-000000000000"
+			}`
+
+			attachment := RecordAttachment{}
+
+			err := unmarshalJSON([]byte(attachmentStr), &attachment)
+			require.NoError(t, err)
+
+			// This test asserts that the std lib behaves "weired" - given that the Id struct field
+			// should be unmarshalled from the JSON field "id" and not "Id".
+			//
+			// In practice however, the std lib will use the last key it finds, accepting all case-variations.
+			assert.Equal(t, "0dd018f8-bf23-455d-8214-44e76b24e5db", attachment.Id.String())
+		})
+
+		t.Run("uses correct key 2", func(t *testing.T) {
+			attachmentStr := `{
+				"Id": "00000000-0000-0000-0000-000000000000",
+				"id": "0dd018f8-bf23-455d-8214-44e76b24e5db"
+			}`
+
+			attachment := RecordAttachment{}
+
+			err := unmarshalJSON([]byte(attachmentStr), &attachment)
+			require.NoError(t, err)
+
+			// This test asserts that the std lib behaves "weired" - given that the Id struct field
+			// should be unmarshalled from the JSON field "id" and not "Id".
+			//
+			// In practice however, the std lib will use the last key it finds, accepting all case-variations.
+			assert.Equal(t, "0dd018f8-bf23-455d-8214-44e76b24e5db", attachment.Id.String())
+		})
+	})
+}

resty.go

@@ -0,0 +1,21 @@
+package easclient
+
+import (
+	"github.com/go-json-experiment/json"
+	"gopkg.in/resty.v1"
+)
+
+func copyRestyClient(c *resty.Client) *resty.Client {
+	// dereference the pointer and copy the value
+	cc := *c
+	return &cc
+}
+
+func adaptRestyClient(c *resty.Client) {
+	c.JSONUnmarshal = func(data []byte, v interface{}) error {
+		opts := []json.Options{
+			json.MatchCaseInsensitiveNames(false),
+		}
+		return unmarshalJSON(data, v, opts...)
+	}
+}

resty_test.go

@@ -0,0 +1,26 @@
+package easclient
+
+import (
+	"errors"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"gopkg.in/resty.v1"
+)
+
+func Test_copyRestyClient(t *testing.T) {
+	t.Run("returns valid copy", func(t *testing.T) {
+		original := resty.New()
+		copied := copyRestyClient(original)
+
+		require.NotSame(t, original, copied)
+
+		t.Run("copy modifications do not affect original", func(t *testing.T) {
+			copied.JSONUnmarshal = func(data []byte, v interface{}) error {
+				return errors.New("this is some error")
+			}
+
+			require.NotSame(t, original.JSONUnmarshal, copied.JSONUnmarshal)
+		})
+	})
+}

server_client.go

@@ -6,6 +6,9 @@ type ServerClient struct {
 	c *resty.Client
 }
 
+// NewServerClient creates a new client for server interaction.
 func NewServerClient(c *resty.Client) *ServerClient {
+	c = copyRestyClient(c)
+	adaptRestyClient(c)
 	return &ServerClient{c: c}
 }

store_client.go

@@ -11,7 +11,10 @@ type StoreClient struct {
 	c *resty.Client
 }
 
+// NewStoreClient creates a new client for store interaction.
 func NewStoreClient(c *resty.Client) *StoreClient {
+	c = copyRestyClient(c)
+	adaptRestyClient(c)
 	return &StoreClient{c: c}
 }