Skip to content

Commit 7cadc27

Browse files
authored
Merge pull request #138 from deploymenttheory/dev
Refactored api response success handling and moved it from api handler to the response package
2 parents 06ddcb5 + da1e92c commit 7cadc27

File tree

14 files changed

+189
-164
lines changed

14 files changed

+189
-164
lines changed

Diff for: .azuredevops/PULL_REQUEST_TEMPLATE.md

+2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ Please delete options that are not relevant.
1111
- [ ] New feature (non-breaking change which adds functionality)
1212
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
1313
- [ ] This change requires a documentation update (Wiki)
14+
- [ ] Refactor (refactoring code, removing code, changing code structure)
15+
1416

1517
## Checklist
1618

Diff for: .azuredevops/pull_request_template/branches/dev.md

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Please delete options that are not relevant.
1010
- [ ] New feature (non-breaking change which adds functionality)
1111
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
1212
- [ ] This change requires a documentation update (Wiki)
13+
- [ ] Refactor (refactoring code, removing code, changing code structure)
1314

1415
## Checklist
1516

Diff for: .azuredevops/pull_request_template/branches/terraform.md

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Please delete options that are not relevant.
1010
- [ ] New feature (non-breaking change which adds functionality)
1111
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
1212
- [ ] This change requires a documentation update (Wiki)
13+
- [ ] Refactor (refactoring code, removing code, changing code structure)
1314

1415
## Checklist
1516

Diff for: .github/PULL_REQUEST_TEMPLATE.md

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ Please **DELETE** options that are not relevant.
1313
- [ ] New feature (non-breaking change which adds functionality)
1414
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
1515
- [ ] This change requires a documentation update (Wiki)
16+
- [ ] Refactor (refactoring code, removing code, changing code structure)
1617

1718
## Checklist
1819

Diff for: apiintegrations/apihandler/apihandler.go

+1-3
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
package apihandler
33

