feat: add options pattern #276

denopink · 2025-01-23T19:15:44Z

client options
- get logger
- store base url
- store api path
- http client
- store bucket
listener options
- set and get logger
- pull interval
- listeners (called at each pull interval)
allow users to define their own db client instead of using argus

anclafx/provide.go

chrysom/client/pushResult.go

denopink · 2025-01-23T19:27:10Z

chrysom/fx.go

-// GetLogger returns a logger from the given context.
-type GetLogger func(context.Context) *zap.Logger
-
-// SetLogger embeds the `Listener.logger` in outgoing request contexts for `Listener.Update` calls.
-type SetLogger func(context.Context, *zap.Logger) context.Context
-
-type BasicClientIn struct {
-	fx.In
-
-	// Ancla Client config.
-	Config BasicClientConfig
-	// GetLogger returns a logger from the given context.
-	GetLogger GetLogger


Most code was moved to ancla/chrysom/listener/fx.go while the ProvideBasicClient related code was moved to ancla/fx.go

denopink · 2025-01-23T19:29:43Z

chrysom/store.go

+type Items []model.Item
+


Better home for Items

denopink · 2025-01-23T19:31:16Z

service.go

@@ -95,7 +92,7 @@ func (s *service) GetAll(ctx context.Context) ([]InternalWebhook, error) {
 }

 // NewService returns an ancla client used to interact with an Argus database.
-func NewService(client *chrysom.BasicClient) *service {


use the chrysom.PushReader interface instead

denopink · 2025-01-23T19:31:36Z

service.go

-	// BasicClientConfig is the configuration for the Argus database client.
-	BasicClientConfig chrysom.BasicClientConfig
-


using client.Options instead

denopink · 2025-01-23T20:13:47Z

chrysom/basicClient.go

-type BasicClientConfig struct {
-	// Address is the Argus URL (i.e. https://example-argus.io:8090)
-	Address string
-


replaced by clientOptions

denopink · 2025-01-23T20:15:09Z

chrysom/basicClient.go

-
-func validateBasicConfig(config *BasicClientConfig) error {
-	if config.Address == "" {
-		return ErrAddressEmpty
-	}
-
-	if config.Bucket == "" {


replaced by clientOptions' validation

denopink · 2025-01-23T20:17:48Z

chrysom/basicClientOptions.go

+			url, err := url.JoinPath(c.storeBaseURL, c.storeAPIPath)
+			if err != nil {
+				return errors.Join(err, ErrMisconfiguredClient)
+			}
+
+			c.storeBaseURL = url


url.JoinPath is safer than using fmt.Sprintf

fyi, validateStoreBaseURL is called after all options are applied

denopink · 2025-01-23T20:19:12Z

chrysom/fx.go

 	}

 	return client, nil
 }

-func ProvideDefaultListenerReader(client *BasicClient) Reader {


renamed to ProvideReaderOption

denopink · 2025-01-23T20:19:36Z

chrysom/listenerClient.go

-type observerConfig struct {
-	listener          Listener


unnecessary struct

denopink · 2025-01-23T20:20:05Z

chrysom/listenerClient.go

 }

 // Start begins listening for updates on an interval given that client configuration
 // is setup correctly. If a listener process is already in progress, calling Start()
 // is a NoOp. If you want to restart the current listener process, call Stop() first.
 func (c *ListenerClient) Start(ctx context.Context) error {
 	logger := c.getLogger(ctx)
-	if c.observer == nil || c.observer.listener == nil {


.observer struct was removed

denopink · 2025-01-23T20:21:53Z

chrysom/store.go

@@ -22,7 +24,7 @@ type Pusher interface {
 	RemoveItem(ctx context.Context, id, owner string) (model.Item, error)
 }

-type Listener interface {


renamed to ListenerInterface because it collided with the listener option Listener

denopink · 2025-01-23T20:22:54Z

chrysom/fx.go

-}
-
-// ProvideBasicClient provides a new BasicClient.
-func ProvideBasicClient(in BasicClientIn) (*BasicClient, error) {


code moved into ProvideService, allowing users to define their own PushReader client instead of using the default argus db client.

denopink · 2025-01-23T20:24:36Z

fx.go

+// ProvideService provides the Argus client service from the given configuration and client options.
+func ProvideService(in ServiceIn) (ProvideServiceOut, error) {
+	// If the user provides a non-nil chrysom.PushReader (their own db client), then use that instead
+	// of an Argus db client.
+	// Otherwise, create and use a new Argus db client.


moved the logic from ProvideBasicClient into here, allowing users to define their own PushReader client instead of using the default argus db client.

- client options - get logger - store base url - store api path - http client - store bucket - listener options - set and get logger - pull interval - listeners (called at each pull interval) - allow users to define their own db client instead of using argus

chrysom/basicClientOptions.go

chrysom/listenerClientOptions.go

schmidtw · 2025-01-24T06:40:18Z

chrysom/listenerClient_test.go

+		Client         ListenerClient
+	}
+
+	tcs := []testCase{


These tests seem a bit overly explicit. I'd probably favor just passing an array of options and confirming the New(opts...) returns an error or not. That makes the code less brittle later.

chrysom/listenerClient_test.go

codecov · 2025-01-31T18:29:42Z

Codecov Report

Attention: Patch coverage is 85.27919% with 29 lines in your changes missing coverage. Please review.

Project coverage is 83.83%. Comparing base (24d29af) to head (1dc9f50).
Report is 5 commits behind head on main.

Files with missing lines	Patch %	Lines
fx.go	0.00%	13 Missing ⚠️
chrysom/fx.go	0.00%	8 Missing ⚠️
chrysom/basicClientOptions.go	95.58%	2 Missing and 1 partial ⚠️
auth/acquire.go	0.00%	2 Missing ⚠️
chrysom/listenerClient.go	92.85%	1 Missing and 1 partial ⚠️
service.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #276      +/-   ##
==========================================
+ Coverage   80.83%   83.83%   +2.99%     
==========================================
  Files          20       23       +3     
  Lines         793      903     +110     
==========================================
+ Hits          641      757     +116     
+ Misses        134      131       -3     
+ Partials       18       15       -3

Flag	Coverage Δ
unittests	`83.83% <85.27%> (+2.99%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

denopink · 2025-02-03T18:43:35Z

auth/acquire.go

+
+func (f DecoratorFunc) Decorate(ctx context.Context, req *http.Request) error { return f(ctx, req) }
+
+var Nop = DecoratorFunc(func(context.Context, *http.Request) error { return nil })


Made Nop public since it's used by our tests and I think it'll be useful for clients when they want to explicitly indicate their ancla client is not configured for auth.

denopink · 2025-02-03T18:47:56Z

anclafx/provide.go

@@ -19,7 +19,6 @@ func Provide() fx.Option {
 			ancla.ProvideListener,
 			ancla.ProvideDefaultListenerWatchers,
 			chrysom.ProvideBasicClient,
-			chrysom.ProvideDefaultListenerReader,


ancla will solely use the argus client until we discover a use case for swappable db clients

chore: cherry pick old options code (starter code) and update ignore

a72aed0