Switch Builder.Do() to return a Result

pkg/requests/builder.go

@@ -2,27 +2,23 @@ package requests
 
 import (
 	"context"
-	"encoding/json"
 	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
-
-	"github.com/bitly/go-simplejson"
 )
 
-// Builder allows users to construct a request and then either get the requests
+// Builder allows users to construct a request and then execute the
-// response via Do(), parse the response into a simplejson.Json via JSON(),
+// request via Do().
-// or to parse the json response into an object via UnmarshalInto().
+// Do returns a Result which allows the user to get the body,
+// unmarshal the body into an interface, or into a simplejson.Json.
 type Builder interface {
 	WithContext(context.Context) Builder
 	WithBody(io.Reader) Builder
 	WithMethod(string) Builder
 	WithHeaders(http.Header) Builder
 	SetHeader(key, value string) Builder
-	Do() (*http.Response, error)
+	Do() Result
-	UnmarshalInto(interface{}) error
-	UnmarshalJSON() (*simplejson.Json, error)
 }
 
 type builder struct {
@@ -31,7 +27,7 @@ type builder struct {
 	endpoint string
 	body     io.Reader
 	header   http.Header
-	response *http.Response
+	result   *result
 }
 
 // New provides a new Builder for the given endpoint.
@@ -80,10 +76,10 @@ func (r *builder) SetHeader(key, value string) Builder {
 // Do performs the request and returns the response in its raw form.
 // If the request has already been performed, returns the previous result.
 // This will not allow you to repeat a request.
-func (r *builder) Do() (*http.Response, error) {
+func (r *builder) Do() Result {
-	if r.response != nil {
+	if r.result != nil {
 		// Request has already been done
-		return r.response, nil
+		return r.result
 	}
 
 	// Must provide a non-nil context to NewRequestWithContext
@@ -91,83 +87,32 @@ func (r *builder) Do() (*http.Response, error) {
 		r.context = context.Background()
 	}
 
+	return r.do()
+}
+
+// do creates the request, executes it with the default client and extracts the
+// the body into the response
+func (r *builder) do() Result {
 	req, err := http.NewRequestWithContext(r.context, r.method, r.endpoint, r.body)
 	if err != nil {
-		return nil, fmt.Errorf("error creating request: %v", err)
+		r.result = &result{err: fmt.Errorf("error creating request: %v", err)}
+		return r.result
 	}
 	req.Header = r.header
 
 	resp, err := http.DefaultClient.Do(req)
 	if err != nil {
-		return nil, fmt.Errorf("error performing request: %v", err)
+		r.result = &result{err: fmt.Errorf("error performing request: %v", err)}
+		return r.result
 	}
 
-	r.response = resp
-	return resp, nil
-}
-
-// UnmarshalInto performs the request and attempts to unmarshal the response into the
-// the given interface. The response body is assumed to be JSON.
-// The response must have a 200 status otherwise an error will be returned.
-func (r *builder) UnmarshalInto(into interface{}) error {
-	resp, err := r.Do()
-	if err != nil {
-		return err
-	}
-
-	return UnmarshalInto(resp, into)
-}
-
-// UnmarshalJSON performs the request and attempts to unmarshal the response into a
-// simplejson.Json. The response body is assume to be JSON.
-// The response must have a 200 status otherwise an error will be returned.
-func (r *builder) UnmarshalJSON() (*simplejson.Json, error) {
-	resp, err := r.Do()
-	if err != nil {
-		return nil, err
-	}
-
-	body, err := getResponseBody(resp)
-	if err != nil {
-		return nil, err
-	}
-
-	data, err := simplejson.NewJson(body)
-	if err != nil {
-		return nil, fmt.Errorf("error reading json: %v", err)
-	}
-	return data, nil
-}
-
-// UnmarshalInto attempts to unmarshal the response into the the given interface.
-// The response body is assumed to be JSON.
-// The response must have a 200 status otherwise an error will be returned.
-func UnmarshalInto(resp *http.Response, into interface{}) error {
-	body, err := getResponseBody(resp)
-	if err != nil {
-		return err
-	}
-
-	if err := json.Unmarshal(body, into); err != nil {
-		return fmt.Errorf("error unmarshalling body: %v", err)
-	}
-
-	return nil
-}
-
-// getResponseBody extracts the response body, but will only return the body
-// if the response was successful.
-func getResponseBody(resp *http.Response) ([]byte, error) {
 	defer resp.Body.Close()
 	body, err := ioutil.ReadAll(resp.Body)
 	if err != nil {
-		return nil, fmt.Errorf("error reading response body: %v", err)
+		r.result = &result{err: fmt.Errorf("error reading response body: %v", err)}
+		return r.result
 	}
 
-	// Only unmarshal body if the response was successful
+	r.result = &result{response: resp, body: body}
-	if resp.StatusCode != http.StatusOK {
+	return r.result
-		return nil, fmt.Errorf("unexpected status \"%d\": %s", resp.StatusCode, body)
-	}
-
-	return body, nil
 }

pkg/requests/builder_test.go

@@ -6,7 +6,6 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 
 	"github.com/bitly/go-simplejson"
@@ -215,8 +214,8 @@ var _ = Describe("Builder suite", func() {
 
 	Context("if the request has been completed and then modified", func() {
 		BeforeEach(func() {
-			_, err := b.Do()
+			result := b.Do()
-			Expect(err).ToNot(HaveOccurred())
+			Expect(result.Error()).ToNot(HaveOccurred())
 
 			b.WithMethod("POST")
 		})
@@ -250,25 +249,20 @@ var _ = Describe("Builder suite", func() {
 
 func assertSuccessfulRequest(builder func() Builder, expectedRequest testHTTPRequest) {
 	Context("Do", func() {
-		var resp *http.Response
+		var result Result
 
 		BeforeEach(func() {
-			var err error
+			result = builder().Do()
-			resp, err = builder().Do()
+			Expect(result.Error()).ToNot(HaveOccurred())
-			Expect(err).ToNot(HaveOccurred())
 		})
 
 		It("returns a successful status", func() {
-			Expect(resp.StatusCode).To(Equal(http.StatusOK))
+			Expect(result.StatusCode()).To(Equal(http.StatusOK))
 		})
 
 		It("made the expected request", func() {
-			body, err := ioutil.ReadAll(resp.Body)
-			Expect(err).ToNot(HaveOccurred())
-			resp.Body.Close()
-
 			actualRequest := testHTTPRequest{}
-			Expect(json.Unmarshal(body, &actualRequest)).To(Succeed())
+			Expect(json.Unmarshal(result.Body(), &actualRequest)).To(Succeed())
 
 			Expect(actualRequest).To(Equal(expectedRequest))
 		})
@@ -278,7 +272,7 @@ func assertSuccessfulRequest(builder func() Builder, expectedRequest testHTTPReq
 		var actualRequest testHTTPRequest
 
 		BeforeEach(func() {
-			Expect(builder().UnmarshalInto(&actualRequest)).To(Succeed())
+			Expect(builder().Do().UnmarshalInto(&actualRequest)).To(Succeed())
 		})
 
 		It("made the expected request", func() {
@@ -291,7 +285,7 @@ func assertSuccessfulRequest(builder func() Builder, expectedRequest testHTTPReq
 
 		BeforeEach(func() {
 			var err error
-			response, err = builder().UnmarshalJSON()
+			response, err = builder().Do().UnmarshalJSON()
 			Expect(err).ToNot(HaveOccurred())
 		})
 
@@ -328,16 +322,15 @@ func assertSuccessfulRequest(builder func() Builder, expectedRequest testHTTPReq
 func assertRequestError(builder func() Builder, errorMessage string) {
 	Context("Do", func() {
 		It("returns an error", func() {
-			resp, err := builder().Do()
+			result := builder().Do()
-			Expect(err).To(MatchError(ContainSubstring(errorMessage)))
+			Expect(result.Error()).To(MatchError(ContainSubstring(errorMessage)))
-			Expect(resp).To(BeNil())
 		})
 	})
 
 	Context("UnmarshalInto", func() {
 		It("returns an error", func() {
 			var actualRequest testHTTPRequest
-			err := builder().UnmarshalInto(&actualRequest)
+			err := builder().Do().UnmarshalInto(&actualRequest)
 			Expect(err).To(MatchError(ContainSubstring(errorMessage)))
 
 			// Should be empty
@@ -347,7 +340,7 @@ func assertRequestError(builder func() Builder, errorMessage string) {
 
 	Context("UnmarshalJSON", func() {
 		It("returns an error", func() {
-			resp, err := builder().UnmarshalJSON()
+			resp, err := builder().Do().UnmarshalJSON()
 			Expect(err).To(MatchError(ContainSubstring(errorMessage)))
 			Expect(resp).To(BeNil())
 		})
@@ -357,16 +350,15 @@ func assertRequestError(builder func() Builder, errorMessage string) {
 func assertJSONError(builder func() Builder, errorMessage string) {
 	Context("Do", func() {
 		It("does not return an error", func() {
-			resp, err := builder().Do()
+			result := builder().Do()
-			Expect(err).To(BeNil())
+			Expect(result.Error()).To(BeNil())
-			Expect(resp).ToNot(BeNil())
 		})
 	})
 
 	Context("UnmarshalInto", func() {
 		It("returns an error", func() {
 			var actualRequest testHTTPRequest
-			err := builder().UnmarshalInto(&actualRequest)
+			err := builder().Do().UnmarshalInto(&actualRequest)
 			Expect(err).To(MatchError(ContainSubstring(errorMessage)))
 
 			// Should be empty
@@ -376,7 +368,7 @@ func assertJSONError(builder func() Builder, errorMessage string) {
 
 	Context("UnmarshalJSON", func() {
 		It("returns an error", func() {
-			resp, err := builder().UnmarshalJSON()
+			resp, err := builder().Do().UnmarshalJSON()
 			Expect(err).To(MatchError(ContainSubstring(errorMessage)))
 			Expect(resp).To(BeNil())
 		})

pkg/requests/result.go

@@ -0,0 +1,98 @@
+package requests
+
+import (
+	"encoding/json"
+	"fmt"
+	"net/http"
+
+	"github.com/bitly/go-simplejson"
+)
+
+// Result is the result of a request created by a Builder
+type Result interface {
+	Error() error
+	StatusCode() int
+	Headers() http.Header
+	Body() []byte
+	UnmarshalInto(interface{}) error
+	UnmarshalJSON() (*simplejson.Json, error)
+}
+
+type result struct {
+	err      error
+	response *http.Response
+	body     []byte
+}
+
+// Error returns an error from the result if present
+func (r *result) Error() error {
+	return r.err
+}
+
+// StatusCode returns the response's status code
+func (r *result) StatusCode() int {
+	if r.response != nil {
+		return r.response.StatusCode
+	}
+	return 0
+}
+
+// Headers returns the response's headers
+func (r *result) Headers() http.Header {
+	if r.response != nil {
+		return r.response.Header
+	}
+	return nil
+}
+
+// Body returns the response's body
+func (r *result) Body() []byte {
+	return r.body
+}
+
+// UnmarshalInto attempts to unmarshal the response into the the given interface.
+// The response body is assumed to be JSON.
+// The response must have a 200 status otherwise an error will be returned.
+func (r *result) UnmarshalInto(into interface{}) error {
+	body, err := r.getBodyForUnmarshal()
+	if err != nil {
+		return err
+	}
+
+	if err := json.Unmarshal(body, into); err != nil {
+		return fmt.Errorf("error unmarshalling body: %v", err)
+	}
+
+	return nil
+}
+
+// UnmarshalJSON performs the request and attempts to unmarshal the response into a
+// simplejson.Json. The response body is assume to be JSON.
+// The response must have a 200 status otherwise an error will be returned.
+func (r *result) UnmarshalJSON() (*simplejson.Json, error) {
+	body, err := r.getBodyForUnmarshal()
+	if err != nil {
+		return nil, err
+	}
+
+	data, err := simplejson.NewJson(body)
+	if err != nil {
+		return nil, fmt.Errorf("error reading json: %v", err)
+	}
+	return data, nil
+}
+
+// getBodyForUnmarshal returns the body if there wasn't an error and the status
+// code was 200.
+func (r *result) getBodyForUnmarshal() ([]byte, error) {
+	if r.Error() != nil {
+		return nil, r.Error()
+	}
+
+	// Only unmarshal body if the response was successful
+	if r.StatusCode() != http.StatusOK {
+		return nil, fmt.Errorf("unexpected status \"%d\": %s", r.StatusCode(), r.Body())
+	}
+
+	return r.Body(), nil
+}