From 23af64475a9fb7365de2d7fb5fe3686815436a94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=87=91=E6=88=9F?= Date: Sat, 15 May 2021 11:04:45 +0800 Subject: [PATCH] support target exist checking of mock method --- .../agent/handler/MockClassHandler.java | 68 ++++++++++++++++++- .../agent/transformer/MockClassParser.java | 25 ++++--- .../transformer/TestableClassTransformer.java | 4 ++ .../testable/agent/util/AnnotationUtil.java | 16 ----- .../testable/agent/util/MethodUtil.java | 35 +++++++++- .../transformer/MockClassParserTest.java | 23 ------- .../testable/agent/util/MethodUtilTest.java | 31 ++++++++- .../exception/TargetNotExistException.java | 18 +++++ 8 files changed, 163 insertions(+), 57 deletions(-) delete mode 100644 testable-agent/src/test/java/com/alibaba/testable/agent/transformer/MockClassParserTest.java create mode 100644 testable-core/src/main/java/com/alibaba/testable/core/exception/TargetNotExistException.java diff --git a/testable-agent/src/main/java/com/alibaba/testable/agent/handler/MockClassHandler.java b/testable-agent/src/main/java/com/alibaba/testable/agent/handler/MockClassHandler.java index 1d02cfa..c4a4626 100644 --- a/testable-agent/src/main/java/com/alibaba/testable/agent/handler/MockClassHandler.java +++ b/testable-agent/src/main/java/com/alibaba/testable/agent/handler/MockClassHandler.java @@ -4,6 +4,7 @@ import com.alibaba.testable.agent.constant.ByteCodeConst; import com.alibaba.testable.agent.constant.ConstPool; import com.alibaba.testable.agent.tool.ImmutablePair; import com.alibaba.testable.agent.util.*; +import com.alibaba.testable.core.exception.TargetNotExistException; import com.alibaba.testable.core.model.MockScope; import com.alibaba.testable.core.util.MockAssociationUtil; import org.objectweb.asm.Label; @@ -15,7 +16,6 @@ import java.util.List; import static com.alibaba.testable.agent.constant.ByteCodeConst.TYPE_ARRAY; import static com.alibaba.testable.agent.constant.ByteCodeConst.TYPE_CLASS; import static com.alibaba.testable.agent.constant.ConstPool.CLASS_OBJECT; -import static com.alibaba.testable.agent.util.ClassUtil.toJavaStyleClassName; import static com.alibaba.testable.core.constant.ConstPool.CONSTRUCTOR; /** @@ -239,15 +239,77 @@ public class MockClassHandler extends BaseClassWithContextHandler { return false; } for (AnnotationNode an : mn.visibleAnnotations) { - if (isMockMethodAnnotation(an) && AnnotationUtil.isValidMockMethod(mn, an)) { + if (isMockMethodAnnotation(an)) { + checkTargetMethodExists(mn, an); return true; } else if (isMockConstructorAnnotation(an)) { + checkTargetConstructorExists(mn); return true; } } return false; } + private void checkTargetMethodExists(MethodNode mn, AnnotationNode an) { + String targetMethodName = AnnotationUtil.getAnnotationParameter(an, ConstPool.FIELD_TARGET_METHOD, null, String.class); + if (targetMethodName == null) { + targetMethodName = mn.name; + } + String targetClassName; + String targetMethodDesc; + Type targetClass = AnnotationUtil.getAnnotationParameter(an, ConstPool.FIELD_TARGET_CLASS, null, Type.class); + if (targetClass != null) { + targetClassName = targetClass.getClassName(); + targetMethodDesc = mn.desc; + checkMethodExists(mn.name, targetClassName, targetMethodName, targetMethodDesc); + } else if (mn.desc.charAt(1) == TYPE_CLASS) { + ImmutablePair parameterPair = MethodUtil.splitFirstAndRestParameters(mn.desc); + targetClassName = ClassUtil.toDotSeparatedName(parameterPair.left); + targetMethodDesc = parameterPair.right; + checkMethodExists(mn.name, targetClassName, targetMethodName, + MethodUtil.removeFirstParameter(targetMethodDesc)); + } else { + throw new TargetNotExistException("target class not exist", mn.name); + } + } + + private void checkMethodExists(String mockMethodName, String targetClassName, String targetMethodName, + String targetMethodDesc) { + ClassNode targetClassNode = ClassUtil.getClassNode(targetClassName); + if (targetClassNode == null) { + throw new TargetNotExistException("target class not found", mockMethodName); + } + boolean targetFound = false; + for (MethodNode targetMethodNode : targetClassNode.methods) { + if (targetMethodNode.name.equals(targetMethodName)) { + targetFound = true; + if (targetMethodNode.desc.equals(targetMethodDesc)) { + return; + } + } + } + throw new TargetNotExistException(targetFound ? + "mock method does not match original method" : "no such method in target class", mockMethodName); + } + + private void checkTargetConstructorExists(MethodNode mn) { + String returnType = MethodUtil.getReturnType(mn.desc); + if (returnType.charAt(0) != TYPE_CLASS) { + throw new TargetNotExistException("return type is not a class", mn.name); + } + ClassNode targetClassNode = ClassUtil.getClassNode(ClassUtil.toJavaStyleClassName(returnType)); + if (targetClassNode == null) { + throw new TargetNotExistException("target class not found", mn.name); + } + for (MethodNode targetMethodNode : targetClassNode.methods) { + if (CONSTRUCTOR.equals(targetMethodNode.name) && + MethodUtil.getParameters(targetMethodNode.desc).equals(MethodUtil.getParameters(mn.desc))) { + return; + } + } + throw new TargetNotExistException("no such constructor in target class", mn.name); + } + private boolean isMockConstructorAnnotation(AnnotationNode an) { return ClassUtil.toByteCodeClassName(ConstPool.MOCK_CONSTRUCTOR).equals(an.desc); } @@ -294,7 +356,7 @@ public class MockClassHandler extends BaseClassWithContextHandler { private boolean isMockForConstructor(MethodNode mn) { for (AnnotationNode an : mn.visibleAnnotations) { - String annotationName = toJavaStyleClassName(an.desc); + String annotationName = ClassUtil.toJavaStyleClassName(an.desc); if (ConstPool.MOCK_CONSTRUCTOR.equals(annotationName)) { return true; } else if (ConstPool.MOCK_METHOD.equals(annotationName)) { diff --git a/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/MockClassParser.java b/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/MockClassParser.java index cc0be3c..9cd9622 100644 --- a/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/MockClassParser.java +++ b/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/MockClassParser.java @@ -16,6 +16,7 @@ import org.objectweb.asm.tree.MethodNode; import java.util.ArrayList; import java.util.List; +import static com.alibaba.testable.agent.constant.ByteCodeConst.TYPE_CLASS; import static com.alibaba.testable.agent.constant.ConstPool.CLASS_OBJECT; import static com.alibaba.testable.agent.util.ClassUtil.toJavaStyleClassName; import static com.alibaba.testable.agent.util.MethodUtil.isStatic; @@ -89,7 +90,7 @@ public class MockClassParser { ClassUtil.toJavaStyleClassName(MethodUtil.getReturnType(mn.desc)), mn.desc)); } addMockConstructor(methodInfos, cn, mn); - } else if (fullClassName.equals(ConstPool.MOCK_METHOD) && AnnotationUtil.isValidMockMethod(mn, an)) { + } else if (fullClassName.equals(ConstPool.MOCK_METHOD) && isValidMockMethod(mn, an)) { if (LogUtil.isVerboseEnabled()) { LogUtil.verbose(" Mock method \"%s\" as \"%s\"", mn.name, MethodUtil.toJavaMethodDesc( getTargetMethodOwner(mn, an), getTargetMethodName(mn, an), getTargetMethodDesc(mn, an))); @@ -132,8 +133,8 @@ public class MockClassParser { boolean isStatic = isStatic(mn); if (targetType == null) { // "targetClass" unset, use first parameter as target class type - ImmutablePair methodDescPair = extractFirstParameter(mn.desc); - if (methodDescPair == null) { + ImmutablePair methodDescPair = MethodUtil.splitFirstAndRestParameters(mn.desc); + if (methodDescPair.left.isEmpty()) { return null; } return new MethodInfo(methodDescPair.left, targetMethod, methodDescPair.right, cn.name, mn.name, mn.desc, @@ -152,14 +153,18 @@ public class MockClassParser { } /** - * Split desc to "first parameter" and "desc of rest parameters" - * @param desc method desc + * Check is MockMethod annotation is used on a valid mock method + * @param mn mock method + * @param an MockMethod annotation + * @return valid or not */ - private ImmutablePair extractFirstParameter(String desc) { - // assume first parameter is a class - int pos = desc.indexOf(";"); - return pos < 0 ? null : ImmutablePair.of(desc.substring(2, pos), "(" + desc.substring(pos + 1)); + private boolean isValidMockMethod(MethodNode mn, AnnotationNode an) { + Type targetClass = AnnotationUtil.getAnnotationParameter(an, ConstPool.FIELD_TARGET_CLASS, null, Type.class); + if (targetClass != null) { + return true; + } + String firstParameter = MethodUtil.getFirstParameter(mn.desc); + return !firstParameter.isEmpty() && firstParameter.charAt(0) == TYPE_CLASS; } - } diff --git a/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/TestableClassTransformer.java b/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/TestableClassTransformer.java index 9f495b0..9bedd14 100644 --- a/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/TestableClassTransformer.java +++ b/testable-agent/src/main/java/com/alibaba/testable/agent/transformer/TestableClassTransformer.java @@ -7,6 +7,7 @@ import com.alibaba.testable.agent.handler.SourceClassHandler; import com.alibaba.testable.agent.handler.TestClassHandler; import com.alibaba.testable.agent.model.MethodInfo; import com.alibaba.testable.agent.util.*; +import com.alibaba.testable.core.exception.TargetNotExistException; import com.alibaba.testable.core.model.ClassType; import com.alibaba.testable.core.util.LogUtil; import com.alibaba.testable.core.util.MockAssociationUtil; @@ -85,6 +86,9 @@ public class TestableClassTransformer implements ClassFileTransformer { BytecodeUtil.dumpByte(className, GlobalConfig.getDumpPath(), bytes); } } + } catch (TargetNotExistException e) { + LogUtil.error("Invalid mock method %s::%s - %s", className, e.getMethodName(), e.getMessage()); + System.exit(0); } catch (Throwable t) { LogUtil.warn("Failed to transform class " + className); LogUtil.warn(t.toString()); diff --git a/testable-agent/src/main/java/com/alibaba/testable/agent/util/AnnotationUtil.java b/testable-agent/src/main/java/com/alibaba/testable/agent/util/AnnotationUtil.java index 12342f4..a09f01d 100644 --- a/testable-agent/src/main/java/com/alibaba/testable/agent/util/AnnotationUtil.java +++ b/testable-agent/src/main/java/com/alibaba/testable/agent/util/AnnotationUtil.java @@ -1,11 +1,6 @@ package com.alibaba.testable.agent.util; -import com.alibaba.testable.agent.constant.ConstPool; -import org.objectweb.asm.Type; import org.objectweb.asm.tree.AnnotationNode; -import org.objectweb.asm.tree.MethodNode; - -import static com.alibaba.testable.agent.constant.ByteCodeConst.TYPE_CLASS; /** * @author flin @@ -65,15 +60,4 @@ public class AnnotationUtil { return false; } - /** - * Check is MockMethod annotation is used on a valid mock method - * @param mn mock method - * @param an MockMethod annotation - * @return valid or not - */ - public static boolean isValidMockMethod(MethodNode mn, AnnotationNode an) { - Type targetClass = AnnotationUtil.getAnnotationParameter(an, ConstPool.FIELD_TARGET_CLASS, null, Type.class); - String firstParameter = MethodUtil.getFirstParameter(mn.desc); - return targetClass != null || firstParameter.charAt(0) == TYPE_CLASS; - } } diff --git a/testable-agent/src/main/java/com/alibaba/testable/agent/util/MethodUtil.java b/testable-agent/src/main/java/com/alibaba/testable/agent/util/MethodUtil.java index 94476cd..876e300 100644 --- a/testable-agent/src/main/java/com/alibaba/testable/agent/util/MethodUtil.java +++ b/testable-agent/src/main/java/com/alibaba/testable/agent/util/MethodUtil.java @@ -1,5 +1,6 @@ package com.alibaba.testable.agent.util; +import com.alibaba.testable.agent.tool.ImmutablePair; import org.objectweb.asm.tree.MethodNode; import java.util.ArrayList; @@ -11,6 +12,7 @@ import static org.objectweb.asm.Opcodes.ACC_STATIC; public class MethodUtil { private static final String COMMA_SPACE = ", "; + private static final int MINIMAL_DESC_LENGTH_OF_TWO_PARAMETERS_METHOD = 4; /** * Judge whether a method is static @@ -65,7 +67,7 @@ public class MethodUtil { } /** - * Parse method desc, fetch return value types + * Parse method desc, fetch return value type * @param desc method description * @return types of return value */ @@ -74,14 +76,41 @@ public class MethodUtil { return desc.substring(returnTypeEdge + 1); } + /** + * Parse method desc, fetch parameter types string + * @param desc method description + * @return parameter types + */ + public static Object getParameters(String desc) { + int returnTypeEdge = desc.lastIndexOf(PARAM_END); + return desc.substring(1, returnTypeEdge); + } + /** * Parse method desc, fetch first parameter type (assume first parameter is an object type) * @param desc method description * @return types of first parameter */ public static String getFirstParameter(String desc) { - int typeEdge = desc.indexOf(CLASS_END); - return typeEdge > 0 ? desc.substring(1, typeEdge + 1) : ""; + // assume first parameter is class type + return desc.substring(1, desc.indexOf(CLASS_END) + 1); + } + + /** + * Split desc to "first parameter" and "desc of rest parameters" + * @param desc method desc + * @return pair of [slash separated first parameter type, desc of rest parameters] + */ + public static ImmutablePair splitFirstAndRestParameters(String desc) { + if (desc.length() < MINIMAL_DESC_LENGTH_OF_TWO_PARAMETERS_METHOD) { + return ImmutablePair.of("", ""); + } + if (desc.charAt(1) != TYPE_CLASS) { + return ImmutablePair.of(desc.substring(1, 2), "(" + desc.substring(2)); + } + int pos = desc.indexOf(";"); + return pos < 0 ? ImmutablePair.of("", "") + : ImmutablePair.of(desc.substring(2, pos), "(" + desc.substring(pos + 1)); } /** diff --git a/testable-agent/src/test/java/com/alibaba/testable/agent/transformer/MockClassParserTest.java b/testable-agent/src/test/java/com/alibaba/testable/agent/transformer/MockClassParserTest.java deleted file mode 100644 index 1f778cb..0000000 --- a/testable-agent/src/test/java/com/alibaba/testable/agent/transformer/MockClassParserTest.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.alibaba.testable.agent.transformer; - -import com.alibaba.testable.agent.tool.ImmutablePair; -import com.alibaba.testable.core.tool.PrivateAccessor; -import org.junit.jupiter.api.Test; - -import static org.junit.jupiter.api.Assertions.*; - -class MockClassParserTest { - - private MockClassParser mockClassParser = new MockClassParser(); - - @Test - void should_split_parameters() { - ImmutablePair parameters = - PrivateAccessor.invoke(mockClassParser, "extractFirstParameter", "()"); - assertNull(parameters); - parameters = PrivateAccessor.invoke(mockClassParser, "extractFirstParameter", "(Lcom.alibaba.demo.Class;ILjava.lang.String;Z)"); - assertNotNull(parameters); - assertEquals("com.alibaba.demo.Class", parameters.left); - assertEquals("(ILjava.lang.String;Z)", parameters.right); - } -} diff --git a/testable-agent/src/test/java/com/alibaba/testable/agent/util/MethodUtilTest.java b/testable-agent/src/test/java/com/alibaba/testable/agent/util/MethodUtilTest.java index fbd0432..0030671 100644 --- a/testable-agent/src/test/java/com/alibaba/testable/agent/util/MethodUtilTest.java +++ b/testable-agent/src/test/java/com/alibaba/testable/agent/util/MethodUtilTest.java @@ -1,9 +1,11 @@ package com.alibaba.testable.agent.util; +import com.alibaba.testable.agent.tool.ImmutablePair; import com.alibaba.testable.core.tool.PrivateAccessor; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.function.Executable; -import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.*; class MethodUtilTest { @@ -31,11 +33,36 @@ class MethodUtilTest { assertEquals("[Ljava/lang/String;", MethodUtil.getReturnType("(Ljava/lang/String;)[Ljava/lang/String;")); } + @Test + void should_get_parameters() { + assertEquals("Ljava/lang/String;", MethodUtil.getParameters("(Ljava/lang/String;)V")); + assertEquals("I", MethodUtil.getParameters("(I)Ljava/lang/String;")); + assertEquals("", MethodUtil.getParameters("()Ljava/lang/String;")); + } + + @Test + void should_split_parameters() { + ImmutablePair parameters = MethodUtil.splitFirstAndRestParameters("()"); + assertEquals("", parameters.left); + assertEquals("", parameters.right); + parameters = MethodUtil.splitFirstAndRestParameters("(ZZILjava/lang/String;Z)"); + assertEquals("Z", parameters.left); + assertEquals("(ZILjava/lang/String;Z)", parameters.right); + parameters = MethodUtil.splitFirstAndRestParameters("(Lcom/alibaba/demo/Class;ILjava/lang/String;Z)"); + assertEquals("com/alibaba/demo/Class", parameters.left); + assertEquals("(ILjava/lang/String;Z)", parameters.right); + } + @Test void should_get_first_parameter() { assertEquals("Ljava/lang/String;", MethodUtil.getFirstParameter("(Ljava/lang/String;Ljava/lang/Object;I)V")); assertEquals("Ljava/lang/String;", MethodUtil.getFirstParameter("(Ljava/lang/String;)V")); - assertEquals("", MethodUtil.getFirstParameter("()V")); + assertThrows(IndexOutOfBoundsException.class, new Executable() { + @Override + public void execute() { + MethodUtil.getFirstParameter("()V"); + } + }); } @Test diff --git a/testable-core/src/main/java/com/alibaba/testable/core/exception/TargetNotExistException.java b/testable-core/src/main/java/com/alibaba/testable/core/exception/TargetNotExistException.java new file mode 100644 index 0000000..32e358f --- /dev/null +++ b/testable-core/src/main/java/com/alibaba/testable/core/exception/TargetNotExistException.java @@ -0,0 +1,18 @@ +package com.alibaba.testable.core.exception; + +/** + * @author flin + */ +public class TargetNotExistException extends RuntimeException { + + private String methodName; + + public TargetNotExistException(String message, String methodName) { + super(message); + this.methodName = methodName; + } + + public String getMethodName() { + return methodName; + } +}