diff --git a/internal/app/app.go b/internal/app/app.go index 2f5836a6..6a17a5cb 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -41,7 +41,7 @@ func NewClient( os types.Os, ) *Client { return &Client{ - Manifest: NewManifestClient(apiClient, config), + Manifest: NewManifestClient(apiClient, config, fs), AppClientInterface: NewAppClient(config, fs, os), } } diff --git a/internal/app/manifest.go b/internal/app/manifest.go index 2dc96ab8..d8a904a6 100644 --- a/internal/app/manifest.go +++ b/internal/app/manifest.go @@ -17,6 +17,7 @@ package app import ( "context" "encoding/json" + "path/filepath" "strings" "github.com/slackapi/slack-cli/internal/api" @@ -24,11 +25,15 @@ import ( "github.com/slackapi/slack-cli/internal/hooks" "github.com/slackapi/slack-cli/internal/shared/types" "github.com/slackapi/slack-cli/internal/slackerror" + "github.com/spf13/afero" ) +const manifestFileName = "manifest.json" + // ManifestClient can manage the state of the project's app manifest file type ManifestClient struct { apiClient api.APIInterface + fs afero.Fs domainAuthTokens string Env map[string]string } @@ -59,23 +64,45 @@ func SetManifestEnvTeamVars(manifestEnv map[string]string, appTeamDomain string, func NewManifestClient( apiClient api.APIInterface, config *config.Config, + fs afero.Fs, ) *ManifestClient { client := &ManifestClient{ apiClient: apiClient, + fs: fs, domainAuthTokens: config.DomainAuthTokens, Env: config.ManifestEnv, } return client } -// GetManifestLocal gathers manifest content from the "get-manifest" hook +// GetManifestLocal reads the local manifest, preferring the "get-manifest" hook +// when available. Falls back to reading manifest.json from the project root. func (c *ManifestClient) GetManifestLocal(ctx context.Context, sdkConfig hooks.SDKCLIConfig, hookExecutor hooks.HookExecutor) (types.SlackYaml, error) { - var sl types.SlackYaml + if sdkConfig.Hooks.GetManifest.IsAvailable() { + return c.getManifestFromHook(ctx, sdkConfig, hookExecutor) + } + return c.getManifestFromFile(sdkConfig) +} - if !sdkConfig.Hooks.GetManifest.IsAvailable() { - return sl, slackerror.New(slackerror.ErrSDKHookNotFound). - WithMessage("The `get-manifest` script was not found") +func (c *ManifestClient) getManifestFromFile(sdkConfig hooks.SDKCLIConfig) (types.SlackYaml, error) { + var sl types.SlackYaml + manifestPath := filepath.Join(sdkConfig.WorkingDirectory, manifestFileName) + data, err := afero.ReadFile(c.fs, manifestPath) + if err != nil { + return sl, slackerror.New("Failed to read manifest file"). + WithRootCause(err). + WithCode(slackerror.ErrNoFile) } + if err := json.Unmarshal(data, &sl); err != nil { + return sl, slackerror.New("Failed to parse manifest file"). + WithRootCause(err). + WithCode(slackerror.ErrInvalidManifest) + } + return sl, nil +} + +func (c *ManifestClient) getManifestFromHook(ctx context.Context, sdkConfig hooks.SDKCLIConfig, hookExecutor hooks.HookExecutor) (types.SlackYaml, error) { + var sl types.SlackYaml var manifestHookOpts = hooks.HookExecOpts{ Args: map[string]string{ @@ -104,7 +131,6 @@ func (c *ManifestClient) GetManifestLocal(ctx context.Context, sdkConfig hooks.S if start != -1 { slackManifestInfo = slackManifestInfo[start:] } else { - // the app manifest has to be a json so needs to have the character `{` return sl, slackerror.New("Invalid app manifest format, must be valid JSON"). WithRootCause(err). WithCode(slackerror.ErrInvalidManifest) diff --git a/internal/app/manifest_test.go b/internal/app/manifest_test.go index 41184e74..04b08a9c 100644 --- a/internal/app/manifest_test.go +++ b/internal/app/manifest_test.go @@ -24,6 +24,7 @@ import ( "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" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" @@ -69,76 +70,117 @@ func Test_AppManifest_SetManifestEnvTeamVars(t *testing.T) { func Test_AppManifest_GetManifestLocal(t *testing.T) { tests := map[string]struct { - mockManifestInfo string - mockManifestErr error - expectedErr error + hookCommand string + hookOutput string + hookErr error + manifestFile string expectedManifest types.SlackYaml + expectedErrCode string + expectHookCall bool }{ - "errors if no get-manifest hook exists": { - expectedErr: slackerror.New(slackerror.ErrSDKHookNotFound), + "prefers hook over manifest.json when hook is available": { + hookCommand: "echo manifest", + hookOutput: `{"display_information":{"name":"hook-app"}}`, + manifestFile: `{"display_information":{"name":"file-app"}}`, + expectedManifest: types.SlackYaml{ + AppManifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "hook-app"}, + }, + }, + expectHookCall: true, }, - "returns an existing manifest without errors": { - mockManifestInfo: `{"display_information":{"name":"my-example-app"}}`, + "falls back to manifest.json when no hook exists": { + manifestFile: `{"display_information":{"name":"file-app"}}`, expectedManifest: types.SlackYaml{ AppManifest: types.AppManifest{ - DisplayInformation: types.DisplayInformation{ - Name: "my-example-app", - }, + DisplayInformation: types.DisplayInformation{Name: "file-app"}, }, }, }, - "errors if the hook execution errors": { - mockManifestInfo: `{}`, - mockManifestErr: slackerror.New(slackerror.ErrNoFile), - expectedErr: slackerror.New(slackerror.ErrInvalidManifest), + "errors if no hook and no manifest.json": { + expectedErrCode: slackerror.ErrNoFile, }, - "parses a manifest with random leading characters": { - mockManifestInfo: `...{"display_information":{"name":"my-showcased-app"}}`, + "errors if manifest.json contains invalid JSON": { + manifestFile: `not json`, + expectedErrCode: slackerror.ErrInvalidManifest, + }, + "returns manifest from hook output": { + hookCommand: "generate-manifest", + hookOutput: `{"display_information":{"name":"hook-app"}}`, expectedManifest: types.SlackYaml{ AppManifest: types.AppManifest{ - DisplayInformation: types.DisplayInformation{ - Name: "my-showcased-app", - }, + DisplayInformation: types.DisplayInformation{Name: "hook-app"}, }, }, + expectHookCall: true, }, - "errors if a manifest is not present in output": { - mockManifestInfo: `...unknown`, - expectedErr: slackerror.New(slackerror.ErrInvalidManifest), + "parses hook output with leading characters": { + hookCommand: "generate-manifest", + hookOutput: `...{"display_information":{"name":"hook-app"}}`, + expectedManifest: types.SlackYaml{ + AppManifest: types.AppManifest{ + DisplayInformation: types.DisplayInformation{Name: "hook-app"}, + }, + }, + expectHookCall: true, + }, + "errors if hook execution errors": { + hookCommand: "generate-manifest", + hookOutput: `{}`, + hookErr: slackerror.New(slackerror.ErrNoFile), + expectedErrCode: slackerror.ErrInvalidManifest, + expectHookCall: true, + }, + "errors if hook output has no JSON": { + hookCommand: "generate-manifest", + hookOutput: `...unknown`, + expectedErrCode: slackerror.ErrInvalidManifest, + expectHookCall: true, }, } for name, tc := range tests { t.Run(name, func(t *testing.T) { ctx := slackcontext.MockContext(t.Context()) - mockManifestEnv := map[string]string{"EXAMPLE": "12"} - mockSDKConfig := hooks.NewSDKConfigMock() - mockHookExecutor := &hooks.MockHookExecutor{} - if tc.mockManifestInfo != "" { - mockSDKConfig.Hooks.GetManifest = hooks.HookScript{ - Name: "GetManifest", - Command: "cat manifest.json", - } - mockHookExecutor.On("Execute", mock.Anything, mock.Anything). - Return(tc.mockManifestInfo, tc.mockManifestErr) - } else { - mockSDKConfig.Hooks.GetManifest = hooks.HookScript{Name: "GetManifest"} - } fsMock := slackdeps.NewFsMock() osMock := slackdeps.NewOsMock() osMock.AddDefaultMocks() configMock := config.NewConfig(fsMock, osMock) configMock.DomainAuthTokens = "api.slack.com" - configMock.ManifestEnv = mockManifestEnv - manifestClient := NewManifestClient(&api.APIMock{}, configMock) + mockSDKConfig := hooks.NewSDKConfigMock() + mockSDKConfig.WorkingDirectory = "/project" + + if tc.hookCommand != "" { + mockSDKConfig.Hooks.GetManifest = hooks.HookScript{Name: "GetManifest", Command: tc.hookCommand} + } else { + mockSDKConfig.Hooks.GetManifest = hooks.HookScript{Name: "GetManifest"} + } + + if tc.manifestFile != "" { + _ = fsMock.MkdirAll("/project", 0755) + _ = afero.WriteFile(fsMock, "/project/manifest.json", []byte(tc.manifestFile), 0644) + } - actualManifest, err := manifestClient.GetManifestLocal(ctx, mockSDKConfig, mockHookExecutor) - if tc.expectedErr != nil { + mockHookExecutor := &hooks.MockHookExecutor{} + if tc.hookCommand != "" { + mockHookExecutor.On("Execute", mock.Anything, mock.Anything). + Return(tc.hookOutput, tc.hookErr) + } + + manifestClient := NewManifestClient(&api.APIMock{}, configMock, fsMock) + result, err := manifestClient.GetManifestLocal(ctx, mockSDKConfig, mockHookExecutor) + + if tc.expectedErrCode != "" { require.Error(t, err) - assert.Equal(t, - tc.expectedErr.(*slackerror.Error).Code, err.(*slackerror.Error).Code) + assert.Equal(t, tc.expectedErrCode, err.(*slackerror.Error).Code) } else { require.NoError(t, err) - assert.Equal(t, tc.expectedManifest, actualManifest) + assert.Equal(t, tc.expectedManifest, result) + } + + if tc.expectHookCall { + mockHookExecutor.AssertCalled(t, "Execute", mock.Anything, mock.Anything) + } else { + mockHookExecutor.AssertNotCalled(t, "Execute", mock.Anything, mock.Anything) } }) } @@ -186,7 +228,7 @@ func Test_AppManifest_GetManifestRemote(t *testing.T) { apic := &api.APIMock{} apic.On("ExportAppManifest", mock.Anything, mock.Anything, mock.Anything). Return(api.ExportAppResult{Manifest: tc.mockManifestResponse}, tc.mockManifestError) - manifestClient := NewManifestClient(apic, configMock) + manifestClient := NewManifestClient(apic, configMock, fsMock) manifest, err := manifestClient.GetManifestRemote(ctx, tc.mockToken, tc.mockAppID) if tc.expectedError != nil { diff --git a/internal/manifest/sync_test.go b/internal/manifest/sync_test.go index a9feeb6c..bbdcd8ac 100644 --- a/internal/manifest/sync_test.go +++ b/internal/manifest/sync_test.go @@ -216,7 +216,6 @@ func Test_Sync(t *testing.T) { require.NotNil(t, result) assert.True(t, result.HasDifferences) assert.True(t, result.WriteBack.Written) - // Verify remote value was used — the merged manifest should have "Remote" description assert.Equal(t, "Remote", result.Merged.DisplayInformation.Description) })