From 3d8a3ef358be1d0fad85b9606b35cd21b9ff8e2d Mon Sep 17 00:00:00 2001 From: abhishek Date: Thu, 26 Aug 2010 17:17:30 -0700 Subject: [PATCH] Refactoring the snapshot policies cmd --- .../commands/DeleteSnapshotPoliciesCmd.java | 162 ++++++++---------- .../com/cloud/server/ManagementServer.java | 2 +- .../cloud/server/ManagementServerImpl.java | 28 +-- .../storage/snapshot/SnapshotManager.java | 2 + .../storage/snapshot/SnapshotManagerImpl.java | 80 +++++++-- 5 files changed, 158 insertions(+), 116 deletions(-) diff --git a/server/src/com/cloud/api/commands/DeleteSnapshotPoliciesCmd.java b/server/src/com/cloud/api/commands/DeleteSnapshotPoliciesCmd.java index 5bcbd1fb0f8..6f26af78352 100644 --- a/server/src/com/cloud/api/commands/DeleteSnapshotPoliciesCmd.java +++ b/server/src/com/cloud/api/commands/DeleteSnapshotPoliciesCmd.java @@ -18,38 +18,21 @@ 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.exception.InvalidParameterValueException; -import com.cloud.server.ManagementServer; -import com.cloud.storage.SnapshotPolicyVO; -import com.cloud.storage.VolumeVO; -import com.cloud.utils.Pair; - +import com.cloud.api.BaseCmd.Manager; + +@Implementation(method="deleteSnapshotPolicies", manager=Manager.SnapshotManager) public class DeleteSnapshotPoliciesCmd extends BaseCmd { public static final Logger s_logger = Logger.getLogger(DeleteSnapshotPoliciesCmd.class.getName()); - private static final String s_name = "deletesnapshotpoliciesresponse"; - private static final List> s_properties = new ArrayList>(); - - static { - s_properties.add(new Pair(BaseCmd.Properties.ACCOUNT, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.DOMAIN_ID, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.ID, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.IDS, Boolean.FALSE)); - - s_properties.add(new Pair(BaseCmd.Properties.ACCOUNT_OBJ, Boolean.FALSE)); - s_properties.add(new Pair(BaseCmd.Properties.USER_ID, Boolean.FALSE)); - } - + private static final String s_name = "deletesnapshotpoliciesresponse"; + ///////////////////////////////////////////////////// //////////////// API parameters ///////////////////// ///////////////////////////////////////////////////// @@ -94,70 +77,73 @@ public class DeleteSnapshotPoliciesCmd extends BaseCmd { public String getName() { return s_name; - } - public List> getProperties() { - return s_properties; - } + } + + @Override + public String getResponse() { + // TODO Auto-generated method stub + return null; + } - @Override - public List> execute(Map params) { - Long policyId = (Long)params.get(BaseCmd.Properties.ID.getName()); - String policyIds = (String)params.get(BaseCmd.Properties.IDS.getName()); - Long userId = (Long)params.get(BaseCmd.Properties.USER_ID.getName()); - - // If command is executed via 8096 port, set userId to the id of System account (1) - if (userId == null) { - userId = Long.valueOf(1); - } - - if ((policyId == null) && (policyIds == null)) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "No policy id (or list off ids) specified."); - } - - List policyIdList = new ArrayList(); - - if (policyId != null) { - policyIdList.add(policyId); - } else if (policyIds != null) { - StringTokenizer st = new StringTokenizer(policyIds, ","); - while (st.hasMoreTokens()) { - String token = st.nextToken(); - try { - Long nextId = Long.parseLong(token); - policyIdList.add(nextId); - } catch (NumberFormatException nfe) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "The policy id " + token + " is not a valid parameter."); - } - } - } - - ManagementServer managementServer = getManagementServer(); - for (Long policy : policyIdList) { - SnapshotPolicyVO snapshotPolicyVO = managementServer.findSnapshotPolicyById(policy); - if (snapshotPolicyVO == null) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Policy id given: " + policy + " does not exist"); - } - VolumeVO volume = managementServer.findVolumeById(snapshotPolicyVO.getVolumeId()); - if (volume == null) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Policy id given: " + policy + " does not belong to a valid volume"); - } - - // If an account was passed in, make sure that it matches the account of the volume - checkAccountPermissions(params, volume.getAccountId(), volume.getDomainId(), "volume", volume.getId()); - } - - try { - boolean success = true; - if (policyIdList.size() > 0) { - success = managementServer.deleteSnapshotPolicies(userId, policyIdList); - } - - List> returnValues = new ArrayList>(); - returnValues.add(new Pair(BaseCmd.Properties.SUCCESS.getName(), Boolean.valueOf(success).toString())); - return returnValues; - - } catch (InvalidParameterValueException ex) { - throw new ServerApiException(BaseCmd.PARAM_ERROR, "Error deleting snapshot policy: " + ex.getMessage()); - } - } +// @Override +// public List> execute(Map params) { +// Long policyId = (Long)params.get(BaseCmd.Properties.ID.getName()); +// String policyIds = (String)params.get(BaseCmd.Properties.IDS.getName()); +// Long userId = (Long)params.get(BaseCmd.Properties.USER_ID.getName()); +// +// // If command is executed via 8096 port, set userId to the id of System account (1) +// if (userId == null) { +// userId = Long.valueOf(1); +// } +// +// if ((policyId == null) && (policyIds == null)) { +// throw new ServerApiException(BaseCmd.PARAM_ERROR, "No policy id (or list off ids) specified."); +// } +// +// List policyIdList = new ArrayList(); +// +// if (policyId != null) { +// policyIdList.add(policyId); +// } else if (policyIds != null) { +// StringTokenizer st = new StringTokenizer(policyIds, ","); +// while (st.hasMoreTokens()) { +// String token = st.nextToken(); +// try { +// Long nextId = Long.parseLong(token); +// policyIdList.add(nextId); +// } catch (NumberFormatException nfe) { +// throw new ServerApiException(BaseCmd.PARAM_ERROR, "The policy id " + token + " is not a valid parameter."); +// } +// } +// } +// +// ManagementServer managementServer = getManagementServer(); +// for (Long policy : policyIdList) { +// SnapshotPolicyVO snapshotPolicyVO = managementServer.findSnapshotPolicyById(policy); +// if (snapshotPolicyVO == null) { +// throw new ServerApiException(BaseCmd.PARAM_ERROR, "Policy id given: " + policy + " does not exist"); +// } +// VolumeVO volume = managementServer.findVolumeById(snapshotPolicyVO.getVolumeId()); +// if (volume == null) { +// throw new ServerApiException(BaseCmd.PARAM_ERROR, "Policy id given: " + policy + " does not belong to a valid volume"); +// } +// +// // If an account was passed in, make sure that it matches the account of the volume +// checkAccountPermissions(params, volume.getAccountId(), volume.getDomainId(), "volume", volume.getId()); +// } +// +// try { +// boolean success = true; +// if (policyIdList.size() > 0) { +// success = managementServer.deleteSnapshotPolicies(userId, policyIdList); +// } +// +// List> returnValues = new ArrayList>(); +// returnValues.add(new Pair(BaseCmd.Properties.SUCCESS.getName(), Boolean.valueOf(success).toString())); +// return returnValues; +// +// } catch (InvalidParameterValueException ex) { +// throw new ServerApiException(BaseCmd.PARAM_ERROR, "Error deleting snapshot policy: " + ex.getMessage()); +// } +// } } diff --git a/server/src/com/cloud/server/ManagementServer.java b/server/src/com/cloud/server/ManagementServer.java index 3aac64179a0..aad942e950f 100644 --- a/server/src/com/cloud/server/ManagementServer.java +++ b/server/src/com/cloud/server/ManagementServer.java @@ -1810,7 +1810,7 @@ public interface ManagementServer { /** * Deletes snapshot scheduling policies */ - boolean deleteSnapshotPolicies(long userId, List policyIds) throws InvalidParameterValueException; +// boolean deleteSnapshotPolicies(long userId, List policyIds) throws InvalidParameterValueException; /** * Get the recurring snapshots scheduled for this volume currently along with the time at which they are scheduled diff --git a/server/src/com/cloud/server/ManagementServerImpl.java b/server/src/com/cloud/server/ManagementServerImpl.java index b23e66f29a9..2af461fe526 100755 --- a/server/src/com/cloud/server/ManagementServerImpl.java +++ b/server/src/com/cloud/server/ManagementServerImpl.java @@ -7691,20 +7691,20 @@ public class ManagementServerImpl implements ManagementServer { return _snapshotPolicyDao.findById(policyId); } - @Override - public boolean deleteSnapshotPolicies(long userId, List policyIds) throws InvalidParameterValueException { - boolean result = true; - if (policyIds.contains(Snapshot.MANUAL_POLICY_ID)) { - throw new InvalidParameterValueException("Invalid Policy id given: " + Snapshot.MANUAL_POLICY_ID); - } - for (long policyId : policyIds) { - if (!_snapMgr.deletePolicy(userId, policyId)) { - result = false; - s_logger.warn("Failed to delete snapshot policy with Id: " + policyId); - } - } - return result; - } +// @Override +// public boolean deleteSnapshotPolicies(long userId, List policyIds) throws InvalidParameterValueException { +// boolean result = true; +// if (policyIds.contains(Snapshot.MANUAL_POLICY_ID)) { +// throw new InvalidParameterValueException("Invalid Policy id given: " + Snapshot.MANUAL_POLICY_ID); +// } +// for (long policyId : policyIds) { +// if (!_snapMgr.deletePolicy(userId, policyId)) { +// result = false; +// s_logger.warn("Failed to delete snapshot policy with Id: " + policyId); +// } +// } +// return result; +// } @Override public String getSnapshotIntervalTypes(long snapshotId){ diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManager.java b/server/src/com/cloud/storage/snapshot/SnapshotManager.java index 39efdd14b15..ed356103ed1 100644 --- a/server/src/com/cloud/storage/snapshot/SnapshotManager.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManager.java @@ -21,6 +21,7 @@ import java.util.List; import com.cloud.api.commands.CreateSnapshotPolicyCmd; import com.cloud.api.commands.DeleteSnapshotCmd; +import com.cloud.api.commands.DeleteSnapshotPoliciesCmd; import com.cloud.exception.InternalErrorException; import com.cloud.exception.InvalidParameterValueException; import com.cloud.exception.ResourceAllocationException; @@ -154,4 +155,5 @@ public interface SnapshotManager extends Manager { void validateSnapshot(Long userId, SnapshotVO snapshot); + boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd) throws InvalidParameterValueException; } diff --git a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java index 49fdb938fcd..6aaf939bf1d 100644 --- a/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java +++ b/server/src/com/cloud/storage/snapshot/SnapshotManagerImpl.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Date; import java.util.List; import java.util.Map; +import java.util.StringTokenizer; import java.util.TimeZone; import javax.ejb.Local; @@ -45,6 +46,7 @@ import com.cloud.api.commands.CreateSnapshotCmd; import com.cloud.api.commands.CreateSnapshotPolicyCmd; import com.cloud.api.commands.CreateVolumeCmd; import com.cloud.api.commands.DeleteSnapshotCmd; +import com.cloud.api.commands.DeleteSnapshotPoliciesCmd; import com.cloud.async.AsyncJobExecutor; import com.cloud.async.AsyncJobManager; import com.cloud.async.AsyncJobVO; @@ -53,6 +55,7 @@ import com.cloud.async.executor.SnapshotOperationParam; import com.cloud.configuration.ResourceCount.ResourceType; import com.cloud.configuration.dao.ConfigurationDao; import com.cloud.dc.dao.DataCenterDao; +import com.cloud.domain.dao.DomainDao; import com.cloud.event.EventState; import com.cloud.event.EventTypes; import com.cloud.event.EventVO; @@ -63,6 +66,7 @@ import com.cloud.exception.ResourceAllocationException; import com.cloud.host.dao.DetailsDao; import com.cloud.host.dao.HostDao; import com.cloud.serializer.GsonHelper; +import com.cloud.server.ManagementServer; import com.cloud.storage.Snapshot; import com.cloud.storage.Snapshot.SnapshotType; import com.cloud.storage.Snapshot.Status; @@ -130,7 +134,7 @@ public class SnapshotManagerImpl implements SnapshotManager { @Inject protected VMTemplateDao _templateDao; @Inject protected VMTemplatePoolDao _templatePoolDao; @Inject protected VMTemplateHostDao _templateHostDao; - + @Inject protected DomainDao _domainDao; @Inject protected StorageManager _storageMgr; @Inject protected AgentManager _agentMgr; @Inject protected SnapshotScheduler _snapSchedMgr; @@ -775,16 +779,11 @@ public class SnapshotManagerImpl implements SnapshotManager { } - protected Long checkAccountPermissions(Map params, - long targetAccountId, - long targetDomainId, - String targetDesc, - long targetId) - throws ServerApiException + protected Long checkAccountPermissions(long targetAccountId,long targetDomainId,String targetDesc,long targetId) throws ServerApiException { Long accountId = null; - Account account = getAccount(params); + Account account = (Account)UserContext.current().getAccountObject(); if (account != null) { if (!isAdmin(account.getType())) @@ -794,7 +793,7 @@ public class SnapshotManagerImpl implements SnapshotManager { throw new ServerApiException(BaseCmd.PARAM_ERROR, "Unable to find a " + targetDesc + " with id " + targetId + " for this account"); } } - else if (!getManagementServer().isChildDomain(account.getDomainId(), targetDomainId)) + else if (!_domainDao.isChildDomain(account.getDomainId(), targetDomainId)) { throw new ServerApiException(BaseCmd.PARAM_ERROR, "Unable to perform operation for " + targetDesc + " with id " + targetId + ", permission denied."); } @@ -803,6 +802,12 @@ public class SnapshotManagerImpl implements SnapshotManager { return accountId; } + + private 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 deleteSnapshot(DeleteSnapshotCmd cmd) { @@ -822,7 +827,7 @@ public class SnapshotManagerImpl implements SnapshotManager { if (snapshotOwner == null) { throw new ServerApiException(BaseCmd.SNAPSHOT_INVALID_PARAM_ERROR, "Snapshot id " + snapshotId + " does not have a valid account"); } - checkAccountPermissions(params, snapshotOwner.getId(), snapshotOwner.getDomainId(), "snapshot", snapshotId); + checkAccountPermissions(snapshotOwner.getId(), snapshotOwner.getDomainId(), "snapshot", snapshotId); //If command is executed via 8096 port, set userId to the id of System account (1) if (userId == null) { @@ -1257,7 +1262,6 @@ public class SnapshotManagerImpl implements SnapshotManager { return policy; } - @Override public boolean deletePolicy(long userId, long policyId) { SnapshotPolicyVO snapshotPolicy = _snapshotPolicyDao.findById(policyId); @@ -1461,7 +1465,57 @@ public class SnapshotManagerImpl implements SnapshotManager { public boolean stop() { return true; } - - + + @Override + public boolean deleteSnapshotPolicies(DeleteSnapshotPoliciesCmd cmd) throws InvalidParameterValueException { + + Long policyId = (Long)cmd.getId(); + List policyIds = cmd.getIds(); + Long userId = UserContext.current().getUserId(); + + // If command is executed via 8096 port, set userId to the id of System account (1) + if (userId == null) { + userId = Long.valueOf(1); + } + + if ((policyId == null) && (policyIds == null)) { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "No policy id (or list off ids) specified."); + } + + if(policyIds.size()<=0){ + throw new ServerApiException(BaseCmd.INTERNAL_ERROR,"There are no policy ids"); + } + + + for (Long policy : policyIds) { + SnapshotPolicyVO snapshotPolicyVO = _snapshotPolicyDao.findById(policy); + if (snapshotPolicyVO == null) { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "Policy id given: " + policy + " does not exist"); + } + VolumeVO volume = _volsDao.findById(snapshotPolicyVO.getVolumeId()); + if (volume == null) { + throw new ServerApiException(BaseCmd.PARAM_ERROR, "Policy id given: " + policy + " does not belong to a valid volume"); + } + + // If an account was passed in, make sure that it matches the account of the volume + checkAccountPermissions(volume.getAccountId(), volume.getDomainId(), "volume", volume.getId()); + } + + boolean success = true; + + if (policyIds.contains(Snapshot.MANUAL_POLICY_ID)) { + throw new InvalidParameterValueException("Invalid Policy id given: " + Snapshot.MANUAL_POLICY_ID); + } + + for (long pId : policyIds) { + if (!deletePolicy(userId, pId)) { + success = false; + s_logger.warn("Failed to delete snapshot policy with Id: " + policyId); + return success; + } + } + + return success; + } }