feat: Allow WithJSONResponse() and WithXMLResponse() to work with…

… empty response bodies (#163) * Allow WithJSONResponse() to work with empty response bodies * Allow WithXMLResponse() to work with empty response bodies * fix lint
ConductorOne · Jun 26, 2024 · 5aba0b6 · 5aba0b6
1 parent a0c1f11
commit 5aba0b6
Show file tree

Hide file tree

Showing 2 changed files with 119 additions and 34 deletions.
diff --git a/pkg/uhttp/wrapper.go b/pkg/uhttp/wrapper.go
@@ -45,11 +45,17 @@ func NewBaseHttpClient(httpClient *http.Client) *BaseHttpClient {
 	}
 }
 
+// WithJSONResponse is a wrapper that marshals the returned response body into
+// the provided shape. If the API should return an empty JSON body (i.e. HTTP
+// status code 204 No Content), then pass a `nil` to `response`.
 func WithJSONResponse(response interface{}) DoOption {
 	return func(resp *WrapperResponse) error {
 		if !helpers.IsJSONContentType(resp.Header.Get(ContentType)) {
 			return fmt.Errorf("unexpected content type for json response: %s", resp.Header.Get(ContentType))
 		}
+		if response == nil && len(resp.Body) == 0 {
+			return nil
+		}
 		return json.Unmarshal(resp.Body, response)
 	}
 }
@@ -101,6 +107,9 @@ func WithXMLResponse(response interface{}) DoOption {
 		if !helpers.IsXMLContentType(resp.Header.Get(ContentType)) {
 			return fmt.Errorf("unexpected content type for xml response: %s", resp.Header.Get(ContentType))
 		}
+		if response == nil && len(resp.Body) == 0 {
+			return nil
+		}
 		return xml.Unmarshal(resp.Body, response)
 	}
 }

diff --git a/pkg/uhttp/wrapper_test.go b/pkg/uhttp/wrapper_test.go
@@ -87,33 +87,77 @@ func TestWrapper_WithJSONResponse(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	resp := http.Response{
-		Header: map[string][]string{
-			"Content-Type": {"application/json"},
-		},
-	}
-
 	responseBody := example{}
 	option := WithJSONResponse(&responseBody)
-	wrapperResp := WrapperResponse{
-		Header:     resp.Header,
-		Body:       exampleResponseBuffer.Bytes(),
-		StatusCode: 200,
-		Status:     "200 OK",
-	}
-	err = option(&wrapperResp)
 
-	require.Nil(t, err)
-	require.Equal(t, exampleResponse, responseBody)
+	t.Run("should marshal a JSON response", func(t *testing.T) {
+		resp := http.Response{
+			Header: map[string][]string{
+				"Content-Type": {"application/json"},
+			},
+		}
+
+		wrapperResp := WrapperResponse{
+			Header:     resp.Header,
+			Body:       exampleResponseBuffer.Bytes(),
+			StatusCode: 200,
+			Status:     "200 OK",
+		}
+		err = option(&wrapperResp)
+
+		require.Nil(t, err)
+		require.Equal(t, exampleResponse, responseBody)
+	})
+
+	t.Run("should return an error when the response is not JSON", func(t *testing.T) {
+		resp := http.Response{
+			Header: map[string][]string{
+				"Content-Type": {"application/xml"},
+			},
+		}
+
+		wrapperResp := WrapperResponse{
+			Header:     resp.Header,
+			Body:       exampleResponseBuffer.Bytes(),
+			StatusCode: 200,
+			Status:     "200 OK",
+		}
+		err = option(&wrapperResp)
+
+		require.NotNil(t, err)
+	})
+
+	t.Run("should not marshal when the JSON response is empty (HTTP 204)", func(t *testing.T) {
+		wrapperResp := WrapperResponse{
+			Header: map[string][]string{
+				"Content-Type": {"application/json"},
+			},
+			Body:       []byte(""),
+			StatusCode: 204,
+			Status:     "204 No Content",
+		}
+		err = WithJSONResponse(nil)(&wrapperResp)
 
-	wrapperResp.Header = map[string][]string{
-		"Content-Type": {"application/xml"},
-	}
-	responseBody = example{}
-	option = WithJSONResponse(&responseBody)
-	err = option(&wrapperResp)
+		require.Nil(t, err)
+	})
 
-	require.NotNil(t, err)
+	t.Run("should marshal an empty JSON response (\"{}\")", func(t *testing.T) {
+		type empty struct{}
+
+		wrapperResp := WrapperResponse{
+			Header: map[string][]string{
+				"Content-Type": {"application/json"},
+			},
+			Body:       []byte("{}"),
+			StatusCode: 204,
+			Status:     "204 No Content",
+		}
+		responseBody := empty{}
+		err = WithJSONResponse(&responseBody)(&wrapperResp)
+
+		require.Nil(t, err)
+		require.Equal(t, empty{}, responseBody)
+	})
 }
 
 func TestWrapper_WithXMLResponse(t *testing.T) {
@@ -133,18 +177,50 @@ func TestWrapper_WithXMLResponse(t *testing.T) {
 		},
 	}
 
-	responseBody := example{}
-	option := WithXMLResponse(&responseBody)
-	wrapperResp := WrapperResponse{
-		Header:     resp.Header,
-		Body:       exampleResponseBuffer.Bytes(),
-		StatusCode: 200,
-		Status:     "200 OK",
-	}
-	err = option(&wrapperResp)
-
-	require.Nil(t, err)
-	require.Equal(t, exampleResponse, responseBody)
+	t.Run("should marshal an XML response", func(t *testing.T) {
+		responseBody := example{}
+		option := WithXMLResponse(&responseBody)
+		wrapperResp := WrapperResponse{
+			Header:     resp.Header,
+			Body:       exampleResponseBuffer.Bytes(),
+			StatusCode: 200,
+			Status:     "200 OK",
+		}
+		err = option(&wrapperResp)
+
+		require.Nil(t, err)
+		require.Equal(t, exampleResponse, responseBody)
+	})
+
+	t.Run("should return an error when the response is not XML", func(t *testing.T) {
+		responseBody := example{}
+		option := WithXMLResponse(&responseBody)
+		wrapperResp := WrapperResponse{
+			Header: map[string][]string{
+				"Content-Type": {"application/json"},
+			},
+			Body:       exampleResponseBuffer.Bytes(),
+			StatusCode: 200,
+			Status:     "200 OK",
+		}
+		err = option(&wrapperResp)
+
+		require.NotNil(t, err)
+	})
+
+	t.Run("should not marshal when the XML response is empty (HTTP 204)", func(t *testing.T) {
+		wrapperResp := WrapperResponse{
+			Header: map[string][]string{
+				"Content-Type": {"application/xml"},
+			},
+			Body:       []byte(""),
+			StatusCode: 204,
+			Status:     "204 No Content",
+		}
+		err = WithXMLResponse(nil)(&wrapperResp)
+
+		require.Nil(t, err)
+	})
 }
 
 func TestWrapper_WithResponse(t *testing.T) {