search

ical4j / ical4j

A Java library for parsing and building iCalendar data models
https://www.ical4j.org
BSD 3-Clause "New" or "Revised" License
761 stars 201 forks source link

Recurrence Rule Generating Invalid Dates #232

Closed gregorydlogan closed 5 years ago

gregorydlogan commented 6 years ago

Hi,

We're using iCal4j over at Opencast to calculate recurring capture events, and then generating the iCal data for the end capture agents. We're experiencing an issue where users in some (most?) timezones are unable to get past some unit tests. After much investigation I've found what I believe to be an issue in the underlying iCal4j library. It stems from here.

Our RRULE looks like

FREQ=WEEKLY;BYDAY=MO,TH,FR,SA,SU;BYHOUR=11;BYMINUTE=5

Generating the dates with

rRule.getRecur().getDates(periodStartTz, periodEndTz, net.fortuna.ical4j.model.parameter.Value.DATE_TIME)

Where periodStartTz is 20160325T110500 (a Thursday) and periodEndTz is 20160329T121000 (a Monday). The dates generated are:

where the 21st is a Monday. Generating the 24th is likely a bug on our end - we're dealing with some timezone issues. but generating the 21st is just completely incorrect. Hopefully can provide a patch relatively soon, unless you can see a quick fix. Or are we doing something wrong? Our ticket lives here

gregorydlogan commented 6 years ago

Further to this, and I'm betting this is part of a larger underlying issue: Several tests fail in non DST observant timezones. This is on Debian 9, openjdk version "1.8.0_162":

America/Phoenix:

net.fortuna.ical4j.model.VEventRecurrenceParametrizedTest > test[3] FAILED
    org.codehaus.groovy.runtime.powerassert.PowerAssertionError at VEventRecurrenceParametrizedTest.groovy:103

net.fortuna.ical4j.model.VEventRecurrenceParametrizedTest > test[4] FAILED
    org.codehaus.groovy.runtime.powerassert.PowerAssertionError at VEventRecurrenceParametrizedTest.groovy:103

net.fortuna.ical4j.model.VEventRecurrenceParametrizedTest > test[5] FAILED
    org.codehaus.groovy.runtime.powerassert.PowerAssertionError at VEventRecurrenceParametrizedTest.groovy:103

1592 tests completed, 3 failed, 100 skipped

America/Regina:

net.fortuna.ical4j.util.CalendarsTest > testShouldSerializeDeserializeCorrectly FAILED
    junit.framework.AssertionFailedError at CalendarsTest.java:211
karendolan commented 5 years ago

@gregorydlogan and @benfortuna, does this bug only manifest in non-DST TimeZones like America/Phoenix or UTC? So that if the system is located in a DST observant timezone, it's not affected?

benfortuna commented 5 years ago

I've just undertaken a major refactor of the recurrence rules implementation in ical4j and I think the results are now a lot more promising:

20160325T110500
20160326T110500
20160327T110500
20160328T110500

I will be releasing the full change shortly in ical4j 3.0.4.

gregorydlogan commented 5 years ago

@karendolan it's been a long time at this point, but IIRC yes the issue only manifested in non DST-observant zones. @benfortuna I'm returning to work in January, happy to look at this again at that point, although I'm betting that Karen's going to be beat me to the punch :)