client ID

.coderabbit.yml

@@ -0,0 +1,15 @@
+# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
+language: "en-US"
+early_access: false
+reviews:
+  profile: chill
+  request_changes_workflow: true
+  high_level_summary: true
+  poem: true
+  review_status: true
+  collapse_walkthrough: false
+  auto_review:
+    enabled: true
+    drafts: true
+chat:
+  auto_reply: true

client.go

@@ -23,6 +23,11 @@ var (
 	nextPart       = func(mpr *multipart.Reader) (*multipart.Part, error) { return mpr.NextPart() }
 )
 
+var (
+	// seq is used to generate unique client ids; it is incremented each time a client is created
+	seq int
+)
+
 // RequestOption is a function that applies an option to a request
 type RequestOption = func(*http.Request) error
 
@@ -55,8 +60,8 @@ type ClientOption func(*client) error
 // This type is not exported; functionality is accessed through the implmented
 // HttpClient interface.
 type client struct {
-	// name is used to identify the client in error messages
-	name string
+	// id is used to identify the client in error messages
+	id string
 
 	// url is prepended to the url of any request made with the client
 	url string
@@ -80,9 +85,10 @@ type client struct {
 // The url typically includes the protocol, hostname and port for the client
 // but may include any additional url components consistently required for
 // requests performed using the client.
-func NewClient(name string, opts ...ClientOption) (HttpClient, error) {
+func NewClient(opts ...ClientOption) (HttpClient, error) {
+	seq++
 	w := client{
-		name:    name,
+		id:      "http-" + strconv.Itoa(seq),
 		wrapped: http.DefaultClient,
 	}
 	errs := make([]error, 0, len(opts))
@@ -273,7 +279,7 @@ func (c client) execute(
 ) (*http.Response, error) {
 	rq, err := c.NewRequest(ctx, method, url, opts...)
 	if err != nil {
-		return nil, errorcontext.Errorf(ctx, "%s: %s: %w", c.name, method, err)
+		return nil, errorcontext.Errorf(ctx, "%s: %s: %w", c.id, method, err)
 	}
 	return c.Do(rq)
 }
@@ -283,7 +289,7 @@ func (c client) execute(
 func (c client) Do(rq *http.Request) (*http.Response, error) {
 	ctx := rq.Context()
 	handle := func(r *http.Response, err error) (*http.Response, error) {
-		return r, errorcontext.Errorf(ctx, "%s: %s %s: %w", c.name, rq.Method, rq.URL, err)
+		return r, errorcontext.Errorf(ctx, "%s: %s %s: %w", c.id, rq.Method, rq.URL, err)
 	}
 
 	retries, statusCodes, bodyRequired, stream, err := c.parseRequestHeaders(rq)
@@ -417,11 +423,9 @@ func MapFromMultipartFormData[K comparable, V any](
 //
 // The function returns an error if the body cannot be read or if the body does not
 // contain valid JSON and the result will be the zero value of the generic type.
-func UnmarshalJSON[T any](ctx context.Context, r *http.Response) (T, error) {
-	result := *new(T)
-
-	handle := func(sen, err error) (T, error) {
-		return result, errorcontext.Errorf(ctx, "http.UnmarshalJSON: %w: %w", sen, err)
+func UnmarshalJSON[T any](r *http.Response) (*T, error) {
+	handle := func(sen, err error) (*T, error) {
+		return nil, fmt.Errorf("http.UnmarshalJSON: %w: %w", sen, err)
 	}
 
 	body, err := ioReadAll(r.Body)
@@ -430,9 +434,9 @@ func UnmarshalJSON[T any](ctx context.Context, r *http.Response) (T, error) {
 		return handle(ErrReadingResponseBody, err)
 	}
 
-	if err := json.Unmarshal(body, &result); err != nil {
+	result := new(T)
+	if err := json.Unmarshal(body, result); err != nil {
 		return handle(ErrInvalidJSON, err)
 	}
-
 	return result, nil
 }

clientOptions.go

@@ -6,6 +6,17 @@ import (
 	"net/url"
 )
 
+// ClientID sets the client ID for requests made using the client.  The client ID is
+// typically used to differentate between clients in log entries.  If no client ID is
+// provided a default value will be applied consisting of the string "http-<seq>"
+// where <seq> is a number that increments for each client created.
+func ClientID(id string) ClientOption {
+	return func(c *client) error {
+		c.id = id
+		return nil
+	}
+}
+
 // MaxRetries sets the maximum number of retries for requests made using the client.
 // Individual requests may be configured to override this value on a case-by-case basis.
 func MaxRetries(n uint) ClientOption {
@@ -16,11 +27,11 @@ func MaxRetries(n uint) ClientOption {
 }
 
 // URL sets the base URL for requests made using the client.  The URL may be specified
-// as a string or a *url.URL.
-//
-// If a string is provided, it will be parsed to ensure it is a valid, absolute URL.
+// as any of:
 //
-// If a URL is provided is must be absolute.
+//	string        // a which parses to a valid, absolute URL
+//	url.URL       // a valid, absolute URL
+//	*url.URL      // a valid, absolute URL
 func URL(u any) ClientOption {
 	return func(c *client) error {
 		switch u := u.(type) {
@@ -31,6 +42,12 @@ func URL(u any) ClientOption {
 			}
 			return URL(url)(c)
 
+		case url.URL:
+			if !u.IsAbs() {
+				return fmt.Errorf("http: URL option: %w: URL must be absolute", ErrInvalidURL)
+			}
+			c.url = u.String()
+
 		case *url.URL:
 			if !u.IsAbs() {
 				return fmt.Errorf("http: URL option: %w: URL must be absolute", ErrInvalidURL)

clientOptions_test.go

@@ -25,6 +25,19 @@ func TestClientOptions(t *testing.T) {
 		scenario string
 		exec     func(t *testing.T)
 	}{
+		{scenario: "ClientID",
+			exec: func(t *testing.T) {
+				// ARRANGE
+				sut := &client{}
+
+				// ACT
+				err := ClientID("foo")(sut)
+
+				// ASSERT
+				test.That(t, err).IsNil()
+				test.That(t, sut).Equals(&client{id: "foo"})
+			},
+		},
 		{scenario: "URL/int",
 			exec: func(t *testing.T) {
 				// ARRANGE
@@ -96,6 +109,33 @@ func TestClientOptions(t *testing.T) {
 				// ACT
 				err := URL(url)(client)
 
+				// ASSERT
+				test.Error(t, err).IsNil()
+				test.That(t, client.url).Equals("http://example.com")
+			},
+		},
+		{scenario: "URL/*URL/relative",
+			exec: func(t *testing.T) {
+				// ARRANGE
+				client := &client{}
+				url, _ := url.Parse("example.com")
+
+				// ACT
+				err := URL(*url)(client)
+
+				// ASSERT
+				test.Error(t, err).Is(ErrInvalidURL)
+			},
+		},
+		{scenario: "URL/*URL/successful",
+			exec: func(t *testing.T) {
+				// ARRANGE
+				client := &client{}
+				url, _ := url.Parse("http://example.com")
+
+				// ACT
+				err := URL(*url)(client)
+
 				// ASSERT
 				test.Error(t, err).IsNil()
 				test.That(t, client.url).Equals("http://example.com")

client_test.go

@@ -24,12 +24,12 @@ func TestNewClient(t *testing.T) {
 		{scenario: "no errors",
 			exec: func(t *testing.T) {
 				// ACT
-				result, err := NewClient("name", func(c *client) error { return nil })
+				result, err := NewClient(func(c *client) error { return nil })
 
 				// ASSERT
 				test.That(t, err).IsNil()
 				test.That(t, result).Equals(client{
-					name:    "name",
+					id:      "http-1",
 					wrapped: http.DefaultClient,
 				})
 			},
@@ -40,7 +40,7 @@ func TestNewClient(t *testing.T) {
 				opts := []ClientOption{func(c *client) error { return opterr }}
 
 				// ACT
-				result, err := NewClient("name", opts...)
+				result, err := NewClient(opts...)
 
 				// ASSERT
 				test.Error(t, err).Is(ErrInitialisingClient)
@@ -140,6 +140,10 @@ func TestNewRequest(t *testing.T) {
 	}
 	for _, tc := range testcases {
 		t.Run(tc.scenario, func(t *testing.T) {
+			// ARRANGE
+			seq = 0
+
+			// ACT
 			tc.exec(t)
 		})
 	}
@@ -705,7 +709,7 @@ func TestConvenienceMethods(t *testing.T) {
 				ioReadAll = func(r io.Reader) ([]byte, error) { return nil, readerr }
 
 				// ACT
-				result, err := UnmarshalJSON[map[string]string](ctx, response)
+				result, err := UnmarshalJSON[map[string]string](response)
 
 				// ASSERT
 				test.Error(t, err).Is(readerr)
@@ -718,7 +722,7 @@ func TestConvenienceMethods(t *testing.T) {
 				response := &http.Response{Body: io.NopCloser(bytes.NewReader([]byte("not valid JSON")))}
 
 				// ACT
-				result, err := UnmarshalJSON[map[string]string](ctx, response)
+				result, err := UnmarshalJSON[map[string]string](response)
 
 				// ASSERT
 				test.Error(t, err).Is(ErrInvalidJSON)
@@ -731,24 +735,27 @@ func TestConvenienceMethods(t *testing.T) {
 				response := &http.Response{Body: io.NopCloser(bytes.NewReader([]byte(`{"key":"value"}`)))}
 
 				// ACT
-				result, err := UnmarshalJSON[int](ctx, response)
+				result, err := UnmarshalJSON[int](response)
 
 				// ASSERT
 				test.Error(t, err).Is(ErrInvalidJSON)
-				test.That(t, result).Equals(0)
+				test.That(t, result).IsNil()
 			},
 		},
 		{scenario: "UnmarshalJSON/ok",
 			exec: func(t *testing.T) {
 				// ARRANGE
+				type body struct {
+					Key string `json:"key"`
+				}
 				response := &http.Response{Body: io.NopCloser(bytes.NewReader([]byte(`{"key":"value"}`)))}
 
 				// ACT
-				result, err := UnmarshalJSON[map[string]string](ctx, response)
+				result, err := UnmarshalJSON[body](response)
 
 				// ASSERT
 				test.Error(t, err).Is(nil)
-				test.That(t, result).Equals(map[string]string{"key": "value"})
+				test.That(t, result).Equals(&body{Key: "value"})
 			},
 		},
 	}

mockClient.go

@@ -33,7 +33,7 @@ type MockClient interface {
 // methods for configuring request and response expectations and
 // verifying that those expectations have been met.
 type mockClient struct {
-	name         string
+	id           string
 	hostname     string
 	expectations []*MockRequest
 	unexpected   []*http.Request
@@ -46,7 +46,7 @@ type mockClient struct {
 //
 // # params
 //
-//	name          // used to identify the mock client in test failure reports and errors
+//	id            // used to identify the mock client in test failure reports and errors
 //	wrap          // optional function(s) to wrap the client with some other client
 //	              // implementation, if required; nil functions are ignored
 //
@@ -65,7 +65,7 @@ func NewMockClient(name string, wrap ...func(c interface {
 	Do(*http.Request) (*http.Response, error)
 }) (HttpClient, MockClient) {
 	def := &mockClient{
-		name:     name,
+		id:       name,
 		hostname: "mock://hostname",
 		next:     noExpectedRequests,
 	}
@@ -80,7 +80,8 @@ func NewMockClient(name string, wrap ...func(c interface {
 		mock = wrap(mock)
 	}
 
-	c, _ := NewClient(def.name,
+	c, _ := NewClient(
+		ClientID(def.id),
 		URL(def.hostname),
 		Using(mock),
 	)
@@ -189,7 +190,7 @@ func (mock mockClient) ExpectationsWereMet() error {
 	}
 
 	if len(errs) > 0 {
-		return MockExpectationsError{mock.name, errs}
+		return MockExpectationsError{mock.id, errs}
 	}
 
 	return nil
@@ -207,7 +208,7 @@ func (mock mockClient) ExpectationsWereMet() error {
 func (mock *mockClient) Expect(method string, path string) *MockRequest {
 	if mock.next > 0 {
 		msg := "requests have already been made"
-		panic(fmt.Errorf("%s: %w: %s", mock.name, ErrCannotChangeExpectations, msg))
+		panic(fmt.Errorf("%s: %w: %s", mock.id, ErrCannotChangeExpectations, msg))
 	}
 
 	fqu, err := url.JoinPath(mock.hostname, path)

mockClient_test.go

@@ -36,11 +36,11 @@ func TestNewMockClient(t *testing.T) {
 
 	// ASSERT
 	if c, ok := test.IsType[client](t, c); ok {
-		test.That(t, c.name).Equals("foo")
+		test.That(t, c.id).Equals("foo")
 		test.That(t, c.url).Equals("mock://hostname")
 	}
 	if m, ok := test.IsType[*mockClient](t, m); ok {
-		test.That(t, m.name).Equals("foo")
+		test.That(t, m.id).Equals("foo")
 		test.That(t, m.hostname).Equals("mock://hostname")
 	}
 	test.IsTrue(t, wrappersAreApplied)
@@ -198,7 +198,7 @@ func TestMockClient(t *testing.T) {
 			exec: func(t *testing.T) {
 				// ARRANGE
 				client := &mockClient{
-					name:       "foo",
+					id:         "foo",
 					next:       noExpectedRequests,
 					unexpected: []*http.Request{{Method: http.MethodGet, URL: &url.URL{Scheme: "http", Host: "hostname", Path: "path"}}},
 				}
@@ -219,7 +219,7 @@ func TestMockClient(t *testing.T) {
 			exec: func(t *testing.T) {
 				// ARRANGE
 				client := &mockClient{
-					name:         "foo",
+					id:           "foo",
 					next:         0,
 					expectations: []*MockRequest{{}},
 					unexpected:   []*http.Request{{Method: http.MethodGet, URL: &url.URL{Scheme: "http", Host: "hostname", Path: "path"}}},
@@ -265,7 +265,7 @@ func TestMockClient(t *testing.T) {
 				// ARRANGE
 				m := http.MethodPost
 				client := &mockClient{
-					name: "foo",
+					id: "foo",
 					expectations: []*MockRequest{
 						{
 							isExpected: true,
@@ -297,7 +297,7 @@ func TestMockClient(t *testing.T) {
 			exec: func(t *testing.T) {
 				// ARRANGE
 				client := &mockClient{
-					name: "foo",
+					id: "foo",
 					expectations: []*MockRequest{
 						{
 							isExpected: true,