Skip to content

Commit

Permalink
httpcli: Content-Length as obj type indicator
Browse files Browse the repository at this point in the history
For higher level clients using ResponseClassAuto (e.g. scheduler
callers) infer the response object type based on the Content-Length
header in the HTTP response. Streaming responses using recordio will not
have sent a Content-Length header, whereas "singleton" JSON object
responses will have one.
  • Loading branch information
James DeFelice authored and jdef committed Dec 7, 2018
1 parent 10c19a3 commit c333af7
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 12 deletions.
14 changes: 11 additions & 3 deletions api/v1/lib/httpcli/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -299,13 +299,21 @@ func validateSuccessfulResponse(codec encoding.Codec, res *http.Response, rc cli
return nil
}

func newSourceFactory(rc client.ResponseClass) encoding.SourceFactoryFunc {
func newSourceFactory(rc client.ResponseClass, knownLen bool) encoding.SourceFactoryFunc {
switch rc {
case client.ResponseClassNoData:
return nil
case client.ResponseClassSingleton:
return encoding.SourceReader
case client.ResponseClassStreaming, client.ResponseClassAuto:
case client.ResponseClassStreaming:
return recordIOSourceFactory
case client.ResponseClassAuto:
// Some operations return the same content-type header (json) but actually
// return different content types (recordio vs singleton json obj).
// Check the content-length header to distinguish between these cases.
if knownLen {
return encoding.SourceReader
}
return recordIOSourceFactory
default:
panic(fmt.Sprintf("unsupported response-class: %q", rc))
Expand Down Expand Up @@ -344,7 +352,7 @@ func (c *Client) HandleResponse(res *http.Response, rc client.ResponseClass, err
case http.StatusOK:
debug.Log("request OK, decoding response")

sf := newSourceFactory(rc)
sf := newSourceFactory(rc, res.ContentLength > -1)
if sf == nil {
if rc != client.ResponseClassNoData {
panic("nil Source for response that expected data")
Expand Down
20 changes: 11 additions & 9 deletions api/v1/lib/httpcli/http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,25 @@ func TestPrepareForResponse(t *testing.T) {

func TestNewSourceFactory(t *testing.T) {
for ti, tc := range []struct {
rc client.ResponseClass
wantsNil bool
wantsPanic bool
rc client.ResponseClass
knownLength bool
wantsNil bool
wantsPanic bool
}{
{client.ResponseClass(-1), false, true},
{client.ResponseClassNoData, true, false},
{client.ResponseClassAuto, false, false},
{client.ResponseClassSingleton, false, false},
{client.ResponseClassStreaming, false, false},
{client.ResponseClass(-1), false, false, true},
{client.ResponseClassNoData, false, true, false},
{client.ResponseClassAuto, false, false, false},
{client.ResponseClassAuto, true, false, false},
{client.ResponseClassSingleton, false, false, false},
{client.ResponseClassStreaming, false, false, false},
} {
f := func() encoding.SourceFactoryFunc {
defer func() {
if x := recover(); (x != nil) != tc.wantsPanic {
panic(x)
}
}()
return newSourceFactory(tc.rc)
return newSourceFactory(tc.rc, tc.knownLength)
}()
if tc.wantsPanic {
continue
Expand Down

0 comments on commit c333af7

Please sign in to comment.