transport/http: NewExplicitClient (#971)

* transport/http: NewExplicitClient

* transport/http: improve NewClient/NewExplicitClient relationship

transport/http/client.go

@@ -21,9 +21,7 @@ type HTTPClient interface {
 // Client wraps a URL and provides a method that implements endpoint.Endpoint.
 type Client struct {
 	client         HTTPClient
-	method         string
-	tgt            *url.URL
-	enc            EncodeRequestFunc
+	req            CreateRequestFunc
 	dec            DecodeResponseFunc
 	before         []RequestFunc
 	after          []ClientResponseFunc
@@ -32,22 +30,18 @@ type Client struct {
 }
 
 // NewClient constructs a usable Client for a single remote method.
-func NewClient(
-	method string,
-	tgt *url.URL,
-	enc EncodeRequestFunc,
-	dec DecodeResponseFunc,
-	options ...ClientOption,
-) *Client {
+func NewClient(method string, tgt *url.URL, enc EncodeRequestFunc, dec DecodeResponseFunc, options ...ClientOption) *Client {
+	return NewExplicitClient(makeCreateRequestFunc(method, tgt, enc), dec, options...)
+}
+
+// NewExplicitClient is like NewClient but uses a CreateRequestFunc instead of a
+// method, target URL, and EncodeRequestFunc, which allows for more control over
+// the outgoing HTTP request.
+func NewExplicitClient(req CreateRequestFunc, dec DecodeResponseFunc, options ...ClientOption) *Client {
 	c := &Client{
-		client:         http.DefaultClient,
-		method:         method,
-		tgt:            tgt,
-		enc:            enc,
-		dec:            dec,
-		before:         []RequestFunc{},
-		after:          []ClientResponseFunc{},
-		bufferedStream: false,
+		client: http.DefaultClient,
+		req:    req,
+		dec:    dec,
 	}
 	for _, option := range options {
 		option(c)
@@ -64,33 +58,35 @@ func SetClient(client HTTPClient) ClientOption {
 	return func(c *Client) { c.client = client }
 }
 
-// ClientBefore sets the RequestFuncs that are applied to the outgoing HTTP
+// ClientBefore adds one or more RequestFuncs to be applied to the outgoing HTTP
 // request before it's invoked.
 func ClientBefore(before ...RequestFunc) ClientOption {
 	return func(c *Client) { c.before = append(c.before, before...) }
 }
 
-// ClientAfter sets the ClientResponseFuncs applied to the incoming HTTP
-// request prior to it being decoded. This is useful for obtaining anything off
-// of the response and adding onto the context prior to decoding.
+// ClientAfter adds one or more ClientResponseFuncs, which are applied to the
+// incoming HTTP response prior to it being decoded. This is useful for
+// obtaining anything off of the response and adding it into the context prior
+// to decoding.
 func ClientAfter(after ...ClientResponseFunc) ClientOption {
 	return func(c *Client) { c.after = append(c.after, after...) }
 }
 
-// ClientFinalizer is executed at the end of every HTTP request.
-// By default, no finalizer is registered.
+// ClientFinalizer adds one or more ClientFinalizerFuncs to be executed at the
+// end of every HTTP request. Finalizers are executed in the order in which they
+// were added. By default, no finalizer is registered.
 func ClientFinalizer(f ...ClientFinalizerFunc) ClientOption {
 	return func(s *Client) { s.finalizer = append(s.finalizer, f...) }
 }
 
-// BufferedStream sets whether the Response.Body is left open, allowing it
+// BufferedStream sets whether the HTTP response body is left open, allowing it
 // to be read from later. Useful for transporting a file as a buffered stream.
-// That body has to be Closed to propery end the request.
+// That body has to be drained and closed to properly end the request.
 func BufferedStream(buffered bool) ClientOption {
 	return func(c *Client) { c.bufferedStream = buffered }
 }
 
-// Endpoint returns a usable endpoint that invokes the remote endpoint.
+// Endpoint returns a usable Go kit endpoint that calls the remote HTTP endpoint.
 func (c Client) Endpoint() endpoint.Endpoint {
 	return func(ctx context.Context, request interface{}) (interface{}, error) {
 		ctx, cancel := context.WithCancel(ctx)
@@ -111,30 +107,25 @@ func (c Client) Endpoint() endpoint.Endpoint {
 			}()
 		}
 
-		req, err := http.NewRequest(c.method, c.tgt.String(), nil)
+		req, err := c.req(ctx, request)
 		if err != nil {
 			cancel()
 			return nil, err
 		}
 
-		if err = c.enc(ctx, req, request); err != nil {
-			cancel()
-			return nil, err
-		}
-
 		for _, f := range c.before {
 			ctx = f(ctx, req)
 		}
 
 		resp, err = c.client.Do(req.WithContext(ctx))
-
 		if err != nil {
 			cancel()
 			return nil, err
 		}
 
-		// If we expect a buffered stream, we don't cancel the context when the endpoint returns.
-		// Instead, we should call the cancel func when closing the response body.
+		// If the caller asked for a buffered stream, we don't cancel the
+		// context when the endpoint returns. Instead, we should call the
+		// cancel func when closing the response body.
 		if c.bufferedStream {
 			resp.Body = bodyWithCancel{ReadCloser: resp.Body, cancel: cancel}
 		} else {
@@ -207,3 +198,22 @@ func EncodeXMLRequest(c context.Context, r *http.Request, request interface{}) e
 	r.Body = ioutil.NopCloser(&b)
 	return xml.NewEncoder(&b).Encode(request)
 }
+
+//
+//
+//
+
+func makeCreateRequestFunc(method string, target *url.URL, enc EncodeRequestFunc) CreateRequestFunc {
+	return func(ctx context.Context, request interface{}) (*http.Request, error) {
+		req, err := http.NewRequest(method, target.String(), nil)
+		if err != nil {
+			return nil, err
+		}
+
+		if err = enc(ctx, req, request); err != nil {
+			return nil, err
+		}
+
+		return req, nil
+	}
+}

transport/http/client_test.go

@@ -3,11 +3,13 @@ package http_test
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 	"time"
 
@@ -297,6 +299,36 @@ func TestSetClient(t *testing.T) {
 	}
 }
 
+func TestNewExplicitClient(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, "%d", r.ContentLength)
+	}))
+	defer srv.Close()
+
+	req := func(ctx context.Context, request interface{}) (*http.Request, error) {
+		req, _ := http.NewRequest("POST", srv.URL, strings.NewReader(request.(string)))
+		return req, nil
+	}
+
+	dec := func(_ context.Context, resp *http.Response) (response interface{}, err error) {
+		buf, err := ioutil.ReadAll(resp.Body)
+		resp.Body.Close()
+		return string(buf), err
+	}
+
+	client := httptransport.NewExplicitClient(req, dec)
+
+	request := "hello world"
+	response, err := client.Endpoint()(context.Background(), request)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	if want, have := "11", response.(string); want != have {
+		t.Fatalf("want %q, have %q", want, have)
+	}
+}
+
 func mustParse(s string) *url.URL {
 	u, err := url.Parse(s)
 	if err != nil {

transport/http/encode_decode.go

@@ -17,6 +17,12 @@ type DecodeRequestFunc func(context.Context, *http.Request) (request interface{}
 // encodes the object directly to the request body.
 type EncodeRequestFunc func(context.Context, *http.Request, interface{}) error
 
+// CreateRequestFunc creates an outgoing HTTP request based on the passed
+// request object. It's designed to be used in HTTP clients, for client-side
+// endpoints. It's a more powerful version of EncodeRequestFunc, and can be used
+// if more fine-grained control of the HTTP request is required.
+type CreateRequestFunc func(context.Context, interface{}) (*http.Request, error)
+
 // EncodeResponseFunc encodes the passed response object to the HTTP response
 // writer. It's designed to be used in HTTP servers, for server-side
 // endpoints. One straightforward EncodeResponseFunc could be something that