[jboss-cvs] JBossAS SVN: r61111 - in projects/aop/trunk/aop/src: resources/test/beforeafterArgs and 1 other directories.

jboss-cvs-commits at lists.jboss.org jboss-cvs-commits at lists.jboss.org
Mon Mar 5 18:54:28 EST 2007


Author: flavia.rainone at jboss.com
Date: 2007-03-05 18:54:28 -0500 (Mon, 05 Mar 2007)
New Revision: 61111

Modified:
   projects/aop/trunk/aop/src/main/org/jboss/aop/advice/annotation/AnnotatedParameterAdviceInfo.java
   projects/aop/trunk/aop/src/resources/test/beforeafterArgs/jboss-aop.xml
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgAspect.java
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgTestCase.java
   projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgsPOJO.java
Log:
[JBAOP-37] Two bugs solved and tested: duplicate @Arg indexes were not being checked,
           and advice parameter order was being prioritized instead of @Arg index values.

Modified: projects/aop/trunk/aop/src/main/org/jboss/aop/advice/annotation/AnnotatedParameterAdviceInfo.java
===================================================================
--- projects/aop/trunk/aop/src/main/org/jboss/aop/advice/annotation/AnnotatedParameterAdviceInfo.java	2007-03-05 23:40:39 UTC (rev 61110)
+++ projects/aop/trunk/aop/src/main/org/jboss/aop/advice/annotation/AnnotatedParameterAdviceInfo.java	2007-03-05 23:54:28 UTC (rev 61111)
@@ -583,12 +583,29 @@
                   }
                   return false;
                }
+               // index set more than once
+               if (taken[indexes[i][1]])
+               {
+                  if (AspectManager.verbose)
+                  {
+                     AdviceMethodFactory.adviceMatchingMessage.append("\n[warn] - Joinpoint parameter index '");
+                     AdviceMethodFactory.adviceMatchingMessage.append(indexes[i][0]);
+                     AdviceMethodFactory.adviceMatchingMessage.append("' cannot be assigned to more than one '@Arg' advice parameter");
+                  }
+                  return false;
+               }
+               // mark index as set
                taken[indexes[i][1]] = true;
-               // TODO give priority to indexes set first
-               // TODO Check on taken for indexes set
                continue;
             }
