Skip to content

Commit bcd326d

Browse files
committed
refactor: simplify inner dependences and structure for easier setup with and without fx
- add ability for clients to setup an ancla client with only `anclafx.Provide()` - simplify calls to setup ancla without fx - simplify inner dependences and structure
1 parent aa0ca33 commit bcd326d

11 files changed

+245
-329
lines changed

anclafx/provide.go

+23-3
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@
33
package anclafx
44

55
import (
6+
"github.com/prometheus/client_golang/prometheus"
67
"github.com/xmidt-org/ancla"
8+
"github.com/xmidt-org/ancla/chrysom"
9+
"github.com/xmidt-org/touchstone"
710
"go.uber.org/fx"
811
)
912

@@ -12,8 +15,25 @@ const Module = "ancla"
1215
func Provide() fx.Option {
1316
return fx.Module(
1417
Module,
15-
ancla.ProvideMetrics(),
16-
ancla.ProvideListener(),
17-
ancla.ProvideService(),
18+
fx.Invoke(chrysom.ProvideStartListenerClient),
19+
fx.Provide(
20+
ancla.ProvideListener,
21+
ancla.ProvideService,
22+
chrysom.ProvideBasicClient,
23+
chrysom.ProvideDefaultListenerReader,
24+
chrysom.ProvideListenerClient,
25+
),
26+
touchstone.Gauge(
27+
prometheus.GaugeOpts{
28+
Name: chrysom.WebhookListSizeGaugeName,
29+
Help: chrysom.WebhookListSizeGaugeHelp,
30+
}),
31+
touchstone.CounterVec(
32+
prometheus.CounterOpts{
33+
Name: chrysom.PollsTotalCounterName,
34+
Help: chrysom.PollsTotalCounterHelp,
35+
},
36+
chrysom.OutcomeLabel,
37+
),
1838
)
1939
}

chrysom/basicClient.go

+38-15
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,11 @@ import (
1111
"fmt"
1212
"io"
1313
"net/http"
14+
"time"
1415

1516
"github.com/xmidt-org/ancla/model"
1617
"github.com/xmidt-org/sallust"
18+
"go.uber.org/fx"
1719
"go.uber.org/zap"
1820
)
1921

@@ -35,6 +37,7 @@ var (
3537
errReadingBodyFailure = errors.New("failed while reading http response body")
3638
errJSONUnmarshal = errors.New("failed unmarshaling JSON response payload")
3739
errJSONMarshal = errors.New("failed marshaling item as JSON payload")
40+
errFailedConfig = errors.New("ancla configuration error")
3841
)
3942

