Repository: yarpc/yab
Pull request #321 (2021-02-25 20:16:29)

Change input-decoder interface to return YAML request body
Prior to this change input decoder exposed method which returns JSON body, but existing `Serializer` interface already exposes methods with YAML body. This change upholds the consistency between all the interfaces and also avoids the lossy conversion from YAML to JSON type.

Fixes https://github.com/yarpc/yab/pull/314#discussion_r565766802

codecov[bot]: # [Codecov](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=h1) Report
> Merging [#321](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=desc) (6f3bbdc) into [dev](https://codecov.io/gh/yarpc/yab/commit/2a495adb848796c98f72f6a83702339c0114f9c5?el=desc) (2a495ad) will **increase** coverage by `0.18%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/yarpc/yab/pull/321/graphs/tree.svg?width=650&height=150&src=pr&token=yEgrqUTCuh)](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##              dev     #321      +/-   ##
==========================================
+ Coverage   90.27%   90.45%   +0.18%     
==========================================
  Files          52       52              
  Lines        2590     2587       -3     
==========================================
+ Hits         2338     2340       +2     
+ Misses        178      175       -3     
+ Partials       74       72       -2     
```


| [Impacted Files](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [encoding/inputdecoder/decoder.go](https://codecov.io/gh/yarpc/yab/pull/321/diff?src=pr&el=tree#diff-ZW5jb2RpbmcvaW5wdXRkZWNvZGVyL2RlY29kZXIuZ28=) | `100.00% <100.00%> (+8.69%)` | :arrow_up: |
| [encoding/protobuf.go](https://codecov.io/gh/yarpc/yab/pull/321/diff?src=pr&el=tree#diff-ZW5jb2RpbmcvcHJvdG9idWYuZ28=) | `92.72% <100.00%> (ø)` | |
| [handler.go](https://codecov.io/gh/yarpc/yab/pull/321/diff?src=pr&el=tree#diff-aGFuZGxlci5nbw==) | `81.57% <0.00%> (+1.97%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=footer). Last update [2a495ad...6f3bbdc](https://codecov.io/gh/yarpc/yab/pull/321?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/encoding/inputdecoder/decoder.go b/encoding/inputdecoder/decoder.go
index 4087dbc..52b2d0d 100644
--- a/encoding/inputdecoder/decoder.go
+++ b/encoding/inputdecoder/decoder.go
@@ -25,22 +25,21 @@ import (
 	"encoding/json"
 	"io"
 
-	gyaml "github.com/ghodss/yaml"
 	"gopkg.in/yaml.v2"
 )
 
 // Decoder interface exposes method for reading multiple input requests
 type Decoder interface {
-	// NextJSONBytes returns JSON marshaled bytes of the next request body
+	// NextYAMLBytes returns YAML marshaled bytes of the next request body
 	// It returns io.EOF when there are no more requests
-	NextJSONBytes() ([]byte, error)
+	NextYAMLBytes() ([]byte, error)
 }
 
 // jsonInputDecoder parses multiple JSON objects from given reader
 // JSON objects can be delimited by space or newline
 type jsonInputDecoder struct{ dec *json.Decoder }
 
-func (r *jsonInputDecoder) NextJSONBytes() ([]byte, error) {
+func (r *jsonInputDecoder) NextYAMLBytes() ([]byte, error) {
 	if !r.dec.More() {
 		return nil, io.EOF
 	}
@@ -54,18 +53,13 @@ func (r *jsonInputDecoder) NextJSONBytes() ([]byte, error) {
 // consecutive YAML objects must be delimited by `---`
 type yamlInputDecoder struct{ dec *yaml.Decoder }
 
-func (r *yamlInputDecoder) NextJSONBytes() ([]byte, error) {
+func (r *yamlInputDecoder) NextYAMLBytes() ([]byte, error) {
 	var v interface{}
 	if err := r.dec.Decode(&v); err != nil {
 		return nil, err
 	}
 
-	bytes, err := yaml.Marshal(v)
-	if err != nil {
-		return nil, err
-	}
-
-	return gyaml.YAMLToJSON(bytes)
+	return yaml.Marshal(v)
 }
 
 // isJSONInput assumes the input is JSON compatible if the initial
diff --git a/encoding/protobuf.go b/encoding/protobuf.go
index 75c6731..556e886 100644
--- a/encoding/protobuf.go
+++ b/encoding/protobuf.go
@@ -6,12 +6,12 @@ import (
 	"io"
 	"strings"
 
+	"github.com/ghodss/yaml"
 	"github.com/yarpc/yab/encoding/encodingerror"
 	"github.com/yarpc/yab/encoding/inputdecoder"
 	"github.com/yarpc/yab/protobuf"
 	"github.com/yarpc/yab/transport"
 
-	"github.com/ghodss/yaml"
 	"github.com/golang/protobuf/jsonpb"
 	"github.com/golang/protobuf/proto"
 	"github.com/jhump/protoreflect/desc"
@@ -126,11 +126,7 @@ func (p protoSerializer) Request(body []byte) (*transport.Request, error) {
 		return nil, fmt.Errorf("request method must be invoked only with unary rpc method: %q", p.method.GetInputType().GetFullyQualifiedName())
 	}
 
-	jsonContent, err := yaml.YAMLToJSON(body)
-	if err != nil {
-		return nil, err
-	}
-	bytes, err := p.encode(jsonContent)
+	bytes, err := p.encode(body)
 	if err != nil {
 		return nil, err
 	}
@@ -190,7 +186,12 @@ func (p protoSerializer) CheckSuccess(body *transport.Response) error {
 	return err
 }
 
-func (p protoSerializer) encode(jsonBytes []byte) ([]byte, error) {
+func (p protoSerializer) encode(yamlBytes []byte) ([]byte, error) {
+	jsonBytes, err := yaml.YAMLToJSON(yamlBytes)
+	if err != nil {
+		return nil, err
+	}
+
 	req := dynamic.NewMessage(p.method.GetInputType())
 	if err := req.UnmarshalJSON(jsonBytes); err != nil {
 		return nil, fmt.Errorf("could not parse given request body as message of type %q: %v", p.method.GetInputType().GetFullyQualifiedName(), err)
@@ -210,7 +211,7 @@ type protoStreamRequestReader struct {
 }
 
 func (p protoStreamRequestReader) NextBody() ([]byte, error) {
-	body, err := p.decoder.NextJSONBytes()
+	body, err := p.decoder.NextYAMLBytes()
 	if err != nil {
 		return nil, err
 	}


Test patch:
diff --git a/encoding/inputdecoder/decoder_test.go b/encoding/inputdecoder/decoder_test.go
index cb997aa..7e51e18 100644
--- a/encoding/inputdecoder/decoder_test.go
+++ b/encoding/inputdecoder/decoder_test.go
@@ -62,13 +62,13 @@ func TestRequestReader(t *testing.T) {
 		reader, err := New(buf)
 		assert.NotNil(t, reader, "expected non-nil reader")
 		assert.NoError(t, err)
-		body, err := reader.NextJSONBytes()
+		body, err := reader.NextYAMLBytes()
 		assert.Equal(t, []byte(`{"test":1}`), body)
 		assert.NoError(t, err)
-		body, err = reader.NextJSONBytes()
+		body, err = reader.NextYAMLBytes()
 		assert.Equal(t, []byte(`{"test":2}`), body)
 		assert.NoError(t, err)
-		body, err = reader.NextJSONBytes()
+		body, err = reader.NextYAMLBytes()
 		assert.Nil(t, body)
 		assert.EqualError(t, err, io.EOF.Error())
 	})
@@ -79,13 +79,13 @@ test: 2`))
 		reader, err := New(buf)
 		assert.NotNil(t, reader, "expected non-nil reader")
 		assert.NoError(t, err)
-		body, err := reader.NextJSONBytes()
-		assert.Equal(t, []byte(`{"test":1}`), body)
+		body, err := reader.NextYAMLBytes()
+		assert.Equal(t, []byte("test: 1\n"), body)
 		assert.NoError(t, err)
-		body, err = reader.NextJSONBytes()
-		assert.Equal(t, []byte(`{"test":2}`), body)
+		body, err = reader.NextYAMLBytes()
+		assert.Equal(t, []byte("test: 2\n"), body)
 		assert.NoError(t, err)
-		body, err = reader.NextJSONBytes()
+		body, err = reader.NextYAMLBytes()
 		assert.Nil(t, body)
 		assert.EqualError(t, err, io.EOF.Error())
 	})
@@ -96,10 +96,10 @@ test: 2`))
 		reader, err := New(buf)
 		assert.NotNil(t, reader, "expected non-nil reader")
 		assert.NoError(t, err)
-		body, err := reader.NextJSONBytes()
+		body, err := reader.NextYAMLBytes()
 		assert.Equal(t, []byte(`{"test": 1}`), body)
 		assert.NoError(t, err)
-		body, err = reader.NextJSONBytes()
+		body, err = reader.NextYAMLBytes()
 		assert.Error(t, err)
 	})
 	t.Run("empty input", func(t *testing.T) {
@@ -107,7 +107,7 @@ test: 2`))
 		reader, err := New(buf)
 		assert.NotNil(t, reader, "expected non-nil reader")
 		assert.NoError(t, err)
-		body, err := reader.NextJSONBytes()
+		body, err := reader.NextYAMLBytes()
 		assert.Equal(t, []byte(nil), body)
 		assert.EqualError(t, err, io.EOF.Error())
 	})
@@ -116,7 +116,7 @@ test: 2`))
 		reader, err := New(buf)
 		assert.NotNil(t, reader, "expected non-nil reader")
 		assert.NoError(t, err)
-		body, err := reader.NextJSONBytes()
+		body, err := reader.NextYAMLBytes()
 		assert.Equal(t, []byte(nil), body)
 		assert.EqualError(t, err, io.EOF.Error())
 	})
@@ -126,7 +126,7 @@ test: 2`))
 		reader, err := New(buf)
 		assert.NotNil(t, reader, "expected non-nil reader")
 		assert.NoError(t, err)
-		body, err := reader.NextJSONBytes()
+		body, err := reader.NextYAMLBytes()
 		assert.Equal(t, []byte(nil), body)
 		assert.EqualError(t, err, `yaml: line 2: found character that cannot start any token`)
 	})