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 <[email protected]>
> 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 <[email protected]> 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 <[email protected]>
> >
> > > 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
