diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slotchain/MethodResourceWrapper.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slotchain/MethodResourceWrapper.java index 4d8b4c33..91e20a4c 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slotchain/MethodResourceWrapper.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/slotchain/MethodResourceWrapper.java @@ -32,7 +32,7 @@ public class MethodResourceWrapper extends ResourceWrapper { public MethodResourceWrapper(Method method, EntryType type) { this.method = method; - this.name = MethodUtil.getMethodName(method); + this.name = MethodUtil.resolveMethodName(method); this.type = type; } diff --git a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/MethodUtil.java b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/MethodUtil.java index e8bc894d..5f4a0faa 100755 --- a/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/MethodUtil.java +++ b/sentinel-core/src/main/java/com/alibaba/csp/sentinel/util/MethodUtil.java @@ -16,9 +16,6 @@ package com.alibaba.csp.sentinel.util; import java.lang.reflect.Method; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -29,14 +26,20 @@ import java.util.concurrent.ConcurrentHashMap; */ public final class MethodUtil { - private static volatile Map methodNameMap = new HashMap(); + private static final Map methodNameMap = new ConcurrentHashMap(); private static final Object LOCK = new Object(); /** - * Parse and get the method name. + * Parse and resolve the method name, then cache to the map. + * + * @param method method instance + * @return resolved method name */ - public static String getMethodName(Method method) { + public static String resolveMethodName(Method method) { + if (method == null) { + throw new IllegalArgumentException("Null method"); + } String methodName = methodNameMap.get(method); if (methodName == null) { synchronized (LOCK) { @@ -52,7 +55,7 @@ public final class MethodUtil { int paramPos = 0; for (Class clazz : params) { - sb.append(clazz.getName()); + sb.append(clazz.getCanonicalName()); if (++paramPos < params.length) { sb.append(","); } @@ -60,12 +63,17 @@ public final class MethodUtil { sb.append(")"); methodName = sb.toString(); - HashMap newMap = new HashMap(methodNameMap); - newMap.put(method, methodName); - methodNameMap = newMap; + methodNameMap.put(method, methodName); } } } return methodName; } + + /** + * For test. + */ + static void clearMethodMap() { + methodNameMap.clear(); + } } diff --git a/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/MethodUtilTest.java b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/MethodUtilTest.java new file mode 100644 index 00000000..ce70663c --- /dev/null +++ b/sentinel-core/src/test/java/com/alibaba/csp/sentinel/util/MethodUtilTest.java @@ -0,0 +1,63 @@ +package com.alibaba.csp.sentinel.util; + +import java.lang.reflect.Method; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Test cases for {@link MethodUtil}. + * + * @author Eric Zhao + */ +public class MethodUtilTest { + + @Before + public void setUp() { + MethodUtil.clearMethodMap(); + } + + @After + public void cleanUp() { + MethodUtil.clearMethodMap(); + } + + @Test + public void testResolveMethodName() { + Method fooMethod = null; + for (Method m : GoodClass.class.getMethods()) { + if (m.getName().contains("foo")) { + fooMethod = m; + break; + } + } + assertNotNull(fooMethod); + assertEquals("com.alibaba.csp.sentinel.util.MethodUtilTest$GoodClass:foo(long[],java.lang.String,java.lang.Integer[])", + MethodUtil.resolveMethodName(fooMethod)); + + Method bazMethod = null; + for (Method m : GoodClass.class.getMethods()) { + if (m.getName().contains("baz")) { + bazMethod = m; + break; + } + } + assertNotNull(bazMethod); + assertEquals("com.alibaba.csp.sentinel.util.MethodUtilTest$GoodClass:baz(double)", + MethodUtil.resolveMethodName(bazMethod)); + } + + interface GoodClass { + void foo(long[] p1, String p2, Integer[] p3); + + String baz(double a); + } + + @Test(expected = IllegalArgumentException.class) + public void testResolveNullMethod() { + MethodUtil.resolveMethodName(null); + } +} \ No newline at end of file diff --git a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java index c955b091..0c3ae621 100644 --- a/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java +++ b/sentinel-extension/sentinel-annotation-aspectj/src/main/java/com/alibaba/csp/sentinel/annotation/aspectj/SentinelResourceAspect.java @@ -26,6 +26,7 @@ import com.alibaba.csp.sentinel.annotation.SentinelResource; import com.alibaba.csp.sentinel.context.ContextUtil; import com.alibaba.csp.sentinel.slots.block.BlockException; import com.alibaba.csp.sentinel.slots.block.degrade.DegradeException; +import com.alibaba.csp.sentinel.util.MethodUtil; import com.alibaba.csp.sentinel.util.StringUtil; import org.aspectj.lang.ProceedingJoinPoint; @@ -76,28 +77,16 @@ public class SentinelResourceAspect { ContextUtil.exit(); } } - + private String getResourceName(String resourceName, Method method) { - if(StringUtil.isNotBlank(resourceName)){ + // If resource name is present in annotation, use this value. + if (StringUtil.isNotBlank(resourceName)) { return resourceName; } - StringBuilder buf = new StringBuilder(64); - buf.append(method.getDeclaringClass().getName()) - .append(":") - .append(method.getName()) - .append("("); - boolean isFirst = true; - for (Class clazz : method.getParameterTypes()) { - if (!isFirst) { - buf.append(","); - } - buf.append(clazz.getName()); - isFirst = false; - } - buf.append(")"); - return buf.toString(); + // Parse name of target method. + return MethodUtil.resolveMethodName(method); } - + private Object handleBlockException(ProceedingJoinPoint pjp, SentinelResource annotation, BlockException ex) throws Exception { // Execute fallback for degrading if configured. @@ -224,7 +213,8 @@ public class SentinelResourceAspect { MethodSignature signature = (MethodSignature)joinPoint.getSignature(); Class targetClass = joinPoint.getTarget().getClass(); - Method method = getDeclaredMethodFor(targetClass, signature.getName(), signature.getMethod().getParameterTypes()); + Method method = getDeclaredMethodFor(targetClass, signature.getName(), + signature.getMethod().getParameterTypes()); if (method == null) { throw new IllegalStateException("Cannot resolve target method: " + signature.getMethod().getName()); } @@ -235,8 +225,8 @@ public class SentinelResourceAspect { * Get declared method with provided name and parameterTypes in given class and its super classes. * All parameters should be valid. * - * @param clazz class where the method is located - * @param name method name + * @param clazz class where the method is located + * @param name method name * @param parameterTypes method parameter type list * @return resolved method, null if not found */