You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tomee.apache.org by jl...@apache.org on 2021/01/14 09:51:38 UTC
[tomee] 01/01: TOMEE-2937 Make sure endDate in the past does not
fail but puts the Timer into a state where it won't trigger
This is an automated email from the ASF dual-hosted git repository.
jlmonteiro pushed a commit to branch TOMEE-2937_EjbCronTriggerEnd
in repository https://gitbox.apache.org/repos/asf/tomee.git
commit 5c9d471b7c7ee5bc2090aebe50d2423e03134bd9
Author: Jean-Louis Monteiro <jl...@tomitribe.com>
AuthorDate: Thu Jan 14 10:50:52 2021 +0100
TOMEE-2937 Make sure endDate in the past does not fail but puts the Timer into a state where it won't trigger
---
.../openejb/core/timer/CalendarTimerData.java | 3 +-
.../apache/openejb/core/timer/EJBCronTrigger.java | 48 ++++++++-
.../openejb/core/timer/EjbTimerServiceImpl.java | 2 +-
.../org/apache/openejb/core/timer/TimerData.java | 16 ++-
.../openejb/core/timer/TimerExpiredException.java | 23 ++++
.../apache/openejb/timer/EJBCronTriggerTest.java | 17 +++
.../openejb/timer/InitialIntervalTimerTest.java | 116 +++++++++++++++++++--
7 files changed, 208 insertions(+), 17 deletions(-)
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/CalendarTimerData.java b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/CalendarTimerData.java
index 975b804..8fb3199 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/CalendarTimerData.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/CalendarTimerData.java
@@ -17,6 +17,7 @@
package org.apache.openejb.core.timer;
+import org.apache.commons.lang3.builder.ToStringBuilder;
import org.apache.openejb.core.timer.EJBCronTrigger.ParseException;
import org.apache.openejb.quartz.impl.triggers.AbstractTrigger;
@@ -83,6 +84,6 @@ public class CalendarTimerData extends TimerData {
@Override
public String toString() {
- return TimerType.Calendar.name() + " scheduleExpression = [" + scheduleExpression + "]";
+ return TimerType.Calendar.name() + " scheduleExpression = [" + ToStringBuilder.reflectionToString(scheduleExpression) + "]";
}
}
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EJBCronTrigger.java b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EJBCronTrigger.java
index 52b6b16..4b4e9f3 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EJBCronTrigger.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EJBCronTrigger.java
@@ -104,6 +104,7 @@ public class EJBCronTrigger extends CronTriggerImpl {
private final TimeZone timezone;
private final String rawValue;
+ private final boolean isExpired;
public EJBCronTrigger(final ScheduleExpression expr) throws ParseException {
@@ -118,7 +119,32 @@ public class EJBCronTrigger extends CronTriggerImpl {
timezone = expr.getTimezone() == null ? TimeZone.getDefault() : TimeZone.getTimeZone(expr.getTimezone());
setStartTime(expr.getStart() == null ? new Date() : expr.getStart());
- setEndTime(expr.getEnd());
+
+ /*
+ * @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.
+ *
+ * EJB32 TCK test tries to create an already expired Timer and it's supposed to not fail.
+ * This may happen whe you restart an application for instance.
+ * On the other hand, Quartz does not allow endTime to be before StartTime so we need to check first so we don't
+ * set the endDate but we flag up this timer as being expired.
+ *
+ * When the first time is computed we will fail and as a consequence TimerData will be flagged up as being expired.
+ * So if endDate is not set or endTime after startTime, then we can consider this timer as not expired.
+ * If endTime is set and it's before startTime, we swallow setEndTime to Quartz and set the expired flag to true
+ */
+ if (expr.getEnd() == null || !isBefore(expr.getEnd(), getStartTime())) {
+ setEndTime(expr.getEnd());
+ isExpired = false;
+
+ } else {
+ isExpired = true;
+ }
// If parsing fails on a field, record the error and move to the next field
final Map<Integer, ParseException> errors = new HashMap<>();
@@ -143,6 +169,10 @@ public class EJBCronTrigger extends CronTriggerImpl {
+ DELIMITER + expr.getHour() + DELIMITER + expr.getMinute() + DELIMITER + expr.getSecond();
}
+ private boolean isBefore(final Date end, final Date start) {
+ return start != null && end != null && start.after(end);
+ }
+
/**
* Computes a set of allowed values for the given field of a calendar based
* time expression.
@@ -354,7 +384,7 @@ public class EJBCronTrigger extends CronTriggerImpl {
} else if (currentFieldIndex >= 1) {
// No suitable value was found, so move back to the previous field
// and decrease the value
- final int maxAffectedFieldType = upadteCalendar(calendar, expressions[currentFieldIndex - 1].field, -1);
+ final int maxAffectedFieldType = updateCalendar(calendar, expressions[currentFieldIndex - 1].field, -1);
currentFieldIndex = CALENDAR_FIELD_TYPE_ORDERED_INDEX_MAP.get(maxAffectedFieldType);
resetFields(calendar, maxAffectedFieldType, true);
} else {
@@ -368,6 +398,16 @@ public class EJBCronTrigger extends CronTriggerImpl {
}
@Override
+ public Date computeFirstFireTime(final org.apache.openejb.quartz.Calendar calendar) {
+ // timer may be expired up on creation (see constructor comments)
+ if (isExpired) {
+ throw new TimerExpiredException(String.format("Timer %s expired.", this));
+ }
+
+ return super.computeFirstFireTime(calendar);
+ }
+
+ @Override
public Date getFireTimeAfter(final Date afterTime) {
log.debug("start to getFireTimeAfter:" + afterTime);
final Calendar calendar = new GregorianCalendar(timezone);
@@ -447,7 +487,7 @@ public class EJBCronTrigger extends CronTriggerImpl {
// When current field is HOUR_OF_DAY, its upper field is DAY_OF_MONTH, so we need to -2 due to
// DAY_OF_WEEK.
final int parentFieldIndex = currentFieldIndex == 4 ? currentFieldIndex - 2 : currentFieldIndex - 1;
- final int maxAffectedFieldType = upadteCalendar(calendar, expressions[parentFieldIndex].field, 1);
+ final int maxAffectedFieldType = updateCalendar(calendar, expressions[parentFieldIndex].field, 1);
currentFieldIndex = CALENDAR_FIELD_TYPE_ORDERED_INDEX_MAP.get(maxAffectedFieldType);
resetFields(calendar, maxAffectedFieldType, false);
}
@@ -491,7 +531,7 @@ public class EJBCronTrigger extends CronTriggerImpl {
* @param field
* @return
*/
- private int upadteCalendar(final Calendar calendar, final int field, final int amount) {
+ private int updateCalendar(final Calendar calendar, final int field, final int amount) {
final Calendar old = new GregorianCalendar(timezone);
old.setTime(calendar.getTime());
calendar.add(field, amount);
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EjbTimerServiceImpl.java b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EjbTimerServiceImpl.java
index 22f5b00..effee7c 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EjbTimerServiceImpl.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/EjbTimerServiceImpl.java
@@ -773,7 +773,7 @@ public class EjbTimerServiceImpl implements EjbTimerService, Serializable {
try {
transactionManager.begin();
} catch (final Exception e) {
- log.warning("Exception occured while starting container transaction", e);
+ log.warning("Exception occurred while starting container transaction", e);
return;
}
}
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerData.java b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerData.java
index 0ae1403..5c3d5b6 100644
--- a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerData.java
+++ b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerData.java
@@ -219,13 +219,19 @@ public abstract class TimerData implements Serializable {
public void newTimer() {
//Initialize the Quartz Trigger
- trigger = initializeTrigger();
- trigger.computeFirstFireTime(null);
- trigger.setGroup(OPEN_EJB_TIMEOUT_TRIGGER_GROUP_NAME);
- trigger.setName(OPEN_EJB_TIMEOUT_TRIGGER_NAME_PREFIX + deploymentId + "_" + id);
- newTimer = true;
try {
+ trigger = initializeTrigger();
+ trigger.computeFirstFireTime(null);
+ trigger.setGroup(OPEN_EJB_TIMEOUT_TRIGGER_GROUP_NAME);
+ trigger.setName(OPEN_EJB_TIMEOUT_TRIGGER_NAME_PREFIX + deploymentId + "_" + id);
+ newTimer = true;
+
registerTimerDataSynchronization();
+
+ } catch (final TimerExpiredException e) {
+ setExpired(true);
+ log.warning("Timer {} is expired and will never trigger.", trigger);
+
} catch (final TimerStoreException e) {
throw new EJBException("Failed to register new timer data synchronization", e);
}
diff --git a/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerExpiredException.java b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerExpiredException.java
new file mode 100644
index 0000000..ec8f7e6
--- /dev/null
+++ b/container/openejb-core/src/main/java/org/apache/openejb/core/timer/TimerExpiredException.java
@@ -0,0 +1,23 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.openejb.core.timer;
+
+public class TimerExpiredException extends RuntimeException {
+ public TimerExpiredException(final String message) {
+ super(message);
+ }
+}
diff --git a/container/openejb-core/src/test/java/org/apache/openejb/timer/EJBCronTriggerTest.java b/container/openejb-core/src/test/java/org/apache/openejb/timer/EJBCronTriggerTest.java
index e1fdcb3..0cf244a 100644
--- a/container/openejb-core/src/test/java/org/apache/openejb/timer/EJBCronTriggerTest.java
+++ b/container/openejb-core/src/test/java/org/apache/openejb/timer/EJBCronTriggerTest.java
@@ -19,6 +19,7 @@ package org.apache.openejb.timer;
import org.apache.openejb.core.timer.EJBCronTrigger;
import org.apache.openejb.core.timer.EJBCronTrigger.ParseException;
+import org.apache.openejb.core.timer.TimerExpiredException;
import org.junit.Test;
import javax.ejb.ScheduleExpression;
@@ -27,12 +28,28 @@ import java.util.Date;
import java.util.GregorianCalendar;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
public class EJBCronTriggerTest {
+ @Test
+ public void shouldBeAbleToCreateExpiredTrigger() throws ParseException {
+ final ScheduleExpression expr = new ScheduleExpression().year(2008).month(12).dayOfMonth(1).end(new Date(0));
+ final EJBCronTrigger trigger = new EJBCronTrigger(expr);
+ assertNotNull(trigger);
+ }
+
+ @Test(expected = TimerExpiredException.class)
+ public void computeFailsOnExpiredTriggers() throws ParseException {
+ final ScheduleExpression expr = new ScheduleExpression().year(2008).month(12).dayOfMonth(1).end(new Date(0));
+ final EJBCronTrigger trigger = new EJBCronTrigger(expr);
+ assertNotNull(trigger);
+ trigger.computeFirstFireTime(null);
+ }
+
@Test(timeout = 1000)
public void testSimpleDate() throws ParseException {
final ScheduleExpression expr = new ScheduleExpression().year(2008).month(12).dayOfMonth(1).start(new Date(0));
diff --git a/container/openejb-core/src/test/java/org/apache/openejb/timer/InitialIntervalTimerTest.java b/container/openejb-core/src/test/java/org/apache/openejb/timer/InitialIntervalTimerTest.java
index 9f9252a..1390e48 100644
--- a/container/openejb-core/src/test/java/org/apache/openejb/timer/InitialIntervalTimerTest.java
+++ b/container/openejb-core/src/test/java/org/apache/openejb/timer/InitialIntervalTimerTest.java
@@ -16,10 +16,10 @@
*/
package org.apache.openejb.timer;
+import org.apache.commons.lang3.time.DateUtils;
import org.apache.openejb.jee.EnterpriseBean;
-import org.apache.openejb.jee.SingletonBean;
import org.apache.openejb.junit.ApplicationComposer;
-import org.apache.openejb.testing.Module;
+import org.apache.openejb.testing.Classes;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -29,6 +29,9 @@ import javax.annotation.Resource;
import javax.ejb.EJB;
import javax.ejb.Lock;
import javax.ejb.LockType;
+import javax.ejb.NoMoreTimeoutsException;
+import javax.ejb.NoSuchObjectLocalException;
+import javax.ejb.ScheduleExpression;
import javax.ejb.Singleton;
import javax.ejb.Startup;
import javax.ejb.Timeout;
@@ -36,24 +39,71 @@ import javax.ejb.Timer;
import javax.ejb.TimerConfig;
import javax.ejb.TimerService;
+import java.util.Calendar;
+import java.util.Date;
+
import static org.junit.Assert.assertEquals;
@RunWith(ApplicationComposer.class)
+@Classes(innerClassesAsBean = true)
public class InitialIntervalTimerTest {
- @Module
- public EnterpriseBean bean() {
- return new SingletonBean(TimerWithDelay.class).localBean();
- }
@EJB
private TimerWithDelay bean;
+ @EJB
+ private TimerNeverExpires scheduleBean;
+
@Test
public void test() throws InterruptedException {
Thread.sleep(5400);
assertEquals(3, bean.getOk());
}
+ @Test
+ public void endNeverExpires() {
+ final Calendar cal = Calendar.getInstance();
+ final int currentYear = getForSchedule(Calendar.YEAR, cal);
+ cal.add(Calendar.SECOND, 10);
+ final ScheduleExpression exp = getPreciseScheduleExpression(cal);
+ final Date end = DateUtils.setYears(cal.getTime(), cal.get(Calendar.YEAR) - 1);
+ exp.end(end);
+ exp.year((currentYear) + " - " + (currentYear + 1));
+
+ final Timer timer = scheduleBean.createTimer(exp);
+ scheduleBean.passIfNoMoreTimeouts(timer);
+ }
+
+ public static ScheduleExpression getPreciseScheduleExpression(final Calendar... cals) {
+ Calendar cal = (cals.length == 0) ? Calendar.getInstance() : cals[0];
+ return new ScheduleExpression()
+ .year(cal.get(Calendar.YEAR)).month(getForSchedule(Calendar.MONTH, cal))
+ .dayOfMonth(getForSchedule(Calendar.DAY_OF_MONTH, cal))
+ .hour(getForSchedule(Calendar.HOUR_OF_DAY, cal))
+ .minute(getForSchedule(Calendar.MINUTE, cal))
+ .second(getForSchedule(Calendar.SECOND, cal));
+ }
+
+ public static int getForSchedule(final int field, final Calendar... calendars) {
+ int result = 0;
+ Calendar cal = null;
+ if (calendars.length == 0) {
+ cal = Calendar.getInstance();
+ } else {
+ cal = calendars[0];
+ }
+ result = cal.get(field);
+ if (field == Calendar.DAY_OF_WEEK) {
+ result--; // 0 and 7 are both Sunday
+ if (result == 0) {
+ result = (Math.random() < 0.5) ? 0 : 7;
+ }
+ } else if (field == Calendar.MONTH) {
+ result++;
+ }
+ return result;
+ }
+
@Singleton
@Startup
@Lock(LockType.READ)
@@ -85,4 +135,58 @@ public class InitialIntervalTimerTest {
timer.cancel();
}
}
+
+ @Singleton
+ @Startup
+ @Lock(LockType.READ)
+ public static class TimerNeverExpires {
+
+ @Resource
+ private TimerService timerService;
+
+ private int ok = 0;
+
+ @Timeout
+ public void timeout(final Timer timer) {
+ ok++;
+ }
+
+ public Timer createTimer(final ScheduleExpression exp) {
+ return timerService.createCalendarTimer(exp, new TimerConfig("TimerNeverExpires", false));
+ }
+
+ public String passIfNoMoreTimeouts(final Timer t) {
+ String result = "";
+ try {
+ Date nextTimeout = t.getNextTimeout();
+ throw new RuntimeException(
+ "Expecting NoSuchObjectLocalException or NoMoreTimeoutsException "
+ + "when accessing getNextTimeout, but actual " + nextTimeout);
+
+ } catch (final NoMoreTimeoutsException e) {
+ result += " Got expected " + e;
+
+ } catch (final NoSuchObjectLocalException e) {
+ result += " Got expected " + e;
+
+ }
+
+ try {
+ long timeRemaining = t.getTimeRemaining();
+ throw new RuntimeException(
+ "Expecting NoSuchObjectLocalException or NoMoreTimeoutsException "
+ + "when accessing getTimeRemaining, but actual " + timeRemaining);
+
+ } catch (final NoMoreTimeoutsException e) {
+ result += " Got expected " + e;
+
+ } catch (final NoSuchObjectLocalException e) {
+ result += " Got expected " + e;
+
+ }
+ return result;
+ }
+
+
+ }
}