Skip to content

feat: add options pattern #276

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ report.json
# Ignore the vendor structure
*vendor/

# VSCode
*.code-workspace
.vscode/*
.dev/*

# Ignore the various build artifacts
.ignore

Expand Down
1 change: 0 additions & 1 deletion anclafx/provide.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ func Provide() fx.Option {
ancla.ProvideListener,
ancla.ProvideDefaultListenerWatchers,
chrysom.ProvideBasicClient,
chrysom.ProvideDefaultListenerReader,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

chrysom.ProvideListenerClient,
),
chrysom.ProvideMetrics(),
Expand Down
31 changes: 14 additions & 17 deletions anclafx/provide_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
package anclafx_test

import (
"context"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -19,10 +18,9 @@ import (
type out struct {
fx.Out

Factory *touchstone.Factory
BasicClientConfig chrysom.BasicClientConfig
GetLogger chrysom.GetLogger
SetLogger chrysom.SetLogger
Factory *touchstone.Factory
ClientOptions chrysom.ClientOptions `group:"client_options,flatten"`
ListenerOptions chrysom.ListenerOptions `group:"listener_options,flatten"`
}

func provideDefaults() (out, error) {
Expand All @@ -37,21 +35,20 @@ func provideDefaults() (out, error) {

return out{
Factory: touchstone.NewFactory(cfg, zap.NewNop(), pr),
BasicClientConfig: chrysom.BasicClientConfig{
Address: "example.com",
Bucket: "bucket-name",
ClientOptions: chrysom.ClientOptions{
chrysom.Bucket("bucket-name"),
},
GetLogger: func(context.Context) *zap.Logger { return zap.NewNop() },
SetLogger: func(context.Context, *zap.Logger) context.Context { return context.Background() },
// Listener has no required options
ListenerOptions: chrysom.ListenerOptions{},
}, nil
}

func TestProvide(t *testing.T) {
t.Run("Test anclafx.Provide() defaults", func(t *testing.T) {
var (
svc ancla.Service
bc *chrysom.BasicClient
l *chrysom.ListenerClient
svc ancla.Service
pushReader chrysom.PushReader
listener *chrysom.ListenerClient
)

app := fxtest.New(t,
Expand All @@ -61,8 +58,8 @@ func TestProvide(t *testing.T) {
),
fx.Populate(
&svc,
&bc,
&l,
&pushReader,
&listener,
),
)

Expand All @@ -71,8 +68,8 @@ func TestProvide(t *testing.T) {
require.NoError(app.Err())
app.RequireStart()
require.NotNil(svc)
require.NotNil(bc)
require.NotNil(l)
require.NotNil(pushReader)
require.NotNil(listener)
app.RequireStop()
})
}
6 changes: 6 additions & 0 deletions auth/acquire.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,9 @@
// Decorate decorates the given http request with authorization header(s).
Decorate(ctx context.Context, req *http.Request) error
}

type DecoratorFunc func(context.Context, *http.Request) error

func (f DecoratorFunc) Decorate(ctx context.Context, req *http.Request) error { return f(ctx, req) }

Check warning on line 19 in auth/acquire.go

View check run for this annotation

Codecov / codecov/patch

auth/acquire.go#L19

Added line #L19 was not covered by tests

var Nop = DecoratorFunc(func(context.Context, *http.Request) error { return nil })

Check warning on line 21 in auth/acquire.go

View check run for this annotation

Codecov / codecov/patch

auth/acquire.go#L21

Added line #L21 was not covered by tests
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

37 changes: 37 additions & 0 deletions auth/context_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// SPDX-FileCopyrightText: 2025 Comcast Cable Communications Management, LLC
// SPDX-License-Identifier: Apache-2.0

package auth

import (
"context"
"testing"

"github.com/stretchr/testify/assert"
)

func TestPrincipal(t *testing.T) {
t.Run("Test SetPartnerIDs, GetPartnerIDs", func(t *testing.T) {
assert := assert.New(t)
partnerIDs := []string{"foo", "bar"}
ctx := SetPartnerIDs(context.Background(), partnerIDs)
actualPartnerIDs, ok := GetPartnerIDs(ctx)
assert.True(ok)
assert.Equal(partnerIDs, actualPartnerIDs)
actualPartnerIDs, ok = GetPartnerIDs(context.Background())
assert.False(ok)
var empty []string
assert.Equal(empty, actualPartnerIDs)
})
t.Run("Test SetPrincipal, GetPrincipal", func(t *testing.T) {
assert := assert.New(t)
principal := "foo"
ctx := SetPrincipal(context.Background(), principal)
actualPrincipal, ok := GetPrincipal(ctx)
assert.True(ok)
assert.Equal(principal, actualPrincipal)
actualPrincipal, ok = GetPrincipal(context.Background())
assert.False(ok)
assert.Equal("", actualPrincipal)
})
}
101 changes: 30 additions & 71 deletions chrysom/basicClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (
"fmt"
"io"
"net/http"
"time"

"github.com/xmidt-org/ancla/auth"
"github.com/xmidt-org/ancla/model"
Expand All @@ -25,53 +24,28 @@ const (
)

var (
ErrNilMeasures = errors.New("measures cannot be nil")
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")
ErrAuthDecoratorFailure = errors.New("failed decorating auth header")
ErrBadRequest = errors.New("argus rejected the request as invalid")
ErrItemIDEmpty = errors.New("item ID is required")
ErrItemDataEmpty = errors.New("data field in item is required")
ErrAuthDecoratorFailure = errors.New("failed decorating auth header")
ErrBadRequest = errors.New("argus rejected the request as invalid")
)

var (
errNonSuccessResponse = errors.New("argus responded with a non-success status code")
errNewRequestFailure = errors.New("failed creating an HTTP request")
errDoRequestFailure = errors.New("http client failed while sending request")
errReadingBodyFailure = errors.New("failed while reading http response body")
errJSONUnmarshal = errors.New("failed unmarshaling JSON response payload")
errJSONMarshal = errors.New("failed marshaling item as JSON payload")
errFailedConfig = errors.New("ancla configuration error")
ErrFailedAuthentication = errors.New("failed to authentication with argus")
errNonSuccessResponse = errors.New("argus responded with a non-success status code")
errNewRequestFailure = errors.New("failed creating an HTTP request")
errDoRequestFailure = errors.New("http client failed while sending request")
errReadingBodyFailure = errors.New("failed while reading http response body")
errJSONUnmarshal = errors.New("failed unmarshaling JSON response payload")
errJSONMarshal = errors.New("failed marshaling item as JSON payload")
)

// BasicClientConfig contains config data for the client that will be used to
// make requests to the Argus client.
type BasicClientConfig struct {
// Address is the Argus URL (i.e. https://example-argus.io:8090)
Address string

Comment on lines -50 to -53
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced by clientOptions

// Bucket partition to be used by this client.
Bucket string

// HTTPClient refers to the client that will be used to send requests.
// (Optional) Defaults to http.DefaultClient.
HTTPClient *http.Client

// Auth provides the mechanism to add auth headers to outgoing requests.
// (Optional) If not provided, no auth headers are added.
Auth auth.Decorator

// PullInterval is how often listeners should get updates.
// (Optional). Defaults to 5 seconds.
PullInterval time.Duration
}

// BasicClient is the client used to make requests to Argus.
type BasicClient struct {
client *http.Client
auth auth.Decorator
storeBaseURL string
storeAPIPath string
bucket string
getLogger func(context.Context) *zap.Logger
}
Expand All @@ -83,31 +57,32 @@ type response struct {
}

const (
storeAPIPath = "/api/v1/store"
storeV1APIPath = "/api/v1/store"
errWrappedFmt = "%w: %s"
errStatusCodeFmt = "%w: received status %v"
errorHeaderKey = "errorHeader"
)

// Items is a slice of model.Item(s) .
type Items []model.Item

// NewBasicClient creates a new BasicClient that can be used to
// make requests to Argus.
func NewBasicClient(config BasicClientConfig,
getLogger func(context.Context) *zap.Logger) (*BasicClient, error) {
err := validateBasicConfig(&config)
if err != nil {
return nil, err
}
func NewBasicClient(opts ...ClientOption) (*BasicClient, error) {
var (
client BasicClient
defaultClientOptions = ClientOptions{
// localhost defaults
StoreBaseURL(""),
StoreAPIPath(""),
// Nop defaults
HTTPClient(nil),
GetClientLogger(nil),
Auth(nil),
}
)

return &BasicClient{
client: config.HTTPClient,
auth: config.Auth,
bucket: config.Bucket,
storeBaseURL: config.Address + storeAPIPath,
getLogger: getLogger,
}, nil
opts = append(defaultClientOptions, opts...)
opts = append(opts, clientValidator())

return &client, ClientOptions(opts).apply(&client)
}

// GetItems fetches all items that belong to a given owner.
Expand Down Expand Up @@ -251,19 +226,3 @@ func translateNonSuccessStatusCode(code int) error {
return errNonSuccessResponse
}
}

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

if config.Bucket == "" {
Comment on lines -254 to -260
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

replaced by clientOptions' validation

return ErrBucketEmpty
}

if config.HTTPClient == nil {
config.HTTPClient = http.DefaultClient
}

return nil
}
Loading