Refactoring some more vm commands, as well as refactoring the account and user validation into a method. Please use the same for future validation calls

server/src/com/cloud/api/commands/UpdateVMCmd.java

@@ -25,27 +25,21 @@ import java.util.Map;
 import org.apache.log4j.Logger;
 
 import com.cloud.api.BaseCmd;
+import com.cloud.api.Implementation;
 import com.cloud.api.Parameter;
 import com.cloud.api.ServerApiException;
+import com.cloud.api.BaseCmd.Manager;
+import com.cloud.api.response.UpgradeVmResponse;
 import com.cloud.user.Account;
 import com.cloud.utils.Pair;
 import com.cloud.vm.UserVmVO;
 
+@Implementation(method="updateVirtualMachine", manager=Manager.UserVmManager)
 public class UpdateVMCmd extends BaseCmd{
     public static final Logger s_logger = Logger.getLogger(UpdateVMCmd.class.getName());
     private static final String s_name = "updatevirtualmachineresponse";
     private static final List<Pair<Enum, Boolean>> s_properties = new ArrayList<Pair<Enum, Boolean>>();
 
-    static {
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.DISPLAY_NAME, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.GROUP, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.HA_ENABLE, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.ID, Boolean.TRUE));
-
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.ACCOUNT_OBJ, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.USER_ID, Boolean.FALSE));
-    }
-
     /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
     /////////////////////////////////////////////////////
