Skip to content

Commit

Permalink
Merge pull request #635 from iamhopaul123/init/refactor
Browse files Browse the repository at this point in the history 
refactor: better option initialization pattern to handle errors
  • Loading branch information
@iamhopaul123
iamhopaul123 authored Feb 5, 2020
2 parents 04910d8 + 16d3bb1 commit f981533
Show file tree
Hide file tree
Showing 31 changed files with 964 additions and 768 deletions.
44 changes: 17 additions & 27 deletions internal/pkg/cli/app_delete.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ var (
errAppDeleteCancelled = errors.New("app delete cancelled - no changes made")
)

type deleteAppOpts struct {
// Flags or arguments that are user inputs.
type deleteAppVars struct {
*GlobalOpts
SkipConfirmation bool
AppName string
}

type deleteAppOpts struct {
deleteAppVars

// Interfaces to dependencies.
projectService projectService
Expand All @@ -45,7 +48,7 @@ type deleteAppOpts struct {
projectEnvironments []*archer.Environment
}

func newDeleteAppOpts() (*deleteAppOpts, error) {
func newDeleteAppOpts(vars deleteAppVars) (*deleteAppOpts, error) {
workspaceService, err := workspace.New()
if err != nil {
return nil, fmt.Errorf("intialize workspace service: %w", err)
Expand All @@ -57,11 +60,12 @@ func newDeleteAppOpts() (*deleteAppOpts, error) {
}

return &deleteAppOpts{
GlobalOpts: NewGlobalOpts(),
spinner: termprogress.NewSpinner(),
sessProvider: session.NewProvider(),
deleteAppVars: vars,

workspaceService: workspaceService,
projectService: projectService,
spinner: termprogress.NewSpinner(),
sessProvider: session.NewProvider(),
}, nil
}

Expand Down Expand Up @@ -277,12 +281,9 @@ func (o *deleteAppOpts) RecommendedActions() []string {

// BuildAppDeleteCmd builds the command to delete application(s).
func BuildAppDeleteCmd() *cobra.Command {
opts := &deleteAppOpts{
GlobalOpts: NewGlobalOpts(),
spinner: termprogress.NewSpinner(),
sessProvider: session.NewProvider(),
vars := deleteAppVars{
GlobalOpts: NewGlobalOpts(),
}

cmd := &cobra.Command{
Use: "delete",
Short: "Deletes an application from your project.",
Expand All @@ -292,22 +293,11 @@ func BuildAppDeleteCmd() *cobra.Command {
Delete the "test" application without prompting.
/code $ ecs-preview app delete --name test --yes`,
PreRunE: runCmdE(func(cmd *cobra.Command, args []string) error {
workspaceService, err := workspace.New()
if err != nil {
return fmt.Errorf("intialize workspace service: %w", err)
}
opts.workspaceService = workspaceService

projectService, err := store.New()
RunE: runCmdE(func(cmd *cobra.Command, args []string) error {
opts, err := newDeleteAppOpts(vars)
if err != nil {
return fmt.Errorf("create project service: %w", err)
return err
}
opts.projectService = projectService

return nil
}),
RunE: runCmdE(func(cmd *cobra.Command, args []string) error {
if err := opts.Validate(); err != nil {
return err
}
Expand All @@ -326,8 +316,8 @@ func BuildAppDeleteCmd() *cobra.Command {
}),
}

cmd.Flags().StringVarP(&opts.AppName, nameFlag, nameFlagShort, "", appFlagDescription)
cmd.Flags().BoolVar(&opts.SkipConfirmation, yesFlag, false, yesFlagDescription)
cmd.Flags().StringVarP(&vars.AppName, nameFlag, nameFlagShort, "", appFlagDescription)
cmd.Flags().BoolVar(&vars.SkipConfirmation, yesFlag, false, yesFlagDescription)

return cmd
}
28 changes: 17 additions & 11 deletions internal/pkg/cli/app_delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,12 @@ func TestDeleteAppOpts_Validate(t *testing.T) {
t.Run(name, func(t *testing.T) {
test.setupMocks()
opts := deleteAppOpts{
GlobalOpts: &GlobalOpts{
projectName: test.inProjectName,
deleteAppVars: deleteAppVars{
GlobalOpts: &GlobalOpts{
projectName: test.inProjectName,
},
AppName: test.inAppName,
},
AppName: test.inAppName,
projectService: mockProjectService,
}

Expand Down Expand Up @@ -202,13 +204,15 @@ func TestDeleteAppOpts_Ask(t *testing.T) {
test.mockProjectService(mockProjectService)

opts := deleteAppOpts{
GlobalOpts: &GlobalOpts{
projectName: mockProjectName,
prompt: mockPrompter,
deleteAppVars: deleteAppVars{
SkipConfirmation: test.skipConfirmation,
GlobalOpts: &GlobalOpts{
projectName: mockProjectName,
prompt: mockPrompter,
},
AppName: test.inAppName,
},
AppName: test.inAppName,
projectService: mockProjectService,
SkipConfirmation: test.skipConfirmation,
projectService: mockProjectService,
}

got := opts.Ask()
Expand Down Expand Up @@ -259,8 +263,10 @@ func TestDeleteAppOpts_sourceProjectEnvironments(t *testing.T) {
t.Run(name, func(t *testing.T) {
test.setupMocks()
opts := deleteAppOpts{
GlobalOpts: &GlobalOpts{
projectName: mockProjectName,
deleteAppVars: deleteAppVars{
GlobalOpts: &GlobalOpts{
projectName: mockProjectName,
},
},
projectService: mockProjectService,
}
Expand Down
59 changes: 33 additions & 26 deletions internal/pkg/cli/app_deploy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,15 @@ var (
errNoLocalManifestsFound = errors.New("no manifest files found")
)

type appDeployOpts struct {
type appDeployVars struct {
*GlobalOpts
AppName string
EnvName string
ImageTag string
}

type appDeployOpts struct {
appDeployVars

projectService projectService
workspaceService archer.Workspace
Expand All @@ -56,23 +60,27 @@ type appDeployOpts struct {
targetEnvironment *archer.Environment
}

func (o *appDeployOpts) String() string {
return fmt.Sprintf("project: %s, app: %s, env: %s, tag: %s", o.ProjectName(), o.AppName, o.EnvName, o.ImageTag)
}

func (o *appDeployOpts) init() error {
func newAppDeployOpts(vars appDeployVars) (*appDeployOpts, error) {
projectService, err := store.New()
if err != nil {
return fmt.Errorf("create project service: %w", err)
return nil, fmt.Errorf("create project service: %w", err)
}
o.projectService = projectService

workspaceService, err := workspace.New()
if err != nil {
return fmt.Errorf("intialize workspace service: %w", err)
return nil, fmt.Errorf("intialize workspace service: %w", err)
}
o.workspaceService = workspaceService
return nil

return &appDeployOpts{
appDeployVars: vars,

projectService: projectService,
workspaceService: workspaceService,
spinner: termprogress.NewSpinner(),
dockerService: docker.New(),
runner: command.New(),
sessProvider: session.NewProvider(),
}, nil
}

// Validate returns an error if the user inputs are invalid.
Expand Down Expand Up @@ -349,15 +357,18 @@ func (o *appDeployOpts) getAppDeployTemplate() (string, error) {
buffer := &bytes.Buffer{}

appPackage := packageAppOpts{
AppName: o.AppName,
EnvName: o.targetEnvironment.Name,
Tag: o.ImageTag,
packageAppVars: packageAppVars{
AppName: o.AppName,
EnvName: o.targetEnvironment.Name,
Tag: o.ImageTag,
GlobalOpts: o.GlobalOpts,
},

stackWriter: buffer,
paramsWriter: ioutil.Discard,
store: o.projectService,
describer: o.appPackageCfClient,
ws: o.workspaceService,
GlobalOpts: o.GlobalOpts,
}

if err := appPackage.Execute(); err != nil {
Expand Down Expand Up @@ -408,14 +419,9 @@ func (o *appDeployOpts) getAppDockerfilePath() (string, error) {

// BuildAppDeployCmd builds the `app deploy` subcommand.
func BuildAppDeployCmd() *cobra.Command {
opts := &appDeployOpts{
GlobalOpts: NewGlobalOpts(),
spinner: termprogress.NewSpinner(),
dockerService: docker.New(),
runner: command.New(),
sessProvider: session.NewProvider(),
vars := appDeployVars{
GlobalOpts: NewGlobalOpts(),
}

cmd := &cobra.Command{
Use: "deploy",
Short: "Deploys an application to an environment.",
Expand All @@ -424,7 +430,8 @@ func BuildAppDeployCmd() *cobra.Command {
Deploys an application named "frontend" to a "test" environment.
/code $ ecs-preview app deploy --name frontend --env test`,
RunE: runCmdE(func(cmd *cobra.Command, args []string) error {
if err := opts.init(); err != nil {
opts, err := newAppDeployOpts(vars)
if err != nil {
return err
}
if err := opts.Validate(); err != nil {
Expand All @@ -439,9 +446,9 @@ func BuildAppDeployCmd() *cobra.Command {
return nil
}),
}
cmd.Flags().StringVarP(&opts.AppName, nameFlag, nameFlagShort, "", appFlagDescription)
cmd.Flags().StringVarP(&opts.EnvName, envFlag, envFlagShort, "", envFlagDescription)
cmd.Flags().StringVar(&opts.ImageTag, imageTagFlag, "", imageTagFlagDescription)
cmd.Flags().StringVarP(&vars.AppName, nameFlag, nameFlagShort, "", appFlagDescription)
cmd.Flags().StringVarP(&vars.EnvName, envFlag, envFlagShort, "", envFlagDescription)
cmd.Flags().StringVar(&vars.ImageTag, imageTagFlag, "", imageTagFlagDescription)

return cmd
}
38 changes: 23 additions & 15 deletions internal/pkg/cli/app_deploy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,13 @@ func TestAppDeployOpts_Validate(t *testing.T) {
tc.mockWs(mockWs)
tc.mockStore(mockStore)
opts := appDeployOpts{
GlobalOpts: &GlobalOpts{
projectName: tc.inProjectName,
appDeployVars: appDeployVars{
GlobalOpts: &GlobalOpts{
projectName: tc.inProjectName,
},
AppName: tc.inAppName,
EnvName: tc.inEnvName,
},
AppName: tc.inAppName,
EnvName: tc.inEnvName,
workspaceService: mockWs,
projectService: mockStore,
}
Expand Down Expand Up @@ -272,13 +274,15 @@ func TestAppDeployOpts_Ask(t *testing.T) {
tc.mockPrompt(mockPrompt)

opts := appDeployOpts{
GlobalOpts: &GlobalOpts{
projectName: tc.inProjectName,
prompt: mockPrompt,
appDeployVars: appDeployVars{
GlobalOpts: &GlobalOpts{
projectName: tc.inProjectName,
prompt: mockPrompt,
},
AppName: tc.inAppName,
EnvName: tc.inEnvName,
ImageTag: tc.inImageTag,
},
AppName: tc.inAppName,
EnvName: tc.inEnvName,
ImageTag: tc.inImageTag,
workspaceService: mockWs,
projectService: mockStore,
}
Expand Down Expand Up @@ -382,7 +386,9 @@ image:
defer ctrl.Finish()
test.setupMocks(ctrl)
opts := appDeployOpts{
AppName: test.inputApp,
appDeployVars: appDeployVars{
AppName: test.inputApp,
},
workspaceService: mockWorkspace,
}

Expand Down Expand Up @@ -449,11 +455,13 @@ func TestAppDeployOpts_askImageTag(t *testing.T) {
ctrl := gomock.NewController(t)
test.setupMocks(ctrl)
opts := &appDeployOpts{
GlobalOpts: &GlobalOpts{
prompt: mockPrompter,
appDeployVars: appDeployVars{
GlobalOpts: &GlobalOpts{
prompt: mockPrompter,
},
ImageTag: test.inputImageTag,
},
ImageTag: test.inputImageTag,
runner: mockRunner,
runner: mockRunner,
}

got := opts.askImageTag()
Expand Down
Loading

0 comments on commit f981533

Please sign in to comment.