feat: add options pattern to BasicClient

- feature is required because tr1d1um requires the ability to add a custom transport to BasicClient's http client (`HTTPTransport`)
- improve main fx test

Author: denopink

anclafx/provide_test.go

@@ -20,6 +20,7 @@ type out struct {
 
 	Factory           *touchstone.Factory
 	BasicClientConfig chrysom.BasicClientConfig
+	Options           chrysom.Options
 }
 
 func provideDefaults() (out, error) {
@@ -38,25 +39,39 @@ func provideDefaults() (out, error) {
 			Address: "example.com",
 			Bucket:  "bucket-name",
 		},
+		Options: chrysom.Options{
+			chrysom.Address("example.com"),
+			chrysom.Bucket("bucket-name"),
+		},
 	}, nil
 }
 
 func TestProvide(t *testing.T) {
 	t.Run("Test anclafx.Provide() defaults", func(t *testing.T) {
-		var svc *ancla.ClientService
+		var (
+			svc *ancla.ClientService
+			bc  *chrysom.BasicClient
+			l   *chrysom.ListenerClient
+		)
 		app := fxtest.New(t,
 			anclafx.Provide(),
 			fx.Provide(
 				provideDefaults,
 			),
-			fx.Populate(&svc),
+			fx.Populate(
+				&svc,
+				&bc,
+				&l,
+			),
 		)
 
 		require := require.New(t)
 		require.NotNil(app)
 		require.NoError(app.Err())
 		app.RequireStart()
 		require.NotNil(svc)
+		require.NotNil(bc)
+		require.NotNil(l)
 		app.RequireStop()
 	})
 }

chrysom/basicClient.go

