Merge pull request #1319 from nitin-maharana/CloudStack-Nitin15_4.7

CLOUDSTACK-9132: API createVolume takes empty string for name parameterSteps to Reproduce:
================

Create a volume using createVolume API where parameter name is empty.
It creates a volume with empty name.
But the name parameter is mandatory.(Issue)

Expected Behaviour:
================

It shouldn't create a volume with an empty name. Error should be returned.

Solution:
=======

Added a condition to check in case of empty string. If the name is an empty string, it generates a random name for the volume. Made the name field optional in UI as well as in API.

* pr/1319:
  CLOUDSTACK-9132: API createVolume takes empty string for name parameter

Signed-off-by: Remi Bergsma <github@remi.nl>

api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java

@@ -79,7 +79,7 @@ public class CreateVolumeCmd extends BaseAsyncCreateCustomIdCmd {
                description = "the ID of the disk offering. Either diskOfferingId or snapshotId must be passed in.")
     private Long diskOfferingId;
 
-    @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the disk volume")
+    @Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "the name of the disk volume")
     private String volumeName;
 
     @Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, description = "Arbitrary volume size")

server/src/com/cloud/storage/VolumeApiServiceImpl.java

@@ -476,6 +476,25 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
         });
     }
 
+    /**
+     * Retrieves the volume name from CreateVolumeCmd object.
+     *
+     * If the retrieved volume name is null, empty or blank, then A random name
+     * will be generated using getRandomVolumeName method.
+     *
+     * @param cmd
+     * @return Either the retrieved name or a random name.
+     */
+    public String getVolumeNameFromCommand(CreateVolumeCmd cmd) {
+        String userSpecifiedName = cmd.getVolumeName();
+
+        if (org.apache.commons.lang.StringUtils.isBlank(userSpecifiedName)) {
+            userSpecifiedName = getRandomVolumeName();
+        }
+
+        return userSpecifiedName;
+    }
+
     /*
      * Just allocate a volume in the database, don't send the createvolume cmd
      * to hypervisor. The volume will be finally created only when it's attached
@@ -671,10 +690,7 @@ public class VolumeApiServiceImpl extends ManagerBase implements VolumeApiServic
             throw new InvalidParameterValueException("Zone is not configured to use local storage but volume's disk offering " + diskOffering.getName() + " uses it");
         }
 
-        String userSpecifiedName = cmd.getVolumeName();
+        String userSpecifiedName = getVolumeNameFromCommand(cmd);
-        if (userSpecifiedName == null) {
-            userSpecifiedName = getRandomVolumeName();
-        }
 
         VolumeVO volume = commitVolume(cmd, caller, owner, displayVolume, zoneId, diskOfferingId, provisioningType, size,
                 minIops, maxIops, parentVolume, userSpecifiedName, _uuidMgr.generateUuid(Volume.class, cmd.getCustomId()));

server/test/com/cloud/storage/VolumeApiServiceImplTest.java

@@ -29,6 +29,8 @@ import java.util.UUID;
 import javax.inject.Inject;
 
 import com.cloud.user.User;
+import junit.framework.Assert;
+import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
@@ -97,6 +99,8 @@ public class VolumeApiServiceImplTest {
     SnapshotInfo snapshotInfoMock;
     @Mock
     VolumeService volService;
+    @Mock
+    CreateVolumeCmd createVol;
 
     DetachVolumeCmd detachCmd = new DetachVolumeCmd();
     Class<?> _detachCmdClass = detachCmd.getClass();
@@ -355,6 +359,30 @@ public class VolumeApiServiceImplTest {
         _svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false);
     }
 
+    @Test
+    public void testNullGetVolumeNameFromCmd() {
+        when(createVol.getVolumeName()).thenReturn(null);
+        Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
+    }
+
+    @Test
+    public void testEmptyGetVolumeNameFromCmd() {
+        when(createVol.getVolumeName()).thenReturn("");
+        Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
+    }
+
+    @Test
+    public void testBlankGetVolumeNameFromCmd() {
+        when(createVol.getVolumeName()).thenReturn("   ");
+        Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
+    }
+
+    @Test
+    public void testNonEmptyGetVolumeNameFromCmd() {
+        when(createVol.getVolumeName()).thenReturn("abc");
+        Assert.assertSame(_svc.getVolumeNameFromCommand(createVol), "abc");
+    }
+
     @After
     public void tearDown() {
         CallContext.unregister();

ui/scripts/docs.js

@@ -1008,7 +1008,7 @@ cloudStack.docs = {
     },
     // Add volume
     helpVolumeName: {
-        desc: 'Give the volume a unique name so you can find it later.',
+        desc: 'Give a unique volume name. If it is not provided, a name will be generated randomly.',
         externalLink: ''
     },
     helpVolumeAvailabilityZone: {

ui/scripts/storage.js

@@ -80,10 +80,7 @@
                                 fields: {
                                     name: {
                                         docID: 'helpVolumeName',
-                                        label: 'label.name',
+                                        label: 'label.name'
-                                        validation: {
-                                            required: true
-                                        }
                                     },
                                     availabilityZone: {
                                         label: 'label.availability.zone',