#722 added StorageManager.GetVolumeId function, used it in volume-detail to resolve IDENTIFIER

Author: allmightyspiff

plugin/commands/block/volume_detail.go

@@ -4,7 +4,6 @@ import (
 	"bytes"
 	"fmt"
 	"net/url"
-	"strconv"
 	"strings"
 
 	"github.com/IBM-Cloud/ibm-cloud-cli-sdk/bluemix/terminal"
@@ -43,21 +42,9 @@ func NewVolumeDetailCommand(sl *metadata.SoftlayerStorageCommand) *VolumeDetailC
 
 func (cmd *VolumeDetailCommand) Run(args []string) error {
 
-	volumeID, err := strconv.Atoi(args[0])
+	volumeID, err := cmd.StorageManager.GetVolumeId(args[0], cmd.StorageType)
 	if err != nil {
-		// Maybe this is a volume username
-		volumes, err := cmd.StorageManager.ListVolumes(cmd.StorageType, "", args[0], "", "", 0, "mask[id,username]")
-		if err != nil {
-			fmt.Printf("=============== ERROR: %s\n", err.Error())
-			return slErr.NewInvalidSoftlayerIdInputError("Volume ID")
-		}
-		if len(volumes) != 1 {
-			subs := map[string]interface{}{"VolumeName": args[0], "VolumeCount": len(volumes)}
-			return slErr.New(
-				T("Search for volume {{.VolumeName}} found {{.VolumeCount}} volumes, expected 1.", subs),
-			)
-		}
-		volumeID = *volumes[0].Id
+		return err
 	}
 
 	outputFormat := cmd.GetOutputFlag()

plugin/commands/block/volume_detail_test.go

@@ -27,25 +27,18 @@ var _ = Describe("Block Volume-Detail Tests", func() {
 		slCommand = metadata.NewSoftlayerStorageCommand(fakeUI, fakeSession, "block")
 		cliCommand = block.NewVolumeDetailCommand(slCommand)
 		cliCommand.Command.PersistentFlags().Var(cliCommand.OutputFlag, "output", "--output=JSON for json output.")
-
 	})
 
-	Describe("Volume detail", func() {
+	Describe("Volume detail usage tests", func() {
 		Context("Volume detail without volume id", func() {
 			It("return error", func() {
 				err := testhelpers.RunCobraCommand(cliCommand.Command)
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring("Incorrect Usage: This command requires one argument"))
 			})
 		})
-		Context("Volume detail with wrong volume id", func() {
-			It("return error", func() {
-				err := testhelpers.RunCobraCommand(cliCommand.Command, "abc")
-				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(ContainSubstring("Invalid input for 'Volume ID'. It must be a positive integer."))
-			})
-		})
-
+	})
+	Describe("Volume Detail API response tests", func() {
 		Context("Volume detail with correct volume id but server API call fails", func() {
 			BeforeEach(func() {
 				fakeHandler.AddApiError("SoftLayer_Network_Storage", "getObject", 500, "Internal Server Error")

plugin/managers/file_test.go

@@ -112,6 +112,7 @@ type StorageManager interface {
 	ListItems(packageId int, categoryCode string, mask string) ([]datatypes.Product_Item, error)
 	GetRegions(packageId int) ([]datatypes.Location_Region, error)
 	GetNetworkMessageDeliveryAccounts(storageId int, mask string) (datatypes.Network_Storage_Hub_Cleversafe_Account, error)
+	GetVolumeId(indentifier string, storageType string) (int, error)
 }
 
 type storageManager struct {
@@ -964,3 +965,32 @@ func (s storageManager) GetEndpoints(storageId int) ([]datatypes.Container_Netwo
 
 	return NetworkStorageHubCleversafeAccountService.Id(storageId).GetEndpoints(nil)
 }
+
+// Trys to resolve identifier to a block volume ID
+// if identifier is a int, will return that, otherwise does a API lookup assuming identifier is a volume username
+func (s storageManager) GetVolumeId(identifier string, storageType string) (int, error) {
+	var volumeId int
+	var err error
+	// identifier is a number, which means its already an ID
+	volumeId, err = strconv.Atoi(identifier)
+
+	// If there was an error, identifier is likely a string and username, search the API for it
+	if err != nil {
+		// Maybe this is a volume username
+		volumes, err := s.ListVolumes(storageType, "", identifier, "", "", 0, "mask[id,username]")
+		if err != nil {
+			// API error
+			return 0, err
+		}
+		// If we don't get 1 volume back, something went wrong. Either 0 volumes, or too many and the
+		// user should pick a better identifier
+		if len(volumes) != 1 {
+			subs := map[string]interface{}{"VolumeName": identifier, "VolumeCount": len(volumes)}
+			return 0, errors.New(
+				T("Search for volume {{.VolumeName}} found {{.VolumeCount}} volumes, expected 1.", subs),
+			)
+		}
+		volumeId = *volumes[0].Id
+	}
+	return volumeId, nil
+}