Skip to content

Commit

Permalink
fix: support expected (but empty) response body (#111)
Browse files Browse the repository at this point in the history
This commit modifies the BaseService.Request() method so that a "missing" response body 
will result in a nil "result" being returned to the caller (generated operation code).
In this context, "missing" implies that an operation is expected to have a non-empty response body
but the server simply didn't return anything in the body of the HTTP response message.
This could be a valid scenario in which a server might return one of several response status codes
for an operation, some that include a response body and some that don't.

Previously, if a response body was expected (i.e. the Request() method's `result` parameter 
was passed in as a non-nil value), an unmarshalling error ("EOF") would occur.
This new behavior is aligned with the Java core where a null "result" object would be returned
in this same type of scenario.

Co-authored-by: Mike Kistler <mkistler@us.ibm.com>
  • Loading branch information
padamstx and Mike Kistler authored Apr 27, 2021
1 parent 689eaf7 commit 2f857c2
Show file tree
Hide file tree
Showing 4 changed files with 331 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ before_install:
- sudo apt-get install python

install:
- curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.39.0
- curl -sfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh| sh -s -- -b $(go env GOPATH)/bin v1.37.0
- curl -s https://codecov.io/bash > $HOME/codecov-bash.sh && chmod +x $HOME/codecov-bash.sh

script:
Expand Down
40 changes: 18 additions & 22 deletions v5/core/base_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,7 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
rResult := reflect.ValueOf(result).Elem()
rResult.Set(reflect.ValueOf(httpResponse.Body))
detailedResponse.Result = httpResponse.Body
} else if IsJSONMimeType(contentType) {
// If the content-type indicates JSON, then unmarshal the response body as JSON.
} else {

// First, read the response body into a byte array.
defer httpResponse.Body.Close()
Expand All @@ -448,32 +447,29 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
return
}

// Decode the byte array as JSON.
decodeErr := json.NewDecoder(bytes.NewReader(responseBody)).Decode(result)
if decodeErr != nil {
// Error decoding the response body.
// Return the response body in RawResult, along with an error.
err = fmt.Errorf(ERRORMSG_UNMARSHAL_RESPONSE_BODY, decodeErr.Error())
detailedResponse.RawResult = responseBody
// If the response body is empty, then skip any attempt to deserialize and just return
if len(responseBody) == 0 {
return
}

// Decode step was successful. Return the decoded response object in the Result field.
detailedResponse.Result = reflect.ValueOf(result).Elem().Interface()
return
} else {
// For any other type of response (i.e. it's not JSON and the caller didn't pass in a io.ReadCloser)
// then read the response body bytes into a []byte so that we don't have an open io.ReadCloser hanging
// around.
defer httpResponse.Body.Close()
responseBody, readErr := ioutil.ReadAll(httpResponse.Body)
if readErr != nil {
err = fmt.Errorf(ERRORMSG_READ_RESPONSE_BODY, readErr.Error())
// If the content-type indicates JSON, then unmarshal the response body as JSON.
if IsJSONMimeType(contentType) {
// Decode the byte array as JSON.
decodeErr := json.NewDecoder(bytes.NewReader(responseBody)).Decode(result)
if decodeErr != nil {
// Error decoding the response body.
// Return the response body in RawResult, along with an error.
err = fmt.Errorf(ERRORMSG_UNMARSHAL_RESPONSE_BODY, decodeErr.Error())
detailedResponse.RawResult = responseBody
return
}

// Decode step was successful. Return the decoded response object in the Result field.
detailedResponse.Result = reflect.ValueOf(result).Elem().Interface()
return
}

// After reading the response body into a []byte, check to see if the caller wanted the
// response body as a string.
// Check to see if the caller wanted the response body as a string.
// If the caller passed in 'result' as the address of *string,
// then we'll reflectively set result to point to it.
if reflect.TypeOf(result).String() == "**string" {
Expand Down
253 changes: 253 additions & 0 deletions v5/core/base_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,259 @@ func TestRequestGoodResponseNoBody(t *testing.T) {
assert.Nil(t, detailedResponse.RawResult)
}

// Test a good response with unexpected response body.
func TestRequestGoodResponseUnexpectedBody(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", "application/json")
w.WriteHeader(http.StatusCreated)
fmt.Fprint(w, "{}")
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

detailedResponse, err := service.Request(req, nil)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.Nil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Test request with result that receives a good response with no response body.
func TestRequestWithResultGoodResponseNoBody(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(http.StatusCreated)
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

var rawResult map[string]json.RawMessage
detailedResponse, err := service.Request(req, &rawResult)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.Equal(t, "", detailedResponse.Headers.Get("Content-Type"))
assert.Nil(t, rawResult)
assert.Nil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Test request with result that receives a good response with no response body and JSON content-type.
func TestRequestWithResultGoodResponseNoBodyJSONObject(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", "application/json")
w.WriteHeader(http.StatusCreated)
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

var rawResult map[string]json.RawMessage
detailedResponse, err := service.Request(req, &rawResult)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.Nil(t, rawResult)
assert.Nil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Test request with result that receives a good response with no response body and JSON content-type.
func TestRequestWithResultGoodResponseNoBodyJSONArray(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", "application/json")
w.WriteHeader(http.StatusCreated)
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

var rawSlice []json.RawMessage
detailedResponse, err := service.Request(req, &rawSlice)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.Nil(t, rawSlice)
assert.Nil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Test request with result that receives a good response with no response body and JSON content-type.
func TestRequestWithResultGoodResponseNoBodyString(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", "text/plain")
w.WriteHeader(http.StatusCreated)
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

var result *string
detailedResponse, err := service.Request(req, &result)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.Nil(t, result)
assert.Nil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Test request with result that receives a good response with an empty object body.
func TestRequestWithResultGoodResponseEmptyObjectBody(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", "application/json")
w.WriteHeader(http.StatusCreated)
fmt.Fprint(w, "{}")
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

var rawResult map[string]json.RawMessage
detailedResponse, err := service.Request(req, &rawResult)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.NotNil(t, rawResult)
assert.NotNil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Test request with result that receives a good response with an empty array body and JSON content-type.
func TestRequestGoodResponseEmptyArrayBody(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-type", "application/json")
w.WriteHeader(http.StatusCreated)
fmt.Fprint(w, "[]")
}))
defer server.Close()

builder := NewRequestBuilder("GET")
_, err := builder.ResolveRequestURL(server.URL, "", nil)
assert.Nil(t, err)
req, _ := builder.Build()

authenticator, err := NewBasicAuthenticator("xxx", "yyy")
assert.Nil(t, err)
assert.NotNil(t, authenticator)

options := &ServiceOptions{
URL: server.URL,
Authenticator: authenticator,
}
service, err := NewBaseService(options)
assert.Nil(t, err)
assert.NotNil(t, service.Options.Authenticator)
assert.Equal(t, AUTHTYPE_BASIC, service.Options.Authenticator.AuthenticationType())

var rawSlice []json.RawMessage
detailedResponse, err := service.Request(req, &rawSlice)
assert.Nil(t, err)
assert.NotNil(t, detailedResponse)
assert.Equal(t, 201, detailedResponse.StatusCode)
assert.NotNil(t, rawSlice)
assert.NotNil(t, detailedResponse.Result)
assert.Nil(t, detailedResponse.RawResult)
}

// Example of a JSON error structure.
var jsonErrorResponse string = `{
"errors":[
Expand Down
Loading

0 comments on commit 2f857c2

Please sign in to comment.