Skip to content

Commit 4d6420e

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 4d6420e

15 files changed

+347
-407
lines changed

anclafx/provide.go

+10-3
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package anclafx
44

55
import (
66
"github.com/xmidt-org/ancla"
7+
"github.com/xmidt-org/ancla/chrysom"
78
"go.uber.org/fx"
89
)
910

@@ -12,8 +13,14 @@ const Module = "ancla"
1213
func Provide() fx.Option {
1314
return fx.Module(
1415
Module,
15-
ancla.ProvideMetrics(),
16-
ancla.ProvideListener(),
17-
ancla.ProvideService(),
16+
fx.Invoke(chrysom.ProvideStartListenerClient),
17+
fx.Provide(
18+
ancla.ProvideListener,
19+
ancla.ProvideService,
20+
chrysom.ProvideBasicClient,
21+
chrysom.ProvideDefaultListenerReader,
22+
chrysom.ProvideListenerClient,
23+
),
24+
chrysom.ProvideMetrics(),
1825
)
1926
}

anclafx/provide_test.go

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// SPDX-FileCopyrightText: 2024 Comcast Cable Communications Management, LLC
2+
// SPDX-License-Identifier: Apache-2.0
3+
package anclafx_test
4+
5+
import (
6+
"testing"
7+
8+
"github.com/stretchr/testify/require"
9+
"github.com/xmidt-org/ancla"
10+
"github.com/xmidt-org/ancla/anclafx"
11+
"github.com/xmidt-org/ancla/chrysom"
12+
"github.com/xmidt-org/sallust"
13+
"github.com/xmidt-org/touchstone"
14+
"go.uber.org/fx"
15+
"go.uber.org/fx/fxtest"
16+
)
17+
18+
type out struct {
19+
fx.Out
20+
21+
Factory *touchstone.Factory
22+
BasicClientConfig chrysom.BasicClientConfig
23+
}
24+
25+
func provideDefaults() (out, error) {
26+
cfg := touchstone.Config{
27+
DefaultNamespace: "n",
28+
DefaultSubsystem: "s",
29+
}
30+
_, pr, err := touchstone.New(cfg)
31+
if err != nil {
32+
return out{}, err
33+
}
34+
35+
return out{
36+
Factory: touchstone.NewFactory(cfg, sallust.Default(), pr),
37+
BasicClientConfig: chrysom.BasicClientConfig{
38+
Address: "example.com",
39+
Bucket: "bucket-name",
40+
},
41+
}, nil
42+
}
43+
44+
func TestProvide(t *testing.T) {
45+
t.Run("Test anclafx.Provide() defaults", func(t *testing.T) {
46+
var svc *ancla.ClientService
47+
app := fxtest.New(t,
48+
anclafx.Provide(),
49+
fx.Provide(
50+
provideDefaults,
51+
),
52+
fx.Populate(&svc),
53+
)
54+
55+
require := require.New(t)
56+
require.NotNil(app)
57+
require.NoError(app.Err())
58+
app.RequireStart()
59+
require.NotNil(svc)
60+
app.RequireStop()
61+
})
62+
}

chrysom/basicClient.go

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

1516
"github.com/xmidt-org/ancla/model"
17+
"github.com/xmidt-org/arrange/arrangehttp"
1618
"github.com/xmidt-org/sallust"
1719
"go.uber.org/zap"
1820
)
@@ -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,13 +55,16 @@ 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.
5658
// (Optional) Defaults to http.DefaultClient.
57-
HTTPClient *http.Client
59+
HTTPClient arrangehttp.ClientConfig
5860

5961
// Auth provides the mechanism to add auth headers to outgoing requests.
6062
// (Optional) If not provided, no auth headers are added.
6163
Auth Acquirer
64+
65+
// PullInterval is how often listeners should get updates.
66+
// (Optional). Defaults to 5 seconds.
67+
PullInterval time.Duration
6268
}
6369

6470
// BasicClient is the client used to make requests to Argus.
@@ -85,6 +91,16 @@ const (
8591
// Items is a slice of model.Item(s) .
8692
type Items []model.Item
8793

94+
// ProvideBasicClient provides a new BasicClient.
95+
func ProvideBasicClient(config BasicClientConfig) (*BasicClient, error) {
96+
client, err := NewBasicClient(config)
97+
if err != nil {
98+
return nil, errors.Join(errFailedConfig, err)
99+
}
100+
101+
return client, nil
102+
}
103+
88104
// NewBasicClient creates a new BasicClient that can be used to
89105
// make requests to Argus.
90106
func NewBasicClient(config BasicClientConfig) (*BasicClient, error) {
@@ -93,14 +109,17 @@ func NewBasicClient(config BasicClientConfig) (*BasicClient, error) {
93109
return nil, err
94110
}
95111

96-
clientStore := &BasicClient{
97-
client: config.HTTPClient,
112+
client, err := config.HTTPClient.NewClient()
113+
if err != nil {
114+
return nil, err
115+
}
116+
117+
return &BasicClient{
118+
client: client,
98119
auth: config.Auth,
99120
bucket: config.Bucket,
100121
storeBaseURL: config.Address + storeAPIPath,
101-
}
102-
103-
return clientStore, nil
122+
}, nil
104123
}
105124

106125
// GetItems fetches all items that belong to a given owner.
@@ -254,9 +273,5 @@ func validateBasicConfig(config *BasicClientConfig) error {
254273
return ErrBucketEmpty
255274
}
256275

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

chrysom/basicClient_test.go

+34-29
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 *http.Client
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: "example.com",
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: "example.com",
46+
Bucket: "amazing-bucket",
5047
}
5148

