From 4add4ccd0e754d489ab23c16dd6d3847b886ce97 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 2 Apr 2025 12:44:21 -0700 Subject: [PATCH 1/2] refactor: Add slackcontext package to DRY context management --- .vscode/settings.json | 1 + cmd/env/list.go | 5 +- cmd/root.go | 30 +- cmd/root_test.go | 6 +- internal/api/activity_test.go | 38 ++- internal/api/app_test.go | 50 ++-- internal/api/auth_test.go | 26 +- internal/api/client.go | 19 +- internal/api/datastore_test.go | 17 +- internal/api/externalauth_test.go | 22 +- internal/api/functiondistribution_test.go | 52 ++-- internal/api/icon.go | 7 +- internal/api/icon_test.go | 14 +- internal/api/s3_upload.go | 7 +- internal/api/s3_upload_test.go | 5 +- internal/api/session_test.go | 38 ++- internal/api/steps_test.go | 14 +- internal/api/teams_test.go | 6 +- internal/api/triggerpermissions_test.go | 50 ++-- internal/api/variables_test.go | 38 ++- internal/api/workflows_test.go | 14 +- internal/auth/auth_test.go | 17 +- internal/auth/revoke_test.go | 4 +- internal/config/context.go | 51 ---- internal/contextutil/contextutil.go | 53 ---- internal/iostreams/logfile.go | 15 +- internal/slackcontext/slackcontext.go | 176 +++++++++++ internal/slackcontext/slackcontext_mock.go | 34 +++ internal/slackcontext/slackcontext_test.go | 331 +++++++++++++++++++++ internal/slackerror/errors.go | 28 +- internal/tracking/tracking.go | 7 +- internal/tracking/tracking_test.go | 5 +- main.go | 29 +- 33 files changed, 894 insertions(+), 315 deletions(-) delete mode 100644 internal/contextutil/contextutil.go create mode 100644 internal/slackcontext/slackcontext.go create mode 100644 internal/slackcontext/slackcontext_mock.go create mode 100644 internal/slackcontext/slackcontext_test.go diff --git a/.vscode/settings.json b/.vscode/settings.json index 118c7363..6810f17c 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -16,6 +16,7 @@ "iostreams", "opentracing", "safeexec", + "slackcontext", "slackdeps", "slackerror", "slackhq", diff --git a/cmd/env/list.go b/cmd/env/list.go index 09b1deca..2bb9a71a 100644 --- a/cmd/env/list.go +++ b/cmd/env/list.go @@ -51,7 +51,7 @@ func NewEnvListCommand(clients *shared.ClientFactory) *cobra.Command { return preRunEnvListCommandFunc(ctx, clients) }, RunE: func(cmd *cobra.Command, args []string) error { - return runEnvListCommandFunc(clients) + return runEnvListCommandFunc(clients, cmd) }, } @@ -74,8 +74,9 @@ func preRunEnvListCommandFunc(ctx context.Context, clients *shared.ClientFactory // runEnvListCommandFunc outputs environment variables for a selected app func runEnvListCommandFunc( clients *shared.ClientFactory, + cmd *cobra.Command, ) error { - ctx := context.Background() + ctx := cmd.Context() selection, err := teamAppSelectPromptFunc( ctx, diff --git a/cmd/root.go b/cmd/root.go index 21e9cf2c..7e9f5d6f 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -22,7 +22,6 @@ import ( "strings" "syscall" - "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/cmd/app" "github.com/slackapi/slack-cli/cmd/auth" "github.com/slackapi/slack-cli/cmd/collaborators" @@ -47,6 +46,7 @@ import ( "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/pkg/version" "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" "github.com/slackapi/slack-cli/internal/update" @@ -222,6 +222,7 @@ func Init() (*cobra.Command, *shared.ClientFactory) { // TODO: consider using arguments for this function for certain dependencies, like working directory and other OS-specific strings, that OnInitialize above can provide during actual execution, but that we can override with test values for easier testing. func InitConfig(clients *shared.ClientFactory, rootCmd *cobra.Command) error { ctx := rootCmd.Context() + // Get the current working directory (usually, but not always the project) workingDirPath, err := clients.Os.Getwd() if err != nil { @@ -261,43 +262,35 @@ func InitConfig(clients *shared.ClientFactory, rootCmd *cobra.Command) error { clients.Config.ApiHostResolved = clients.AuthInterface().ResolveApiHost(ctx, clients.Config.ApiHostFlag, nil) clients.Config.LogstashHostResolved = clients.AuthInterface().ResolveLogstashHost(ctx, clients.Config.ApiHostResolved, clients.CliVersion) - // TODO: should we store system ID in config or in context? // Init System ID if systemID, err := clients.Config.SystemConfig.InitSystemID(ctx); err != nil { clients.IO.PrintDebug(ctx, "Error initializing user-level config system_id: %s", err.Error()) } else { // Used by Logstash + // TODO(slackcontext) Consolidate storing SystemID to slackcontext clients.Config.SystemID = systemID - // Used by OpenTracing - if span := opentracing.SpanFromContext(ctx); span != nil { - span.SetTag("system_id", clients.Config.SystemID) - ctx = opentracing.ContextWithSpan(ctx, span) - } - + ctx = slackcontext.SetSystemID(ctx, systemID) + rootCmd.SetContext(ctx) // Debug logging clients.IO.PrintDebug(ctx, "system_id: %s", clients.Config.SystemID) } - // TODO: should we store project ID in config or in context? // Init Project ID, if current directory is a project if projectID, _ := clients.Config.ProjectConfig.InitProjectID(ctx, false); projectID != "" { // Used by Logstash + // TODO(slackcontext) Consolidate storing ProjectID to slackcontext clients.Config.ProjectID = projectID - // Used by OpenTracing - if span := opentracing.SpanFromContext(ctx); span != nil { - span.SetTag("project_id", clients.Config.ProjectID) - ctx = opentracing.ContextWithSpan(ctx, span) - } - + ctx = slackcontext.SetProjectID(ctx, projectID) + rootCmd.SetContext(ctx) // Debug logging clients.IO.PrintDebug(ctx, "project_id: %s", clients.Config.ProjectID) } // Init configurations clients.Config.LoadExperiments(ctx, clients.IO.PrintDebug) - // TODO: consolidate where we store CLI version; here are two locations, plus some code uses `contextutil.ContextFromVersion`, plus pkg/version/version + // TODO(slackcontext) Consolidate storing CLI version to slackcontext clients.Config.Version = clients.CliVersion // The domain auths (token->domain) shouldn't change for the execution of the CLI so preload them into config! @@ -334,10 +327,8 @@ func InitConfig(clients *shared.ClientFactory, rootCmd *cobra.Command) error { // listens for process interrupts and sends to IOStreams' GetInterruptChannel() for use in // in communicating process interrupts elsewhere in the code. func Execute(ctx context.Context, rootCmd *cobra.Command, clients *shared.ClientFactory) { - // TODO: ensure context is only used with setters and not with getters. After investigating, report: - // - internal/contextutil/contextutil.go has a `VersionFromContext` getter method which is used in various API methods when setting the user agent on HTTP requests, and when sending data up to logstash - // - internal/config/context.go has a variety of set and get methods related to app, token, team/enterprise/session/user/trace IDs, domains ctx, cancel := context.WithCancel(ctx) + completedChan := make(chan bool, 1) // completed is used for signalling an end to command exitChan := make(chan bool, 1) // exit blocks the command from exiting until completed interruptChan := make(chan os.Signal, 1) // interrupt catches signals to avoid abrupt exits @@ -366,6 +357,7 @@ func Execute(ctx context.Context, rootCmd *cobra.Command, clients *shared.Client }() case <-ctx.Done(): // No interrupt signal sent, command executed to completion + // FIXME - `.Done()` channel is triggered by `cancel()` and not a successfully completion case <-completedChan: // No canceled context, but command has completed execution exitChan <- true diff --git a/cmd/root_test.go b/cmd/root_test.go index 6c17e8a1..065cb779 100644 --- a/cmd/root_test.go +++ b/cmd/root_test.go @@ -21,12 +21,15 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/test/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) func TestRootCommand(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + tmp, _ := os.MkdirTemp("", "") _ = os.Chdir(tmp) defer os.RemoveAll(tmp) @@ -38,7 +41,7 @@ func TestRootCommand(t *testing.T) { clientsMock := shared.NewClientsMock() testutil.MockCmdIO(clientsMock.IO, cmd) - err := cmd.Execute() + err := cmd.ExecuteContext(ctx) if err != nil { assert.Fail(t, "cmd.Execute had unexpected error") } @@ -131,6 +134,7 @@ func Test_Aliases(t *testing.T) { tmp, _ := os.MkdirTemp("", "") _ = os.Chdir(tmp) defer os.RemoveAll(tmp) + Init() tests := map[string]struct { diff --git a/internal/api/activity_test.go b/internal/api/activity_test.go index b93c9d75..92e8be71 100644 --- a/internal/api/activity_test.go +++ b/internal/api/activity_test.go @@ -15,10 +15,10 @@ package api import ( - "context" "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/stretchr/testify/require" ) @@ -28,11 +28,12 @@ var fakeResult = `{"ok":true, }` func Test_ApiClient_ActivityErrorsIfAppIdIsEmpty(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, }) defer teardown() - _, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + _, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "", }) require.Error(t, err) @@ -40,13 +41,14 @@ func Test_ApiClient_ActivityErrorsIfAppIdIsEmpty(t *testing.T) { } func Test_ApiClient_ActivityBasicSuccessfulGET(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", }) require.NoError(t, err) @@ -54,13 +56,14 @@ func Test_ApiClient_ActivityBasicSuccessfulGET(t *testing.T) { } func Test_ApiClient_ActivityEventType(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&log_event_type=silly", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", EventType: "silly", }) @@ -69,13 +72,14 @@ func Test_ApiClient_ActivityEventType(t *testing.T) { } func Test_ApiClient_ActivityLogLevel(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&min_log_level=silly", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", MinimumLogLevel: "silly", }) @@ -84,13 +88,14 @@ func Test_ApiClient_ActivityLogLevel(t *testing.T) { } func Test_ApiClient_ActivityMinDateCreated(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&min_date_created=1337", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", MinimumDateCreated: 1337, }) @@ -99,13 +104,14 @@ func Test_ApiClient_ActivityMinDateCreated(t *testing.T) { } func Test_ApiClient_ActivityComponentType(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&component_type=defirbulator", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", ComponentType: "defirbulator", }) @@ -114,13 +120,14 @@ func Test_ApiClient_ActivityComponentType(t *testing.T) { } func Test_ApiClient_ActivityComponentId(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&component_id=raspberry", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", ComponentId: "raspberry", }) @@ -129,13 +136,14 @@ func Test_ApiClient_ActivityComponentId(t *testing.T) { } func Test_ApiClient_ActivitySource(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&source=beer", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", Source: "beer", }) @@ -144,13 +152,14 @@ func Test_ApiClient_ActivitySource(t *testing.T) { } func Test_ApiClient_ActivityTraceId(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123&limit=0&trace_id=stealth", Response: fakeResult, }) defer teardown() - result, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + result, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", TraceId: "stealth", }) @@ -159,13 +168,14 @@ func Test_ApiClient_ActivityTraceId(t *testing.T) { } func Test_ApiClient_ActivityResponseNotOK(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123", Response: `{"ok":false, "error": "internal_error"}`, }) defer teardown() - _, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + _, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", }) require.Error(t, err) @@ -173,13 +183,14 @@ func Test_ApiClient_ActivityResponseNotOK(t *testing.T) { } func Test_ApiClient_ActivityInvalidResponse(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123", Response: `badjson`, }) defer teardown() - _, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + _, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", }) require.Error(t, err) @@ -187,13 +198,14 @@ func Test_ApiClient_ActivityInvalidResponse(t *testing.T) { } func Test_ApiClient_ActivityInvalidJSON(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appActivityMethod, ExpectedQuerystring: "app_id=A123", Response: `badtime`, }) defer teardown() - _, err := c.Activity(context.Background(), "token", types.ActivityRequest{ + _, err := c.Activity(ctx, "token", types.ActivityRequest{ AppId: "A123", }) require.Error(t, err) diff --git a/internal/api/app_test.go b/internal/api/app_test.go index d8215719..4e2bb361 100644 --- a/internal/api/app_test.go +++ b/internal/api/app_test.go @@ -26,6 +26,7 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/stretchr/testify/mock" @@ -33,13 +34,14 @@ import ( ) func TestClient_CreateApp_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appManifestCreateMethod, ExpectedRequest: `{"manifest":{"display_information":{"name":"TestApp"}},"enable_distribution":true}`, Response: `{"ok":true,"app_id":"A123"}`, }) defer teardown() - result, err := c.CreateApp(context.Background(), "token", types.AppManifest{ + result, err := c.CreateApp(ctx, "token", types.AppManifest{ DisplayInformation: types.DisplayInformation{ Name: "TestApp", }, @@ -49,39 +51,43 @@ func TestClient_CreateApp_Ok(t *testing.T) { } func TestClient_CreateApp_CommonErrors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) verifyCommonErrorCases(t, appManifestCreateMethod, func(c *Client) error { - _, err := c.CreateApp(context.Background(), "token", types.AppManifest{}, false) + _, err := c.CreateApp(ctx, "token", types.AppManifest{}, false) return err }) } func TestClient_ExportAppManifest_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appManifestExportMethod, ExpectedRequest: `{"app_id":"A0123456789"}`, Response: `{"ok":true,"manifest":{"display_information":{"name":"example"}}}`, }) defer teardown() - result, err := c.ExportAppManifest(context.Background(), "token", "A0123456789") + result, err := c.ExportAppManifest(ctx, "token", "A0123456789") require.NoError(t, err) require.Equal(t, "example", result.Manifest.AppManifest.DisplayInformation.Name) } func TestClient_ExportAppManifest_CommonErrors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) verifyCommonErrorCases(t, appManifestExportMethod, func(c *Client) error { - _, err := c.ExportAppManifest(context.Background(), "token", "A0000000001") + _, err := c.ExportAppManifest(ctx, "token", "A0000000001") return err }) } func TestClient_UpdateApp_OK(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appManifestUpdateMethod, ExpectedRequest: `{"manifest":{"display_information":{"name":"TestApp"},"outgoing_domains":[]},"app_id":"A123","force_update":true,"consent_breaking_changes":true}`, Response: `{"ok":true,"app_id":"A123"}`, }) defer teardown() - result, err := c.UpdateApp(context.Background(), "token", "A123", types.AppManifest{ + result, err := c.UpdateApp(ctx, "token", "A123", types.AppManifest{ DisplayInformation: types.DisplayInformation{ Name: "TestApp", }, @@ -94,8 +100,9 @@ func TestClient_UpdateApp_OK(t *testing.T) { } func TestClient_UpdateApp_CommonErrors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) verifyCommonErrorCases(t, appManifestUpdateMethod, func(c *Client) error { - _, err := c.UpdateApp(context.Background(), "token", "A123", types.AppManifest{}, + _, err := c.UpdateApp(ctx, "token", "A123", types.AppManifest{}, /* forceUpdate= */ true, /* continueWithBreakingChanges= */ true) return err @@ -103,12 +110,13 @@ func TestClient_UpdateApp_CommonErrors(t *testing.T) { } func TestClient_UpdateApp_SchemaCompatibilityError(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appManifestUpdateMethod, Response: `{"ok": false, "error": "err", "errors": [{"message": "schema_compatibility_error: Following datatstore(s) will be deleted after the deploy: tasks"}]}`, }) defer teardown() - _, err := c.UpdateApp(context.Background(), "token", "A123", types.AppManifest{}, + _, err := c.UpdateApp(ctx, "token", "A123", types.AppManifest{}, /* forceUpdate= */ true, /* continueWithBreakingChanges= */ true) require.Error(t, err) @@ -247,11 +255,13 @@ func TestClient_ValidateAppManifest(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + var ts = httptest.NewServer(http.HandlerFunc(tt.handlerFunc)) defer ts.Close() c := NewClient(&http.Client{}, ts.URL, nil) - got, err := c.ValidateAppManifest(context.Background(), tt.args.token, tt.args.manifest, tt.args.appID) + got, err := c.ValidateAppManifest(ctx, tt.args.token, tt.args.manifest, tt.args.appID) if tt.wantErr { require.Error(t, err) require.Contains(t, err.Error(), tt.wantErrVal.Error()) @@ -264,13 +274,14 @@ func TestClient_ValidateAppManifest(t *testing.T) { } func TestClient_GetPresignedS3PostParams_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appGeneratePresignedPostMethod, ExpectedRequest: `{"app_id":"A123"}`, Response: `{"ok":true,"url":"example.com/upload","file_name":"foo.tar.gz","fields":{"X-Amz-Credential":"cred"}}`, }) defer teardown() - result, err := c.GetPresignedS3PostParams(context.Background(), "token", "A123") + result, err := c.GetPresignedS3PostParams(ctx, "token", "A123") require.NoError(t, err) require.Equal(t, "foo.tar.gz", result.FileName) require.Equal(t, "example.com/upload", result.Url) @@ -278,8 +289,9 @@ func TestClient_GetPresignedS3PostParams_Ok(t *testing.T) { } func TestClient_GetPresignedS3PostParams_CommonErrors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) verifyCommonErrorCases(t, appGeneratePresignedPostMethod, func(c *Client) error { - _, err := c.GetPresignedS3PostParams(context.Background(), "token", "A123") + _, err := c.GetPresignedS3PostParams(ctx, "token", "A123") return err }) } @@ -298,6 +310,7 @@ func TestClient_CertifiedAppInstall(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -316,7 +329,7 @@ func TestClient_CertifiedAppInstall(t *testing.T) { mockAppId := "A123" // execute - _, err := c.CertifiedAppInstall(context.Background(), "token", mockAppId) + _, err := c.CertifiedAppInstall(ctx, "token", mockAppId) // check if (err != nil) != tt.wantErr { @@ -339,7 +352,7 @@ func TestClient_InstallApp(t *testing.T) { var setup = func(t *testing.T) (context.Context, *iostreams.IOStreamsMock) { // Mocks - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -368,7 +381,7 @@ func TestClient_InstallApp(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, ioMock := setup(t) + ctx, ioMock := setup(t) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -391,7 +404,7 @@ func TestClient_InstallApp(t *testing.T) { TeamDomain: "mock", } // execute - _, _, err := c.DeveloperAppInstall(context.Background(), ioMock, "token", mockApp, []string{}, []string{}, "", false) + _, _, err := c.DeveloperAppInstall(ctx, ioMock, "token", mockApp, []string{}, []string{}, "", false) // check if (err != nil) != tt.wantErr { @@ -430,6 +443,7 @@ func TestClient_UninstallApp(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -447,7 +461,7 @@ func TestClient_UninstallApp(t *testing.T) { c := NewClient(&http.Client{}, ts.URL, nil) // execute - err := c.UninstallApp(context.Background(), "token", "A123", "T123") + err := c.UninstallApp(ctx, "token", "A123", "T123") // check if (err != nil) != tt.wantErr { @@ -486,6 +500,7 @@ func TestClient_DeleteApp(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -503,7 +518,7 @@ func TestClient_DeleteApp(t *testing.T) { c := NewClient(&http.Client{}, ts.URL, nil) // execute - err := c.DeleteApp(context.Background(), "token", "A123") + err := c.DeleteApp(ctx, "token", "A123") // check if (err != nil) != tt.wantErr { @@ -559,6 +574,7 @@ func TestClient_DeveloperAppInstall_RequestAppApproval(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -590,7 +606,7 @@ func TestClient_DeveloperAppInstall_RequestAppApproval(t *testing.T) { iostreamMock.On("PrintTrace", mock.Anything, mock.Anything, mock.Anything).Return() // execute - _, _, err := c.DeveloperAppInstall(context.Background(), iostreamMock, "token", tt.app, []string{}, []string{}, tt.orgGrantWorkspaceID, true) + _, _, err := c.DeveloperAppInstall(ctx, iostreamMock, "token", tt.app, []string{}, []string{}, tt.orgGrantWorkspaceID, true) require.NoError(t, err) }) } diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 71408508..1abc4cbf 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -15,47 +15,50 @@ package api import ( - "context" "testing" "time" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) func TestClient_GenerateAuthTicket_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: generateAuthTicketMethod, ExpectedRequest: ``, Response: `{"ok":true,"ticket":"valid-ticket"}`, }) defer teardown() - result, err := c.GenerateAuthTicket(context.Background(), "", false) + result, err := c.GenerateAuthTicket(ctx, "", false) require.NoError(t, err) require.Equal(t, "valid-ticket", result.Ticket) } func TestClient_GenerateAuthTicket_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: generateAuthTicketMethod, ExpectedRequest: ``, Response: `{"ok":false,"error":"fatal_error"}`, }) defer teardown() - _, err := c.GenerateAuthTicket(context.Background(), "", false) + _, err := c.GenerateAuthTicket(ctx, "", false) require.Error(t, err) require.Contains(t, err.Error(), "fatal_error") require.Contains(t, err.Error(), generateAuthTicketMethod) } func TestClient_ExchangeAuthTicket_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: exchangeAuthTicketMethod, ExpectedRequest: `challenge=valid-challenge&slack_cli_version=&ticket=valid-ticket`, Response: `{"ok":true,"is_ready":true,"token":"valid-token","refresh_token":"valid-refresh-token"}`, }) defer teardown() - result, err := c.ExchangeAuthTicket(context.Background(), "valid-ticket", "valid-challenge", "") + result, err := c.ExchangeAuthTicket(ctx, "valid-ticket", "valid-challenge", "") require.NoError(t, err) require.True(t, result.IsReady) require.Equal(t, "valid-token", result.Token) @@ -63,24 +66,26 @@ func TestClient_ExchangeAuthTicket_Ok(t *testing.T) { } func TestClient_ExchangeAuthTicket_Ok_MissingToken(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: exchangeAuthTicketMethod, ExpectedRequest: `challenge=dummychallenge&slack_cli_version=&ticket=valid-ticket`, Response: `{"ok":true,"token":"","refresh_token":""}`, }) defer teardown() - _, err := c.ExchangeAuthTicket(context.Background(), "valid-ticket", "dummychallenge", "") + _, err := c.ExchangeAuthTicket(ctx, "valid-ticket", "dummychallenge", "") require.Error(t, err, "No token returned from the following Slack API method") } func TestClient_ExchangeAuthTicket_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: exchangeAuthTicketMethod, ExpectedRequest: `challenge=valid-challenge&slack_cli_version=&ticket=valid-ticket`, Response: `{"ok":false,"error":"fatal_error"}`, }) defer teardown() - result, err := c.ExchangeAuthTicket(context.Background(), "valid-ticket", "valid-challenge", "") + result, err := c.ExchangeAuthTicket(ctx, "valid-ticket", "valid-challenge", "") require.False(t, result.IsReady) require.Error(t, err) require.Contains(t, err.Error(), "fatal_error") @@ -88,6 +93,7 @@ func TestClient_ExchangeAuthTicket_Error(t *testing.T) { } func TestClient_RotateToken_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: rotateTokenMethod, ExpectedRequest: `refresh_token=valid-refresh-token`, @@ -95,7 +101,7 @@ func TestClient_RotateToken_Ok(t *testing.T) { }) defer teardown() c.httpClient.Timeout = 60 * time.Second - result, err := c.RotateToken(context.Background(), types.SlackAuth{ + result, err := c.RotateToken(ctx, types.SlackAuth{ Token: `valid-token`, RefreshToken: `valid-refresh-token`, }) @@ -106,6 +112,7 @@ func TestClient_RotateToken_Ok(t *testing.T) { } func TestClient_RotateToken_OkDevHostRestoreTimeout(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: rotateTokenMethod, ExpectedRequest: `refresh_token=valid-refresh-token`, @@ -115,7 +122,7 @@ func TestClient_RotateToken_OkDevHostRestoreTimeout(t *testing.T) { devHost := "https://dev1234.slack.com" c.host = devHost c.httpClient.Timeout = 24 * time.Second - _, err := c.RotateToken(context.Background(), types.SlackAuth{ + _, err := c.RotateToken(ctx, types.SlackAuth{ ApiHost: &devHost, Token: `valid-token`, RefreshToken: `valid-refresh-token`, @@ -125,13 +132,14 @@ func TestClient_RotateToken_OkDevHostRestoreTimeout(t *testing.T) { } func TestClient_RotateToken_Error(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: rotateTokenMethod, ExpectedRequest: `refresh_token=valid-refresh-token`, Response: `{"ok":false,"error":"fatal_error"}`, }) defer teardown() - _, err := c.RotateToken(context.Background(), types.SlackAuth{ + _, err := c.RotateToken(ctx, types.SlackAuth{ Token: `valid-token`, RefreshToken: `valid-refresh-token`, }) diff --git a/internal/api/client.go b/internal/api/client.go index 1e336f5c..a3c3a3f6 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -30,9 +30,9 @@ import ( "github.com/opentracing/opentracing-go" "github.com/pkg/errors" "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/contextutil" "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/uber/jaeger-client-go" @@ -114,7 +114,11 @@ func (c *Client) postForm(ctx context.Context, endpoint string, formValues url.V if err != nil { return nil, err } - cliVersion := contextutil.VersionFromContext(ctx) + + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + return nil, err + } var userAgent = fmt.Sprintf("slack-cli/%s (os: %s)", cliVersion, runtime.GOOS) request.Header.Add("content-type", "application/x-www-form-urlencoded") @@ -162,9 +166,11 @@ func (c *Client) postJSON(ctx context.Context, endpoint, token string, cookie st request.Header.Add("Authorization", "Bearer "+token) } - cliVersion := contextutil.VersionFromContext(ctx) + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + return nil, err + } var userAgent = fmt.Sprintf("slack-cli/%s (os: %s)", cliVersion, runtime.GOOS) - request.Header.Add("User-Agent", userAgent) if jaegerSpanContext, ok := span.Context().(jaeger.SpanContext); ok { request.Header.Add("x-b3-sampled", "0") @@ -213,7 +219,10 @@ func (c *Client) get(ctx context.Context, endpoint, token string, cookie string) request.Header.Add("Authorization", "Bearer "+token) } - cliVersion := contextutil.VersionFromContext(ctx) + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + return nil, err + } var userAgent = fmt.Sprintf("slack-cli/%s (os: %s)", cliVersion, runtime.GOOS) request.Header.Add("User-Agent", userAgent) diff --git a/internal/api/datastore_test.go b/internal/api/datastore_test.go index 151ad7d6..6d6540a2 100644 --- a/internal/api/datastore_test.go +++ b/internal/api/datastore_test.go @@ -15,13 +15,13 @@ package api import ( - "context" "fmt" "net/http" "net/http/httptest" "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/stretchr/testify/require" ) @@ -127,10 +127,11 @@ func TestClient_AppsDatastorePut(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) ts := httptest.NewServer(http.HandlerFunc(tt.testHandler.handlerFunc)) defer ts.Close() apiClient := NewClient(&http.Client{}, ts.URL, nil) - got, err := apiClient.AppsDatastorePut(context.Background(), "shhh", tt.args.request) + got, err := apiClient.AppsDatastorePut(ctx, "shhh", tt.args.request) if (err != nil) != tt.wantErr { t.Errorf("Client.AppsDatastorePut() error = %v, wantErr %v", err, tt.wantErr) return @@ -249,10 +250,11 @@ func TestClient_AppsDatastoreUpdate(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) ts := httptest.NewServer(http.HandlerFunc(tt.testHandler.handlerFunc)) defer ts.Close() apiClient := NewClient(&http.Client{}, ts.URL, nil) - got, err := apiClient.AppsDatastoreUpdate(context.Background(), "shhh", tt.args.request) + got, err := apiClient.AppsDatastoreUpdate(ctx, "shhh", tt.args.request) if (err != nil) != tt.wantErr { t.Errorf("Client.AppsDatastoreUpdate() error = %v, wantErr %v", err, tt.wantErr) return @@ -359,10 +361,11 @@ func TestClient_AppsDatastoreQuery(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) ts := httptest.NewServer(http.HandlerFunc(tt.testHandler.handlerFunc)) defer ts.Close() apiClient := NewClient(&http.Client{}, ts.URL, nil) - got, err := apiClient.AppsDatastoreQuery(context.Background(), "shhh", tt.args.query) + got, err := apiClient.AppsDatastoreQuery(ctx, "shhh", tt.args.query) if (err != nil) != tt.wantErr { t.Errorf("Client.AppsDatastoreQuery() error = %v, wantErr %v", err, tt.wantErr) return @@ -470,10 +473,11 @@ func TestClient_AppsDatastoreDelete(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) ts := httptest.NewServer(http.HandlerFunc(tt.testHandler.handlerFunc)) defer ts.Close() apiClient := NewClient(&http.Client{}, ts.URL, nil) - got, err := apiClient.AppsDatastoreDelete(context.Background(), "shhh", tt.args.request) + got, err := apiClient.AppsDatastoreDelete(ctx, "shhh", tt.args.request) if (err != nil) != tt.wantErr { t.Errorf("Client.AppsDatastoreDelete() error = %v, wantErr %v", err, tt.wantErr) return @@ -583,10 +587,11 @@ func TestClient_AppsDatastoreGet(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) ts := httptest.NewServer(http.HandlerFunc(tt.testHandler.handlerFunc)) defer ts.Close() apiClient := NewClient(&http.Client{}, ts.URL, nil) - got, err := apiClient.AppsDatastoreGet(context.Background(), "shhh", tt.args.request) + got, err := apiClient.AppsDatastoreGet(ctx, "shhh", tt.args.request) if (err != nil) != tt.wantErr { t.Errorf("Client.AppsDatastoreGet() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/internal/api/externalauth_test.go b/internal/api/externalauth_test.go index 8ff82797..df3f6174 100644 --- a/internal/api/externalauth_test.go +++ b/internal/api/externalauth_test.go @@ -15,13 +15,13 @@ package api import ( - "context" "fmt" "net/http" "net/http/httptest" "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) @@ -66,6 +66,8 @@ func Test_API_AppsAuthExternalStart(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + // Setup HTTP test server httpHandlerFunc := func(w http.ResponseWriter, r *http.Request) { json := tt.httpResponseJSON @@ -77,7 +79,7 @@ func Test_API_AppsAuthExternalStart(t *testing.T) { apiClient := NewClient(&http.Client{}, ts.URL, nil) // Execute test - authorizationURL, err := apiClient.AppsAuthExternalStart(context.Background(), tt.argsToken, tt.argsAppID, tt.argsProviderKey) + authorizationURL, err := apiClient.AppsAuthExternalStart(ctx, tt.argsToken, tt.argsAppID, tt.argsProviderKey) // Assertions require.Equal(t, tt.expectedAuthorizationURL, authorizationURL) @@ -127,6 +129,8 @@ func Test_API_AppsAuthExternalRemove(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + // Setup HTTP test server httpHandlerFunc := func(w http.ResponseWriter, r *http.Request) { json := tt.httpResponseJSON @@ -138,7 +142,7 @@ func Test_API_AppsAuthExternalRemove(t *testing.T) { apiClient := NewClient(&http.Client{}, ts.URL, nil) // Execute test - err := apiClient.AppsAuthExternalDelete(context.Background(), tt.argsToken, tt.argsAppID, tt.argsProviderKey, "") + err := apiClient.AppsAuthExternalDelete(ctx, tt.argsToken, tt.argsAppID, tt.argsProviderKey, "") // Assertions if tt.expectedErrorContains == "" { @@ -190,6 +194,8 @@ func Test_API_AppsAuthExternalClientSecretAdd(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + // Setup HTTP test server httpHandlerFunc := func(w http.ResponseWriter, r *http.Request) { json := tt.httpResponseJSON @@ -201,7 +207,7 @@ func Test_API_AppsAuthExternalClientSecretAdd(t *testing.T) { apiClient := NewClient(&http.Client{}, ts.URL, nil) // Execute test - err := apiClient.AppsAuthExternalClientSecretAdd(context.Background(), tt.argsToken, tt.argsAppID, tt.argsProviderKey, tt.argsClientSecret) + err := apiClient.AppsAuthExternalClientSecretAdd(ctx, tt.argsToken, tt.argsAppID, tt.argsProviderKey, tt.argsClientSecret) // Assertions if tt.expectedErrorContains == "" { @@ -367,6 +373,8 @@ func Test_API_AppsAuthExternalList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + // Setup HTTP test server httpHandlerFunc := func(w http.ResponseWriter, r *http.Request) { json := tt.httpResponseJSON @@ -378,7 +386,7 @@ func Test_API_AppsAuthExternalList(t *testing.T) { apiClient := NewClient(&http.Client{}, ts.URL, nil) // Execute test - actual, err := apiClient.AppsAuthExternalList(context.Background(), tt.argsToken, tt.argsAppID, false /*include_workflows flag to return workflow auth info*/) + actual, err := apiClient.AppsAuthExternalList(ctx, tt.argsToken, tt.argsAppID, false /*include_workflows flag to return workflow auth info*/) // Assertions if tt.expectedErrorContains == "" { @@ -438,6 +446,8 @@ func Test_API_AppsAuthExternalSelectAuth(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + // Setup HTTP test server httpHandlerFunc := func(w http.ResponseWriter, r *http.Request) { json := tt.httpResponseJSON @@ -449,7 +459,7 @@ func Test_API_AppsAuthExternalSelectAuth(t *testing.T) { apiClient := NewClient(&http.Client{}, ts.URL, nil) // Execute test - err := apiClient.AppsAuthExternalSelectAuth(context.Background(), tt.argsToken, tt.argsAppID, tt.argsProviderKey, tt.argsWorkflowId, tt.argsExternalTokenId) + err := apiClient.AppsAuthExternalSelectAuth(ctx, tt.argsToken, tt.argsAppID, tt.argsProviderKey, tt.argsWorkflowId, tt.argsExternalTokenId) // Assertions if tt.expectedErrorContains == "" { diff --git a/internal/api/functiondistribution_test.go b/internal/api/functiondistribution_test.go index 3beddae5..b29ad623 100644 --- a/internal/api/functiondistribution_test.go +++ b/internal/api/functiondistribution_test.go @@ -15,13 +15,13 @@ package api import ( - "context" "fmt" "net/http" "net/http/httptest" "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) @@ -30,7 +30,7 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { name string expectedPath string resultJson string - testFunc func(c *Client) error + testFunc func(t *testing.T, c *Client) error want string wantErr bool errMessage string @@ -38,15 +38,17 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { { name: "Add user success", resultJson: `{"ok": true, "distribution_type": "named_entities", "user_ids": ["user1", "user2"]}`, - testFunc: func(c *Client) error { - return c.FunctionDistributionAddUsers(context.Background(), "valid_function", "app", "user1,user2") + testFunc: func(t *testing.T, c *Client) error { + ctx := slackcontext.MockContext(t.Context()) + return c.FunctionDistributionAddUsers(ctx, "valid_function", "app", "user1,user2") }, }, { name: "Add user: validation error", resultJson: `{"ok": false, "error":"user_not_found"}`, - testFunc: func(c *Client) error { - return c.FunctionDistributionAddUsers(context.Background(), "valid_function", "app", "user1,user2") + testFunc: func(t *testing.T, c *Client) error { + ctx := slackcontext.MockContext(t.Context()) + return c.FunctionDistributionAddUsers(ctx, "valid_function", "app", "user1,user2") }, wantErr: true, errMessage: "user_not_found", @@ -54,15 +56,17 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { { name: "Remove user success", resultJson: `{"ok": true, "distribution_type": "named_entities", "user_ids": []}`, - testFunc: func(c *Client) error { - return c.FunctionDistributionRemoveUsers(context.Background(), "valid_function", "app", "user1,user2") + testFunc: func(t *testing.T, c *Client) error { + ctx := slackcontext.MockContext(t.Context()) + return c.FunctionDistributionRemoveUsers(ctx, "valid_function", "app", "user1,user2") }, }, { name: "Remove user: distribution type not named_entitied", resultJson: `{"ok":false,"error":"invalid_distribution_type"}`, - testFunc: func(c *Client) error { - return c.FunctionDistributionRemoveUsers(context.Background(), "valid_function", "app", "user1,user2") + testFunc: func(t *testing.T, c *Client) error { + ctx := slackcontext.MockContext(t.Context()) + return c.FunctionDistributionRemoveUsers(ctx, "valid_function", "app", "user1,user2") }, wantErr: true, errMessage: "invalid_distribution_type", @@ -70,16 +74,18 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { { name: "Set access type success", resultJson: `{"ok": true, "distribution_type": "everyone", "user_ids": []}`, - testFunc: func(c *Client) error { - _, err := c.FunctionDistributionSet(context.Background(), "valid_function", "app", types.EVERYONE, "") + testFunc: func(t *testing.T, c *Client) error { + ctx := slackcontext.MockContext(t.Context()) + _, err := c.FunctionDistributionSet(ctx, "valid_function", "app", types.EVERYONE, "") return err }, }, { name: "Set access type: access type not recognized by backend", resultJson: `{"ok":false,"error":"invalid_arguments"}`, - testFunc: func(c *Client) error { - _, err := c.FunctionDistributionSet(context.Background(), "valid_function", "app", types.EVERYONE, "") + testFunc: func(t *testing.T, c *Client) error { + ctx := slackcontext.MockContext(t.Context()) + _, err := c.FunctionDistributionSet(ctx, "valid_function", "app", types.EVERYONE, "") return err }, wantErr: true, @@ -89,7 +95,6 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { result := tt.resultJson @@ -101,7 +106,7 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { c := NewClient(&http.Client{}, ts.URL, nil) // execute - err := tt.testFunc(c) + err := tt.testFunc(t, c) // check if (err != nil) != tt.wantErr { @@ -121,55 +126,60 @@ func TestClient_AddRemoveSetAccess(t *testing.T) { } func TestClient_FunctionDistributionList_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: functionDistributionsPermissionsListMethod, Response: `{"ok":true,"distribution_type":"everyone","users":[{"user_id":"W123","username":"grace","email":"grace@gmail.com"}]}`, }) defer teardown() - _, _, err := c.FunctionDistributionList(context.Background(), "dummy_callback_id", "dummy_app_id") + _, _, err := c.FunctionDistributionList(ctx, "dummy_callback_id", "dummy_app_id") require.NoError(t, err) } func TestClient_FunctionDistributionList_HTTPRequestFailed(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: functionDistributionsPermissionsListMethod, StatusCode: http.StatusInternalServerError, }) defer teardown() - _, _, err := c.FunctionDistributionList(context.Background(), "dummy_callback_id", "dummy_app_id") + _, _, err := c.FunctionDistributionList(ctx, "dummy_callback_id", "dummy_app_id") require.Error(t, err) require.Contains(t, err.Error(), "http_request_failed") } func TestClient_FunctionDistributionList_HTTPResponseInvalid(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: functionDistributionsPermissionsListMethod, Response: "}{", }) defer teardown() - _, _, err := c.FunctionDistributionList(context.Background(), "dummy_callback_id", "dummy_app_id") + _, _, err := c.FunctionDistributionList(ctx, "dummy_callback_id", "dummy_app_id") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") } func TestClient_FunctionDistributionList_NotOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: functionDistributionsPermissionsListMethod, Response: `{"ok":false}`, }) defer teardown() - _, _, err := c.FunctionDistributionList(context.Background(), "dummy_callback_id", "dummy_app_id") + _, _, err := c.FunctionDistributionList(ctx, "dummy_callback_id", "dummy_app_id") require.Error(t, err) require.Contains(t, err.Error(), functionDistributionsPermissionsListMethod) } func TestClient_FunctionDistributionList_InvalidDistType(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: functionDistributionsPermissionsListMethod, Response: `{"ok":true,"distribution_type":"banana"}`, }) defer teardown() - _, _, err := c.FunctionDistributionList(context.Background(), "dummy_callback_id", "dummy_app_id") + _, _, err := c.FunctionDistributionList(ctx, "dummy_callback_id", "dummy_app_id") require.Error(t, err) require.Contains(t, err.Error(), "unrecognized access type banana") } diff --git a/internal/api/icon.go b/internal/api/icon.go index 0fd7ffb6..087e7f58 100644 --- a/internal/api/icon.go +++ b/internal/api/icon.go @@ -27,8 +27,8 @@ import ( "runtime" "github.com/opentracing/opentracing-go" - "github.com/slackapi/slack-cli/internal/contextutil" "github.com/slackapi/slack-cli/internal/image" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/spf13/afero" ) @@ -115,7 +115,10 @@ func (c *Client) Icon(ctx context.Context, fs afero.Fs, token, appID, iconFilePa } request.Header.Add("Content-Type", writer.FormDataContentType()) request.Header.Add("Authorization", "Bearer "+token) - cliVersion := contextutil.VersionFromContext(ctx) + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + return IconResult{}, err + } var userAgent = fmt.Sprintf("slack-cli/%s (os: %s)", cliVersion, runtime.GOOS) request.Header.Add("User-Agent", userAgent) diff --git a/internal/api/icon_test.go b/internal/api/icon_test.go index 711db40b..b5833ba8 100644 --- a/internal/api/icon_test.go +++ b/internal/api/icon_test.go @@ -15,13 +15,13 @@ package api import ( - "context" "image" "image/color" "image/png" "math/rand" "testing" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/spf13/afero" "github.com/stretchr/testify/require" ) @@ -29,28 +29,31 @@ import ( var imgFile = ".assets/icon.png" func TestClient_IconErrorIfMissingArgs(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) fs := afero.NewMemMapFs() c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appIconMethod, }) defer teardown() - _, err := c.Icon(context.Background(), fs, "token", "", "") + _, err := c.Icon(ctx, fs, "token", "", "") require.Error(t, err) require.Contains(t, err.Error(), "missing required args") } func TestClient_IconErrorNoFile(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) fs := afero.NewMemMapFs() c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: appIconMethod, }) defer teardown() - _, err := c.Icon(context.Background(), fs, "token", "12345", imgFile) + _, err := c.Icon(ctx, fs, "token", "12345", imgFile) require.Error(t, err) require.Contains(t, err.Error(), "file does not exist") } func TestClient_IconErrorWrongFile(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) fs := afero.NewMemMapFs() err := afero.WriteFile(fs, "test.txt", []byte("this is a text file"), 0666) require.NoError(t, err) @@ -58,12 +61,13 @@ func TestClient_IconErrorWrongFile(t *testing.T) { ExpectedMethod: appIconMethod, }) defer teardown() - _, err = c.Icon(context.Background(), fs, "token", "12345", "test.txt") + _, err = c.Icon(ctx, fs, "token", "12345", "test.txt") require.Error(t, err) require.Contains(t, err.Error(), "unknown format") } func TestClient_IconSuccess(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) fs := afero.NewMemMapFs() myimage := image.NewRGBA(image.Rectangle{image.Point{0, 0}, image.Point{100, 100}}) @@ -83,6 +87,6 @@ func TestClient_IconSuccess(t *testing.T) { Response: `{"ok":true}`, }) defer teardown() - _, err = c.Icon(context.Background(), fs, "token", "12345", imgFile) + _, err = c.Icon(ctx, fs, "token", "12345", imgFile) require.NoError(t, err) } diff --git a/internal/api/s3_upload.go b/internal/api/s3_upload.go index 8626317e..fcfbf081 100644 --- a/internal/api/s3_upload.go +++ b/internal/api/s3_upload.go @@ -27,7 +27,7 @@ import ( "runtime" "github.com/opentracing/opentracing-go" - "github.com/slackapi/slack-cli/internal/contextutil" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/spf13/afero" ) @@ -106,7 +106,10 @@ func (c *Client) UploadPackageToS3(ctx context.Context, fs afero.Fs, appID strin } request.Header.Add("Content-Type", writer.FormDataContentType()) request.Header.Add("Content-MD5", md5s) - cliVersion := contextutil.VersionFromContext(ctx) + cliVersion, err := slackcontext.Version(ctx) + if err != nil { + return fileName, err + } var userAgent = fmt.Sprintf("slack-cli/%s (os: %s)", cliVersion, runtime.GOOS) request.Header.Add("User-Agent", userAgent) diff --git a/internal/api/s3_upload_test.go b/internal/api/s3_upload_test.go index f2d6ac61..0aa94f16 100644 --- a/internal/api/s3_upload_test.go +++ b/internal/api/s3_upload_test.go @@ -15,7 +15,6 @@ package api import ( - "context" "io" "mime" "mime/multipart" @@ -23,11 +22,13 @@ import ( "net/http/httptest" "testing" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/spf13/afero" "github.com/stretchr/testify/require" ) func TestClient_UploadPackageToS3(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) fs := afero.NewMemMapFs() err := afero.WriteFile(fs, "foo.txt", []byte("this is the package"), 0666) require.NoError(t, err) @@ -87,7 +88,7 @@ func TestClient_UploadPackageToS3(t *testing.T) { AmzToken: token, }, } - resp, err := client.UploadPackageToS3(context.Background(), fs, "appID", s3Params, "foo.txt") + resp, err := client.UploadPackageToS3(ctx, fs, "appID", s3Params, "foo.txt") require.NoError(t, err) require.Equal(t, "foo.txt", resp) } diff --git a/internal/api/session_test.go b/internal/api/session_test.go index 09600e2a..0c0f40f8 100644 --- a/internal/api/session_test.go +++ b/internal/api/session_test.go @@ -15,20 +15,21 @@ package api import ( - "context" "net/http" "testing" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) func TestClient_ValidateSession_AuthRespOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, Response: `{"ok":true,"url":"https://www.example.com/","team":"Example Workspace", "user":"grace","team_id":"T123","user_id":"W123","enterprise_id":"E123","is_enterprise_install":true}`, }) defer teardown() - resp, err := c.ValidateSession(context.Background(), "token") + resp, err := c.ValidateSession(ctx, "token") require.NoError(t, err) require.Equal(t, *resp.UserName, "grace") require.Equal(t, *resp.UserID, "W123") @@ -40,124 +41,135 @@ func TestClient_ValidateSession_AuthRespOk(t *testing.T) { } func TestClient_ValidateSession_AuthRespOk_MissingUserID(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, Response: `{"ok":true,"url":"https://www.example.com/","team":"Example Workspace", "user":"grace","team_id":"T123"}`, }) defer teardown() - _, err := c.ValidateSession(context.Background(), "token") + _, err := c.ValidateSession(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") require.Contains(t, err.Error(), "invalid user_id") } func TestClient_ValidateSession_AuthRespNotOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, Response: `{"ok":false}`, }) defer teardown() - _, err := c.ValidateSession(context.Background(), "token") + _, err := c.ValidateSession(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), sessionValidateMethod) require.Contains(t, err.Error(), "unknown_error") } func TestClient_ValidateSession_AuthRespNotOk_NotAuthed(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, Response: `{"ok":false,"error":"not_authed"}`, }) defer teardown() - _, err := c.ValidateSession(context.Background(), "token") + _, err := c.ValidateSession(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "not_authed") require.Contains(t, err.Error(), "You are either not logged in or your login session has expired") } func TestClient_ValidateSession_AuthRespNotOk_InvalidAuth(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, Response: `{"ok":false,"error":"invalid_auth"}`, }) defer teardown() - _, err := c.ValidateSession(context.Background(), "token") + _, err := c.ValidateSession(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "invalid_auth") require.Contains(t, err.Error(), "Your user account authorization isn't valid") } func TestClient_ValidateSession_HTTPRequestFailed(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, StatusCode: http.StatusInternalServerError, }) defer teardown() - _, err := c.ValidateSession(context.Background(), "token") + _, err := c.ValidateSession(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "http_request_failed") } func TestClient_RevokeToken_ErrNotLoggedIn(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: revokeTokenMethod, StatusCode: http.StatusInternalServerError, }) defer teardown() - err := c.RevokeToken(context.Background(), "token") + err := c.RevokeToken(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "http_request_failed") } func TestClient_ValidateSession_HTTPResponseInvalid(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: sessionValidateMethod, Response: `badresponse`, }) defer teardown() - _, err := c.ValidateSession(context.Background(), "token") + _, err := c.ValidateSession(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") } func TestClient_RevokeToken_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: revokeTokenMethod, Response: `{"ok":true,"revoked":true}`, }) defer teardown() - err := c.RevokeToken(context.Background(), "token") + err := c.RevokeToken(ctx, "token") require.NoError(t, err) } func TestClient_RevokeToken_NotOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: revokeTokenMethod, Response: `{"ok":false}`, }) defer teardown() - err := c.RevokeToken(context.Background(), "token") + err := c.RevokeToken(ctx, "token") require.Error(t, err) } func TestClient_RevokeToken_NotOk_MappedErrorMsgs(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: revokeTokenMethod, Response: `{"ok":false,"error":"token_expired"}`, }) defer teardown() - err := c.RevokeToken(context.Background(), "token") + err := c.RevokeToken(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "token_expired") } func TestClient_RevokeToken_JsonUnmarshalFail(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: revokeTokenMethod, Response: `}{`, }) defer teardown() - err := c.RevokeToken(context.Background(), "token") + err := c.RevokeToken(ctx, "token") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") } diff --git a/internal/api/steps_test.go b/internal/api/steps_test.go index 92a8f9a3..562e1719 100644 --- a/internal/api/steps_test.go +++ b/internal/api/steps_test.go @@ -15,20 +15,21 @@ package api import ( - "context" "testing" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) func TestClient_StepsList_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: workflowsStepsListMethod, ExpectedRequest: `{"workflow_app_id":"A123","workflow":"#/workflows/my-workflow","function_id":"Fn010N"}`, Response: `{"ok":true,"steps_versions":[{"title":"cool form","workflow_id":"Wf123","step_id":"0","is_deleted":false,"workflow_version_created":"1234"}]}`, }) defer teardown() - versions, err := c.StepsList(context.Background(), "token", "#/workflows/my-workflow", "A123") + versions, err := c.StepsList(ctx, "token", "#/workflows/my-workflow", "A123") require.NoError(t, err) require.ElementsMatch(t, versions, []StepVersion{ { @@ -42,25 +43,28 @@ func TestClient_StepsList_Ok(t *testing.T) { } func TestClient_StepsList_Errors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) verifyCommonErrorCases(t, workflowsStepsListMethod, func(c *Client) error { - _, err := c.StepsList(context.Background(), "token", "#/workflows/my-workflow", "A123") + _, err := c.StepsList(ctx, "token", "#/workflows/my-workflow", "A123") return err }) } func TestClient_StepsResponsesExport_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: workflowsStepsResponsesExportMethod, ExpectedRequest: `{"workflow_app_id":"A123","workflow":"#/workflows/my-workflow","step_id":"0"}`, Response: `{"ok":true}`, }) defer teardown() - err := c.StepsResponsesExport(context.Background(), "token", "#/workflows/my-workflow", "A123", "0") + err := c.StepsResponsesExport(ctx, "token", "#/workflows/my-workflow", "A123", "0") require.NoError(t, err) } func TestClient_StepsResponsesExport_Errors(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) verifyCommonErrorCases(t, workflowsStepsResponsesExportMethod, func(c *Client) error { - return c.StepsResponsesExport(context.Background(), "token", "#/workflows/my-workflow", "A123", "0") + return c.StepsResponsesExport(ctx, "token", "#/workflows/my-workflow", "A123", "0") }) } diff --git a/internal/api/teams_test.go b/internal/api/teams_test.go index 3d1f4e30..dd572e7e 100644 --- a/internal/api/teams_test.go +++ b/internal/api/teams_test.go @@ -15,13 +15,13 @@ package api import ( - "context" "fmt" "net/http" "net/http/httptest" "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) @@ -63,6 +63,8 @@ func Test_API_TeamInfoResponse(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) + // Setup HTTP test server httpHandlerFunc := func(w http.ResponseWriter, r *http.Request) { json := tt.httpResponseJSON @@ -74,7 +76,7 @@ func Test_API_TeamInfoResponse(t *testing.T) { apiClient := NewClient(&http.Client{}, ts.URL, nil) // Execute test - actual, err := apiClient.TeamsInfo(context.Background(), tt.argsToken, tt.argsTeamID) + actual, err := apiClient.TeamsInfo(ctx, tt.argsToken, tt.argsTeamID) // Assertions if tt.expectedErrorContains == "" { diff --git a/internal/api/triggerpermissions_test.go b/internal/api/triggerpermissions_test.go index 10c416a6..6f322225 100644 --- a/internal/api/triggerpermissions_test.go +++ b/internal/api/triggerpermissions_test.go @@ -15,10 +15,10 @@ package api import ( - "context" "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) @@ -90,6 +90,7 @@ func TestClient_TriggerPermissionsSet(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare c, teardown := NewFakeClient(t, FakeClientParams{ @@ -101,7 +102,7 @@ func TestClient_TriggerPermissionsSet(t *testing.T) { // execute if tt.users != "" { - _, err := c.TriggerPermissionsSet(context.Background(), fakeToken, fakeTriggerID, tt.users, tt.permissionType, "users") + _, err := c.TriggerPermissionsSet(ctx, fakeToken, fakeTriggerID, tt.users, tt.permissionType, "users") // check if (err != nil) != tt.wantErr { @@ -117,7 +118,7 @@ func TestClient_TriggerPermissionsSet(t *testing.T) { ) } } else if tt.channels != "" { - _, err := c.TriggerPermissionsSet(context.Background(), fakeToken, fakeTriggerID, tt.channels, tt.permissionType, "channels") + _, err := c.TriggerPermissionsSet(ctx, fakeToken, fakeTriggerID, tt.channels, tt.permissionType, "channels") // check if (err != nil) != tt.wantErr { @@ -133,7 +134,7 @@ func TestClient_TriggerPermissionsSet(t *testing.T) { ) } } else if tt.workspaces != "" { - _, err := c.TriggerPermissionsSet(context.Background(), fakeToken, fakeTriggerID, tt.workspaces, tt.permissionType, "workspaces") + _, err := c.TriggerPermissionsSet(ctx, fakeToken, fakeTriggerID, tt.workspaces, tt.permissionType, "workspaces") // check if (err != nil) != tt.wantErr { @@ -149,7 +150,7 @@ func TestClient_TriggerPermissionsSet(t *testing.T) { ) } } else if tt.organizations != "" { - _, err := c.TriggerPermissionsSet(context.Background(), fakeToken, fakeTriggerID, tt.organizations, tt.permissionType, "organizations") + _, err := c.TriggerPermissionsSet(ctx, fakeToken, fakeTriggerID, tt.organizations, tt.permissionType, "organizations") // check if (err != nil) != tt.wantErr { @@ -169,7 +170,8 @@ func TestClient_TriggerPermissionsSet(t *testing.T) { } verifyCommonErrorCases(t, workflowsTriggersPermissionsSetMethod, func(c *Client) error { - _, err := c.TriggerPermissionsSet(context.Background(), "xoxp-123", "Ft123", "user1", types.APP_COLLABORATORS, "users") + ctx := slackcontext.MockContext(t.Context()) + _, err := c.TriggerPermissionsSet(ctx, "xoxp-123", "Ft123", "user1", types.APP_COLLABORATORS, "users") return err }) } @@ -223,6 +225,7 @@ func TestClient_TriggerPermissionsAddEntities(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare c, teardown := NewFakeClient(t, FakeClientParams{ @@ -234,7 +237,7 @@ func TestClient_TriggerPermissionsAddEntities(t *testing.T) { // execute if tt.users != "" { - err := c.TriggerPermissionsAddEntities(context.Background(), fakeToken, fakeTriggerID, tt.users, "users") + err := c.TriggerPermissionsAddEntities(ctx, fakeToken, fakeTriggerID, tt.users, "users") // check if (err != nil) != tt.wantErr { t.Errorf("%s test error = %v, wantErr %v", tt.name, err, tt.wantErr) @@ -249,7 +252,7 @@ func TestClient_TriggerPermissionsAddEntities(t *testing.T) { ) } } else if tt.channels != "" { - err := c.TriggerPermissionsAddEntities(context.Background(), fakeToken, fakeTriggerID, tt.channels, "channels") + err := c.TriggerPermissionsAddEntities(ctx, fakeToken, fakeTriggerID, tt.channels, "channels") // check if (err != nil) != tt.wantErr { t.Errorf("%s test error = %v, wantErr %v", tt.name, err, tt.wantErr) @@ -264,7 +267,7 @@ func TestClient_TriggerPermissionsAddEntities(t *testing.T) { ) } } else if tt.workspaces != "" { - err := c.TriggerPermissionsAddEntities(context.Background(), fakeToken, fakeTriggerID, tt.workspaces, "workspaces") + err := c.TriggerPermissionsAddEntities(ctx, fakeToken, fakeTriggerID, tt.workspaces, "workspaces") // check if (err != nil) != tt.wantErr { t.Errorf("%s test error = %v, wantErr %v", tt.name, err, tt.wantErr) @@ -279,7 +282,7 @@ func TestClient_TriggerPermissionsAddEntities(t *testing.T) { ) } } else if tt.organizations != "" { - err := c.TriggerPermissionsAddEntities(context.Background(), fakeToken, fakeTriggerID, tt.organizations, "organizations") + err := c.TriggerPermissionsAddEntities(ctx, fakeToken, fakeTriggerID, tt.organizations, "organizations") // check if (err != nil) != tt.wantErr { t.Errorf("%s test error = %v, wantErr %v", tt.name, err, tt.wantErr) @@ -299,7 +302,8 @@ func TestClient_TriggerPermissionsAddEntities(t *testing.T) { } verifyCommonErrorCases(t, workflowsTriggersPermissionsAddMethod, func(c *Client) error { - return c.TriggerPermissionsAddEntities(context.Background(), "xoxp-123", "Ft123", "user1", "users") + ctx := slackcontext.MockContext(t.Context()) + return c.TriggerPermissionsAddEntities(ctx, "xoxp-123", "Ft123", "user1", "users") }) } @@ -352,6 +356,7 @@ func TestClient_TriggerPermissionsRemoveEntities(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare c, teardown := NewFakeClient(t, FakeClientParams{ @@ -363,7 +368,7 @@ func TestClient_TriggerPermissionsRemoveEntities(t *testing.T) { // execute if tt.users != "" { - err := c.TriggerPermissionsRemoveEntities(context.Background(), fakeToken, fakeTriggerID, tt.users, "users") + err := c.TriggerPermissionsRemoveEntities(ctx, fakeToken, fakeTriggerID, tt.users, "users") // check if (err != nil) != tt.wantErr { @@ -379,7 +384,7 @@ func TestClient_TriggerPermissionsRemoveEntities(t *testing.T) { ) } } else if tt.channels != "" { - err := c.TriggerPermissionsRemoveEntities(context.Background(), fakeToken, fakeTriggerID, tt.channels, "channels") + err := c.TriggerPermissionsRemoveEntities(ctx, fakeToken, fakeTriggerID, tt.channels, "channels") // check if (err != nil) != tt.wantErr { @@ -395,7 +400,7 @@ func TestClient_TriggerPermissionsRemoveEntities(t *testing.T) { ) } } else if tt.workspaces != "" { - err := c.TriggerPermissionsRemoveEntities(context.Background(), fakeToken, fakeTriggerID, tt.workspaces, "workspaces") + err := c.TriggerPermissionsRemoveEntities(ctx, fakeToken, fakeTriggerID, tt.workspaces, "workspaces") // check if (err != nil) != tt.wantErr { @@ -411,7 +416,7 @@ func TestClient_TriggerPermissionsRemoveEntities(t *testing.T) { ) } } else if tt.organizations != "" { - err := c.TriggerPermissionsRemoveEntities(context.Background(), fakeToken, fakeTriggerID, tt.organizations, "organizations") + err := c.TriggerPermissionsRemoveEntities(ctx, fakeToken, fakeTriggerID, tt.organizations, "organizations") // check if (err != nil) != tt.wantErr { @@ -431,7 +436,8 @@ func TestClient_TriggerPermissionsRemoveEntities(t *testing.T) { } verifyCommonErrorCases(t, workflowsTriggersPermissionsRemoveMethod, func(c *Client) error { - return c.TriggerPermissionsRemoveEntities(context.Background(), "xoxp-123", "Ft123", "user1", "users") + ctx := slackcontext.MockContext(t.Context()) + return c.TriggerPermissionsRemoveEntities(ctx, "xoxp-123", "Ft123", "user1", "users") }) } @@ -504,6 +510,7 @@ func TestClient_TriggerPermissionsList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare c, teardown := NewFakeClient(t, FakeClientParams{ @@ -515,7 +522,7 @@ func TestClient_TriggerPermissionsList(t *testing.T) { // execute if len(tt.expectedUsers) != 0 { - actualType, actualUsers, err := c.TriggerPermissionsList(context.Background(), fakeToken, fakeTriggerID) + actualType, actualUsers, err := c.TriggerPermissionsList(ctx, fakeToken, fakeTriggerID) require.Equal(t, tt.expectedPermissionType, actualType) require.Equal(t, tt.expectedUsers, actualUsers) @@ -533,7 +540,7 @@ func TestClient_TriggerPermissionsList(t *testing.T) { ) } } else if len(tt.expectedChannels) != 0 { - actualType, actualChannels, err := c.TriggerPermissionsList(context.Background(), fakeToken, fakeTriggerID) + actualType, actualChannels, err := c.TriggerPermissionsList(ctx, fakeToken, fakeTriggerID) require.Equal(t, tt.expectedPermissionType, actualType) require.Equal(t, tt.expectedChannels, actualChannels) @@ -551,7 +558,7 @@ func TestClient_TriggerPermissionsList(t *testing.T) { ) } } else if len(tt.expectedWorkspaces) != 0 { - actualType, actualWorkspaces, err := c.TriggerPermissionsList(context.Background(), fakeToken, fakeTriggerID) + actualType, actualWorkspaces, err := c.TriggerPermissionsList(ctx, fakeToken, fakeTriggerID) require.Equal(t, tt.expectedPermissionType, actualType) require.Equal(t, tt.expectedWorkspaces, actualWorkspaces) @@ -569,7 +576,7 @@ func TestClient_TriggerPermissionsList(t *testing.T) { ) } } else if len(tt.expectedOrganizations) != 0 { - actualType, actualOrganizations, err := c.TriggerPermissionsList(context.Background(), fakeToken, fakeTriggerID) + actualType, actualOrganizations, err := c.TriggerPermissionsList(ctx, fakeToken, fakeTriggerID) require.Equal(t, tt.expectedPermissionType, actualType) require.Equal(t, tt.expectedOrganizations, actualOrganizations) @@ -591,7 +598,8 @@ func TestClient_TriggerPermissionsList(t *testing.T) { } verifyCommonErrorCases(t, workflowsTriggersPermissionsListMethod, func(c *Client) error { - _, _, err := c.TriggerPermissionsList(context.Background(), "xoxp-123", "Ft123") + ctx := slackcontext.MockContext(t.Context()) + _, _, err := c.TriggerPermissionsList(ctx, "xoxp-123", "Ft123") return err }) } diff --git a/internal/api/variables_test.go b/internal/api/variables_test.go index 130206a6..fbbab617 100644 --- a/internal/api/variables_test.go +++ b/internal/api/variables_test.go @@ -15,141 +15,153 @@ package api import ( - "context" "net/http" "testing" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) func TestClient_AddVariable_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varAddMethod, ExpectedRequest: `{"app_id":"A123","variables":[{"name":"dummy_var","value":"dummy_val"}]}`, Response: `{"ok":true}`, }) defer teardown() - err := c.AddVariable(context.Background(), "token", "A123", "dummy_var", "dummy_val") + err := c.AddVariable(ctx, "token", "A123", "dummy_var", "dummy_val") require.NoError(t, err) } func TestClient_AddVariable_NotOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varAddMethod, ExpectedRequest: `{"app_id":"A123","variables":[{"name":"dummy_var","value":"dummy_val"}]}`, Response: `{"ok":false}`, }) defer teardown() - err := c.AddVariable(context.Background(), "token", "A123", "dummy_var", "dummy_val") + err := c.AddVariable(ctx, "token", "A123", "dummy_var", "dummy_val") require.Error(t, err) require.Contains(t, err.Error(), "unknown_error") } func TestClient_AddVariable_HTTPResponseInvalid(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varAddMethod, }) defer teardown() - err := c.AddVariable(context.Background(), "token", "", "", "") + err := c.AddVariable(ctx, "token", "", "", "") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") } func TestClient_AddVariable_HTTPRequestFailed(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varAddMethod, StatusCode: http.StatusInternalServerError, }) defer teardown() - err := c.AddVariable(context.Background(), "token", "", "", "") + err := c.AddVariable(ctx, "token", "", "", "") require.Error(t, err) require.Contains(t, err.Error(), "http_request_failed") } func TestClient_ListVariables_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varListMethod, ExpectedRequest: `{"app_id":"A123"}`, Response: `{"ok":true,"variable_names":["var1"]}`, }) defer teardown() - VariableNames, err := c.ListVariables(context.Background(), "token", "A123") + VariableNames, err := c.ListVariables(ctx, "token", "A123") require.NoError(t, err) require.Equal(t, VariableNames[0], "var1") } func TestClient_ListVariables_NotOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varListMethod, ExpectedRequest: `{"app_id":"A123"}`, Response: `{"ok":false}`, }) defer teardown() - _, err := c.ListVariables(context.Background(), "token", "A123") + _, err := c.ListVariables(ctx, "token", "A123") require.Error(t, err) require.Contains(t, err.Error(), "unknown_error") } func TestClient_ListVariables_HTTPResponseInvalid(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varListMethod, }) defer teardown() - _, err := c.ListVariables(context.Background(), "token", "") + _, err := c.ListVariables(ctx, "token", "") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") } func TestClient_ListVariables_HTTPRequestFailed(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varListMethod, StatusCode: http.StatusInternalServerError, }) defer teardown() - _, err := c.ListVariables(context.Background(), "token", "") + _, err := c.ListVariables(ctx, "token", "") require.Error(t, err) require.Contains(t, err.Error(), "http_request_failed") } func TestClient_RemoveVariable_Ok(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varRemoveMethod, ExpectedRequest: `{"app_id":"A123","variable_names":["dummy_var_name"]}`, Response: `{"ok":true}`, }) defer teardown() - err := c.RemoveVariable(context.Background(), "token", "A123", "dummy_var_name") + err := c.RemoveVariable(ctx, "token", "A123", "dummy_var_name") require.NoError(t, err) } func TestClient_RemoveVariable_NotOk(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varRemoveMethod, ExpectedRequest: `{"app_id":"A123","variable_names":["dummy_var_name"]}`, Response: `{"ok":false}`, }) defer teardown() - err := c.RemoveVariable(context.Background(), "token", "A123", "dummy_var_name") + err := c.RemoveVariable(ctx, "token", "A123", "dummy_var_name") require.Error(t, err) } func TestClient_RemoveVariable_HTTPResponseInvalid(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varRemoveMethod, }) defer teardown() - err := c.RemoveVariable(context.Background(), "token", "", "") + err := c.RemoveVariable(ctx, "token", "", "") require.Error(t, err) require.Contains(t, err.Error(), "http_response_invalid") } func TestClient_RemoveVariable_HTTPRequestFailed(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: varRemoveMethod, StatusCode: http.StatusInternalServerError, }) defer teardown() - err := c.RemoveVariable(context.Background(), "token", "", "") + err := c.RemoveVariable(ctx, "token", "", "") require.Error(t, err) require.Contains(t, err.Error(), "http_request_failed") } diff --git a/internal/api/workflows_test.go b/internal/api/workflows_test.go index bb259244..ad24fd77 100644 --- a/internal/api/workflows_test.go +++ b/internal/api/workflows_test.go @@ -15,7 +15,6 @@ package api import ( - "context" "fmt" "io" "net/http" @@ -23,6 +22,7 @@ import ( "testing" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/stretchr/testify/require" ) @@ -114,6 +114,7 @@ func TestClient_WorkflowsTriggerCreate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -132,7 +133,7 @@ func TestClient_WorkflowsTriggerCreate(t *testing.T) { c := NewClient(&http.Client{}, ts.URL, nil) // execute - _, err := c.WorkflowsTriggersCreate(context.Background(), "token", tt.inputTrigger) + _, err := c.WorkflowsTriggersCreate(ctx, "token", tt.inputTrigger) // check if (err != nil) != tt.wantErr { @@ -254,6 +255,7 @@ func TestClient_WorkflowsTriggerUpdate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -272,7 +274,7 @@ func TestClient_WorkflowsTriggerUpdate(t *testing.T) { c := NewClient(&http.Client{}, ts.URL, nil) // execute - _, err := c.WorkflowsTriggersUpdate(context.Background(), "token", tt.input) + _, err := c.WorkflowsTriggersUpdate(ctx, "token", tt.input) // check if (err != nil) != tt.wantErr { @@ -311,6 +313,7 @@ func TestClient_WorkflowsTriggerDelete(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) // prepare handlerFunc := func(w http.ResponseWriter, r *http.Request) { @@ -328,7 +331,7 @@ func TestClient_WorkflowsTriggerDelete(t *testing.T) { c := NewClient(&http.Client{}, ts.URL, nil) // execute - err := c.WorkflowsTriggersDelete(context.Background(), "token", "FtABC") + err := c.WorkflowsTriggersDelete(ctx, "token", "FtABC") // check if (err != nil) != tt.wantErr { @@ -425,6 +428,7 @@ func Test_API_WorkflowTriggersList(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) c, teardown := NewFakeClient(t, FakeClientParams{ ExpectedMethod: workflowsTriggersListMethod, @@ -438,7 +442,7 @@ func Test_API_WorkflowTriggersList(t *testing.T) { Limit: tt.argsLimit, Cursor: tt.argsCursor, } - actual, _, err := c.WorkflowsTriggersList(context.Background(), tt.argsToken, args) + actual, _, err := c.WorkflowsTriggersList(ctx, tt.argsToken, args) // Assertions if tt.expectedErrorContains == "" { diff --git a/internal/auth/auth_test.go b/internal/auth/auth_test.go index b5affa7c..88fd4b25 100644 --- a/internal/auth/auth_test.go +++ b/internal/auth/auth_test.go @@ -27,6 +27,7 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/shared/types" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/spf13/afero" @@ -35,7 +36,6 @@ import ( ) func Test_AuthWithToken(t *testing.T) { - ctx := context.Background() os := slackdeps.NewOsMock() os.AddDefaultMocks() fs := slackdeps.NewFsMock() @@ -81,6 +81,7 @@ func Test_AuthWithToken(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) apic, teardown := api.NewFakeClient(t, api.FakeClientParams{ ExpectedMethod: "auth.test", ExpectedRequest: fmt.Sprintf("token=%s", tt.token), @@ -131,7 +132,7 @@ func Test_AuthGettersAndSetters(t *testing.T) { var setup = func(t *testing.T) (context.Context, *Client) { // Mocks - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -226,7 +227,7 @@ func Test_AuthGettersAndSetters(t *testing.T) { func Test_AuthsRotation(t *testing.T) { // Setup tests var setup = func(t *testing.T) (context.Context, *Client) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -416,7 +417,7 @@ func Test_AuthsRotation(t *testing.T) { func Test_Auths(t *testing.T) { var setup = func(t *testing.T) (context.Context, *Client) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -474,7 +475,7 @@ func Test_Auths(t *testing.T) { func Test_migrateToAuthByTeamID(t *testing.T) { var setup = func(t *testing.T) (context.Context, *Client) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -528,7 +529,7 @@ func Test_migrateToAuthByTeamID(t *testing.T) { func Test_SetSelectedAuth(t *testing.T) { var setup = func(t *testing.T) (context.Context, *Client, *slackdeps.OsMock) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -567,7 +568,7 @@ func Test_SetSelectedAuth(t *testing.T) { func Test_IsApiHostSlackDev(t *testing.T) { var setup = func(t *testing.T) (context.Context, *Client) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() @@ -642,7 +643,7 @@ func Test_FilterKnownAuthErrors(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) os := slackdeps.NewOsMock() os.AddDefaultMocks() fs := slackdeps.NewFsMock() diff --git a/internal/auth/revoke_test.go b/internal/auth/revoke_test.go index 9c59c893..cf4ce603 100644 --- a/internal/auth/revoke_test.go +++ b/internal/auth/revoke_test.go @@ -15,7 +15,6 @@ package auth import ( - "context" "fmt" "testing" @@ -23,6 +22,7 @@ import ( "github.com/slackapi/slack-cli/internal/app" "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/stretchr/testify/assert" @@ -68,7 +68,7 @@ func Test_AuthRevokeToken(t *testing.T) { } for name, tt := range tests { t.Run(name, func(t *testing.T) { - ctx := context.Background() + ctx := slackcontext.MockContext(t.Context()) os := slackdeps.NewOsMock() os.AddDefaultMocks() fs := slackdeps.NewFsMock() diff --git a/internal/config/context.go b/internal/config/context.go index 29aca7a7..319c6e5e 100644 --- a/internal/config/context.go +++ b/internal/config/context.go @@ -16,42 +16,15 @@ package config import ( "context" - - "github.com/slackapi/slack-cli/internal/shared/types" ) type contextKey string -const CONTEXT_ENV contextKey = "env" const CONTEXT_TOKEN contextKey = "token" const CONTEXT_TEAM_ID contextKey = "team_id" const CONTEXT_TEAM_DOMAIN contextKey = "team_domain" // e.g. "subarachnoid" const CONTEXT_USER_ID contextKey = "user_id" -const CONTEXT_SESSION_ID contextKey = "session_id" const CONTEXT_ENTERPRISE_ID contextKey = "enterprise_id" -const CONTEXT_TRACE_ID contextKey = "trace_id" - -// SetContextApp sets the app on the context -// -// [LEGACY] Please do not use and prefer to -// directly pass the app to methods which require -// an app if possible. -func SetContextApp(ctx context.Context, app types.App) context.Context { - return context.WithValue(ctx, CONTEXT_ENV, app) -} - -// GetContextApp gets an app from the context -// -// [LEGACY] Please do not use and prefer to -// directly pass the app to methods which require -// an app if possible. -func GetContextApp(ctx context.Context) types.App { - app, ok := ctx.Value(CONTEXT_ENV).(types.App) - if !ok { - return types.App{} - } - return app -} func SetContextToken(ctx context.Context, token string) context.Context { return context.WithValue(ctx, CONTEXT_TOKEN, token) @@ -111,27 +84,3 @@ func GetContextUserID(ctx context.Context) string { } return userID } - -func GetContextSessionID(ctx context.Context) string { - sessionID, ok := ctx.Value(CONTEXT_SESSION_ID).(string) - if !ok { - return "" - } - return sessionID -} - -func SetContextSessionID(ctx context.Context, sessionID string) context.Context { - return context.WithValue(ctx, CONTEXT_SESSION_ID, sessionID) -} - -func SetContextTraceID(ctx context.Context, traceID string) context.Context { - return context.WithValue(ctx, CONTEXT_TRACE_ID, traceID) -} - -func GetContextTraceID(ctx context.Context) string { - traceID, ok := ctx.Value(CONTEXT_TRACE_ID).(string) - if !ok { - return "" - } - return traceID -} diff --git a/internal/contextutil/contextutil.go b/internal/contextutil/contextutil.go deleted file mode 100644 index f7d03ad7..00000000 --- a/internal/contextutil/contextutil.go +++ /dev/null @@ -1,53 +0,0 @@ -// Copyright 2022-2025 Salesforce, Inc. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package contextutil - -import ( - "context" - - "github.com/opentracing/opentracing-go" -) - -type ctxKey string - -const ( - TracerCtxKey ctxKey = "__TRACER__" - SpanCtxKey ctxKey = "__SPAN__" - VersionCtxKey ctxKey = "__VERSION__" -) - -// AddTracerToContext adds the given tracer to request context -func AddTracerToContext(ctx context.Context, tracer opentracing.Tracer) context.Context { - return context.WithValue(ctx, TracerCtxKey, tracer) -} - -// AddVersionToContext adds the slack-cli version to request context -func AddVersionToContext(ctx context.Context, version string) context.Context { - return context.WithValue(ctx, VersionCtxKey, version) -} - -// AddSpanToContext adds the given span to request context -func AddSpanToContext(ctx context.Context, span opentracing.Span) context.Context { - return context.WithValue(ctx, SpanCtxKey, span) -} - -// VersionFromContext get the slack-cli version to request context -func VersionFromContext(ctx context.Context) string { - var version, ok = ctx.Value(VersionCtxKey).(string) - if !ok { - version = "" - } - return version -} diff --git a/internal/iostreams/logfile.go b/internal/iostreams/logfile.go index a24df6bc..44c2c4da 100644 --- a/internal/iostreams/logfile.go +++ b/internal/iostreams/logfile.go @@ -26,6 +26,7 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/goutils" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackerror" "github.com/slackapi/slack-cli/internal/style" ) @@ -62,12 +63,22 @@ func (io *IOStreams) InitLogFile(ctx context.Context) error { // Added line break for each session logger.Println("------------------------------------") + traceID, err := slackcontext.OpenTracingTraceID(ctx) + if err != nil { + return err + } + + sessionID, err := slackcontext.SessionID(ctx) + if err != nil { + return err + } + // Log the Slack-CLI version, User's OS, SessionID, TraceID // But format data before writing them to the log file formatAndWriteDataToLogFile(logger, map[string]string{ "Command": goutils.RedactPII(strings.Join(os.Args[0:], " ")), - "SessionID": config.GetContextSessionID(ctx), - "Slack-CLI-TraceID": config.GetContextTraceID(ctx), + "SessionID": sessionID, + "Slack-CLI-TraceID": traceID, "Slack-CLI Version": io.config.Version, "Operating System (OS)": runtime.GOOS, "System ID": io.config.SystemID, diff --git a/internal/slackcontext/slackcontext.go b/internal/slackcontext/slackcontext.go new file mode 100644 index 00000000..03086573 --- /dev/null +++ b/internal/slackcontext/slackcontext.go @@ -0,0 +1,176 @@ +// Copyright 2022-2025 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package slackcontext + +import ( + "context" + + "github.com/opentracing/opentracing-go" + "github.com/slackapi/slack-cli/internal/slackerror" +) + +// contextKey is an unexported type to avoid context key collisions. +// Learn more: https://github.com/golang/net/blob/236b8f043b920452504e263bc21d354427127473/context/context.go#L100 +type contextKey int + +const ( + contextKeyOpenTracingTraceID contextKey = iota + contextKeyOpenTracingTracer + contextKeyProjectID + contextKeySessionID + contextKeySystemID + contextKeyVersion +) + +// OpenTracingSpan returns the `opentracing.Span“ associated with `ctx`, or +// `nil` and `slackerror.ErrContextValueNotFound` if no `Span` could be found. +func OpenTracingSpan(ctx context.Context) (opentracing.Span, error) { + span := opentracing.SpanFromContext(ctx) + if span == nil { + return nil, slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for OpenTracing Span could not be found") + } + return span, nil +} + +// SetOpenTracingSpan returns a new `context.Context` that holds a reference to +// the span. If span is nil, a new context without an active span is returned. +func SetOpenTracingSpan(ctx context.Context, span opentracing.Span) context.Context { + ctx = opentracing.ContextWithSpan(ctx, span) + return ctx +} + +// OpenTracingTraceID returns the trace ID associated with `ctx`, or +// `""` and `slackerror.ErrContextValueNotFound` if no trace ID could be found. +func OpenTracingTraceID(ctx context.Context) (string, error) { + traceID, ok := ctx.Value(contextKeyOpenTracingTraceID).(string) + if !ok || traceID == "" { + return "", slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for OpenTracing Trace ID could not be found") + } + return traceID, nil +} + +// SetOpenTracingTraceID returns a new `context.Context` that holds a reference to +// the `traceID`. +func SetOpenTracingTraceID(ctx context.Context, traceID string) context.Context { + ctx = context.WithValue(ctx, contextKeyOpenTracingTraceID, traceID) + return ctx +} + +// OpenTracingTracer returns the `opentracing.Tracer` associated with `ctx`, or +// `nil` and `slackerror.ErrContextValueNotFound` if no `Tracer` could be found. +func OpenTracingTracer(ctx context.Context) (opentracing.Tracer, error) { + var tracer, ok = ctx.Value(contextKeyOpenTracingTracer).(opentracing.Tracer) + if !ok || tracer == nil { + return nil, slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for OpenTracing Tracer could not be found") + } + return tracer, nil +} + +// SetOpenTracingSpan returns a new `context.Context` that holds a reference to +// the `opentracing.Tracer`. If tracer is nil, a new context without a tracer returned. +func SetOpenTracingTracer(ctx context.Context, tracer opentracing.Tracer) context.Context { + ctx = context.WithValue(ctx, contextKeyOpenTracingTracer, tracer) + return ctx +} + +// ProjectID returns the project ID associated with `ctx`, or +// `""` and `slackerror.ErrContextValueNotFound` if no project ID could be found. +func ProjectID(ctx context.Context) (string, error) { + var projectID, ok = ctx.Value(contextKeyProjectID).(string) + if !ok || projectID == "" { + return "", slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for Project ID could not be found") + } + return projectID, nil +} + +// SetProjectID returns a new `context.Context` that holds a reference to +// the `projectID` and updates the `opentracing.Span` tag with the `projectID`. +func SetProjectID(ctx context.Context, projectID string) context.Context { + // Set projectID in the context + ctx = context.WithValue(ctx, contextKeyProjectID, projectID) + + // Set projectID in OpenTracing by extracting the span and updating the tag + if span := opentracing.SpanFromContext(ctx); span != nil { + span.SetTag("project_id", projectID) + ctx = opentracing.ContextWithSpan(ctx, span) + } + + return ctx +} + +// SessionID returns the session ID associated with `ctx`, or +// `""` and `slackerror.ErrContextValueNotFound` if no session ID could be found. +func SessionID(ctx context.Context) (string, error) { + sessionID, ok := ctx.Value(contextKeySessionID).(string) + if !ok || sessionID == "" { + return "", slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for Session ID could not be found") + } + return sessionID, nil +} + +// SetSessionID returns a new `context.Context` that holds a reference to +// the `sessionID`. +func SetSessionID(ctx context.Context, sessionID string) context.Context { + ctx = context.WithValue(ctx, contextKeySessionID, sessionID) + return ctx +} + +// SystemID returns the session ID associated with `ctx`, or +// `""` and `slackerror.ErrContextValueNotFound` if no system ID could be found. +func SystemID(ctx context.Context) (string, error) { + var systemID, ok = ctx.Value(contextKeySystemID).(string) + if !ok || systemID == "" { + return "", slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for System ID could not be found") + } + return systemID, nil +} + +// SetSystemID returns a new `context.Context` that holds a reference to +// the `systemID` and updates the `opentracing.Span` tag with the `systemID`. +func SetSystemID(ctx context.Context, systemID string) context.Context { + // Set systemID in the context + ctx = context.WithValue(ctx, contextKeySystemID, systemID) + + // Set projectID in OpenTracing by extracting the span and updating the tag + if span := opentracing.SpanFromContext(ctx); span != nil { + span.SetTag("system_id", systemID) + ctx = opentracing.ContextWithSpan(ctx, span) + } + + return ctx +} + +// Version returns the CLI version associated with `ctx`, or +// `""` and `slackerror.ErrContextValueNotFound` if no version could be found. +func Version(ctx context.Context) (string, error) { + var version, ok = ctx.Value(contextKeyVersion).(string) + if !ok || version == "" { + return "", slackerror.New(slackerror.ErrContextValueNotFound). + WithMessage("The value for Version could not be found") + } + return version, nil +} + +// SetVersion adds the slack-cli version to Golang context for trace logging +func SetVersion(ctx context.Context, version string) context.Context { + ctx = context.WithValue(ctx, contextKeyVersion, version) + return ctx +} diff --git a/internal/slackcontext/slackcontext_mock.go b/internal/slackcontext/slackcontext_mock.go new file mode 100644 index 00000000..053d596d --- /dev/null +++ b/internal/slackcontext/slackcontext_mock.go @@ -0,0 +1,34 @@ +// Copyright 2022-2025 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package slackcontext + +import ( + "context" + + "github.com/google/uuid" +) + +const ( + mockCLIVersion = "v1.2.3" +) + +// MockContext sets values in the context that are guaranteed to exist before +// the Cobra root command is executed. +func MockContext(ctx context.Context) context.Context { + ctx = SetOpenTracingTraceID(ctx, uuid.New().String()) + ctx = SetSessionID(ctx, uuid.New().String()) + ctx = SetVersion(ctx, mockCLIVersion) + return ctx +} diff --git a/internal/slackcontext/slackcontext_test.go b/internal/slackcontext/slackcontext_test.go new file mode 100644 index 00000000..d701d83e --- /dev/null +++ b/internal/slackcontext/slackcontext_test.go @@ -0,0 +1,331 @@ +// Copyright 2022-2025 Salesforce, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package slackcontext + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/opentracing/opentracing-go" + "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/slackapi/slack-cli/internal/tracer" + "github.com/stretchr/testify/require" +) + +func Test_SlackContext_OpenTracingSpan(t *testing.T) { + tests := map[string]struct { + expectedSpan opentracing.Span + expectedError error + }{ + "returns span when span exists": { + expectedSpan: opentracing.StartSpan("test.span"), + expectedError: nil, + }, + "returns error when span not found": { + expectedSpan: nil, + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for OpenTracing Span could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = opentracing.ContextWithSpan(ctx, tt.expectedSpan) + actualSpan, actualError := OpenTracingSpan(ctx) + require.Equal(t, tt.expectedSpan, actualSpan) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetOpenTracingSpan(t *testing.T) { + tests := map[string]struct { + expectedSpan opentracing.Span + }{ + "sets the span in the context": { + expectedSpan: opentracing.StartSpan("test.span"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetOpenTracingSpan(ctx, tt.expectedSpan) + actualSpan := opentracing.SpanFromContext(ctx) + require.Equal(t, tt.expectedSpan, actualSpan) + }) + } +} + +func Test_SlackContext_OpenTracingTraceID(t *testing.T) { + tests := map[string]struct { + expectedTraceID string + expectedError error + }{ + "returns Trace ID when it exists": { + expectedTraceID: uuid.New().String(), + expectedError: nil, + }, + "returns error when Trace ID not found": { + expectedTraceID: "", + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for OpenTracing Trace ID could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, contextKeyOpenTracingTraceID, tt.expectedTraceID) + actualTraceID, actualError := OpenTracingTraceID(ctx) + require.Equal(t, tt.expectedTraceID, actualTraceID) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetOpenTracingTraceID(t *testing.T) { + tests := map[string]struct { + expectedTraceID string + }{ + "sets the Trace ID in the context": { + expectedTraceID: uuid.New().String(), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetOpenTracingTraceID(ctx, tt.expectedTraceID) + actualTraceID := ctx.Value(contextKeyOpenTracingTraceID).(string) + require.Equal(t, tt.expectedTraceID, actualTraceID) + }) + } +} + +func Test_SlackContext_OpenTracingTracer(t *testing.T) { + _, _tracer := tracer.SetupTracer(true) + + tests := map[string]struct { + expectedTracer opentracing.Tracer + expectedError error + }{ + "returns Tracer when it exists": { + expectedTracer: _tracer, + expectedError: nil, + }, + "returns error when Tracer not found": { + expectedTracer: nil, + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for OpenTracing Tracer could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, contextKeyOpenTracingTracer, tt.expectedTracer) + actualTracer, actualError := OpenTracingTracer(ctx) + require.Equal(t, tt.expectedTracer, actualTracer) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetOpenTracingTracer(t *testing.T) { + _, _tracer := tracer.SetupTracer(true) + + tests := map[string]struct { + expectedTracer opentracing.Tracer + }{ + "sets the Tracer in the context": { + expectedTracer: _tracer, + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetOpenTracingTracer(ctx, tt.expectedTracer) + actualTracer := ctx.Value(contextKeyOpenTracingTracer).(opentracing.Tracer) + require.Equal(t, tt.expectedTracer, actualTracer) + }) + } +} + +func Test_SlackContext_ProjectID(t *testing.T) { + tests := map[string]struct { + expectedProjectID string + expectedError error + }{ + "returns Project ID when it exists": { + expectedProjectID: uuid.New().String(), + expectedError: nil, + }, + "returns error when Project ID not found": { + expectedProjectID: "", + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for Project ID could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, contextKeyProjectID, tt.expectedProjectID) + actualProjectID, actualError := ProjectID(ctx) + require.Equal(t, tt.expectedProjectID, actualProjectID) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetProjectID(t *testing.T) { + tests := map[string]struct { + expectedProjectID string + }{ + "sets the Project ID in the context": { + expectedProjectID: uuid.New().String(), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetProjectID(ctx, tt.expectedProjectID) + actualProjectID := ctx.Value(contextKeyProjectID).(string) + require.Equal(t, tt.expectedProjectID, actualProjectID) + }) + } +} + +func Test_SlackContext_SessionID(t *testing.T) { + tests := map[string]struct { + expectedSessionID string + expectedError error + }{ + "returns Session ID when it exists": { + expectedSessionID: uuid.New().String(), + expectedError: nil, + }, + "returns error when Session ID not found": { + expectedSessionID: "", + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for Session ID could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, contextKeySessionID, tt.expectedSessionID) + actualSessionID, actualError := SessionID(ctx) + require.Equal(t, tt.expectedSessionID, actualSessionID) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetSessionID(t *testing.T) { + tests := map[string]struct { + expectedSessionID string + }{ + "sets the Session ID in the context": { + expectedSessionID: uuid.New().String(), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetSessionID(ctx, tt.expectedSessionID) + actualSessionID := ctx.Value(contextKeySessionID).(string) + require.Equal(t, tt.expectedSessionID, actualSessionID) + }) + } +} + +func Test_SlackContext_SystemID(t *testing.T) { + tests := map[string]struct { + expectedSystemID string + expectedError error + }{ + "returns System ID when it exists": { + expectedSystemID: uuid.New().String(), + expectedError: nil, + }, + "returns error when System ID not found": { + expectedSystemID: "", + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for System ID could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, contextKeySystemID, tt.expectedSystemID) + actualSystemID, actualError := SystemID(ctx) + require.Equal(t, tt.expectedSystemID, actualSystemID) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetSystemID(t *testing.T) { + tests := map[string]struct { + expectedSystemID string + }{ + "sets the System ID in the context": { + expectedSystemID: uuid.New().String(), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetSystemID(ctx, tt.expectedSystemID) + actualSystemID := ctx.Value(contextKeySystemID).(string) + require.Equal(t, tt.expectedSystemID, actualSystemID) + }) + } +} + +func Test_SlackContext_Version(t *testing.T) { + tests := map[string]struct { + expectedVersion string + expectedError error + }{ + "returns Version when it exists": { + expectedVersion: "v1.2.3", + expectedError: nil, + }, + "returns error when Version not found": { + expectedVersion: "", + expectedError: slackerror.New(slackerror.ErrContextValueNotFound).WithMessage("The value for Version could not be found"), + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = context.WithValue(ctx, contextKeyVersion, tt.expectedVersion) + actualVersion, actualError := Version(ctx) + require.Equal(t, tt.expectedVersion, actualVersion) + require.Equal(t, tt.expectedError, actualError) + }) + } +} + +func Test_SlackContext_SetVersion(t *testing.T) { + tests := map[string]struct { + expectedVersion string + }{ + "sets the Version in the context": { + expectedVersion: "v1.2.3", + }, + } + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + ctx := context.Background() + ctx = SetVersion(ctx, tt.expectedVersion) + actualVersion := ctx.Value(contextKeyVersion).(string) + require.Equal(t, tt.expectedVersion, actualVersion) + }) + } +} diff --git a/internal/slackerror/errors.go b/internal/slackerror/errors.go index 50fee84e..db19d0f4 100644 --- a/internal/slackerror/errors.go +++ b/internal/slackerror/errors.go @@ -81,6 +81,7 @@ const ( ErrCommentRequired = "comment_required" ErrConnectedOrgDenied = "connected_org_denied" ErrConnectedTeamDenied = "connected_team_denied" + ErrContextValueNotFound = "context_value_not_found" ErrCredentialsNotFound = "credentials_not_found" ErrCustomizableInputMissingMatchingWorkflowInput = "customizable_input_missing_matching_workflow_input" ErrCustomizableInputsNotAllowedOnOptionalInputs = "customizable_inputs_not_allowed_on_optional_inputs" @@ -595,6 +596,11 @@ Otherwise start your app for local development with: %s`, Message: "The admin does not allow connected teams to be named_entities", }, + ErrContextValueNotFound: { + Code: ErrContextValueNotFound, + Message: "The context value could not be found", + }, + ErrCredentialsNotFound: { Code: ErrCredentialsNotFound, Message: "No authentication found for this team", @@ -806,13 +812,9 @@ Otherwise start your app for local development with: %s`, }, ErrInvalidAppDirectory: { - Code: ErrInvalidAppDirectory, - Message: "This is an invalid Slack app project directory", - Remediation: strings.Join([]string{ - fmt.Sprintf("A valid Slack project includes the Slack hooks file: %s", filepath.Join(".slack", "hooks.json")), - "", - "If this is a Slack project, you can initialize it with " + style.Commandf("init", false), - }, "\n"), + Code: ErrInvalidAppDirectory, + Message: "This is an invalid Slack app project directory", + Remediation: fmt.Sprintf("A valid Slack project includes the Slack hooks file: %s", filepath.Join(".slack", "hooks.json")), }, ErrInvalidAppFlag: { @@ -1296,16 +1298,12 @@ Otherwise start your app for local development with: %s`, Code: ErrSDKHookNotFound, Message: fmt.Sprintf("A script in %s was not found", style.Highlight(filepath.Join(".slack", "hooks.json"))), Remediation: strings.Join([]string{ - "Hook scripts are defined in one of these Slack hooks files:", - "- slack.json", - "- " + filepath.Join(".slack", "hooks.json"), - "", - "Every app requires a Slack hooks file and you can find an example at:", - style.Highlight("https://github.com/slack-samples/deno-starter-template/blob/main/slack.json"), + fmt.Sprintf("Hook scripts are defined in the Slack hooks file ('%s').", filepath.Join(".slack", "hooks.json")), + "Every app requires a Slack hooks file and you can find a working example at:", "", - "You can create a hooks file manually or with the " + style.Commandf("init", false) + " command.", + style.Highlight("https://github.com/slack-samples/deno-starter-template/blob/main/.slack/hooks.json"), "", - "When manually creating the hooks file, you must install the hook dependencies.", + "After creating the hooks file, you must install related hook dependencies.", }, "\n"), }, diff --git a/internal/tracking/tracking.go b/internal/tracking/tracking.go index d9ec7d0b..b393d5b2 100644 --- a/internal/tracking/tracking.go +++ b/internal/tracking/tracking.go @@ -30,6 +30,7 @@ import ( "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/ioutils" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/style" ) @@ -169,7 +170,11 @@ func (e *EventTracker) FlushToLogstash(ctx context.Context, cfg *config.Config, } versionString, _ := strings.CutPrefix(cfg.Version, "v") eventData := e.cleanSessionData(e.getSessionData()) - sessionID := config.GetContextSessionID(ctx) + sessionID, err := slackcontext.SessionID(ctx) + if err != nil { + return err + } + var eventName EventType switch exitCode { case iostreams.ExitCancel: diff --git a/internal/tracking/tracking_test.go b/internal/tracking/tracking_test.go index c5fbd549..865a3312 100644 --- a/internal/tracking/tracking_test.go +++ b/internal/tracking/tracking_test.go @@ -15,7 +15,6 @@ package tracking import ( - "context" "fmt" "io" "net/http" @@ -24,6 +23,7 @@ import ( "github.com/slackapi/slack-cli/internal/config" "github.com/slackapi/slack-cli/internal/iostreams" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/slackdeps" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -140,6 +140,7 @@ func Test_Tracking_FlushToLogstash(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { + ctx := slackcontext.MockContext(t.Context()) et := NewEventTracker() var requestSent = false testServer := httptest.NewServer(http.HandlerFunc(func(res http.ResponseWriter, req *http.Request) { @@ -161,7 +162,7 @@ func Test_Tracking_FlushToLogstash(t *testing.T) { } ioMock := iostreams.NewIOStreamsMock(cfg, fs, os) ioMock.AddDefaultMocks() - err := et.FlushToLogstash(context.Background(), cfg, ioMock, tt.exitCode) + err := et.FlushToLogstash(ctx, cfg, ioMock, tt.exitCode) require.NoError(t, err) if tt.shouldNotSendRequest && requestSent { require.Fail(t, "Expected no event tracking request to be sent, but request was sent") diff --git a/main.go b/main.go index 3db3fcd8..de3cf3ec 100644 --- a/main.go +++ b/main.go @@ -23,13 +23,12 @@ import ( "github.com/google/uuid" "github.com/opentracing/opentracing-go" "github.com/slackapi/slack-cli/cmd" - "github.com/slackapi/slack-cli/internal/config" - "github.com/slackapi/slack-cli/internal/contextutil" "github.com/slackapi/slack-cli/internal/goutils" "github.com/slackapi/slack-cli/internal/iostreams" "github.com/slackapi/slack-cli/internal/ioutils" "github.com/slackapi/slack-cli/internal/pkg/version" "github.com/slackapi/slack-cli/internal/shared" + "github.com/slackapi/slack-cli/internal/slackcontext" "github.com/slackapi/slack-cli/internal/tracer" "github.com/uber/jaeger-client-go" ) @@ -37,49 +36,55 @@ import ( func main() { defer recoveryFunc() + // Create the parent context for the CLI execution var ctx = context.Background() + // TODO - Could we refactor this to cmd/root.go to initialize open tracing after the CLI flags are parsed? // - This would allow us to choose the correct API host based on flags // - Uncomment `isDevTarget` if we refactor to cmd/root.go and update to call `ResolveApiHost` // var isDevTarget = shared.NewClientFactory().AuthClient().UserDefaultAuthIsProd(ctx) // TODO - hack, remove shared.clients var jaegerCloser, tracer = tracer.SetupTracer(false) // Always setup open tracing on prod - ctx = contextutil.AddTracerToContext(ctx, tracer) defer jaegerCloser.Close() + ctx = slackcontext.SetOpenTracingTracer(ctx, tracer) - // set various metrics we will be tracking on the context - ctx = config.SetContextSessionID(ctx, uuid.New().String()) - // add slack-cli version to context - ctx = contextutil.AddVersionToContext(ctx, version.Get()) + // Set context values + sessionID := uuid.New().String() + cliVersion := version.Get() + ctx = slackcontext.SetSessionID(ctx, sessionID) + ctx = slackcontext.SetVersion(ctx, cliVersion) osStr := os.Args[0:] processName := goutils.RedactPII(strings.Join(osStr, " ")) - var span = tracer.StartSpan("main", opentracing.Tag{Key: "version", Value: version.Get()}) - span.SetTag("slack_cli_sessionID", config.GetContextSessionID(ctx)) + var span = tracer.StartSpan("main", opentracing.Tag{Key: "version", Value: cliVersion}) + span.SetTag("slack_cli_sessionID", sessionID) span.SetTag("hashed_hostname", ioutils.GetHostname()) span.SetTag("slack_cli_process", processName) // system_id is set in root.go initConfig() // project_id is set in root.go initConfig() if jaegerSpanContext, ok := span.Context().(jaeger.SpanContext); ok { - ctx = config.SetContextTraceID(ctx, jaegerSpanContext.TraceID().String()) + ctx = slackcontext.SetOpenTracingTraceID(ctx, jaegerSpanContext.TraceID().String()) } defer span.Finish() // add root span to context - ctx = opentracing.ContextWithSpan(ctx, span) + ctx = slackcontext.SetOpenTracingSpan(ctx, span) rootCmd, clients := cmd.Init() cmd.Execute(ctx, rootCmd, clients) } +// TODO(slackcontext) Use closure to pass in the ctx, which includes the sessionID func recoveryFunc() { // in the event of a panic, log panic if r := recover(); r != nil { var clients = shared.NewClientFactory(shared.SetVersion(version.Raw())) + var ctx = context.Background() - ctx = config.SetContextSessionID(ctx, uuid.New().String()) + ctx = slackcontext.SetSessionID(ctx, uuid.New().String()) + // set host for logging clients.Config.LogstashHostResolved = clients.AuthInterface().ResolveLogstashHost(ctx, clients.Config.ApiHostResolved, clients.Config.Version) clients.IO.PrintError(ctx, "Recovered from panic: %s\n%s", r, string(debug.Stack())) From 6c8907a0e1c0931c4eb846397eaf6a51d9544c67 Mon Sep 17 00:00:00 2001 From: Michael Brooks Date: Wed, 2 Apr 2025 14:12:58 -0700 Subject: [PATCH 2/2] refactor: Revert changes to unrelated error codes --- internal/slackerror/errors.go | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/internal/slackerror/errors.go b/internal/slackerror/errors.go index db19d0f4..8fb060ca 100644 --- a/internal/slackerror/errors.go +++ b/internal/slackerror/errors.go @@ -812,9 +812,13 @@ Otherwise start your app for local development with: %s`, }, ErrInvalidAppDirectory: { - Code: ErrInvalidAppDirectory, - Message: "This is an invalid Slack app project directory", - Remediation: fmt.Sprintf("A valid Slack project includes the Slack hooks file: %s", filepath.Join(".slack", "hooks.json")), + Code: ErrInvalidAppDirectory, + Message: "This is an invalid Slack app project directory", + Remediation: strings.Join([]string{ + fmt.Sprintf("A valid Slack project includes the Slack hooks file: %s", filepath.Join(".slack", "hooks.json")), + "", + "If this is a Slack project, you can initialize it with " + style.Commandf("init", false), + }, "\n"), }, ErrInvalidAppFlag: { @@ -1298,12 +1302,16 @@ Otherwise start your app for local development with: %s`, Code: ErrSDKHookNotFound, Message: fmt.Sprintf("A script in %s was not found", style.Highlight(filepath.Join(".slack", "hooks.json"))), Remediation: strings.Join([]string{ - fmt.Sprintf("Hook scripts are defined in the Slack hooks file ('%s').", filepath.Join(".slack", "hooks.json")), - "Every app requires a Slack hooks file and you can find a working example at:", + "Hook scripts are defined in one of these Slack hooks files:", + "- slack.json", + "- " + filepath.Join(".slack", "hooks.json"), + "", + "Every app requires a Slack hooks file and you can find an example at:", + style.Highlight("https://github.com/slack-samples/deno-starter-template/blob/main/slack.json"), "", - style.Highlight("https://github.com/slack-samples/deno-starter-template/blob/main/.slack/hooks.json"), + "You can create a hooks file manually or with the " + style.Commandf("init", false) + " command.", "", - "After creating the hooks file, you must install related hook dependencies.", + "When manually creating the hooks file, you must install the hook dependencies.", }, "\n"), },