Skip to content

Commit

Permalink
Add response data streaming facility
Browse files Browse the repository at this point in the history 
Here we add a new ResponseDecoder: ByteStreamer. This allows user to get
a stream of bytes 'as is' i.e. without any decoding. This is useful for
all those streaming use cases like downloading files, binary blobs etc.

This is also useful when response data is just plain text for e.g. the
5XX responses from API gateways.

Side changes:
- The xml stuff in testing code no longer needed as this new
decoder is used as non-default decoder.

- Code reformatting as per gopls/gofumpt style.
  • Loading branch information
@rsjethani
rsjethani committed Jul 11, 2023
1 parent b04385a commit b285c20
Show file tree
Hide file tree
Showing 2 changed files with 118 additions and 65 deletions.
31 changes: 29 additions & 2 deletions response.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package sling

import (
"encoding/json"
"fmt"
"io"
"net/http"
)

Expand All @@ -12,11 +14,36 @@ type ResponseDecoder interface {
}

// jsonDecoder decodes http response JSON into a JSON-tagged struct value.
type jsonDecoder struct {
}
type jsonDecoder struct{}

// Decode decodes the Response Body into the value pointed to by v.
// Caller must provide a non-nil v and close the resp.Body.
func (d jsonDecoder) Decode(resp *http.Response, v interface{}) error {
return json.NewDecoder(resp.Body).Decode(v)
}

// ByteStreamer is a [sling.ResponseDecoder] which simply forwards response data 'as is' rather than trying to deocde
// it. This is useful when 1/ response is actually just plain text (like 5XX response from API gateways). 2/ response
// data is a byte stream representing some file or a binary blob.
//
// It leverages existing facilities of automatic discarding and closing of response body so the user does not need to
// care about it.
type ByteStreamer struct{}

