Skip to content

Commit ba2c99d

Browse files
aThorp96aThorp96
committed
refactor: change providers' inner Client privacy to private
Requiring the client object be accessed via an accessor method allows for hooking into API usage for things like API metrics, since the github-client library API is designed in a way difficult to decorate.
1 parent 1f2a957 commit ba2c99d

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

61 files changed

+362
-315
lines changed

Diff for: ‎pkg/matcher/annotation_matcher_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -1350,9 +1350,9 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
13501350
fakeclient, mux, ghTestServerURL, teardown := ghtesthelper.SetupGH()
13511351
defer teardown()
13521352
vcx := &ghprovider.Provider{
1353-
Client: fakeclient,
1354-
Token: github.Ptr("None"),
1353+
Token: github.Ptr("None"),
13551354
}
1355+
vcx.SetGithubClient(fakeclient)
13561356
if tt.args.runevent.Request == nil {
13571357
tt.args.runevent.Request = &info.Request{Header: http.Header{}, Payload: nil}
13581358
}

Diff for: ‎pkg/pipelineascode/match_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -254,10 +254,10 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
254254
ConsoleURL: "https://console.url",
255255
}
256256
vcx := &ghprovider.Provider{
257-
Client: fakeclient,
258257
Token: github.Ptr("None"),
259258
Logger: logger,
260259
}
260+
vcx.SetGithubClient(fakeclient)
261261
pacInfo := &info.PacOpts{
262262
Settings: settings.Settings{
263263
ApplicationName: "Pipelines as Code CI",

Diff for: ‎pkg/pipelineascode/pipelineascode_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -606,11 +606,11 @@ func TestRun(t *testing.T) {
606606
},
607607
}
608608
vcx := &ghprovider.Provider{
609-
Client: fakeclient,
610609
Run: cs,
611610
Token: github.Ptr("None"),
612611
Logger: logger,
613612
}
613+
vcx.SetGithubClient(fakeclient)
614614
vcx.SetPacInfo(pacInfo)
615615
p := NewPacs(&tt.runevent, vcx, cs, pacInfo, k8int, logger, nil)
616616
err := p.Run(ctx)

Diff for: ‎pkg/provider/bitbucketcloud/acl.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ func (v *Provider) IsAllowed(ctx context.Context, event *info.Event) (bool, erro
2727
}
2828

2929
func (v *Provider) isWorkspaceMember(event *info.Event) (bool, error) {
30-
members, err := v.Client.Workspaces.Members(event.Organization)
30+
members, err := v.Client().Workspaces.Members(event.Organization)
3131
if err != nil {
3232
return false, err
3333
}
@@ -83,7 +83,7 @@ func (v *Provider) checkMember(ctx context.Context, event *info.Event) (bool, er
8383
}
8484

8585
func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, event *info.Event) (bool, error) {
86-
commentsIntf, err := v.Client.Repositories.PullRequests.GetComments(&bitbucket.PullRequestsOptions{
86+
commentsIntf, err := v.Client().Repositories.PullRequests.GetComments(&bitbucket.PullRequestsOptions{
8787
Owner: event.Organization,
8888
RepoSlug: event.Repository,
8989
ID: strconv.Itoa(event.PullRequestNumber),

Diff for: ‎pkg/provider/bitbucketcloud/acl_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ func TestIsAllowed(t *testing.T) {
179179
bbcloudtest.MuxComments(t, mux, tt.event, tt.fields.comments)
180180
bbcloudtest.MuxFiles(t, mux, tt.event, tt.fields.filescontents, "")
181181

182-
v := &Provider{Client: bbclient}
182+
v := &Provider{bbClient: bbclient}
183183
got, err := v.IsAllowed(ctx, tt.event)
184184
if (err != nil) != tt.wantErr {
185185
t.Errorf("Provider.IsAllowed() error = %v, wantErr %v", err, tt.wantErr)

Diff for: ‎pkg/provider/bitbucketcloud/bitbucket.go

+13-9
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
var _ provider.Interface = (*Provider)(nil)
2323

2424
type Provider struct {
25-
Client *bitbucket.Client
25+
bbClient *bitbucket.Client
2626
Logger *zap.SugaredLogger
2727
run *params.Run
2828
pacInfo *info.PacOpts
@@ -31,6 +31,10 @@ type Provider struct {
3131
provenance string
3232
}
3333

34+
func (v Provider) Client() *bitbucket.Client {
35+
return v.bbClient
36+
}
37+
3438
// CheckPolicyAllowing TODO: Implement ME.
3539
func (v *Provider) CheckPolicyAllowing(_ context.Context, _ *info.Event, _ []string) (bool, string) {
3640
return false, ""
@@ -104,11 +108,11 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
104108
Revision: event.SHA,
105109
}
106110

107-
if v.Client == nil {
111+
if v.bbClient == nil {
108112
return fmt.Errorf("no token has been set, cannot set status")
109113
}
110114

111-
_, err := v.Client.Repositories.Commits.CreateCommitStatus(cmo, cso)
115+
_, err := v.Client().Repositories.Commits.CreateCommitStatus(cmo, cso)
112116
if err != nil {
113117
return err
114118
}
@@ -121,7 +125,7 @@ func (v *Provider) CreateStatus(_ context.Context, event *info.Event, statusopts
121125
if statusopts.OriginalPipelineRunName != "" {
122126
onPr = "/" + statusopts.OriginalPipelineRunName
123127
}
124-
_, err = v.Client.Repositories.PullRequests.AddComment(
128+
_, err = v.Client().Repositories.PullRequests.AddComment(
125129
&bitbucket.PullRequestCommentOptions{
126130
Owner: event.Organization,
127131
RepoSlug: event.Repository,
@@ -161,7 +165,7 @@ func (v *Provider) getDir(event *info.Event, path string) ([]bitbucket.Repositor
161165
Path: path,
162166
}
163167

164-
repositoryFiles, err := v.Client.Repositories.Repository.ListFiles(repoFileOpts)
168+
repositoryFiles, err := v.Client().Repositories.Repository.ListFiles(repoFileOpts)
165169
if err != nil {
166170
return nil, err
167171
}
@@ -183,7 +187,7 @@ func (v *Provider) SetClient(_ context.Context, run *params.Run, event *info.Eve
183187
if event.Provider.User == "" {
184188
return fmt.Errorf("no git_provider.user has been in repo crd")
185189
}
186-
v.Client = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token)
190+
v.bbClient = bitbucket.NewBasicAuth(event.Provider.User, event.Provider.Token)
187191
v.Token = &event.Provider.Token
188192
v.Username = &event.Provider.User
189193
v.run = run
@@ -195,7 +199,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error {
195199
if branchortag == "" {
196200
branchortag = event.HeadBranch
197201
}
198-
response, err := v.Client.Repositories.Commits.GetCommits(&bitbucket.CommitsOptions{
202+
response, err := v.Client().Repositories.Commits.GetCommits(&bitbucket.CommitsOptions{
199203
Owner: event.Organization,
200204
RepoSlug: event.Repository,
201205
Branchortag: branchortag,
@@ -226,7 +230,7 @@ func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error {
226230
event.SHA = commitinfo.Hash
227231

228232
// now to get the default branch from repository.Get
229-
repo, err := v.Client.Repositories.Repository.Get(&bitbucket.RepositoryOptions{
233+
repo, err := v.Client().Repositories.Repository.Get(&bitbucket.RepositoryOptions{
230234
Owner: event.Organization,
231235
RepoSlug: event.Repository,
232236
})
@@ -278,7 +282,7 @@ func (v *Provider) concatAllYamlFiles(objects []bitbucket.RepositoryFile, event
278282
}
279283

280284
func (v *Provider) getBlob(runevent *info.Event, ref, path string) (string, error) {
281-
blob, err := v.Client.Repositories.Repository.GetFileBlob(&bitbucket.RepositoryBlobOptions{
285+
blob, err := v.Client().Repositories.Repository.GetFileBlob(&bitbucket.RepositoryBlobOptions{
282286
Owner: runevent.Organization,
283287
RepoSlug: runevent.Repository,
284288
Ref: ref,

Diff for: ‎pkg/provider/bitbucketcloud/bitbucket_test.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,7 @@ func TestGetTektonDir(t *testing.T) {
7575
ctx, _ := rtesting.SetupFakeContext(t)
7676
bbclient, mux, tearDown := bbcloudtest.SetupBBCloudClient(t)
7777
defer tearDown()
78-
v := &Provider{Logger: fakelogger, Client: bbclient}
78+
v := &Provider{Logger: fakelogger, bbClient: bbclient}
7979
bbcloudtest.MuxDirContent(t, mux, tt.event, tt.testDirPath, tt.provenance)
8080
content, err := v.GetTektonDir(ctx, tt.event, ".tekton", tt.provenance)
8181
if tt.wantErr != "" {
@@ -202,7 +202,7 @@ func TestGetCommitInfo(t *testing.T) {
202202
ctx, _ := rtesting.SetupFakeContext(t)
203203
bbclient, mux, tearDown := bbcloudtest.SetupBBCloudClient(t)
204204
defer tearDown()
205-
v := &Provider{Client: bbclient}
205+
v := &Provider{bbClient: bbclient}
206206
bbcloudtest.MuxCommits(t, mux, tt.event, []types.Commit{
207207
tt.commitinfo,
208208
})
@@ -295,8 +295,8 @@ func TestCreateStatus(t *testing.T) {
295295
bbclient, mux, tearDown := bbcloudtest.SetupBBCloudClient(t)
296296
defer tearDown()
297297
v := &Provider{
298-
Client: bbclient,
299-
run: params.New(),
298+
bbClient: bbclient,
299+
run: params.New(),
300300
pacInfo: &info.PacOpts{
301301
Settings: settings.Settings{
302302
ApplicationName: settings.PACApplicationNameDefaultValue,

Diff for: ‎pkg/provider/bitbucketdatacenter/acl.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ func (v *Provider) checkOkToTestCommentFromApprovedMember(ctx context.Context, e
5353
if nextPage > 0 {
5454
localVarOptionals["start"] = int(nextPage)
5555
}
56-
return v.Client.DefaultApi.GetActivities(v.projectKey, event.Repository, v.pullRequestNumber, localVarOptionals)
56+
return v.Client().DefaultApi.GetActivities(v.projectKey, event.Repository, v.pullRequestNumber, localVarOptionals)
5757
})
5858
if err != nil {
5959
return false, err
@@ -120,7 +120,7 @@ func (v *Provider) checkMemberShip(ctx context.Context, event *info.Event) (bool
120120
if nextPage > 0 {
121121
localVarOptionals["start"] = int(nextPage)
122122
}
123-
return v.Client.DefaultApi.GetUsersWithAnyPermission_23(v.projectKey, localVarOptionals)
123+
return v.Client().DefaultApi.GetUsersWithAnyPermission_23(v.projectKey, localVarOptionals)
124124
})
125125
if err != nil {
126126
return false, err
@@ -139,7 +139,7 @@ func (v *Provider) checkMemberShip(ctx context.Context, event *info.Event) (bool
139139
if nextPage > 0 {
140140
localVarOptionals["start"] = int(nextPage)
141141
}
142-
return v.Client.DefaultApi.GetUsersWithAnyPermission_24(v.projectKey, event.Repository, localVarOptionals)
142+
return v.Client().DefaultApi.GetUsersWithAnyPermission_24(v.projectKey, event.Repository, localVarOptionals)
143143
})
144144
if err != nil {
145145
return false, err

Diff for: ‎pkg/provider/bitbucketdatacenter/acl_test.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,8 @@ func TestIsAllowed(t *testing.T) {
212212

213213
v := &Provider{
214214
baseURL: tURL,
215-
Client: bbclient,
216-
ScmClient: scmClient,
215+
bbClient: bbclient,
216+
scmClient: scmClient,
217217
defaultBranchLatestCommit: tt.fields.defaultBranchLatestCommit,
218218
pullRequestNumber: tt.fields.pullRequestNumber,
219219
projectKey: tt.event.Organization,

Diff for: ‎pkg/provider/bitbucketdatacenter/bitbucketdatacenter.go

+32-16
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,8 @@ const apiResponseLimit = 100
3131
var _ provider.Interface = (*Provider)(nil)
3232

3333
type Provider struct {
34-
Client *bbv1.APIClient // temporarily keeping it after the refactor finishes, will be removed
35-
ScmClient *scm.Client
34+
bbClient *bbv1.APIClient // temporarily keeping it after the refactor finishes, will be removed
35+
scmClient *scm.Client
3636
Logger *zap.SugaredLogger
3737
run *params.Run
3838
pacInfo *info.PacOpts
@@ -44,6 +44,22 @@ type Provider struct {
4444
projectKey string
4545
}
4646

47+
func (v Provider) Client() *bbv1.APIClient {
48+
return v.bbClient
49+
}
50+
51+
func (v *Provider) SetBitBucketClient(client *bbv1.APIClient) {
52+
v.bbClient = client
53+
}
54+
55+
func (v Provider) ScmClient() *scm.Client {
56+
return v.scmClient
57+
}
58+
59+
func (v *Provider) SetScmClient(client *scm.Client) {
60+
v.scmClient = client
61+
}
62+
4763
func (v *Provider) SetPacInfo(pacInfo *info.PacOpts) {
4864
v.pacInfo = pacInfo
4965
}
@@ -99,7 +115,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
99115
if statusOpts.DetailsURL != "" {
100116
detailsURL = statusOpts.DetailsURL
101117
}
102-
if v.ScmClient == nil {
118+
if v.scmClient == nil {
103119
return fmt.Errorf("no token has been set, cannot set status")
104120
}
105121

@@ -119,7 +135,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
119135
Desc: statusOpts.Title,
120136
Link: detailsURL,
121137
}
122-
_, _, err := v.ScmClient.Repositories.CreateStatus(ctx, OrgAndRepo, event.SHA, opts)
138+
_, _, err := v.ScmClient().Repositories.CreateStatus(ctx, OrgAndRepo, event.SHA, opts)
123139
if err != nil {
124140
return err
125141
}
@@ -135,7 +151,7 @@ func (v *Provider) CreateStatus(ctx context.Context, event *info.Event, statusOp
135151
input := &scm.CommentInput{
136152
Body: bbComment,
137153
}
138-
_, _, err := v.ScmClient.PullRequests.CreateComment(ctx, OrgAndRepo, event.PullRequestNumber, input)
154+
_, _, err := v.ScmClient().PullRequests.CreateComment(ctx, OrgAndRepo, event.PullRequestNumber, input)
139155
if err != nil {
140156
return err
141157
}
@@ -185,7 +201,7 @@ func (v *Provider) concatAllYamlFiles(ctx context.Context, objects []string, sha
185201

186202
func (v *Provider) getRaw(ctx context.Context, runevent *info.Event, revision, path string) (string, error) {
187203
repo := fmt.Sprintf("%s/%s", runevent.Organization, runevent.Repository)
188-
content, _, err := v.ScmClient.Contents.Find(ctx, repo, path, revision)
204+
content, _, err := v.ScmClient().Contents.Find(ctx, repo, path, revision)
189205
if err != nil {
190206
return "", fmt.Errorf("cannot find %s inside the %s repository: %w", path, runevent.Repository, err)
191207
}
@@ -206,7 +222,7 @@ func (v *Provider) GetTektonDir(ctx context.Context, event *info.Event, path, pr
206222
var fileEntries []*scm.FileEntry
207223
opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit}
208224
for {
209-
entries, _, err := v.ScmClient.Contents.List(ctx, orgAndRepo, path, at, opts)
225+
entries, _, err := v.ScmClient().Contents.List(ctx, orgAndRepo, path, at, opts)
210226
if err != nil {
211227
return "", fmt.Errorf("cannot list content of %s directory: %w", path, err)
212228
}
@@ -281,11 +297,11 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
281297

282298
ctx = context.WithValue(ctx, bbv1.ContextBasicAuth, basicAuth)
283299
cfg := bbv1.NewConfiguration(event.Provider.URL)
284-
if v.Client == nil {
285-
v.Client = bbv1.NewAPIClient(ctx, cfg)
300+
if v.bbClient == nil {
301+
v.bbClient = bbv1.NewAPIClient(ctx, cfg)
286302
}
287303

288-
if v.ScmClient == nil {
304+
if v.scmClient == nil {
289305
client, err := stash.New(removeLastSegment(event.Provider.URL)) // remove `/rest` from url
290306
if err != nil {
291307
return err
@@ -299,10 +315,10 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
299315
),
300316
},
301317
}
302-
v.ScmClient = client
318+
v.scmClient = client
303319
}
304320
v.run = run
305-
_, resp, err := v.ScmClient.Users.FindLogin(ctx, event.Provider.User)
321+
_, resp, err := v.ScmClient().Users.FindLogin(ctx, event.Provider.User)
306322
if resp != nil && resp.Status == http.StatusUnauthorized {
307323
return fmt.Errorf("cannot get user %s with token: %w", event.Provider.User, err)
308324
}
@@ -315,14 +331,14 @@ func (v *Provider) SetClient(ctx context.Context, run *params.Run, event *info.E
315331

316332
func (v *Provider) GetCommitInfo(_ context.Context, event *info.Event) error {
317333
OrgAndRepo := fmt.Sprintf("%s/%s", event.Organization, event.Repository)
318-
commit, _, err := v.ScmClient.Git.FindCommit(context.Background(), OrgAndRepo, event.SHA)
334+
commit, _, err := v.ScmClient().Git.FindCommit(context.Background(), OrgAndRepo, event.SHA)
319335
if err != nil {
320336
return err
321337
}
322338
event.SHATitle = sanitizeTitle(commit.Message)
323339
event.SHAURL = fmt.Sprintf("%s/projects/%s/repos/%s/commits/%s", v.baseURL, v.projectKey, event.Repository, event.SHA)
324340

325-
ref, _, err := v.ScmClient.Git.GetDefaultBranch(context.Background(), OrgAndRepo)
341+
ref, _, err := v.ScmClient().Git.GetDefaultBranch(context.Background(), OrgAndRepo)
326342
if err != nil {
327343
return err
328344
}
@@ -345,7 +361,7 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf
345361
opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit}
346362
changedFiles := changedfiles.ChangedFiles{}
347363
for {
348-
changes, _, err := v.ScmClient.PullRequests.ListChanges(ctx, OrgAndRepo, runevent.PullRequestNumber, opts)
364+
changes, _, err := v.ScmClient().PullRequests.ListChanges(ctx, OrgAndRepo, runevent.PullRequestNumber, opts)
349365
if err != nil {
350366
return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for pull request: %w", err)
351367
}
@@ -383,7 +399,7 @@ func (v *Provider) GetFiles(ctx context.Context, runevent *info.Event) (changedf
383399
opts := &scm.ListOptions{Page: 1, Size: apiResponseLimit}
384400
changedFiles := changedfiles.ChangedFiles{}
385401
for {
386-
changes, _, err := v.ScmClient.Git.ListChanges(ctx, OrgAndRepo, runevent.SHA, opts)
402+
changes, _, err := v.ScmClient().Git.ListChanges(ctx, OrgAndRepo, runevent.SHA, opts)
387403
if err != nil {
388404
return changedfiles.ChangedFiles{}, fmt.Errorf("failed to list changes for commit %s: %w", runevent.SHA, err)
389405
}

0 commit comments

Comments
 (0)