Skip to content

Commit

Permalink
chore: update based on pr feedback (move fx code into fx.go & test up…
Browse files Browse the repository at this point in the history
…date)

- move fx code into fx.go
- update test defaults for baseClient_test.go
  • Loading branch information
denopink committed Jan 15, 2025
1 parent 2f1165a commit 0d34eeb
Show file tree
Hide file tree
Showing 6 changed files with 178 additions and 159 deletions.
26 changes: 0 additions & 26 deletions chrysom/basicClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (

"github.com/xmidt-org/ancla/auth"
"github.com/xmidt-org/ancla/model"
"go.uber.org/fx"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -93,31 +92,6 @@ const (
// Items is a slice of model.Item(s) .
type Items []model.Item

// 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
}

// ProvideBasicClient provides a new BasicClient.
func ProvideBasicClient(in BasicClientIn) (*BasicClient, error) {
client, err := NewBasicClient(in.Config, in.GetLogger)
if err != nil {
return nil, errors.Join(errFailedConfig, err)
}

return client, nil
}

// NewBasicClient creates a new BasicClient that can be used to
// make requests to Argus.
func NewBasicClient(config BasicClientConfig,
Expand Down
36 changes: 14 additions & 22 deletions chrysom/basicClient_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,34 +54,30 @@ func TestValidateBasicConfig(t *testing.T) {
{
Description: "No address",
Input: &BasicClientConfig{
HTTPClient: http.DefaultClient,
Bucket: "bucket-name",
Bucket: "bucket-name",
},
ExpectedErr: ErrAddressEmpty,
},
{
Description: "No bucket",
Input: &BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: "example.com",
Address: "example.com",
},
ExpectedErr: ErrBucketEmpty,
},
{
Description: "All default values",
Input: &BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: "example.com",
Bucket: "bucket-name",
Address: "example.com",
Bucket: "bucket-name",
},
ExpectedConfig: allDefaultsCaseConfig,
},
{
Description: "All defined",
Input: &BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: "example.com",
Bucket: "amazing-bucket",
Address: "example.com",
Bucket: "amazing-bucket",
},
ExpectedConfig: allDefinedCaseConfig,
},
Expand Down Expand Up @@ -188,9 +184,8 @@ func TestSendRequest(t *testing.T) {
defer server.Close()

client, err := NewBasicClient(BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: "example.com",
Bucket: "bucket-name",
Address: "example.com",
Bucket: "bucket-name",
},
func(context.Context) *zap.Logger {
return zap.NewNop()
Expand Down Expand Up @@ -300,9 +295,8 @@ func TestGetItems(t *testing.T) {
}))

client, err := NewBasicClient(BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: server.URL,
Bucket: bucket,
Address: server.URL,
Bucket: bucket,
},
func(context.Context) *zap.Logger {
return zap.NewNop()
Expand Down Expand Up @@ -446,9 +440,8 @@ func TestPushItem(t *testing.T) {
}))

client, err := NewBasicClient(BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: server.URL,
Bucket: bucket,
Address: server.URL,
Bucket: bucket,
},
func(context.Context) *zap.Logger {
return zap.NewNop()
Expand Down Expand Up @@ -554,9 +547,8 @@ func TestRemoveItem(t *testing.T) {
}))

client, err := NewBasicClient(BasicClientConfig{
HTTPClient: http.DefaultClient,
Address: server.URL,
Bucket: bucket,
Address: server.URL,
Bucket: bucket,
}, func(context.Context) *zap.Logger {
return zap.NewNop()
})
Expand Down
87 changes: 87 additions & 0 deletions chrysom/fx.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// SPDX-FileCopyrightText: 2025 Comcast Cable Communications Management, LLC
// SPDX-License-Identifier: Apache-2.0

package chrysom

import (
"context"
"errors"

"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx"
"go.uber.org/zap"
)

// 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
}

