From bf326e083efa2e131a097e964dee211b9d2f59c1 Mon Sep 17 00:00:00 2001 From: Brad Moylan Date: Tue, 30 Aug 2022 17:52:36 -0700 Subject: [PATCH 1/4] improvement: codecs.JSON recognizes objects implementing json.Marshaler and json.Unmarshaler --- conjure-go-contract/codecs/json.go | 59 +++++++++++++++++++--- conjure-go-contract/codecs/json_test.go | 62 +++++++++++++++++++++++ conjure-go-contract/codecs/json_types.go | 63 ++++++++++++++++++++++++ 3 files changed, 177 insertions(+), 7 deletions(-) create mode 100644 conjure-go-contract/codecs/json_test.go create mode 100644 conjure-go-contract/codecs/json_types.go diff --git a/conjure-go-contract/codecs/json.go b/conjure-go-contract/codecs/json.go index 998eb8da..ee083be2 100644 --- a/conjure-go-contract/codecs/json.go +++ b/conjure-go-contract/codecs/json.go @@ -15,10 +15,12 @@ package codecs import ( - "fmt" + "bytes" + "encoding/json" "io" "github.com/palantir/pkg/safejson" + werror "github.com/palantir/witchcraft-go-error" ) const ( @@ -37,14 +39,37 @@ func (codecJSON) Accept() string { } func (codecJSON) Decode(r io.Reader, v interface{}) error { + if decoder, ok := v.(jsonDecoder); ok { + err := decoder.DecodeJSON(r) + return werror.Wrap(err, "DecodeJSON") + } + if unmarshaler, ok := v.(json.Unmarshaler); ok { + data, err := io.ReadAll(r) + if err != nil { + return werror.Wrap(err, "read failed") + } + err = unmarshaler.UnmarshalJSON(data) + return werror.Wrap(err, "UnmarshalJSON") + } if err := safejson.Decoder(r).Decode(v); err != nil { - return fmt.Errorf("failed to decode JSON-encoded value: %s", err.Error()) + return werror.Wrap(err, "json.Decode") } return nil } func (c codecJSON) Unmarshal(data []byte, v interface{}) error { - return safejson.Unmarshal(data, v) + if unmarshaler, ok := v.(json.Unmarshaler); ok { + err := unmarshaler.UnmarshalJSON(data) + return werror.Wrap(err, "UnmarshalJSON") + } + if decoder, ok := v.(jsonDecoder); ok { + err := decoder.DecodeJSON(bytes.NewReader(data)) + return werror.Wrap(err, "DecodeJSON") + } + if err := safejson.Unmarshal(data, v); err != nil { + return werror.Wrap(err, "json.Unmarshal") + } + return nil } func (codecJSON) ContentType() string { @@ -52,12 +77,32 @@ func (codecJSON) ContentType() string { } func (codecJSON) Encode(w io.Writer, v interface{}) error { - if err := safejson.Encoder(w).Encode(v); err != nil { - return fmt.Errorf("failed to JSON-encode value: %s", err.Error()) + if encoder, ok := v.(jsonEncoder); ok { + err := encoder.EncodeJSON(w) + return werror.Wrap(err, "EncodeJSON") } - return nil + if marshaler, ok := v.(json.Marshaler); ok { + out, err := marshaler.MarshalJSON() + if err != nil { + return werror.Wrap(err, "MarshalJSON") + } + _, err = w.Write(out) + return werror.Wrap(err, "write failed") + } + err := safejson.Encoder(w).Encode(v) + return werror.Wrap(err, "json.Encode") } func (c codecJSON) Marshal(v interface{}) ([]byte, error) { - return safejson.Marshal(v) + if marshaler, ok := v.(json.Marshaler); ok { + out, err := marshaler.MarshalJSON() + return out, werror.Wrap(err, "MarshalJSON") + } + if encoder, ok := v.(jsonEncoder); ok { + data := bytes.NewBuffer(nil) + err := encoder.EncodeJSON(data) + return data.Bytes(), werror.Wrap(err, "EncodeJSON") + } + out, err := safejson.Marshal(v) + return out, werror.Wrap(err, "json.Marshal") } diff --git a/conjure-go-contract/codecs/json_test.go b/conjure-go-contract/codecs/json_test.go new file mode 100644 index 00000000..e60435ce --- /dev/null +++ b/conjure-go-contract/codecs/json_test.go @@ -0,0 +1,62 @@ +// Copyright (c) 2022 Palantir Technologies. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package codecs_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/palantir/conjure-go-runtime/v2/conjure-go-contract/codecs" + "github.com/stretchr/testify/require" +) + +func TestJSON_UsesMethodsWhenImplemented(t *testing.T) { + t.Run("Marshal_Unmarshal", func(t *testing.T) { + obj := map[string]testJSONObject{} + err := codecs.JSON.Unmarshal([]byte(`{"key":null}`), &obj) + require.NoError(t, err) + require.Contains(t, obj, "key") + require.Equal(t, `null`, string(obj["key"].data)) + data, err := codecs.JSON.Marshal(obj) + require.NoError(t, err) + require.Equal(t, `{"key":null}`, string(data)) + }) + t.Run("Encode_Decode", func(t *testing.T) { + obj := map[string]testJSONObject{} + r := strings.NewReader(`{"key":"abc"}`) + err := codecs.JSON.Decode(r, &obj) + require.NoError(t, err) + require.Contains(t, obj, "key") + require.Equal(t, `"abc"`, string(obj["key"].data)) + out := bytes.Buffer{} + err = codecs.JSON.Encode(&out, obj) + require.NoError(t, err) + require.Equal(t, "{\"key\":\"abc\"}\n", out.String()) + }) +} + +type testJSONObject struct { + data []byte +} + +func (t testJSONObject) MarshalJSON() ([]byte, error) { + return t.data, nil +} + +func (t *testJSONObject) UnmarshalJSON(data []byte) error { + t.data = data + return nil +} diff --git a/conjure-go-contract/codecs/json_types.go b/conjure-go-contract/codecs/json_types.go new file mode 100644 index 00000000..b496e156 --- /dev/null +++ b/conjure-go-contract/codecs/json_types.go @@ -0,0 +1,63 @@ +// Copyright (c) 2022 Palantir Technologies. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package codecs + +import ( + "io" +) + +// jsonDecoder is an interface which may be implemented by objects passed to the Decode and Unmarshal methods. +// If implemented, the standard json.Decoder is bypassed. +type jsonDecoder interface { + DecodeJSON(r io.Reader) error +} + +// JSONDecoderFunc is an alias type which implements jsonDecoder. +// It can be used to declare anonymous/dynamic unmarshal logic. +type JSONDecoderFunc func(r io.Reader) error + +func (f JSONDecoderFunc) DecodeJSON(r io.Reader) error { + return f(r) +} + +// JSONUnmarshalFunc is an alias type which implements json.Unmarshaler. +// It can be used to declare anonymous/dynamic unmarshal logic. +type JSONUnmarshalFunc func([]byte) error + +func (f JSONUnmarshalFunc) UnmarshalJSON(data []byte) error { + return f(data) +} + +// jsonEncoder is an interface which may be implemented by objects passed to the Encode and Marshal methods. +// If implemented, the standard json.Encoder is bypassed. +type jsonEncoder interface { + EncodeJSON(w io.Writer) error +} + +// JSONEncoderFunc is an alias type which implements jsonEncoder. +// It can be used to declare anonymous/dynamic marshal logic. +type JSONEncoderFunc func(w io.Writer) error + +func (f JSONEncoderFunc) EncodeJSON(w io.Writer) error { + return f(w) +} + +// JSONMarshalFunc is an alias type which implements json.Marshaler. +// It can be used to declare anonymous/dynamic marshal logic. +type JSONMarshalFunc func() ([]byte, error) + +func (f JSONMarshalFunc) MarshalJSON() ([]byte, error) { + return f() +} From c7b0b2055ea6774a519cd97107382ef94950fb87 Mon Sep 17 00:00:00 2001 From: svc-changelog Date: Wed, 31 Aug 2022 00:57:16 +0000 Subject: [PATCH 2/4] Add generated changelog entries --- changelog/@unreleased/pr-353.v2.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/@unreleased/pr-353.v2.yml diff --git a/changelog/@unreleased/pr-353.v2.yml b/changelog/@unreleased/pr-353.v2.yml new file mode 100644 index 00000000..4ccc0767 --- /dev/null +++ b/changelog/@unreleased/pr-353.v2.yml @@ -0,0 +1,5 @@ +type: improvement +improvement: + description: Codecs.JSON recognizes objects implementing Marshaler and Unmarshaler + links: + - https://github.com/palantir/conjure-go-runtime/pull/353 From b94ac18dfb19bc7c229d38a0bd1cfe9f2037ec13 Mon Sep 17 00:00:00 2001 From: Brad Moylan Date: Tue, 30 Aug 2022 18:04:21 -0700 Subject: [PATCH 3/4] tests --- changelog/@unreleased/pr-353.v2.yml | 4 +++- conjure-go-contract/errors/unmarshal_test.go | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/changelog/@unreleased/pr-353.v2.yml b/changelog/@unreleased/pr-353.v2.yml index 4ccc0767..0a71106b 100644 --- a/changelog/@unreleased/pr-353.v2.yml +++ b/changelog/@unreleased/pr-353.v2.yml @@ -1,5 +1,7 @@ type: improvement improvement: - description: Codecs.JSON recognizes objects implementing Marshaler and Unmarshaler + description: > + codecs.JSON recognizes objects implementing Marshaler and Unmarshaler. + Added utility types for anonymous functions implementing JSON encoding. links: - https://github.com/palantir/conjure-go-runtime/pull/353 diff --git a/conjure-go-contract/errors/unmarshal_test.go b/conjure-go-contract/errors/unmarshal_test.go index 0664196c..f05a5e91 100644 --- a/conjure-go-contract/errors/unmarshal_test.go +++ b/conjure-go-contract/errors/unmarshal_test.go @@ -127,17 +127,17 @@ func TestUnmarshalError(t *testing.T) { { name: "plaintext", inRaw: "404 Not Found", - expectErr: "failed to unmarshal body as conjure error: json: cannot unmarshal number into Go value of type struct { Name string \"json:\\\"errorName\\\"\" }", + expectErr: "failed to unmarshal body as conjure error: json.Unmarshal: json: cannot unmarshal number into Go value of type struct { Name string \"json:\\\"errorName\\\"\" }", }, { name: "other json", inRaw: `{"foo":"bar"}`, - expectErr: "failed to unmarshal body using registered type: errors: error name does not match regexp `^(([A-Z][a-z0-9]+)+):(([A-Z][a-z0-9]+)+)$`", + expectErr: "failed to unmarshal body using registered type: json.Unmarshal: errors: error name does not match regexp `^(([A-Z][a-z0-9]+)+):(([A-Z][a-z0-9]+)+)$`", }, { name: "incomplete error json", inRaw: `{"errorName":"Default:Internal"}`, - expectErr: "failed to unmarshal body using registered type: errors: invalid combination of default error name and error code", + expectErr: "failed to unmarshal body using registered type: json.Unmarshal: errors: invalid combination of default error name and error code", }, } { t.Run(test.name, func(t *testing.T) { From cde1f868fa45be144532976f18249093889c2fe2 Mon Sep 17 00:00:00 2001 From: Brad Moylan Date: Sat, 15 Apr 2023 08:31:10 -0700 Subject: [PATCH 4/4] shrink diff --- conjure-go-contract/codecs/json.go | 78 +++++++++----------- conjure-go-contract/errors/unmarshal_test.go | 6 +- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/conjure-go-contract/codecs/json.go b/conjure-go-contract/codecs/json.go index ee083be2..e013a6ac 100644 --- a/conjure-go-contract/codecs/json.go +++ b/conjure-go-contract/codecs/json.go @@ -39,37 +39,29 @@ func (codecJSON) Accept() string { } func (codecJSON) Decode(r io.Reader, v interface{}) error { - if decoder, ok := v.(jsonDecoder); ok { - err := decoder.DecodeJSON(r) - return werror.Wrap(err, "DecodeJSON") - } - if unmarshaler, ok := v.(json.Unmarshaler); ok { + switch vv := v.(type) { + case jsonDecoder: + return werror.Convert(vv.DecodeJSON(r)) + case json.Unmarshaler: data, err := io.ReadAll(r) if err != nil { - return werror.Wrap(err, "read failed") + return werror.Convert(err) } - err = unmarshaler.UnmarshalJSON(data) - return werror.Wrap(err, "UnmarshalJSON") - } - if err := safejson.Decoder(r).Decode(v); err != nil { - return werror.Wrap(err, "json.Decode") + return werror.Convert(vv.UnmarshalJSON(data)) + default: + return werror.Convert(safejson.Decoder(r).Decode(v)) } - return nil } func (c codecJSON) Unmarshal(data []byte, v interface{}) error { - if unmarshaler, ok := v.(json.Unmarshaler); ok { - err := unmarshaler.UnmarshalJSON(data) - return werror.Wrap(err, "UnmarshalJSON") - } - if decoder, ok := v.(jsonDecoder); ok { - err := decoder.DecodeJSON(bytes.NewReader(data)) - return werror.Wrap(err, "DecodeJSON") + switch vv := v.(type) { + case json.Unmarshaler: + return werror.Convert(vv.UnmarshalJSON(data)) + case jsonDecoder: + return werror.Convert(vv.DecodeJSON(bytes.NewReader(data))) + default: + return werror.Convert(safejson.Unmarshal(data, v)) } - if err := safejson.Unmarshal(data, v); err != nil { - return werror.Wrap(err, "json.Unmarshal") - } - return nil } func (codecJSON) ContentType() string { @@ -77,32 +69,32 @@ func (codecJSON) ContentType() string { } func (codecJSON) Encode(w io.Writer, v interface{}) error { - if encoder, ok := v.(jsonEncoder); ok { - err := encoder.EncodeJSON(w) - return werror.Wrap(err, "EncodeJSON") - } - if marshaler, ok := v.(json.Marshaler); ok { - out, err := marshaler.MarshalJSON() + switch vv := v.(type) { + case jsonEncoder: + return werror.Convert(vv.EncodeJSON(w)) + case json.Marshaler: + out, err := vv.MarshalJSON() if err != nil { - return werror.Wrap(err, "MarshalJSON") + return werror.Convert(err) } _, err = w.Write(out) - return werror.Wrap(err, "write failed") + return werror.Convert(err) + default: + return werror.Convert(safejson.Encoder(w).Encode(v)) } - err := safejson.Encoder(w).Encode(v) - return werror.Wrap(err, "json.Encode") } func (c codecJSON) Marshal(v interface{}) ([]byte, error) { - if marshaler, ok := v.(json.Marshaler); ok { - out, err := marshaler.MarshalJSON() - return out, werror.Wrap(err, "MarshalJSON") - } - if encoder, ok := v.(jsonEncoder); ok { - data := bytes.NewBuffer(nil) - err := encoder.EncodeJSON(data) - return data.Bytes(), werror.Wrap(err, "EncodeJSON") + switch vv := v.(type) { + case json.Marshaler: + out, err := vv.MarshalJSON() + return out, werror.Convert(err) + case jsonEncoder: + data := bytes.NewBuffer(nil) // TODO: Use bytesbuffer pool + err := vv.EncodeJSON(data) + return data.Bytes(), werror.Convert(err) + default: + out, err := safejson.Marshal(v) + return out, werror.Convert(err) } - out, err := safejson.Marshal(v) - return out, werror.Wrap(err, "json.Marshal") } diff --git a/conjure-go-contract/errors/unmarshal_test.go b/conjure-go-contract/errors/unmarshal_test.go index f05a5e91..0664196c 100644 --- a/conjure-go-contract/errors/unmarshal_test.go +++ b/conjure-go-contract/errors/unmarshal_test.go @@ -127,17 +127,17 @@ func TestUnmarshalError(t *testing.T) { { name: "plaintext", inRaw: "404 Not Found", - expectErr: "failed to unmarshal body as conjure error: json.Unmarshal: json: cannot unmarshal number into Go value of type struct { Name string \"json:\\\"errorName\\\"\" }", + expectErr: "failed to unmarshal body as conjure error: json: cannot unmarshal number into Go value of type struct { Name string \"json:\\\"errorName\\\"\" }", }, { name: "other json", inRaw: `{"foo":"bar"}`, - expectErr: "failed to unmarshal body using registered type: json.Unmarshal: errors: error name does not match regexp `^(([A-Z][a-z0-9]+)+):(([A-Z][a-z0-9]+)+)$`", + expectErr: "failed to unmarshal body using registered type: errors: error name does not match regexp `^(([A-Z][a-z0-9]+)+):(([A-Z][a-z0-9]+)+)$`", }, { name: "incomplete error json", inRaw: `{"errorName":"Default:Internal"}`, - expectErr: "failed to unmarshal body using registered type: json.Unmarshal: errors: invalid combination of default error name and error code", + expectErr: "failed to unmarshal body using registered type: errors: invalid combination of default error name and error code", }, } { t.Run(test.name, func(t *testing.T) {