You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by th...@apache.org on 2015/06/19 04:51:39 UTC

[4/5] tapestry-5 git commit: TAP5-2029: a best-effort attempt to deal with generic methods and their annotations in service proxies.

TAP5-2029: a best-effort attempt to deal with generic methods and their
annotations in service proxies.

Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/72334aa6
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/72334aa6
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/72334aa6

Branch: refs/heads/master
Commit: 72334aa632d6ccade195af45014f7903fc0765fa
Parents: 8c8a13a
Author: Thiago H. de Paula Figueiredo <th...@apache.org>
Authored: Thu Jun 18 23:50:12 2015 -0300
Committer: Thiago H. de Paula Figueiredo <th...@apache.org>
Committed: Thu Jun 18 23:50:12 2015 -0300

----------------------------------------------------------------------
 .../internal/plastic/asm/tree/MethodNode.java   |   8 ++
 .../internal/plastic/PlasticClassImpl.java      | 136 +++++++++++++++----
 .../MethodInvocationGetAnnotationSpec.groovy    |  46 ++++++-
 .../tapestry5/ioc/internal/AdviceModule.java    |   7 +
 4 files changed, 168 insertions(+), 29 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/72334aa6/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/MethodNode.java
----------------------------------------------------------------------
diff --git a/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/MethodNode.java b/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/MethodNode.java
index ce959df..48115c2 100644
--- a/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/MethodNode.java
+++ b/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/MethodNode.java
@@ -29,6 +29,7 @@
  */
 package org.apache.tapestry5.internal.plastic.asm.tree;
 
+import java.lang.reflect.Modifier;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
@@ -835,4 +836,11 @@ public class MethodNode extends MethodVisitor {
         }
         mv.visitEnd();
     }
+
+	@Override
+	public String toString() {
+		return "MethodNode [name=" + name + ", bridge=" + Modifier.isVolatile(access)
+				+ ", desc=" + desc + "]";
+	}
+    
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/72334aa6/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
----------------------------------------------------------------------
diff --git a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
index 1bb08f1..e224bd1 100644
--- a/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
+++ b/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
@@ -20,10 +20,13 @@ import org.apache.tapestry5.plastic.*;
 
 import java.io.IOException;
 import java.lang.annotation.Annotation;
+import java.lang.reflect.Array;
 import java.lang.reflect.Constructor;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.*;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 @SuppressWarnings("all")
 public class PlasticClassImpl extends Lockable implements PlasticClass, InternalPlasticClassTransformation, Opcodes
@@ -398,40 +401,121 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
 
     }
     
+    private static String getParametersDesc(MethodNode methodNode) {
+        return methodNode.desc.substring(methodNode.desc.indexOf('(') + 1, methodNode.desc.lastIndexOf(')'));
+    }
 
