openejb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ivan <xhh...@gmail.com>
Subject 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/
Date Thu, 29 Jul 2010 04:55:09 GMT
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 <karan.malhi@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 <xhhsld@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 <kmalhi@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message