Adding the updatetemplateoriso command refactoring,

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

@@ -1,36 +1,20 @@
 package com.cloud.api.commands;
 
-import java.util.ArrayList;
 import java.util.List;
-import java.util.Map;
-import java.util.StringTokenizer;
 
 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.exception.InvalidParameterValueException;
-import com.cloud.exception.PermissionDeniedException;
-import com.cloud.storage.VMTemplateVO;
-import com.cloud.user.Account;
-import com.cloud.utils.Pair;
 
+@Implementation(method="updateTemplatePermissions", manager=Manager.ManagementServer)
 public abstract class UpdateTemplateOrIsoPermissionsCmd extends BaseCmd {
 	public Logger s_logger = getLogger();
-    protected static final List<Pair<Enum, Boolean>> s_properties = new ArrayList<Pair<Enum, Boolean>>();
     protected String s_name = getResponseName();
 
-    static {
-        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));
 
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.ACCOUNT_NAMES, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.ID, Boolean.TRUE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.IS_FEATURED, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.IS_PUBLIC, Boolean.FALSE));
-        s_properties.add(new Pair<Enum, Boolean>(BaseCmd.Properties.OP, Boolean.FALSE));
-    }
     
     /////////////////////////////////////////////////////
     //////////////// API parameters /////////////////////
@@ -83,96 +67,18 @@ public abstract class UpdateTemplateOrIsoPermissionsCmd extends BaseCmd {
     public String getName() {
         return s_name;
     }    
-    @Override
-    public List<Pair<Enum, Boolean>> getProperties() {
-        return s_properties;
-    }
-    
-    protected boolean templateIsCorrectType(VMTemplateVO template) {
-    	return true;
-    }
     
     protected String getResponseName() {
     	return "updatetemplateorisopermissionsresponse";
     }
     
-    protected String getMediaType() {
-    	return "templateOrIso";
-    }
-    
     protected Logger getLogger() {
     	return Logger.getLogger(UpdateTemplateOrIsoPermissionsCmd.class.getName());    
     }
     
     @Override
-    public List<Pair<String, Object>> execute(Map<String, Object> params) {
+    public String getResponse() 
-        Long id = (Long)params.get(BaseCmd.Properties.ID.getName());
+    {
-        Account account = (Account)params.get(BaseCmd.Properties.ACCOUNT_OBJ.getName());
+    	return null;//return the response here
-        Boolean isPublic = (Boolean)params.get(BaseCmd.Properties.IS_PUBLIC.getName());
-        Boolean isFeatured = (Boolean)params.get(BaseCmd.Properties.IS_FEATURED.getName());
-        String accoutNames = (String)params.get(BaseCmd.Properties.ACCOUNT_NAMES.getName());
-        String operation = (String)params.get(BaseCmd.Properties.OP.getName());
-
-        Boolean publishTemplateResult = Boolean.FALSE;
-
-        VMTemplateVO template = getManagementServer().findTemplateById(id.longValue());
-        if (template == null || !templateIsCorrectType(template)) {
-            throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find " + getMediaType() + " with id " + id);
-        }
-
-        if (account != null) {
-            if (!isAdmin(account.getType()) && (template.getAccountId() != account.getId())) {
-                throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id);
-            } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) {
-                Long templateOwnerDomainId = getManagementServer().findDomainIdByAccountId(template.getAccountId());
-                if (!getManagementServer().isChildDomain(account.getDomainId(), templateOwnerDomainId)) {
-                    throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to update permissions for " + getMediaType() + " with id " + id);
-                }
-            }
-        }
-
-        // If the template is removed throw an error.
-        if (template.getRemoved() != null){
-        	s_logger.error("unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed  ");
-        	throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed ");
-        }
-        
-        if (id == Long.valueOf(1)) {
-            throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id);
-        }
-        
-        boolean isAdmin = ((account == null) || isAdmin(account.getType()));
-        boolean allowPublicUserTemplates = Boolean.parseBoolean(getManagementServer().getConfigurationValue("allow.public.user.templates"));        
-        if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) {
-        	throw new ServerApiException(BaseCmd.PARAM_ERROR, "Only private " + getMediaType() + "s can be created.");
-        }
-
-        // package up the accountNames as a list
-        List<String> accountNameList = new ArrayList<String>();
-        if (accoutNames != null) {
-            if ((operation == null) || (!operation.equalsIgnoreCase("add") && !operation.equalsIgnoreCase("remove") && !operation.equalsIgnoreCase("reset"))) {
-                throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid operation on accounts, the operation must be either 'add' or 'remove' in order to modify launch permissions." +
-                        "  Given operation is: '" + operation + "'");
-            }
-            StringTokenizer st = new StringTokenizer(accoutNames, ",");
-            while (st.hasMoreTokens()) {
-                accountNameList.add(st.nextToken());
-            }
-        }
-
-        try {
-            publishTemplateResult = getManagementServer().updateTemplatePermissions(id, operation, isPublic, isFeatured, accountNameList);
-        } catch (InvalidParameterValueException ex) {
-            throw new ServerApiException(BaseCmd.PARAM_ERROR, "Failed to update " + getMediaType() + " permissions for template " + template.getName() + ":  internal error.");
-        } catch (PermissionDeniedException ex) {
-            throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Failed to update " + getMediaType() + " permissions for template " + template.getName() + ":  internal error.");
-        } catch (Exception ex) {
-             s_logger.error("Exception editing template", ex);
-             throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "Failed to update " + getMediaType() + " permissions for template " + template.getName() + ":  internal error.");
-        }
-
-        List<Pair<String, Object>> returnValues = new ArrayList<Pair<String, Object>>();
-        returnValues.add(new Pair<String, Object>(BaseCmd.Properties.SUCCESS.getName(), publishTemplateResult.toString()));
-        return returnValues;
     }
 }

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