-    private static void addMethodAndParameterAnnotationsFromExistingClass(MethodNode methodNode, ClassNode source)
-    {
-        if (source != null)
+    private static MethodNode findExactMatchMethod(MethodNode methodNode, ClassNode source) {
+
+    	MethodNode found = null;
+    	
+    	final String methodDescription = getParametersDesc(methodNode);
+    	
+        for (MethodNode implementationMethodNode : source.methods)
         {
+        	
+            final String implementationMethodDescription = getParametersDesc(implementationMethodNode);
+            if (methodNode.name.equals(implementationMethodNode.name) && 
+            		// We don't want synthetic methods.
+            		((implementationMethodNode.access & Opcodes.ACC_SYNTHETIC) == 0)
+            		&& (methodDescription.equals(implementationMethodDescription))) 
+            {
+            	found = implementationMethodNode;
+            	break;
+            }
+        }
+        
+        return found;
 
-        	MethodNode candidate = null;
+    }
+    
+    private static List<Class> getJavaParameterTypes(MethodNode methodNode) {
+        final ClassLoader classLoader = PlasticInternalUtils.class.getClassLoader();
+        Type[] parameterTypes = Type.getArgumentTypes(methodNode.desc);
+    	List<Class> list = new ArrayList<Class>();
+    	for (Type type : parameterTypes)
+        {
+            try
+            {
+                list.add(PlasticInternalUtils.toClass(classLoader, type.getClassName()));
+            }
+            catch (ClassNotFoundException e)
+            {
+                throw new RuntimeException(e); // shouldn't happen anyway
+            }
+        }
+    	return list;
+    }
+    
+    /**
+     * Returns the first method which matches the given methodNode.
+     * FIXME: this may not find the correct method if the correct one is declared after
+     * another in which all parameters are supertypes of the parameters of methodNode.
+     * To solve this, we would need to dig way deeper than we have time for this.
+     * @param methodNode
+     * @param classNode
+     * @return
+     */
+    private static MethodNode findGenericMethod(MethodNode methodNode, ClassNode classNode)
+    {
+
+        MethodNode found = null;
+
+        List<Class> parameterTypes = getJavaParameterTypes(methodNode);
+
+        for (MethodNode implementationMethodNode : classNode.methods)
+        {
 
-            for (MethodNode implementationMethodNode : source.methods)
+            if (methodNode.name.equals(implementationMethodNode.name))
             {
-            	
-                // Find corresponding methods in the implementation class MethodNode
-                if (methodNode.name.equals(implementationMethodNode.name) && 
-//                		methodNode.parameters.size() == implementationMethodNode.parameters.size() &&
-                		// We don't want synthetic methods.
-                		((implementationMethodNode.access & Opcodes.ACC_SYNTHETIC) == 0) 
-                		/*methodNode.desc.equals(implementationMethodNode.desc)*/)
+
+                final List<Class> implementationParameterTypes = getJavaParameterTypes(implementationMethodNode);
+
+                if (parameterTypes.size() == implementationParameterTypes.size())
                 {
-                	if (candidate == null)
-                	{
-                		candidate = implementationMethodNode;
-                	}
-                	// Generics implementation. Two methods with same name: The one which isn't a bridge is the one we're looking for.
-                	else 
-                	{
-                		if (isBridge(candidate))
-                		{
-                			candidate = implementationMethodNode;
-                		}
-                	}
-                	
+
+                    boolean matches = true;
+                    for (int i = 0; i < parameterTypes.size(); i++)
+                    {
+                        final Class implementationParameterType = implementationParameterTypes.get(i);
+						final Class parameterType = parameterTypes.get(i);
+						if (!parameterType.isAssignableFrom(implementationParameterType)) {
+                            matches = false;
+                            break;
+                        }
+                        
+                    }
+
+                    if (matches && !isBridge(implementationMethodNode))
+                    {
+                        found = implementationMethodNode;
+                        break;
+                    }
+                    
                 }
 
             }
+
+        }
+
+        return found;
+
+    }
+
+    private static void addMethodAndParameterAnnotationsFromExistingClass(MethodNode methodNode, ClassNode source)
+    {
+        if (source != null)
+        {
+
+        	MethodNode candidate = findExactMatchMethod(methodNode, source);
+
+        	final String parametersDesc = getParametersDesc(methodNode);
+        	
+        	// candidate will be null when the method has generic parameters
+        	if (candidate == null && parametersDesc.trim().length() > 0) 
+        	{
+        		candidate = findGenericMethod(methodNode, source);
+        	}
             
             if (candidate != null)
             {
@@ -843,8 +927,8 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
 
         if (!isOverride)
         {
+        	addMethodAndParameterAnnotationsFromExistingClass(methodNode, implementationClassNode);
             addMethodAndParameterAnnotationsFromExistingClass(methodNode, interfaceClassNode);
-            addMethodAndParameterAnnotationsFromExistingClass(methodNode, implementationClassNode);
         }
 
         if (isOverride)

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/72334aa6/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy b/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy
index b4e4c0e..b1f00b5 100644
--- a/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy
+++ b/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy
@@ -5,6 +5,7 @@ import org.apache.tapestry5.ioc.annotations.Advise
 import org.apache.tapestry5.ioc.internal.AdviceModule
 import org.apache.tapestry5.ioc.internal.AnnotatedServiceInterface
 import org.apache.tapestry5.ioc.internal.DecoratorModule
+import org.apache.tapestry5.ioc.internal.NonAnnotatedGenericSetServiceInterface
 import org.apache.tapestry5.ioc.internal.NonAnnotatedServiceInterface
 import org.apache.tapestry5.ioc.internal.TestAdvice
 
@@ -16,7 +17,7 @@ import org.apache.tapestry5.ioc.internal.TestAdvice
  * @see TestAdvice
  */
 class MethodInvocationGetAnnotationSpec extends AbstractRegistrySpecification {
-
+    
   def "MethodAdvice.getAnnotation() and getMethod() in service decoration"() {
     when:
 
@@ -62,11 +63,27 @@ class MethodInvocationGetAnnotationSpec extends AbstractRegistrySpecification {
     def annotatedService = registry.getService AnnotatedServiceInterface
     def annotatedResult = annotatedService.execute(0);
     def annotatedMethod = annotatedService.getClass().getMethod("execute", int.class);
+
+    def nonAnnotatedGenSetService = registry.getService NonAnnotatedGenericSetServiceInterface.class
     
+    def nonAnnotatedGenSetResult1 = nonAnnotatedGenSetService.execute1(0)
+    def nonAnnotatedGenSetMethod1 = nonAnnotatedGenSetService.getClass().getMethod("execute1", int.class)
+    
+    def nonAnnotatedGenSetResult2 = nonAnnotatedGenSetService.execute2("execute2")
+    // We need to look for a method that accept Object instead of string (maybe because of generics...)
+    def nonAnnotatedGenSetMethod2 = nonAnnotatedGenSetService.getClass().getMethod("execute2", Object.class)
+    
+    def nonAnnotatedGenSetResult3 = nonAnnotatedGenSetService.execute3(0)
+    def nonAnnotatedGenSetMethod3 = nonAnnotatedGenSetService.getClass().getMethod("execute3", int.class)
+
+    def nonAnnotatedGenSetResult4 = nonAnnotatedGenSetService.execute2("execute2")
+    // We need to look for a method that accept Object instead of string (maybe because of generics...)
+    def nonAnnotatedGenSetMethod4 = nonAnnotatedGenSetService.getClass().getMethod("execute2", Object.class, String.class)
+
     then:
     nonAnnotatedMethod != null
     nonAnnotatedMethod.getAnnotation(Advise.class) != null
-    nonAnnotatedMethod.getAnnotation(Advise.class).id().equals("id")
+    nonAnnotatedMethod.getAnnotation(Advise.class).id() == "id"
     nonAnnotatedMethod.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
     nonAnnotatedService.getClass().getAnnotation(ReorderProperties.class) != null
     nonAnnotatedService.getClass().getAnnotation(ReorderProperties.class).value().equals("reorder")
@@ -77,9 +94,32 @@ class MethodInvocationGetAnnotationSpec extends AbstractRegistrySpecification {
     annotatedMethod.getAnnotation(Advise.class).id().equals("id")
     annotatedMethod.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
     annotatedService.getClass().getAnnotation(ReorderProperties.class) != null
-    annotatedService.getClass().getAnnotation(ReorderProperties.class).value().equals("reorder")
+    annotatedService.getClass().getAnnotation(ReorderProperties.class).value() == "reorder"
     annotatedResult == TestAdvice.ANNOTATION_FOUND
 
+    nonAnnotatedGenSetMethod1 != null
+    nonAnnotatedGenSetMethod1.getAnnotation(Advise.class) != null
+    nonAnnotatedGenSetMethod1.getAnnotation(Advise.class).id() == "id"
+    nonAnnotatedGenSetMethod1.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
+    nonAnnotatedGenSetResult1 == TestAdvice.ANNOTATION_FOUND
+
+    nonAnnotatedGenSetMethod2 != null
+    nonAnnotatedGenSetMethod2.getAnnotation(Advise.class) != null
+    nonAnnotatedGenSetMethod2.getAnnotation(Advise.class).id() == "id"
+    nonAnnotatedGenSetMethod2.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
+    nonAnnotatedGenSetResult2 == TestAdvice.ANNOTATION_FOUND
+
+    nonAnnotatedGenSetMethod3 != null
+    nonAnnotatedGenSetMethod3.getAnnotation(Advise.class) != null
+    nonAnnotatedGenSetMethod3.getAnnotation(Advise.class).id() == "id"
+    nonAnnotatedGenSetMethod3.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
+    nonAnnotatedGenSetResult3 == TestAdvice.ANNOTATION_FOUND
+
+    nonAnnotatedGenSetMethod4 != null
+    nonAnnotatedGenSetMethod4.getAnnotation(Advise.class) != null
+    nonAnnotatedGenSetMethod4.getAnnotation(Advise.class).id() == "id"
+    nonAnnotatedGenSetMethod4.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
+    nonAnnotatedGenSetResult4 == TestAdvice.ANNOTATION_FOUND
   }
 
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/72334aa6/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
index 466b528..0e0a186 100644
--- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/AdviceModule.java
@@ -24,6 +24,8 @@ public class AdviceModule
     {
         binder.bind(NonAnnotatedServiceInterface.class, NonAnnotatedServiceInterfaceImpl.class);
         binder.bind(AnnotatedServiceInterface.class, AnnotatedServiceInterfaceImpl.class);
+        binder.bind(NonAnnotatedGenericSetServiceInterface.class,
+                NonAnnotatedGenericSetServiceImpl.class);
     }
 
     @Advise(serviceInterface = NonAnnotatedServiceInterface.class)
@@ -38,4 +40,9 @@ public class AdviceModule
         methodAdviceReceiver.adviseAllMethods(new TestAdvice());
     }
 
+    @Advise(serviceInterface = NonAnnotatedGenericSetServiceInterface.class)
+    public static void adviseNonAnnotatedGenericSetServiceInterface(
+            final MethodAdviceReceiver methodAdviceReceiver) {
+        methodAdviceReceiver.adviseAllMethods(new TestAdvice());
+    }
 }