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/27 05:19:24 UTC

tapestry-5 git commit: TAP5-2029 : fixing the handling of annotations placed both on interface and implementation of a service

Repository: tapestry-5
Updated Branches:
  refs/heads/master ff3ac8c90 -> b65ad362e


TAP5-2029 : fixing the handling of annotations placed both on interface and implementation of a service


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

Branch: refs/heads/master
Commit: b65ad362e1d0bacf2f243255ee9b1e45db1c76c0
Parents: ff3ac8c
Author: Thiago H. de Paula Figueiredo <th...@apache.org>
Authored: Sat Jun 27 00:18:45 2015 -0300
Committer: Thiago H. de Paula Figueiredo <th...@apache.org>
Committed: Sat Jun 27 00:18:45 2015 -0300

----------------------------------------------------------------------
 .../plastic/asm/tree/AnnotationNode.java        |  6 ++
 .../internal/plastic/asm/tree/ClassNode.java    |  6 ++
 .../internal/plastic/PlasticClassImpl.java      | 77 +++++++++++++++++++-
 .../MethodInvocationGetAnnotationSpec.groovy    | 12 ++-
 .../internal/NonAnnotatedServiceInterface.java  | 10 ++-
 .../NonAnnotatedServiceInterfaceImpl.java       |  6 ++
 6 files changed, 113 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b65ad362/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/AnnotationNode.java
----------------------------------------------------------------------
diff --git a/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/AnnotationNode.java b/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/AnnotationNode.java
index 9d07669..f78c810 100644
--- a/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/AnnotationNode.java
+++ b/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/AnnotationNode.java
@@ -228,4 +228,10 @@ public class AnnotationNode extends AnnotationVisitor {
             }
         }
     }
+
+	@Override
+	public String toString() {
+		return "AnnotationNode [desc=" + desc + ", values=" + values + "]";
+	}
+    
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b65ad362/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/ClassNode.java
----------------------------------------------------------------------
diff --git a/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/ClassNode.java b/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/ClassNode.java
index 0bdcc54..94075d6 100644
--- a/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/ClassNode.java
+++ b/plastic/src/external/java/org/apache/tapestry5/internal/plastic/asm/tree/ClassNode.java
@@ -414,4 +414,10 @@ public class ClassNode extends ClassVisitor {
         // visits end
         cv.visitEnd();
     }
+
+	@Override
+	public String toString() {
+		return "ClassNode [name=" + name + "]";
+	}
+	
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b65ad362/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 e224bd1..46f3b41 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
@@ -401,6 +401,79 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
 
     }
     
+    private static void removeDuplicatedAnnotations(MethodNode node)
+    {
+    	
+    	removeDuplicatedAnnotations(node.visibleAnnotations);
+    	removeDuplicatedAnnotations(node.invisibleAnnotations);
+    	
+    	if (node.visibleParameterAnnotations != null)
+    	{
+	    	for (List<AnnotationNode> list : node.visibleParameterAnnotations)
+	    	{
+	    		removeDuplicatedAnnotations(list);
+	    	}
+    	}
+    	
+    	if (node.invisibleParameterAnnotations != null)
+    	{
+	    	for (List<AnnotationNode> list : node.invisibleParameterAnnotations)
+	    	{
+	    		removeDuplicatedAnnotations(list);
+	    	}
+    	}
+    	
+    }
+
+    private static void removeDuplicatedAnnotations(ClassNode node)
+    {
+    	removeDuplicatedAnnotations(node.visibleAnnotations, true);
+    	removeDuplicatedAnnotations(node.invisibleAnnotations, true);
+    }
+    
+    private static void removeDuplicatedAnnotations(List<AnnotationNode> list) {
+    	removeDuplicatedAnnotations(list, false);
+    }
+    
+    private static void removeDuplicatedAnnotations(List<AnnotationNode> list, boolean reverse) {
+    	
+    	if (list != null)
+    	{
+    	
+	    	final Set<String> annotations = new HashSet<String>();
+	    	final List<AnnotationNode> toBeRemoved = new ArrayList<AnnotationNode>();
+	    	final List<AnnotationNode> toBeIterated;
+	    	
+	    	if (reverse) 
+	    	{
+	    		toBeIterated = new ArrayList<AnnotationNode>(list);
+	    		Collections.reverse(toBeIterated);
+	    	}
+	    	else {
+	    		toBeIterated = list;
+	    	}
+	    	
+	    	for (AnnotationNode annotationNode : toBeIterated) 
+	    	{
+				if (annotations.contains(annotationNode.desc))
+				{
+					toBeRemoved.add(annotationNode);
+				}
+				else
+				{
+					annotations.add(annotationNode.desc);
+				}
+			}
+	    	
+	    	for (AnnotationNode annotationNode : toBeRemoved) 
+	    	{
+				list.remove(annotationNode);
+			}
+	    	
+    	}
+    	
+    }
+
     private static String getParametersDesc(MethodNode methodNode) {
         return methodNode.desc.substring(methodNode.desc.indexOf('(') + 1, methodNode.desc.lastIndexOf(')'));
     }
@@ -501,7 +574,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
         return found;
 
     }
