Skip to content

Commit 5dd0fb6

Browse files
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.
1 parent 60f0065 commit 5dd0fb6

File tree

5 files changed

+51
-10
lines changed

5 files changed

+51
-10
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ public class CreateVolumeCmd extends BaseAsyncCreateCustomIdCmd {
7979
description = "the ID of the disk offering. Either diskOfferingId or snapshotId must be passed in.")
8080
private Long diskOfferingId;
8181

82-
@Parameter(name = ApiConstants.NAME, type = CommandType.STRING, required = true, description = "the name of the disk volume")
82+
@Parameter(name = ApiConstants.NAME, type = CommandType.STRING, description = "the name of the disk volume")
8383
private String volumeName;
8484

8585
@Parameter(name = ApiConstants.SIZE, type = CommandType.LONG, description = "Arbitrary volume size")

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,21 @@ public VolumeVO doInTransaction(TransactionStatus status) {
476476
});
477477
}
478478

479+
/*
480+
* It retrieves the volume name from CreateVolumeCmd object.
481+
* If the retrieved volume name is null, empty or blank, then A random name
482+
* will be generated using getRandomVolumeName method.
483+
*/
484+
public String getVolumeNameFromCommand(CreateVolumeCmd cmd) {
485+
String userSpecifiedName = cmd.getVolumeName();
486+
487+
if (userSpecifiedName == null || userSpecifiedName.isEmpty() || (userSpecifiedName.trim().length() == 0)) {
488+
userSpecifiedName = getRandomVolumeName();
489+
}
490+
491+
return userSpecifiedName;
492+
}
493+
479494
/*
480495
* Just allocate a volume in the database, don't send the createvolume cmd
481496
* to hypervisor. The volume will be finally created only when it's attached
@@ -671,10 +686,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept
671686
throw new InvalidParameterValueException("Zone is not configured to use local storage but volume's disk offering " + diskOffering.getName() + " uses it");
672687
}
673688

674-
String userSpecifiedName = cmd.getVolumeName();
675-
if (userSpecifiedName == null) {
676-
userSpecifiedName = getRandomVolumeName();
677-
}
689+
String userSpecifiedName = getVolumeNameFromCommand(cmd);
678690

679691
VolumeVO volume = commitVolume(cmd, caller, owner, displayVolume, zoneId, diskOfferingId, provisioningType, size,
680692
minIops, maxIops, parentVolume, userSpecifiedName, _uuidMgr.generateUuid(Volume.class, cmd.getCustomId()));

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

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@
2929
import javax.inject.Inject;
3030

3131
import com.cloud.user.User;
32+
import junit.framework.Assert;
33+
import org.apache.cloudstack.api.command.user.volume.CreateVolumeCmd;
3234
import org.junit.After;
3335
import org.junit.Before;
3436
import org.junit.Rule;
@@ -97,6 +99,8 @@ public class VolumeApiServiceImplTest {
9799
SnapshotInfo snapshotInfoMock;
98100
@Mock
99101
VolumeService volService;
102+
@Mock
103+
CreateVolumeCmd createVol;
100104

101105
DetachVolumeCmd detachCmd = new DetachVolumeCmd();
102106
Class<?> _detachCmdClass = detachCmd.getClass();
@@ -355,6 +359,34 @@ public void testTakeSnapshotF2() throws ResourceAllocationException {
355359
_svc.takeSnapshot(5L, Snapshot.MANUAL_POLICY_ID, 3L, null, false);
356360
}
357361

362+
// When the volume name is Null.
363+
@Test
364+
public void testNullGetVolumeNameFromCmd() {
365+
when(createVol.getVolumeName()).thenReturn(null);
366+
Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
367+
}
368+
369+
// When the volume name is empty.
370+
@Test
371+
public void testEmptyGetVolumeNameFromCmd() {
372+
when(createVol.getVolumeName()).thenReturn("");
373+
Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
374+
}
375+
376+
// When the volume name is blank.
377+
@Test
378+
public void testBlankGetVolumeNameFromCmd() {
379+
when(createVol.getVolumeName()).thenReturn(" ");
380+
Assert.assertNotNull(_svc.getVolumeNameFromCommand(createVol));
381+
}
382+
383+
// When the volume name is non-empty.
384+
@Test
385+
public void testNonEmptyGetVolumeNameFromCmd() {
386+
when(createVol.getVolumeName()).thenReturn("abc");
387+
Assert.assertSame(_svc.getVolumeNameFromCommand(createVol), "abc");
388+
}
389+
358390
@After
359391
public void tearDown() {
360392
CallContext.unregister();

ui/scripts/docs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1008,7 +1008,7 @@ cloudStack.docs = {
10081008
},
10091009
// Add volume
10101010
helpVolumeName: {
1011-
desc: 'Give the volume a unique name so you can find it later.',
1011+
desc: 'Give a unique volume name. If it is not provided, a name will be generated randomly.',
10121012
externalLink: ''
10131013
},
10141014
helpVolumeAvailabilityZone: {

ui/scripts/storage.js

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,7 @@
8080
fields: {
8181
name: {
8282
docID: 'helpVolumeName',
83-
label: 'label.name',
84-
validation: {
85-
required: true
86-
}
83+
label: 'label.name'
8784
},
8885
availabilityZone: {
8986
label: 'label.availability.zone',

0 commit comments

Comments
 (0)