Support private blockHandler/fallback method for @SentinelResource an…

…notation (alibaba#2163)
shivagowda · Aug 14, 2021 · 3734d75 · 3734d75
1 parent babe5a7
commit 3734d75
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 27 deletions.
diff --git a/.../main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java b/.../main/java/com/alibaba/csp/sentinel/annotation/aspectj/AbstractSentinelAspectSupport.java
@@ -102,15 +102,7 @@ protected Object handleFallback(ProceedingJoinPoint pjp, String fallback, String
                 args[args.length - 1] = ex;
             }
 
-            try {
-                if (isStatic(fallbackMethod)) {
-                    return fallbackMethod.invoke(null, args);
-                }
-                return fallbackMethod.invoke(pjp.getTarget(), args);
-            } catch (InvocationTargetException e) {
-                // throw the actual exception
-                throw e.getTargetException();
-            }
+            return invoke(pjp, fallbackMethod, args);
         }
         // If fallback is absent, we'll try the defaultFallback if provided.
         return handleDefaultFallback(pjp, defaultFallback, fallbackClass, ex);
@@ -123,15 +115,7 @@ protected Object handleDefaultFallback(ProceedingJoinPoint pjp, String defaultFa
         if (fallbackMethod != null) {
             // Construct args.
             Object[] args = fallbackMethod.getParameterTypes().length == 0 ? new Object[0] : new Object[] {ex};
-            try {
-                if (isStatic(fallbackMethod)) {
-                    return fallbackMethod.invoke(null, args);
-                }
-                return fallbackMethod.invoke(pjp.getTarget(), args);
-            } catch (InvocationTargetException e) {
-                // throw the actual exception
-                throw e.getTargetException();
-            }
+            return invoke(pjp, fallbackMethod, args);
         }
 
         // If no any fallback is present, then directly throw the exception.
@@ -149,21 +133,44 @@ protected Object handleBlockException(ProceedingJoinPoint pjp, SentinelResource
             // Construct args.
             Object[] args = Arrays.copyOf(originArgs, originArgs.length + 1);
             args[args.length - 1] = ex;
-            try {
-                if (isStatic(blockHandlerMethod)) {
-                    return blockHandlerMethod.invoke(null, args);
-                }
-                return blockHandlerMethod.invoke(pjp.getTarget(), args);
-            } catch (InvocationTargetException e) {
-                // throw the actual exception
-                throw e.getTargetException();
-            }
+            return invoke(pjp, blockHandlerMethod, args);
         }
 
         // If no block handler is present, then go to fallback.
         return handleFallback(pjp, annotation, ex);
     }
 
+    private Object invoke(ProceedingJoinPoint pjp, Method method, Object[] args) throws Throwable {
+        try {
+            if (!method.isAccessible()) {
+                makeAccessible(method);
+            }
+            if (isStatic(method)) {
+                return method.invoke(null, args);
+            }
+            return method.invoke(pjp.getTarget(), args);
+        } catch (InvocationTargetException e) {
+            // throw the actual exception
+            throw e.getTargetException();
+        }
+    }
+
+    /**
+     * Make the given method accessible, explicitly setting it accessible if
+     * necessary. The {@code setAccessible(true)} method is only called
+     * when actually necessary, to avoid unnecessary conflicts with a JVM
+     * SecurityManager (if active).
+     * @param method the method to make accessible
+     * @see java.lang.reflect.Method#setAccessible
+     */
+    private static void makeAccessible(Method method) {
+        boolean isNotPublic = !Modifier.isPublic(method.getModifiers()) ||
+                !Modifier.isPublic(method.getDeclaringClass().getModifiers());
+        if (isNotPublic && !method.isAccessible()) {
+            method.setAccessible(true);
+        }
+    }
+
     private Method extractFallbackMethod(ProceedingJoinPoint pjp, String fallbackName, Class<?>[] locationClass) {
         if (StringUtil.isBlank(fallbackName)) {
             return null;

diff --git a/...libaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java b/...libaba/csp/sentinel/annotation/aspectj/integration/SentinelAnnotationIntegrationTest.java
@@ -208,6 +208,28 @@ public void testClassLevelDefaultFallbackWithSingleParam() {
         assertThat(cn1.blockQps()).isZero();
     }
 
+    @Test
+    public void testFallBackPrivateMethod() throws Exception {
+        String resourceName = "apiFooWithFallback";
+        ClusterNode cn = ClusterBuilderSlot.getClusterNode(resourceName);
+
+        try {
+            fooService.fooWithPrivateFallback(5758);
+            fail("should not reach here");
+        } catch (Exception ex) {
+            // Should not be traced.
+            assertThat(cn.exceptionQps()).isZero();
+        }
+
+        assertThat(fooService.fooWithPrivateFallback(5763)).isEqualTo("EEE...");
+
+        // Test for blockHandler
+        FlowRuleManager.loadRules(Collections.singletonList(
+                new FlowRule(resourceName).setCount(0)
+        ));
+        assertThat(fooService.fooWithPrivateFallback(2221)).isEqualTo("Oops, 2221");
+    }
+
     @Before
     public void setUp() throws Exception {
         FlowRuleManager.loadRules(new ArrayList<FlowRule>());

diff --git a/...test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java b/...test/java/com/alibaba/csp/sentinel/annotation/aspectj/integration/service/FooService.java
@@ -76,11 +76,31 @@ public String baz(String name) {
         return "cheers, " + name;
     }
 
+    @SentinelResource(value = "apiFooWithFallback", blockHandler = "fooBlockHandlerPrivate", fallback = "fooFallbackFuncPrivate",
+            exceptionsToTrace = {IllegalArgumentException.class})
+    public String fooWithPrivateFallback(int i) throws Exception {
+        if (i == 5758) {
+            throw new IllegalAccessException();
+        }
+        if (i == 5763) {
+            throw new IllegalArgumentException();
+        }
+        return "Hello for " + i;
+    }
+
     public String fooBlockHandler(int i, BlockException ex) {
         return "Oops, " + i;
     }
 
     public String fooFallbackFunc(int i) {
         return "eee...";
     }
+
+    private String fooFallbackFuncPrivate(int i) {
+        return "EEE...";
+    }
+
+    private String fooBlockHandlerPrivate(int i, BlockException ex) {
+        return "Oops, " + i;
+    }
 }