fix: support expected (but empty) response body (#111)

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 <[email protected]>
IBM · Apr 27, 2021 · 2f857c2 · 2f857c2
1 parent 689eaf7
commit 2f857c2
Show file tree

Hide file tree

Showing 4 changed files with 331 additions and 29 deletions.
diff --git a/.travis.yml b/.travis.yml
@@ -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:

diff --git a/v5/core/base_service.go b/v5/core/base_service.go
@@ -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()
@@ -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" {

diff --git a/v5/core/base_service_test.go b/v5/core/base_service_test.go
@@ -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":[