From 31b76f75b19369e049924bcaa7f32daf95700500 Mon Sep 17 00:00:00 2001 From: Marko Mudrinic Date: Tue, 4 Oct 2016 22:39:27 +0200 Subject: [PATCH 1/5] Added confirmation on Droplet delete --- commands/confirmation.go | 23 +++++++++++++++++++++++ commands/doit.go | 4 ++++ commands/droplets.go | 29 ++++++++++++++++++++--------- 3 files changed, 47 insertions(+), 9 deletions(-) create mode 100644 commands/confirmation.go diff --git a/commands/confirmation.go b/commands/confirmation.go new file mode 100644 index 000000000..a2049c7ce --- /dev/null +++ b/commands/confirmation.go @@ -0,0 +1,23 @@ +package commands + +import ( + "fmt" + "strings" +) + +func askForConfirm(message string) bool { + var answer string + warn("Are you sure you want to " + message + " (y/N) ? ") + _, err := fmt.Scanln(&answer) + if err != nil { + return false + } + return verifyAnswer(answer) +} + +func verifyAnswer(answer string) bool { + if strings.ToLower(answer) == "y" || strings.ToLower(answer) == "yes" { + return true + } + return false +} diff --git a/commands/doit.go b/commands/doit.go index cf5a9dc8e..a66bbf71c 100644 --- a/commands/doit.go +++ b/commands/doit.go @@ -53,6 +53,9 @@ var Output string // Verbose toggles verbose output. var Verbose bool +// Force froces command execution. +var Force bool + var requiredColor = color.New(color.Bold, color.FgWhite).SprintfFunc() // Writer is where output should be written to. @@ -77,6 +80,7 @@ func init() { DoitCmd.PersistentFlags().StringVarP(&Token, "access-token", "t", "", "API V2 Access Token") DoitCmd.PersistentFlags().StringVarP(&Output, "output", "o", "text", "output format [text|json]") DoitCmd.PersistentFlags().BoolVarP(&Verbose, "verbose", "v", false, "verbose output") + DoitCmd.PersistentFlags().BoolVarP(&Force, "force", "f", false, "force execution") DoitCmd.PersistentFlags().BoolVarP(&Trace, "trace", "", false, "trace api access") viper.SetEnvPrefix("DIGITALOCEAN") diff --git a/commands/droplets.go b/commands/droplets.go index 20bf9e6e2..195b5d570 100644 --- a/commands/droplets.go +++ b/commands/droplets.go @@ -267,7 +267,7 @@ func RunDropletCreate(c *CmdConfig) error { wg.Wait() close(errs) - + item := &droplet{droplets: createdList} c.Display(item) @@ -434,20 +434,31 @@ func RunDropletDelete(c *CmdConfig) error { } else if len(c.Args) > 0 && tagName != "" { return fmt.Errorf("please specify droplets identifiers or a tag name") } else if tagName != "" { - return ds.DeleteByTag(tagName) + if Force || askForConfirm("delete droplet by \""+tagName+"\" tag") { + return ds.DeleteByTag(tagName) + } else { + return fmt.Errorf("Operation aborted.") + } + return nil } - fn := func(ids []int) error { - for _, id := range ids { - if err := ds.Delete(id); err != nil { - return fmt.Errorf("unable to delete droplet %d: %v", id, err) + if Force || askForConfirm("delete droplet(s)") { + + fn := func(ids []int) error { + for _, id := range ids { + if err := ds.Delete(id); err != nil { + return fmt.Errorf("unable to delete droplet %d: %v", id, err) + } } + return nil } - - return nil + return matchDroplets(c.Args, ds, fn) + } else { + return fmt.Errorf("Operation aborted.") } - return matchDroplets(c.Args, ds, fn) + return nil + } type matchDropletsFn func(ids []int) error From 4cd40ac3f0c63fdf992727e3e5088d60c9fb0506 Mon Sep 17 00:00:00 2001 From: Marko Mudrinic Date: Thu, 6 Oct 2016 20:15:08 +0200 Subject: [PATCH 2/5] Updated output and askForConfirmation --- commands/confirmation.go | 21 ++++++++------------- commands/droplets.go | 4 ++-- commands/errors.go | 3 +++ 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/commands/confirmation.go b/commands/confirmation.go index a2049c7ce..4388ac5b7 100644 --- a/commands/confirmation.go +++ b/commands/confirmation.go @@ -1,23 +1,18 @@ package commands import ( - "fmt" + "bufio" + "os" "strings" ) -func askForConfirm(message string) bool { - var answer string - warn("Are you sure you want to " + message + " (y/N) ? ") - _, err := fmt.Scanln(&answer) +func AskForConfirm(message string) bool { + reader := bufio.NewReader(os.Stdin) + warnConfirm("Are you sure you want to " + message + " (y/N) ? ") + answer, err := reader.ReadString('\n') if err != nil { return false } - return verifyAnswer(answer) -} - -func verifyAnswer(answer string) bool { - if strings.ToLower(answer) == "y" || strings.ToLower(answer) == "yes" { - return true - } - return false + answer = strings.ToLower(strings.Replace(answer, "\n", "", 1)) + return answer == "y" || answer == "ye" || answer == "yes" } diff --git a/commands/droplets.go b/commands/droplets.go index 195b5d570..b107646b8 100644 --- a/commands/droplets.go +++ b/commands/droplets.go @@ -434,7 +434,7 @@ func RunDropletDelete(c *CmdConfig) error { } else if len(c.Args) > 0 && tagName != "" { return fmt.Errorf("please specify droplets identifiers or a tag name") } else if tagName != "" { - if Force || askForConfirm("delete droplet by \""+tagName+"\" tag") { + if Force || AskForConfirm("delete droplet by \""+tagName+"\" tag") { return ds.DeleteByTag(tagName) } else { return fmt.Errorf("Operation aborted.") @@ -442,7 +442,7 @@ func RunDropletDelete(c *CmdConfig) error { return nil } - if Force || askForConfirm("delete droplet(s)") { + if Force || AskForConfirm("delete droplet(s)") { fn := func(ids []int) error { for _, id := range ids { diff --git a/commands/errors.go b/commands/errors.go index de6a8735f..7c14db78d 100644 --- a/commands/errors.go +++ b/commands/errors.go @@ -77,6 +77,9 @@ func checkErr(err error, cmd ...*cobra.Command) { func warn(msg string) { fmt.Fprintf(color.Output, "%s: %s\n\n", colorWarn, msg) } +func warnConfirm(msg string) { + fmt.Fprintf(color.Output, "%s: %s", colorWarn, msg) +} func notice(msg string) { fmt.Fprintf(color.Output, "%s: %s\n\n", colorNotice, msg) From ab634b978f6bb29f6c64633a11741e88e2952537 Mon Sep 17 00:00:00 2001 From: Marko Mudrinic Date: Fri, 7 Oct 2016 09:50:11 +0200 Subject: [PATCH 3/5] Created initial test units, AskForConfirm refactor --- commands/confirmation.go | 20 ++++++++--- commands/confirmation_test.go | 64 +++++++++++++++++++++++++++++++++++ commands/droplets.go | 4 +-- commands/droplets_test.go | 5 +++ 4 files changed, 87 insertions(+), 6 deletions(-) create mode 100644 commands/confirmation_test.go diff --git a/commands/confirmation.go b/commands/confirmation.go index 4388ac5b7..2af3efb6c 100644 --- a/commands/confirmation.go +++ b/commands/confirmation.go @@ -2,17 +2,29 @@ package commands import ( "bufio" + "fmt" "os" "strings" ) -func AskForConfirm(message string) bool { +var retrieveUserInput = func(message string) (string, error) { reader := bufio.NewReader(os.Stdin) warnConfirm("Are you sure you want to " + message + " (y/N) ? ") answer, err := reader.ReadString('\n') if err != nil { - return false + return "", err + } + return strings.ToLower(strings.Replace(answer, "\n", "", 1)), nil +} + +func AskForConfirm(message string) error { + answer, err := retrieveUserInput(message) + if err != nil { + return fmt.Errorf("unable to parse users input: %s", err) + } + if answer == "y" || answer == "ye" || answer == "yes" { + return nil + } else { + return fmt.Errorf("invaild user input") } - answer = strings.ToLower(strings.Replace(answer, "\n", "", 1)) - return answer == "y" || answer == "ye" || answer == "yes" } diff --git a/commands/confirmation_test.go b/commands/confirmation_test.go new file mode 100644 index 000000000..252f80e99 --- /dev/null +++ b/commands/confirmation_test.go @@ -0,0 +1,64 @@ +package commands + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestAskForConfirmYes(t *testing.T) { + rui := retrieveUserInput + defer func() { + retrieveUserInput = rui + }() + + retrieveUserInput = func(string) (string, error) { + return "yes", nil + } + + err := AskForConfirm("test") + assert.NoError(t, err) +} + +func TestAskForConfirmNo(t *testing.T) { + rui := retrieveUserInput + defer func() { + retrieveUserInput = rui + }() + + retrieveUserInput = func(string) (string, error) { + return "no", nil + } + + err := AskForConfirm("test") + assert.Error(t, err) +} + +func TestAskForConfirmAny(t *testing.T) { + rui := retrieveUserInput + defer func() { + retrieveUserInput = rui + }() + + retrieveUserInput = func(string) (string, error) { + return "some-random-message", nil + } + + err := AskForConfirm("test") + assert.Error(t, err) +} + +func TestAskForConfirmError(t *testing.T) { + rui := retrieveUserInput + defer func() { + retrieveUserInput = rui + }() + + retrieveUserInput = func(string) (string, error) { + return "", fmt.Errorf("test-error") + } + + err := AskForConfirm("test") + assert.Error(t, err) +} diff --git a/commands/droplets.go b/commands/droplets.go index b107646b8..71fd303bf 100644 --- a/commands/droplets.go +++ b/commands/droplets.go @@ -434,7 +434,7 @@ func RunDropletDelete(c *CmdConfig) error { } else if len(c.Args) > 0 && tagName != "" { return fmt.Errorf("please specify droplets identifiers or a tag name") } else if tagName != "" { - if Force || AskForConfirm("delete droplet by \""+tagName+"\" tag") { + if Force || AskForConfirm("delete droplet by \""+tagName+"\" tag") == nil { return ds.DeleteByTag(tagName) } else { return fmt.Errorf("Operation aborted.") @@ -442,7 +442,7 @@ func RunDropletDelete(c *CmdConfig) error { return nil } - if Force || AskForConfirm("delete droplet(s)") { + if Force || AskForConfirm("delete droplet(s)") == nil { fn := func(ids []int) error { for _, id := range ids { diff --git a/commands/droplets_test.go b/commands/droplets_test.go index d0b5165fa..6f8b186d5 100644 --- a/commands/droplets_test.go +++ b/commands/droplets_test.go @@ -140,10 +140,12 @@ func TestDropletDelete(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { tm.droplets.On("Delete", 1).Return(nil) + Force = true config.Args = append(config.Args, strconv.Itoa(testDroplet.ID)) err := RunDropletDelete(config) assert.NoError(t, err) + }) } @@ -151,6 +153,7 @@ func TestDropletDeleteByTag(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { tm.droplets.On("DeleteByTag", "my-tag").Return(nil) + Force = true config.Doit.Set(config.NS, doctl.ArgTagName, "my-tag") err := RunDropletDelete(config) @@ -163,6 +166,7 @@ func TestDropletDeleteRepeatedID(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { tm.droplets.On("Delete", 1).Return(nil).Once() + Force = true id := strconv.Itoa(testDroplet.ID) config.Args = append(config.Args, id, id) @@ -206,6 +210,7 @@ func TestDropletDelete_MixedNameAndType(t *testing.T) { err := RunDropletDelete(config) assert.NoError(t, err) + Force = false }) } From 958cdb4ae9ade081f326838a51089569b9fb42af Mon Sep 17 00:00:00 2001 From: Marko Mudrinic Date: Sat, 8 Oct 2016 19:25:13 +0200 Subject: [PATCH 4/5] force flag is now tied to delete command instead of global flag --- args.go | 3 +++ commands/doit.go | 4 ---- commands/droplets.go | 12 +++++++++--- commands/droplets_test.go | 10 ++++++---- 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/args.go b/args.go index 986f09efc..790e7228a 100644 --- a/args.go +++ b/args.go @@ -122,4 +122,7 @@ const ( ArgVolumeRegion = "region" // ArgVolumeList is the IDs of many volumes. ArgVolumeList = "volumes" + + // ArgDeleteForce forces deletion actions + ArgDeleteForce = "force" ) diff --git a/commands/doit.go b/commands/doit.go index a66bbf71c..cf5a9dc8e 100644 --- a/commands/doit.go +++ b/commands/doit.go @@ -53,9 +53,6 @@ var Output string // Verbose toggles verbose output. var Verbose bool -// Force froces command execution. -var Force bool - var requiredColor = color.New(color.Bold, color.FgWhite).SprintfFunc() // Writer is where output should be written to. @@ -80,7 +77,6 @@ func init() { DoitCmd.PersistentFlags().StringVarP(&Token, "access-token", "t", "", "API V2 Access Token") DoitCmd.PersistentFlags().StringVarP(&Output, "output", "o", "text", "output format [text|json]") DoitCmd.PersistentFlags().BoolVarP(&Verbose, "verbose", "v", false, "verbose output") - DoitCmd.PersistentFlags().BoolVarP(&Force, "force", "f", false, "force execution") DoitCmd.PersistentFlags().BoolVarP(&Trace, "trace", "", false, "trace api access") viper.SetEnvPrefix("DIGITALOCEAN") diff --git a/commands/droplets.go b/commands/droplets.go index 71fd303bf..24b5881eb 100644 --- a/commands/droplets.go +++ b/commands/droplets.go @@ -67,8 +67,9 @@ func Droplet() *Command { AddStringSliceFlag(cmdDropletCreate, doctl.ArgVolumeList, []string{}, "Volumes to attach") - CmdBuilder(cmd, RunDropletDelete, "delete ID [ID|Name ...]", "Delete droplet by id or name", Writer, + cmdRunDropletDelete := CmdBuilder(cmd, RunDropletDelete, "delete ID [ID|Name ...]", "Delete droplet by id or name", Writer, aliasOpt("d", "del", "rm"), docCategories("droplet")) + AddBoolFlag(cmdRunDropletDelete, doctl.ArgDeleteForce, false, "Froce droplet delete") CmdBuilder(cmd, RunDropletGet, "get", "get droplet", Writer, aliasOpt("g"), displayerType(&droplet{}), docCategories("droplet")) @@ -424,6 +425,11 @@ func allInt(in []string) ([]int, error) { func RunDropletDelete(c *CmdConfig) error { ds := c.Droplets() + force, err := c.Doit.GetBool(c.NS, doctl.ArgDeleteForce) + if err != nil { + return err + } + tagName, err := c.Doit.GetString(c.NS, doctl.ArgTagName) if err != nil { return err @@ -434,7 +440,7 @@ func RunDropletDelete(c *CmdConfig) error { } else if len(c.Args) > 0 && tagName != "" { return fmt.Errorf("please specify droplets identifiers or a tag name") } else if tagName != "" { - if Force || AskForConfirm("delete droplet by \""+tagName+"\" tag") == nil { + if force || AskForConfirm("delete droplet by \""+tagName+"\" tag") == nil { return ds.DeleteByTag(tagName) } else { return fmt.Errorf("Operation aborted.") @@ -442,7 +448,7 @@ func RunDropletDelete(c *CmdConfig) error { return nil } - if Force || AskForConfirm("delete droplet(s)") == nil { + if force || AskForConfirm("delete droplet(s)") == nil { fn := func(ids []int) error { for _, id := range ids { diff --git a/commands/droplets_test.go b/commands/droplets_test.go index 6f8b186d5..2b04a6832 100644 --- a/commands/droplets_test.go +++ b/commands/droplets_test.go @@ -140,8 +140,8 @@ func TestDropletDelete(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { tm.droplets.On("Delete", 1).Return(nil) - Force = true config.Args = append(config.Args, strconv.Itoa(testDroplet.ID)) + config.Doit.Set(config.NS, doctl.ArgDeleteForce, true) err := RunDropletDelete(config) assert.NoError(t, err) @@ -153,8 +153,8 @@ func TestDropletDeleteByTag(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { tm.droplets.On("DeleteByTag", "my-tag").Return(nil) - Force = true config.Doit.Set(config.NS, doctl.ArgTagName, "my-tag") + config.Doit.Set(config.NS, doctl.ArgDeleteForce, true) err := RunDropletDelete(config) assert.NoError(t, err) @@ -166,9 +166,9 @@ func TestDropletDeleteRepeatedID(t *testing.T) { withTestClient(t, func(config *CmdConfig, tm *tcMocks) { tm.droplets.On("Delete", 1).Return(nil).Once() - Force = true id := strconv.Itoa(testDroplet.ID) config.Args = append(config.Args, id, id) + config.Doit.Set(config.NS, doctl.ArgDeleteForce, true) err := RunDropletDelete(config) assert.NoError(t, err) @@ -181,6 +181,7 @@ func TestDropletDeleteByName(t *testing.T) { tm.droplets.On("Delete", 1).Return(nil) config.Args = append(config.Args, testDroplet.Name) + config.Doit.Set(config.NS, doctl.ArgDeleteForce, true) err := RunDropletDelete(config) assert.NoError(t, err) @@ -193,6 +194,7 @@ func TestDropletDeleteByName_Ambiguous(t *testing.T) { tm.droplets.On("List").Return(list, nil) config.Args = append(config.Args, testDroplet.Name) + config.Doit.Set(config.NS, doctl.ArgDeleteForce, true) err := RunDropletDelete(config) t.Log(err) @@ -207,10 +209,10 @@ func TestDropletDelete_MixedNameAndType(t *testing.T) { id := strconv.Itoa(testDroplet.ID) config.Args = append(config.Args, id, testDroplet.Name) + config.Doit.Set(config.NS, doctl.ArgDeleteForce, true) err := RunDropletDelete(config) assert.NoError(t, err) - Force = false }) } From 7198ea8550442bdf2f4758fc216f6222b5358166 Mon Sep 17 00:00:00 2001 From: Marko Mudrinic Date: Mon, 10 Oct 2016 15:19:09 +0200 Subject: [PATCH 5/5] fixed typo --- commands/droplets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/commands/droplets.go b/commands/droplets.go index 24b5881eb..5835e0b46 100644 --- a/commands/droplets.go +++ b/commands/droplets.go @@ -69,7 +69,7 @@ func Droplet() *Command { cmdRunDropletDelete := CmdBuilder(cmd, RunDropletDelete, "delete ID [ID|Name ...]", "Delete droplet by id or name", Writer, aliasOpt("d", "del", "rm"), docCategories("droplet")) - AddBoolFlag(cmdRunDropletDelete, doctl.ArgDeleteForce, false, "Froce droplet delete") + AddBoolFlag(cmdRunDropletDelete, doctl.ArgDeleteForce, false, "Force droplet delete") CmdBuilder(cmd, RunDropletGet, "get", "get droplet", Writer, aliasOpt("g"), displayerType(&droplet{}), docCategories("droplet"))