Skip to content

Commit 0c37128

Browse files
authored
refactor: consolidate version access to slackcontext.Version(ctx) (#431)
* refactor: remove vestigial CLIVersion field and SetVersion option The CLIVersion field and SetVersion functional option on ClientFactory were a workaround for a circular dependency that no longer exists. The API client reads version from context via slackcontext.Version(ctx), making this indirection unnecessary. Replace with direct version.Raw() calls. * refactor: remove vestigial Config.Version field Consumers now call version.Raw() directly instead of reading from Config.Version, which was a redundant copy set in two places. * refactor: migrate version.Raw() callers to slackcontext.Version(ctx) Replace direct version.Raw() calls with slackcontext.Version(ctx) across all callers for improved testability via context-based dependency injection. - slackcontext.Version(ctx) now falls back to version.Raw() when the context is missing the version, returning both the fallback value and an error so callers can log a warning - ResolveLogstashHost no longer takes a cliVersion parameter; it reads the version from context internally - Tracking tests use context-based version injection instead of mutating the global version.Version variable * refactor: update slackcontext.Version fallback error message * refactor: use version.Raw() consistently in main.go
1 parent 49fd7d8 commit 0c37128

20 files changed

Lines changed: 93 additions & 82 deletions

cmd/app/add_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -894,7 +894,7 @@ func prepareAddMocks(t *testing.T, clients *shared.ClientFactory, clientsMock *s
894894

895895
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).
896896
Return("api host")
897-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).
897+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).
898898
Return("logstash host")
899899

900900
manifestMock := &app.ManifestMockObject{}

cmd/app/delete_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ func prepareCommonDeleteMocks(t *testing.T, cf *shared.ClientFactory, cm *shared
164164

165165
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).
166166
Return("api host")
167-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).
167+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).
168168
Return("logstash host")
169169

170170
// Mock list command

cmd/app/uninstall_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func prepareCommonUninstallMocks(ctx context.Context, clients *shared.ClientFact
104104
// Mock API calls
105105
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).
106106
Return("api host")
107-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).
107+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).
108108
Return("logstash host")
109109

110110
clientsMock.API.On("ValidateSession", mock.Anything, mock.Anything).Return(api.AuthSession{

cmd/auth/logout.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ func handleAuthRemoval(ctx context.Context, clients *shared.ClientFactory, auth
9595

9696
// Update the API Host and Logstash Host to be the selected/default auth
9797
clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, &auth)
98-
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.Config.Version)
98+
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved)
9999

100100
// First, try to revoke the xoxe-xoxp (auth) token credential
101101
var xoxpToken = auth.Token
@@ -118,7 +118,7 @@ func handleAuthRemoval(ctx context.Context, clients *shared.ClientFactory, auth
118118

119119
// Update the API Host and Logstash Host to be the selected/default auth
120120
clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil)
121-
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, clients.Config.Version)
121+
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved)
122122

123123
return nil
124124
}

