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

transport/http: NewExplicitClient #971

Merged
merged 2 commits into from Mar 20, 2020
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
82 changes: 46 additions & 36 deletions transport/http/client.go
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Comment on lines +202 to +206

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add some meaningful comments here instead ? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's an unexported function, and I find it pretty self-explanatory.

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
}
}
32 changes: 32 additions & 0 deletions transport/http/client_test.go
Expand Up @@ -3,11 +3,13 @@ package http_test
import (
"bytes"
"context"
"fmt"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 6 additions & 0 deletions transport/http/encode_decode.go
Expand Up @@ -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)
Comment on lines +20 to +24

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you want to mark EncodeRequestFunc as // deprecated linking this issue for more information ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, because I don't consider it deprecated, this is just an alternative.


// 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
Expand Down