-
+    
     private static void addMethodAndParameterAnnotationsFromExistingClass(MethodNode methodNode, ClassNode source)
     {
         if (source != null)
@@ -558,6 +631,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
         lock();
 
         addClassAnnotations(implementationClassNode);
+        removeDuplicatedAnnotations(classNode);
 
         createShimIfNeeded();
 
@@ -929,6 +1003,7 @@ public class PlasticClassImpl extends Lockable implements PlasticClass, Internal
         {
         	addMethodAndParameterAnnotationsFromExistingClass(methodNode, implementationClassNode);
             addMethodAndParameterAnnotationsFromExistingClass(methodNode, interfaceClassNode);
+            removeDuplicatedAnnotations(methodNode);
         }
 
         if (isOverride)

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b65ad362/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 b1f00b5..d7c0f09 100644
--- a/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy
+++ b/tapestry-ioc/src/test/groovy/ioc/specs/MethodInvocationGetAnnotationSpec.groovy
@@ -2,6 +2,7 @@ package ioc.specs
 
 import org.apache.tapestry5.beaneditor.ReorderProperties
 import org.apache.tapestry5.ioc.annotations.Advise
+import org.apache.tapestry5.ioc.annotations.IntermediateType;
 import org.apache.tapestry5.ioc.internal.AdviceModule
 import org.apache.tapestry5.ioc.internal.AnnotatedServiceInterface
 import org.apache.tapestry5.ioc.internal.DecoratorModule
@@ -37,7 +38,7 @@ class MethodInvocationGetAnnotationSpec extends AbstractRegistrySpecification {
     nonAnnotatedMethod.getAnnotation(Advise.class).id().equals("id")
     nonAnnotatedMethod.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
     nonAnnotatedService.getClass().getAnnotation(ReorderProperties.class) != null
-    nonAnnotatedService.getClass().getAnnotation(ReorderProperties.class).value().equals("reorder") 
+    nonAnnotatedService.getClass().getAnnotation(ReorderProperties.class).value() == "reorder" 
 	nonAnnotatedResult == TestAdvice.ANNOTATION_FOUND
     
     annotatedMethod != null
@@ -59,6 +60,7 @@ class MethodInvocationGetAnnotationSpec extends AbstractRegistrySpecification {
     def nonAnnotatedService = registry.getService NonAnnotatedServiceInterface.class
     def nonAnnotatedResult = nonAnnotatedService.execute(0);
     def nonAnnotatedMethod = nonAnnotatedService.getClass().getMethod("execute", int.class);
+    def duplicatedAnnotationMethod = nonAnnotatedService.getClass().getMethod("duplicatedAnnotation", String.class);
 
     def annotatedService = registry.getService AnnotatedServiceInterface
     def annotatedResult = annotatedService.execute(0);
@@ -89,9 +91,15 @@ class MethodInvocationGetAnnotationSpec extends AbstractRegistrySpecification {
     nonAnnotatedService.getClass().getAnnotation(ReorderProperties.class).value().equals("reorder")
     nonAnnotatedResult == TestAdvice.ANNOTATION_FOUND
     
+    duplicatedAnnotationMethod != null
+    duplicatedAnnotationMethod.getAnnotation(Advise.class) != null
+    duplicatedAnnotationMethod.getAnnotation(Advise.class).id() == "right"
+    duplicatedAnnotationMethod.getParameterAnnotations()[0].length > 0
+    ((IntermediateType) duplicatedAnnotationMethod.getParameterAnnotations()[0][0]).value() == String.class
+
     annotatedMethod != null
     annotatedMethod.getAnnotation(Advise.class) != null
-    annotatedMethod.getAnnotation(Advise.class).id().equals("id")
+    annotatedMethod.getAnnotation(Advise.class).id() == "id"
     annotatedMethod.getAnnotation(Advise.class).serviceInterface() == NonAnnotatedServiceInterface.class
     annotatedService.getClass().getAnnotation(ReorderProperties.class) != null
     annotatedService.getClass().getAnnotation(ReorderProperties.class).value() == "reorder"

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b65ad362/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterface.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterface.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterface.java
index fc26347..bff57bc 100644
--- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterface.java
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterface.java
@@ -13,12 +13,20 @@
 // limitations under the License.
 package org.apache.tapestry5.ioc.internal;
 
+import org.apache.tapestry5.beaneditor.ReorderProperties;
+import org.apache.tapestry5.ioc.annotations.Advise;
+import org.apache.tapestry5.ioc.annotations.IntermediateType;
+
 /**
- * Service definition without any annotations so we can test copying the service implementation
+ * Service definition so we can test copying the service implementation
  * annotations, both in class and methods, to the service proxy.
  */
+@ReorderProperties("wrong") // no meaning, just for testing whether the proxy will have it
 public interface NonAnnotatedServiceInterface {
 
     public String execute(int i);
     
+    @Advise(id = "wrong")
+    public String duplicatedAnnotation(@IntermediateType(Object.class) String parameter);
+    
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/b65ad362/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterfaceImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterfaceImpl.java b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterfaceImpl.java
index a15bfba..5a14841 100644
--- a/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterfaceImpl.java
+++ b/tapestry-ioc/src/test/java/org/apache/tapestry5/ioc/internal/NonAnnotatedServiceInterfaceImpl.java
@@ -39,5 +39,11 @@ public class NonAnnotatedServiceInterfaceImpl implements NonAnnotatedServiceInte
         return null;
 
     }
+    
+    @Advise(id = "right")
+    public String duplicatedAnnotation(@IntermediateType(String.class) String parameter)
+    {
+        return null;
+    }
 
 }