// ProvideBasicClient provides a new BasicClient.
func ProvideBasicClient(in BasicClientIn) (*BasicClient, error) {
client, err := NewBasicClient(in.Config, in.GetLogger)
if err != nil {
return nil, errors.Join(errFailedConfig, err)
}

Check warning on line 35 in chrysom/fx.go

View check run for this annotation

Codecov / codecov/patch

chrysom/fx.go#L31-L35

Added lines #L31 - L35 were not covered by tests

return client, nil

Check warning on line 37 in chrysom/fx.go

View check run for this annotation

Codecov / codecov/patch

chrysom/fx.go#L37

Added line #L37 was not covered by tests
}

// ListenerConfig contains config data for polling the Argus client.
type ListenerClientIn struct {
fx.In

// Listener fetches a copy of all items within a bucket on
// an interval based on `BasicClientConfig.PullInterval`.
// (Optional). If not provided, listening won't be enabled for this client.
Listener Listener
// Config configures the ancla client and its listeners.
Config BasicClientConfig
// PollsTotalCounter measures the number of polls (and their success/failure outcomes) to fetch new items.
PollsTotalCounter *prometheus.CounterVec `name:"chrysom_polls_total"`
// Reader is the DB interface used to fetch new items using `GeItems`.
Reader Reader
// GetLogger returns a logger from the given context.
GetLogger GetLogger
// SetLogger embeds the `Listener.logger` in outgoing request contexts for `Listener.Update` calls.
SetLogger SetLogger
}

// ProvideListenerClient provides a new ListenerClient.
func ProvideListenerClient(in ListenerClientIn) (*ListenerClient, error) {
client, err := NewListenerClient(in.Listener, in.GetLogger, in.SetLogger, in.Config.PullInterval, in.PollsTotalCounter, in.Reader)
if err != nil {
return nil, errors.Join(err, errFailedConfig)
}

Check warning on line 65 in chrysom/fx.go

View check run for this annotation

Codecov / codecov/patch

chrysom/fx.go#L61-L65

Added lines #L61 - L65 were not covered by tests

return client, nil

Check warning on line 67 in chrysom/fx.go

View check run for this annotation

Codecov / codecov/patch

chrysom/fx.go#L67

Added line #L67 was not covered by tests
}

func ProvideDefaultListenerReader(client *BasicClient) Reader {
return client

Check warning on line 71 in chrysom/fx.go

View check run for this annotation

Codecov / codecov/patch

chrysom/fx.go#L70-L71

Added lines #L70 - L71 were not covered by tests
}

type StartListenerIn struct {
fx.In

Listener *ListenerClient
LC fx.Lifecycle
}

// ProvideStartListenerClient starts the Argus listener client service.
func ProvideStartListenerClient(in StartListenerIn) error {
in.Listener.Start(context.Background())
in.LC.Append(fx.StopHook(in.Listener.Stop))

return nil

Check warning on line 86 in chrysom/fx.go

View check run for this annotation

Codecov / codecov/patch

chrysom/fx.go#L82-L86

Added lines #L82 - L86 were not covered by tests
}
50 changes: 0 additions & 50 deletions chrysom/listenerClient.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"time"

"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx"
"go.uber.org/zap"
)

Expand Down Expand Up @@ -38,26 +37,6 @@ const (
defaultPullInterval = time.Second * 5
)

// ListenerConfig contains config data for polling the Argus client.
type ListenerClientIn struct {
fx.In

// Listener fetches a copy of all items within a bucket on
// an interval based on `BasicClientConfig.PullInterval`.
// (Optional). If not provided, listening won't be enabled for this client.
Listener Listener
// Config configures the ancla client and its listeners.
Config BasicClientConfig
// PollsTotalCounter measures the number of polls (and their success/failure outcomes) to fetch new items.
PollsTotalCounter *prometheus.CounterVec `name:"chrysom_polls_total"`
// Reader is the DB interface used to fetch new items using `GeItems`.
Reader Reader
// GetLogger returns a logger from the given context.
GetLogger GetLogger
// SetLogger embeds the `Listener.logger` in outgoing request contexts for `Listener.Update` calls.
SetLogger SetLogger
}

