Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor!: remove response bytes method in-favor Body field, cleanup, apply appropriate naming struct, methods #871

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ const (
)

var (
ErrNotHttpTransportType = errors.New("resty: not a http.Transport type")
ErrUnsupportedRequestBodyKind = errors.New("resty: unsupported request body kind")

hdrUserAgentKey = http.CanonicalHeaderKey("User-Agent")
hdrAcceptKey = http.CanonicalHeaderKey("Accept")
hdrAcceptEncodingKey = http.CanonicalHeaderKey("Accept-Encoding")
Expand Down Expand Up @@ -1383,8 +1386,6 @@ func (c *Client) SetRateLimiter(rl RateLimiter) *Client {
return c
}

var ErrNotHttpTransportType = errors.New("resty: not a http.Transport type")

// Transport method returns [http.Transport] currently in use or error
// in case the currently used `transport` is not a [http.Transport].
//
Expand Down Expand Up @@ -1816,9 +1817,9 @@ func (c *Client) execute(req *Request) (*Response, error) {
response.wrapLimitReadCloser()
}
if !req.DoNotParseResponse && (req.Debug || req.ResponseBodyUnlimitedReads) {
response.wrapReadCopier()
response.wrapCopyReadCloser()

if err := response.readAllBytes(); err != nil {
if err := response.readAll(); err != nil {
return response, err
}
}
Expand Down
34 changes: 26 additions & 8 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ func TestClientRedirectPolicy(t *testing.T) {

c.SetRedirectPolicy(NoRedirectPolicy())
_, err = c.R().Get(ts.URL + "/redirect-1")
assertEqual(t, true, (err.Error() == "Get /redirect-2: auto redirect is disabled" ||
err.Error() == "Get \"/redirect-2\": auto redirect is disabled"))
assertEqual(t, true, (err.Error() == "Get /redirect-2: resty: auto redirect is disabled" ||
err.Error() == "Get \"/redirect-2\": resty: auto redirect is disabled"))
}

func TestClientTimeout(t *testing.T) {
Expand Down Expand Up @@ -380,7 +380,6 @@ func TestClientOnBeforeRequestModification(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, "TestGet: text response", resp.String())

logResponse(t, resp)
Expand Down Expand Up @@ -481,6 +480,13 @@ func TestClientSettingsCoverage(t *testing.T) {

ct.outputLogTo(io.Discard)
// [End] Custom Transport scenario

// Response - for now stay here
resp := &Response{Request: &Request{}}
s, err := resp.fmtBodyString(0)
assertNil(t, err)
assertEqual(t, "***** NO CONTENT *****", s)
fmt.Println(err, s)
}

func TestContentLengthWhenBodyIsNil(t *testing.T) {
Expand Down Expand Up @@ -533,6 +539,21 @@ func TestClientPreRequestHook(t *testing.T) {
assertEqual(t, `{ "id": "success", "message": "login successful" }`, resp.String())
}

func TestClientPreRequestHookError(t *testing.T) {
ts := createGetServer(t)
defer ts.Close()

c := dcnl()
c.SetPreRequestHook(func(c *Client, r *http.Request) error {
return errors.New("error from PreRequestHook")
})

resp, err := c.R().Get(ts.URL)
assertNotNil(t, err)
assertEqual(t, "error from PreRequestHook", err.Error())
assertNil(t, resp)
}

func TestClientAllowsGetMethodPayload(t *testing.T) {
ts := createGetServer(t)
defer ts.Close()
Expand Down Expand Up @@ -661,7 +682,6 @@ func TestGzipCompress(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, tc.want, resp.String())

logResponse(t, resp)
Expand All @@ -684,7 +704,6 @@ func TestDeflateCompress(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, tc.want, resp.String())

logResponse(t, resp)
Expand Down Expand Up @@ -738,7 +757,6 @@ func TestLzwCompress(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, tc.want, resp.String())

logResponse(t, resp)
Expand Down Expand Up @@ -1216,7 +1234,7 @@ func TestResponseBodyLimit(t *testing.T) {

res, err := c.R().SetResponseBodyLimit(800*100 + 10).Get(ts.URL + "/")
assertNil(t, err)
assertEqual(t, 800*100, len(res.BodyBytes()))
assertEqual(t, 800*100, len(res.bodyBytes))
assertEqual(t, int64(800*100), res.Size())
})

Expand All @@ -1225,7 +1243,7 @@ func TestResponseBodyLimit(t *testing.T) {

res, err := c.R().Get(ts.URL + "/")
assertNil(t, err)
assertEqual(t, 800*100, len(res.BodyBytes()))
assertEqual(t, 800*100, len(res.bodyBytes))
assertEqual(t, int64(800*100), res.Size())
})

Expand Down
1 change: 0 additions & 1 deletion context_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ func TestSetContext(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertEqual(t, true, resp.BodyBytes() != nil)
assertEqual(t, "TestGet: text response", resp.String())

logResponse(t, resp)
Expand Down
18 changes: 8 additions & 10 deletions middleware.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ package resty

import (
"bytes"
"errors"
"fmt"
"io"
"mime/multipart"
Expand Down Expand Up @@ -166,8 +165,11 @@ func parseRequestHeader(c *Client, r *Request) error {
r.Header.Set(hdrUserAgentKey, hdrUserAgentValue)
}

if ct := r.Header.Get(hdrContentTypeKey); isStringEmpty(r.Header.Get(hdrAcceptKey)) && !isStringEmpty(ct) && (isJSONContentType(ct) || isXMLContentType(ct)) {
r.Header.Set(hdrAcceptKey, r.Header.Get(hdrContentTypeKey))
if isStringEmpty(r.Header.Get(hdrAcceptKey)) {
ct := r.Header.Get(hdrContentTypeKey)
if isJSONContentType(ct) || isXMLContentType(ct) {
r.Header.Set(hdrAcceptKey, ct)
}
}

if isStringEmpty(r.Header.Get(hdrAcceptEncodingKey)) {
Expand Down Expand Up @@ -390,8 +392,6 @@ func parseResponseBody(c *Client, res *Response) (err error) {
return
}

// TODO Attention Required when working on Compression

rct := firstNonEmpty(
res.Request.ForceResponseContentType,
res.Header().Get(hdrContentTypeKey),
Expand All @@ -401,7 +401,7 @@ func parseResponseBody(c *Client, res *Response) (err error) {
decFunc, found := c.inferContentTypeDecoder(rct, decKey)
if !found {
// the Content-Type decoder is not found; just read all the body bytes
err = res.readAllBytes()
err = res.readAll()
return
}

Expand Down Expand Up @@ -430,7 +430,7 @@ func parseResponseBody(c *Client, res *Response) (err error) {
}

// read all bytes when auto-unmarshal didn't take place
err = res.readAllBytes()
err = res.readAll()
return
}

Expand Down Expand Up @@ -499,8 +499,6 @@ func handleFormData(c *Client, r *Request) {
r.isFormData = true
}

var ErrUnsupportedRequestBodyKind = errors.New("resty: unsupported request body kind")

func handleRequestBody(c *Client, r *Request) error {
contentType := r.Header.Get(hdrContentTypeKey)
if isStringEmpty(contentType) {
Expand All @@ -513,7 +511,7 @@ func handleRequestBody(c *Client, r *Request) error {

switch body := r.Body.(type) {
case io.Reader:
// TODO create pass through reader to capture content-length
// TODO create pass through reader to capture content-length, really needed??
if r.setContentLength { // keep backward compatibility
if _, err := r.bodyBuf.ReadFrom(body); err != nil {
releaseBuffer(r.bodyBuf)
Expand Down
6 changes: 2 additions & 4 deletions redirect.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import (
"strings"
)

var (
ErrAutoRedirectDisabled = errors.New("auto redirect is disabled")
)
var ErrAutoRedirectDisabled = errors.New("resty: auto redirect is disabled")

type (
// RedirectPolicy to regulate the redirects in the Resty client.
Expand All @@ -23,7 +21,7 @@ type (
// Apply function should return nil to continue the redirect journey; otherwise
// return error to stop the redirect.
RedirectPolicy interface {
Apply(req *http.Request, via []*http.Request) error
Apply(*http.Request, []*http.Request) error
}

// The [RedirectPolicyFunc] type is an adapter to allow the use of ordinary
Expand Down
5 changes: 0 additions & 5 deletions request.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,17 +89,12 @@ func (r *Request) GenerateCurlCommand() string {
if !(r.Debug && r.generateCurlOnDebug) {
return ""
}

if r.resultCurlCmd != nil {
return *r.resultCurlCmd
}

if r.RawRequest == nil {
r.client.executeBefore(r) // mock with r.Get("/")
}
if r.resultCurlCmd == nil {
r.resultCurlCmd = new(string)
}
*r.resultCurlCmd = buildCurlRequest(r)
return *r.resultCurlCmd
}
Expand Down
7 changes: 4 additions & 3 deletions request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ func TestGet(t *testing.T) {
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "HTTP/1.1", resp.Proto())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, "TestGet: text response", resp.String())

logResponse(t, resp)
Expand Down Expand Up @@ -1873,7 +1872,6 @@ func TestRequestQueryStringOrder(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, "TestGet: text response", resp.String())

logResponse(t, resp)
Expand Down Expand Up @@ -1937,7 +1935,6 @@ func TestHostHeaderOverride(t *testing.T) {
assertError(t, err)
assertEqual(t, http.StatusOK, resp.StatusCode())
assertEqual(t, "200 OK", resp.Status())
assertNotNil(t, resp.BodyBytes())
assertEqual(t, "myhostname", resp.String())

logResponse(t, resp)
Expand Down Expand Up @@ -2392,4 +2389,8 @@ func TestRequestSettingsCoverage(t *testing.T) {
c.R().SetResponseBodyUnlimitedReads(true)

c.R().DisableDebug()

invalidJsonBytes := []byte(`{\" \": "value here"}`)
result := jsonIndent(invalidJsonBytes)
assertEqual(t, string(invalidJsonBytes), string(result))
}
33 changes: 10 additions & 23 deletions response.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,6 @@ type Response struct {
receivedAt time.Time
}

// BodyBytes method returns the HTTP response as `[]byte` slice for the executed request.
//
// NOTE:
// - [Response.BodyBytes] might be `nil` if [Request.SetOutputFile], [Request.SetDoNotParseResponse],
// [Client.SetDoNotParseResponse] method is used.
// - [Response.BodyBytes] might be `nil` if [Response].Body is already auto-unmarshal performed.
//
// TODO remove it
func (r *Response) BodyBytes() []byte {
if r.RawResponse == nil {
return []byte{}
}
return r.bodyBytes
}

// Status method returns the HTTP status string for the executed request.
//
// Example: 200 OK
Expand Down Expand Up @@ -107,10 +92,12 @@ func (r *Response) Cookies() []*http.Cookie {
// It returns an empty string if it is nil or the body is zero length.
//
// NOTE:
// - Returns an empty string on auto-unmarshal scenarios
// - Returns an empty string on auto-unmarshal scenarios, unless
// [Client.SetResponseBodyUnlimitedReads] or [Request.SetResponseBodyUnlimitedReads] set.
// - Returns an empty string when [Client.SetDoNotParseResponse] or [Request.SetDoNotParseResponse] set
func (r *Response) String() string {
if len(r.bodyBytes) == 0 && !r.Request.DoNotParseResponse {
_ = r.readAllBytes()
_ = r.readAll()
}
return strings.TrimSpace(string(r.bodyBytes))
}
Expand Down Expand Up @@ -191,17 +178,17 @@ func (r *Response) fmtBodyString(sl int) (string, error) {

// auto-unmarshal didn't happen, so fallback to
// old behavior of reading response as body bytes
func (r *Response) readAllBytes() (err error) {
func (r *Response) readAll() (err error) {
if r.Body == nil || r.IsRead {
return nil
}

if _, ok := r.Body.(*readCopier); ok {
if _, ok := r.Body.(*copyReadCloser); ok {
_, err = io.ReadAll(r.Body)
} else {
r.bodyBytes, err = io.ReadAll(r.Body)
closeq(r.Body)
r.Body = &readNoOpCloser{r: bytes.NewReader(r.bodyBytes)}
r.Body = &nopReadCloser{r: bytes.NewReader(r.bodyBytes)}
}
if err == io.ErrUnexpectedEOF {
// content-encoding scenario's - empty/no response body from server
Expand All @@ -222,14 +209,14 @@ func (r *Response) wrapLimitReadCloser() {
}
}

func (r *Response) wrapReadCopier() {
r.Body = &readCopier{
func (r *Response) wrapCopyReadCloser() {
r.Body = &copyReadCloser{
s: r.Body,
t: acquireBuffer(),
f: func(b *bytes.Buffer) {
r.bodyBytes = append([]byte{}, b.Bytes()...)
closeq(r.Body)
r.Body = &readNoOpCloser{r: bytes.NewReader(r.bodyBytes)}
r.Body = &nopReadCloser{r: bytes.NewReader(r.bodyBytes)}
releaseBuffer(b)
},
}
Expand Down
Loading