From 8f6721ed4c4e1b31081a951c62ffbe5331cf16d4 Mon Sep 17 00:00:00 2001 From: Daniel Augusto Veronezi Salvador <38945620+GutoVeronezi@users.noreply.github.com> Date: Mon, 19 Feb 2024 15:35:54 -0300 Subject: [PATCH] Improve some UserVmManagerImpl's methods name and docs (#8673) Co-authored-by: Daniel Augusto Veronezi Salvador --- .../java/com/cloud/vm/UserVmManagerImpl.java | 33 ++++++++++--------- .../com/cloud/vm/UserVmManagerImplTest.java | 26 +++++++-------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java b/server/src/main/java/com/cloud/vm/UserVmManagerImpl.java index 770826ecfa54..c2ba8b59bf55 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 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 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 UserVm upgradeVirtualMachine(ScaleVMCmd cmd) throws ResourceUnavailableEx 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 4ed0e5b6250a..07d2e9bd59f0 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 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"); @@ -1467,12 +1467,12 @@ public void updateInstanceDetailsKeepCurrentValueIfNullTestDetailsConstantIsNotN } @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 void updateInstanceDetailsKeepCurrentValueIfNullTestNewValueIsNotNullDoNo } @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 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"); @@ -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