// ListenerClient is the client used to poll Argus for updates.
type ListenerClient struct {
observer *observerConfig
Expand All @@ -76,20 +55,6 @@ type observerConfig struct {
state int32
}

// ProvideListenerClient provides a new ListenerClient.
func ProvideListenerClient(in ListenerClientIn) (*ListenerClient, error) {
client, err := NewListenerClient(in.Listener, in.GetLogger, in.SetLogger, in.Config.PullInterval, in.PollsTotalCounter, in.Reader)
if err != nil {
return nil, errors.Join(err, errFailedConfig)
}

return client, nil
}

func ProvideDefaultListenerReader(client *BasicClient) Reader {
return client
}

// NewListenerClient creates a new ListenerClient to be used to poll Argus
// for updates.
func NewListenerClient(listener Listener,
Expand Down Expand Up @@ -188,18 +153,3 @@ func (c *ListenerClient) Stop(ctx context.Context) error {
atomic.SwapInt32(&c.observer.state, stopped)
return nil
}

type StartListenerIn struct {
fx.In

Listener *ListenerClient
LC fx.Lifecycle
}

// ProvideStartListenerClient starts the Argus listener client service.
func ProvideStartListenerClient(in StartListenerIn) error {
in.Listener.Start(context.Background())
in.LC.Append(fx.StopHook(in.Listener.Stop))

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

package ancla

import (
"github.com/xmidt-org/ancla/chrysom"

"github.com/prometheus/client_golang/prometheus"
"go.uber.org/fx"
)

type ServiceIn struct {
fx.In

// Ancla Client.
BasicClient *chrysom.BasicClient
}

// ProvideService builds the Argus client service from the given configuration.
func ProvideService(in ServiceIn) Service {
return NewService(in.BasicClient)

Check warning on line 22 in fx.go

View check run for this annotation

Codecov / codecov/patch

fx.go#L21-L22

Added lines #L21 - L22 were not covered by tests
}

// TODO: Refactor and move Watch and Listener related code to chrysom.
type DefaultListenersIn struct {
fx.In

// Metric for webhook list size, used by the webhook list size watcher listener.
WebhookListSizeGauge prometheus.Gauge `name:"webhook_list_size"`
}

type DefaultListenerOut struct {
fx.Out

Watchers []Watch `group:"watchers,flatten"`
}

func ProvideDefaultListenerWatchers(in DefaultListenersIn) DefaultListenerOut {
var watchers []Watch

watchers = append(watchers, webhookListSizeWatch(in.WebhookListSizeGauge))

return DefaultListenerOut{
Watchers: watchers,
}

Check warning on line 46 in fx.go

View check run for this annotation

Codecov / codecov/patch

fx.go#L39-L46

Added lines #L39 - L46 were not covered by tests
}

type ListenerIn struct {
fx.In

Shutdowner fx.Shutdowner
// Watchers are called by the Listener when new webhooks are fetched.
Watchers []Watch `group:"watchers"`
}

func ProvideListener(in ListenerIn) chrysom.Listener {
return chrysom.ListenerFunc(func(items chrysom.Items) {
iws, err := ItemsToInternalWebhooks(items)
if err != nil {
in.Shutdowner.Shutdown(fx.ExitCode(1))

return
}

Check warning on line 64 in fx.go

View check run for this annotation

Codecov / codecov/patch

fx.go#L57-L64

Added lines #L57 - L64 were not covered by tests

for _, watch := range in.Watchers {
watch.Update(iws)
}

Check warning on line 68 in fx.go

View check run for this annotation

Codecov / codecov/patch

fx.go#L66-L68

Added lines #L66 - L68 were not covered by tests
})
}
Loading

0 comments on commit 0d34eeb

Please sign in to comment.