Updated Branches: refs/heads/master d9b85e397 -> fb94b7221
CLOUDSTACK-1568,CLOUDSTACK-1664: Fix ActionEvent interception and optimize @DB lookup with caching Project: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/commit/fb94b722 Tree: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/tree/fb94b722 Diff: http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/diff/fb94b722 Branch: refs/heads/master Commit: fb94b72213bf96f2878b90260067f61629c6a956 Parents: d9b85e3 Author: Kelven Yang <kelv...@gmail.com> Authored: Mon Mar 18 18:05:09 2013 -0700 Committer: Kelven Yang <kelv...@gmail.com> Committed: Mon Mar 18 18:07:52 2013 -0700 ---------------------------------------------------------------------- .../com/cloud/event/ActionEventInterceptor.java | 15 ++- .../ConsoleProxyHttpHandlerHelper.java | 14 ++- .../utils/component/ComponentMethodProxyCache.java | 90 +++++++++++++++ .../utils/component/SpringComponentScanUtils.java | 1 - .../cloud/utils/db/TransactionContextBuilder.java | 14 +-- 5 files changed, 119 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/fb94b722/server/src/com/cloud/event/ActionEventInterceptor.java ---------------------------------------------------------------------- diff --git a/server/src/com/cloud/event/ActionEventInterceptor.java b/server/src/com/cloud/event/ActionEventInterceptor.java index fb89498..a6c2565 100644 --- a/server/src/com/cloud/event/ActionEventInterceptor.java +++ b/server/src/com/cloud/event/ActionEventInterceptor.java @@ -19,22 +19,29 @@ package com.cloud.event; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; +import org.apache.log4j.Logger; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.reflect.MethodSignature; import com.cloud.user.UserContext; +import com.cloud.utils.component.ComponentMethodProxyCache; public class ActionEventInterceptor { + private static final Logger s_logger = Logger.getLogger(ActionEventInterceptor.class); public ActionEventInterceptor() { } public Object AroundAnyMethod(ProceedingJoinPoint call) throws Throwable { MethodSignature methodSignature = (MethodSignature)call.getSignature(); - Method targetMethod = methodSignature.getMethod(); - if(needToIntercept(targetMethod)) { + + // Note: AOP for ActionEvent is triggered annotation, no need to check the annotation on method again + Method targetMethod = ComponentMethodProxyCache.getTargetMethod( + methodSignature.getMethod(), call.getTarget()); + + if(targetMethod != null) { EventVO event = interceptStart(targetMethod); - + boolean success = true; Object ret = null; try { @@ -49,6 +56,8 @@ public class ActionEventInterceptor { } } return ret; + } else { + s_logger.error("Unable to find the proxied method behind. Method: " + methodSignature.getMethod().getName()); } return call.proceed(); } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/fb94b722/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java ---------------------------------------------------------------------- diff --git a/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java b/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java index 6815b0d..297e711 100644 --- a/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java +++ b/services/console-proxy/server/src/com/cloud/consoleproxy/ConsoleProxyHttpHandlerHelper.java @@ -53,7 +53,7 @@ public class ConsoleProxyHttpHandlerHelper { ConsoleProxyClientParam param = encryptor.decryptObject(ConsoleProxyClientParam.class, map.get("token")); // make sure we get information from token only - map.clear(); + guardUserInput(map); if(param != null) { if(param.getClientHostAddress() != null) map.put("host", param.getClientHostAddress()); @@ -72,9 +72,19 @@ public class ConsoleProxyHttpHandlerHelper { } } else { // we no longer accept information from parameter other than token - map.clear(); + guardUserInput(map); } return map; } + + private static void guardUserInput(Map<String, String> map) { + map.remove("host"); + map.remove("port"); + map.remove("tag"); + map.remove("sid"); + map.remove("consoleurl"); + map.remove("sessionref"); + map.remove("ticket"); + } } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/fb94b722/utils/src/com/cloud/utils/component/ComponentMethodProxyCache.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/component/ComponentMethodProxyCache.java b/utils/src/com/cloud/utils/component/ComponentMethodProxyCache.java new file mode 100644 index 0000000..ea3b685 --- /dev/null +++ b/utils/src/com/cloud/utils/component/ComponentMethodProxyCache.java @@ -0,0 +1,90 @@ +// 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 +// 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 com.cloud.utils.component; + +import java.lang.ref.WeakReference; +import java.lang.reflect.Method; +import java.util.WeakHashMap; + +public class ComponentMethodProxyCache { + + private static WeakHashMap<TargetKey, WeakReference<Method>> s_cache = new WeakHashMap<TargetKey, WeakReference<Method>>(); + + public ComponentMethodProxyCache() { + } + + public static Method getTargetMethod(Method method, Object target) { + synchronized(s_cache) { + WeakReference<Method> targetMethod = s_cache.get(new TargetKey(method, target)); + if(targetMethod != null && targetMethod.get() != null) + return targetMethod.get(); + + Class<?> clazz = target.getClass(); + for(Method m : clazz.getMethods()) { + if(isMethodMatched(method, m)) { + s_cache.put(new TargetKey(method, target), new WeakReference<Method>(m)); + return m; + } + } + + return method; + } + } + + private static boolean isMethodMatched(Method m1, Method m2) { + if(!m1.getName().equals(m2.getName())) + return false; + + Class<?>[] params1 = m1.getParameterTypes(); + Class<?>[] params2 = m2.getParameterTypes(); + + if(params1.length != params2.length) + return false; + + for(int i = 0; i < params1.length; i++) { + if(!params1[i].isAssignableFrom(params2[i])) + return false; + } + + return true; + } + + public static class TargetKey { + Method _method; + Object _target; + + public TargetKey(Method method, Object target) { + _method = method; + _target = target; + } + + @Override + public boolean equals(Object obj) { + if(!(obj instanceof TargetKey)) + return false; + + // for target object, we just check the reference + return _method.equals(((TargetKey)obj)._method) && + _target == ((TargetKey)obj)._target; + } + + public int hashCode() { + return _target.hashCode() ^ _target.hashCode(); + } + } +} http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/fb94b722/utils/src/com/cloud/utils/component/SpringComponentScanUtils.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/component/SpringComponentScanUtils.java b/utils/src/com/cloud/utils/component/SpringComponentScanUtils.java index fda11b7..9a85c79 100644 --- a/utils/src/com/cloud/utils/component/SpringComponentScanUtils.java +++ b/utils/src/com/cloud/utils/component/SpringComponentScanUtils.java @@ -38,5 +38,4 @@ public class SpringComponentScanUtils { } return false; } - } http://git-wip-us.apache.org/repos/asf/incubator-cloudstack/blob/fb94b722/utils/src/com/cloud/utils/db/TransactionContextBuilder.java ---------------------------------------------------------------------- diff --git a/utils/src/com/cloud/utils/db/TransactionContextBuilder.java b/utils/src/com/cloud/utils/db/TransactionContextBuilder.java index e03b25f..7ca33ab 100644 --- a/utils/src/com/cloud/utils/db/TransactionContextBuilder.java +++ b/utils/src/com/cloud/utils/db/TransactionContextBuilder.java @@ -22,9 +22,10 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; import org.apache.log4j.Logger; import org.aspectj.lang.ProceedingJoinPoint; -import org.aspectj.lang.Signature; import org.aspectj.lang.reflect.MethodSignature; +import com.cloud.utils.component.ComponentMethodProxyCache; + public class TransactionContextBuilder implements MethodInterceptor { private static final Logger s_logger = Logger.getLogger(TransactionContextBuilder.class); public TransactionContextBuilder() { @@ -72,14 +73,9 @@ public class TransactionContextBuilder implements MethodInterceptor { Class<?> clazz = method.getDeclaringClass(); if(clazz.isInterface()) { clazz = target.getClass(); - for(Method m : clazz.getMethods()) { - // it is supposed that we need to check against type arguments, - // this can be simplified by just checking method name - if(m.getName().equals(method.getName())) { - if(m.getAnnotation(DB.class) != null) - return true; - } - } + Method targetMethod = ComponentMethodProxyCache.getTargetMethod(method, target); + if(targetMethod != null && targetMethod.getAnnotation(DB.class) != null) + return true; } do {
