Refactoring the updateZone command

build/.project

@@ -0,0 +1,11 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<projectDescription>
+	<name>build</name>
+	<comment></comment>
+	<projects>
+	</projects>
+	<buildSpec>
+	</buildSpec>
+	<natures>
+	</natures>
+</projectDescription>

server/src/com/cloud/api/commands/.gitignore

@@ -0,0 +1,3 @@
+/AddConfigCmd.java
+/UpdateZoneCmd.java
+/UpdateZoneCmd.java

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

@@ -18,37 +18,22 @@
 
 package com.cloud.api.commands;
 
-import java.util.ArrayList;
-import java.util.List;
-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.dc.DataCenterVO;
-import com.cloud.user.User;
+import com.cloud.serializer.Param;
-import com.cloud.utils.Pair;
+import com.cloud.serializer.SerializerHelper;
 
+@Implementation(method="updateZone", manager=Manager.ConfigManager)
 public class UpdateZoneCmd extends BaseCmd {
     public static final Logger s_logger = Logger.getLogger(UpdateZoneCmd.class.getName());
 
     private static final String s_name = "updatezoneresponse";
-    private static final List<Pair<Enum, Boolean>> s_properties = new ArrayList<Pair<Enum, Boolean>>();
-
-    static {
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.DNS1, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.DNS2, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.GUEST_CIDR_ADDRESS, Boolean.FALSE));
-    	s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.ID, Boolean.TRUE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.INTERNAL_DNS1, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.INTERNAL_DNS2, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.NAME, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.VNET, Boolean.FALSE));
-
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.USER_ID, Boolean.FALSE));
-    }
 
     /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
