quartznet/quartznet — pull request #72 by amazing-andrew (2012-11-03 05:32:18+00:00)
Title: Issues with CronExpression, Calendars, CalendarIntervalTriggerImpl (TimeZones)

While working on a new project I wanted for quartz (just for fun) that translated plain text into quartz schedules.

plug: https://github.com/amazing-andrew/AndrewSmith.Quartz.TextToSchedule

I noticed that the Calendars were matching dates that I didn't want because of the difference in TimeZones. For example, creating a daily calendar to only allow 9am to 5pm in Eastern Standard Time, you can create it but it doesn't seem to behavior very well with offsets because the start dates and other things were assuming a utc offset while other things turned and looked only at the *local* time zone, which could be different from the TimeZoneInfo set by the TimeZone property. The comparisons being done were not paying much attention to the offsets, and this caused problems with the calendar's not seeming to behave properly.

1) Cron Trigger was returning extra hours on a DST transition dates

Cron functions were returning extra hours on daylight saving time transitions. It seemed to match an hour before when the time changes. I also saw that someone else experiencing the same issue I noticed on the quartznet google group
https://groups.google.com/forum/?fromgroups=#!topic/quartznet/6okQzAA7c8o

this was actually very simple to fix in the CronExpression class. Only need very minimal code to apply the TimeZone before returning the results. Making this change however, caused one test to fail. I looked at this test and it was clear that it was not expecting the correct time offsets. I updated this so that it would expect the correct results.

all tests are passing.

2)

For the Calendars, I knew there were TimeZone properties and looked through the code to see that they were not being used. These commits resolve that issue for the all built-in Calendars. I adjusted these calendars by apply the time zone in locations right before certain checks are going to be made. This seem to resolve the expected behavior of the timezones differences.

I added at least 1 simple test for each calendar to test these time zone transition. The functionality broke a few of the calendar tests because it was working with *local* time and the results of the tests would be variable depending upon what timezone your system is set to. I changed most of this by setting the TimeZone property for the Calendars to UTC in the test class setup method and the tests were passing again.

all calendar tests seem to be passing.

3)

CalendarIntervalTriggerImpl did not seem to follow several things. I first noticed that the PreserveHourOfDayAcrossDaylightSavings was not working properly and started issue #71 here. I suggested some code, but it took a bit more than this to get things to work properly after testing several timezone transitions.

I noticed a problem I had when the DST would shift back an hour, I had to add special code in the DAY handling code to adjust for this. After this the PreserveHourOfDayAcrossDaylightSavings was working correctly. During this I cam across that the SkipDayIfHourDoesNotExist, doesn't seem to be used.

The CalendarIntervalTriggerImpl also did not follow the SkipDayIfHourDoesNotExist. I toiled over this for a while after researching the java implementation. I believe I have this working, I found out we can see if an hour does not exists by calling the TimeZoneInfo.IsInvalidTime method. with this info it was pretty easy to see if we needed to skip the time or not. If we don't want to skip the time, I returned the next closest valid time found.

I added a number of tests to test the SkipDayIfHourDoesNotExist and PreserveHourOfDayAcrossDaylightSavings functionality.

The final result of this pull resolves Issue #71.
all tests seem to be passing.

_notes about the commits_

I tried really hard to keep the commits isolated, I was doing good near the start, and then noticed mistakes I had made later and make a few more changes to files in the later commits, and I'm not that fancy at git to get these things _just right_. Anyways, even though it's not perfect, I think it is still very manageable. I was struggling to figure out what the proper approach for the CalendarIntervalTriggerImpl should be, and I had to change that one over a few commits until I got it right.

_other things I've thought about_

For the triggers, I only changed the CalendarIntervalTriggerImpl, because I knew it clearly handled time zone issues and it's the one with the functionality that I want to use. I did not go looking too much into the other triggers. I see some TimeZone code in the NthIncludedDayTrigger, I have *not* done anything with this. But there just may be some additional TimeZoneUtil.Convert to do in there to get it to work with different TimeZones. But it shouldn't be much, unless the NthIncludedDayTrigger has to keep track or do anything special for DST transitions.

Thanks,
~Andrew