Dispatcher corrections, refactoring and tests Corrects problems from previous attempt. Fixes based on help comments from the community and conflict resolution
Signed-off-by: Daan Hoogland <d...@onecht.net> Project: http://git-wip-us.apache.org/repos/asf/cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/cloudstack/commit/c211f0bb Tree: http://git-wip-us.apache.org/repos/asf/cloudstack/tree/c211f0bb Diff: http://git-wip-us.apache.org/repos/asf/cloudstack/diff/c211f0bb Branch: refs/heads/master Commit: c211f0bbbe940ed3b9a6e9ef4b5a29be16062e85 Parents: 4552ec6 Author: Antonio Fornie <afor...@schubergphilis.com> Authored: Fri Mar 7 09:57:31 2014 -0600 Committer: Daan Hoogland <d...@onecht.net> Committed: Fri Mar 7 19:12:07 2014 +0100 ---------------------------------------------------------------------- .../org/apache/cloudstack/api/ApiConstants.java | 15 +- api/src/org/apache/cloudstack/api/BaseCmd.java | 185 ++++--- .../org/apache/cloudstack/api/BaseListCmd.java | 30 +- .../org/apache/cloudstack/api/BaseCmdTest.java | 69 +++ .../core/spring-server-core-misc-context.xml | 12 + server/src/com/cloud/api/ApiDispatcher.java | 490 +------------------ server/src/com/cloud/api/ApiServer.java | 374 +++++++------- server/src/com/cloud/api/ApiServlet.java | 133 ++--- .../api/dispatch/CommandCreationWorker.java | 56 +++ .../com/cloud/api/dispatch/DispatchChain.java | 40 ++ .../api/dispatch/DispatchChainFactory.java | 72 +++ .../com/cloud/api/dispatch/DispatchTask.java | 41 ++ .../com/cloud/api/dispatch/DispatchWorker.java | 30 ++ .../dispatch/ParamGenericValidationWorker.java | 103 ++++ .../cloud/api/dispatch/ParamProcessWorker.java | 428 ++++++++++++++++ .../cloud/api/dispatch/ParamUnpackWorker.java | 114 +++++ .../dispatch/SpecificCmdValidationWorker.java | 34 ++ .../cloud/network/as/AutoScaleManagerImpl.java | 466 +++++++++--------- .../VirtualNetworkApplianceManagerImpl.java | 2 - .../storage/snapshot/SnapshotSchedulerImpl.java | 105 ++-- .../test/com/cloud/api/ApiDispatcherTest.java | 106 ---- .../api/dispatch/CommandCreationWorkerTest.java | 48 ++ .../api/dispatch/DispatchChainFactoryTest.java | 55 +++ .../ParamGenericValidationWorkerTest.java | 173 +++++++ .../api/dispatch/ParamProcessWorkerTest.java | 107 ++++ .../SpecificCmdValidationWorkerTest.java | 48 ++ 26 files changed, 2157 insertions(+), 1179 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c211f0bb/api/src/org/apache/cloudstack/api/ApiConstants.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/ApiConstants.java b/api/src/org/apache/cloudstack/api/ApiConstants.java index 14df653..f0de48e 100755 --- a/api/src/org/apache/cloudstack/api/ApiConstants.java +++ b/api/src/org/apache/cloudstack/api/ApiConstants.java @@ -23,7 +23,8 @@ public class ApiConstants { public static final String ACCOUNT_ID = "accountid"; public static final String ALGORITHM = "algorithm"; public static final String ALLOCATED_ONLY = "allocatedonly"; - public static final String API_KEY = "userapikey"; + public static final String API_KEY = "apikey"; + public static final String USER_API_KEY = "userapikey"; public static final String APPLIED = "applied"; public static final String AVAILABLE = "available"; public static final String BITS = "bits"; @@ -49,10 +50,16 @@ public class ApiConstants { public static final String CLUSTER_ID = "clusterid"; public static final String CLUSTER_NAME = "clustername"; public static final String CLUSTER_TYPE = "clustertype"; + public static final String COMMAND = "command"; + public static final String CMD_EVENT_TYPE = "cmdeventtype"; public static final String COMPONENT = "component"; public static final String CPU_NUMBER = "cpunumber"; public static final String CPU_SPEED = "cpuspeed"; public static final String CREATED = "created"; + public static final String CTX_ACCOUNT_ID = "ctxaccountid"; + public static final String CTX_USER_ID = "ctxuserid"; + public static final String CTXSTARTEVENTID = "ctxstarteventid"; + public static final String CTX_START_EVENT_ID = "ctxStartEventId"; public static final String CUSTOMIZED = "customized"; public static final String CUSTOMIZED_IOPS = "customizediops"; public static final String CUSTOM_ID = "customid"; @@ -79,6 +86,7 @@ public class ApiConstants { public static final String IP6_DNS2 = "ip6dns2"; public static final String DOMAIN = "domain"; public static final String DOMAIN_ID = "domainid"; + public static final String DOMAIN__ID = "domainId"; public static final String DURATION = "duration"; public static final String EMAIL = "email"; public static final String END_DATE = "enddate"; @@ -86,6 +94,7 @@ public class ApiConstants { public static final String END_IPV6 = "endipv6"; public static final String END_PORT = "endport"; public static final String ENTRY_TIME = "entrytime"; + public static final String EXPIRES = "expires"; public static final String FETCH_LATEST = "fetchlatest"; public static final String FIRSTNAME = "firstname"; public static final String FORCED = "forced"; @@ -209,8 +218,11 @@ public class ApiConstants { public static final String SENT = "sent"; public static final String SENT_BYTES = "sentbytes"; public static final String SERVICE_OFFERING_ID = "serviceofferingid"; + public static final String SESSIONKEY = "sessionkey"; public static final String SHOW_CAPACITIES = "showcapacities"; public static final String SHOW_REMOVED = "showremoved"; + public static final String SIGNATURE = "signature"; + public static final String SIGNATURE_VERSION = "signatureversion"; public static final String SIZE = "size"; public static final String SNAPSHOT_ID = "snapshotid"; public static final String SNAPSHOT_POLICY_ID = "snapshotpolicyid"; @@ -277,6 +289,7 @@ public class ApiConstants { public static final String NETWORKRATE = "networkrate"; public static final String HOST_TAGS = "hosttags"; public static final String SSH_KEYPAIR = "keypair"; + public static final String HTTPMETHOD = "httpmethod"; public static final String HOST_CPU_CAPACITY = "hostcpucapacity"; public static final String HOST_CPU_NUM = "hostcpunum"; public static final String HOST_MEM_CAPACITY = "hostmemcapacity"; http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c211f0bb/api/src/org/apache/cloudstack/api/BaseCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/BaseCmd.java b/api/src/org/apache/cloudstack/api/BaseCmd.java index e869ddf..02250d7 100644 --- a/api/src/org/apache/cloudstack/api/BaseCmd.java +++ b/api/src/org/apache/cloudstack/api/BaseCmd.java @@ -17,22 +17,28 @@ package org.apache.cloudstack.api; +import java.lang.reflect.Field; import java.text.DateFormat; import java.text.SimpleDateFormat; +import java.util.ArrayList; import java.util.Date; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.regex.Pattern; import javax.inject.Inject; +import org.apache.cloudstack.acl.RoleType; import org.apache.cloudstack.affinity.AffinityGroupService; import org.apache.cloudstack.alert.AlertService; +import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.network.element.InternalLoadBalancerElementService; import org.apache.cloudstack.network.lb.ApplicationLoadBalancerService; import org.apache.cloudstack.network.lb.InternalLoadBalancerVMService; import org.apache.cloudstack.query.QueryService; import org.apache.cloudstack.usage.UsageService; + import org.apache.log4j.Logger; import com.cloud.configuration.ConfigurationService; @@ -74,6 +80,7 @@ import com.cloud.user.Account; import com.cloud.user.AccountService; import com.cloud.user.DomainService; import com.cloud.user.ResourceLimitService; +import com.cloud.utils.ReflectUtil; import com.cloud.utils.db.EntityManager; import com.cloud.utils.db.UUIDManager; import com.cloud.vm.UserVmService; @@ -97,7 +104,9 @@ public abstract class BaseCmd { public static Pattern newInputDateFormat = Pattern.compile("[\\d]+-[\\d]+-[\\d]+ [\\d]+:[\\d]+:[\\d]+"); private static final DateFormat s_outputFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ssZ"); - private Object _responseObject = null; + protected static final Map<Class<?>, List<Field>> fieldsForCmdClass = new HashMap<Class<?>, List<Field>>(); + + private Object _responseObject; private Map<String, String> fullUrlParams; public enum HTTPMethod { @@ -205,7 +214,7 @@ public abstract class BaseCmd { return httpMethod; } - public void setHttpMethod(String method) { + public void setHttpMethod(final String method) { if (method != null) { if (method.equalsIgnoreCase("GET")) httpMethod = HTTPMethod.GET; @@ -227,12 +236,36 @@ public abstract class BaseCmd { return responseType; } - public void setResponseType(String responseType) { + public void setResponseType(final String responseType) { this.responseType = responseType; } + /** + * For some reason this method does not return the actual command name, but more a name that + * is used to create the response. So you can expect for a XCmd a value like xcmdresponse. Anyways + * this methods is used in too many places so for now instead of changing it we just create another + * method {@link BaseCmd#getActualCommandName()} that returns the value from {@link APICommand#name()} + * + * @return + */ public abstract String getCommandName(); + + /** + * Gets the CommandName based on the class annotations: the value from {@link APICommand#name()} + * + * @return the value from {@link APICommand#name()} + */ + public String getActualCommandName() { + String cmdName = null; + if (this.getClass().getAnnotation(APICommand.class) != null) { + cmdName = this.getClass().getAnnotation(APICommand.class).name(); + } else { + cmdName = this.getClass().getName(); + } + return cmdName; + } + /** * For commands the API framework needs to know the owner of the object being acted upon. This method is * used to determine that information. @@ -245,7 +278,7 @@ public abstract class BaseCmd { return _responseObject; } - public void setResponseObject(Object responseObject) { + public void setResponseObject(final Object responseObject) { _responseObject = responseObject; } @@ -253,7 +286,7 @@ public abstract class BaseCmd { return _mgr; } - public static String getDateString(Date date) { + public static String getDateString(final Date date) { if (date == null) { return ""; } @@ -264,101 +297,86 @@ public abstract class BaseCmd { return formattedString; } - // FIXME: move this to a utils method so that maps can be unpacked and integer/long values can be appropriately cast - @SuppressWarnings({"unchecked", "rawtypes"}) - public Map<String, Object> unpackParams(Map<String, String> params) { - Map<String, Object> lowercaseParams = new HashMap<String, Object>(); - for (String key : params.keySet()) { - int arrayStartIndex = key.indexOf('['); - int arrayStartLastIndex = key.lastIndexOf('['); - if (arrayStartIndex != arrayStartLastIndex) { - throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, "Unable to decode parameter " + key + - "; if specifying an object array, please use parameter[index].field=XXX, e.g. userGroupList[0].group=httpGroup"); - } + protected List<Field> getAllFieldsForClass(final Class<?> clazz) { + List<Field> filteredFields = fieldsForCmdClass.get(clazz); - if (arrayStartIndex > 0) { - int arrayEndIndex = key.indexOf(']'); - int arrayEndLastIndex = key.lastIndexOf(']'); - if ((arrayEndIndex < arrayStartIndex) || (arrayEndIndex != arrayEndLastIndex)) { - // malformed parameter - throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, "Unable to decode parameter " + key + - "; if specifying an object array, please use parameter[index].field=XXX, e.g. userGroupList[0].group=httpGroup"); - } + // If list of fields was not cached yet + if (filteredFields == null) { + final List<Field> allFields = ReflectUtil.getAllFieldsForClass(this.getClass(), BaseCmd.class); + filteredFields = new ArrayList<Field>(); - // Now that we have an array object, check for a field name in the case of a complex object - int fieldIndex = key.indexOf('.'); - String fieldName = null; - if (fieldIndex < arrayEndIndex) { - throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, "Unable to decode parameter " + key + - "; if specifying an object array, please use parameter[index].field=XXX, e.g. userGroupList[0].group=httpGroup"); - } else { - fieldName = key.substring(fieldIndex + 1); + for (final Field field : allFields) { + final Parameter parameterAnnotation = field.getAnnotation(Parameter.class); + if ((parameterAnnotation != null) && parameterAnnotation.expose()) { + filteredFields.add(field); } + } - // parse the parameter name as the text before the first '[' character - String paramName = key.substring(0, arrayStartIndex); - paramName = paramName.toLowerCase(); - - Map<Integer, Map> mapArray = null; - Map<String, Object> mapValue = null; - String indexStr = key.substring(arrayStartIndex + 1, arrayEndIndex); - int index = 0; - boolean parsedIndex = false; - try { - if (indexStr != null) { - index = Integer.parseInt(indexStr); - parsedIndex = true; - } - } catch (NumberFormatException nfe) { - s_logger.warn("Invalid parameter " + key + " received, unable to parse object array, returning an error."); - } + // Cache the prepared list for future use + fieldsForCmdClass.put(clazz, filteredFields); + } + return filteredFields; + } - if (!parsedIndex) { - throw new ServerApiException(ApiErrorCode.MALFORMED_PARAMETER_ERROR, "Unable to decode parameter " + key + - "; if specifying an object array, please use parameter[index].field=XXX, e.g. userGroupList[0].group=httpGroup"); - } + protected Account getCurrentContextAccount() { + return CallContext.current().getCallingAccount(); + } - Object value = lowercaseParams.get(paramName); - if (value == null) { - // for now, assume object array with sub fields - mapArray = new HashMap<Integer, Map>(); - mapValue = new HashMap<String, Object>(); - mapArray.put(Integer.valueOf(index), mapValue); - } else if (value instanceof Map) { - mapArray = (HashMap)value; - mapValue = mapArray.get(Integer.valueOf(index)); - if (mapValue == null) { - mapValue = new HashMap<String, Object>(); - mapArray.put(Integer.valueOf(index), mapValue); + /** + * This method doesn't return all the @{link Parameter}, but only the ones exposed + * and allowed for current @{link RoleType}. This method will get the fields for a given + * Cmd class only once and never again, so in case of a dynamic update the result would + * be obsolete (this might be a plugin update. It is agreed upon that we will not do + * upgrades dynamically but in case we come back on that decision we need to revisit this) + * + * @return + */ + public List<Field> getParamFields() { + final List<Field> allFields = getAllFieldsForClass(this.getClass()); + final List<Field> validFields = new ArrayList<Field>(); + final Account caller = getCurrentContextAccount(); + + for (final Field field : allFields) { + final Parameter parameterAnnotation = field.getAnnotation(Parameter.class); + + //TODO: Annotate @Validate on API Cmd classes, FIXME how to process Validate + final RoleType[] allowedRoles = parameterAnnotation.authorized(); + boolean roleIsAllowed = true; + if (allowedRoles.length > 0) { + roleIsAllowed = false; + for (final RoleType allowedRole : allowedRoles) { + if (allowedRole.getValue() == caller.getType()) { + roleIsAllowed = true; + break; } } + } - // we are ready to store the value for a particular field into the map for this object - mapValue.put(fieldName, params.get(key)); - - lowercaseParams.put(paramName, mapArray); + if (roleIsAllowed) { + validFields.add(field); } else { - lowercaseParams.put(key.toLowerCase(), params.get(key)); + s_logger.debug("Ignoring paremeter " + parameterAnnotation.name() + " as the caller is not authorized to pass it in"); } } - return lowercaseParams; + + return validFields; } - protected long getInstanceIdFromJobSuccessResult(String result) { + protected long getInstanceIdFromJobSuccessResult(final String result) { s_logger.debug("getInstanceIdFromJobSuccessResult not overridden in subclass " + this.getClass().getName()); return 0; } - public static boolean isAdmin(short accountType) { + public static boolean isAdmin(final short accountType) { return ((accountType == Account.ACCOUNT_TYPE_ADMIN) || (accountType == Account.ACCOUNT_TYPE_RESOURCE_DOMAIN_ADMIN) || (accountType == Account.ACCOUNT_TYPE_DOMAIN_ADMIN) || (accountType == Account.ACCOUNT_TYPE_READ_ONLY_ADMIN)); } - public static boolean isRootAdmin(short accountType) { + public static boolean isRootAdmin(final short accountType) { return ((accountType == Account.ACCOUNT_TYPE_ADMIN)); } - public void setFullUrlParams(Map<String, String> map) { + public void setFullUrlParams(final Map<String, String> map) { fullUrlParams = map; } @@ -366,18 +384,18 @@ public abstract class BaseCmd { return fullUrlParams; } - public Long finalyzeAccountId(String accountName, Long domainId, Long projectId, boolean enabledOnly) { + public Long finalyzeAccountId(final String accountName, final Long domainId, final Long projectId, final boolean enabledOnly) { if (accountName != null) { if (domainId == null) { throw new InvalidParameterValueException("Account must be specified with domainId parameter"); } - Domain domain = _domainService.getDomain(domainId); + final Domain domain = _domainService.getDomain(domainId); if (domain == null) { throw new InvalidParameterValueException("Unable to find domain by id"); } - Account account = _accountService.getActiveAccountByName(accountName, domainId); + final Account account = _accountService.getActiveAccountByName(accountName, domainId); if (account != null && account.getType() != Account.ACCOUNT_TYPE_PROJECT) { if (!enabledOnly || account.getState() == Account.State.enabled) { return account.getId(); @@ -394,12 +412,12 @@ public abstract class BaseCmd { } if (projectId != null) { - Project project = _projectService.getProject(projectId); + final Project project = _projectService.getProject(projectId); if (project != null) { if (!enabledOnly || project.getState() == Project.State.Active) { return project.getProjectAccountId(); } else { - PermissionDeniedException ex = + final PermissionDeniedException ex = new PermissionDeniedException("Can't add resources to the project with specified projectId in state=" + project.getState() + " as it's no longer active"); ex.addProxyObject(project.getUuid(), "projectId"); @@ -413,6 +431,13 @@ public abstract class BaseCmd { } /** + * To be overwritten by any class who needs specific validation + */ + public void validateSpecificParameters(final Map<String, String> params){ + // To be overwritten by any class who needs specific validation + } + + /** * display flag is used to control the display of the resource only to the end user. It doesnt affect Root Admin. * @return display flag */ http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c211f0bb/api/src/org/apache/cloudstack/api/BaseListCmd.java ---------------------------------------------------------------------- diff --git a/api/src/org/apache/cloudstack/api/BaseListCmd.java b/api/src/org/apache/cloudstack/api/BaseListCmd.java index c1a4b4c..f280003 100644 --- a/api/src/org/apache/cloudstack/api/BaseListCmd.java +++ b/api/src/org/apache/cloudstack/api/BaseListCmd.java @@ -16,7 +16,10 @@ // under the License. package org.apache.cloudstack.api; +import java.util.Map; + import com.cloud.exception.InvalidParameterValueException; +import com.cloud.utils.exception.CSExceptionErrorCode; public abstract class BaseListCmd extends BaseCmd { @@ -83,7 +86,7 @@ public abstract class BaseListCmd extends BaseCmd { public Long getPageSizeVal() { Long defaultPageSize = s_maxPageSize; - Integer pageSizeInt = getPageSize(); + final Integer pageSizeInt = getPageSize(); if (pageSizeInt != null) { defaultPageSize = pageSizeInt.longValue(); } @@ -96,12 +99,12 @@ public abstract class BaseListCmd extends BaseCmd { public Long getStartIndex() { Long startIndex = Long.valueOf(0); - Long pageSizeVal = getPageSizeVal(); + final Long pageSizeVal = getPageSizeVal(); if (pageSizeVal == null) { startIndex = null; } else if (page != null) { - int pageNum = page.intValue(); + final int pageNum = page.intValue(); if (pageNum > 0) { startIndex = Long.valueOf(pageSizeVal * (pageNum - 1)); } @@ -112,4 +115,25 @@ public abstract class BaseListCmd extends BaseCmd { public ApiCommandJobType getInstanceType() { return ApiCommandJobType.None; } + + @Override + public void validateSpecificParameters(final Map<String, String> params){ + super.validateSpecificParameters(params); + + final Object pageSizeObj = params.get(ApiConstants.PAGE_SIZE); + Long pageSize = null; + if (pageSizeObj != null) { + pageSize = Long.valueOf((String)pageSizeObj); + } + + if (params.get(ApiConstants.PAGE) == null && + pageSize != null && + !pageSize.equals(BaseListCmd.s_pageSizeUnlimited)) { + final ServerApiException ex = new ServerApiException(ApiErrorCode.PARAM_ERROR, "\"page\" parameter is required when \"pagesize\" is specified"); + ex.setCSErrorCode(CSExceptionErrorCode.getCSErrCode(ex.getClass().getName())); + throw ex; + } else if (pageSize == null && (params.get(ApiConstants.PAGE) != null)) { + throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "\"pagesize\" parameter is required when \"page\" is specified"); + } + } } http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c211f0bb/api/test/org/apache/cloudstack/api/BaseCmdTest.java ---------------------------------------------------------------------- diff --git a/api/test/org/apache/cloudstack/api/BaseCmdTest.java b/api/test/org/apache/cloudstack/api/BaseCmdTest.java new file mode 100644 index 0000000..edf5776 --- /dev/null +++ b/api/test/org/apache/cloudstack/api/BaseCmdTest.java @@ -0,0 +1,69 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. +package org.apache.cloudstack.api; + +import static org.junit.Assert.assertEquals; + +import org.junit.Test; + +import com.cloud.exception.ConcurrentOperationException; +import com.cloud.exception.InsufficientCapacityException; +import com.cloud.exception.NetworkRuleConflictException; +import com.cloud.exception.ResourceAllocationException; +import com.cloud.exception.ResourceUnavailableException; + +public class BaseCmdTest { + + private static final String NON_EXPECTED_COMMAND_NAME = "Non expected command name"; + protected static final String CMD1_NAME = "Cmd1Name"; + protected static final String CMD2_NAME = "Cmd2Name"; + protected static final String CMD1_RESPONSE = "cmd1response"; + protected static final String CMD2_RESPONSE = "cmd2response"; + + @Test + public void testGetActualCommandName(){ + BaseCmd cmd1 = new Cmd1(); + BaseCmd cmd2 = new Cmd2(); + + assertEquals(NON_EXPECTED_COMMAND_NAME, CMD1_NAME, cmd1.getActualCommandName()); + assertEquals(NON_EXPECTED_COMMAND_NAME, CMD2_NAME, cmd2.getActualCommandName()); + } +} + +@APICommand(name=BaseCmdTest.CMD1_NAME, responseObject=BaseResponse.class) +class Cmd1 extends BaseCmd { + @Override + public void execute() throws ResourceUnavailableException, InsufficientCapacityException, ServerApiException, ConcurrentOperationException, ResourceAllocationException, + NetworkRuleConflictException { + } + @Override + public String getCommandName() { + return BaseCmdTest.CMD1_RESPONSE; + } + @Override + public long getEntityOwnerId() { + return 0; + } +} + +@APICommand(name=BaseCmdTest.CMD2_NAME, responseObject=BaseResponse.class) +class Cmd2 extends Cmd1 { + @Override + public String getCommandName() { + return BaseCmdTest.CMD2_RESPONSE; + } +} http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c211f0bb/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml ---------------------------------------------------------------------- diff --git a/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml b/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml index fd2f5fb..91401e3 100644 --- a/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml +++ b/server/resources/META-INF/cloudstack/core/spring-server-core-misc-context.xml @@ -50,6 +50,18 @@ <bean id="apiDispatcher" class="com.cloud.api.ApiDispatcher" /> + <bean id="dispatchChainFactory" class="com.cloud.api.dispatch.DispatchChainFactory" /> + + <bean id="paramProcessWorker" class="com.cloud.api.dispatch.ParamProcessWorker" /> + + <bean id="paramUnpackWorker" class="com.cloud.api.dispatch.ParamUnpackWorker" /> + + <bean id="paramGenericValidationWorker" class="com.cloud.api.dispatch.ParamGenericValidationWorker" /> + + <bean id="specificCmdValidationWorker" class="com.cloud.api.dispatch.SpecificCmdValidationWorker" /> + + <bean id="commandCreationWorker" class="com.cloud.api.dispatch.CommandCreationWorker" /> + <bean id="apiResponseHelper" class="com.cloud.api.ApiResponseHelper" /> <bean id="apiServer" class="com.cloud.api.ApiServer"> http://git-wip-us.apache.org/repos/asf/cloudstack/blob/c211f0bb/server/src/com/cloud/api/ApiDispatcher.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/api/ApiDispatcher.java b/server/src/com/cloud/api/ApiDispatcher.java index 5bdefe7..5d4b4d3 100755 --- a/server/src/com/cloud/api/ApiDispatcher.java +++ b/server/src/com/cloud/api/ApiDispatcher.java @@ -16,127 +16,72 @@ // under the License. package com.cloud.api; -import static org.apache.commons.lang.StringUtils.isNotBlank; - -import java.lang.reflect.Field; -import java.text.DateFormat; -import java.text.ParseException; -import java.util.ArrayList; -import java.util.Calendar; -import java.util.Date; -import java.util.HashMap; -import java.util.List; import java.util.Map; -import java.util.StringTokenizer; -import java.util.regex.Matcher; import javax.annotation.PostConstruct; import javax.inject.Inject; -import org.apache.cloudstack.acl.ControlledEntity; -import org.apache.cloudstack.acl.InfrastructureEntity; -import org.apache.cloudstack.acl.RoleType; -import org.apache.cloudstack.acl.SecurityChecker.AccessType; -import org.apache.cloudstack.api.ACL; import org.apache.cloudstack.api.ApiConstants; -import org.apache.cloudstack.api.ApiErrorCode; import org.apache.cloudstack.api.BaseAsyncCmd; import org.apache.cloudstack.api.BaseAsyncCreateCmd; import org.apache.cloudstack.api.BaseAsyncCustomIdCmd; import org.apache.cloudstack.api.BaseCmd; -import org.apache.cloudstack.api.BaseCmd.CommandType; import org.apache.cloudstack.api.BaseCustomIdCmd; -import org.apache.cloudstack.api.BaseListCmd; -import org.apache.cloudstack.api.EntityReference; -import org.apache.cloudstack.api.InternalIdentity; -import org.apache.cloudstack.api.Parameter; -import org.apache.cloudstack.api.ServerApiException; -import org.apache.cloudstack.api.command.admin.resource.ArchiveAlertsCmd; -import org.apache.cloudstack.api.command.admin.resource.DeleteAlertsCmd; -import org.apache.cloudstack.api.command.user.event.ArchiveEventsCmd; -import org.apache.cloudstack.api.command.user.event.DeleteEventsCmd; -import org.apache.cloudstack.api.command.user.event.ListEventsCmd; import org.apache.cloudstack.context.CallContext; import org.apache.cloudstack.framework.jobs.AsyncJob; import org.apache.cloudstack.framework.jobs.AsyncJobManager; import org.apache.log4j.Logger; -import com.cloud.exception.InvalidParameterValueException; -import com.cloud.user.Account; -import com.cloud.user.AccountManager; -import com.cloud.utils.DateUtil; -import com.cloud.utils.ReflectUtil; -import com.cloud.utils.db.EntityManager; -import com.cloud.utils.exception.CSExceptionErrorCode; -import com.cloud.utils.exception.CloudRuntimeException; +import com.cloud.api.dispatch.DispatchChain; +import com.cloud.api.dispatch.DispatchChainFactory; +import com.cloud.api.dispatch.DispatchTask; public class ApiDispatcher { private static final Logger s_logger = Logger.getLogger(ApiDispatcher.class.getName()); Long _createSnapshotQueueSizeLimit; + @Inject - AsyncJobManager _asyncMgr = null; - @Inject - AccountManager _accountMgr = null; - @Inject - EntityManager _entityMgr = null; + AsyncJobManager _asyncMgr; - private static ApiDispatcher s_instance; + @Inject() + protected DispatchChainFactory dispatchChainFactory; - public static ApiDispatcher getInstance() { - return s_instance; - } + protected DispatchChain standardDispatchChain; + + protected DispatchChain asyncCreationDispatchChain; public ApiDispatcher() { } @PostConstruct - void init() { - s_instance = this; + public void setup() { + standardDispatchChain = dispatchChainFactory.getStandardDispatchChain(); + asyncCreationDispatchChain = dispatchChainFactory.getAsyncCreationDispatchChain(); } - public void setCreateSnapshotQueueSizeLimit(Long snapshotLimit) { + public void setCreateSnapshotQueueSizeLimit(final Long snapshotLimit) { _createSnapshotQueueSizeLimit = snapshotLimit; } - public void dispatchCreateCmd(BaseAsyncCreateCmd cmd, Map<String, String> params) throws Exception { - processParameters(cmd, params); - CallContext.current().setEventDisplayEnabled(cmd.isDisplayResourceEnabled()); - cmd.create(); + public void dispatchCreateCmd(final BaseAsyncCreateCmd cmd, final Map<String, String> params) throws Exception { + asyncCreationDispatchChain.dispatch(new DispatchTask(cmd, params)); + CallContext.current().setEventDisplayEnabled(cmd.isDisplayResourceEnabled()); } - private void doAccessChecks(BaseCmd cmd, Map<Object, AccessType> entitiesToAccess) { - Account caller = CallContext.current().getCallingAccount(); - Account owner = _accountMgr.getActiveAccountById(cmd.getEntityOwnerId()); + public void dispatch(final BaseCmd cmd, final Map<String, String> params, final boolean execute) throws Exception { + // Let the chain of responsibility dispatch gradually + standardDispatchChain.dispatch(new DispatchTask(cmd, params)); - if (cmd instanceof BaseAsyncCreateCmd) { - //check that caller can access the owner account. - _accountMgr.checkAccess(caller, null, true, owner); - } - - if (!entitiesToAccess.isEmpty()) { - //check that caller can access the owner account. - _accountMgr.checkAccess(caller, null, true, owner); - for (Object entity : entitiesToAccess.keySet()) { - if (entity instanceof ControlledEntity) { - _accountMgr.checkAccess(caller, entitiesToAccess.get(entity), true, (ControlledEntity)entity); - } else if (entity instanceof InfrastructureEntity) { - //FIXME: Move this code in adapter, remove code from Account manager - } - } - } - } - - public void dispatch(BaseCmd cmd, Map<String, String> params, boolean execute) throws Exception { - processParameters(cmd, params); - CallContext ctx = CallContext.current(); + final CallContext ctx = CallContext.current(); ctx.setEventDisplayEnabled(cmd.isDisplayResourceEnabled()); + // TODO This if shouldn't be here. Use polymorphism and move it to validateSpecificParameters if (cmd instanceof BaseAsyncCmd) { - BaseAsyncCmd asyncCmd = (BaseAsyncCmd)cmd; - String startEventId = params.get("ctxStartEventId"); + final BaseAsyncCmd asyncCmd = (BaseAsyncCmd)cmd; + final String startEventId = params.get(ApiConstants.CTX_START_EVENT_ID); ctx.setStartEventId(Long.valueOf(startEventId)); // Synchronise job on the object if needed @@ -160,6 +105,7 @@ public class ApiDispatcher { } } + // TODO This if shouldn't be here. Use polymorphism and move it to validateSpecificParameters if (cmd instanceof BaseAsyncCustomIdCmd) { ((BaseAsyncCustomIdCmd)cmd).checkUuid(); } else if (cmd instanceof BaseCustomIdCmd) { @@ -167,392 +113,6 @@ public class ApiDispatcher { } cmd.execute(); - } - @SuppressWarnings({"unchecked", "rawtypes"}) - public static void processParameters(BaseCmd cmd, Map<String, String> params) { - Map<Object, AccessType> entitiesToAccess = new HashMap<Object, AccessType>(); - Map<String, Object> unpackedParams = cmd.unpackParams(params); - - if (cmd instanceof BaseListCmd) { - Object pageSizeObj = unpackedParams.get(ApiConstants.PAGE_SIZE); - Long pageSize = null; - if (pageSizeObj != null) { - pageSize = Long.valueOf((String)pageSizeObj); - } - - if ((unpackedParams.get(ApiConstants.PAGE) == null) && (pageSize != null && !pageSize.equals(BaseListCmd.s_pageSizeUnlimited))) { - ServerApiException ex = new ServerApiException(ApiErrorCode.PARAM_ERROR, "\"page\" parameter is required when \"pagesize\" is specified"); - ex.setCSErrorCode(CSExceptionErrorCode.getCSErrCode(ex.getClass().getName())); - throw ex; - } else if (pageSize == null && (unpackedParams.get(ApiConstants.PAGE) != null)) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "\"pagesize\" parameter is required when \"page\" is specified"); - } - } - - List<Field> fields = ReflectUtil.getAllFieldsForClass(cmd.getClass(), BaseCmd.class); - - for (Field field : fields) { - Parameter parameterAnnotation = field.getAnnotation(Parameter.class); - if ((parameterAnnotation == null) || !parameterAnnotation.expose()) { - continue; - } - - //TODO: Annotate @Validate on API Cmd classes, FIXME how to process Validate - RoleType[] allowedRoles = parameterAnnotation.authorized(); - if (allowedRoles.length > 0) { - boolean permittedParameter = false; - Account caller = CallContext.current().getCallingAccount(); - for (RoleType allowedRole : allowedRoles) { - if (allowedRole.getValue() == caller.getType()) { - permittedParameter = true; - break; - } - } - if (!permittedParameter) { - s_logger.debug("Ignoring paremeter " + parameterAnnotation.name() + " as the caller is not authorized to pass it in"); - continue; - } - } - - Object paramObj = unpackedParams.get(parameterAnnotation.name()); - if (paramObj == null) { - if (parameterAnnotation.required()) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unable to execute API command " + - cmd.getCommandName().substring(0, cmd.getCommandName().length() - 8) + " due to missing parameter " + parameterAnnotation.name()); - } - continue; - } - - // marshall the parameter into the correct type and set the field value - try { - setFieldValue(field, cmd, paramObj, parameterAnnotation); - } catch (IllegalArgumentException argEx) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Unable to execute API command " + cmd.getCommandName() + " due to invalid value " + paramObj + " for parameter " + - parameterAnnotation.name()); - } - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unable to execute API command " + - cmd.getCommandName().substring(0, cmd.getCommandName().length() - 8) + " due to invalid value " + paramObj + " for parameter " + - parameterAnnotation.name()); - } catch (ParseException parseEx) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("Invalid date parameter " + paramObj + " passed to command " + cmd.getCommandName().substring(0, cmd.getCommandName().length() - 8)); - } - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unable to parse date " + paramObj + " for command " + - cmd.getCommandName().substring(0, cmd.getCommandName().length() - 8) + ", please pass dates in the format mentioned in the api documentation"); - } catch (InvalidParameterValueException invEx) { - throw new ServerApiException(ApiErrorCode.PARAM_ERROR, "Unable to execute API command " + - cmd.getCommandName().substring(0, cmd.getCommandName().length() - 8) + " due to invalid value. " + invEx.getMessage()); - } catch (CloudRuntimeException cloudEx) { - s_logger.error("CloudRuntimeException", cloudEx); - // FIXME: Better error message? This only happens if the API command is not executable, which typically - //means - // there was - // and IllegalAccessException setting one of the parameters. - throw new ServerApiException(ApiErrorCode.INTERNAL_ERROR, "Internal error executing API command " + - cmd.getCommandName().substring(0, cmd.getCommandName().length() - 8)); - } - - //check access on the resource this field points to - try { - ACL checkAccess = field.getAnnotation(ACL.class); - CommandType fieldType = parameterAnnotation.type(); - - if (checkAccess != null) { - // Verify that caller can perform actions in behalf of vm owner - //acumulate all Controlled Entities together. - - //parse the array of resource types and in case of map check access on key or value or both as specified in @acl - //implement external dao for classes that need findByName - //for maps, specify access to be checkd on key or value. - - // find the controlled entity DBid by uuid - if (parameterAnnotation.entityType() != null) { - Class<?>[] entityList = parameterAnnotation.entityType()[0].getAnnotation(EntityReference.class).value(); - - for (Class entity : entityList) { - // Check if the parameter type is a single - // Id or list of id's/name's - switch (fieldType) { - case LIST: - CommandType listType = parameterAnnotation.collectionType(); - switch (listType) { - case LONG: - case UUID: - List<Long> listParam = (List<Long>)field.get(cmd); - for (Long entityId : listParam) { - Object entityObj = s_instance._entityMgr.findById(entity, entityId); - entitiesToAccess.put(entityObj, checkAccess.accessType()); - } - break; - /* - * case STRING: List<String> listParam = - * new ArrayList<String>(); listParam = - * (List)field.get(cmd); for(String - * entityName: listParam){ - * ControlledEntity entityObj = - * (ControlledEntity - * )daoClassInstance(entityId); - * entitiesToAccess.add(entityObj); } - * break; - */ - default: - break; - } - break; - case LONG: - case UUID: - Object entityObj = s_instance._entityMgr.findById(entity, (Long)field.get(cmd)); - entitiesToAccess.put(entityObj, checkAccess.accessType()); - break; - default: - break; - } - - if (ControlledEntity.class.isAssignableFrom(entity)) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("ControlledEntity name is:" + entity.getName()); - } - } - - if (InfrastructureEntity.class.isAssignableFrom(entity)) { - if (s_logger.isDebugEnabled()) { - s_logger.debug("InfrastructureEntity name is:" + entity.getName()); - } - } - } - - } - - } - - } catch (IllegalArgumentException e) { - s_logger.error("Error initializing command " + cmd.getCommandName() + ", field " + field.getName() + " is not accessible."); - throw new CloudRuntimeException("Internal error initializing parameters for command " + cmd.getCommandName() + " [field " + field.getName() + - " is not accessible]"); - } catch (IllegalAccessException e) { - s_logger.error("Error initializing command " + cmd.getCommandName() + ", field " + field.getName() + " is not accessible."); - throw new CloudRuntimeException("Internal error initializing parameters for command " + cmd.getCommandName() + " [field " + field.getName() + - " is not accessible]"); - } - - } - - //check access on the entities. - getInstance().doAccessChecks(cmd, entitiesToAccess); - - } - - private static Long translateUuidToInternalId(String uuid, Parameter annotation) { - if (uuid.equals("-1")) { - // FIXME: This is to handle a lot of hardcoded special cases where -1 is sent - // APITODO: Find and get rid of all hardcoded params in API Cmds and service layer - return -1L; - } - Long internalId = null; - // If annotation's empty, the cmd existed before 3.x try conversion to long - boolean isPre3x = annotation.since().isEmpty(); - // Match against Java's UUID regex to check if input is uuid string - boolean isUuid = uuid.matches("^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$"); - // Enforce that it's uuid for newly added apis from version 3.x - if (!isPre3x && !isUuid) - return null; - // Allow both uuid and internal id for pre3x apis - if (isPre3x && !isUuid) { - try { - internalId = Long.parseLong(uuid); - } catch (NumberFormatException e) { - internalId = null; - } - if (internalId != null) - return internalId; - } - // There may be multiple entities defined on the @EntityReference of a Response.class - // UUID CommandType would expect only one entityType, so use the first entityType - Class<?>[] entities = annotation.entityType()[0].getAnnotation(EntityReference.class).value(); - // Go through each entity which is an interface to a VO class and get a VO object - // Try to getId() for the object using reflection, break on first non-null value - for (Class<?> entity : entities) { - // For backward compatibility, we search within removed entities and let service layer deal - // with removed ones, return empty response or error - Object objVO = s_instance._entityMgr.findByUuidIncludingRemoved(entity, uuid); - if (objVO == null) { - continue; - } - // Invoke the getId method, get the internal long ID - // If that fails hide exceptions as the uuid may not exist - try { - internalId = ((InternalIdentity)objVO).getId(); - } catch (IllegalArgumentException e) { - } catch (NullPointerException e) { - } - // Return on first non-null Id for the uuid entity - if (internalId != null) - break; - } - if (internalId == null) { - if (s_logger.isDebugEnabled()) - s_logger.debug("Object entity uuid = " + uuid + " does not exist in the database."); - throw new InvalidParameterValueException("Invalid parameter " + annotation.name() + " value=" + uuid + - " due to incorrect long value format, or entity does not exist or due to incorrect parameter annotation for the field in api cmd class."); - } - return internalId; - } - - @SuppressWarnings({"unchecked", "rawtypes"}) - private static void setFieldValue(Field field, BaseCmd cmdObj, Object paramObj, Parameter annotation) throws IllegalArgumentException, ParseException { - try { - field.setAccessible(true); - CommandType fieldType = annotation.type(); - switch (fieldType) { - case BOOLEAN: - field.set(cmdObj, Boolean.valueOf(paramObj.toString())); - break; - case DATE: - // This piece of code is for maintaining backward compatibility - // and support both the date formats(Bug 9724) - // Do the date messaging for ListEventsCmd only - if (cmdObj instanceof ListEventsCmd || cmdObj instanceof DeleteEventsCmd || cmdObj instanceof ArchiveEventsCmd || - cmdObj instanceof ArchiveAlertsCmd || cmdObj instanceof DeleteAlertsCmd) { - boolean isObjInNewDateFormat = isObjInNewDateFormat(paramObj.toString()); - if (isObjInNewDateFormat) { - DateFormat newFormat = BaseCmd.NEW_INPUT_FORMAT; - synchronized (newFormat) { - field.set(cmdObj, newFormat.parse(paramObj.toString())); - } - } else { - DateFormat format = BaseCmd.INPUT_FORMAT; - synchronized (format) { - Date date = format.parse(paramObj.toString()); - if (field.getName().equals("startDate")) { - date = messageDate(date, 0, 0, 0); - } else if (field.getName().equals("endDate")) { - date = messageDate(date, 23, 59, 59); - } - field.set(cmdObj, date); - } - } - } else { - DateFormat format = BaseCmd.INPUT_FORMAT; - synchronized (format) { - format.setLenient(false); - field.set(cmdObj, format.parse(paramObj.toString())); - } - } - break; - case FLOAT: - // Assuming that the parameters have been checked for required before now, - // we ignore blank or null values and defer to the command to set a default - // value for optional parameters ... - if (paramObj != null && isNotBlank(paramObj.toString())) { - field.set(cmdObj, Float.valueOf(paramObj.toString())); - } - break; - case INTEGER: - // Assuming that the parameters have been checked for required before now, - // we ignore blank or null values and defer to the command to set a default - // value for optional parameters ... - if (paramObj != null && isNotBlank(paramObj.toString())) { - field.set(cmdObj, Integer.valueOf(paramObj.toString())); - } - break; - case LIST: - List listParam = new ArrayList(); - StringTokenizer st = new StringTokenizer(paramObj.toString(), ","); - while (st.hasMoreTokens()) { - String token = st.nextToken(); - CommandType listType = annotation.collectionType(); - switch (listType) { - case INTEGER: - listParam.add(Integer.valueOf(token)); - break; - case UUID: - if (token.isEmpty()) - break; - Long internalId = translateUuidToInternalId(token, annotation); - listParam.add(internalId); - break; - case LONG: { - listParam.add(Long.valueOf(token)); - } - break; - case SHORT: - listParam.add(Short.valueOf(token)); - case STRING: - listParam.add(token); - break; - } - } - field.set(cmdObj, listParam); - break; - case UUID: - if (paramObj.toString().isEmpty()) - break; - Long internalId = translateUuidToInternalId(paramObj.toString(), annotation); - field.set(cmdObj, internalId); - break; - case LONG: - field.set(cmdObj, Long.valueOf(paramObj.toString())); - break; - case SHORT: - field.set(cmdObj, Short.valueOf(paramObj.toString())); - break; - case STRING: - if ((paramObj != null) && paramObj.toString().length() > annotation.length()) { - s_logger.error("Value greater than max allowed length " + annotation.length() + " for param: " + field.getName()); - throw new InvalidParameterValueException("Value greater than max allowed length " + annotation.length() + " for param: " + field.getName()); - } - field.set(cmdObj, paramObj.toString()); - break; - case TZDATE: - field.set(cmdObj, DateUtil.parseTZDateString(paramObj.toString())); - break; - case MAP: - default: - field.set(cmdObj, paramObj); - break; - } - } catch (IllegalAccessException ex) { - s_logger.error("Error initializing command " + cmdObj.getCommandName() + ", field " + field.getName() + " is not accessible."); - throw new CloudRuntimeException("Internal error initializing parameters for command " + cmdObj.getCommandName() + " [field " + field.getName() + - " is not accessible]"); - } - } - - private static boolean isObjInNewDateFormat(String string) { - Matcher matcher = BaseCmd.newInputDateFormat.matcher(string); - return matcher.matches(); - } - - private static Date messageDate(Date date, int hourOfDay, int minute, int second) { - Calendar cal = Calendar.getInstance(); - cal.setTime(date); - cal.set(Calendar.HOUR_OF_DAY, hourOfDay); - cal.set(Calendar.MINUTE, minute); - cal.set(Calendar.SECOND, second); - return cal.getTime(); - } - - public static void plugService(Field field, BaseCmd cmd) { - - Class<?> fc = field.getType(); - Object instance = null; - - if (instance == null) { - throw new CloudRuntimeException("Unable to plug service " + fc.getSimpleName() + " in command " + cmd.getClass().getSimpleName()); - } - - try { - field.setAccessible(true); - field.set(cmd, instance); - } catch (IllegalArgumentException e) { - s_logger.error("IllegalArgumentException at plugService for command " + cmd.getCommandName() + ", field " + field.getName()); - throw new CloudRuntimeException("Internal error at plugService for command " + cmd.getCommandName() + " [Illegal argumet at field " + field.getName() + "]"); - } catch (IllegalAccessException e) { - s_logger.error("Error at plugService for command " + cmd.getCommandName() + ", field " + field.getName() + " is not accessible."); - throw new CloudRuntimeException("Internal error at plugService for command " + cmd.getCommandName() + " [field " + field.getName() + " is not accessible]"); - } - } }
