Skip to content

Commit 3063149

Browse files
ShocOneShocOne
authored
Merge pull request #144 from deploymenttheory/dev
Refactored handling of success response
2 parents 632cb54 + 828c770 commit 3063149

File tree

2 files changed

+106
-84
lines changed

2 files changed

+106
-84
lines changed

‎response/parse.go‎

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,31 @@ package response
33

44
import "strings"
55

6-
// ParseContentTypeHeader parses the Content-Type header and extracts the MIME type and parameters.
6+
// ParseContentTypeHeader parses the Content-Type header and returns the MIME type and any parameters.
77
func ParseContentTypeHeader(header string) (string, map[string]string) {
8-
parts := strings.Split(header, ";")
9-
mimeType := strings.TrimSpace(parts[0])
8+
return parseHeader(header)
9+
}
10+
11+
// ParseContentDisposition parses the Content-Disposition header and returns the type and any parameters.
12+
func ParseContentDisposition(header string) (string, map[string]string) {
13+
return parseHeader(header)
14+
}
15+
16+
// parseHeader generalizes the parsing of headers like Content-Type and Content-Disposition.
17+
// It extracts the main value (e.g., MIME type for Content-Type) and any parameters (like charset).
18+
func parseHeader(header string) (string, map[string]string) {
19+
parts := strings.SplitN(header, ";", 2) // Split into main value and parameters
20+
mainValue := strings.TrimSpace(parts[0])
21+
1022
params := make(map[string]string)
11-
for _, part := range parts[1:] {
12-
kv := strings.SplitN(part, "=", 2)
13-
if len(kv) == 2 {
14-
params[strings.TrimSpace(kv[0])] = strings.TrimSpace(kv[1])
23+
if len(parts) > 1 { // Check if there are parameters
24+
for _, part := range strings.Split(parts[1], ";") {
25+
kv := strings.SplitN(part, "=", 2)
26+
if len(kv) == 2 {
27+
params[strings.TrimSpace(kv[0])] = strings.Trim(strings.TrimSpace(kv[1]), "\"")
28+
}
1529
}
1630
}
17-
return mimeType, params
31+
32+
return mainValue, params
1833
}

‎response/success.go‎

Lines changed: 83 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package response
66
import (
77
"encoding/json"
88
"encoding/xml"
9+
"errors"
910
"fmt"
1011
"io"
1112
"net/http"
@@ -15,32 +16,39 @@ import (
1516
"go.uber.org/zap"
1617
)
1718

18-
// HandleAPISuccessResponse handles the HTTP success response from an API and unmarshals the response body into the provided output struct.
19+
// Refactored contentHandler to accept io.Reader instead of []byte for streaming support.
20+
type contentHandler func(io.Reader, interface{}, logger.Logger, string) error
21+
22+
// Updated handlers map to use the new contentHandler signature.
23+
var handlers = map[string]contentHandler{
24+
"application/json": unmarshalJSON,
25+
"application/xml": unmarshalXML,
26+
"text/xml": unmarshalXML,
27+
}
28+
29+
// HandleAPISuccessResponse reads the response body and unmarshals it based on the content type.
1930
func HandleAPISuccessResponse(resp *http.Response, out interface{}, log logger.Logger) error {
20-
// Special handling for DELETE requests
2131
if resp.Request.Method == "DELETE" {
2232
return handleDeleteRequest(resp, log)
2333
}
2434

25-
// Read the response body
26-
bodyBytes, err := readResponseBody(resp, log)
27-
if err != nil {
28-
return err
29-
}
30-
31-
// Log the raw response details for debugging
32-
logResponseDetails(resp, bodyBytes, log)
33-
34-
// Unmarshal the response based on content type
35-
contentType := resp.Header.Get("Content-Type")
35+
// No need to read the entire body into memory, pass resp.Body directly.
36+
logResponseDetails(resp, nil, log) // Updated to handle nil bodyBytes.
3637

37-
// Check for binary data handling
38+
mimeType, _ := ParseContentTypeHeader(resp.Header.Get("Content-Type"))
3839
contentDisposition := resp.Header.Get("Content-Disposition")
39-
if err := handleBinaryData(contentType, contentDisposition, bodyBytes, log, out); err != nil {
40-
return err
41-
}
4240

43-
return unmarshalResponse(contentType, bodyBytes, log, out)
41+
if handler, ok := handlers[mimeType]; ok {
42+
// Pass resp.Body directly to the handler for streaming.
43+
return handler(resp.Body, out, log, mimeType)
44+
} else if isBinaryData(mimeType, contentDisposition) {
45+
// For binary data, we still need to handle the body directly.
46+
return handleBinaryData(resp.Body, log, out, mimeType, contentDisposition)
47+
} else {
48+
errMsg := fmt.Sprintf("unexpected MIME type: %s", mimeType)
49+
log.Error("Unmarshal error", zap.String("content type", mimeType), zap.Error(errors.New(errMsg)))
50+
return errors.New(errMsg)
51+
}
4452
}
4553

4654
// handleDeleteRequest handles the special case for DELETE requests, where a successful response might not contain a body.
@@ -52,79 +60,78 @@ func handleDeleteRequest(resp *http.Response, log logger.Logger) error {
5260
return log.Error("DELETE request failed", zap.String("URL", resp.Request.URL.String()), zap.Int("Status Code", resp.StatusCode))
5361
}
5462

55-
// readResponseBody reads and returns the body of an HTTP response. It logs an error if reading fails.
56-
func readResponseBody(resp *http.Response, log logger.Logger) ([]byte, error) {
57-
// Read the response body
58-
bodyBytes, err := io.ReadAll(resp.Body)
59-
if err != nil {
60-
log.Error("Failed reading response body", zap.Error(err))
61-
return nil, err
63+
// Adjusted logResponseDetails to handle a potential nil bodyBytes.
64+
func logResponseDetails(resp *http.Response, bodyBytes []byte, log logger.Logger) {
65+
// Conditional logging if bodyBytes is not nil.
66+
if bodyBytes != nil {
67+
log.Debug("Raw HTTP Response", zap.String("Body", string(bodyBytes)))
6268
}
63-
return bodyBytes, nil
69+
// Logging headers remains unchanged.
70+
log.Debug("HTTP Response Headers", zap.Any("Headers", resp.Header))
6471
}
6572

66-
// logResponseDetails logs the raw HTTP response body and headers for debugging purposes.
67-
func logResponseDetails(resp *http.Response, bodyBytes []byte, log logger.Logger) {
68-
// Log the response body as a string
69-
log.Debug("Raw HTTP Response", zap.String("Body", string(bodyBytes)))
70-
// Log the response headers
71-
log.Debug("HTTP Response Headers", zap.Any("Headers", resp.Header))
73+
// unmarshalJSON unmarshals JSON content from an io.Reader into the provided output structure.
74+
func unmarshalJSON(reader io.Reader, out interface{}, log logger.Logger, mimeType string) error {
75+
decoder := json.NewDecoder(reader)
76+
if err := decoder.Decode(out); err != nil {
77+
log.Error("JSON Unmarshal error", zap.Error(err))
78+
return err
79+
}
80+
log.Info("Successfully unmarshalled JSON response", zap.String("content type", mimeType))
81+
return nil
7282
}
7383

74-
// handleBinaryData checks if the response should be treated as binary data based on the Content-Type or Content-Disposition headers. It assigns the response body to 'out' if 'out' is of type *[]byte.
75-
func handleBinaryData(contentType, contentDisposition string, bodyBytes []byte, log logger.Logger, out interface{}) error {
76-
// Check if response is binary data either by Content-Type or Content-Disposition
77-
if strings.Contains(contentType, "application/octet-stream") || strings.HasPrefix(contentDisposition, "attachment") {
78-
// Assert that 'out' is of the correct type to receive binary data
79-
if outPointer, ok := out.(*[]byte); ok {
80-
*outPointer = bodyBytes // Assign the response body to 'out'
81-
log.Debug("Handled binary data", // Log handling of binary data
82-
zap.String("Content-Type", contentType),
83-
zap.String("Content-Disposition", contentDisposition),
84-
)
85-
return nil
86-
} else {
87-
errMsg := "output parameter is not a *[]byte for binary data"
88-
log.Error("Binary data handling error", // Log error for incorrect 'out' type
89-
zap.String("error", errMsg),
90-
zap.String("Content-Type", contentType),
91-
zap.String("Content-Disposition", contentDisposition),
92-
)
93-
return fmt.Errorf(errMsg)
94-
}
84+
// unmarshalXML unmarshals XML content from an io.Reader into the provided output structure.
85+
func unmarshalXML(reader io.Reader, out interface{}, log logger.Logger, mimeType string) error {
86+
decoder := xml.NewDecoder(reader)
87+
if err := decoder.Decode(out); err != nil {
88+
log.Error("XML Unmarshal error", zap.Error(err))
89+
return err
9590
}
96-
return nil // If not binary data, no action needed
91+
log.Info("Successfully unmarshalled XML response", zap.String("content type", mimeType))
92+
return nil
9793
}
9894

99-
// unmarshalResponse unmarshals the response body into the provided output structure based on the MIME
100-
// type extracted from the Content-Type header.
101-
func unmarshalResponse(contentTypeHeader string, bodyBytes []byte, log logger.Logger, out interface{}) error {
102-
// Extract MIME type from Content-Type header
103-
mimeType, _ := ParseContentTypeHeader(contentTypeHeader)
104-
105-
// Determine the MIME type and unmarshal accordingly
106-
switch {
107-
case strings.Contains(mimeType, "application/json"):
108-
// Unmarshal JSON content
109-
if err := json.Unmarshal(bodyBytes, out); err != nil {
110-
log.Error("JSON Unmarshal error", zap.Error(err))
95+
// isBinaryData checks if the MIME type or Content-Disposition indicates binary data.
96+
func isBinaryData(contentType, contentDisposition string) bool {
97+
return strings.Contains(contentType, "application/octet-stream") || strings.HasPrefix(contentDisposition, "attachment")
98+
}
99+
100+
// handleBinaryData reads binary data from an io.Reader and stores it in *[]byte or streams it to an io.Writer.
101+
func handleBinaryData(reader io.Reader, log logger.Logger, out interface{}, mimeType, contentDisposition string) error {
102+
// Check if the output interface is either *[]byte or io.Writer
103+
switch out := out.(type) {
104+
case *[]byte:
105+
// Read all data from reader and store it in *[]byte
106+
data, err := io.ReadAll(reader)
107+
if err != nil {
108+
log.Error("Failed to read binary data", zap.Error(err))
111109
return err
112110
}
113-
log.Info("Successfully unmarshalled JSON response", zap.String("content type", mimeType))
111+
*out = data
114112

115-
case strings.Contains(mimeType, "application/xml") || strings.Contains(mimeType, "text/xml"):
116-
// Unmarshal XML content
117-
if err := xml.Unmarshal(bodyBytes, out); err != nil {
118-
log.Error("XML Unmarshal error", zap.Error(err))
113+
case io.Writer:
114+
// Stream data directly to the io.Writer
115+
_, err := io.Copy(out, reader)
116+
if err != nil {
117+
log.Error("Failed to stream binary data to io.Writer", zap.Error(err))
119118
return err
120119
}
121-
log.Info("Successfully unmarshalled XML response", zap.String("content type", mimeType))
122120

123121
default:
124-
// Log and return an error for unexpected MIME types
125-
errMsg := fmt.Sprintf("unexpected MIME type: %s", mimeType)
126-
log.Error("Unmarshal error", zap.String("content type", mimeType), zap.Error(fmt.Errorf(errMsg)))
127-
return fmt.Errorf(errMsg)
122+
errMsg := "output parameter is not suitable for binary data (*[]byte or io.Writer)"
123+
log.Error(errMsg, zap.String("Content-Type", mimeType))
124+
return errors.New(errMsg)
128125
}
126+
127+
// Handle Content-Disposition if present
128+
if contentDisposition != "" {
129+
_, params := ParseContentDisposition(contentDisposition)
130+
if filename, ok := params["filename"]; ok {
131+
log.Debug("Extracted filename from Content-Disposition", zap.String("filename", filename))
132+
// Additional processing for the filename can be done here if needed
133+
}
134+
}
135+
129136
return nil
130137
}

0 commit comments

Comments
 (0)