You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by km...@apache.org on 2010/07/21 04:51:58 UTC

svn commit: r966073 - in /openejb/trunk/openejb3/container/openejb-core/src: main/java/org/apache/openejb/config/rules/ main/resources/org/apache/openejb/config/rules/ test/java/org/apache/openejb/config/rules/

Author: kmalhi
Date: Wed Jul 21 02:51:58 2010
New Revision: 966073

URL: http://svn.apache.org/viewvc?rev=966073&view=rev
Log:
Updated CheckCallbacks to test for scenario where a class has two or more overloaded methods, but the method with the wrong signature is annotated with @Timeout
Commented out keys from Messages.properties and also updated a detailed message for a key
Added @Timeout tests

Modified:
    openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
    openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
    openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java?rev=966073&r1=966072&r2=966073&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java Wed Jul 21 02:51:58 2010
@@ -16,6 +16,7 @@
  */
 package org.apache.openejb.config.rules;
 
+import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
 import java.util.ArrayList;
@@ -29,6 +30,7 @@ import javax.ejb.PostActivate;
 import javax.ejb.PrePassivate;
 import javax.ejb.Remove;
 import javax.ejb.SessionSynchronization;
+import javax.ejb.Timeout;
 import javax.interceptor.InvocationContext;
 
 import org.apache.openejb.OpenEJBException;
