Last review comments addressed

Author: nvazquez

server/src/main/java/com/cloud/vm/UserVmManagerImpl.java

@@ -650,6 +650,14 @@ public class UserVmManagerImpl extends ManagerBase implements UserVmManager, Vir
             HypervisorType.Simulator
     ));
 
+    protected static final List<HypervisorType> ROOT_DISK_SIZE_OVERRIDE_SUPPORTING_HYPERVISORS = Arrays.asList(
+            HypervisorType.KVM,
+            HypervisorType.XenServer,
+            HypervisorType.VMware,
+            HypervisorType.Simulator,
+            HypervisorType.Custom
+    );
+
     private static final List<HypervisorType> HYPERVISORS_THAT_CAN_DO_STORAGE_MIGRATION_ON_NON_USER_VMS = Arrays.asList(HypervisorType.KVM, HypervisorType.VMware);
 
     @Override
@@ -4327,8 +4335,7 @@ protected long configureCustomRootDiskSize(Map<String, String> customParameters,
      * @throws InvalidParameterValueException if the hypervisor does not support rootdisksize override
      */
     protected void verifyIfHypervisorSupportsRootdiskSizeOverride(HypervisorType hypervisorType) {
-        if (!(hypervisorType == HypervisorType.KVM || hypervisorType == HypervisorType.XenServer ||hypervisorType == HypervisorType.VMware ||
-                hypervisorType == HypervisorType.Simulator || hypervisorType == HypervisorType.Custom)) {
+        if (!ROOT_DISK_SIZE_OVERRIDE_SUPPORTING_HYPERVISORS.contains(hypervisorType)) {
             throw new InvalidParameterValueException("Hypervisor " + hypervisorType + " does not support rootdisksize override");
         }
     }
@@ -5056,11 +5063,7 @@ public boolean finalizeStart(VirtualMachineProfile profile, long hostId, Command
             }
         }
 
-        if (returnedVncPassword != null && !originalVncPassword.equals(returnedVncPassword)) {
-            UserVmVO userVm = _vmDao.findById(profile.getId());
-            userVm.setVncPassword(returnedVncPassword);
-            _vmDao.update(userVm.getId(), userVm);
-        }
+        updateVncPasswordIfItHasChanged(originalVncPassword, returnedVncPassword, profile);
 
         // get system ip and create static nat rule for the vm
         try {
@@ -5096,6 +5099,14 @@ public void doInTransactionWithoutResult(TransactionStatus status) {
         return true;
     }
 
+    protected void updateVncPasswordIfItHasChanged(String originalVncPassword, String returnedVncPassword, VirtualMachineProfile profile) {
+        if (returnedVncPassword != null && !originalVncPassword.equals(returnedVncPassword)) {
+            UserVmVO userVm = _vmDao.findById(profile.getId());
+            userVm.setVncPassword(returnedVncPassword);
+            _vmDao.update(userVm.getId(), userVm);
+        }
+    }
+
     @Override
     public void finalizeExpunge(VirtualMachine vm) {
     }

server/src/test/java/com/cloud/vm/UserVmManagerImplTest.java

@@ -221,6 +221,9 @@ public class UserVmManagerImplTest {
     @Mock
     UserDataManager userDataManager;
 
+    @Mock
+    VirtualMachineProfile virtualMachineProfile;
+
     private static final long vmId = 1l;
     private static final long zoneId = 2L;
     private static final long accountId = 3L;
@@ -248,6 +251,8 @@ public void beforeTest() {
         customParameters.put(VmDetailConstants.ROOT_DISK_SIZE, "123");
         lenient().doNothing().when(resourceLimitMgr).incrementResourceCount(anyLong(), any(Resource.ResourceType.class));
         lenient().doNothing().when(resourceLimitMgr).decrementResourceCount(anyLong(), any(Resource.ResourceType.class), anyLong());
+
+        Mockito.when(virtualMachineProfile.getId()).thenReturn(vmId);
     }
 
     @After
@@ -564,11 +569,7 @@ public void verifyIfHypervisorSupportRootdiskSizeOverrideTest() {
         int expectedExceptionCounter = hypervisorTypeArray.length - 5;
 
         for(int i = 0; i < hypervisorTypeArray.length; i++) {
-            if (Hypervisor.HypervisorType.KVM == hypervisorTypeArray[i]
-                    || Hypervisor.HypervisorType.XenServer == hypervisorTypeArray[i]
-                    || Hypervisor.HypervisorType.VMware == hypervisorTypeArray[i]
-                    || Hypervisor.HypervisorType.Custom == hypervisorTypeArray[i]
-                    || Hypervisor.HypervisorType.Simulator == hypervisorTypeArray[i]) {
+            if (UserVmManagerImpl.ROOT_DISK_SIZE_OVERRIDE_SUPPORTING_HYPERVISORS.contains(hypervisorTypeArray[i])) {
                 userVmManagerImpl.verifyIfHypervisorSupportsRootdiskSizeOverride(hypervisorTypeArray[i]);
             } else {
                 try {
@@ -1042,4 +1043,21 @@ public void testChooseVmMigrationDestinationUsingVolumePoolMapValid() {
         Pair<VMInstanceVO, Host> pair = mockObjectsForChooseVmMigrationDestinationUsingVolumePoolMapTest(false, destinationHost);
         Assert.assertEquals(destinationHost, userVmManagerImpl.chooseVmMigrationDestinationUsingVolumePoolMap(pair.first(), pair.second(), null));
     }
+
+    @Test
+    public void testUpdateVncPasswordIfItHasChanged() {
+        String vncPassword = "12345678";
+        userVmManagerImpl.updateVncPasswordIfItHasChanged(vncPassword, vncPassword, virtualMachineProfile);
+        Mockito.verify(userVmDao, Mockito.never()).update(vmId, userVmVoMock);
+    }
+
+    @Test
+    public void testUpdateVncPasswordIfItHasChangedNewPassword() {
+        String vncPassword = "12345678";
+        String newPassword = "87654321";
+        Mockito.when(userVmVoMock.getId()).thenReturn(vmId);
+        userVmManagerImpl.updateVncPasswordIfItHasChanged(vncPassword, newPassword, virtualMachineProfile);
+        Mockito.verify(userVmDao).findById(vmId);
+        Mockito.verify(userVmDao).update(vmId, userVmVoMock);
+    }
 }