Skip to content

Commit de8e639

Browse files
CLOUDSTACK-9132: API createVolume takes empty string for name parameter
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 as optional in UI as well as in API. Added the unit tests.
1 parent 7e12ebf commit de8e639

File tree

5 files changed

+41
-10
lines changed

5 files changed

+41
-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: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,18 @@ public VolumeVO doInTransaction(TransactionStatus status) {
476476
});
477477
}
478478

479+
//Returns the volume name.
480+
//If it is null or not specified then generates a random name.
481+
public String getVolumeNameFromCommand(CreateVolumeCmd cmd) {
482+
String userSpecifiedName = cmd.getVolumeName();
483+
484+
if (userSpecifiedName == null || userSpecifiedName.isEmpty()) {
485+
userSpecifiedName = getRandomVolumeName();
486+
}
487+
488+
return userSpecifiedName;
489+
}
490+
479491
/*
480492
* Just allocate a volume in the database, don't send the createvolume cmd
481493
* to hypervisor. The volume will be finally created only when it's attached
@@ -671,10 +683,7 @@ public VolumeVO allocVolume(CreateVolumeCmd cmd) throws ResourceAllocationExcept
671683
throw new InvalidParameterValueException("Zone is not configured to use local storage but volume's disk offering " + diskOffering.getName() + " uses it");
672684
}
673685

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

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

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

Lines changed: 25 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,27 @@ 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 non-empty.
377+
@Test
378+
public void testNonEmptyGetVolumeNameFromCmd() {
379+
when(createVol.getVolumeName()).thenReturn("abc");
380+
Assert.assertSame(_svc.getVolumeNameFromCommand(createVol), "abc");
381+
}
382+
358383
@After
359384
public void tearDown() {
360385
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 the volume a unique name. If not, it 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)