-            
+         }
+         for (int i = 0; i < indexesLength; i++)
+         {
+            // parameter index is already set
+            if (indexes[i][1] != -1)
+            {
+               continue;
+            }
             boolean found = false;
             for (int j = 0; j < expectedTypes.length; j++)
             {

Modified: projects/aop/trunk/aop/src/resources/test/beforeafterArgs/jboss-aop.xml
===================================================================
--- projects/aop/trunk/aop/src/resources/test/beforeafterArgs/jboss-aop.xml	2007-03-05 23:40:39 UTC (rev 61110)
+++ projects/aop/trunk/aop/src/resources/test/beforeafterArgs/jboss-aop.xml	2007-03-05 23:54:28 UTC (rev 61111)
@@ -105,12 +105,23 @@
       <after name="afterInterface3" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
       <after name="afterInterface4" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
       <after name="afterInterface5" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
-       <throwing name="throwingInterface1" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <throwing name="throwingInterface1" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
       <throwing name="throwingInterface2" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
       <throwing name="throwingInterface3" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
       <throwing name="throwingInterface4" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
       <throwing name="throwingInterface5" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
    </bind>
+   
+   <bind pointcut="execution(void org.jboss.test.aop.beforeafterArgs.ArgsPOJO->*(java.lang.String,java.util.Collection))">
+      <before name="beforeInvertedArgs1" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <before name="beforeInvertedArgs2" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <advice name="aroundInvertedArgs1" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <advice name="aroundInvertedArgs2" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <after name="afterInvertedArgs1" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <after name="afterInvertedArgs2" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <throwing name="throwingInvertedArgs1" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+      <throwing name="throwingInvertedArgs2" aspect="org.jboss.test.aop.beforeafterArgs.ArgAspect"/>
+   </bind>
 
    <!-- @Args test -->
 	<aspect class="org.jboss.test.aop.beforeafterArgs.ArgsAspect" scope="PER_VM"/>

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgAspect.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgAspect.java	2007-03-05 23:40:39 UTC (rev 61110)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgAspect.java	2007-03-05 23:54:28 UTC (rev 61111)
@@ -84,6 +84,11 @@
    static boolean throwingInterface2 = false;
    static boolean throwingInterface3 = false;
    
+   static boolean beforeInverted1 = false;
+   static boolean aroundInverted1 = false;
+   static boolean afterInverted1 = false;
+   static boolean throwingInverted1 = false;
+   
    public static void clear()
    {
       before1 = false;
@@ -123,6 +128,11 @@
       throwingInterface1 = false;
       throwingInterface2 = false;
       throwingInterface3 = false;
+      
+      beforeInverted1 = false;
+      aroundInverted1 = false;
+      afterInverted1 = false;
+      throwingInverted1 = false;
    }
    
    public void before1(@Arg(index=0) int x)
@@ -345,4 +355,51 @@
    {
       Assert.fail("This advice should never be executed");
    }
+   
+   public void beforeInvertedArgs1(@Arg Object arg2, @Arg (index = 0) String arg1)
+   {
+      beforeInverted1 = true;
+   }
+   
+   public void beforeInvertedArgs2(@Arg (index = 0) Object arg2,
+         @Arg (index = 0) String arg1)
+   {
+      Assert.fail("This advice should never be executed");
+   }
+   
+   public Object aroundInvertedArgs1(@Arg Object arg2, @Arg (index = 0) String arg1)
+      throws Throwable
+   {
+      aroundInverted1 = true;
+      return CurrentInvocation.proceed();
+   }
+   
+   public void aroundInvertedArgs2(@Arg (index = 0) Object arg2,
+         @Arg (index = 0) String arg1)
+   {
+      Assert.fail("This advice should never be executed");
+   }
+   
+   public void afterInvertedArgs1(@Arg Object arg2, @Arg (index = 0) String arg1)
+   {
+      afterInverted1 = true;
+   }
+   
+   public void afterInvertedArgs2(@Arg (index = 0) Object arg2,
+         @Arg (index = 0) String arg1)
+   {
+      Assert.fail("This advice should never be executed");
+   }
+   
+   public void throwingInvertedArgs1(@Thrown Throwable thrown, @Arg Object arg2,
+         @Arg (index = 0) String arg1)
+   {
+      throwingInverted1 = true;
+   }
+   
+   public void throwingInvertedArgs2(@Arg (index = 0) Object arg2,
+         @Arg (index = 0) String arg1)
+   {
+      Assert.fail("This advice should never be executed");
+   }
 }
\ No newline at end of file

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgTestCase.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgTestCase.java	2007-03-05 23:40:39 UTC (rev 61110)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgTestCase.java	2007-03-05 23:54:28 UTC (rev 61111)
@@ -311,4 +311,34 @@
       assertTrue(ArgAspect.throwingInterface2);
       assertTrue(ArgAspect.throwingInterface3);
    }
+   
+   public void testInverted1()
+   {
+      pojo.method7("testInverted", null);
+      
+      assertTrue(ArgAspect.beforeInverted1);
+      assertTrue(ArgAspect.aroundInverted1);
+      assertTrue(ArgAspect.afterInverted1);
+      assertFalse(ArgAspect.throwingInverted1);
+   }
+   
+   public void testInverted2()
+   {
+      boolean thrown = false;
+      try
+      {
+         pojo.method8("testInverted", null);
+      }
+      catch(POJOException e)
+      {
+         thrown = true;
+      }
+      // verify precondition for this test
+      assertTrue(thrown);
+      
+      assertTrue(ArgAspect.beforeInverted1);
+      assertTrue(ArgAspect.aroundInverted1);
+      assertFalse(ArgAspect.afterInverted1);
+      assertTrue(ArgAspect.throwingInverted1);
+   }
 }
\ No newline at end of file

Modified: projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgsPOJO.java
===================================================================
--- projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgsPOJO.java	2007-03-05 23:40:39 UTC (rev 61110)
+++ projects/aop/trunk/aop/src/test/org/jboss/test/aop/beforeafterArgs/ArgsPOJO.java	2007-03-05 23:54:28 UTC (rev 61111)
@@ -21,6 +21,8 @@
  */
 package org.jboss.test.aop.beforeafterArgs;
 
+import java.util.Collection;
+
 /**
  * Plain old java object used on @Args parameter tests.
  * 
@@ -87,12 +89,16 @@
       this.method3((short) -4, (long) 4);
    }
    
-   public void method5(Interface param) throws POJOException 
+   public void method5(Interface param) throws POJOException {}
+   
+   public void method6(Interface param) throws POJOException
    {
-      
+      throw new POJOException();
    }
    
-   public void method6(Interface param) throws POJOException
+   public void method7(String param1, Collection param2) {}
+   
+   public void method8(String param1, Collection param2) throws POJOException
    {
       throw new POJOException();
    }




More information about the jboss-cvs-commits mailing list