Optimize advice avoid test errors caused by mock java.lang.reflect.Me…

…thod (#32603) * Optimize advice avoid test errors caused by mock java.lang.reflect.Method * Add java doc for TargetAdviceMethod
apache · Aug 21, 2024 · 2a48c6a · 2a48c6a
1 parent e6b29d7
commit 2a48c6a
Show file tree

Hide file tree

Showing 45 changed files with 165 additions and 140 deletions.
diff --git a/agent/api/src/main/java/org/apache/shardingsphere/agent/api/advice/TargetAdviceMethod.java b/agent/api/src/main/java/org/apache/shardingsphere/agent/api/advice/TargetAdviceMethod.java
@@ -0,0 +1,31 @@
+/*
+ * 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.shardingsphere.agent.api.advice;
+
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+
+/**
+ * Target advice method.
+ */
+@RequiredArgsConstructor
+@Getter
+public final class TargetAdviceMethod {
+
+    private final String name;
+}
diff --git a/...i/src/main/java/org/apache/shardingsphere/agent/api/advice/type/InstanceMethodAdvice.java b/...i/src/main/java/org/apache/shardingsphere/agent/api/advice/type/InstanceMethodAdvice.java
@@ -17,11 +17,10 @@
 
 package org.apache.shardingsphere.agent.api.advice.type;
 
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.api.advice.AgentAdvice;
 import org.apache.shardingsphere.agent.api.advice.TargetAdviceObject;
 
-import java.lang.reflect.Method;
-
 /**
  * Instance method advice.
  */
@@ -36,7 +35,7 @@ public interface InstanceMethodAdvice extends AgentAdvice {
      * @param args all method arguments
      * @param pluginType plugin type
      */
-    default void beforeMethod(final TargetAdviceObject target, final Method method, final Object[] args, String pluginType) {
+    default void beforeMethod(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, String pluginType) {
     }
 
     /**
@@ -49,7 +48,7 @@ default void beforeMethod(final TargetAdviceObject target, final Method method,
      * @param result original call result
      * @param pluginType plugin type
      */
-    default void afterMethod(final TargetAdviceObject target, final Method method, final Object[] args, final Object result, String pluginType) {
+    default void afterMethod(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final Object result, String pluginType) {
     }
 
     /**
@@ -61,6 +60,6 @@ default void afterMethod(final TargetAdviceObject target, final Method method, f
      * @param throwable exception from target method
      * @param pluginType plugin type
      */
-    default void onThrowing(final TargetAdviceObject target, final Method method, final Object[] args, final Throwable throwable, String pluginType) {
+    default void onThrowing(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final Throwable throwable, String pluginType) {
     }
 }
diff --git a/...api/src/main/java/org/apache/shardingsphere/agent/api/advice/type/StaticMethodAdvice.java b/...api/src/main/java/org/apache/shardingsphere/agent/api/advice/type/StaticMethodAdvice.java
@@ -17,10 +17,9 @@
 
 package org.apache.shardingsphere.agent.api.advice.type;
 
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.api.advice.AgentAdvice;
 
-import java.lang.reflect.Method;
-
 /**
  * Static method advice.
  */
@@ -35,7 +34,7 @@ public interface StaticMethodAdvice extends AgentAdvice {
      * @param args all method arguments
      * @param pluginType plugin type
      */
-    default void beforeMethod(final Class<?> clazz, final Method method, final Object[] args, String pluginType) {
+    default void beforeMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, String pluginType) {
     }
 
     /**
@@ -48,7 +47,7 @@ default void beforeMethod(final Class<?> clazz, final Method method, final Objec
      * @param result original call result
      * @param pluginType plugin type
      */
-    default void afterMethod(final Class<?> clazz, final Method method, final Object[] args, final Object result, String pluginType) {
+    default void afterMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Object result, String pluginType) {
     }
 
     /**
@@ -60,6 +59,6 @@ default void afterMethod(final Class<?> clazz, final Method method, final Object
      * @param throwable exception from target method
      * @param pluginType plugin type
      */
-    default void onThrowing(final Class<?> clazz, final Method method, final Object[] args, final Throwable throwable, String pluginType) {
+    default void onThrowing(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Throwable throwable, String pluginType) {
     }
 }
diff --git a/.../apache/shardingsphere/agent/core/advisor/executor/type/InstanceMethodAdviceExecutor.java b/.../apache/shardingsphere/agent/core/advisor/executor/type/InstanceMethodAdviceExecutor.java
@@ -28,6 +28,7 @@
 import net.bytebuddy.implementation.bind.annotation.SuperCall;
 import net.bytebuddy.implementation.bind.annotation.This;
 import net.bytebuddy.matcher.ElementMatchers;
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.api.plugin.AgentPluginEnable;
 import org.apache.shardingsphere.agent.api.advice.TargetAdviceObject;
 import org.apache.shardingsphere.agent.api.advice.type.InstanceMethodAdvice;
@@ -83,7 +84,7 @@ private void adviceBefore(final TargetAdviceObject target, final Method method,
             for (Entry<String, Collection<InstanceMethodAdvice>> entry : advices.entrySet()) {
                 for (InstanceMethodAdvice each : entry.getValue()) {
                     if (isPluginEnabled(each)) {
-                        each.beforeMethod(target, method, args, entry.getKey());
+                        each.beforeMethod(target, new TargetAdviceMethod(method.getName()), args, entry.getKey());
                     }
                 }
             }
@@ -99,7 +100,7 @@ private void adviceThrow(final TargetAdviceObject target, final Method method, f
             for (Entry<String, Collection<InstanceMethodAdvice>> entry : advices.entrySet()) {
                 for (InstanceMethodAdvice each : entry.getValue()) {
                     if (isPluginEnabled(each)) {
-                        each.onThrowing(target, method, args, ex, entry.getKey());
+                        each.onThrowing(target, new TargetAdviceMethod(method.getName()), args, ex, entry.getKey());
                     }
                 }
             }
@@ -115,7 +116,7 @@ private void adviceAfter(final TargetAdviceObject target, final Method method, f
             for (Entry<String, Collection<InstanceMethodAdvice>> entry : advices.entrySet()) {
                 for (InstanceMethodAdvice each : entry.getValue()) {
                     if (isPluginEnabled(each)) {
-                        each.afterMethod(target, method, args, result, entry.getKey());
+                        each.afterMethod(target, new TargetAdviceMethod(method.getName()), args, result, entry.getKey());
                     }
                 }
             }

diff --git a/...rg/apache/shardingsphere/agent/core/advisor/executor/type/StaticMethodAdviceExecutor.java b/...rg/apache/shardingsphere/agent/core/advisor/executor/type/StaticMethodAdviceExecutor.java
@@ -27,6 +27,7 @@
 import net.bytebuddy.implementation.bind.annotation.RuntimeType;
 import net.bytebuddy.implementation.bind.annotation.SuperCall;
 import net.bytebuddy.matcher.ElementMatchers;
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.api.plugin.AgentPluginEnable;
 import org.apache.shardingsphere.agent.api.advice.type.StaticMethodAdvice;
 import org.apache.shardingsphere.agent.core.advisor.executor.AdviceExecutor;
@@ -81,7 +82,7 @@ private void adviceBefore(final Class<?> klass, final Method method, final Objec
             for (Entry<String, Collection<StaticMethodAdvice>> entry : advices.entrySet()) {
                 for (StaticMethodAdvice each : entry.getValue()) {
                     if (isPluginEnabled(each)) {
-                        each.beforeMethod(klass, method, args, entry.getKey());
+                        each.beforeMethod(klass, new TargetAdviceMethod(method.getName()), args, entry.getKey());
                     }
                 }
             }
@@ -97,7 +98,7 @@ private void adviceThrow(final Class<?> klass, final Method method, final Object
             for (Entry<String, Collection<StaticMethodAdvice>> entry : advices.entrySet()) {
                 for (StaticMethodAdvice each : entry.getValue()) {
                     if (isPluginEnabled(each)) {
-                        each.onThrowing(klass, method, args, ex, entry.getKey());
+                        each.onThrowing(klass, new TargetAdviceMethod(method.getName()), args, ex, entry.getKey());
                     }
                 }
             }
@@ -113,7 +114,7 @@ private void adviceAfter(final Class<?> klass, final Method method, final Object
             for (Entry<String, Collection<StaticMethodAdvice>> entry : advices.entrySet()) {
                 for (StaticMethodAdvice each : entry.getValue()) {
                     if (isPluginEnabled(each)) {
-                        each.afterMethod(klass, method, args, result, entry.getKey());
+                        each.afterMethod(klass, new TargetAdviceMethod(method.getName()), args, result, entry.getKey());
                     }
                 }
             }

diff --git a/agent/core/src/test/java/org/apache/shardingsphere/fixture/advice/BarAdvice.java b/agent/core/src/test/java/org/apache/shardingsphere/fixture/advice/BarAdvice.java
@@ -17,12 +17,12 @@
 
 package org.apache.shardingsphere.fixture.advice;
 
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.api.advice.TargetAdviceObject;
 import org.apache.shardingsphere.agent.api.advice.type.ConstructorAdvice;
 import org.apache.shardingsphere.agent.api.advice.type.InstanceMethodAdvice;
 import org.apache.shardingsphere.agent.api.advice.type.StaticMethodAdvice;
 
-import java.lang.reflect.Method;
 import java.util.List;
 
 @SuppressWarnings("unchecked")
@@ -34,37 +34,37 @@ public void onConstructor(final TargetAdviceObject target, final Object[] args,
     }
 
     @Override
-    public void beforeMethod(final TargetAdviceObject target, final Method method, final Object[] args, final String pluginType) {
+    public void beforeMethod(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("bar before instance method");
     }
 
     @Override
-    public void beforeMethod(final Class<?> clazz, final Method method, final Object[] args, final String pluginType) {
+    public void beforeMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("bar before static method");
     }
 
     @Override
-    public void afterMethod(final TargetAdviceObject target, final Method method, final Object[] args, final Object result, final String pluginType) {
+    public void afterMethod(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final Object result, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("bar after instance method");
     }
 
     @Override
-    public void afterMethod(final Class<?> clazz, final Method method, final Object[] args, final Object result, final String pluginType) {
+    public void afterMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Object result, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("bar after static method");
     }
 
     @Override
-    public void onThrowing(final TargetAdviceObject target, final Method method, final Object[] args, final Throwable throwable, final String pluginType) {
+    public void onThrowing(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final Throwable throwable, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("bar throw instance method exception");
     }
 
     @Override
-    public void onThrowing(final Class<?> clazz, final Method method, final Object[] args, final Throwable throwable, final String pluginType) {
+    public void onThrowing(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Throwable throwable, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("bar throw static method exception");
     }

diff --git a/agent/core/src/test/java/org/apache/shardingsphere/fixture/advice/FooAdvice.java b/agent/core/src/test/java/org/apache/shardingsphere/fixture/advice/FooAdvice.java
@@ -17,12 +17,12 @@
 
 package org.apache.shardingsphere.fixture.advice;
 
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.api.advice.TargetAdviceObject;
 import org.apache.shardingsphere.agent.api.advice.type.ConstructorAdvice;
 import org.apache.shardingsphere.agent.api.advice.type.InstanceMethodAdvice;
 import org.apache.shardingsphere.agent.api.advice.type.StaticMethodAdvice;
 
-import java.lang.reflect.Method;
 import java.util.List;
 
 @SuppressWarnings("unchecked")
@@ -34,37 +34,37 @@ public void onConstructor(final TargetAdviceObject target, final Object[] args,
     }
 
     @Override
-    public void beforeMethod(final TargetAdviceObject target, final Method method, final Object[] args, final String pluginType) {
+    public void beforeMethod(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("foo before instance method");
     }
 
     @Override
-    public void beforeMethod(final Class<?> clazz, final Method method, final Object[] args, final String pluginType) {
+    public void beforeMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("foo before static method");
     }
 
     @Override
-    public void afterMethod(final TargetAdviceObject target, final Method method, final Object[] args, final Object result, final String pluginType) {
+    public void afterMethod(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final Object result, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("foo after instance method");
     }
 
     @Override
-    public void afterMethod(final Class<?> clazz, final Method method, final Object[] args, final Object result, final String pluginType) {
+    public void afterMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Object result, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("foo after static method");
     }
 
     @Override
-    public void onThrowing(final TargetAdviceObject target, final Method method, final Object[] args, final Throwable throwable, final String pluginType) {
+    public void onThrowing(final TargetAdviceObject target, final TargetAdviceMethod method, final Object[] args, final Throwable throwable, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("foo throw instance method exception");
     }
 
     @Override
-    public void onThrowing(final Class<?> clazz, final Method method, final Object[] args, final Throwable throwable, final String pluginType) {
+    public void onThrowing(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Throwable throwable, final String pluginType) {
         List<String> queue = (List<String>) args[0];
         queue.add("foo throw static method exception");
     }

diff --git a/...rc/main/java/org/apache/shardingsphere/agent/plugin/core/recorder/MethodTimeRecorder.java b/...rc/main/java/org/apache/shardingsphere/agent/plugin/core/recorder/MethodTimeRecorder.java
@@ -19,8 +19,8 @@
 
 import lombok.RequiredArgsConstructor;
 import org.apache.shardingsphere.agent.api.advice.AgentAdvice;
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 
-import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
 
@@ -39,7 +39,7 @@ public final class MethodTimeRecorder {
      *
      * @param method method to be recorded
      */
-    public void recordNow(final Method method) {
+    public void recordNow(final TargetAdviceMethod method) {
         CURRENT_RECORDER.get().put(getKey(method), System.currentTimeMillis());
     }
 
@@ -49,7 +49,7 @@ public void recordNow(final Method method) {
      * @param method method to be recorded
      * @return elapsed time
      */
-    public long getElapsedTimeAndClean(final Method method) {
+    public long getElapsedTimeAndClean(final TargetAdviceMethod method) {
         String key = getKey(method);
         try {
             return getElapsedTime(key);
@@ -58,7 +58,7 @@ public long getElapsedTimeAndClean(final Method method) {
         }
     }
 
-    private String getKey(final Method method) {
+    private String getKey(final TargetAdviceMethod method) {
         return String.format("%s@%s", adviceClass.getName(), method.getName());
     }
 

diff --git a/...est/java/org/apache/shardingsphere/agent/plugin/core/recorder/MethodTimeRecorderTest.java b/...est/java/org/apache/shardingsphere/agent/plugin/core/recorder/MethodTimeRecorderTest.java
@@ -18,6 +18,7 @@
 package org.apache.shardingsphere.agent.plugin.core.recorder;
 
 import org.apache.shardingsphere.agent.api.advice.AgentAdvice;
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.awaitility.Awaitility;
 import org.junit.jupiter.api.Test;
 
@@ -32,13 +33,15 @@ class MethodTimeRecorderTest {
     @Test
     void assertGetElapsedTimeAndCleanWithRecorded() throws NoSuchMethodException {
         MethodTimeRecorder methodTimeRecorder = new MethodTimeRecorder(AgentAdvice.class);
-        methodTimeRecorder.recordNow(Object.class.getDeclaredMethod("toString"));
+        TargetAdviceMethod method = new TargetAdviceMethod("test");
+        methodTimeRecorder.recordNow(method);
         Awaitility.await().pollDelay(5L, TimeUnit.MILLISECONDS).until(() -> true);
-        assertThat(methodTimeRecorder.getElapsedTimeAndClean(Object.class.getDeclaredMethod("toString")), greaterThanOrEqualTo(5L));
+        assertThat(methodTimeRecorder.getElapsedTimeAndClean(method), greaterThanOrEqualTo(5L));
     }
 
     @Test
     void assertGetElapsedTimeAndCleanWithoutRecorded() throws NoSuchMethodException {
-        assertThat(new MethodTimeRecorder(AgentAdvice.class).getElapsedTimeAndClean(Object.class.getDeclaredMethod("toString")), is(0L));
+        TargetAdviceMethod method = new TargetAdviceMethod("test");
+        assertThat(new MethodTimeRecorder(AgentAdvice.class).getElapsedTimeAndClean(method), is(0L));
     }
 }
diff --git a/...apache/shardingsphere/agent/plugin/logging/file/advice/MetaDataContextsFactoryAdvice.java b/...apache/shardingsphere/agent/plugin/logging/file/advice/MetaDataContextsFactoryAdvice.java
@@ -18,11 +18,10 @@
 package org.apache.shardingsphere.agent.plugin.logging.file.advice;
 
 import lombok.extern.slf4j.Slf4j;
+import org.apache.shardingsphere.agent.api.advice.TargetAdviceMethod;
 import org.apache.shardingsphere.agent.plugin.core.advice.AbstractStaticMethodAdvice;
 import org.apache.shardingsphere.agent.plugin.core.recorder.MethodTimeRecorder;
 
-import java.lang.reflect.Method;
-
 /**
  * Meta data contexts factory advice.
  */
@@ -32,12 +31,12 @@ public final class MetaDataContextsFactoryAdvice extends AbstractStaticMethodAdv
     private final MethodTimeRecorder methodTimeRecorder = new MethodTimeRecorder(MetaDataContextsFactoryAdvice.class);
 
     @Override
-    public void beforeMethod(final Class<?> clazz, final Method method, final Object[] args, final String pluginType) {
+    public void beforeMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final String pluginType) {
         methodTimeRecorder.recordNow(method);
     }
 
     @Override
-    public void afterMethod(final Class<?> clazz, final Method method, final Object[] args, final Object result, final String pluginType) {
+    public void afterMethod(final Class<?> clazz, final TargetAdviceMethod method, final Object[] args, final Object result, final String pluginType) {
         log.info("Build meta data contexts finished, cost {} milliseconds.", methodTimeRecorder.getElapsedTimeAndClean(method));
     }
 }