Backport SessionScheduleTest coverage for previous-day start-of-week handling

quickfixj-core/src/main/java/quickfix/DefaultSessionSchedule.java

@@ -22,8 +22,12 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import java.text.SimpleDateFormat;
-import java.util.Calendar;
+import java.time.Instant;
+import java.time.ZoneId;
+import java.time.ZonedDateTime;
+import java.time.format.DateTimeFormatter;
+import java.time.temporal.WeekFields;
+import java.util.Locale;
 import java.util.TimeZone;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -41,13 +45,11 @@ public class DefaultSessionSchedule implements SessionSchedule {
     private final int[] weekdayOffsets;
     protected static final Logger LOG = LoggerFactory.getLogger(DefaultSessionSchedule.class);
 
-    //Cache recent time data to reduce creation of calendar objects
-    private final ThreadLocal<Calendar> threadLocalCalendar;
+    //Cache recent time data to reduce creation of objects
     private final ThreadLocal<TimeInterval> threadLocalRecentTimeInterval;
 
     public DefaultSessionSchedule(SessionSettings settings, SessionID sessionID) throws ConfigError,
             FieldConvertError {
-        threadLocalCalendar = ThreadLocal.withInitial(SystemTime::getUtcCalendar);
         threadLocalRecentTimeInterval = new ThreadLocal<>();
         isNonStopSession = settings.isSetting(sessionID, Session.SETTING_NON_STOP_SESSION)
             && settings.getBool(sessionID, Session.SETTING_NON_STOP_SESSION);
@@ -187,73 +189,70 @@ TimeZone getTimeZone() {
     /**
      * find the most recent session date/time range on or before t
      * if t is in a session then that session will be returned
-     * @param t specific date/time
+     * @param epochMillis specific date/time as epoch milliseconds
      * @return relevant session date/time range
      */
-    private TimeInterval theMostRecentIntervalBefore(Calendar t) {
-        TimeInterval timeInterval = new TimeInterval();
-        Calendar intervalStart = timeInterval.getStart();
-        intervalStart.setTimeZone(startTime.getTimeZone());
-        intervalStart.setTimeInMillis(t.getTimeInMillis());
-        intervalStart.set(Calendar.HOUR_OF_DAY, startTime.getHour());
-        intervalStart.set(Calendar.MINUTE, startTime.getMinute());
-        intervalStart.set(Calendar.SECOND, startTime.getSecond());
-        intervalStart.set(Calendar.MILLISECOND, 0);
-
-        Calendar intervalEnd = timeInterval.getEnd();
-        intervalEnd.setTimeZone(endTime.getTimeZone());
-        intervalEnd.setTimeInMillis(t.getTimeInMillis());
-        intervalEnd.set(Calendar.HOUR_OF_DAY, endTime.getHour());
-        intervalEnd.set(Calendar.MINUTE, endTime.getMinute());
-        intervalEnd.set(Calendar.SECOND, endTime.getSecond());
-        intervalEnd.set(Calendar.MILLISECOND, 0);
+    private TimeInterval theMostRecentIntervalBefore(long epochMillis) {
+        ZonedDateTime intervalStart = ZonedDateTime
+                .ofInstant(Instant.ofEpochMilli(epochMillis), startTime.getTimeZone().toZoneId())
+                .withHour(startTime.getHour()).withMinute(startTime.getMinute())
+                .withSecond(startTime.getSecond()).withNano(0);
+
+        ZonedDateTime intervalEnd = ZonedDateTime
+                .ofInstant(Instant.ofEpochMilli(epochMillis), endTime.getTimeZone().toZoneId())
+                .withHour(endTime.getHour()).withMinute(endTime.getMinute())
+                .withSecond(endTime.getSecond()).withNano(0);
 
         if (isWeekdaySession) {
-            while (intervalStart.getTimeInMillis() > t.getTimeInMillis() ||
-                    !validDayOfWeek(intervalStart)) {
-                intervalStart.add(Calendar.DAY_OF_WEEK, -1);
-                intervalEnd.add(Calendar.DAY_OF_WEEK, -1);
+            while (intervalStart.toInstant().toEpochMilli() > epochMillis || !validDayOfWeek(intervalStart)) {
+                intervalStart = intervalStart.minusDays(1);
+                intervalEnd = intervalEnd.minusDays(1);
             }
-
-            while (intervalEnd.getTimeInMillis() <= intervalStart.getTimeInMillis()) {
-                intervalEnd.add(Calendar.DAY_OF_WEEK, 1);
+            while (intervalEnd.toInstant().toEpochMilli() <= intervalStart.toInstant().toEpochMilli()) {
+                intervalEnd = intervalEnd.plusDays(1);
             }
-
         } else {
             if (isSet(startTime.getDay())) {
-                intervalStart.set(Calendar.DAY_OF_WEEK, startTime.getDay());
-                if (intervalStart.getTimeInMillis() > t.getTimeInMillis()) {
-                    intervalStart.add(Calendar.WEEK_OF_YEAR, -1);
-                    intervalEnd.add(Calendar.WEEK_OF_YEAR, -1);
+                intervalStart = intervalStart.with(WeekFields.SUNDAY_START.dayOfWeek(), startTime.getDay());
+                if (intervalStart.toInstant().toEpochMilli() > epochMillis) {
+                    intervalStart = intervalStart.minusWeeks(1);
+                    intervalEnd = intervalEnd.minusWeeks(1);
                 }
-            } else if (intervalStart.getTimeInMillis() > t.getTimeInMillis()) {
-                intervalStart.add(Calendar.DAY_OF_YEAR, -1);
-                intervalEnd.add(Calendar.DAY_OF_YEAR, -1);
+            } else if (intervalStart.toInstant().toEpochMilli() > epochMillis) {
+                intervalStart = intervalStart.minusDays(1);
+                intervalEnd = intervalEnd.minusDays(1);
             }
 
             if (isSet(endTime.getDay())) {
-                intervalEnd.set(Calendar.DAY_OF_WEEK, endTime.getDay());
-                if (intervalEnd.getTimeInMillis() <= intervalStart.getTimeInMillis()) {
-                    intervalEnd.add(Calendar.WEEK_OF_MONTH, 1);
+                intervalEnd = intervalEnd.with(WeekFields.SUNDAY_START.dayOfWeek(), endTime.getDay());
+                if (intervalEnd.toInstant().toEpochMilli() <= intervalStart.toInstant().toEpochMilli()) {
+                    intervalEnd = intervalEnd.plusWeeks(1);
                 }
-            } else if (intervalEnd.getTimeInMillis() <= intervalStart.getTimeInMillis()) {
-                intervalEnd.add(Calendar.DAY_OF_WEEK, 1);
+            } else if (intervalEnd.toInstant().toEpochMilli() <= intervalStart.toInstant().toEpochMilli()) {
+                intervalEnd = intervalEnd.plusDays(1);
             }
         }
 
-        return timeInterval;
+        return new TimeInterval(
+                intervalStart.toInstant().toEpochMilli(),
+                intervalEnd.toInstant().toEpochMilli());
     }
 
     private static class TimeInterval {
-        private final Calendar start = SystemTime.getUtcCalendar();
-        private final Calendar end = SystemTime.getUtcCalendar();
+        private final long startMs;
+        private final long endMs;
+
+        TimeInterval(long startMs, long endMs) {
+            this.startMs = startMs;
+            this.endMs = endMs;
+        }
 
-        boolean isContainingTime(Calendar t) {
-            return t.compareTo(start) >= 0 && t.compareTo(end) <= 0;
+        boolean isContainingTime(long epochMillis) {
+            return epochMillis >= startMs && epochMillis <= endMs;
         }
 
         public String toString() {
-            return start.getTime() + " --> " + end.getTime();
+            return Instant.ofEpochMilli(startMs) + " --> " + Instant.ofEpochMilli(endMs);
         }
 
         public boolean equals(Object other) {
@@ -264,25 +263,25 @@ public boolean equals(Object other) {
                 return false;
             }
             TimeInterval otherInterval = (TimeInterval) other;
-            return start.equals(otherInterval.start) && end.equals(otherInterval.end);
+            return startMs == otherInterval.startMs && endMs == otherInterval.endMs;
         }
 
         public int hashCode() {
             assert false : "hashCode not supported";
             return 0;
         }
 
-        Calendar getStart() {
-            return start;
+        long getStartMs() {
+            return startMs;
         }
 
-        Calendar getEnd() {
-            return end;
+        long getEndMs() {
+            return endMs;
         }
     }
 
     @Override
-    public boolean isSameSession(Calendar time1, Calendar time2) {
+    public boolean isSameSession(long time1, long time2) {
         if (isNonStopSession())
             return true;
         TimeInterval interval1 = theMostRecentIntervalBefore(time1);
@@ -307,28 +306,23 @@ public boolean isSessionTime() {
         if(isNonStopSession()) {
             return true;
         }
-        Calendar now = threadLocalCalendar.get();
-        now.setTimeInMillis(SystemTime.currentTimeMillis());
+        long nowMs = SystemTime.currentTimeMillis();
         TimeInterval mostRecentInterval = threadLocalRecentTimeInterval.get();
-        if (mostRecentInterval != null && mostRecentInterval.isContainingTime(now)) {
+        if (mostRecentInterval != null && mostRecentInterval.isContainingTime(nowMs)) {
             return true;
         }
-        mostRecentInterval = theMostRecentIntervalBefore(now);
-        boolean result = mostRecentInterval.isContainingTime(now);
+        mostRecentInterval = theMostRecentIntervalBefore(nowMs);
+        boolean result = mostRecentInterval.isContainingTime(nowMs);
         threadLocalRecentTimeInterval.set(mostRecentInterval);
         return result;
     }
 
     public String toString() {
         StringBuilder buf = new StringBuilder();
 
-        SimpleDateFormat dowFormat = new SimpleDateFormat("EEEE");
-        dowFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
-
-        SimpleDateFormat timeFormat = new SimpleDateFormat("HH:mm:ss-z");
-        timeFormat.setTimeZone(TimeZone.getTimeZone("UTC"));
+        DateTimeFormatter timeFormat = DateTimeFormatter.ofPattern("HH:mm:ss-z", Locale.getDefault());
 
-        TimeInterval ti = theMostRecentIntervalBefore(SystemTime.getUtcCalendar());
+        TimeInterval ti = theMostRecentIntervalBefore(SystemTime.currentTimeMillis());
 
         formatTimeInterval(buf, ti, timeFormat, false);
 
@@ -344,7 +338,7 @@ public String toString() {
     }
 
     private void formatTimeInterval(StringBuilder buf, TimeInterval timeInterval,
-                                    SimpleDateFormat timeFormat, boolean local) {
+                                    DateTimeFormatter timeFormat, boolean local) {
         if (isNonStopSession) {
             buf.append("nonstop");
             return;
@@ -365,21 +359,19 @@ private void formatTimeInterval(StringBuilder buf, TimeInterval timeInterval,
             buf.append("daily, ");
         }
 
-        if (local) {
-            timeFormat.setTimeZone(startTime.getTimeZone());
-        }
-        buf.append(timeFormat.format(timeInterval.getStart().getTime()));
+        ZoneId startZone = local ? startTime.getTimeZone().toZoneId() : TimeZone.getTimeZone("UTC").toZoneId();
+        buf.append(timeFormat.format(Instant.ofEpochMilli(timeInterval.getStartMs())
+                .atZone(startZone)));
 
         buf.append(" - ");
 
         if (!isDailySession()) {
             formatDayOfWeek(buf, endTime.getDay());
             buf.append(" ");
         }
-        if (local) {
-            timeFormat.setTimeZone(endTime.getTimeZone());
-        }
-        buf.append(timeFormat.format(timeInterval.getEnd().getTime()));
+        ZoneId endZone = local ? endTime.getTimeZone().toZoneId() : TimeZone.getTimeZone("UTC").toZoneId();
+        buf.append(timeFormat.format(Instant.ofEpochMilli(timeInterval.getEndMs())
+                .atZone(endZone)));
     }
 
     private void formatDayOfWeek(StringBuilder buf, int dayOfWeek) {
@@ -414,8 +406,8 @@ private static int timeInSeconds(int hour, int minute, int second) {
      * @param startDateTime time to test
      * @return flag indicating if valid
      */
-    private boolean validDayOfWeek(Calendar startDateTime) {
-        int dow = startDateTime.get(Calendar.DAY_OF_WEEK);
+    private boolean validDayOfWeek(ZonedDateTime startDateTime) {
+        int dow = (int) startDateTime.get(WeekFields.SUNDAY_START.dayOfWeek());
         for (int i = 0; i < weekdayOffsets.length; i++)
             if (weekdayOffsets[i] == dow)
                 return true;

quickfixj-core/src/main/java/quickfix/NoopStore.java

@@ -20,6 +20,7 @@
 
 package quickfix;
 
+import java.time.Instant;
 import java.util.Calendar;
 import java.util.Collection;
 import java.util.Date;
@@ -31,8 +32,7 @@
  */
 public class NoopStore implements MessageStore {
 
-    private Date creationTime = new Date();
-    private Calendar creationTimeCalendar = SystemTime.getUtcCalendar(creationTime);
+    private Date creationTime = Date.from(Instant.ofEpochMilli(SystemTime.currentTimeMillis()));
     private int nextSenderMsgSeqNum = 1;
     private int nextTargetMsgSeqNum = 1;
 
@@ -44,7 +44,7 @@ public Date getCreationTime() {
     }
 
     public Calendar getCreationTimeCalendar() {
-        return creationTimeCalendar;
+        return SystemTime.getUtcCalendar(creationTime);
     }
 
     public int getNextSenderMsgSeqNum() {
@@ -64,7 +64,7 @@ public void incrNextTargetMsgSeqNum() {
     }
 
     public void reset() {
-        creationTime = new Date();
+        creationTime = Date.from(Instant.ofEpochMilli(SystemTime.currentTimeMillis()));
         nextSenderMsgSeqNum = 1;
         nextTargetMsgSeqNum = 1;
     }

quickfixj-core/src/main/java/quickfix/Session.java

@@ -658,7 +658,7 @@ public String getRemoteAddress() {
     private boolean isCurrentSession(final long time)
             throws IOException {
         return sessionSchedule == null || sessionSchedule.isSameSession(
-                SystemTime.getUtcCalendar(time), state.getCreationTimeCalendar());
+                time, state.getCreationTime().getTime());
     }
 
     /**

quickfixj-core/src/main/java/quickfix/SessionSchedule.java

@@ -19,20 +19,34 @@
 
 package quickfix;
 
-import java.util.Calendar;
+import java.time.ZonedDateTime;
 
 /**
  * Used to decide when to login and out of FIX sessions
  */
 public interface SessionSchedule {
 
+    /**
+     * Predicate for determining if the two epoch-millisecond times are in the same session.
+     *
+     * @param time1 first time in epoch milliseconds
+     * @param time2 second time in epoch milliseconds
+     * @return true if in the same session
+     */
+    boolean isSameSession(long time1, long time2);
+
     /**
      * Predicate for determining if the two times are in the same session
      * @param time1 test time 1
      * @param time2 test time 2
      * @return return true if in the same session
      */
-    boolean isSameSession(Calendar time1, Calendar time2);
+    default boolean isSameSession(ZonedDateTime time1, ZonedDateTime time2) {
+        if (time1 == null || time2 == null) {
+            return isNonStopSession();
+        }
+        return isSameSession(time1.toInstant().toEpochMilli(), time2.toInstant().toEpochMilli());
+    }
 
     boolean isNonStopSession();
 

quickfixj-core/src/test/java/quickfix/DefaultSessionScheduleTest.java

@@ -8,6 +8,7 @@
 
 import java.io.ByteArrayInputStream;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.when;
@@ -117,4 +118,22 @@ public void isSessionTime_returns_false_for_time_outside_window() throws FieldCo
 
         assertFalse(schedule.isSessionTime());
     }
+
+    @Test
+    public void toString_uses_expected_utc_time_format() throws FieldConvertError, ConfigError {
+        when(mockTimeSource.getTime()).thenReturn(1L);
+        String sessionSettingsString = ""
+            + "[DEFAULT]\n"
+            + "\n"
+            + "[SESSION]\n"
+            + "BeginString=FIX.4.2\n"
+            + "SenderCompID=A\n"
+            + "TargetCompID=B\n"
+            + "StartTime=00:00:00\n"
+            + "EndTime=00:00:01\n";
+        SessionSettings sessionSettings = new SessionSettings(new ByteArrayInputStream(sessionSettingsString.getBytes()));
+        DefaultSessionSchedule schedule = new DefaultSessionSchedule(sessionSettings, sessionID);
+
+        assertEquals("daily, 00:00:00-UTC - 00:00:01-UTC", schedule.toString());
+    }
 }

quickfixj-core/src/test/java/quickfix/LogUtilTest.java

@@ -22,7 +22,6 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.PrintStream;
-import java.util.Calendar;
 import java.util.Date;
 import org.junit.After;
 import static org.junit.Assert.assertTrue;
@@ -61,7 +60,7 @@ private void createSessionAndGenerateException(LogFactory mockLogFactory) throws
         Session session = new Session(null, sessionID1 -> {
             try {
                 return new MemoryStore() {
-                    public Calendar getCreationTimeCalendar() throws IOException {
+                    public Date getCreationTime() throws IOException {
                         throw new IOException("test");
                     }
                 };