4043
const (
@@ -52,18 +55,18 @@ type BasicClientConfig struct {
5255
// Bucket partition to be used by this client.
5356
Bucket string
5457

55-
// HTTPClient refers to the client that will be used to send requests.
56-
// (Optional) Defaults to http.DefaultClient.
57-
HTTPClient *http.Client
58-
5958
// Auth provides the mechanism to add auth headers to outgoing requests.
6059
// (Optional) If not provided, no auth headers are added.
6160
Auth Acquirer
61+
62+
// PullInterval is how often listeners should get updates.
63+
// (Optional). Defaults to 5 seconds.
64+
PullInterval time.Duration
6265
}
6366

6467
// BasicClient is the client used to make requests to Argus.
6568
type BasicClient struct {
66-
client *http.Client
69+
client HTTPClient
6770
auth Acquirer
6871
storeBaseURL string
6972
bucket string
@@ -85,22 +88,46 @@ const (
8588
// Items is a slice of model.Item(s) .
8689
type Items []model.Item
8790

91+
type HTTPClient interface {
92+
Do(*http.Request) (*http.Response, error)
93+
}
94+
95+
type BasicClientIn struct {
96+
fx.In
97+
98+
BasicClientConfig BasicClientConfig
99+
// (Optional) Defaults to http.DefaultClient.
100+
HTTPClient HTTPClient
101+
}
102+
103+
// ProvideBasicClient provides a new BasicClient.
104+
func ProvideBasicClient(in BasicClientIn) (*BasicClient, error) {
105+
client, err := NewBasicClient(in.BasicClientConfig, in.HTTPClient)
106+
if err != nil {
107+
return nil, errors.Join(errFailedConfig, err)
108+
}
109+
110+
return client, nil
111+
}
112+
88113
// NewBasicClient creates a new BasicClient that can be used to
89114
// make requests to Argus.
90-
func NewBasicClient(config BasicClientConfig) (*BasicClient, error) {
115+
func NewBasicClient(config BasicClientConfig, client HTTPClient) (*BasicClient, error) {
91116
err := validateBasicConfig(&config)
92117
if err != nil {
93118
return nil, err
94119
}
95120

96-
clientStore := &BasicClient{
97-
client: config.HTTPClient,
121+
if client == nil {
122+
client = http.DefaultClient
123+
}
124+
125+
return &BasicClient{
126+
client: client,
98127
auth: config.Auth,
99128
bucket: config.Bucket,
100129
storeBaseURL: config.Address + storeAPIPath,
101-
}
102-
103-
return clientStore, nil
130+
}, nil
104131
}
105132

106133
// GetItems fetches all items that belong to a given owner.
@@ -254,9 +281,5 @@ func validateBasicConfig(config *BasicClientConfig) error {
254281
return ErrBucketEmpty
255282
}
256283

257-
if config.HTTPClient == nil {
258-
config.HTTPClient = http.DefaultClient
259-
}
260-
261284
return nil
262285
}

chrysom/basicClient_test.go

+39-27
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"net/http"
1414
"net/http/httptest"
1515
"testing"
16-
"time"
1716

1817
"github.com/aws/aws-sdk-go/aws"
1918
"github.com/stretchr/testify/assert"
@@ -33,20 +32,18 @@ func TestValidateBasicConfig(t *testing.T) {
3332
type testCase struct {
3433
Description string
3534
Input *BasicClientConfig
35+
Client HTTPClient
3636
ExpectedErr error
3737
ExpectedConfig *BasicClientConfig
3838
}
3939

4040
allDefaultsCaseConfig := &BasicClientConfig{
41-
HTTPClient: http.DefaultClient,
42-
Address: "http://awesome-argus-hostname.io",
43-
Bucket: "bucket-name",
41+
Address: "http://awesome-argus-hostname.io",
42+
Bucket: "bucket-name",
4443
}
45-
myAmazingClient := &http.Client{Timeout: time.Hour}
4644
allDefinedCaseConfig := &BasicClientConfig{
47-
HTTPClient: myAmazingClient,
48-
Address: "http://legit-argus-hostname.io",
49-
Bucket: "amazing-bucket",
45+
Address: "http://legit-argus-hostname.io",
46+
Bucket: "amazing-bucket",
5047
}
5148

5249
tcs := []testCase{
@@ -75,9 +72,8 @@ func TestValidateBasicConfig(t *testing.T) {
7572
{
7673
Description: "All defined",
7774
Input: &BasicClientConfig{
78-
Address: "http://legit-argus-hostname.io",
79-
Bucket: "amazing-bucket",
80-
HTTPClient: myAmazingClient,
75+
Address: "http://legit-argus-hostname.io",
76+
Bucket: "amazing-bucket",
8177
},
8278
ExpectedConfig: allDefinedCaseConfig,
8379
},
@@ -103,6 +99,7 @@ func TestSendRequest(t *testing.T) {
10399
URL string
104100
Body []byte
105101
ClientDoFails bool
102+
NilHTTPClient bool
106103
ExpectedResponse response
107104
ExpectedErr error
108105
MockError error
@@ -147,6 +144,20 @@ func TestSendRequest(t *testing.T) {
147144
MockError: nil,
148145
MockAuth: "basic xyz",
149146
},
147+
{
148+
Description: "Happy path with default http client",
149+
Method: http.MethodPut,
150+
URL: "http://argus-hostname.io",
151+
Body: []byte("testing"),
152+
Owner: "HappyCaseOwner",
153+
ExpectedResponse: response{
154+
Code: http.StatusOK,
155+
Body: []byte("testing"),
156+
},
157+
MockError: nil,
158+
MockAuth: "basic xyz",
159+
NilHTTPClient: true,
160+
},
150161
}
151162
for _, tc := range tcs {
152163
t.Run(tc.Description, func(t *testing.T) {
@@ -165,11 +176,15 @@ func TestSendRequest(t *testing.T) {
165176
server := httptest.NewServer(echoHandler)
166177
defer server.Close()
167178

179+
var httpClient HTTPClient = server.Client()
180+
if tc.NilHTTPClient {
181+
httpClient = nil
182+
}
183+
168184
client, err := NewBasicClient(BasicClientConfig{
169-
HTTPClient: server.Client(),
170-
Address: "http://argus-hostname.io",
171-
Bucket: "bucket-name",
172-
})
185+
Address: "http://argus-hostname.io",
186+
Bucket: "bucket-name",
187+
}, httpClient)
173188

174189
acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
175190
client.auth = acquirer
@@ -278,10 +293,9 @@ func TestGetItems(t *testing.T) {
278293
}))
279294

280295
client, err := NewBasicClient(BasicClientConfig{
281-
HTTPClient: server.Client(),
282-
Address: server.URL,
283-
Bucket: bucket,
284-
})
296+
Address: server.URL,
297+
Bucket: bucket,
298+
}, server.Client())
285299

286300
require.Nil(err)
287301

@@ -434,10 +448,9 @@ func TestPushItem(t *testing.T) {
434448
}))
435449

436450
client, err := NewBasicClient(BasicClientConfig{
437-
HTTPClient: server.Client(),
438-
Address: server.URL,
439-
Bucket: bucket,
440-
})
451+
Address: server.URL,
452+
Bucket: bucket,
453+
}, server.Client())
441454

442455
acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
443456
client.auth = acquirer
@@ -547,10 +560,9 @@ func TestRemoveItem(t *testing.T) {
547560
}))
548561

549562
client, err := NewBasicClient(BasicClientConfig{
550-
HTTPClient: server.Client(),
551-
Address: server.URL,
552-
Bucket: bucket,
553-
})
563+
Address: server.URL,
564+
Bucket: bucket,
565+
}, server.Client())
554566

555567
acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
556568
client.auth = acquirer

0 commit comments

Comments
 (0)