@@ -336,12 +338,24 @@ public class CheckCallbacks extends Vali
         if (timeout == null) return;
         try {
             Method method = getMethod(ejbClass, timeout.getMethodName(), javax.ejb.Timer.class);
-
-            Class<?> returnType = method.getReturnType();
-
-            if (!returnType.equals(Void.TYPE)) {
-                fail(bean, "timeout.badReturnType", timeout.getMethodName(), returnType.getName());
+            Annotation[] declaredAnnotations = method.getDeclaredAnnotations();
+            boolean isAnnotated = false;
+            for (Annotation annotation : declaredAnnotations) {
+                if(annotation.equals(Timeout.class)){
+                    isAnnotated = true;
+                    break;
+                }
+            }
+            
+            if (isAnnotated) {
+                Class<?> returnType = method.getReturnType();
+                if (!returnType.equals(Void.TYPE)) {
+                    fail(bean, "timeout.badReturnType", timeout.getMethodName(), returnType.getName());
+                }
+            }else{
+                fail(bean, "timeout.missing.possibleTypo", timeout.getMethodName(), getMethods(ejbClass, timeout.getMethodName()).size());
             }
+           
         } catch (NoSuchMethodException e) {
             List<Method> possibleMethods = getMethods(ejbClass, timeout.getMethodName());
 

Modified: openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties?rev=966073&r1=966072&r2=966073&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties Wed Jul 21 02:51:58 2010
@@ -223,10 +223,11 @@
 2.timeout.badReturnType = Timeout method must return 'void': method {0} returns {1}
 3.timeout.badReturnType = Timeout method "{0}" illegally returns {1} instead of void.  Change the method signature to "void {0}(javax.ejb.Timer)"
 
+#Don't think this is ever going to be used, commenting it out for now. If there is a case where this key can be used, please uncomment it and write a test for it
 # fail(bean, "timeout.missing", timeout.getMethodName());
-1.timeout.missing = Timeout method missing
-2.timeout.missing = Timeout method missing: "{0}" in class {1}
-3.timeout.missing = Timeout method "{0}" not found in class {1}.  The required method signature is "void {0}(javax.ejb.Timer)"
+#1.timeout.missing = Timeout method missing
+#2.timeout.missing = Timeout method missing: "{0}" in class {1}
+#3.timeout.missing = Timeout method "{0}" not found in class {1}.  The required method signature is "void {0}(javax.ejb.Timer)"
 
 # fail(bean, "timeout.invalidArguments", timeout.getMethodName(), getParameters(possibleMethods.get(0)));
 1.timeout.invalidArguments = Invalid Timeout arguments
@@ -236,7 +237,7 @@
 # fail(bean, "timeout.missing.possibleTypo", timeout.getMethodName(), possibleMethods.size());
 1.timeout.missing.possibleTypo = Timeout method missing or invalid
 2.timeout.missing.possibleTypo = Timeout method missing or invalid: looked for "void {0}(javax.ejb.Timer)"
-3.timeout.missing.possibleTypo = Timeout method missing or invalid.  There are {1} methods with the name "{0}" visible, none have the required signature of "void {0}(javax.ejb.Timer)"
+3.timeout.missing.possibleTypo = Timeout method missing or invalid.  There are {1} methods with the name "{0}" visible, either the wrong one has been annotated with @Timeout or none have the required signature of "void {0}(javax.ejb.Timer). A bean should have only one method annotated with @Timeout and the method signature must match void {0}(javax.ejb.Timer)"
 
 # fail(componentName,"timeout.tooManyMethods",timeoutMethods.size(),Join.join(",", timeoutMethods));
 1.timeout.tooManyMethods = More than one method annotated with @Timeout

Modified: openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
URL: http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java?rev=966073&r1=966072&r2=966073&view=diff
==============================================================================
--- openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java (original)
+++ openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java Wed Jul 21 02:51:58 2010
@@ -26,26 +26,40 @@ import org.junit.runner.RunWith;
 
 @RunWith(ValidationRunner.class)
 public class CheckInvalidTimeoutTest extends TestCase {
-    @Keys( { @Key(value = "timeout.badReturnType"), @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") })
+    @Keys( { @Key(value = "timeout.badReturnType"), @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") , @Key("timeout.missing.possibleTypo")})
     public EjbJar test() throws Exception {
         System.setProperty("openejb.validation.output.level", "VERBOSE");
         EjbJar ejbJar = new EjbJar();
         ejbJar.addEnterpriseBean(new StatelessBean(TestBean.class));
         ejbJar.addEnterpriseBean(new StatelessBean(FooBean.class));
+        ejbJar.addEnterpriseBean(new StatelessBean(BarBean.class));
         return ejbJar;
     }
-
+// A case where the class has the method with wrong signature annotated with @Timeout
+    // timeout.badReturnType
+    // timeout.invalidArguments
     public static class TestBean {
         @Timeout
         public Object bar() {
             return null;
         }
     }
-
+// A case where the class has more than one method annotated with @Timeout
+    // timeout.tooManyMethods
     public static class FooBean {
         @Timeout
         public void foo(javax.ejb.Timer timer) {}
         @Timeout
         public void bar(javax.ejb.Timer timer) {}
     }
-}
\ No newline at end of file
+//  A case where the class has overloaded methods, but the method with the wrong signature has been annotated with @Timeout
+    // timeout.missing.possibleTypo
+    public static class BarBean {
+       
+        public void foo(javax.ejb.Timer timer) {}
+        
+        @Timeout
+        public void foo() {}
+    }
+
+}



Re: svn commit: r966073 - in /openejb/trunk/openejb3/container/openejb-core/src: main/java/org/apache/openejb/config/rules/ main/resources/org/apache/openejb/config/rules/ test/java/org/apache/openejb/config/rules/

Posted by Ivan <xh...@gmail.com>.
Hi, Karan:
    I opened a JIRA OpenEJB-1314, and attach a patch for the testcase
improvment, please help to review it.
    Thanks !

2010/7/28 Karan Malhi <ka...@gmail.com>

> Hi Ivan,
>
> You are right, I have not checked for those scenarios. My intial objective
> was to write tests for keys listed in Messages.properties and test for
> atleast one scenario where that key would be used. I focussed more on
> annotations and less on deployment descriptor related stuff. Hence this
> check only for annotations. Ideally, the test should involve all scenarios.
> I have limited time, so it will take a while before I can cover all the
> scenarios.If you would like to fix it and add coverage for TimedObject and
> descriptor, please feel free to do that. Creating a JIRA issue would be the
> first step.
>
> BTW, such feedback is exactly the way we will end up covering more
> scenarios. Keep em coming and thank you very much.
>
>
> On Wed, Jul 28, 2010 at 3:20 AM, Ivan <xh...@gmail.com> wrote:
>
> > Hi, Karan
> >   I have a question for this change, the timeout method could be
> specified
> > by @Timeout or impmented TimedObject or configured in the deployment
> > descriptor, so why the annotation checking is required ?
> >  Thanks !
> >
> > 2010/7/21 <km...@apache.org>
> >
> > > Author: kmalhi
> > > Date: Wed Jul 21 02:51:58 2010
> > > New Revision: 966073
> > >
> > > URL: http://svn.apache.org/viewvc?rev=966073&view=rev
> > > Log:
> > > Updated CheckCallbacks to test for scenario where a class has two or
> more
> > > overloaded methods, but the method with the wrong signature is
> annotated
> > > with @Timeout
> > > Commented out keys from Messages.properties and also updated a detailed
> > > message for a key
> > > Added @Timeout tests
> > >
> > > Modified:
> > >
> > >
> >
>  openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > >
> > >
> >
>  openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > >
> > >
> >
>  openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > >
> > > Modified:
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java?rev=966073&r1=966072&r2=966073&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > > (original)
> > > +++
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > > Wed Jul 21 02:51:58 2010
> > > @@ -16,6 +16,7 @@
> > >  */
> > >  package org.apache.openejb.config.rules;
> > >
> > > +import java.lang.annotation.Annotation;
> > >  import java.lang.reflect.Method;
> > >  import java.lang.reflect.Modifier;
> > >  import java.util.ArrayList;
> > > @@ -29,6 +30,7 @@ import javax.ejb.PostActivate;
> > >  import javax.ejb.PrePassivate;
> > >  import javax.ejb.Remove;
> > >  import javax.ejb.SessionSynchronization;
> > > +import javax.ejb.Timeout;
> > >  import javax.interceptor.InvocationContext;
> > >
> > >  import org.apache.openejb.OpenEJBException;
> > > @@ -336,12 +338,24 @@ public class CheckCallbacks extends Vali
> > >         if (timeout == null) return;
> > >         try {
> > >             Method method = getMethod(ejbClass,
> timeout.getMethodName(),
> > > javax.ejb.Timer.class);
> > > -
> > > -            Class<?> returnType = method.getReturnType();
> > > -
> > > -            if (!returnType.equals(Void.TYPE)) {
> > > -                fail(bean, "timeout.badReturnType",
> > > timeout.getMethodName(), returnType.getName());
> > > +            Annotation[] declaredAnnotations =
> > > method.getDeclaredAnnotations();
> > > +            boolean isAnnotated = false;
> > > +            for (Annotation annotation : declaredAnnotations) {
> > > +                if(annotation.equals(Timeout.class)){
> > > +                    isAnnotated = true;
> > > +                    break;
> > > +                }
> > > +            }
> > > +
> > > +            if (isAnnotated) {
> > > +                Class<?> returnType = method.getReturnType();
> > > +                if (!returnType.equals(Void.TYPE)) {
> > > +                    fail(bean, "timeout.badReturnType",
> > > timeout.getMethodName(), returnType.getName());
> > > +                }
> > > +            }else{
> > > +                fail(bean, "timeout.missing.possibleTypo",
> > > timeout.getMethodName(), getMethods(ejbClass,
> > > timeout.getMethodName()).size());
> > >             }
> > > +
> > >         } catch (NoSuchMethodException e) {
> > >             List<Method> possibleMethods = getMethods(ejbClass,
> > > timeout.getMethodName());
> > >
> > >
> > > Modified:
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties?rev=966073&r1=966072&r2=966073&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > > (original)
> > > +++
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > > Wed Jul 21 02:51:58 2010
> > > @@ -223,10 +223,11 @@
> > >  2.timeout.badReturnType = Timeout method must return 'void': method
> {0}
> > > returns {1}
> > >  3.timeout.badReturnType = Timeout method "{0}" illegally returns {1}
> > > instead of void.  Change the method signature to "void
> > {0}(javax.ejb.Timer)"
> > >
> > > +#Don't think this is ever going to be used, commenting it out for now.
> > If
> > > there is a case where this key can be used, please uncomment it and
> write
> > a
> > > test for it
> > >  # fail(bean, "timeout.missing", timeout.getMethodName());
> > > -1.timeout.missing = Timeout method missing
> > > -2.timeout.missing = Timeout method missing: "{0}" in class {1}
> > > -3.timeout.missing = Timeout method "{0}" not found in class {1}.  The
> > > required method signature is "void {0}(javax.ejb.Timer)"
> > > +#1.timeout.missing = Timeout method missing
> > > +#2.timeout.missing = Timeout method missing: "{0}" in class {1}
> > > +#3.timeout.missing = Timeout method "{0}" not found in class {1}.  The
> > > required method signature is "void {0}(javax.ejb.Timer)"
> > >
> > >  # fail(bean, "timeout.invalidArguments", timeout.getMethodName(),
> > > getParameters(possibleMethods.get(0)));
> > >  1.timeout.invalidArguments = Invalid Timeout arguments
> > > @@ -236,7 +237,7 @@
> > >  # fail(bean, "timeout.missing.possibleTypo", timeout.getMethodName(),
> > > possibleMethods.size());
> > >  1.timeout.missing.possibleTypo = Timeout method missing or invalid
> > >  2.timeout.missing.possibleTypo = Timeout method missing or invalid:
> > looked
> > > for "void {0}(javax.ejb.Timer)"
> > > -3.timeout.missing.possibleTypo = Timeout method missing or invalid.
> >  There
> > > are {1} methods with the name "{0}" visible, none have the required
> > > signature of "void {0}(javax.ejb.Timer)"
> > > +3.timeout.missing.possibleTypo = Timeout method missing or invalid.
> >  There
> > > are {1} methods with the name "{0}" visible, either the wrong one has
> > been
> > > annotated with @Timeout or none have the required signature of "void
> > > {0}(javax.ejb.Timer). A bean should have only one method annotated with
> > > @Timeout and the method signature must match void {0}(javax.ejb.Timer)"
> > >
> > >  #
> > >
> >
> fail(componentName,"timeout.tooManyMethods",timeoutMethods.size(),Join.join(",",
> > > timeoutMethods));
> > >  1.timeout.tooManyMethods = More than one method annotated with
> @Timeout
> > >
> > > Modified:
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > > URL:
> > >
> >
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java?rev=966073&r1=966072&r2=966073&view=diff
> > >
> > >
> >
> ==============================================================================
> > > ---
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > > (original)
> > > +++
> > >
> >
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > > Wed Jul 21 02:51:58 2010
> > > @@ -26,26 +26,40 @@ import org.junit.runner.RunWith;
> > >
> > >  @RunWith(ValidationRunner.class)
> > >  public class CheckInvalidTimeoutTest extends TestCase {
> > > -    @Keys( { @Key(value = "timeout.badReturnType"),
> > > @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") })
> > > +    @Keys( { @Key(value = "timeout.badReturnType"),
> > > @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") ,
> > > @Key("timeout.missing.possibleTypo")})
> > >     public EjbJar test() throws Exception {
> > >         System.setProperty("openejb.validation.output.level",
> "VERBOSE");
> > >         EjbJar ejbJar = new EjbJar();
> > >         ejbJar.addEnterpriseBean(new StatelessBean(TestBean.class));
> > >         ejbJar.addEnterpriseBean(new StatelessBean(FooBean.class));
> > > +        ejbJar.addEnterpriseBean(new StatelessBean(BarBean.class));
> > >         return ejbJar;
> > >     }
> > > -
> > > +// A case where the class has the method with wrong signature
> annotated
> > > with @Timeout
> > > +    // timeout.badReturnType
> > > +    // timeout.invalidArguments
> > >     public static class TestBean {
> > >         @Timeout
> > >         public Object bar() {
> > >             return null;
> > >         }
> > >     }
> > > -
> > > +// A case where the class has more than one method annotated with
> > @Timeout
> > > +    // timeout.tooManyMethods
> > >     public static class FooBean {
> > >         @Timeout
> > >         public void foo(javax.ejb.Timer timer) {}
> > >         @Timeout
> > >         public void bar(javax.ejb.Timer timer) {}
> > >     }
> > > -}
> > > \ No newline at end of file
> > > +//  A case where the class has overloaded methods, but the method with
> > the
> > > wrong signature has been annotated with @Timeout
> > > +    // timeout.missing.possibleTypo
> > > +    public static class BarBean {
> > > +
> > > +        public void foo(javax.ejb.Timer timer) {}
> > > +
> > > +        @Timeout
> > > +        public void foo() {}
> > > +    }
> > > +
> > > +}
> > >
> > >
> > >
> >
> >
> > --
> > Ivan
> >
>
>
>
> --
> Karan Singh Malhi
>



-- 
Ivan

Re: svn commit: r966073 - in /openejb/trunk/openejb3/container/openejb-core/src: main/java/org/apache/openejb/config/rules/ main/resources/org/apache/openejb/config/rules/ test/java/org/apache/openejb/config/rules/

Posted by Karan Malhi <ka...@gmail.com>.
Hi Ivan,

You are right, I have not checked for those scenarios. My intial objective
was to write tests for keys listed in Messages.properties and test for
atleast one scenario where that key would be used. I focussed more on
annotations and less on deployment descriptor related stuff. Hence this
check only for annotations. Ideally, the test should involve all scenarios.
I have limited time, so it will take a while before I can cover all the
scenarios.If you would like to fix it and add coverage for TimedObject and
descriptor, please feel free to do that. Creating a JIRA issue would be the
first step.

BTW, such feedback is exactly the way we will end up covering more
scenarios. Keep em coming and thank you very much.


On Wed, Jul 28, 2010 at 3:20 AM, Ivan <xh...@gmail.com> wrote:

> Hi, Karan
>   I have a question for this change, the timeout method could be specified
> by @Timeout or impmented TimedObject or configured in the deployment
> descriptor, so why the annotation checking is required ?
>  Thanks !
>
> 2010/7/21 <km...@apache.org>
>
> > Author: kmalhi
> > Date: Wed Jul 21 02:51:58 2010
> > New Revision: 966073
> >
> > URL: http://svn.apache.org/viewvc?rev=966073&view=rev
> > Log:
> > Updated CheckCallbacks to test for scenario where a class has two or more
> > overloaded methods, but the method with the wrong signature is annotated
> > with @Timeout
> > Commented out keys from Messages.properties and also updated a detailed
> > message for a key
> > Added @Timeout tests
> >
> > Modified:
> >
> >
>  openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> >
> >
>  openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> >
> >
>  openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> >
> > Modified:
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > URL:
> >
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java?rev=966073&r1=966072&r2=966073&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > (original)
> > +++
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> > Wed Jul 21 02:51:58 2010
> > @@ -16,6 +16,7 @@
> >  */
> >  package org.apache.openejb.config.rules;
> >
> > +import java.lang.annotation.Annotation;
> >  import java.lang.reflect.Method;
> >  import java.lang.reflect.Modifier;
> >  import java.util.ArrayList;
> > @@ -29,6 +30,7 @@ import javax.ejb.PostActivate;
> >  import javax.ejb.PrePassivate;
> >  import javax.ejb.Remove;
> >  import javax.ejb.SessionSynchronization;
> > +import javax.ejb.Timeout;
> >  import javax.interceptor.InvocationContext;
> >
> >  import org.apache.openejb.OpenEJBException;
> > @@ -336,12 +338,24 @@ public class CheckCallbacks extends Vali
> >         if (timeout == null) return;
> >         try {
> >             Method method = getMethod(ejbClass, timeout.getMethodName(),
> > javax.ejb.Timer.class);
> > -
> > -            Class<?> returnType = method.getReturnType();
> > -
> > -            if (!returnType.equals(Void.TYPE)) {
> > -                fail(bean, "timeout.badReturnType",
> > timeout.getMethodName(), returnType.getName());
> > +            Annotation[] declaredAnnotations =
> > method.getDeclaredAnnotations();
> > +            boolean isAnnotated = false;
> > +            for (Annotation annotation : declaredAnnotations) {
> > +                if(annotation.equals(Timeout.class)){
> > +                    isAnnotated = true;
> > +                    break;
> > +                }
> > +            }
> > +
> > +            if (isAnnotated) {
> > +                Class<?> returnType = method.getReturnType();
> > +                if (!returnType.equals(Void.TYPE)) {
> > +                    fail(bean, "timeout.badReturnType",
> > timeout.getMethodName(), returnType.getName());
> > +                }
> > +            }else{
> > +                fail(bean, "timeout.missing.possibleTypo",
> > timeout.getMethodName(), getMethods(ejbClass,
> > timeout.getMethodName()).size());
> >             }
> > +
> >         } catch (NoSuchMethodException e) {
> >             List<Method> possibleMethods = getMethods(ejbClass,
> > timeout.getMethodName());
> >
> >
> > Modified:
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > URL:
> >
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties?rev=966073&r1=966072&r2=966073&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > (original)
> > +++
> >
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> > Wed Jul 21 02:51:58 2010
> > @@ -223,10 +223,11 @@
> >  2.timeout.badReturnType = Timeout method must return 'void': method {0}
> > returns {1}
> >  3.timeout.badReturnType = Timeout method "{0}" illegally returns {1}
> > instead of void.  Change the method signature to "void
> {0}(javax.ejb.Timer)"
> >
> > +#Don't think this is ever going to be used, commenting it out for now.
> If
> > there is a case where this key can be used, please uncomment it and write
> a
> > test for it
> >  # fail(bean, "timeout.missing", timeout.getMethodName());
> > -1.timeout.missing = Timeout method missing
> > -2.timeout.missing = Timeout method missing: "{0}" in class {1}
> > -3.timeout.missing = Timeout method "{0}" not found in class {1}.  The
> > required method signature is "void {0}(javax.ejb.Timer)"
> > +#1.timeout.missing = Timeout method missing
> > +#2.timeout.missing = Timeout method missing: "{0}" in class {1}
> > +#3.timeout.missing = Timeout method "{0}" not found in class {1}.  The
> > required method signature is "void {0}(javax.ejb.Timer)"
> >
> >  # fail(bean, "timeout.invalidArguments", timeout.getMethodName(),
> > getParameters(possibleMethods.get(0)));
> >  1.timeout.invalidArguments = Invalid Timeout arguments
> > @@ -236,7 +237,7 @@
> >  # fail(bean, "timeout.missing.possibleTypo", timeout.getMethodName(),
> > possibleMethods.size());
> >  1.timeout.missing.possibleTypo = Timeout method missing or invalid
> >  2.timeout.missing.possibleTypo = Timeout method missing or invalid:
> looked
> > for "void {0}(javax.ejb.Timer)"
> > -3.timeout.missing.possibleTypo = Timeout method missing or invalid.
>  There
> > are {1} methods with the name "{0}" visible, none have the required
> > signature of "void {0}(javax.ejb.Timer)"
> > +3.timeout.missing.possibleTypo = Timeout method missing or invalid.
>  There
> > are {1} methods with the name "{0}" visible, either the wrong one has
> been
> > annotated with @Timeout or none have the required signature of "void
> > {0}(javax.ejb.Timer). A bean should have only one method annotated with
> > @Timeout and the method signature must match void {0}(javax.ejb.Timer)"
> >
> >  #
> >
> fail(componentName,"timeout.tooManyMethods",timeoutMethods.size(),Join.join(",",
> > timeoutMethods));
> >  1.timeout.tooManyMethods = More than one method annotated with @Timeout
> >
> > Modified:
> >
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > URL:
> >
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java?rev=966073&r1=966072&r2=966073&view=diff
> >
> >
> ==============================================================================
> > ---
> >
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > (original)
> > +++
> >
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> > Wed Jul 21 02:51:58 2010
> > @@ -26,26 +26,40 @@ import org.junit.runner.RunWith;
> >
> >  @RunWith(ValidationRunner.class)
> >  public class CheckInvalidTimeoutTest extends TestCase {
> > -    @Keys( { @Key(value = "timeout.badReturnType"),
> > @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") })
> > +    @Keys( { @Key(value = "timeout.badReturnType"),
> > @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") ,
> > @Key("timeout.missing.possibleTypo")})
> >     public EjbJar test() throws Exception {
> >         System.setProperty("openejb.validation.output.level", "VERBOSE");
> >         EjbJar ejbJar = new EjbJar();
> >         ejbJar.addEnterpriseBean(new StatelessBean(TestBean.class));
> >         ejbJar.addEnterpriseBean(new StatelessBean(FooBean.class));
> > +        ejbJar.addEnterpriseBean(new StatelessBean(BarBean.class));
> >         return ejbJar;
> >     }
> > -
> > +// A case where the class has the method with wrong signature annotated
> > with @Timeout
> > +    // timeout.badReturnType
> > +    // timeout.invalidArguments
> >     public static class TestBean {
> >         @Timeout
> >         public Object bar() {
> >             return null;
> >         }
> >     }
> > -
> > +// A case where the class has more than one method annotated with
> @Timeout
> > +    // timeout.tooManyMethods
> >     public static class FooBean {
> >         @Timeout
> >         public void foo(javax.ejb.Timer timer) {}
> >         @Timeout
> >         public void bar(javax.ejb.Timer timer) {}
> >     }
> > -}
> > \ No newline at end of file
> > +//  A case where the class has overloaded methods, but the method with
> the
> > wrong signature has been annotated with @Timeout
> > +    // timeout.missing.possibleTypo
> > +    public static class BarBean {
> > +
> > +        public void foo(javax.ejb.Timer timer) {}
> > +
> > +        @Timeout
> > +        public void foo() {}
> > +    }
> > +
> > +}
> >
> >
> >
>
>
> --
> Ivan
>



-- 
Karan Singh Malhi

Re: svn commit: r966073 - in /openejb/trunk/openejb3/container/openejb-core/src: main/java/org/apache/openejb/config/rules/ main/resources/org/apache/openejb/config/rules/ test/java/org/apache/openejb/config/rules/

Posted by Ivan <xh...@gmail.com>.
Hi, Karan
   I have a question for this change, the timeout method could be specified
by @Timeout or impmented TimedObject or configured in the deployment
descriptor, so why the annotation checking is required ?
  Thanks !

2010/7/21 <km...@apache.org>

> Author: kmalhi
> Date: Wed Jul 21 02:51:58 2010
> New Revision: 966073
>
> URL: http://svn.apache.org/viewvc?rev=966073&view=rev
> Log:
> Updated CheckCallbacks to test for scenario where a class has two or more
> overloaded methods, but the method with the wrong signature is annotated
> with @Timeout
> Commented out keys from Messages.properties and also updated a detailed
> message for a key
> Added @Timeout tests
>
> Modified:
>
>  openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
>
>  openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
>
>  openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
>
> Modified:
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> URL:
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java?rev=966073&r1=966072&r2=966073&view=diff
>
> ==============================================================================
> ---
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> (original)
> +++
> openejb/trunk/openejb3/container/openejb-core/src/main/java/org/apache/openejb/config/rules/CheckCallbacks.java
> Wed Jul 21 02:51:58 2010
> @@ -16,6 +16,7 @@
>  */
>  package org.apache.openejb.config.rules;
>
> +import java.lang.annotation.Annotation;
>  import java.lang.reflect.Method;
>  import java.lang.reflect.Modifier;
>  import java.util.ArrayList;
> @@ -29,6 +30,7 @@ import javax.ejb.PostActivate;
>  import javax.ejb.PrePassivate;
>  import javax.ejb.Remove;
>  import javax.ejb.SessionSynchronization;
> +import javax.ejb.Timeout;
>  import javax.interceptor.InvocationContext;
>
>  import org.apache.openejb.OpenEJBException;
> @@ -336,12 +338,24 @@ public class CheckCallbacks extends Vali
>         if (timeout == null) return;
>         try {
>             Method method = getMethod(ejbClass, timeout.getMethodName(),
> javax.ejb.Timer.class);
> -
> -            Class<?> returnType = method.getReturnType();
> -
> -            if (!returnType.equals(Void.TYPE)) {
> -                fail(bean, "timeout.badReturnType",
> timeout.getMethodName(), returnType.getName());
> +            Annotation[] declaredAnnotations =
> method.getDeclaredAnnotations();
> +            boolean isAnnotated = false;
> +            for (Annotation annotation : declaredAnnotations) {
> +                if(annotation.equals(Timeout.class)){
> +                    isAnnotated = true;
> +                    break;
> +                }
> +            }
> +
> +            if (isAnnotated) {
> +                Class<?> returnType = method.getReturnType();
> +                if (!returnType.equals(Void.TYPE)) {
> +                    fail(bean, "timeout.badReturnType",
> timeout.getMethodName(), returnType.getName());
> +                }
> +            }else{
> +                fail(bean, "timeout.missing.possibleTypo",
> timeout.getMethodName(), getMethods(ejbClass,
> timeout.getMethodName()).size());
>             }
> +
>         } catch (NoSuchMethodException e) {
>             List<Method> possibleMethods = getMethods(ejbClass,
> timeout.getMethodName());
>
>
> Modified:
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> URL:
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties?rev=966073&r1=966072&r2=966073&view=diff
>
> ==============================================================================
> ---
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> (original)
> +++
> openejb/trunk/openejb3/container/openejb-core/src/main/resources/org/apache/openejb/config/rules/Messages.properties
> Wed Jul 21 02:51:58 2010
> @@ -223,10 +223,11 @@
>  2.timeout.badReturnType = Timeout method must return 'void': method {0}
> returns {1}
>  3.timeout.badReturnType = Timeout method "{0}" illegally returns {1}
> instead of void.  Change the method signature to "void {0}(javax.ejb.Timer)"
>
> +#Don't think this is ever going to be used, commenting it out for now. If
> there is a case where this key can be used, please uncomment it and write a
> test for it
>  # fail(bean, "timeout.missing", timeout.getMethodName());
> -1.timeout.missing = Timeout method missing
> -2.timeout.missing = Timeout method missing: "{0}" in class {1}
> -3.timeout.missing = Timeout method "{0}" not found in class {1}.  The
> required method signature is "void {0}(javax.ejb.Timer)"
> +#1.timeout.missing = Timeout method missing
> +#2.timeout.missing = Timeout method missing: "{0}" in class {1}
> +#3.timeout.missing = Timeout method "{0}" not found in class {1}.  The
> required method signature is "void {0}(javax.ejb.Timer)"
>
>  # fail(bean, "timeout.invalidArguments", timeout.getMethodName(),
> getParameters(possibleMethods.get(0)));
>  1.timeout.invalidArguments = Invalid Timeout arguments
> @@ -236,7 +237,7 @@
>  # fail(bean, "timeout.missing.possibleTypo", timeout.getMethodName(),
> possibleMethods.size());
>  1.timeout.missing.possibleTypo = Timeout method missing or invalid
>  2.timeout.missing.possibleTypo = Timeout method missing or invalid: looked
> for "void {0}(javax.ejb.Timer)"
> -3.timeout.missing.possibleTypo = Timeout method missing or invalid.  There
> are {1} methods with the name "{0}" visible, none have the required
> signature of "void {0}(javax.ejb.Timer)"
> +3.timeout.missing.possibleTypo = Timeout method missing or invalid.  There
> are {1} methods with the name "{0}" visible, either the wrong one has been
> annotated with @Timeout or none have the required signature of "void
> {0}(javax.ejb.Timer). A bean should have only one method annotated with
> @Timeout and the method signature must match void {0}(javax.ejb.Timer)"
>
>  #
> fail(componentName,"timeout.tooManyMethods",timeoutMethods.size(),Join.join(",",
> timeoutMethods));
>  1.timeout.tooManyMethods = More than one method annotated with @Timeout
>
> Modified:
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> URL:
> http://svn.apache.org/viewvc/openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java?rev=966073&r1=966072&r2=966073&view=diff
>
> ==============================================================================
> ---
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> (original)
> +++
> openejb/trunk/openejb3/container/openejb-core/src/test/java/org/apache/openejb/config/rules/CheckInvalidTimeoutTest.java
> Wed Jul 21 02:51:58 2010
> @@ -26,26 +26,40 @@ import org.junit.runner.RunWith;
>
>  @RunWith(ValidationRunner.class)
>  public class CheckInvalidTimeoutTest extends TestCase {
> -    @Keys( { @Key(value = "timeout.badReturnType"),
> @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") })
> +    @Keys( { @Key(value = "timeout.badReturnType"),
> @Key("timeout.invalidArguments"), @Key("timeout.tooManyMethods") ,
> @Key("timeout.missing.possibleTypo")})
>     public EjbJar test() throws Exception {
>         System.setProperty("openejb.validation.output.level", "VERBOSE");
>         EjbJar ejbJar = new EjbJar();
>         ejbJar.addEnterpriseBean(new StatelessBean(TestBean.class));
>         ejbJar.addEnterpriseBean(new StatelessBean(FooBean.class));
> +        ejbJar.addEnterpriseBean(new StatelessBean(BarBean.class));
>         return ejbJar;
>     }
> -
> +// A case where the class has the method with wrong signature annotated
> with @Timeout
> +    // timeout.badReturnType
> +    // timeout.invalidArguments
>     public static class TestBean {
>         @Timeout
>         public Object bar() {
>             return null;
>         }
>     }
> -
> +// A case where the class has more than one method annotated with @Timeout
> +    // timeout.tooManyMethods
>     public static class FooBean {
>         @Timeout
>         public void foo(javax.ejb.Timer timer) {}
>         @Timeout
>         public void bar(javax.ejb.Timer timer) {}
>     }
> -}
> \ No newline at end of file
> +//  A case where the class has overloaded methods, but the method with the
> wrong signature has been annotated with @Timeout
> +    // timeout.missing.possibleTypo
> +    public static class BarBean {
> +
> +        public void foo(javax.ejb.Timer timer) {}
> +
> +        @Timeout
> +        public void foo() {}
> +    }
> +
> +}
>
>
>


-- 
Ivan