Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Configuring StartTime and EndTime on different timezones results in wrong schedule calculation #486

Open
sregueiroqb opened this issue May 9, 2022 · 1 comment · May be fixed by #574
Open

Configuring StartTime and EndTime on different timezones results in wrong schedule calculation #486

sregueiroqb opened this issue May 9, 2022 · 1 comment · May be fixed by #574
Labels
bug

Comments

@sregueiroqb
Copy link

sregueiroqb commented May 9, 2022 •
edited

Describe the bug
We are connecting to an FX market that has an opening time on the Madrid timezone (at 00:00) and close time on the New York timezone (at 17:00), Monday to Friday, due to fixing reasons.
The local time is UTC, the application runs in kubernetes.
To avoid having to change the config when daylight savings changes, we setup the session the following way:

StartTime=23:55:00 Europe/Madrid
EndTime=17:05:00 America/New_York
Weekdays=Sun,Mon,Tue,Wed,Thu

We want to connect 5 minutes before the session opens and disconnect 5 minutes after.

Using this configuration, results on the session connecting at 23:55 Madrid (21:55 UTC) and disconnecting at 00:00 Madrid (22:00 UTC), reconnecting again at 06:00 Madrid (04:00 UTC)

The issue is found in DefaultSessionSchedule. theMostRecentIntervalBefore calculates the wrong interval when changing days and the close time is before open time.

The log shows the correct schedule [FIX.5.0:SENDER->TARGET] sunday, monday, tuesday, wednesday, thursday, 21:55:00-UTC - 21:05:00-UTC (sunday, monday, tuesday, wednesday, thursday, 23:55:00-MESZ - 17:05:00-EDT)

To Reproduce

package quickfix;

import static org.junit.Assert.assertEquals;

import java.io.ByteArrayInputStream;
import java.util.Calendar;
import java.util.GregorianCalendar;
import java.util.Locale;
import java.util.TimeZone;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;

import quickfix.ConfigError;
import quickfix.DefaultSessionSchedule;
import quickfix.SessionID;
import quickfix.SessionSchedule;
import quickfix.SessionSettings;
import quickfix.SystemTime;

import java.text.DateFormat;
import java.text.DateFormatSymbols;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.Date;

import quickfix.field.converter.UtcTimeOnlyConverter;

public class WrongFixSessionScheduleTest {

	private MockSystemTimeSource mockSystemTimeSource;
	private Locale defaultLocale;

	@Before
	public void setUp() throws Exception {
		mockSystemTimeSource = new MockSystemTimeSource();
		SystemTime.setTimeSource(mockSystemTimeSource);
		defaultLocale = Locale.getDefault();
		Locale.setDefault(Locale.GERMANY);
	}

	@After
	public void tearDown() throws Exception {
		SystemTime.setTimeSource(null);
		Locale.setDefault(defaultLocale);
	}

