Refactor logic of getting resource name in Sentinel annotation

- Enhance resolve name logic in MethodUtil
- Add test case for MethodUtil

Signed-off-by: Eric Zhao <sczyh16@gmail.com>

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;
     }
 

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<Method, String> methodNameMap = new HashMap<Method, String>();
+    private static final Map<Method, String> methodNameMap = new ConcurrentHashMap<Method, String>();
 
     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<Method, String> newMap = new HashMap<Method, String>(methodNameMap);
-                    newMap.put(method, methodName);
-                    methodNameMap = newMap;
+                    methodNameMap.put(method, methodName);
                 }
             }
         }
         return methodName;
     }
+
+    /**
+     * For test.
+     */
+    static void clearMethodMap() {
+        methodNameMap.clear();
+    }
 }

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);
+    }
+}

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
      */