Skip to content

Commit

Permalink
Improve some UserVmManagerImpl's methods name and docs (apache#8673)
Browse files Browse the repository at this point in the history 
Co-authored-by: Daniel Augusto Veronezi Salvador <[email protected]>
  • Loading branch information
@GutoVeronezi
GutoVeronezi and GutoVeronezi authored Feb 19, 2024
1 parent 592038a commit 8f6721e
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 29 deletions.
33 changes: 17 additions & 16 deletions server/src/main/java/com/cloud/vm/UserVmManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1217,34 +1217,35 @@ public UserVm upgradeVirtualMachine(UpgradeVMCmd cmd) throws ResourceAllocationE
}

/**
Updates the instance details map with the current values of the instance for the CPU speed, memory, and CPU number if they have not been specified.
Updates the instance details map with the current values for absent details. This only applies to details {@value VmDetailConstants#CPU_SPEED},
{@value VmDetailConstants#MEMORY}, and {@value VmDetailConstants#CPU_NUMBER}. This method only updates the map passed as parameter, not the database.
@param details Map containing the instance details.
@param vmInstance The virtual machine instance.
@param vmInstance The virtual machine instance to retrieve the current values.
@param newServiceOfferingId The ID of the new service offering.
*/

protected void updateInstanceDetails (Map<String, String> details, VirtualMachine vmInstance, Long newServiceOfferingId) {
protected void updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(Map<String, String> details, VirtualMachine vmInstance, Long newServiceOfferingId) {
ServiceOfferingVO currentServiceOffering = serviceOfferingDao.findByIdIncludingRemoved(vmInstance.getId(), vmInstance.getServiceOfferingId());
ServiceOfferingVO newServiceOffering = serviceOfferingDao.findById(newServiceOfferingId);
updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getSpeed(), details, VmDetailConstants.CPU_SPEED, currentServiceOffering.getSpeed());
updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getRamSize(), details, VmDetailConstants.MEMORY, currentServiceOffering.getRamSize());
updateInstanceDetailsKeepCurrentValueIfNull(newServiceOffering.getCpu(), details, VmDetailConstants.CPU_NUMBER, currentServiceOffering.getCpu());
addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(newServiceOffering.getSpeed(), details, VmDetailConstants.CPU_SPEED, currentServiceOffering.getSpeed());
addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(newServiceOffering.getRamSize(), details, VmDetailConstants.MEMORY, currentServiceOffering.getRamSize());
addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(newServiceOffering.getCpu(), details, VmDetailConstants.CPU_NUMBER, currentServiceOffering.getCpu());
}

/**
* Updates a specific instance detail with the current instance value if the new value is null.
* Adds the current detail value to the instance details map if a new value was not specified to it.
*
* @param newValue the new value to be set
* @param details a map of instance details
* @param detailsConstant the name of the detail constant to be updated
* @param currentValue the current value of the detail constant
* @param newValue the new value to be set.
* @param details a map of instance details.
* @param detailKey the detail to be updated.
* @param currentValue the current value of the detail constant.
*/