@@ -118,59 +103,154 @@ public class UpdateZoneCmd extends BaseCmd {
     /////////////// API Implementation///////////////////
     /////////////////////////////////////////////////////
     
+
+    private DataCenterVO responseObject = null;
+
+
     @Override
     public String getName() {
         return s_name;
     }
+
     @Override
-    public List<Pair<Enum, Boolean>> getProperties() {
+    public String getResponse() 
-        return s_properties;
+    {
+        UpdateZoneResponse response = new UpdateZoneResponse();
+        if (responseObject != null) {
+            response.setStatus("true");
+            response.setDisplayText("Successfully updated zone");
+            response.setId(responseObject.getId());
+            response.setGuestCidrAddress(responseObject.getGuestNetworkCidr());
+            response.setDns1(responseObject.getDns1());
+            response.setDns2(responseObject.getDns2());
+            response.setInternalDns1(responseObject.getInternalDns1());
+            response.setInternalDns2(responseObject.getInternalDns2());
+            response.setZoneName(responseObject.getName());
+            response.setVnet(responseObject.getVnet());
+        } 
+        else 
+        {
+        	throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update zone; internal error.");
+        }
+        
+        return SerializerHelper.toSerializedString(responseObject);
     }
     
-    @Override
+    public void setResponseObject(DataCenterVO zone) {
-    public List<Pair<String, Object>> execute(Map<String, Object> params) {
+        responseObject = zone;
-    	Long zoneId = (Long) params.get(BaseCmd.Properties.ID.getName());
+    }
-    	String zoneName = (String) params.get(BaseCmd.Properties.NAME.getName());
+    // helper class for the response object
-    	String dns1 = (String) params.get(BaseCmd.Properties.DNS1.getName());
+    private class UpdateZoneResponse 
-    	String dns2 = (String) params.get(BaseCmd.Properties.DNS2.getName());
+    {
-    	String dns3 = (String) params.get(BaseCmd.Properties.INTERNAL_DNS1.getName());
+        @Param(name="status")
-    	String dns4 = (String) params.get(BaseCmd.Properties.INTERNAL_DNS2.getName());
+        private String status;
-    	String vnet = (String) params.get(BaseCmd.Properties.VNET.getName());
-    	String guestCidr = (String) params.get(BaseCmd.Properties.GUEST_CIDR_ADDRESS.getName());
-    	Long userId = (Long)params.get(BaseCmd.Properties.USER_ID.getName());
 
-    	if (userId == null) {
+        @Param(name="displaytext")
-            userId = Long.valueOf(User.UID_SYSTEM);
+        private String displayText;
+
+		@Param(name="dns1")
+        private String dns1;
+        
+        @Param(name="dns2")
+        private String dns2;
+
+		@Param(name="guestcidraddress")
+        private String guestCidrAddress;
+        
+        @Param(name="id")
+        private Long id;
+
+        @Param(name="internaldns1")
+        private String internalDns1;
+        
+        @Param(name="internaldns2")
+        private String internalDns2;
+
+        @Param(name="zonename")
+        private String zoneName;
+        
+        @Param(name="vnet")
+        private String vnet;
+        
+        public String getGuestCidrAddress() {
+			return guestCidrAddress;
+		}
+
+		public void setGuestCidrAddress(String guestCidrAddress) {
+			this.guestCidrAddress = guestCidrAddress;
+		}
+
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(Long id) {
+			this.id = id;
+		}
+
+		public String getInternalDns1() {
+			return internalDns1;
+		}
+
+		public void setInternalDns1(String internalDns1) {
+			this.internalDns1 = internalDns1;
+		}
+
+		public String getInternalDns2() {
+			return internalDns2;
+		}
+
+		public void setInternalDns2(String internalDns2) {
+			this.internalDns2 = internalDns2;
+		}
+
+		public String getZoneName() {
+			return zoneName;
+		}
+
+		public void setZoneName(String zoneName) {
+			this.zoneName = zoneName;
+		}
+
+		public String getVnet() {
+			return vnet;
+		}
+
+		public void setVnet(String vnet) {
+			this.vnet = vnet;
+		}
+        
+        public String getDns1() {
+			return dns1;
+		}
+
+		public void setDns1(String dns1) {
+			this.dns1 = dns1;
+		}
+
+		public String getDns2() {
+			return dns2;
+		}
+
+		public void setDns2(String dns2) {
+			this.dns2 = dns2;
+		}
+        
+        public String getDisplayText() {
+            return displayText;
         }
 
-    	//verify input parameters
+        public void setDisplayText(String displayText) {
-    	DataCenterVO zone = getManagementServer().findDataCenterById(zoneId);
+            this.displayText = displayText;
-    	if (zone == null) {
-    		throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to find zone by id " + zoneId);
-    	}
-    	
-    	if (zoneName == null) {
-    		zoneName = zone.getName();
-    	}
-
-    	DataCenterVO updatedZone = null;
-    	
-        try {
-             updatedZone = getManagementServer().editZone(userId, zoneId, zoneName, dns1, dns2, dns3, dns4, vnet,guestCidr);
-        } catch (Exception ex) {
-            s_logger.error("Exception updating zone", ex);
-            throw new ServerApiException(BaseCmd.INTERNAL_ERROR, ex.getMessage());
         }
         
-        List<Pair<String, Object>> returnValues = new ArrayList<Pair<String, Object>>();
+        public String getStatus() {
-        
+            return status;
-        if (updatedZone == null) {
+        }
-            throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update zone; internal error.");
+
-        } else {
+        public void setStatus(String status) {
-            returnValues.add(new Pair<String, Object>(BaseCmd.Properties.SUCCESS.getName(), "true"));
+            this.status = status;
-            returnValues.add(new Pair<String, Object>(BaseCmd.Properties.DISPLAY_TEXT.getName(), "Successfully updated zone."));
         }
                 
-        return returnValues;
     }
 }

server/src/com/cloud/configuration/ConfigurationManager.java

@@ -20,6 +20,7 @@ package com.cloud.configuration;
 import java.util.List;
 
 import com.cloud.api.commands.CreateDiskOfferingCmd;
+import com.cloud.api.commands.UpdateZoneCmd;
 import com.cloud.dc.DataCenterVO;
 import com.cloud.dc.HostPodVO;
 import com.cloud.dc.Vlan.VlanType;
@@ -150,18 +151,10 @@ public interface ConfigurationManager extends Manager {
 	
 	/**
      * Edits a zone in the database. Will not allow you to edit DNS values if there are VMs in the specified zone.
-     * @param userId
+     * @param UpdateZoneCmd
-     * @param zoneId
+     * @return Updated zone
-     * @param newZoneName
-     * @param dns1
-     * @param dns2
-     * @param dns3
-     * @param dns4
-     * @param vnetRange
-     * @return Zone
-     * @return guestCidr
      */
-	DataCenterVO editZone(long userId, long zoneId, String newZoneName, String dns1, String dns2, String dns3, String dns4, String vnetRange, String guestCidr) throws InvalidParameterValueException, InternalErrorException;
+	DataCenterVO editZone(UpdateZoneCmd cmd) throws InvalidParameterValueException, InternalErrorException;
 	
 	/**
      * Deletes a zone from the database. Will not allow you to delete zones that are being used anywhere in the system.

server/src/com/cloud/configuration/ConfigurationManagerImpl.java

@@ -31,7 +31,10 @@ import javax.naming.ConfigurationException;
 
 import org.apache.log4j.Logger;
 
+import com.cloud.api.BaseCmd;
+import com.cloud.api.ServerApiException;
 import com.cloud.api.commands.CreateDiskOfferingCmd;
+import com.cloud.api.commands.UpdateZoneCmd;
 import com.cloud.configuration.dao.ConfigurationDao;
 import com.cloud.dc.AccountVlanMapVO;
 import com.cloud.dc.DataCenterVO;
@@ -59,6 +62,8 @@ import com.cloud.service.dao.ServiceOfferingDao;
 import com.cloud.storage.DiskOfferingVO;
 import com.cloud.storage.dao.DiskOfferingDao;
 import com.cloud.user.AccountVO;
+import com.cloud.user.User;
+import com.cloud.user.UserContext;
 import com.cloud.user.UserVO;
 import com.cloud.user.dao.AccountDao;
 import com.cloud.user.dao.UserDao;
@@ -667,7 +672,32 @@ public class ConfigurationManagerImpl implements ConfigurationManager {
     }
     
     @Override
-    public DataCenterVO editZone(long userId, long zoneId, String newZoneName, String dns1, String dns2, String internalDns1, String internalDns2, String vnetRange, String guestCidr) throws InvalidParameterValueException, InternalErrorException {
+    public DataCenterVO editZone(UpdateZoneCmd cmd) throws InvalidParameterValueException, InternalErrorException 
+    {
+    	//Parameter validation as from execute() method in V1
+    	Long zoneId = cmd.getId();
+    	String zoneName = cmd.getZoneName();
+    	String dns1 = cmd.getDns1();
+    	String dns2 = cmd.getDns2();
+    	String internalDns1 = cmd.getInternalDns1();
+    	String internalDns2 = cmd.getInternalDns2();
+    	String vnetRange = cmd.getVnet();
+    	String guestCidr = cmd.getGuestCidrAddress();
+    	Long userId = UserContext.current().getUserId();
+    
+    	if (userId == null) {
+            userId = Long.valueOf(User.UID_SYSTEM);
+        }
+
+    	DataCenterVO zone = _zoneDao.findById(zoneId);
+    	if (zone == null) {
+    		throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to find zone by id " + zoneId);
+    	}
+    	
+    	if (zoneName == null) {
+    		zoneName = zone.getName();
+    	}
+
     	// Make sure the zone exists
     	if (!validZone(zoneId)) {
     		throw new InvalidParameterValueException("A zone with ID: " + zoneId + " does not exist.");
@@ -703,12 +733,11 @@ public class ConfigurationManagerImpl implements ConfigurationManager {
     	}
     	
     	//5. Reached here, hence editable   	
-    	
+    	DataCenterVO zoneHandle = _zoneDao.findById(zoneId);
-    	DataCenterVO zone = _zoneDao.findById(zoneId);
     	String oldZoneName = zone.getName();
     	
-    	if (newZoneName == null) {
+    	if (zoneName == null) {
-    		newZoneName = oldZoneName;
+    		zoneName = oldZoneName;
     	}
     	
     	if (dns1 == null) {
@@ -723,10 +752,10 @@ public class ConfigurationManagerImpl implements ConfigurationManager {
     	if(guestCidr == null)
     		guestCidr = zone.getGuestNetworkCidr();    	
     	
-    	boolean checkForDuplicates = !newZoneName.equals(oldZoneName);
+    	boolean checkForDuplicates = !zoneName.equals(oldZoneName);
-    	checkZoneParameters(newZoneName, dns1, dns2, internalDns1, internalDns2, checkForDuplicates);
+    	checkZoneParameters(zoneName, dns1, dns2, internalDns1, internalDns2, checkForDuplicates);
 
-    	zone.setName(newZoneName);
+    	zone.setName(zoneName);
     	zone.setDns1(dns1);
     	zone.setDns2(dns2);
     	zone.setInternalDns1(internalDns1);