Keep UNTIL Date in UTC for comparison

[scottschmitz]

The problem occurs when the UNTIL date gets converted from UTC to the timezone specified when creating a DateIterator. When getting the next date from the DateIterator, the new date is converted to UTC and compared with the UNTIL (which is no longer in UTC). This results in the last event being outside of the window.

See example code snippet below:

RecurrenceRule recurrenceRule = vEvent.getRecurrenceRule();
if (recurrenceRule != null) {
    DateStart dateStart= vEvent.getDateStart();

    TimeZone timezone;
    if (tzInfo.isFloating(dateStart)){
        timezone = TimeZone.getDefault();
    } else {
        TimezoneAssignment dtstartTimezone = tzInfo.getTimezone(dateStart);
        timezone = (dtstartTimezone == null) ? TimeZone.getTimeZone("UTC") : dtstartTimezone.getTimeZone();
    }
    DateIterator iterator = vEvent.getDateIterator(timezone);

    while (iterator.hasNext()) {
        Date startDate = iterator.next();
        System.out.println(startDate.toString());
    }
}

BEGIN:VEVENT
DTSTART;TZID=America/New_York:20161219T090000
DTEND;TZID=America/New_York:20161219T100000
RRULE:FREQ=DAILY;UNTIL=20161223T140000Z
DTSTAMP:20161216T153224Z
UID:pdhtelaeqstgnbr9f0hrdioij0@google.com
CREATED:20161216T133749Z
DESCRIPTION:
LAST-MODIFIED:20161216T133749Z
LOCATION:
SEQUENCE:0
STATUS:CONFIRMED
SUMMARY:Daily Until 12/23
TRANSP:OPAQUE
END:VEVENT

[codecov-io]

Current coverage is 78% (diff: 100%)

Merging #63 into master will increase coverage by <1%

@@             master        #63   diff @@
==========================================
  Files           278        278          
  Lines         11476      11477     +1   
  Methods           0          0          
  Messages          0          0          
  Branches       2188       2188          
==========================================
+ Hits           9057       9059     +2   
  Misses         1926       1926          
+ Partials        493        492     -1   

Powered by Codecov. Last update 6fcdfad...a0ccda2

[mangstadt]

Thanks! Sorry for taking so long to look at it! 😄

[scottschmitz]

:+1:

[mangstadt]

Hi Scott,

Someone reported that a number of unit tests fail if the JVM's default timezone is "Europe/Paris". Do you know why?

public class CompoundIteratorImplTest {
    private static final TimeZone PST = TimeZone.getTimeZone("America/Los_Angeles");
    private static final TimeZone UTC = TimeUtils.utcTimezone();

    static {
        TimeZone tz = TimeZone.getTimeZone("Europe/Paris");
        TimeZone.setDefault(tz);
    }

    //...
}

[scottschmitz]

Sorry it took so long for a response, but as you might expect it appears as though there are some more timezone related issues. I understand the issue, but I haven't been able to come up with a valid solution. The problem is that Paris is East of UTC and therefore midnight on May 3, 2006 in Paris is late at night on May 2, 2006 in UTC. The comparison is currently doing date only compares that fails when the times get truncated because 3 > 2.

In TestUtils.java line 290, the date is kept in the default time zone.

return date(text, TimeZone.getDefault());

Later in RecurrenceIteratorFactory.java line 414 the UNTIL date is converted to UTC and truncated to just a date.

untilUtc = TimeUtils.toDateValue(untilUtc);

Not entirely sure how to clean this up as I think it would require some fairly significant changes to prevent loss of precision in comparisons.

Hope this helps!

[mangstadt]

Thanks for the information. In the meantime, I'm going to have to rollback this pull request, since the unit tests are failing under certain timezones. If you're bored one weekend and need something to do, you know where to go! ;)

[mangstadt]

This issue has finally been fixed in f1632351d3f023e8337e7bad34292f27f0176618. @scottschmitz your change was part of the solution (thank you!), but I also had to change the way it handled UNTIL values that lack time components (which is what many of the unit tests use).

The unit tests were failing under certain timezones because of the way the UNTIL date object was being constructed in the unit tests. It was constructed using the SimpleDateFormat#parse method. By default, this method assumes the given date string is under the default timezone, which caused a different UNTIL timestamp to be created depending on your timezone. The recurrence iterator code wasn't taking this into account, so I modified it to behave differently if the given UNTIL value lacks a time component (the user must announce this by passing "false" into the second parameter of the ICalDate constructor).

The other problem was that it was chopping off the time components of the UNTIL value if the start date lacked time components, which, under certain timezones, essentially caused the UNTIL value decrement by one day, which is why the last date in a recurrence was not being returned for so many people. See pseudocode below.

until = SimpleDateFormat.parse("2006-05-03") //May 3 2006 00:00 Europe/Paris
until = convertToUtc(until) //May 2 2006 22:00 UTC
until = removeTimeComponents(until) //May 2 2006

I think that basically covers it.