protected void updateInstanceDetailsKeepCurrentValueIfNull(Integer newValue, Map<String, String> details, String detailsConstant, Integer currentValue) {
if (newValue == null && details.get(detailsConstant) == null) {
protected void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(Integer newValue, Map<String, String> details, String detailKey, Integer currentValue) {
if (newValue == null && details.get(detailKey) == null) {
String currentValueString = String.valueOf(currentValue);
logger.debug("{} was not specified, keeping the current value: {}.", detailsConstant, currentValueString);
details.put(detailsConstant, currentValueString);
logger.debug("{} was not specified, keeping the current value: {}.", detailKey, currentValueString);
details.put(detailKey, currentValueString);
}
}

Expand Down Expand Up @@ -1917,7 +1918,7 @@ public UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx

Map<String, String> cmdDetails = cmd.getDetails();

updateInstanceDetails(cmdDetails, vm, newServiceOfferingId);
updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(cmdDetails, vm, newServiceOfferingId);

boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmdDetails);
if (result) {
Expand Down
26 changes: 13 additions & 13 deletions server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1454,11 +1454,11 @@ public void testRestoreVirtualMachineWithVMSnapshots() throws ResourceUnavailabl
}

@Test
public void updateInstanceDetailsKeepCurrentValueIfNullTestDetailsConstantIsNotNullDoNothing() {
public void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecifiedTestDetailsConstantIsNotNullDoNothing() {
int currentValue = 123;

for (String detailsConstant : detailsConstants) {
userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(null, customParameters, detailsConstant, currentValue);
userVmManagerImpl.addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(null, customParameters, detailsConstant, currentValue);
}

Assert.assertEquals(customParameters.get(VmDetailConstants.MEMORY), "2048");
Expand All @@ -1467,12 +1467,12 @@ public void updateInstanceDetailsKeepCurrentValueIfNullTestDetailsConstantIsNotN
}

@Test
public void updateInstanceDetailsKeepCurrentValueIfNullTestNewValueIsNotNullDoNothing() {
public void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecifiedTestNewValueIsNotNullDoNothing() {
Map<String, String> details = new HashMap<>();
int currentValue = 123;

for (String detailsConstant : detailsConstants) {
userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(321, details, detailsConstant, currentValue);
userVmManagerImpl.addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(321, details, detailsConstant, currentValue);
}

Assert.assertNull(details.get(VmDetailConstants.MEMORY));
Expand All @@ -1481,12 +1481,12 @@ public void updateInstanceDetailsKeepCurrentValueIfNullTestNewValueIsNotNullDoNo
}

@Test
public void updateInstanceDetailsKeepCurrentValueIfNullTestBothValuesAreNullKeepCurrentValue() {
public void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecifiedTestBothValuesAreNullKeepCurrentValue() {
Map<String, String> details = new HashMap<>();
int currentValue = 123;

for (String detailsConstant : detailsConstants) {
userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(null, details, detailsConstant, currentValue);
userVmManagerImpl.addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(null, details, detailsConstant, currentValue);
}

Assert.assertEquals(details.get(VmDetailConstants.MEMORY), String.valueOf(currentValue));
Expand All @@ -1495,11 +1495,11 @@ public void updateInstanceDetailsKeepCurrentValueIfNullTestBothValuesAreNullKeep
}

@Test
public void updateInstanceDetailsKeepCurrentValueIfNullTestNeitherValueIsNullDoNothing() {
public void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecifiedTestNeitherValueIsNullDoNothing() {
int currentValue = 123;

for (String detailsConstant : detailsConstants) {
userVmManagerImpl.updateInstanceDetailsKeepCurrentValueIfNull(321, customParameters, detailsConstant, currentValue);
userVmManagerImpl.addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(321, customParameters, detailsConstant, currentValue);
}

Assert.assertEquals(customParameters.get(VmDetailConstants.MEMORY), "2048");
Expand All @@ -1508,16 +1508,16 @@ public void updateInstanceDetailsKeepCurrentValueIfNullTestNeitherValueIsNullDoN
}

@Test
public void updateInstanceDetailsTestAllConstantsAreUpdated() {
public void updateInstanceDetailsMapWithCurrentValuesForAbsentDetailsTestAllConstantsAreUpdated() {
Mockito.doReturn(serviceOffering).when(_serviceOfferingDao).findById(Mockito.anyLong());
Mockito.doReturn(1L).when(vmInstanceMock).getId();
Mockito.doReturn(1L).when(vmInstanceMock).getServiceOfferingId();
Mockito.doReturn(serviceOffering).when(_serviceOfferingDao).findByIdIncludingRemoved(Mockito.anyLong(), Mockito.anyLong());
userVmManagerImpl.updateInstanceDetails(null, vmInstanceMock, 0l);
userVmManagerImpl.updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(null, vmInstanceMock, 0l);

Mockito.verify(userVmManagerImpl).updateInstanceDetailsKeepCurrentValueIfNull(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_SPEED), Mockito.any());
Mockito.verify(userVmManagerImpl).updateInstanceDetailsKeepCurrentValueIfNull(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.MEMORY), Mockito.any());
Mockito.verify(userVmManagerImpl).updateInstanceDetailsKeepCurrentValueIfNull(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_NUMBER), Mockito.any());
Mockito.verify(userVmManagerImpl).addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_SPEED), Mockito.any());
Mockito.verify(userVmManagerImpl).addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.MEMORY), Mockito.any());
Mockito.verify(userVmManagerImpl).addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(Mockito.any(), Mockito.any(), Mockito.eq(VmDetailConstants.CPU_NUMBER), Mockito.any());
}

@Test
Expand Down

0 comments on commit 8f6721e

Please sign in to comment.