You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2015/06/02 00:09:49 UTC
tapestry-5 git commit: TAP5-2421: VerifyError for methods that are
advised and have Errors and RuntimeExceptions in the method signature
Repository: tapestry-5
Updated Branches:
refs/heads/master e25fac7e4 -> 1dbcb377e
TAP5-2421: VerifyError for methods that are advised and have Errors and RuntimeExceptions in the method signature
Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/1dbcb377
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/1dbcb377
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/1dbcb377
Branch: refs/heads/master
Commit: 1dbcb377e5401abc38bf0e31255da6b83edc4b6f
Parents: e25fac7
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Jun 1 15:09:42 2015 -0700
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Jun 1 15:09:42 2015 -0700
----------------------------------------------------------------------
.../internal/plastic/MethodAdviceManager.java | 23 ++--
.../internal/plastic/PlasticClassPool.java | 105 +++++++++++++------
.../tapestry5/plastic/MethodAdviceTests.groovy | 44 ++++++++
.../java/testsubjects/DeclaredExceptions.java | 21 ++++
4 files changed, 151 insertions(+), 42 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/1dbcb377/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
index 551fcde..6c977b5 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/MethodAdviceManager.java
@@ -69,7 +69,7 @@ class MethodAdviceManager
invocationClassNode.visit(PlasticConstants.DEFAULT_VERSION_OPCODE, Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, plasticClass.nameCache.toInternalName(invocationClassName),
null, PlasticClassImpl.ABSTRACT_METHOD_INVOCATION_INTERNAL_NAME, new String[]
- {plasticClass.nameCache.toInternalName(MethodInvocation.class)});
+ {plasticClass.nameCache.toInternalName(MethodInvocation.class)});
constructorTypes = createFieldsAndConstructor();
@@ -341,18 +341,21 @@ class MethodAdviceManager
for (String exceptionName : description.checkedExceptionTypes)
{
- block.addCatch(exceptionName, new InstructionBuilderCallback()
+ if (plasticClass.pool.isCheckedException(exceptionName))
{
- @Override
- public void doBuild(InstructionBuilder builder)
+ block.addCatch(exceptionName, new InstructionBuilderCallback()
{
- builder.loadThis().swap();
- builder.invoke(AbstractMethodInvocation.class, MethodInvocation.class,
- "setCheckedException", Exception.class);
+ @Override
+ public void doBuild(InstructionBuilder builder)
+ {
+ builder.loadThis().swap();
+ builder.invoke(AbstractMethodInvocation.class, MethodInvocation.class,
+ "setCheckedException", Exception.class);
- builder.returnResult();
- }
- });
+ builder.returnResult();
+ }
+ });
+ }
}
}
});
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/1dbcb377/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
index 01e8359..b8e2bb4 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassPool.java
@@ -37,14 +37,16 @@ import java.util.concurrent.CopyOnWriteArrayList;
@SuppressWarnings("rawtypes")
public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticClassListenerHub
{
- private static final Logger LOGGER = LoggerFactory.getLogger(PlasticClassPool.class);
-
+ private static final Logger LOGGER = LoggerFactory.getLogger(PlasticClassPool.class);
+
final PlasticClassLoader loader;
private final PlasticManagerDelegate delegate;
private final Set<String> controlledPackages;
+ private final Map<String, Boolean> checkedExceptionCache = new HashMap<String, Boolean>();
+
// Would use Deque, but that's added in 1.6 and we're still striving for 1.5 code compatibility.
@@ -95,7 +97,7 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
private final Map<String, FieldInstrumentations> instrumentations = PlasticInternalUtils.newMap();
private final Map<String, String> transformedClassNameToImplementationClassName = PlasticInternalUtils.newMap();
-
+
private final FieldInstrumentations placeholder = new FieldInstrumentations(null);
@@ -106,10 +108,14 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
* Creates the pool with a set of controlled packages; all classes in the controlled packages are loaded by the
* pool's class loader, and all top-level classes in the controlled packages are transformed via the delegate.
*
- * @param parentLoader typically, the Thread's context class loader
- * @param delegate responsible for end stages of transforming top-level classes
- * @param controlledPackages set of package names (note: retained, not copied)
- * @param options used when transforming classes
+ * @param parentLoader
+ * typically, the Thread's context class loader
+ * @param delegate
+ * responsible for end stages of transforming top-level classes
+ * @param controlledPackages
+ * set of package names (note: retained, not copied)
+ * @param options
+ * used when transforming classes
*/
public PlasticClassPool(ClassLoader parentLoader, PlasticManagerDelegate delegate, Set<String> controlledPackages,
Set<TransformationOption> options)
@@ -454,16 +460,20 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
String baseClassName = PlasticInternalUtils.toClassName(classNode.superName);
instrumentations.put(classNode.name, new FieldInstrumentations(classNode.superName));
-
+
// TODO: check whether second parameter should really be null
- return createTransformation(baseClassName, classNode, null, false);
+ return createTransformation(baseClassName, classNode, null, false);
}
/**
- * @param baseClassName class from which the transformed class extends
- * @param classNode node for the class
- * @param implementationClassNode node for the implementation class. May be null.
- * @param proxy if true, the class is a new empty class; if false an existing class that's being transformed
+ * @param baseClassName
+ * class from which the transformed class extends
+ * @param classNode
+ * node for the class
+ * @param implementationClassNode
+ * node for the implementation class. May be null.
+ * @param proxy
+ * if true, the class is a new empty class; if false an existing class that's being transformed
* @throws ClassNotFoundException
*/
private InternalPlasticClassTransformation createTransformation(String baseClassName, ClassNode classNode, ClassNode implementationClassNode, boolean proxy)
@@ -489,7 +499,8 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
* Constructs a class node by reading the raw bytecode for a class and instantiating a ClassNode
* (via {@link ClassReader#accept(org.apache.tapestry5.internal.plastic.asm.ClassVisitor, int)}).
*
- * @param className fully qualified class name
+ * @param className
+ * fully qualified class name
* @return corresponding ClassNode
*/
public ClassNode constructClassNodeFromBytecode(String className)
@@ -509,10 +520,11 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
return PlasticInternalUtils.readBytecodeForClass(parentClassLoader, className, true);
}
- public PlasticClassTransformation createTransformation(String baseClassName, String newClassName) {
+ public PlasticClassTransformation createTransformation(String baseClassName, String newClassName)
+ {
return createTransformation(baseClassName, newClassName, null);
}
-
+
public PlasticClassTransformation createTransformation(String baseClassName, String newClassName, String implementationClassName)
{
try
@@ -522,9 +534,9 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
final String internalNewClassNameinternalName = PlasticInternalUtils.toInternalName(newClassName);
final String internalBaseClassName = PlasticInternalUtils.toInternalName(baseClassName);
newClassNode.visit(PlasticConstants.DEFAULT_VERSION_OPCODE, ACC_PUBLIC, internalNewClassNameinternalName, null, internalBaseClassName, null);
-
+
ClassNode implementationClassNode = null;
-
+
if (implementationClassName != null)
{
// When decorating or advising a service, implementationClassName is the name
@@ -534,25 +546,27 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
// for each proxy, even a proxy around a proxy.
if (transformedClassNameToImplementationClassName.containsKey(implementationClassName))
{
- implementationClassName =
+ implementationClassName =
transformedClassNameToImplementationClassName.get(implementationClassName);
}
-
- if (!implementationClassName.startsWith("com.sun.proxy")) {
-
- try {
+
+ if (!implementationClassName.startsWith("com.sun.proxy"))
+ {
+
+ try
+ {
implementationClassNode = readClassNode(implementationClassName);
- }
- catch (IOException e) {
- LOGGER.warn(String.format("Unable to load class %s as the implementation of service %s",
+ } catch (IOException e)
+ {
+ LOGGER.warn(String.format("Unable to load class %s as the implementation of service %s",
implementationClassName, baseClassName));
// Go on. Probably a proxy class
}
-
+
}
-
+
transformedClassNameToImplementationClassName.put(newClassName, implementationClassName);
-
+
}
return createTransformation(baseClassName, newClassNode, implementationClassNode, true);
@@ -561,14 +575,14 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
throw new RuntimeException(String.format("Unable to create class %s as sub-class of %s: %s", newClassName,
baseClassName, PlasticInternalUtils.toMessage(ex)), ex);
}
-
+
}
private ClassNode readClassNode(String className) throws IOException
{
return readClassNode(className, getClassLoader());
}
-
+
static ClassNode readClassNode(String className, ClassLoader classLoader) throws IOException
{
ClassNode classNode = new ClassNode();
@@ -580,7 +594,7 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
bis.close();
classReader.accept(classNode, 0);
return classNode;
-
+
}
public ClassInstantiator getClassInstantiator(String className)
@@ -738,4 +752,31 @@ public class PlasticClassPool implements ClassLoaderDelegate, Opcodes, PlasticCl
instrumentations.get(classInternalName).write.put(fieldName, fi);
}
+ boolean isCheckedException(String exceptionName)
+ {
+ Boolean cached = checkedExceptionCache.get(exceptionName);
+
+ if (cached != null)
+ {
+ return cached;
+ }
+
+ try
+ {
+ Class asClass = getClassLoader().loadClass(exceptionName);
+
+
+ boolean checked = !(Error.class.isAssignableFrom(asClass) ||
+ RuntimeException.class.isAssignableFrom(asClass));
+
+ checkedExceptionCache.put(exceptionName, checked);
+
+ return checked;
+ } catch (Exception e)
+ {
+ throw new RuntimeException(e);
+ }
+ }
}
+
+
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/1dbcb377/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
----------------------------------------------------------------------
diff --git a/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy b/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
index bd20978..a5296c5 100644
--- a/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
+++ b/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
@@ -336,4 +336,48 @@ class MethodAdviceTests extends AbstractPlasticSpecification {
m.isOverride() == true
}
+ def "ignores Error and RuntimeException in throws declaration"() {
+
+ def mgr = createMgr({ PlasticClass pc ->
+ def methods = pc.getMethodsWithAnnotation(MethodAnnotation)
+
+ methods.each { PlasticMethod m ->
+ m.addAdvice({ MethodInvocation iv ->
+ iv.proceed()
+ } as MethodAdvice)
+ }
+ } as PlasticClassTransformer)
+
+ when:
+
+ def o = mgr.getClassInstantiator(testsubjects.DeclaredExceptions.name).newInstance()
+
+ o.throwsRuntime()
+
+ then:
+
+ def e = thrown(RuntimeException)
+
+ e.message == "throwsRuntime";
+
+ when:
+
+ o.throwsError()
+
+ then:
+
+ e = thrown(Error)
+
+ e.message == "throwsError"
+
+ when:
+
+ o.throwsException()
+
+ then:
+
+ e = thrown(Exception)
+
+ e.message == "throwsException"
+ }
}
http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/1dbcb377/plastic/src/test/java/testsubjects/DeclaredExceptions.java
----------------------------------------------------------------------
diff --git a/plastic/src/test/java/testsubjects/DeclaredExceptions.java b/plastic/src/test/java/testsubjects/DeclaredExceptions.java
new file mode 100644
index 0000000..83e52f0
--- /dev/null
+++ b/plastic/src/test/java/testsubjects/DeclaredExceptions.java
@@ -0,0 +1,21 @@
+package testsubjects;
+
+import testannotations.MethodAnnotation;
+
+public class DeclaredExceptions
+{
+ @MethodAnnotation
+ public void throwsRuntime() throws RuntimeException {
+ throw new RuntimeException("throwsRuntime");
+ }
+
+ @MethodAnnotation
+ public void throwsError() throws Error {
+ throw new Error("throwsError");
+ }
+
+ @MethodAnnotation
+ public void throwsException() throws Exception {
+ throw new Exception("throwsException");
+ }
+}