cmd/auth/logout_test.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ func TestLogoutCommand(t *testing.T) {
3737
CmdArgs: []string{"--all"},
3838
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
3939
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com")
40-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com")
40+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com")
4141
clientsMock.Auth.On("Auths", mock.Anything).Return(fakeAuthsByTeamSlice, nil)
4242
clientsMock.Auth.On("RevokeToken", mock.Anything, fakeAuthsByTeamSlice[0].Token).Return(nil)
4343
clientsMock.Auth.On("RevokeToken", mock.Anything, fakeAuthsByTeamSlice[0].RefreshToken).Return(nil)
@@ -61,7 +61,7 @@ func TestLogoutCommand(t *testing.T) {
6161
CmdArgs: []string{"--team", "team1"},
6262
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
6363
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com")
64-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com")
64+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com")
6565
clientsMock.Auth.On("Auths", mock.Anything).Return(fakeAuthsByTeamSlice, nil)
6666
clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an authorization to revoke", mock.Anything, iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{
6767
Flag: clientsMock.Config.Flags.Lookup("team"),
@@ -85,7 +85,7 @@ func TestLogoutCommand(t *testing.T) {
8585
CmdArgs: []string{"--team", "T2"},
8686
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
8787
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com")
88-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com")
88+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com")
8989
clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an authorization to revoke", mock.Anything, iostreams.MatchPromptConfig(iostreams.SelectPromptConfig{
9090
Flag: clientsMock.Config.Flags.Lookup("team"),
9191
})).Return(iostreams.SelectPromptResponse{
@@ -134,7 +134,7 @@ func TestLogoutCommand(t *testing.T) {
134134
CmdArgs: []string{},
135135
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
136136
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com")
137-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com")
137+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com")
138138
clientsMock.Auth.On("Auths", mock.Anything).Return(fakeAuthsByTeamSlice, nil)
139139
clientsMock.IO.On("SelectPrompt", mock.Anything, "Select an authorization to revoke", []string{
140140
FormatAuthLabel(fakeAuthsByTeamSlice[0]),
@@ -162,7 +162,7 @@ func TestLogoutCommand(t *testing.T) {
162162
CmdArgs: []string{},
163163
Setup: func(t *testing.T, ctx context.Context, clientsMock *shared.ClientsMock, clients *shared.ClientFactory) {
164164
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com")
165-
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("logstash.slack.com")
165+
clientsMock.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("logstash.slack.com")
166166
clientsMock.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
167167
fakeAuthsByTeamSlice[0],
168168
}, nil)

cmd/root.go

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob
265265

266266
// Init clients that use flags
267267
clients.Config.APIHostResolved = clients.Auth().ResolveAPIHost(ctx, clients.Config.APIHostFlag, nil)
268-
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved, version.Raw())
268+
clients.Config.LogstashHostResolved = clients.Auth().ResolveLogstashHost(ctx, clients.Config.APIHostResolved)
269269

270270
// Init System ID
271271
if systemID, err := clients.Config.SystemConfig.InitSystemID(ctx); err != nil {
@@ -296,8 +296,6 @@ func InitConfig(ctx context.Context, clients *shared.ClientFactory, rootCmd *cob
296296
// Init configurations
297297
clients.Config.LoadExperiments(ctx, clients.IO.PrintDebug)
298298
style.ToggleLipgloss(clients.Config.WithExperimentOn(experiment.Lipgloss))
299-
// TODO(slackcontext) Consolidate storing CLI version to slackcontext
300-
clients.Config.Version = version.Raw()
301299

302300
// The domain auths (token->domain) shouldn't change for the execution of the CLI so preload them into config!
303301
clients.Config.DomainAuthTokens = clients.Auth().MapAuthTokensToDomains(ctx)

cmd/sandbox/create_test.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ func TestCreateCommand(t *testing.T) {
4646
testToken := "xoxb-test-token"
4747
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
4848
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
49-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
49+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
5050
cm.API.On("CreateSandbox", mock.Anything, testToken, "test-box", "test-box", "mypass", "", "", 0, "", int64(0), false).
5151
Return("T123", "https://test-box.slack.com", nil)
5252
cm.API.On("UsersInfo", mock.Anything, mock.Anything, mock.Anything).Return(&types.UserInfo{Profile: types.UserProfile{}}, nil)
@@ -73,7 +73,7 @@ func TestCreateCommand(t *testing.T) {
7373
testToken := "xoxb-test-token"
7474
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
7575
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
76-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
76+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
7777
cm.API.On("CreateSandbox", mock.Anything, testToken, "My Test Box", "my-test-box", "pass", "", "", 0, "", int64(0), false).
7878
Return("T789", "https://my-test-box.slack.com", nil)
7979

@@ -98,7 +98,7 @@ func TestCreateCommand(t *testing.T) {
9898
testToken := "xoxb-test-token"
9999
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
100100
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
101-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
101+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
102102
cm.API.On("CreateSandbox", mock.Anything, testToken, "tmp-box", "tmp-box", "pass", "", "", 0, "", mock.MatchedBy(func(v int64) bool { return v > 0 }), false).
103103
Return("T111", "https://tmp-box.slack.com", nil)
104104

@@ -122,7 +122,7 @@ func TestCreateCommand(t *testing.T) {
122122
testToken := "xoxb-test-token"
123123
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
124124
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
125-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
125+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
126126
cm.API.On("CreateSandbox", mock.Anything, testToken, "err-box", "err-box", "pass", "", "", 0, "", int64(0), false).
127127
Return("", "", errors.New("api_error"))
128128

@@ -145,7 +145,7 @@ func TestCreateCommand(t *testing.T) {
145145
testToken := "xoxb-test-token"
146146
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
147147
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
148-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
148+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
149149
cm.API.On("CreateSandbox", mock.Anything, testToken, "tpl-box", "tpl-box", "pass", "", "", 1, "", int64(0), false).
150150
Return("T333", "https://tpl-box.slack.com", nil)
151151

@@ -171,7 +171,7 @@ func TestCreateCommand(t *testing.T) {
171171
testToken := "xoxb-test-token"
172172
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
173173
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
174-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
174+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
175175
cm.API.On("CreateSandbox", mock.Anything, testToken, "partner-box", "partner-box", "pass", "", "", 0, "", int64(0), true).
176176
Return("T555", "https://partner-box.slack.com", nil)
177177

@@ -197,7 +197,7 @@ func TestCreateCommand(t *testing.T) {
197197
testToken := "xoxb-test-token"
198198
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
199199
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
200-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
200+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
201201

202202
cm.AddDefaultMocks()
203203
cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)}
@@ -221,7 +221,7 @@ func TestCreateCommand(t *testing.T) {
221221
testToken := "xoxb-test-token"
222222
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
223223
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
224-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
224+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
225225
cm.API.On("CreateSandbox", mock.Anything, testToken, "date-box", "date-box", "pass", "", "", 0, "", archiveEpoch, false).
226226
Return("T222", "https://date-box.slack.com", nil)
227227

@@ -247,7 +247,7 @@ func TestCreateCommand(t *testing.T) {
247247
testToken := "xoxb-test-token"
248248
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
249249
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
250-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
250+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
251251

252252
cm.AddDefaultMocks()
253253
cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)}
@@ -271,7 +271,7 @@ func TestCreateCommand(t *testing.T) {
271271
testToken := "xoxb-test-token"
272272
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
273273
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
274-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
274+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
275275

276276
cm.AddDefaultMocks()
277277
cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)}
@@ -306,7 +306,7 @@ func setupCreateMocks(t *testing.T, ctx context.Context, cm *shared.ClientsMock,
306306
testToken := "xoxb-test-token"
307307
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
308308
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
309-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
309+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
310310
cm.API.On("CreateSandbox", mock.Anything, testToken, name, domain, password, "", "", 0, "", archiveEpoch, partner).
311311
Return("T222", "https://"+domain+".slack.com", nil)
312312
cm.AddDefaultMocks()
@@ -319,7 +319,7 @@ func setupCreateAuthOnly(t *testing.T, ctx context.Context, cm *shared.ClientsMo
319319
testToken := "xoxb-test-token"
320320
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
321321
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
322-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
322+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
323323
cm.AddDefaultMocks()
324324
cm.Config.ExperimentsFlag = []string{string(experiment.Sandboxes)}
325325
cm.Config.LoadExperiments(ctx, cm.IO.PrintDebug)
@@ -494,7 +494,7 @@ func Test_getTemplateID(t *testing.T) {
494494
testToken := "xoxb-test-token"
495495
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
496496
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
497-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
497+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
498498
cm.API.On("CreateSandbox", mock.Anything, testToken, "tpl-box", "tpl-box", "pass", "", "", 1, "", int64(0), false).
499499
Return("T333", "https://tpl-box.slack.com", nil)
500500
cm.AddDefaultMocks()
@@ -519,7 +519,7 @@ func Test_getTemplateID(t *testing.T) {
519519
testToken := "xoxb-test-token"
520520
cm.Auth.On("AuthWithToken", mock.Anything, testToken).Return(types.SlackAuth{Token: testToken}, nil)
521521
cm.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("https://api.slack.com")
522-
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
522+
cm.Auth.On("ResolveLogstashHost", mock.Anything, mock.Anything).Return("https://slackb.com/events/cli")
523523
cm.API.On("CreateSandbox", mock.Anything, testToken, "tpl-box", "tpl-box", "pass", "", "", 1, "", int64(0), false).
524524
Return("T333", "https://tpl-box.slack.com", nil)
525525
cm.AddDefaultMocks()

0 commit comments

Comments
 (0)