5249
tcs := []testCase{
@@ -60,24 +57,23 @@ func TestValidateBasicConfig(t *testing.T) {
6057
{
6158
Description: "No bucket",
6259
Input: &BasicClientConfig{
63-
Address: "http://awesome-argus-hostname.io",
60+
Address: "example.com",
6461
},
6562
ExpectedErr: ErrBucketEmpty,
6663
},
6764
{
6865
Description: "All default values",
6966
Input: &BasicClientConfig{
70-
Address: "http://awesome-argus-hostname.io",
67+
Address: "example.com",
7168
Bucket: "bucket-name",
7269
},
7370
ExpectedConfig: allDefaultsCaseConfig,
7471
},
7572
{
7673
Description: "All defined",
7774
Input: &BasicClientConfig{
78-
Address: "http://legit-argus-hostname.io",
79-
Bucket: "amazing-bucket",
80-
HTTPClient: myAmazingClient,
75+
Address: "example.com",
76+
Bucket: "amazing-bucket",
8177
},
8278
ExpectedConfig: allDefinedCaseConfig,
8379
},
@@ -113,15 +109,15 @@ func TestSendRequest(t *testing.T) {
113109
{
114110
Description: "New Request fails",
115111
Method: "what method?",
116-
URL: "http://argus-hostname.io",
112+
URL: "example.com",
117113
ExpectedErr: errNewRequestFailure,
118114
MockAuth: "",
119115
MockError: nil,
120116
},
121117
{
122118
Description: "Auth acquirer fails",
123119
Method: http.MethodGet,
124-
URL: "http://argus-hostname.io",
120+
URL: "example.com",
125121
MockError: errFails,
126122
MockAuth: "",
127123
ExpectedErr: ErrAuthAcquirerFailure,
@@ -137,7 +133,20 @@ func TestSendRequest(t *testing.T) {
137133
{
138134
Description: "Happy path",
139135
Method: http.MethodPut,
140-
URL: "http://argus-hostname.io",
136+
URL: "example.com",
137+
Body: []byte("testing"),
138+
Owner: "HappyCaseOwner",
139+
ExpectedResponse: response{
140+
Code: http.StatusOK,
141+
Body: []byte("testing"),
142+
},
143+
MockError: nil,
144+
MockAuth: "basic xyz",
145+
},
146+
{
147+
Description: "Happy path with default http client",
148+
Method: http.MethodPut,
149+
URL: "example.com",
141150
Body: []byte("testing"),
142151
Owner: "HappyCaseOwner",
143152
ExpectedResponse: response{
@@ -166,17 +175,16 @@ func TestSendRequest(t *testing.T) {
166175
defer server.Close()
167176

168177
client, err := NewBasicClient(BasicClientConfig{
169-
HTTPClient: server.Client(),
170-
Address: "http://argus-hostname.io",
171-
Bucket: "bucket-name",
178+
Address: "example.com",
179+
Bucket: "bucket-name",
172180
})
173181

174182
acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
175183
client.auth = acquirer
176184

177185
var URL = server.URL
178186
if tc.ClientDoFails {
179-
URL = "http://should-definitely-fail.net"
187+
URL = ""
180188
}
181189

182190
assert.Nil(err)
@@ -278,9 +286,8 @@ func TestGetItems(t *testing.T) {
278286
}))
279287

280288
client, err := NewBasicClient(BasicClientConfig{
281-
HTTPClient: server.Client(),
282-
Address: server.URL,
283-
Bucket: bucket,
289+
Address: server.URL,
290+
Bucket: bucket,
284291
})
285292

286293
require.Nil(err)
@@ -434,9 +441,8 @@ func TestPushItem(t *testing.T) {
434441
}))
435442

436443
client, err := NewBasicClient(BasicClientConfig{
437-
HTTPClient: server.Client(),
438-
Address: server.URL,
439-
Bucket: bucket,
444+
Address: server.URL,
445+
Bucket: bucket,
440446
})
441447

442448
acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)
@@ -547,9 +553,8 @@ func TestRemoveItem(t *testing.T) {
547553
}))
548554

549555
client, err := NewBasicClient(BasicClientConfig{
550-
HTTPClient: server.Client(),
551-
Address: server.URL,
552-
Bucket: bucket,
556+
Address: server.URL,
557+
Bucket: bucket,
553558
})
554559

555560
acquirer.On("Acquire").Return(tc.MockAuth, tc.MockError)

0 commit comments

Comments
 (0)