search

jakartaee / enterprise-beans

Jakarta Enterprise Beans
https://eclipse.org/ee4j/ejb
Other
20 stars 29 forks source link

[TCK] EJB 3.2 timers issue with endDate #137

Closed jeanouii closed 3 years ago

jeanouii commented 3 years ago

Hi community,

I'm working on the TCK for Apache TomEE and got an interesting failure I'd like to discuss here.

To run them, use the following command

./runtests --web tomee-plume com.sun.ts.tests.ejb32.lite.timer.schedule.expire.Client

This is the part failing

https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/ejb32/lite/timer/schedule/expire/Client.java#L231

java.lang.IllegalArgumentException: End time cannot be before start time at org.apache.openejb.quartz.impl.triggers.CronTriggerImpl.setEndTime(CronTriggerImpl.java:417) at org.apache.openejb.core.timer.EJBCronTrigger.(EJBCronTrigger.java:121) at org.apache.openejb.core.timer.CalendarTimerData.initializeTrigger(CalendarTimerData.java:61) at org.apache.openejb.core.timer.TimerData.newTimer(TimerData.java:222) at org.apache.openejb.core.timer.EjbTimerServiceImpl.initializeNewTimer(EjbTimerServiceImpl.java:738) at org.apache.openejb.core.timer.EjbTimerServiceImpl.createTimer(EjbTimerServiceImpl.java:715) at org.apache.openejb.core.timer.TimerServiceImpl.createCalendarTimer(TimerServiceImpl.java:83) at org.apache.openejb.core.timer.TimerServiceWrapper.createCalendarTimer(TimerServiceWrapper.java:79) at com.sun.ts.tests.ejb30.timer.common.TimerBeanBaseWithoutTimeOutMethod.createTimer(TimerBeanBaseWithoutTimeOutMethod.java:140)   When we want to set the endTime to Quartz, we get the exception above which does not sound stupid.

The test says 

   * @testName: endNeverExpires
   * 
   * @test_Strategy: create a timer with year="currentYear - currentYear+1", and
   * end="currentYear-1". The end value is prior to the year values, and this
   * timer will never expire. Creating this timer will succeed, but any timer
   * method access in a subsequent business method will result in
   * NoSuchObjectLocalException.

To me it appears to be a misuse of the endDate. Did I miss something?

tkburroughs commented 3 years ago

I believe this has been the required behavior of EJB timers since Calendar based timers were introduced.

I don't find anything specific about this in the EJB specification, though I can imagine the following in the "Throws" clause of TimerService.createCalendarTimer(...) might be interpreted to mean it should not be allowed:

IllegalArgumentException - If Schedule represents an invalid schedule expression.

However, I don't think this was intended to mean a schedule is invalid just because it has no future timeouts. I believe the expectation has been that if each individual attribute of the ScheduleExpression is correct, then the ScheduleExpression is considered to be valid and an IllegalArgumentException should not occur. For example, if you do something like this:

ScheduleExpression exp = new ScheduleExpression(); exp.month(35); timerService.createCalendarTimer(exp);

Then the call to createCalendarTimer() would fail because "35" is not a valid month.

However, as long as each attribute has a valid value, then the ScheduleExpression is valid and timer creation should succeed. For example, just because a ScheduleExpression may represent only timeouts that occurred in the past doesn't mean it is invalid.... just that the timer will not expire going forward.... and thus should be considered to not exist immediately on creation; which is what the test is validating.

Likely this approach was taken to handle "automatic" timers. If an application uses @Schedule with attributes with no future timeouts, should the application fail to deploy due to an IllegalArgumentException, or should it deploy just fine, but that particular timer will just have no timeouts. I think most would prefer the later; otherwise applications that used to deploy and work fine could start failing to deploy at some future time.

I'll admit this specific scenario is odd though, as there is no point in time that the schedule would ever result in an expiration... but that can only be determined by validating the different attributes together, rather than individually.

I would not be in favor of changing the current expectations, as it could cause regressions in existing applications that are inadvertently creating timers that never expire... but are working fine otherwise.

-- Tracy Burroughs (tkb@us.ibm.com) -- WebSphere Application Server Development -- IBM Rochester, Dept WG8A H315/050-2 -- 2800 37th Street NW, Rochester MN 55901-4441

From: Jean-Louis Monteiro notifications@github.com To: eclipse-ee4j/ejb-api ejb-api@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Date: 12/02/2020 11:42 AM Subject: [EXTERNAL] [eclipse-ee4j/ejb-api] [TCK] EJB 3.2 timers issue with endDate (#137)

Hi community, I'm working on the TCK for Apache TomEE and got an interesting failure I'd like to discuss here. To run them, use the following command ./runtests --web tomee-plume com.sun.ts.tests.ejb32.lite.timer.schedule.expire.Client This is the part failing https://github.com/eclipse-ee4j/jakartaee-tck/blob/master/src/com/sun/ts/tests/ejb32/lite/timer/schedule/expire/Client.java#L231 java.lang.IllegalArgumentException: End time cannot be before start time at org.apache.openejb.quartz.impl.triggers.CronTriggerImpl.setEndTime(CronTriggerImpl.java:417) at org.apache.openejb.core.timer.EJBCronTrigger.(EJBCronTrigger.java:121) at org.apache.openejb.core.timer.CalendarTimerData.initializeTrigger(CalendarTimerData.java:61) at org.apache.openejb.core.timer.TimerData.newTimer(TimerData.java:222) at org.apache.openejb.core.timer.EjbTimerServiceImpl.initializeNewTimer(EjbTimerServiceImpl.java:738) at org.apache.openejb.core.timer.EjbTimerServiceImpl.createTimer(EjbTimerServiceImpl.java:715) at org.apache.openejb.core.timer.TimerServiceImpl.createCalendarTimer(TimerServiceImpl.java:83) at org.apache.openejb.core.timer.TimerServiceWrapper.createCalendarTimer(TimerServiceWrapper.java:79) at com.sun.ts.tests.ejb30.timer.common.TimerBeanBaseWithoutTimeOutMethod.createTimer(TimerBeanBaseWithoutTimeOutMethod.java:140)

When we want to set the endTime to Quartz, we get the exception above which does not sound stupid. The test says

To me it appears to be a misuse of the endDate. Did I miss something? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

jeanouii commented 3 years ago

Thank you very much for all context and information. It perfectly makes sense. I have been able to successfully fix TomEE to pass this test and behave accordingly.

I'll proceed and close this ticket