tapestry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hls...@apache.org
Subject tapestry-5 git commit: TAP5-2421: VerifyError for methods that are advised and have Errors and RuntimeExceptions in the method signature
Date Mon, 01 Jun 2015 22:09:49 GMT
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 <hlship@apache.org>
Authored: Mon Jun 1 15:09:42 2015 -0700
Committer: Howard M. Lewis Ship <hlship@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");
+    }
+}


Mime
View raw message