@@ -86,71 +80,30 @@ public class UpdateVMCmd extends BaseCmd{
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
 
+    private UserVmVO responseObject = null;
+    
     @Override
     public String getName() {
         return s_name;
     }
-    @Override
+    
-    public List<Pair<Enum, Boolean>> getProperties() {
+    public static String getResultObjectName() {
-        return s_properties;
+    	return "virtualmachine";
     }    
     
     @Override
-    public List<Pair<String, Object>> execute(Map<String, Object> params) {
+    public String getResponse() 
-        Long userId = (Long)params.get(BaseCmd.Properties.USER_ID.getName());
+    {
-    	Account account = (Account)params.get(BaseCmd.Properties.ACCOUNT_OBJ.getName());
+        UpgradeVmResponse response = new UpgradeVmResponse();
-        Long vmId = (Long)params.get(BaseCmd.Properties.ID.getName());
+        UserVmVO userVm = (UserVmVO)getResponseObject();
-        String group = (String)params.get(BaseCmd.Properties.GROUP.getName());
-        String displayName = (String)params.get(BaseCmd.Properties.DISPLAY_NAME.getName());
-        Boolean enable = (Boolean)params.get(BaseCmd.Properties.HA_ENABLE.getName());
-        UserVmVO vmInstance = null;
         
-        // default userId to SYSTEM user
+        UserVmVO responseObject = (UserVmVO)getResponseObject();
-        if (userId == null) {
+        if (responseObject != null) 
-            userId = Long.valueOf(1);
+        {
+        	//just pass back success or failure from here
         }
         
-        // Verify input parameters
+        return null;
-        try {
-        	vmInstance = getManagementServer().findUserVMInstanceById(vmId.longValue());
-        } catch (Exception ex1) {
-        	throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find virtual machine by id");
     }
 
-        if (vmInstance == null) {
-            throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to find virtual machine with id " + vmId);
-        }
-
-        if (account != null) {
-            if (!isAdmin(account.getType()) && (account.getId().longValue() != vmInstance.getAccountId())) {
-                throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + vmId + " for this account");
-            } else if (!getManagementServer().isChildDomain(account.getDomainId(), vmInstance.getDomainId())) {
-                throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid virtual machine id (" + vmId + ") given, unable to update virtual machine.");
-            }
-        }
-
-        if (group == null) {
-    		group = vmInstance.getGroup();
-    	}
-
-    	if (displayName == null) {
-    		displayName = vmInstance.getDisplayName();
-    	}
-    	
-    	if (enable == null) {
-    		enable = vmInstance.isHaEnabled();
-    	}
-
-    	long accountId = vmInstance.getAccountId();
-
-        try {     
-        	getManagementServer().updateVirtualMachine(vmId, displayName, group, enable, userId, accountId);
-        } catch (Exception ex) {
-            throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update virtual machine" + vmId + ":  internal error.");
-        }
-
-        List<Pair<String, Object>> returnValues = new ArrayList<Pair<String, Object>>();
-        returnValues.add(new Pair<String, Object>(BaseCmd.Properties.SUCCESS.getName(), Boolean.TRUE));
-        return returnValues;
-    }  
 }

server/src/com/cloud/api/commands/UpgradeVMCmd.java

@@ -18,8 +18,6 @@
 
 package com.cloud.api.commands;
 
-import java.util.Date;
-
 import org.apache.log4j.Logger;
 
 import com.cloud.api.BaseCmd;
@@ -27,8 +25,6 @@ import com.cloud.api.Implementation;
 import com.cloud.api.Parameter;
 import com.cloud.api.BaseCmd.Manager;
 import com.cloud.api.response.UpgradeVmResponse;
-import com.cloud.domain.DomainVO;
-import com.cloud.serializer.Param;
 import com.cloud.vm.UserVmVO;
 
 @Implementation(method="upgradeVirtualMachine", manager=Manager.UserVmManager)

server/src/com/cloud/server/ManagementServer.java

@@ -744,7 +744,7 @@ public interface ManagementServer {
      * @param userId - id of user performing the update on the virtual machine
      * @param accountId - id of the account that owns the virtual machine
      */
-    void updateVirtualMachine(long vmId, String displayName, String group, boolean enable, Long userId, long accountId);
+//    void updateVirtualMachine(long vmId, String displayName, String group, boolean enable, Long userId, long accountId);
 
     /**
      * Updates a storage pool.

server/src/com/cloud/server/ManagementServerImpl.java

@@ -2710,30 +2710,6 @@ public class ManagementServerImpl implements ManagementServer {
     }
 */
     
-    @Override
-    public void updateVirtualMachine(long vmId, String displayName, String group, boolean enable, Long userId, long accountId) {
-        UserVmVO vm = _userVmDao.findById(vmId);
-        if (vm == null) {
-            throw new CloudRuntimeException("Unable to find virual machine with id " + vmId);
-        }
-
-        boolean haEnabled = vm.isHaEnabled();
-        _userVmDao.updateVM(vmId, displayName, group, enable);
-        if (haEnabled != enable) {
-            String description = null;
-            String type = null;
-            if (enable) {
-                description = "Successfully enabled HA for virtual machine " + vm.getName();
-                type = EventTypes.EVENT_VM_ENABLE_HA;
-            } else {
-                description = "Successfully disabled HA for virtual machine " + vm.getName();
-                type = EventTypes.EVENT_VM_DISABLE_HA;
-            }
-            // create a event for the change in HA Enabled flag
-            EventUtils.saveEvent(userId, accountId, EventVO.LEVEL_INFO, type, description, null);
-        }
-    }
-    
     @Override
     public StoragePoolVO updateStoragePool(long poolId, String tags) throws IllegalArgumentException {
     	return _storageMgr.updateStoragePool(poolId, tags);

server/src/com/cloud/vm/UserVmManager.java

@@ -22,6 +22,7 @@ import java.util.List;
 
 import com.cloud.agent.api.VmStatsEntry;
 import com.cloud.api.ServerApiException;
+import com.cloud.api.commands.UpdateVMCmd;
 import com.cloud.api.commands.UpgradeVMCmd;
 import com.cloud.async.executor.DestroyVMExecutor;
 import com.cloud.async.executor.OperationResponse;
@@ -222,4 +223,6 @@ public interface UserVmManager extends Manager, VirtualMachineManager<UserVmVO>
      */
     void releaseGuestIpAddress(UserVmVO userVm);
 
+	void updateVirtualMachine(UpdateVMCmd cmd);
+
 }

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

@@ -66,6 +66,7 @@ import com.cloud.agent.api.storage.CreatePrivateTemplateCommand;
 import com.cloud.alert.AlertManager;
 import com.cloud.api.BaseCmd;
 import com.cloud.api.ServerApiException;
+import com.cloud.api.commands.UpdateVMCmd;
 import com.cloud.api.commands.UpgradeVMCmd;
 import com.cloud.async.AsyncJobExecutor;
 import com.cloud.async.AsyncJobManager;
@@ -1110,22 +1111,7 @@ public class UserVmManagerImpl implements UserVmManager {
         	throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + virtualMachineId);
         }       
 
-        if (account != null) 
+        userId = accountAndUserValidation(virtualMachineId, account, userId,vmInstance);                         
-        {
-            if (!isAdmin(account.getType()) && (account.getId().longValue() != vmInstance.getAccountId())) 
-            {
-                throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + virtualMachineId + " for this account");
-            } 
-            else if (_domainDao.isChildDomain(account.getDomainId(),vmInstance.getDomainId())) 
-            {
-                throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid virtual machine id (" + virtualMachineId + ") given, unable to upgrade virtual machine.");
-            }
-        }
-
-        // If command is executed via 8096 port, set userId to the id of System account (1)
-        if (userId == null) {
-            userId = Long.valueOf(User.UID_SYSTEM);
-        }                         
             
         // Check that the specified service offering ID is valid
         ServiceOfferingVO newServiceOffering = _offeringDao.findById(serviceOfferingId);
@@ -1185,6 +1171,27 @@ public class UserVmManagerImpl implements UserVmManager {
             
     }
 
+	private Long accountAndUserValidation(Long virtualMachineId,Account account, Long userId, UserVmVO vmInstance) throws ServerApiException
+	{
+		if (account != null) 
+        {
+            if (!isAdmin(account.getType()) && (account.getId().longValue() != vmInstance.getAccountId())) 
+            {
+                throw new ServerApiException(BaseCmd.VM_INVALID_PARAM_ERROR, "unable to find a virtual machine with id " + virtualMachineId + " for this account");
+            } 
+            else if (_domainDao.isChildDomain(account.getDomainId(),vmInstance.getDomainId())) 
+            {
+                throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid virtual machine id (" + virtualMachineId + ") given, unable to upgrade virtual machine.");
+            }
+        }
+
+        // If command is executed via 8096 port, set userId to the id of System account (1)
+        if (userId == null) {
+            userId = Long.valueOf(User.UID_SYSTEM);
+        }
+		return userId;
+	}
+
     @Override
     public HashMap<Long, VmStatsEntry> getVirtualMachineStatistics(long hostId, String hostName, List<Long> vmIds) throws InternalErrorException {
     	HashMap<Long, VmStatsEntry> vmStatsById = new HashMap<Long, VmStatsEntry>();
@@ -3044,4 +3051,74 @@ public class UserVmManagerImpl implements UserVmManager {
 	            (accountType == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) ||
 	            (accountType == Account.ACCOUNT_TYPE_READ_ONLY_ADMIN));
 	}
+	
+    @Override
+    public void updateVirtualMachine(UpdateVMCmd cmd) 
+    {
+
+        String displayName = cmd.getDisplayName();
+        String group = cmd.getGroup();
+        Boolean ha = cmd.getHaEnable();
+        Long id = cmd.getId();
+        Account account = (Account)UserContext.current().getAccountObject();
+        Long userId = UserContext.current().getUserId();
+    
+        //Input validation
+        UserVmVO vmInstance = null;
+        
+        // Verify input parameters
+        try 
+        {
+        	vmInstance = _userVmDao.findById(id.longValue());
+        } 
+        catch (Exception ex1) 
+        {
+        	throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find virtual machine by id");
+        }
+
+        if (vmInstance == null) {
+            throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to find virtual machine with id " + id);
+        }
+
+        userId = accountAndUserValidation(id, account, userId,vmInstance);  
+        
+        if (group == null) {
+    		group = vmInstance.getGroup();
+    	}
+
+    	if (displayName == null) {
+    		displayName = vmInstance.getDisplayName();
+    	}
+    	
+    	if (ha == null) {
+    		ha = vmInstance.isHaEnabled();
+    	}
+
+    	long accountId = vmInstance.getAccountId();
+
+        
+        UserVmVO vm = _userVmDao.findById(id);
+        if (vm == null) {
+            throw new CloudRuntimeException("Unable to find virual machine with id " + id);
+        }
+
+        boolean haEnabled = vm.isHaEnabled();
+        _userVmDao.updateVM(id, displayName, group, ha);
+        if (haEnabled != ha) {
+            String description = null;
+            String type = null;
+            if (ha) 
+            {
+                description = "Successfully enabled HA for virtual machine " + vm.getName();
+                type = EventTypes.EVENT_VM_ENABLE_HA;
+            } 
+            else 
+            {
+                description = "Successfully disabled HA for virtual machine " + vm.getName();
+                type = EventTypes.EVENT_VM_DISABLE_HA;
+            }
+            // create a event for the change in HA Enabled flag
+            EventUtils.saveEvent(userId, accountId, EventVO.LEVEL_INFO, type, description, null);
+        }
+    }
 }