Improve handling of Weekday schedules when EndTime < StartTime

[AndreyNudko]

In FX session typically starts on Sun evening and is reset around 5PM NewYork time. So daily schedule may look like this
Sun 17:05 - Mon 16:55
Mon 17:05 - Tue 16:55
Tue 17:05 - Wed 16:55
Wed 17:05 - Thu 16:55
Thu 17:05 - Fri 16:55

I was expecting that below settings would achieve this:

  Weekdays = SUNDAY,MONDAY,TUESDAY,WEDNESDAY,THURSDAY,FRIDAY
  StartTime = 17:05:00
  EndTime = 16:55:00

But they don't, because DefaultSessionSchedule considers the interval [Fri 17:05 - Sat 16:55] to be in session - even if Sat is not a weekday.

This change improves weekday behaviour by going backwards until both start and end days are weekdays. It does not have any impact in cases when EndTime > StartTime.

As a precaution, if suitable interval could not be found we revert to original behaviour. This should be configuration error, but even then returning something which is not quite right is better than getting stuck in infinite loop.

[AndreyNudko]

As a bit of background
It's not really a problem that session is still trying to login after the end of last weekday - the other end (acceptor) either doesn't care or doesn't allow it.
But there are monitoring tools using DefaultSessionSchedule to decide whether or not session should be active, and current behaviour confuses them
I could implement a custom schedule, but this seems like a generally useful change
Let me know your thoughts - I don't fully understand possible impact, but implementation is quite defensive

[chrjohn]

Hi @AndreyNudko , thanks for the PR. 👍

Sounds like this bug, do you agree? #486
It talks about different timezones but maybe the underlying bug really is the time in conjunction with the weekdays.

[AndreyNudko]

It looks similar, but two TimeZone-s makes it even more complicated
I don't think my change addresses #486 as it doesn't affect attached reproducer test
BTW, that test misleadingly passes as it has assertions on the current behaviour rather than expected one - at least they are marked as // Wrong, should be in session

[AndreyNudko]

I think the other issue is that in certain configurations adjusting time by 1 day is not enough; here's how interval calculations go on Sun in the "23:55:00 Europe/Madrid - 17:05:00 America/New_York" scenario:

Mon 21:55:00 UTC --> Sun 21:05:00 UTC
Sun 21:55:00 UTC --> Sat 21:05:00 UTC
Sun 21:55:00 UTC --> Sun 21:05:00 UTC // stop here ; same day, but times are still the other way around

I can make the other test pass by swapping condition for a loop at DefaultSessionSchedule:212

            while (intervalEnd.getTimeInMillis() <= intervalStart.getTimeInMillis()) {
                intervalEnd.add(Calendar.DAY_OF_WEEK, 1);
            }

I would be inclined to submit that a separate patch, because it seems less intrusive than my change.

One thing which I realized is that my understanding of Weekdays is slightly different from 486.
I expect in FX-like schedule both Sun and Fri to be configured as weekdays, i.e. the setting applies to both start and end of the session.
But they set weekdays as Sun-Thu - and maybe it actually makes more sense to only apply Weekdays to the start. If I misunderstood semantics of this option, my change could have side effects for a number of people - and I probably can just tweak my schedule.

[AndreyNudko]

Actually, looking at #133 which introduced Weekdays: "comma-delimited list of the days to start the session on"
Which I think is also enforced by existing tests

I'm going to close this PR and maybe submit another one to just improve documentation, as it's currently a bit vague

[chrjohn]

@AndreyNudko but #574 is still valid?

[AndreyNudko]

@chrjohn Yes, #574 is actual bugfix