// Decode simply tries to copy response data into v assuming its an [io.Writer] instance. Assuming so little about v
// gives consumers a lot of choice about consuming response data. They can wait for all data to be dumped into some
// buffer then act on it or they can read as soon as data gets written.
func (d ByteStreamer) Decode(resp *http.Response, v any) error {
var w io.Writer
w, ok := v.(io.Writer)
if !ok {
return fmt.Errorf("expected type: %T; got: %T", w, v)
}

_, err := io.Copy(w, resp.Body)
if err != nil {
return fmt.Errorf("failed copying response data to v: %w", err)
}

return nil
}
152 changes: 89 additions & 63 deletions sling_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package sling
import (
"bytes"
"context"
"encoding/xml"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -40,13 +39,6 @@ type FakeModel struct {

var modelA = FakeModel{Text: "note", FavoriteCount: 12}

// Non-Json response decoder
type xmlResponseDecoder struct{}

func (d xmlResponseDecoder) Decode(resp *http.Response, v interface{}) error {
return xml.NewDecoder(resp.Body).Decode(v)
}

func TestNew(t *testing.T) {
sling := New()
if sling.httpClient != http.DefaultClient {
Expand All @@ -64,14 +56,14 @@ func TestSlingNew(t *testing.T) {
fakeBodyProvider := jsonBodyProvider{FakeModel{}}

cases := []*Sling{
&Sling{httpClient: &http.Client{}, method: "GET", rawURL: "http://example.com"},
&Sling{httpClient: nil, method: "", rawURL: "http://example.com"},
&Sling{queryStructs: make([]interface{}, 0)},
&Sling{queryStructs: []interface{}{paramsA}},
&Sling{queryStructs: []interface{}{paramsA, paramsB}},
&Sling{bodyProvider: fakeBodyProvider},
&Sling{bodyProvider: fakeBodyProvider},
&Sling{bodyProvider: nil},
{httpClient: &http.Client{}, method: "GET", rawURL: "http://example.com"},
{httpClient: nil, method: "", rawURL: "http://example.com"},
{queryStructs: make([]interface{}, 0)},
{queryStructs: []interface{}{paramsA}},
{queryStructs: []interface{}{paramsA, paramsB}},
{bodyProvider: fakeBodyProvider},
{bodyProvider: fakeBodyProvider},
{bodyProvider: nil},
New().Add("Content-Type", "application/json"),
New().Add("A", "B").Add("a", "c").New(),
New().Add("A", "B").New().Add("a", "c"),
Expand Down Expand Up @@ -222,14 +214,14 @@ func TestAddHeader(t *testing.T) {
sling *Sling
expectedHeader map[string][]string
}{
{New().Add("authorization", "OAuth key=\"value\""), map[string][]string{"Authorization": []string{"OAuth key=\"value\""}}},
{New().Add("authorization", "OAuth key=\"value\""), map[string][]string{"Authorization": {"OAuth key=\"value\""}}},
// header keys should be canonicalized
{New().Add("content-tYPE", "application/json").Add("User-AGENT", "sling"), map[string][]string{"Content-Type": []string{"application/json"}, "User-Agent": []string{"sling"}}},
{New().Add("content-tYPE", "application/json").Add("User-AGENT", "sling"), map[string][]string{"Content-Type": {"application/json"}, "User-Agent": {"sling"}}},
// values for existing keys should be appended
{New().Add("A", "B").Add("a", "c"), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").Add("a", "c"), map[string][]string{"A": {"B", "c"}}},
// Add should add to values for keys added by parent Slings
{New().Add("A", "B").Add("a", "c").New(), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").New().Add("a", "c"), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").Add("a", "c").New(), map[string][]string{"A": {"B", "c"}}},
{New().Add("A", "B").New().Add("a", "c"), map[string][]string{"A": {"B", "c"}}},
}
for _, c := range cases {
// type conversion from header to alias'd map for deep equality comparison
Expand All @@ -246,11 +238,11 @@ func TestSetHeader(t *testing.T) {
expectedHeader map[string][]string
}{
// should replace existing values associated with key
{New().Add("A", "B").Set("a", "c"), map[string][]string{"A": []string{"c"}}},
{New().Set("content-type", "A").Set("Content-Type", "B"), map[string][]string{"Content-Type": []string{"B"}}},
{New().Add("A", "B").Set("a", "c"), map[string][]string{"A": {"c"}}},
{New().Set("content-type", "A").Set("Content-Type", "B"), map[string][]string{"Content-Type": {"B"}}},
// Set should replace values received by copying parent Slings
{New().Set("A", "B").Add("a", "c").New(), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").New().Set("a", "c"), map[string][]string{"A": []string{"c"}}},
{New().Set("A", "B").Add("a", "c").New(), map[string][]string{"A": {"B", "c"}}},
{New().Add("A", "B").New().Set("a", "c"), map[string][]string{"A": {"c"}}},
}
for _, c := range cases {
// type conversion from Header to alias'd map for deep equality comparison
Expand Down Expand Up @@ -555,20 +547,20 @@ func TestRequest_headers(t *testing.T) {
sling *Sling
expectedHeader map[string][]string
}{
{New().Add("authorization", "OAuth key=\"value\""), map[string][]string{"Authorization": []string{"OAuth key=\"value\""}}},
{New().Add("authorization", "OAuth key=\"value\""), map[string][]string{"Authorization": {"OAuth key=\"value\""}}},
// header keys should be canonicalized
{New().Add("content-tYPE", "application/json").Add("User-AGENT", "sling"), map[string][]string{"Content-Type": []string{"application/json"}, "User-Agent": []string{"sling"}}},
{New().Add("content-tYPE", "application/json").Add("User-AGENT", "sling"), map[string][]string{"Content-Type": {"application/json"}, "User-Agent": {"sling"}}},
// values for existing keys should be appended
{New().Add("A", "B").Add("a", "c"), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").Add("a", "c"), map[string][]string{"A": {"B", "c"}}},
// Add should add to values for keys added by parent Slings
{New().Add("A", "B").Add("a", "c").New(), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").New().Add("a", "c"), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").Add("a", "c").New(), map[string][]string{"A": {"B", "c"}}},
{New().Add("A", "B").New().Add("a", "c"), map[string][]string{"A": {"B", "c"}}},
// Add and Set
{New().Add("A", "B").Set("a", "c"), map[string][]string{"A": []string{"c"}}},
{New().Set("content-type", "A").Set("Content-Type", "B"), map[string][]string{"Content-Type": []string{"B"}}},
{New().Add("A", "B").Set("a", "c"), map[string][]string{"A": {"c"}}},
{New().Set("content-type", "A").Set("Content-Type", "B"), map[string][]string{"Content-Type": {"B"}}},
// Set should replace values received by copying parent Slings
{New().Set("A", "B").Add("a", "c").New(), map[string][]string{"A": []string{"B", "c"}}},
{New().Add("A", "B").New().Set("a", "c"), map[string][]string{"A": []string{"c"}}},
{New().Set("A", "B").Add("a", "c").New(), map[string][]string{"A": {"B", "c"}}},
{New().Add("A", "B").New().Set("a", "c"), map[string][]string{"A": {"c"}}},
}
for _, c := range cases {
req, _ := c.sling.Request()
Expand Down Expand Up @@ -640,7 +632,6 @@ func TestDo_onSuccess(t *testing.T) {
model := new(FakeModel)
apiError := new(APIError)
resp, err := sling.Do(req, model, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand Down Expand Up @@ -668,7 +659,6 @@ func TestDo_onSuccessWithNilValue(t *testing.T) {

apiError := new(APIError)
resp, err := sling.Do(req, nil, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand All @@ -694,7 +684,6 @@ func TestDo_noContent(t *testing.T) {
model := new(FakeModel)
apiError := new(APIError)
resp, err := sling.Do(req, model, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand Down Expand Up @@ -729,7 +718,6 @@ func TestDo_onFailure(t *testing.T) {
model := new(FakeModel)
apiError := new(APIError)
resp, err := sling.Do(req, model, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand Down Expand Up @@ -758,7 +746,6 @@ func TestDo_onFailureWithNilValue(t *testing.T) {

model := new(FakeModel)
resp, err := sling.Do(req, model, nil)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand All @@ -771,39 +758,70 @@ func TestDo_onFailureWithNilValue(t *testing.T) {
}
}

func TestByteSreamer_failed(t *testing.T) {
tests := map[string]struct {
resp *http.Response
v any
}{
"v is not io.Writer": {
&http.Response{
Body: io.NopCloser(strings.NewReader("some response text")),
ContentLength: -1,
},
&struct{}{},
},
"response body ready error": {
&http.Response{
Body: io.NopCloser(&mockReader{
ReadFn: func(b []byte) (int, error) { return 0, fmt.Errorf("some io error") },
}),
ContentLength: -1,
},
&bytes.Buffer{},
},
}

for name, test := range tests {
t.Run(name, func(t *testing.T) {
err := ByteStreamer{}.Decode(test.resp, test.v)
if err == nil {
t.Errorf("expected non nil error")
}
})
}
}

func TestReceive_success_nonDefaultDecoder(t *testing.T) {
client, mux, server := testServer()
defer server.Close()
mux.HandleFunc("/foo/submit", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/xml")
data := ` <response>
<text>Some text</text>
<favorite_count>24</favorite_count>
<temperature>10.5</temperature>
</response>`
fmt.Fprintf(w, xml.Header)
fmt.Fprint(w, data)

expectedData := []byte{11, 22, 33, 44, 55}

mux.HandleFunc("/foo/file", func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/octet-stream")
w.Write(expectedData)
})

endpoint := New().Client(client).Base("http://example.com/").Path("foo/").Post("submit")
endpoint := New().Client(client).Base("http://example.com/").Path("foo/").Get("file")

model := new(FakeModel)
apiError := new(APIError)
resp, err := endpoint.New().ResponseDecoder(xmlResponseDecoder{}).Receive(model, apiError)
success := &bytes.Buffer{}
failure := &bytes.Buffer{}

resp, err := endpoint.New().ResponseDecoder(ByteStreamer{}).Receive(success, failure)
if err != nil {
t.Errorf("expected nil, got %v", err)
}

if resp.StatusCode != 200 {
t.Errorf("expected %d, got %d", 200, resp.StatusCode)
t.Errorf("expected statu code: %d, got: %d", 200, resp.StatusCode)
}
expectedModel := &FakeModel{Text: "Some text", FavoriteCount: 24, Temperature: 10.5}
if !reflect.DeepEqual(expectedModel, model) {
t.Errorf("expected %v, got %v", expectedModel, model)

if failure.Len() != 0 {
t.Errorf("expected failure data to be empry; got: %v", failure.Bytes())
}
expectedAPIError := &APIError{}
if !reflect.DeepEqual(expectedAPIError, apiError) {
t.Errorf("failureV should be zero valued, exepcted %v, got %v", expectedAPIError, apiError)

if !reflect.DeepEqual(expectedData, success.Bytes()) {
t.Errorf("expected response data: %v; got: %v", expectedData, success.Bytes())
}
}

Expand All @@ -824,7 +842,6 @@ func TestReceive_success(t *testing.T) {
model := new(FakeModel)
apiError := new(APIError)
resp, err := endpoint.New().QueryStruct(params).BodyForm(params).Receive(model, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand Down Expand Up @@ -856,7 +873,6 @@ func TestReceive_StatusOKNoContent(t *testing.T) {
model := new(FakeModel)
apiError := new(APIError)
resp, err := endpoint.New().BodyForm(params).Receive(model, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand Down Expand Up @@ -891,7 +907,6 @@ func TestReceive_failure(t *testing.T) {
model := new(FakeModel)
apiError := new(APIError)
resp, err := endpoint.New().QueryStruct(params).BodyForm(params).Receive(model, apiError)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand All @@ -918,7 +933,6 @@ func TestReceive_noContent(t *testing.T) {

endpoint := New().Client(client).Base("http://example.com/").Path("foo/").Head("submit")
resp, err := endpoint.New().Receive(nil, nil)

if err != nil {
t.Errorf("expected nil, got %v", err)
}
Expand Down Expand Up @@ -1044,3 +1058,15 @@ func assertPostForm(t *testing.T, expected map[string]string, req *http.Request)
t.Errorf("expected parameters %v, got %v", expected, req.PostForm)
}
}

// mockReader implements [io.Reader] to create custom IO readers behavior in tests.
type mockReader struct {
ReadFn func([]byte) (int, error)
}

func (mr *mockReader) Read(p []byte) (n int, err error) {
if mr.ReadFn != nil {
return mr.ReadFn(p)
}
return 0, nil
}

0 comments on commit b285c20

Please sign in to comment.