Merge remote-tracking branch 'origin/improvement/BKTCLT-35-goClientCu…

…stomHTTPClient' into w/8.1/improvement/BKTCLT-35-goClientCustomHTTPClient

go/bucketclient.go

@@ -1,12 +1,30 @@
 package bucketclient
 
+import (
+	"net/http"
+)
+
 type BucketClient struct {
+	*http.Client
 	Endpoint string
 	Metrics  *BucketClientMetrics
 }
 
+// New creates a new BucketClient instance, with the provided endpoint (e.g. "localhost:9000")
+// and the default HTTP client.
 func New(bucketdEndpoint string) *BucketClient {
+	bc := &BucketClient{
+		Client:   http.DefaultClient,
+		Endpoint: bucketdEndpoint,
+	}
+	return bc
+}
+
+// NewWithHTTPClient creates a new BucketClient instance, with the provided endpoint
+// (e.g. "localhost:9000") using the provided http.Client instance.
+func NewWithHTTPClient(bucketdEndpoint string, httpClient *http.Client) *BucketClient {
 	return &BucketClient{
+		Client:   httpClient,
 		Endpoint: bucketdEndpoint,
 	}
 }

go/bucketclientrequest.go

@@ -115,7 +115,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.Do(request)
 		}
 	}
 	if err != nil {

go/bucketclientrequest_test.go

@@ -34,7 +34,7 @@ var _ = Describe("BucketClient.Request()", func() {
 		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",
@@ -148,4 +148,54 @@ 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.NewWithHTTPClient("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
+				},
+			)
+			startTime := time.Now()
+			_, err := clientWithTimeout.Request(ctx, "GetObject", "GET",
+				"/default/bucket/somebucket/someobject")
+			duration := time.Since(startTime)
+
+			Expect(err).ToNot(BeNil())
+			bcErr, isBCErr := err.(*bucketclient.BucketClientError)
+			Expect(isBCErr).To(BeTrue())
+			Expect(bcErr.StatusCode).To(Equal(0))
+			// 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, so just checking that there is an
+			// error and that we didn't wait much more than the timeout.
+			Expect(ctx.Err()).To(BeNil())
+			Expect(duration).To(BeNumerically(">=", 900*time.Millisecond))
+			Expect(duration).To(BeNumerically("<=", 1100*time.Millisecond))
+		})
+	})
 })