Merge pull request #36 from digitalocean/droplet-delete-ambiguous

Add more logic around droplet deletes

commands/droplets.go

@@ -14,7 +14,6 @@ limitations under the License.
 package commands
 
 import (
-	"errors"
 	"fmt"
 	"io/ioutil"
 	"strconv"
@@ -268,6 +267,18 @@ func extractUserData(userData, filename string) (string, error) {
 	return userData, nil
 }
 
+func allInt(in []string) ([]int, error) {
+	out := []int{}
+	for _, i := range in {
+		id, err := strconv.Atoi(i)
+		if err != nil {
+			return nil, fmt.Errorf("%s is not an int", i)
+		}
+		out = append(out, id)
+	}
+	return out, nil
+}
+
 // RunDropletDelete destroy a droplet by id.
 func RunDropletDelete(c *CmdConfig) error {
 
@@ -277,35 +288,69 @@ func RunDropletDelete(c *CmdConfig) error {
 		return doit.NewMissingArgsErr(c.NS)
 	}
 
-	listedDroplets := false
-	list := do.Droplets{}
+	// if list is all int, go down list
+	if out, err := allInt(c.Args); err == nil {
+		toDelete := map[int]struct{}{}
+		for _, id := range out {
+			toDelete[id] = struct{}{}
+		}
+
+		for id := range toDelete {
+			if err = ds.Delete(id); err != nil {
+				return fmt.Errorf("unable to delete droplet %d: %v", id, err)
+			}
+			fmt.Printf("deleted droplet %d\n", id)
+		}
+
+		return nil
+	}
+
+	// if list has strings in it, fetch the list
+	list, err := ds.List()
+	if err != nil {
+		return fmt.Errorf("unable to create list of droplets: %v", err)
+	}
 
+	dropletNames := map[string]int{}
+	dropletList := map[string]int{}
+	dropletIDs := map[string][]string{}
+	for _, d := range list {
+		dropletNames[d.Name]++
+		dropletList[d.Name] = d.ID
+		dropletIDs[d.Name] = append(dropletIDs[d.Name], strconv.Itoa(d.ID))
+	}
+
+	toDelete := map[int]bool{}
 	for _, idStr := range c.Args {
+		if dropletNames[idStr] > 1 {
+			return fmt.Errorf("there are %d Droplets with the name %q, please delete by id. [%s]",
+				dropletNames[idStr], idStr, strings.Join(dropletIDs[idStr], ", "))
+		}
+
 		id, err := strconv.Atoi(idStr)
 		if err != nil {
-			if !listedDroplets {
-				list, err = ds.List()
-				if err != nil {
-					return errors.New("unable to build list of droplets")
-				}
-				listedDroplets = true
+			id, ok := dropletList[idStr]
+			if !ok {
+				return fmt.Errorf("droplet with name %q could not be found", idStr)
 			}
 
-			var matchedDroplet *do.Droplet
-			for _, d := range list {
-				if d.Name == idStr {
-					matchedDroplet = &d
-					break
-				}
+			if toDelete[id] {
+				warn(fmt.Sprintf("droplet %q (%d) has already been marked for deletion",
+					idStr, dropletList[idStr]))
 			}
+			toDelete[id] = true
+			continue
+		}
 
-			if matchedDroplet == nil {
-				return fmt.Errorf("unable to find droplet with name %q", idStr)
-			}
+		if toDelete[id] {
+			warn(fmt.Sprintf("droplet %q (%d) has already been marked for deletion",
+				idStr, dropletList[idStr]))
 
-			id = matchedDroplet.ID
 		}
+		toDelete[id] = true
+	}
 
+	for id := range toDelete {
 		err = ds.Delete(id)
 		if err != nil {
 			return fmt.Errorf("unable to delete droplet %d: %v", id, err)

commands/droplets_test.go

@@ -106,6 +106,18 @@ func TestDropletDelete(t *testing.T) {
 	})
 }
 
+func TestDropletDeleteRepeatedID(t *testing.T) {
+	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+		tm.droplets.On("Delete", 1).Return(nil).Once()
+
+		id := strconv.Itoa(testDroplet.ID)
+		config.Args = append(config.Args, id, id)
+
+		err := RunDropletDelete(config)
+		assert.NoError(t, err)
+	})
+}
+
 func TestDropletDeleteByName(t *testing.T) {
 	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
 		tm.droplets.On("List").Return(testDropletList, nil)
@@ -118,6 +130,33 @@ func TestDropletDeleteByName(t *testing.T) {
 	})
 }
 
+func TestDropletDeleteByName_Ambiguous(t *testing.T) {
+	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+		list := do.Droplets{testDroplet, testDroplet}
+		tm.droplets.On("List").Return(list, nil)
+
+		config.Args = append(config.Args, testDroplet.Name)
+
+		err := RunDropletDelete(config)
+		t.Log(err)
+		assert.Error(t, err)
+	})
+}
+
+func TestDropletDelete_MixedNameAndType(t *testing.T) {
+	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
+		tm.droplets.On("List").Return(testDropletList, nil)
+		tm.droplets.On("Delete", 1).Return(nil).Once()
+
+		id := strconv.Itoa(testDroplet.ID)
+		config.Args = append(config.Args, id, testDroplet.Name)
+
+		err := RunDropletDelete(config)
+		assert.NoError(t, err)
+	})
+
+}
+
 func TestDropletGet(t *testing.T) {
 	withTestClient(t, func(config *CmdConfig, tm *tcMocks) {
 		tm.droplets.On("Get", testDroplet.ID).Return(&testDroplet, nil)

commands/errors.go

@@ -24,7 +24,8 @@ import (
 )
 
 var (
-	colorErr = color.New(color.FgRed).SprintFunc()("Error")
+	colorErr  = color.New(color.FgRed).SprintFunc()("Error")
+	colorWarn = color.New(color.FgYellow).SprintFunc()("Warning")
 
 	// errAction specifies what should happen when an error occurs
 	errAction = func() {
@@ -66,3 +67,7 @@ func checkErr(err error, cmd ...*cobra.Command) {
 
 	errAction()
 }
+
+func warn(msg string) {
+	fmt.Fprintf(color.Output, "%s: %s\n", colorWarn, msg)
+}