From 55f8b32aa00baf0609f54891230269ed97bedb5a Mon Sep 17 00:00:00 2001 From: Nitin Kumar Maharana Date: Tue, 29 Dec 2015 16:45:41 +0530 Subject: [PATCH] CLOUDSTACK-9132: API createVolume takes empty string for name parameter Added conditions to check if the name is empty or blank. If it is empty or blank, then it generates a random name. Made the name field as optional in UI as well as in API. Added required unit tests. --- .../command/user/volume/CreateVolumeCmd.java | 2 +- .../cloud/storage/VolumeApiServiceImpl.java | 24 +++++++++++++--- .../storage/VolumeApiServiceImplTest.java | 28 +++++++++++++++++++ ui/scripts/docs.js | 2 +- ui/scripts/storage.js | 5 +--- 5 files changed, 51 insertions(+), 10 deletions(-) diff --git a/api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java b/api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java index 1e3c01cec9e..54c376e2265 100644 --- a/api/src/org/apache/cloudstack/api/command/user/volume/CreateVolumeCmd.java +++ b/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") diff --git a/server/src/com/cloud/storage/VolumeApiServiceImpl.java b/server/src/com/cloud/storage/VolumeApiServiceImpl.java index afa5cc40089..5a53f9c531d 100644 --- a/server/src/com/cloud/storage/VolumeApiServiceImpl.java +++ b/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(); - if (userSpecifiedName == null) { - userSpecifiedName = getRandomVolumeName(); - } + String userSpecifiedName = getVolumeNameFromCommand(cmd); VolumeVO volume = commitVolume(cmd, caller, owner, displayVolume, zoneId, diskOfferingId, provisioningType, size, minIops, maxIops, parentVolume, userSpecifiedName, _uuidMgr.generateUuid(Volume.class, cmd.getCustomId())); diff --git a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java b/server/test/com/cloud/storage/VolumeApiServiceImplTest.java index 0e46142fe2c..4b502a404dc 100644 --- a/server/test/com/cloud/storage/VolumeApiServiceImplTest.java +++ b/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(); diff --git a/ui/scripts/docs.js b/ui/scripts/docs.js index ed6ab0c938c..30e123b950b 100755 --- a/ui/scripts/docs.js +++ b/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: { diff --git a/ui/scripts/storage.js b/ui/scripts/storage.js index 2f93917319c..39dfccf2ebe 100644 --- a/ui/scripts/storage.js +++ b/ui/scripts/storage.js @@ -80,10 +80,7 @@ fields: { name: { docID: 'helpVolumeName', - label: 'label.name', - validation: { - required: true - } + label: 'label.name' }, availabilityZone: { label: 'label.availability.zone',