@@ -16,12 +16,11 @@ import (
 	"github.com/xmidt-org/ancla/model"
 	"github.com/xmidt-org/arrange/arrangehttp"
 	"github.com/xmidt-org/sallust"
+	"go.uber.org/fx"
 	"go.uber.org/zap"
 )
 
 var (
-	ErrAddressEmpty            = errors.New("argus address is required")
-	ErrBucketEmpty             = errors.New("bucket name is required")
 	ErrItemIDEmpty             = errors.New("item ID is required")
 	ErrItemDataEmpty           = errors.New("data field in item is required")
 	ErrUndefinedIntervalTicker = errors.New("interval ticker is nil. Can't listen for updates")
@@ -91,9 +90,15 @@ const (
 // Items is a slice of model.Item(s) .
 type Items []model.Item
 
+type BasicClientIn struct {
+	fx.In
+
+	Options Options `optional:"true" `
+}
+
 // ProvideBasicClient provides a new BasicClient.
-func ProvideBasicClient(config BasicClientConfig) (*BasicClient, error) {
-	client, err := NewBasicClient(config)
+func ProvideBasicClient(in BasicClientIn) (*BasicClient, error) {
+	client, err := NewBasicClient(in.Options)
 	if err != nil {
 		return nil, errors.Join(errFailedConfig, err)
 	}
@@ -103,23 +108,13 @@ func ProvideBasicClient(config BasicClientConfig) (*BasicClient, error) {
 
 // NewBasicClient creates a new BasicClient that can be used to
 // make requests to Argus.
-func NewBasicClient(config BasicClientConfig) (*BasicClient, error) {
-	err := validateBasicConfig(&config)
-	if err != nil {
-		return nil, err
-	}
+func NewBasicClient(opts Options) (*BasicClient, error) {
+	var client BasicClient
 
-	client, err := config.HTTPClient.NewClient()
-	if err != nil {
-		return nil, err
-	}
+	opts = append(defaultOptions, opts)
+	opts = append(opts, defaultValidateOptions)
 
-	return &BasicClient{
-		client:       client,
-		auth:         config.Auth,
-		bucket:       config.Bucket,
-		storeBaseURL: config.Address + storeAPIPath,
-	}, nil
+	return &client, opts.apply(&client)
 }
 
 // GetItems fetches all items that belong to a given owner.
@@ -263,15 +258,3 @@ func translateNonSuccessStatusCode(code int) error {
 		return errNonSuccessResponse
 	}
 }
-
-func validateBasicConfig(config *BasicClientConfig) error {
-	if config.Address == "" {
-		return ErrAddressEmpty
-	}
-
-	if config.Bucket == "" {
-		return ErrBucketEmpty
-	}
-
-	return nil
-}

chrysom/basicClient_test.go

@@ -18,6 +18,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/xmidt-org/ancla/model"
+	"github.com/xmidt-org/arrange/arrangehttp"
+	"github.com/xmidt-org/arrange/arrangetls"
 )
 
 const failingURL = "nowhere://"
@@ -28,65 +30,132 @@ var (
 	errFails        = errors.New("fails")
 )
 
-func TestValidateBasicConfig(t *testing.T) {
+func TestValidateOptions(t *testing.T) {
 	type testCase struct {
 		Description    string
-		Input          *BasicClientConfig
-		Client         *http.Client
+		ValidateOption Option
+		Client         BasicClient
 		ExpectedErr    error
-		ExpectedConfig *BasicClientConfig
 	}
 
-	allDefaultsCaseConfig := &BasicClientConfig{
-		Address: "example.com",
-		Bucket:  "bucket-name",
+	tcs := []testCase{
+		{
+			Description:    "Nil http client",
+			ValidateOption: validateHTTPClient(),
+			Client:         BasicClient{},
+			ExpectedErr:    ErrHttpClient,
+		},
+		{
+			Description:    "Empty bucket",
+			ValidateOption: validateBucket(),
+			Client:         BasicClient{},
+			ExpectedErr:    ErrBucketEmpty,
+		},
+		{
+			Description:    "Empty store base url",
+			ValidateOption: validateStoreBaseURL(),
+			Client:         BasicClient{},
+			ExpectedErr:    ErrStoreBaseURLEmpty,
+		},
+	}
+
+	for _, tc := range tcs {
+		t.Run(tc.Description, func(t *testing.T) {
+			assert := assert.New(t)
+			err := tc.ValidateOption.apply(&tc.Client)
+			assert.ErrorIs(err, tc.ExpectedErr)
+		})
 	}
-	allDefinedCaseConfig := &BasicClientConfig{
-		Address: "example.com",
-		Bucket:  "amazing-bucket",
+}
+
+func TestValidateBasicConfig(t *testing.T) {
+	type testCase struct {
+		Description string
+		Input       BasicClientConfig
+		Transport   http.RoundTripper
+		Client      *http.Client
+		ExpectedErr error
 	}
 
 	tcs := []testCase{
 		{
 			Description: "No address",
-			Input: &BasicClientConfig{
-				Bucket: "bucket-name",
+			Input: BasicClientConfig{
+				Bucket:     "bucket-name",
+				HTTPClient: arrangehttp.ClientConfig{},
 			},
+			Transport:   http.DefaultTransport,
 			ExpectedErr: ErrAddressEmpty,
 		},
 		{
 			Description: "No bucket",
-			Input: &BasicClientConfig{
-				Address: "example.com",
+			Input: BasicClientConfig{
+				Address:    "example.com",
+				HTTPClient: arrangehttp.ClientConfig{},
 			},
+			Transport:   http.DefaultTransport,
 			ExpectedErr: ErrBucketEmpty,
 		},
 		{
-			Description: "All default values",
-			Input: &BasicClientConfig{
+			Description: "Nil http transport",
+			Input: BasicClientConfig{
+				Address:    "example.com",
+				Bucket:     "bucket-name",
+				HTTPClient: arrangehttp.ClientConfig{},
+			},
+			Transport:   nil,
+			ExpectedErr: ErrHttpTransport,
+		},
+		{
+			Description: "Bad http client config",
+			Input: BasicClientConfig{
 				Address: "example.com",
 				Bucket:  "bucket-name",
+				HTTPClient: arrangehttp.ClientConfig{
+					TLS: &arrangetls.Config{
+						Certificates: arrangetls.ExternalCertificates{arrangetls.ExternalCertificate{CertificateFile: "junk-crt"}},
+					},
+				},
 			},
-			ExpectedConfig: allDefaultsCaseConfig,
+			Transport:   http.DefaultTransport,
+			ExpectedErr: ErrHttpClientConfig,
 		},
 		{
 			Description: "All defined",
-			Input: &BasicClientConfig{
-				Address: "example.com",
-				Bucket:  "amazing-bucket",
+			Input: BasicClientConfig{
+				Address:    "example.com",
+				Bucket:     "amazing-bucket",
+				HTTPClient: arrangehttp.ClientConfig{},
 			},
-			ExpectedConfig: allDefinedCaseConfig,
+			Transport: http.DefaultTransport,
 		},
 	}
 
 	for _, tc := range tcs {
 		t.Run(tc.Description, func(t *testing.T) {
+			opts := append(
+				defaultOptions,
+				Address(tc.Input.Address),
+				Bucket(tc.Input.Bucket),
+				HTTPClient(tc.Input.HTTPClient),
+				HTTPTransport(tc.Transport),
+				defaultValidateOptions,
+			)
+
 			assert := assert.New(t)
-			err := validateBasicConfig(tc.Input)
-			assert.Equal(tc.ExpectedErr, err)
-			if tc.ExpectedErr == nil {
-				assert.Equal(tc.ExpectedConfig, tc.Input)
+			client := BasicClient{}
+			errs := opts.apply(&client)
+			if tc.ExpectedErr != nil {
+				assert.ErrorIs(errs, tc.ExpectedErr)
+
+				return
 			}
+
+			assert.NoError(errs)
+			assert.Equal(tc.Input.Address+storeAPIPath, client.storeBaseURL)
+			assert.Equal(tc.Input.Bucket, client.bucket)
+			assert.Equal(tc.Transport, client.client.Transport)
+			assert.NotNil(client.client)
 		})
 	}
 }
@@ -174,10 +243,12 @@ func TestSendRequest(t *testing.T) {
 			server := httptest.NewServer(echoHandler)
 			defer server.Close()
 
-			client, err := NewBasicClient(BasicClientConfig{
-				Address: "example.com",
-				Bucket:  "bucket-name",
-			})
+			opts := Options{
+				Address("example.com"),
+				Bucket("bucket-name"),
+				HTTPClient(arrangehttp.ClientConfig{}),
+			}
+			client, err := NewBasicClient(opts)
 
 			acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
 			client.auth = acquirer
@@ -285,10 +356,12 @@ func TestGetItems(t *testing.T) {
 				rw.Write(tc.ResponsePayload)
 			}))
 
-			client, err := NewBasicClient(BasicClientConfig{
-				Address: server.URL,
-				Bucket:  bucket,
-			})
+			opts := Options{
+				Address(server.URL),
+				Bucket(bucket),
+				HTTPClient(arrangehttp.ClientConfig{}),
+			}
+			client, err := NewBasicClient(opts)
 
 			require.Nil(err)
 
@@ -440,10 +513,12 @@ func TestPushItem(t *testing.T) {
 				}
 			}))
 
-			client, err := NewBasicClient(BasicClientConfig{
-				Address: server.URL,
-				Bucket:  bucket,
-			})
+			opts := Options{
+				Address(server.URL),
+				Bucket(bucket),
+				HTTPClient(arrangehttp.ClientConfig{}),
+			}
+			client, err := NewBasicClient(opts)
 
 			acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
 			client.auth = acquirer
@@ -552,10 +627,12 @@ func TestRemoveItem(t *testing.T) {
 				rw.Write(tc.ResponsePayload)
 			}))
 
-			client, err := NewBasicClient(BasicClientConfig{
-				Address: server.URL,
-				Bucket:  bucket,
-			})
+			opts := Options{
+				Address(server.URL),
+				Bucket(bucket),
+				HTTPClient(arrangehttp.ClientConfig{}),
+			}
+			client, err := NewBasicClient(opts)
 
 			acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
 			client.auth = acquirer