diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 770826ecfa5..c2ba8b59bf5 100644 --- a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java +++ b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java @@ -1217,34 +1217,35 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir } /** - 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 details, VirtualMachine vmInstance, Long newServiceOfferingId) { + protected void updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(Map 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 details, String detailsConstant, Integer currentValue) { - if (newValue == null && details.get(detailsConstant) == null) { + protected void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecified(Integer newValue, Map 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); } } @@ -1917,7 +1918,7 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir Map cmdDetails = cmd.getDetails(); - updateInstanceDetails(cmdDetails, vm, newServiceOfferingId); + updateInstanceDetailsMapWithCurrentValuesForAbsentDetails(cmdDetails, vm, newServiceOfferingId); boolean result = upgradeVirtualMachine(vmId, newServiceOfferingId, cmdDetails); if (result) { diff --git a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java index 4ed0e5b6250..07d2e9bd59f 100644 --- a/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java +++ b/server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java @@ -1454,11 +1454,11 @@ public class UserVmManagerImplTest { } @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"); @@ -1467,12 +1467,12 @@ public class UserVmManagerImplTest { } @Test - public void updateInstanceDetailsKeepCurrentValueIfNullTestNewValueIsNotNullDoNothing() { + public void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecifiedTestNewValueIsNotNullDoNothing() { Map 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)); @@ -1481,12 +1481,12 @@ public class UserVmManagerImplTest { } @Test - public void updateInstanceDetailsKeepCurrentValueIfNullTestBothValuesAreNullKeepCurrentValue() { + public void addCurrentDetailValueToInstanceDetailsMapIfNewValueWasNotSpecifiedTestBothValuesAreNullKeepCurrentValue() { Map 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)); @@ -1495,11 +1495,11 @@ public class UserVmManagerImplTest { } @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"); @@ -1508,16 +1508,16 @@ public class UserVmManagerImplTest { } @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