impr: BKTCLT-35 go client: custom HTTP client

Add the possibility to attach a custom HTTP client to a new instance
of BucketClient.

The first intended use is to add a timeout to HTTP requests sent by
BucketClient.

go/bucketclient.go

@@ -1,9 +1,26 @@
 package bucketclient
 
+import (
+	"net/http"
+)
+
 type BucketClient struct {
-	Endpoint string
+	Endpoint   string
+	HttpClient *http.Client
 }
 
-func New(bucketdEndpoint string) *BucketClient {
-	return &BucketClient{bucketdEndpoint}
+// New creates a new BucketClient instance, with the provided endpoint (e.g. "localhost:9000").
+//
+// If httpClient is non-nil, it uses the provided HTTP client instance, otherwise uses
+// `http.DefaultClient`.
+func New(bucketdEndpoint string, httpClient *http.Client) *BucketClient {
+	client := &BucketClient{
+		Endpoint: bucketdEndpoint,
+	}
+	if httpClient != nil {
+		client.HttpClient = httpClient
+	} else {
+		client.HttpClient = http.DefaultClient
+	}
+	return client
 }

go/bucketclient_suite_test.go

@@ -19,7 +19,7 @@ var client *bucketclient.BucketClient
 
 var _ = BeforeSuite(func() {
 	httpmock.Activate()
-	client = bucketclient.New("http://localhost:9000")
+	client = bucketclient.New("http://localhost:9000", nil)
 })
 
 var _ = AfterSuite(func() {

go/bucketclient_test.go

@@ -9,7 +9,7 @@ import (
 
 var _ = Describe("BucketClient", func() {
 	It("New", func() {
-		client := bucketclient.New("http://localhost:9000")
+		client := bucketclient.New("http://localhost:9000", nil)
 		Expect(client).ToNot(BeNil())
 		Expect(client.Endpoint).To(Equal("http://localhost:9000"))
 	})

go/bucketclientrequest.go

@@ -67,7 +67,7 @@ func (client *BucketClient) Request(ctx context.Context,
 			if options.idempotent {
 				request.Header["Idempotency-Key"] = []string{}
 			}
-			response, err = http.DefaultClient.Do(request)
+			response, err = client.HttpClient.Do(request)
 		}
 	}
 	if err != nil {

go/bucketclientrequest_test.go

@@ -29,12 +29,12 @@ var _ = Describe("BucketClient.Request()", func() {
 		// Here we do a basic test checking that errors from
 		// the connection layer are forwarded correctly, "no
 		// responder found" is the error returned by httpmock
-		invalidClient := bucketclient.New("http://invalid:9000")
+		invalidClient := bucketclient.New("http://invalid:9000", nil)
 		_, err := invalidClient.Request(ctx, "GetSomething", "GET", "/foo/bar")
 		Expect(err).To(MatchError(ContainSubstring("no responder found")))
 	})
 
-	Context("with valid URL", Ordered, func() {
+	Context("with valid URL and default HTTP client", Ordered, func() {
 		It("succeeds with a 200 response on GET request", func(ctx SpecContext) {
 			httpmock.RegisterResponder(
 				"GET", "http://localhost:9000/default/bucket/somebucket/someobject",
@@ -133,4 +133,47 @@ var _ = Describe("BucketClient.Request()", func() {
 			Expect(err).To(MatchError(context.DeadlineExceeded))
 		})
 	})
+
+	Context("with valid URL and custom HTTP client with timeout", Ordered, func() {
+		var clientWithTimeout *bucketclient.BucketClient
+
+		BeforeAll(func() {
+			clientWithTimeout = bucketclient.New("http://localhost:9000", &http.Client{
+				Timeout: 1 * time.Second,
+			})
+		})
+
+		It("succeeds with a 200 response on GET request", func(ctx SpecContext) {
+			httpmock.RegisterResponder(
+				"GET", "http://localhost:9000/default/bucket/somebucket/someobject",
+				httpmock.NewStringResponder(200, `{"some":"metadata","version":"1234"}`),
+			)
+			Expect(clientWithTimeout.Request(ctx, "GetObject", "GET",
+				"/default/bucket/somebucket/someobject")).To(Equal(
+				[]byte(`{"some":"metadata","version":"1234"}`)))
+		})
+
+		It("times out after the configured delay without a response", func(ctx SpecContext) {
+			httpmock.RegisterResponder(
+				"GET", "http://localhost:9000/default/bucket/somebucket/someobject",
+				func(req *http.Request) (*http.Response, error) {
+					// respond after 2 seconds > timeout of one second
+					time.Sleep(2 * time.Second)
+					return httpmock.NewStringResponse(200,
+						`{"some":"metadata","version":"1234"}`), nil
+				},
+			)
+			_, err := clientWithTimeout.Request(ctx, "GetObject", "GET",
+				"/default/bucket/somebucket/someobject")
+			Expect(err).ToNot(BeNil())
+			bcErr, isBCErr := err.(*bucketclient.BucketClientError)
+			Expect(isBCErr).To(BeTrue())
+			Expect(bcErr.StatusCode).To(Equal(0))
+			Expect(ctx.Err()).To(BeNil())
+			// the error returned by the HTTP layer seems to be inconsistent between
+			// "Client.Timeout exceeded" and "context deadline exceeded" for some reason,
+			// I did not dig into it further but it makes it hard to properly test the
+			// properties about the returned error.
+		})
+	})
 })