From 051e2b791c88e09a0f7cdecfb53eb2fd52c1d39c Mon Sep 17 00:00:00 2001 From: Sami Nurminen Date: Wed, 20 Dec 2017 21:47:47 +0200 Subject: [PATCH] AMQ-6327 - getNextScheduledTime() returns incorrect time when working with day of month --- .../activemq/broker/scheduler/CronParser.java | 28 +++++++++---- .../broker/scheduler/CronParserTest.java | 41 ++++++++++++++----- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/activemq-client/src/main/java/org/apache/activemq/broker/scheduler/CronParser.java b/activemq-client/src/main/java/org/apache/activemq/broker/scheduler/CronParser.java index 04a1df7c32..3aad4ccbc4 100644 --- a/activemq-client/src/main/java/org/apache/activemq/broker/scheduler/CronParser.java +++ b/activemq-client/src/main/java/org/apache/activemq/broker/scheduler/CronParser.java @@ -70,13 +70,13 @@ public class CronParser { // so we'll need to check again when done updating month and day. int currentMinutes = working.get(Calendar.MINUTE); if (!isCurrent(minutes, currentMinutes)) { - int nextMinutes = getNext(minutes, currentMinutes); + int nextMinutes = getNext(minutes, currentMinutes, working); working.add(Calendar.MINUTE, nextMinutes); } int currentHours = working.get(Calendar.HOUR_OF_DAY); if (!isCurrent(hours, currentHours)) { - int nextHour = getNext(hours, currentHours); + int nextHour = getNext(hours, currentHours, working); working.add(Calendar.HOUR_OF_DAY, nextHour); } @@ -100,13 +100,13 @@ public class CronParser { currentHours = working.get(Calendar.HOUR_OF_DAY); if (!isCurrent(hours, currentHours)) { - int nextHour = getNext(hours, currentHours); + int nextHour = getNext(hours, currentHours, working); working.add(Calendar.HOUR_OF_DAY, nextHour); } currentMinutes = working.get(Calendar.MINUTE); if (!isCurrent(minutes, currentMinutes)) { - int nextMinutes = getNext(minutes, currentMinutes); + int nextMinutes = getNext(minutes, currentMinutes, working); working.add(Calendar.MINUTE, nextMinutes); } @@ -123,7 +123,7 @@ public class CronParser { int currentMonth = working.get(Calendar.MONTH) + 1; if (!isCurrent(month, currentMonth)) { - int nextMonth = getNext(month, currentMonth); + int nextMonth = getNext(month, currentMonth, working); working.add(Calendar.MONTH, nextMonth); // Reset to start of month. @@ -150,11 +150,13 @@ public class CronParser { int nextCalendarDay = Integer.MAX_VALUE; if (!isCurrent(dayOfWeek, currentDayOfWeek)) { - nextWeekDay = getNext(dayOfWeek, currentDayOfWeek); + nextWeekDay = getNext(dayOfWeek, currentDayOfWeek, working); + System.out.println("nextWeekDay:"+nextCalendarDay); + } if (!isCurrent(dayOfMonth, currentDayOfMonth)) { - nextCalendarDay = getNext(dayOfMonth, currentDayOfMonth); + nextCalendarDay = getNext(dayOfMonth, currentDayOfMonth, working); } if( nextWeekDay < nextCalendarDay ) { @@ -190,7 +192,7 @@ public class CronParser { } } - static int getNext(final CronEntry entry, final int current) throws MessageFormatException { + static int getNext(final CronEntry entry, final int current, final Calendar working) throws MessageFormatException { int result = 0; if (entry.currentWhen == null) { @@ -209,7 +211,15 @@ public class CronParser { result = next - current; } else { int first = list.get(0).intValue(); - result = entry.end + first - entry.start - current; + + int fixedEnd = entry.end; + + //months have different max values + if("DayOfMonth".equals(entry.name)) { + fixedEnd = working.getActualMaximum(Calendar.DAY_OF_MONTH)+1; + } + + result = fixedEnd + first - entry.start - current; // Account for difference of one vs zero based indices. if (entry.name.equals("DayOfWeek") || entry.name.equals("Month")) { diff --git a/activemq-client/src/test/java/org/apache/activemq/broker/scheduler/CronParserTest.java b/activemq-client/src/test/java/org/apache/activemq/broker/scheduler/CronParserTest.java index af9f1aae7c..05be539d6e 100644 --- a/activemq-client/src/test/java/org/apache/activemq/broker/scheduler/CronParserTest.java +++ b/activemq-client/src/test/java/org/apache/activemq/broker/scheduler/CronParserTest.java @@ -20,6 +20,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.fail; +import java.time.*; import java.util.Calendar; import java.util.List; import javax.jms.MessageFormatException; @@ -338,11 +339,11 @@ public class CronParserTest { public void testGetNextCommaSeparated() throws MessageFormatException { String token = "3,5,7"; // test minimum values - int next = CronParser.getNext(createEntry(token, 1, 10), 3); + int next = CronParser.getNext(createEntry(token, 1, 10), 3, null); assertEquals(2, next); - next = CronParser.getNext(createEntry(token, 1, 10), 8); + next = CronParser.getNext(createEntry(token, 1, 10), 8, null); assertEquals(4, next); - next = CronParser.getNext(createEntry(token, 1, 10), 1); + next = CronParser.getNext(createEntry(token, 1, 10), 1, null); assertEquals(2, next); } @@ -350,24 +351,24 @@ public class CronParserTest { public void testGetNextRange() throws MessageFormatException { String token = "3-5"; // test minimum values - int next = CronParser.getNext(createEntry(token, 1, 10), 3); + int next = CronParser.getNext(createEntry(token, 1, 10), 3, null); assertEquals(1, next); - next = CronParser.getNext(createEntry(token, 1, 10), 5); + next = CronParser.getNext(createEntry(token, 1, 10), 5, null); assertEquals(7, next); - next = CronParser.getNext(createEntry(token, 1, 10), 6); + next = CronParser.getNext(createEntry(token, 1, 10), 6, null); assertEquals(6, next); - next = CronParser.getNext(createEntry(token, 1, 10), 1); + next = CronParser.getNext(createEntry(token, 1, 10), 1, null); assertEquals(2, next); } @Test public void testGetNextExact() throws MessageFormatException { String token = "3"; - int next = CronParser.getNext(createEntry(token, 0, 10), 2); + int next = CronParser.getNext(createEntry(token, 0, 10), 2, null); assertEquals(1, next); - next = CronParser.getNext(createEntry(token, 0, 10), 3); + next = CronParser.getNext(createEntry(token, 0, 10), 3, null); assertEquals(10, next); - next = CronParser.getNext(createEntry(token, 0, 10), 1); + next = CronParser.getNext(createEntry(token, 0, 10), 1, null); assertEquals(2, next); } @@ -402,6 +403,26 @@ public class CronParserTest { assertEquals(list.get(4), "4"); } + //added tests from https://issues.apache.org/jira/browse/AMQ-6327 + @Test + public void testGetNext() throws MessageFormatException { + testGetNextSingle("0 0 1 * *", "2016-04-15T00:00:00", "2016-05-01T00:00:00"); + testGetNextSingle("0 0 1,15 * *", "2016-04-15T00:00:00", "2016-05-01T00:00:00"); + testGetNextSingle("0 0 1 * *", "2016-05-15T00:00:00", "2016-06-01T00:00:00"); + testGetNextSingle("0 0 1,15 * *", "2016-05-15T00:00:00", "2016-06-01T00:00:00"); + testGetNextSingle("0 0 1 * *", "2016-06-15T00:00:00", "2016-07-01T00:00:00"); + testGetNextSingle("0 0 1,15 * *", "2016-06-15T00:00:00", "2016-07-01T00:00:00"); + } + + private void testGetNextSingle(String cronExp, String now, String expected) throws MessageFormatException { + + LocalDateTime nowDate = LocalDateTime.parse(now); + LocalDateTime expDate = LocalDateTime.parse(expected); + long next = CronParser.getNextScheduledTime(cronExp, nowDate.toInstant(OffsetDateTime.now().getOffset()).toEpochMilli()); + + assertEquals(expDate, LocalDateTime.ofInstant(Instant.ofEpochMilli(next), ZoneId.systemDefault())); + } + public void testGetNextScheduledTime() { fail("Not yet implemented"); }