	@Test
	public void testSessionTimeEndBeforeStartUsingWeekdays() throws Exception {
		SessionSchedule schedule = newSessionSchedule();
		// Schedule should be (assuming daylight savings time):
		// 23:55 Sunday Madrid to 17:05 Monday NY // 21:55 Sunday UTC to 21:05 Monday UTC
		// 23:55 Monday Madrid to 17:05 Tuesday NY // 21:55 Monday UTC to 21:05 Tuesday UTC
		// 23:55 Tuesday Madrid to 17:05 Wednesday NY // 21:55 Tuesday UTC to 21:05 Wednesday UTC
		// 23:55 Wednesday Madrid to 17:05 Thursday NY // 21:55 Wednesday UTC to 21:05 Thursday UTC
		// 23:55 Thursday Madrid to 17:05 Friday NY // 21:55 Thursday UTC to 21:05 Friday UTC

		// Saturday
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 16, 21, 0, 0); // 23:00 Madrid
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 16, 22, 0, 0); // 00:00 Madrid next day
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 16, 22, 1, 0); // 00:01 Madrid next day
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 16, 23, 0, 0); // 01:00 Madrid next day

		// Sunday
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 17, 0, 1, 0); // 19:01 Madrid
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 17, 21, 54, 59);// 23:54 Madrid
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 17, 21, 55, 00); // 23:55 Madrid. Correct opening time
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 17, 21, 57, 0); // 23:57 Madrid
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 17, 22, 00, 0); // 00:00 Madrid next day. Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 17, 23, 57, 0); // 01:57 Madrid next day. Wrong, should be in session

		// Monday
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 0, 1, 0); // 02:01 Madrid. Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 3, 0, 1); // 05:00 Madrid. Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 3, 59, 59); // 05:59 Madrid. Wrong, should be in session
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 4, 00, 59); // 06:00 Madrid.
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 17, 00, 59); // 19:00 Madrid.
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 21, 00, 00); // 23:00 Madrid / 17:00 NY
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 21, 4, 59); // 23:04:59 Madrid / 17:04:59 NY
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 21, 5, 0); // 23:05:00 Madrid / 17:05 NY
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 21, 5, 1); // 23:05:01 Madrid / 17:05:01 NY. Correct close time
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 21, 54, 0); // 23:54:00 Madrid.
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 21, 55, 00); // 23:55 Madrid. Correct opening time
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 18, 21, 57, 0); // 23:57 Madrid
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 22, 56, 0); // Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 23, 6, 0); // Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 23, 54, 0); // Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 18, 23, 56, 0); // Wrong, should be in session
		// Tuesday
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 19, 0, 1, 0); // Wrong, should be in session
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 19, 3, 0, 1); // Wrong, should be in session
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 19, 17, 00, 59);
		doIsSessionTimeTest(schedule, true, 2022, Calendar.APRIL, 19, 21, 00, 00);
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 19, 21, 6, 0);
		doIsSessionTimeTest(schedule, false, 2022, Calendar.APRIL, 19, 21, 54, 0);

		// Failures repeat through Friday

	}

	private String settingsFile() {
		StringBuilder settings = new StringBuilder();
		settings.append("[default]\n");
		settings.append("StartTime=23:55:00 Europe/Madrid\n");
		settings.append("EndTime=17:05:00 America/New_York\n");
		settings.append("Weekdays=Sun,Mon,Tue,Wed,Thu\n");
		return settings.toString();
	}

	private SessionSettings loadSettings() throws ConfigError {
		SessionSettings settings = new SessionSettings(new ByteArrayInputStream(settingsFile().getBytes()));
		return settings;
	}

	private SessionSchedule newSessionSchedule() throws Exception {
		SessionSettings settings = loadSettings();
		SessionID sessionID = new SessionID("FIX.5.0", "SENDER", "TARGET");
		return new DefaultSessionSchedule(settings, sessionID);
	}

	private void doIsSessionTimeTest(SessionSchedule schedule, boolean expectedInSession, int year,
			int month, int day, int hour, int minute, int second) {
		doIsSessionTimeTest(schedule, expectedInSession, year, month, day, hour, minute, second,
				TimeZone.getTimeZone("UTC"));
	}

	private void doIsSessionTimeTest(SessionSchedule schedule, boolean expectedInSession, int year,
			int month, int day, int hour, int minute, int second, TimeZone timeZone) {
		mockSystemTimeSource
				.setTime(getTimeStamp(year, month, day, hour, minute, second, timeZone));
		assertEquals("in session expectation incorrect", expectedInSession, schedule
				.isSessionTime());
	}

	private Calendar getTimeStamp(int year, int month, int day, int hour, int minute, int second,
			TimeZone timeZone) {
		Calendar c = new GregorianCalendar(year, month, day, hour, minute, second);
		c.setTimeZone(timeZone);
		return c;
	}

}

Expected behavior
The session connects and disconnects at the appropiate times.

system information:

  • OS: Alpine Linux 3 / Fedora 34
  • Java version OpenJDK 11
  • QFJ Version 2.1.1.
@sregueiroqb sregueiroqb added the bug label May 9, 2022
@chrjohn chrjohn mentioned this issue Nov 18, 2022
Closed
@chrjohn chrjohn changed the title Configuring StartTime and EndTime on different timezones results in wrong schedule calculation Configuring StartTime and EndTime on different timezones results in wrong schedule calculation Nov 18, 2022
AndreyNudko pushed a commit to AndreyNudko/quickfixj that referenced this issue Nov 18, 2022
[quickfix-j#486] Fix bug in handling schedules when EndTime < StartTi…
5139a12 
…me and use different timezones

It was possible to end up in a state when interval end is >24 hours before interval start (this is some quirk of the j.u.Calendar which I don't fully understand - but order of calls to setTimeZone() and setTimeInMillis() somehow matters). Adding 1 day in this case is not enough, since resulting interval is still inverted.
This patch addresses the problem by moving end timestamp forward as long as it is behind the start.
AndreyNudko pushed a commit to AndreyNudko/quickfixj that referenced this issue Nov 18, 2022
Fix bug in handling schedules when EndTime < StartTime and are in dif…
958f3da 
…ferent timezones

It was possible to end up in a state when interval end is >24 hours before interval start (this is some quirk of the j.u.Calendar which I don't fully understand - but order of calls to setTimeZone() and setTimeInMillis() somehow matters). Adding 1 day in this case is not enough, since resulting interval is still inverted.
This patch addresses the problem by moving end timestamp forward as long as it is behind the start.

quickfix-j#486
@AndreyNudko AndreyNudko linked a pull request Nov 18, 2022 that will close this issue
Open
@AndreyNudko
Copy link
Contributor

I was working on unrelated changes in the area and I think there is a very small and simple fix
@sregueiroqb would you mind reviewing the test in #574 - I used reproducer from this issue, but had to make some changes to actually make it fail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
No milestone
2 participants
@AndreyNudko @sregueiroqb