Merge pull request #145 from deploymenttheory/dev

Refactor success handling and added tests

Author: ShocOne

Diff for: response/parse_test.go

@@ -0,0 +1,82 @@
+// response/parse_test.go
+package response
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseContentTypeHeader(t *testing.T) {
+	tests := []struct {
+		name       string
+		header     string
+		wantType   string
+		wantParams map[string]string
+	}{
+		{
+			name:       "Basic",
+			header:     "text/html; charset=UTF-8",
+			wantType:   "text/html",
+			wantParams: map[string]string{"charset": "UTF-8"},
+		},
+		{
+			name:       "No Params",
+			header:     "application/json",
+			wantType:   "application/json",
+			wantParams: map[string]string{},
+		},
+		{
+			name:       "Multiple Params",
+			header:     "multipart/form-data; boundary=something; charset=utf-8",
+			wantType:   "multipart/form-data",
+			wantParams: map[string]string{"boundary": "something", "charset": "utf-8"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotType, gotParams := ParseContentTypeHeader(tt.header)
+			if gotType != tt.wantType {
+				t.Errorf("ParseContentTypeHeader() gotType = %v, want %v", gotType, tt.wantType)
+			}
+			if !reflect.DeepEqual(gotParams, tt.wantParams) {
+				t.Errorf("ParseContentTypeHeader() gotParams = %v, want %v", gotParams, tt.wantParams)
+			}
+		})
+	}
+}
+
+func TestParseContentDisposition(t *testing.T) {
+	tests := []struct {
+		name       string
+		header     string
+		wantType   string
+		wantParams map[string]string
+	}{
+		{
+			name:       "Attachment with Filename",
+			header:     "attachment; filename=\"filename.jpg\"",
+			wantType:   "attachment",
+			wantParams: map[string]string{"filename": "filename.jpg"},
+		},
+		{
+			name:       "Inline",
+			header:     "inline",
+			wantType:   "inline",
+			wantParams: map[string]string{},
+		},
+		// Add more test cases as needed
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			gotType, gotParams := ParseContentDisposition(tt.header)
+			if gotType != tt.wantType {
+				t.Errorf("ParseContentDisposition() gotType = %v, want %v", gotType, tt.wantType)
+			}
+			if !reflect.DeepEqual(gotParams, tt.wantParams) {
+				t.Errorf("ParseContentDisposition() gotParams = %v, want %v", gotParams, tt.wantParams)
+			}
+		})
+	}
+}

Diff for: response/success.go

@@ -16,17 +16,17 @@ import (
 	"go.uber.org/zap"
 )
 
-// Refactored contentHandler to accept io.Reader instead of []byte for streaming support.
+// contentHandler defines the signature for unmarshaling content from an io.Reader.
 type contentHandler func(io.Reader, interface{}, logger.Logger, string) error
 
-// Updated handlers map to use the new contentHandler signature.
-var handlers = map[string]contentHandler{
+// responseUnmarshallers maps MIME types to the corresponding contentHandler functions.
+var responseUnmarshallers = map[string]contentHandler{
 	"application/json": unmarshalJSON,
 	"application/xml":  unmarshalXML,
 	"text/xml":         unmarshalXML,
 }
 
-// HandleAPISuccessResponse reads the response body and unmarshals it based on the content type.
+// HandleAPISuccessResponse reads the response body, logs the raw response details, and unmarshals the response based on the content type.
 func HandleAPISuccessResponse(resp *http.Response, out interface{}, log logger.Logger) error {
 	if resp.Request.Method == "DELETE" {
 		return handleDeleteRequest(resp, log)
@@ -38,7 +38,7 @@ func HandleAPISuccessResponse(resp *http.Response, out interface{}, log logger.L
 	mimeType, _ := ParseContentTypeHeader(resp.Header.Get("Content-Type"))
 	contentDisposition := resp.Header.Get("Content-Disposition")
 
-	if handler, ok := handlers[mimeType]; ok {
+	if handler, ok := responseUnmarshallers[mimeType]; ok {
 		// Pass resp.Body directly to the handler for streaming.
 		return handler(resp.Body, out, log, mimeType)
 	} else if isBinaryData(mimeType, contentDisposition) {
@@ -54,10 +54,15 @@ func HandleAPISuccessResponse(resp *http.Response, out interface{}, log logger.L
 // handleDeleteRequest handles the special case for DELETE requests, where a successful response might not contain a body.
 func handleDeleteRequest(resp *http.Response, log logger.Logger) error {
 	if resp.StatusCode >= 200 && resp.StatusCode < 300 {
-		log.Info("Successfully processed DELETE request", zap.String("URL", resp.Request.URL.String()), zap.Int("Status Code", resp.StatusCode))
+		if log != nil {
+			log.Info("Successfully processed DELETE request", zap.String("URL", resp.Request.URL.String()), zap.Int("Status Code", resp.StatusCode))
+		}
 		return nil
 	}
-	return log.Error("DELETE request failed", zap.String("URL", resp.Request.URL.String()), zap.Int("Status Code", resp.StatusCode))
+	if log != nil {
+		return log.Error("DELETE request failed", zap.String("URL", resp.Request.URL.String()), zap.Int("Status Code", resp.StatusCode))
+	}
+	return fmt.Errorf("DELETE request failed, status code: %d", resp.StatusCode)
 }
 
 // Adjusted logResponseDetails to handle a potential nil bodyBytes.

Diff for: response/success_test.go

@@ -0,0 +1,50 @@
+// response/success_test.go
+package response
+
+import (
+	"net/http"
+	"net/url"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestHandleDeleteRequest_Success(t *testing.T) {
+	resp := &http.Response{
+		StatusCode: http.StatusOK, // Simulate a successful DELETE request
+		Request: &http.Request{
+			Method: "DELETE",
+			URL: &url.URL{
+				Scheme: "http",
+				Host:   "example.com",
+				Path:   "/test",
+			},
+		},
+	}
+
+	// Call handleDeleteRequest with a nil logger since logging is ignored in this test
+	err := handleDeleteRequest(resp, nil)
+
+	// No error is expected for a successful DELETE request
+	assert.NoError(t, err, "handleDeleteRequest should not return an error for successful DELETE requests")
+}
+
+func TestHandleDeleteRequest_Failure(t *testing.T) {
+	resp := &http.Response{
+		StatusCode: http.StatusBadRequest, // Simulate a failed DELETE request
+		Request: &http.Request{
+			Method: "DELETE",
+			URL: &url.URL{
+				Scheme: "http",
+				Host:   "example.com",
+				Path:   "/test",
+			},
+		},
+	}
+
+	// Call handleDeleteRequest with a nil logger since logging is ignored in this test
+	err := handleDeleteRequest(resp, nil)
+
+	// An error is expected for a failed DELETE request
+	assert.Error(t, err, "handleDeleteRequest should return an error for failed DELETE requests")
+}