Skip to content

Commit f9ac93b

Browse files
mwbrooksmwbrooks
authored
refactor(config): destructify project-level getter and setters in package config (#138)
* refactor(config): destructify config.GetProjectDirPath(fs, os) * refactor(config): destructify config.ProjectConfigJSONFileExists(fs, os, wd) * refactor(config): destructify config.WriteProjectConfigFile(ctx, fs, os, projectConfig) * refactor(config): destructify config.ReadProjectConfigFile(ctx, fs, os) * refactor(config): destructify config.SetManifestSource(ctx, fs, os, source)
1 parent 0faf469 commit f9ac93b

File tree

14 files changed

+111
-85
lines changed

14 files changed

+111
-85
lines changed

‎.vscode/settings.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
"slackdeps",
2121
"slackerror",
2222
"slackhq",
23+
"slackmock",
2324
"slacktrace",
2425
"tabletests",
2526
"testutil"

‎cmd/app/link.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty
202202
return nil
203203
}
204204

205-
if err := clients.Config.ProjectConfig.SetManifestSource(ctx, config.ManifestSourceRemote); err != nil {
205+
if err := config.SetManifestSource(ctx, clients.Fs, clients.Os, config.ManifestSourceRemote); err != nil {
206206
// Log the error to the verbose output
207207
clients.IO.PrintDebug(ctx, "Error setting manifest source in project-level config: %s", err)
208208
// Display a user-friendly error with a workaround

‎cmd/app/link_test.go‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,7 @@ func Test_Apps_Link(t *testing.T) {
440440
cm.AddDefaultMocks()
441441
setupAppLinkCommandMocks(t, ctx, cm, cf)
442442
// Set manifest source to project to trigger confirmation prompt
443-
if err := cm.Config.ProjectConfig.SetManifestSource(ctx, config.ManifestSourceLocal); err != nil {
443+
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
444444
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
445445
}
446446
// Accept manifest source confirmation prompt
@@ -509,7 +509,7 @@ func Test_Apps_Link(t *testing.T) {
509509
cm.AddDefaultMocks()
510510
setupAppLinkCommandMocks(t, ctx, cm, cf)
511511
// Set manifest source to project to trigger confirmation prompt
512-
if err := cm.Config.ProjectConfig.SetManifestSource(ctx, config.ManifestSourceLocal); err != nil {
512+
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
513513
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
514514
}
515515
// Decline manifest source confirmation prompt
@@ -547,7 +547,7 @@ func Test_Apps_Link(t *testing.T) {
547547
cm.AddDefaultMocks()
548548
setupAppLinkCommandMocks(t, ctx, cm, cf)
549549
// Set manifest source to local
550-
if err := cm.Config.ProjectConfig.SetManifestSource(ctx, config.ManifestSourceLocal); err != nil {
550+
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
551551
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
552552
}
553553
// Mock manifest for Run-on-Slack app
@@ -624,7 +624,7 @@ func Test_Apps_Link(t *testing.T) {
624624
cm.AddDefaultMocks()
625625
setupAppLinkCommandMocks(t, ctx, cm, cf)
626626
// Set manifest source to local
627-
if err := cm.Config.ProjectConfig.SetManifestSource(ctx, config.ManifestSourceLocal); err != nil {
627+
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
628628
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
629629
}
630630
// Mock manifest for Run-on-Slack app
@@ -760,7 +760,7 @@ func setupAppLinkCommandMocks(t *testing.T, ctx context.Context, cm *shared.Clie
760760
require.FailNow(t, fmt.Sprintf("Failed to create the hooks file in the memory-based file system: %s", err))
761761
}
762762

763-
if err := cm.Config.ProjectConfig.SetManifestSource(ctx, config.ManifestSourceRemote); err != nil {
763+
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceRemote); err != nil {
764764
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
765765
}
766766

‎cmd/doctor/check.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"strings"
2323
"time"
2424

25+
"github.com/slackapi/slack-cli/internal/config"
2526
"github.com/slackapi/slack-cli/internal/deputil"
2627
"github.com/slackapi/slack-cli/internal/iostreams"
2728
"github.com/slackapi/slack-cli/internal/pkg/version"
@@ -132,7 +133,7 @@ func checkProjectConfig(ctx context.Context, clients *shared.ClientFactory) Sect
132133
Label: "Configurations",
133134
Value: "your project's CLI settings",
134135
}
135-
projectConfig, err := clients.Config.ProjectConfig.ReadProjectConfigFile(ctx)
136+
projectConfig, err := config.ReadProjectConfigFile(ctx, clients.Fs, clients.Os)
136137
if err != nil {
137138
if slackerror.ToSlackError(err).Code != slackerror.ErrInvalidAppDirectory {
138139
section.Errors = append(section.Errors, *slackerror.ToSlackError(err))

‎cmd/doctor/check_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ import (
2626
"github.com/slackapi/slack-cli/internal/shared"
2727
"github.com/slackapi/slack-cli/internal/shared/types"
2828
"github.com/slackapi/slack-cli/internal/slackcontext"
29+
"github.com/slackapi/slack-cli/internal/slackdeps"
2930
"github.com/slackapi/slack-cli/internal/slackerror"
31+
"github.com/slackapi/slack-cli/test/slackmock"
3032
"github.com/stretchr/testify/assert"
3133
"github.com/stretchr/testify/mock"
3234
"github.com/stretchr/testify/require"
@@ -144,10 +146,13 @@ func TestDoctorCheckProjectConfig(t *testing.T) {
144146
ctx := slackcontext.MockContext(t.Context())
145147
clientsMock := shared.NewClientsMock()
146148
clientsMock.AddDefaultMocks()
147-
pcm := &config.ProjectConfigMock{}
148-
pcm.On("ReadProjectConfigFile", mock.Anything).Return(tt.projectConfig, nil)
149-
clientsMock.Config.ProjectConfig = pcm
150149
clients := shared.NewClientFactory(clientsMock.MockClientFactory())
150+
151+
slackmock.CreateProject(t, ctx, clients.Fs, clients.Os, slackdeps.MockWorkingDirectory)
152+
153+
_, err := config.WriteProjectConfigFile(ctx, clients.Fs, clients.Os, tt.projectConfig)
154+
require.NoError(t, err)
155+
151156
expected := Section{
152157
Label: "Configurations",
153158
Value: "your project's CLI settings",

‎cmd/doctor/doctor_test.go‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ import (
2828
"github.com/slackapi/slack-cli/internal/shared"
2929
"github.com/slackapi/slack-cli/internal/shared/types"
3030
"github.com/slackapi/slack-cli/internal/slackcontext"
31+
"github.com/slackapi/slack-cli/internal/slackdeps"
3132
"github.com/slackapi/slack-cli/internal/slackerror"
33+
"github.com/slackapi/slack-cli/test/slackmock"
3234
"github.com/slackapi/slack-cli/test/testutil"
3335
"github.com/stretchr/testify/assert"
3436
"github.com/stretchr/testify/mock"
@@ -57,14 +59,16 @@ func TestDoctorCommand(t *testing.T) {
5759
clientsMock.Auth.On("ResolveAPIHost", mock.Anything, mock.Anything, mock.Anything).Return("api.slack.com")
5860
clientsMock.API.On("ValidateSession", mock.Anything, mock.Anything).Return(api.AuthSession{}, nil)
5961
clientsMock.AddDefaultMocks()
60-
pcm := &config.ProjectConfigMock{}
61-
pcm.On("ReadProjectConfigFile", mock.Anything).Return(config.ProjectConfig{
62+
63+
slackmock.CreateProject(t, ctx, clientsMock.Fs, clientsMock.Os, slackdeps.MockWorkingDirectory)
64+
_, err := config.WriteProjectConfigFile(ctx, clientsMock.Fs, clientsMock.Os, config.ProjectConfig{
6265
ProjectID: expectedProjectID,
6366
Manifest: &config.ManifestConfig{
6467
Source: config.ManifestSourceLocal.String(),
6568
},
66-
}, nil)
67-
clientsMock.Config.ProjectConfig = pcm
69+
})
70+
require.NoError(t, err)
71+
6872
scm := &config.SystemConfigMock{}
6973
scm.On("GetSurveyConfig", mock.Anything, mock.Anything).Return(config.SurveyConfig{}, nil)
7074
scm.On("SetSurveyConfig", mock.Anything, mock.Anything, mock.Anything).Return(nil)
@@ -88,7 +92,7 @@ func TestDoctorCommand(t *testing.T) {
8892

8993
cmd := NewDoctorCommand(clients)
9094
testutil.MockCmdIO(clients.IO, cmd)
91-
err := cmd.ExecuteContext(ctx)
95+
err = cmd.ExecuteContext(ctx)
9296
require.NoError(t, err)
9397

9498
report, err := performChecks(ctx, clients)

‎internal/app/app_client.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ func (ac *AppClient) CleanUp() {
251251

252252
// if there are no tracked apps anymore and no config file, remove the .slack folder.
253253
// otherwise remove .slack/apps*.json files that contain no apps.
254-
if ac.apps.IsEmpty() && !ac.config.ProjectConfig.ProjectConfigJSONFileExists(wd) {
254+
if ac.apps.IsEmpty() && !config.ProjectConfigJSONFileExists(ac.fs, ac.os, wd) {
255255
var deployedAppsJSONFilePath = filepath.Join(wd, deployedAppsFilename)
256256
var dotSlackFolder = filepath.Dir(deployedAppsJSONFilePath)
257257
_ = ac.fs.RemoveAll(dotSlackFolder)

‎internal/app/app_client_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -637,7 +637,7 @@ func TestAppClient_CleanupSlackFolder(t *testing.T) {
637637
assert.True(t, ac.apps.IsEmpty(), "an unexpected app was found")
638638

639639
require.NoError(t, err)
640-
assert.False(t, ac.config.ProjectConfig.ProjectConfigJSONFileExists(wd),
640+
assert.False(t, config.ProjectConfigJSONFileExists(ac.fs, ac.os, wd),
641641
"an unexpected config was found")
642642

643643
dotSlackFolder := filepath.Dir(pathToAppsJSON)

‎internal/config/experiments.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ func (c *Config) LoadExperiments(
3636
}
3737
printDebug(ctx, fmt.Sprintf("active flag experiments: %s", experiments))
3838
// Load from project config file
39-
projectConfig, err := c.ProjectConfig.ReadProjectConfigFile(ctx)
39+
projectConfig, err := ReadProjectConfigFile(ctx, c.fs, c.os)
4040
if err != nil && slackerror.ToSlackError(err).Code != slackerror.ErrInvalidAppDirectory {
4141
printDebug(ctx, fmt.Sprintf("failed to parse project-level config file: %s", err))
4242
} else if err == nil {

‎internal/config/project.go‎

Lines changed: 19 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,8 @@ type ProjectConfigManager interface {
5252
GetProjectID(ctx context.Context) (string, error)
5353
SetProjectID(ctx context.Context, projectID string) (string, error)
5454
GetManifestSource(ctx context.Context) (ManifestSource, error)
55-
SetManifestSource(ctx context.Context, source ManifestSource) error
5655
GetSurveyConfig(ctx context.Context, name string) (SurveyConfig, error)
5756
SetSurveyConfig(ctx context.Context, name string, surveyConfig SurveyConfig) error
58-
ReadProjectConfigFile(ctx context.Context) (ProjectConfig, error)
59-
WriteProjectConfigFile(ctx context.Context, projectConfig ProjectConfig) (string, error)
60-
ProjectConfigJSONFileExists(projectDirPath string) bool
6157

6258
Cache() cache.Cacher
6359
}
@@ -119,7 +115,7 @@ func (c *ProjectConfig) GetProjectID(ctx context.Context) (string, error) {
119115
span, ctx = opentracing.StartSpanFromContext(ctx, "GetProjectID")
120116
defer span.Finish()
121117

122-
var projectConfig, err = c.ReadProjectConfigFile(ctx)
118+
var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os)
123119
if err != nil {
124120
return "", err
125121
}
@@ -133,14 +129,14 @@ func (c *ProjectConfig) SetProjectID(ctx context.Context, projectID string) (str
133129
span, _ = opentracing.StartSpanFromContext(ctx, "SetProjectID")
134130
defer span.Finish()
135131

136-
var projectConfig, err = c.ReadProjectConfigFile(ctx)
132+
var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os)
137133
if err != nil {
138134
return "", err
139135
}
140136

141137
projectConfig.ProjectID = projectID
142138

143-
_, err = c.WriteProjectConfigFile(ctx, projectConfig)
139+
_, err = WriteProjectConfigFile(ctx, c.fs, c.os, projectConfig)
144140
if err != nil {
145141
return "", err
146142
}
@@ -154,7 +150,7 @@ func (c *ProjectConfig) GetManifestSource(ctx context.Context) (ManifestSource,
154150
span, ctx = opentracing.StartSpanFromContext(ctx, "GetManifestSource")
155151
defer span.Finish()
156152

157-
var projectConfig, err = c.ReadProjectConfigFile(ctx)
153+
var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os)
158154
if err != nil {
159155
return "", err
160156
}
@@ -175,18 +171,18 @@ func (c *ProjectConfig) GetManifestSource(ctx context.Context) (ManifestSource,
175171
}
176172

177173
// SetManifestSource saves the manifest source preference for the project
178-
func (c *ProjectConfig) SetManifestSource(ctx context.Context, source ManifestSource) error {
174+
func SetManifestSource(ctx context.Context, fs afero.Fs, os types.Os, source ManifestSource) error {
179175
span, ctx := opentracing.StartSpanFromContext(ctx, "SetManifestSource")
180176
defer span.Finish()
181-
projectConfig, err := c.ReadProjectConfigFile(ctx)
177+
projectConfig, err := ReadProjectConfigFile(ctx, fs, os)
182178
if err != nil {
183179
return err
184180
}
185181
if projectConfig.Manifest == nil {
186182
projectConfig.Manifest = &ManifestConfig{}
187183
}
188184
projectConfig.Manifest.Source = source.String()
189-
_, err = c.WriteProjectConfigFile(ctx, projectConfig)
185+
_, err = WriteProjectConfigFile(ctx, fs, os, projectConfig)
190186
if err != nil {
191187
return err
192188
}
@@ -199,7 +195,7 @@ func (c *ProjectConfig) GetSurveyConfig(ctx context.Context, name string) (Surve
199195
span, ctx = opentracing.StartSpanFromContext(ctx, "GetSurveyConfig")
200196
defer span.Finish()
201197

202-
var projectConfig, err = c.ReadProjectConfigFile(ctx)
198+
var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os)
203199
if err != nil {
204200
return SurveyConfig{}, err
205201
}
@@ -218,7 +214,7 @@ func (c *ProjectConfig) SetSurveyConfig(ctx context.Context, name string, survey
218214
span, ctx = opentracing.StartSpanFromContext(ctx, "SetSurveyConfig")
219215
defer span.Finish()
220216

221-
var projectConfig, err = c.ReadProjectConfigFile(ctx)
217+
var projectConfig, err = ReadProjectConfigFile(ctx, c.fs, c.os)
222218
if err != nil {
223219
return err
224220
}
@@ -228,7 +224,7 @@ func (c *ProjectConfig) SetSurveyConfig(ctx context.Context, name string, survey
228224
CompletedAt: surveyConfig.CompletedAt,
229225
}
230226

231-
_, err = c.WriteProjectConfigFile(ctx, projectConfig)
227+
_, err = WriteProjectConfigFile(ctx, c.fs, c.os, projectConfig)
232228
if err != nil {
233229
return err
234230
}
@@ -237,24 +233,24 @@ func (c *ProjectConfig) SetSurveyConfig(ctx context.Context, name string, survey
237233
}
238234

239235
// ReadProjectConfigFile reads the project-level config.json file
240-
func (c *ProjectConfig) ReadProjectConfigFile(ctx context.Context) (ProjectConfig, error) {
236+
func ReadProjectConfigFile(ctx context.Context, fs afero.Fs, os types.Os) (ProjectConfig, error) {
241237
var span opentracing.Span
242238
span, _ = opentracing.StartSpanFromContext(ctx, "ReadProjectConfigFile")
243239
defer span.Finish()
244240

245241
var projectConfig ProjectConfig
246242

247-
projectDirPath, err := GetProjectDirPath(c.fs, c.os)
243+
projectDirPath, err := GetProjectDirPath(fs, os)
248244
if err != nil {
249245
return projectConfig, err
250246
}
251247

252-
if !c.ProjectConfigJSONFileExists(projectDirPath) {
248+
if !ProjectConfigJSONFileExists(fs, os, projectDirPath) {
253249
return projectConfig, nil
254250
}
255251

256252
var projectConfigFilePath = GetProjectConfigJSONFilePath(projectDirPath)
257-
projectConfigFileBytes, err := afero.ReadFile(c.fs, projectConfigFilePath)
253+
projectConfigFileBytes, err := afero.ReadFile(fs, projectConfigFilePath)
258254
if err != nil {
259255
return projectConfig, err
260256
}
@@ -274,7 +270,7 @@ func (c *ProjectConfig) ReadProjectConfigFile(ctx context.Context) (ProjectConfi
274270
}
275271

276272
// WriteProjectConfigFile writes the project-level config.json file
277-
func (c *ProjectConfig) WriteProjectConfigFile(ctx context.Context, projectConfig ProjectConfig) (string, error) {
273+
func WriteProjectConfigFile(ctx context.Context, fs afero.Fs, os types.Os, projectConfig ProjectConfig) (string, error) {
278274
var span opentracing.Span
279275
span, _ = opentracing.StartSpanFromContext(ctx, "WriteProjectConfigFile")
280276
defer span.Finish()
@@ -284,13 +280,13 @@ func (c *ProjectConfig) WriteProjectConfigFile(ctx context.Context, projectConfi
284280
return "", err
285281
}
286282

287-
projectDirPath, err := GetProjectDirPath(c.fs, c.os)
283+
projectDirPath, err := GetProjectDirPath(fs, os)
288284
if err != nil {
289285
return "", err
290286
}
291287

292288
projectConfigFilePath := GetProjectConfigJSONFilePath(projectDirPath)
293-
err = afero.WriteFile(c.fs, projectConfigFilePath, projectConfigBytes, 0600)
289+
err = afero.WriteFile(fs, projectConfigFilePath, projectConfigBytes, 0644)
294290
if err != nil {
295291
return "", err
296292
}
@@ -299,9 +295,9 @@ func (c *ProjectConfig) WriteProjectConfigFile(ctx context.Context, projectConfi
299295
}
300296

301297
// ProjectConfigJSONFileExists returns true if the .slack/config.json file exists
302-
func (c *ProjectConfig) ProjectConfigJSONFileExists(projectDirPath string) bool {
298+
func ProjectConfigJSONFileExists(fs afero.Fs, os types.Os, projectDirPath string) bool {
303299
var projectConfigFilePath = GetProjectConfigJSONFilePath(projectDirPath)
304-
_, err := c.fs.Stat(projectConfigFilePath)
300+
_, err := fs.Stat(projectConfigFilePath)
305301
return !os.IsNotExist(err)
306302
}
307303

0 commit comments

Comments
 (0)