44
import (
5-
"net/http"
6-
75
"github.com/deploymenttheory/go-api-http-client/apiintegrations/jamfpro"
86
"github.com/deploymenttheory/go-api-http-client/apiintegrations/msgraph"
97
"github.com/deploymenttheory/go-api-http-client/logger"
@@ -17,7 +15,7 @@ type APIHandler interface {
1715
ConstructAPIAuthEndpoint(instanceName string, endpointPath string, log logger.Logger) string
1816
MarshalRequest(body interface{}, method string, endpoint string, log logger.Logger) ([]byte, error)
1917
MarshalMultipartRequest(fields map[string]string, files map[string]string, log logger.Logger) ([]byte, string, error)
20-
HandleAPISuccessResponse(resp *http.Response, out interface{}, log logger.Logger) error
18+
//HandleAPISuccessResponse(resp *http.Response, out interface{}, log logger.Logger) error
2119
GetContentTypeHeader(method string, log logger.Logger) string
2220
GetAcceptHeader() string
2321
GetDefaultBaseDomain() string

Diff for: go.mod

-2
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ module github.com/deploymenttheory/go-api-http-client
33
go 1.21
44

55
require (
6-
github.com/PuerkitoBio/goquery v1.9.1
76
github.com/antchfx/xmlquery v1.3.18
87
github.com/google/uuid v1.6.0
98
github.com/stretchr/testify v1.9.0
@@ -12,7 +11,6 @@ require (
1211
)
1312

1413
require (
15-
github.com/andybalholm/cascadia v1.3.2 // indirect
1614
github.com/antchfx/xpath v1.2.4 // indirect
1715
github.com/davecgh/go-spew v1.1.1 // indirect
1816
github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect

Diff for: go.sum

-12
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,3 @@
1-
github.com/PuerkitoBio/goquery v1.9.1 h1:mTL6XjbJTZdpfL+Gwl5U2h1l9yEkJjhmlTeV9VPW7UI=
2-
github.com/PuerkitoBio/goquery v1.9.1/go.mod h1:cW1n6TmIMDoORQU5IU/P1T3tGFunOeXEpGP2WHRwkbY=
3-
github.com/andybalholm/cascadia v1.3.2 h1:3Xi6Dw5lHF15JtdcmAHD3i1+T8plmv7BQ/nsViSLyss=
4-
github.com/andybalholm/cascadia v1.3.2/go.mod h1:7gtRlve5FxPPgIgX36uWBX58OdBsSS6lUvCFb+h7KvU=
51
github.com/antchfx/xmlquery v1.3.18 h1:FSQ3wMuphnPPGJOFhvc+cRQ2CT/rUj4cyQXkJcjOwz0=
62
github.com/antchfx/xmlquery v1.3.18/go.mod h1:Afkq4JIeXut75taLSuI31ISJ/zeq+3jG7TunF7noreA=
73
github.com/antchfx/xpath v1.2.4 h1:dW1HB/JxKvGtJ9WyVGJ0sIoEcqftV3SqIstujI+B9XY=
@@ -28,42 +24,34 @@ go.uber.org/zap v1.27.0/go.mod h1:GB2qFLM7cTU87MWRP2mPIjqfIDnGu+VIO4V/SdhGo2E=
2824
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
2925
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
3026
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
31-
golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
3227
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
3328
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
3429
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
35-
golang.org/x/net v0.6.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
3630
golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
37-
golang.org/x/net v0.9.0/go.mod h1:d48xBJpPfHeWQsugry2m+kC02ZBRGRgulfHnEXEuWns=
3831
golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc=
3932
golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg=
4033
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
4134
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
42-
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
4335
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
4436
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
4537
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
4638
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
4739
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
4840
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
49-
golang.org/x/sys v0.7.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
5041
golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4=
5142
golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
5243
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
5344
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
5445
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
55-
golang.org/x/term v0.7.0/go.mod h1:P32HKFT3hSsZrRxla30E9HqToFYAQPCMs/zFMBUFqPY=
5646
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
5747
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
5848
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
5949
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
60-
golang.org/x/text v0.9.0/go.mod h1:e1OnstbJyHTd6l/uOt8jFFHp6TRDWZR/bV3emEE/zU8=
6150
golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
6251
golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
6352
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
6453
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
6554
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
66-
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
6755
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
6856
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
6957
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=

Diff for: httpclient/multipartrequest.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,6 @@ func (c *Client) DoMultipartRequest(method, endpoint string, fields map[string]s
9090
return nil, response.HandleAPIErrorResponse(resp, log)
9191
} else {
9292
// Handle successful responses
93-
return resp, c.handleSuccessResponse(resp, out, log, method, endpoint)
93+
return resp, response.HandleAPISuccessResponse(resp, out, log)
9494
}
9595
}

Diff for: httpclient/request.go

+20-20
Original file line numberDiff line numberDiff line change
@@ -186,8 +186,8 @@ func (c *Client) executeRequestWithRetries(method, endpoint string, body, out in
186186
if resp.StatusCode >= 300 {
187187
log.Warn("Redirect response received", zap.Int("status_code", resp.StatusCode), zap.String("location", resp.Header.Get("Location")))
188188
}
189-
// Handle the response as successful, even if it's a redirect.
190-
return resp, c.handleSuccessResponse(resp, out, log, method, endpoint)
189+
// Handle the response as successful.
190+
return resp, response.HandleAPISuccessResponse(resp, out, log)
191191
}
192192

193193
// Leverage TranslateStatusCode for more descriptive error logging
@@ -348,11 +348,11 @@ func (c *Client) executeRequest(method, endpoint string, body, out interface{})
348348
if resp.StatusCode >= 300 {
349349
log.Warn("Redirect response received", zap.Int("status_code", resp.StatusCode), zap.String("location", resp.Header.Get("Location")))
350350
}
351-
return resp, c.handleSuccessResponse(resp, out, log, method, endpoint)
351+
return resp, response.HandleAPISuccessResponse(resp, out, log)
352+
352353
}
353354

354355
// Handle error responses for status codes outside the successful range
355-
//return nil, c.handleErrorResponse(resp, out, log, method, endpoint)
356356
return nil, response.HandleAPIErrorResponse(resp, log)
357357
}
358358

@@ -430,19 +430,19 @@ func (c *Client) do(req *http.Request, log logger.Logger, method, endpoint strin
430430
//
431431
// Returns:
432432
// - nil if the response was successfully unmarshalled into the 'out' parameter, or an error if unmarshalling failed.
433-
func (c *Client) handleSuccessResponse(resp *http.Response, out interface{}, log logger.Logger, method, endpoint string) error {
434-
if err := c.APIHandler.HandleAPISuccessResponse(resp, out, log); err != nil {
435-
log.Error("Failed to unmarshal HTTP response",
436-
zap.String("method", method),
437-
zap.String("endpoint", endpoint),
438-
zap.Error(err),
439-
)
440-
return err
441-
}
442-
log.Info("HTTP request succeeded",
443-
zap.String("method", method),
444-
zap.String("endpoint", endpoint),
445-
zap.Int("status_code", resp.StatusCode),
446-
)
447-
return nil
448-
}
433+
// func (c *Client) handleSuccessResponse(resp *http.Response, out interface{}, log logger.Logger, method, endpoint string) error {
434+
// if err := c.APIHandler.HandleAPISuccessResponse(resp, out, log); err != nil {
435+
// log.Error("Failed to unmarshal HTTP response",
436+
// zap.String("method", method),
437+
// zap.String("endpoint", endpoint),
438+
// zap.Error(err),
439+
// )
440+
// return err
441+
// }
442+
// log.Info("HTTP request succeeded",
443+
// zap.String("method", method),
444+
// zap.String("endpoint", endpoint),
445+
// zap.Int("status_code", resp.StatusCode),
446+
// )
447+
// return nil
448+
// }

Diff for: response/error.go

+15-22
Original file line numberDiff line numberDiff line change
@@ -77,20 +77,6 @@ func HandleAPIErrorResponse(resp *http.Response, log logger.Logger) *APIError {
7777
return apiError
7878
}
7979

80-
// ParseContentTypeHeader parses the Content-Type header and extracts the MIME type and parameters.
81-
func ParseContentTypeHeader(header string) (string, map[string]string) {
82-
parts := strings.Split(header, ";")
83-
mimeType := strings.TrimSpace(parts[0])
84-
params := make(map[string]string)
85-
for _, part := range parts[1:] {
86-
kv := strings.SplitN(part, "=", 2)
87-
if len(kv) == 2 {
88-
params[strings.TrimSpace(kv[0])] = strings.TrimSpace(kv[1])
89-
}
90-
}
91-
return mimeType, params
92-
}
93-
9480
// parseJSONResponse attempts to parse the JSON error response and update the APIError structure.
9581
func parseJSONResponse(bodyBytes []byte, apiError *APIError, log logger.Logger, resp *http.Response) {
9682
if err := json.Unmarshal(bodyBytes, apiError); err != nil {
@@ -185,33 +171,40 @@ func parseHTMLResponse(bodyBytes []byte, apiError *APIError, log logger.Logger,
185171
parse = func(n *html.Node) {
186172
if n.Type == html.ElementNode && n.Data == "p" {
187173
var pContent strings.Builder
188-
for c := n.FirstChild; c != nil; c = c.NextSibling {
174+
// Define a function to traverse child nodes of the <p> tag.
175+
var traverseChildren func(*html.Node)
176+
traverseChildren = func(c *html.Node) {
189177
if c.Type == html.TextNode {
190178
// Append text content directly.
191179
pContent.WriteString(strings.TrimSpace(c.Data) + " ")
192180
} else if c.Type == html.ElementNode && c.Data == "a" {
193181
// Extract href attribute value for links.
194-
var href string
195182
for _, attr := range c.Attr {
196183
if attr.Key == "href" {
197-
href = attr.Val
184+
// Append the link to the pContent builder.
185+
pContent.WriteString("[Link: " + attr.Val + "] ")
198186
break
199187
}
200188
}
201-
if href != "" {
202-
// Append the link to the pContent builder.
203-
pContent.WriteString(fmt.Sprintf("[Link: %s] ", href))
204-
}
205189
}
190+
// Recursively traverse all children of the current node.
191+
for child := c.FirstChild; child != nil; child = child.NextSibling {
192+
traverseChildren(child)
193+
}
194+
}
195+
// Start traversing child nodes of the current <p> tag.
196+
for child := n.FirstChild; child != nil; child = child.NextSibling {
197+
traverseChildren(child)
206198
}
207199
finalContent := strings.TrimSpace(pContent.String())
208200
if finalContent != "" {
209201
// Add the content of the <p> tag to messages.
210202
messages = append(messages, finalContent)
211203
}
212204
}
205+
// Continue traversing the document.
213206
for c := n.FirstChild; c != nil; c = c.NextSibling {
214-
parse(c) // Continue traversing the document.
207+
parse(c)
215208
}
216209
}
217210

Diff for: response/error_test.go.TODO

-104
This file was deleted.

Diff for: response/parse.go

+18
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// response/parse.go
2+
package response
3+
4+
import "strings"
5+
6+
// ParseContentTypeHeader parses the Content-Type header and extracts the MIME type and parameters.
7+
func ParseContentTypeHeader(header string) (string, map[string]string) {
8+
parts := strings.Split(header, ";")
9+
mimeType := strings.TrimSpace(parts[0])
10+
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])
15+
}
16+
}
17+
return mimeType, params
18+
}

0 commit comments

Comments
 (0)