@@ -32,6 +32,7 @@ import com.cloud.api.commands.GetCloudIdentifierCmd;
 import com.cloud.api.commands.UpdateAccountCmd;
 import com.cloud.api.commands.UpdateDomainCmd;
 import com.cloud.api.commands.UpdateTemplateCmd;
+import com.cloud.api.commands.UpdateTemplateOrIsoPermissionsCmd;
 import com.cloud.api.commands.UpdateUserCmd;
 import com.cloud.async.AsyncJobResult;
 import com.cloud.async.AsyncJobVO;
@@ -1784,7 +1785,7 @@ public interface ManagementServer {
      * @throws PermissionDeniedException
      * @throws InternalErrorException
      */
-    boolean updateTemplatePermissions(long templateId, String operation, Boolean isPublic, Boolean isFeatured, List<String> accountNames) throws InvalidParameterValueException, PermissionDeniedException, InternalErrorException;
+//    boolean updateTemplatePermissions(long templateId, String operation, Boolean isPublic, Boolean isFeatured, List<String> accountNames) throws InvalidParameterValueException, PermissionDeniedException, InternalErrorException;
 
     /**
      * List the permissions on a template.  This will return a list of account names that have been granted permission to launch instances from the template.
@@ -2141,4 +2142,5 @@ public interface ManagementServer {
 	
 	boolean validateCustomVolumeSizeRange(long size) throws InvalidParameterValueException;
 	boolean updateUser(UpdateUserCmd cmd) throws InvalidParameterValueException;
+	boolean updateTemplatePermissions(UpdateTemplateOrIsoPermissionsCmd cmd)throws InvalidParameterValueException, PermissionDeniedException,InternalErrorException;
 }

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

@@ -38,6 +38,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.StringTokenizer;
 import java.util.TimeZone;
 import java.util.concurrent.Executors;
 import java.util.concurrent.ScheduledExecutorService;
@@ -84,6 +85,7 @@ import com.cloud.api.commands.StartVMCmd;
 import com.cloud.api.commands.UpdateAccountCmd;
 import com.cloud.api.commands.UpdateDomainCmd;
 import com.cloud.api.commands.UpdateTemplateCmd;
+import com.cloud.api.commands.UpdateTemplateOrIsoPermissionsCmd;
 import com.cloud.api.commands.UpdateUserCmd;
 import com.cloud.api.commands.UpgradeVMCmd;
 import com.cloud.async.AsyncInstanceCreateStatus;
@@ -6751,14 +6753,84 @@ public class ManagementServerImpl implements ManagementServer {
         return _diskOfferingDao.findPrivateDiskOffering();
     }
     
+    protected boolean templateIsCorrectType(VMTemplateVO template) {
+    	return true;
+    }
+    
+    protected String getMediaType() {
+    	return "templateOrIso";
+    }
+    
+	public static boolean isAdmin(short accountType) {
+	    return ((accountType == Account.ACCOUNT_TYPE_ADMIN) ||
+	            (accountType == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) ||
+	            (accountType == Account.ACCOUNT_TYPE_READ_ONLY_ADMIN));
+	}
+
     @Override
     @DB
-    public boolean updateTemplatePermissions(long templateId, String operation, Boolean isPublic, Boolean isFeatured, List<String> accountNames) throws InvalidParameterValueException,
+    public boolean updateTemplatePermissions(UpdateTemplateOrIsoPermissionsCmd cmd) throws InvalidParameterValueException,
             PermissionDeniedException, InternalErrorException {
         Transaction txn = Transaction.currentTxn();
-        VMTemplateVO template = _templateDao.findById(templateId);
+        
-        if (template == null) {
+        //Input validation
-            throw new InvalidParameterValueException("Unable to find template with id " + templateId);
+        Long id = cmd.getId();
+        Account account = (Account) UserContext.current().getAccountObject();
+        List<String> accountNames = cmd.getAccountNames();
+        Long userId = UserContext.current().getUserId();
+        Boolean isFeatured = cmd.isFeatured();
+        Boolean isPublic = cmd.isPublic();
+        String operation = cmd.getOperation();
+        
+        Boolean publishTemplateResult = Boolean.FALSE;
+
+        VMTemplateVO template = _templateDao.findById(id);
+        
+        if (template == null || !templateIsCorrectType(template)) {
+            throw new ServerApiException(BaseCmd.INTERNAL_ERROR, "unable to find " + getMediaType() + " with id " + id);
+        }
+
+        if (account != null) 
+        {
+            if (!isAdmin(account.getType()) && (template.getAccountId() != account.getId())) {
+                throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id);
+            } else if (account.getType() != Account.ACCOUNT_TYPE_ADMIN) {
+                Long templateOwnerDomainId = findDomainIdByAccountId(template.getAccountId());
+                if (!isChildDomain(account.getDomainId(), templateOwnerDomainId)) {
+                    throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "Unable to update permissions for " + getMediaType() + " with id " + id);
+                }
+            }
+        }
+
+        // If the template is removed throw an error.
+        if (template.getRemoved() != null){
+        	s_logger.error("unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed  ");
+        	throw new ServerApiException(BaseCmd.ACCOUNT_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id + " as it is removed ");
+        }
+        
+        if (id == Long.valueOf(1)) {
+            throw new ServerApiException(BaseCmd.PARAM_ERROR, "unable to update permissions for " + getMediaType() + " with id " + id);
+        }
+        
+        boolean isAdmin = ((account == null) || isAdmin(account.getType()));
+        boolean allowPublicUserTemplates = Boolean.parseBoolean(getConfigurationValue("allow.public.user.templates"));        
+        if (!isAdmin && !allowPublicUserTemplates && isPublic != null && isPublic) {
+        	throw new ServerApiException(BaseCmd.PARAM_ERROR, "Only private " + getMediaType() + "s can be created.");
+        }
+
+//        // package up the accountNames as a list
+//        List<String> accountNameList = new ArrayList<String>();
+        if (accountNames != null) 
+        {
+            if ((operation == null) || (!operation.equalsIgnoreCase("add") && !operation.equalsIgnoreCase("remove") && !operation.equalsIgnoreCase("reset"))) 
+            {
+                throw new ServerApiException(BaseCmd.PARAM_ERROR, "Invalid operation on accounts, the operation must be either 'add' or 'remove' in order to modify launch permissions." +
+                        "  Given operation is: '" + operation + "'");
+            }
+//            StringTokenizer st = new StringTokenizer(accountNames, ",");
+//            while (st.hasMoreTokens()) {
+//                accountNameList.add(st.nextToken());
+//            }
         }
 
         Long accountId = template.getAccountId();
@@ -6767,11 +6839,6 @@ public class ManagementServerImpl implements ManagementServer {
             throw new InvalidParameterValueException("Update template permissions is an invalid operation on template " + template.getName());
         }
 
-        Account account = _accountDao.findById(accountId);
-        if (account == null) {
-            throw new PermissionDeniedException("Unable to verify owner of template " + template.getName());
-        }
-
         VMTemplateVO updatedTemplate = _templateDao.createForUpdate();
         
         if (isPublic != null) {
@@ -6793,9 +6860,9 @@ public class ManagementServerImpl implements ManagementServer {
                     if (permittedAccount.getId().longValue() == account.getId().longValue()) {
                         continue; // don't grant permission to the template owner, they implicitly have permission
                     }
-                    LaunchPermissionVO existingPermission = _launchPermissionDao.findByTemplateAndAccount(templateId, permittedAccount.getId().longValue());
+                    LaunchPermissionVO existingPermission = _launchPermissionDao.findByTemplateAndAccount(id, permittedAccount.getId().longValue());
                     if (existingPermission == null) {
-                        LaunchPermissionVO launchPermission = new LaunchPermissionVO(templateId, permittedAccount.getId().longValue());
+                        LaunchPermissionVO launchPermission = new LaunchPermissionVO(id, permittedAccount.getId().longValue());
                         _launchPermissionDao.persist(launchPermission);
                     }
                 } else {
@@ -6814,7 +6881,7 @@ public class ManagementServerImpl implements ManagementServer {
                         accountIds.add(permittedAccount.getId());
                     }
                 }
-                _launchPermissionDao.removePermissions(templateId, accountIds);
+                _launchPermissionDao.removePermissions(id, accountIds);
             } catch (CloudRuntimeException ex) {
                 throw new InternalErrorException("Internal error removing launch permissions for template " + template.getName());
             }
@@ -6825,7 +6892,7 @@ public class ManagementServerImpl implements ManagementServer {
             updatedTemplate.setPublicTemplate(false);
             updatedTemplate.setFeatured(false);
             _templateDao.update(template.getId(), updatedTemplate);
-            _launchPermissionDao.removeAllPermissions(templateId);
+            _launchPermissionDao.removeAllPermissions(id);
         }
         return true;
     }