Skip to content

Correcting issue regarding Calendar.getFirstDayOfWeek() which is dependent on locale #1147

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

Open
alexwibowo wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Correcting issue regarding Calendar.getFirstDayOfWeek() which is dependent on locale #1147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions quickfixj-core/src/main/java/quickfix/DefaultSessionSchedule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,8 @@ private TimeInterval theMostRecentIntervalBefore(Calendar t) {
}

} else {
// Note that depending on locale, calling 'set(Calendar.DAY_OF_WEEK, ...)' could result in intervalStart being
// a date in the future.
if (isSet(startTime.getDay())) {
intervalStart.set(Calendar.DAY_OF_WEEK, startTime.getDay());
if (intervalStart.getTimeInMillis() > t.getTimeInMillis()) {
Expand All @@ -236,6 +238,11 @@ private TimeInterval theMostRecentIntervalBefore(Calendar t) {
if (intervalEnd.getTimeInMillis() <= intervalStart.getTimeInMillis()) {
intervalEnd.add(Calendar.WEEK_OF_MONTH, 1);
Copy link
Copy Markdown
Member

@chrjohn chrjohn Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
intervalEnd.add(Calendar.WEEK_OF_MONTH, 1);
intervalEnd.add(Calendar.DAY_OF_YEAR, 7);

Copy link
Copy Markdown
Contributor Author

@alexwibowo alexwibowo Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Chris. Thanks for the suggestion, I just ran the unit test SessionScheduleTest#testSessionStartInPreviousDayOfWeek in OpenJDK 25.

The test sets up the current time to be Monday, 23 February 2026, in AEDT. With session settings:

  • Start time: Sunday, 13:00:00 Australia/Victoria
  • End time: Saturday, 17:00:00 America/New York

With your change, the test still fails, because it ends up creating interval:

  • start: Sun Feb 22 13:00:00 AEDT
  • end: Sun Feb 22 09:00:00 AEDT
    (this interval by itself is already invalid)

The correct interval should have been:

  • start: Sun Feb 22 13:00:00 AEDT
  • end: Sun Mar 1 09:00:00 AEDT

Copy link
Copy Markdown
Member

@chrjohn chrjohn Feb 28, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Alex, ok sorry. I will give it another thought then.
Regarding your fix: I could integrate it into the release after 3.0.0. Up to then you could implement your fix in a custom session schedule and pass it to your application. I think that should work. Let me know what you think.

Copy link
Copy Markdown
Contributor Author

@alexwibowo alexwibowo Feb 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Chris. Thank you again. If I can use my own implemention of custom session schedule, that would be great . I don't think I can though ? Without doing some hack - overriding that class, put in the classpath ahead of quick fix jar (so that my implementation will get loaded first). AFAICT, the session schedule factory, etc are constructed within constructor, and there is no way to pass my own implementation. I could be wrong though .

Copy link
Copy Markdown
Member

@chrjohn chrjohn Mar 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, not anywhere near a dev box right now, but I thought you can pass your own session factory to a constructor of your connector. If not, then we should definitely add it because that was the point in having a schedule interface.
Another thought: does it work when we eliminate everything locale specific by setting this everywhere:

setFirstDayOfWeek(Calendar.SUNDAY)
setMinimalDaysInFirstWeek(1)

???

}

// in some cases (certain locale), we could run into scenario where the interval end is before the interval start.
if (intervalEnd.getTimeInMillis() < intervalStart.getTimeInMillis()) {
intervalEnd.add(Calendar.WEEK_OF_YEAR, 1);
}
} else if (intervalEnd.getTimeInMillis() <= intervalStart.getTimeInMillis()) {
intervalEnd.add(Calendar.DAY_OF_WEEK, 1);
}
Expand Down
25 changes: 25 additions & 0 deletions quickfixj-core/src/test/java/quickfix/SessionScheduleTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,31 @@ public void testIsSameSessionWithDay() throws Exception {
doIsSameSessionTest(schedule, t1, t2, true);
}

@Test
public void testSessionStartInPreviousDayOfWeek() throws Exception {
Locale.setDefault(new Locale("en", "AU"));

final TimeZone tz = TimeZone.getTimeZone("Australia/Victoria");
mockSystemTimeSource.setTime(getTimeStamp(2026, Calendar.FEBRUARY, 23, 10, 0, 0, tz));

final SessionSettings settings = new SessionSettings();
settings.setString(Session.SETTING_START_TIME, "13:00:00 Australia/Victoria");

// AU week starts on Sunday on Java 17 and below, but on Monday on Java 21 and above (not sure about in between).
settings.setString(Session.SETTING_START_DAY, "Sunday");

settings.setString(Session.SETTING_END_TIME, "17:00:00 America/New_York");
settings.setString(Session.SETTING_END_DAY, "Saturday");

SessionID sessionID = new SessionID("FIX.4.2", "SENDER", "TARGET");
final SessionSchedule schedule = new DefaultSessionSchedule(settings, sessionID);

mockSystemTimeSource
.setTime(getTimeStamp(2026, Calendar.FEBRUARY, 23, 10, 0, 0, tz));
assertEquals("in session expectation incorrect", true, schedule
.isSessionTime());
}

@Test
public void testSettingsWithoutStartEndDay() throws Exception {
SessionSettings settings = new SessionSettings();
Expand Down
Loading