When UNTIL < DSTART then event never happens

[PowerKiKi]

Google Calendar generate this kind of data when the end-user create
repeating events and then edit them in a specific, unknown way.

The new interpretation to consider that the event never happens if
its UNTIL is before its DSTART matches with the interpration of
Google Calendar (when subscribed via URL) and Apple Calendar.

Unfortunately not everyone agrees on that interpretation and
Thunderbird 45, MS Calendar 17, MS Outlook 2016 and Google Calendar
via file upload (not URL subscription) thinks that the event happens
only once.

In all RFCs 5545, 5546, 6868, 7529, 7953 and 7986, the single most
relevant part is on page 41 of RFC 5545:

The UNTIL rule part defines a DATE or DATE-TIME value that bounds
the recurrence rule in an inclusive manner. If the value
specified by UNTIL is synchronized with the specified recurrence,
this DATE or DATE-TIME becomes the last instance of the
recurrence. The value of the UNTIL rule part MUST have the same
value type as the "DTSTART" property. Furthermore, if the
"DTSTART" property is specified as a date with local time, then
the UNTIL rule part MUST also be specified as a date with local
time. If the "DTSTART" property is specified as a date with UTC
time or a date with local time and time zone reference, then the
UNTIL rule part MUST be specified as a date with UTC time. In the
case of the "STANDARD" and "DAYLIGHT" sub-components the UNTIL
rule part MUST always be specified as a date with UTC time. If
specified as a DATE-TIME value, then it MUST be specified in a UTC
time format. If not present, and the COUNT rule part is also not
present, the "RRULE" is considered to repeat forever.

While the RFC clearly states what should happen when UNTIL == DSTART,
it says nothing about UNTIL < DSTART.

So it would seems reasonnable to resort to common sense and assume
that something cannot start after it ends, and thus this patch is
needed to have the most logical behavior.

Finally, historically speaking, this project introduced the previous
beahvior via https://github.com/fruux/sabre-vobject/pull/17 (more
specifically ca06d40), before the refactoring of RecurrenceIterator
into RRuleIterator. Unfortunately that PR does not give much
information about why the change was made. And the issue it refers
to is no longer accessible. So there is no argument to keep the
previous behavior.

[evert]

I'm kind of tempted to just say, why not just throw an exception in this case? I think this is definitely a case that we should consider as 'undefined' because it's nonsense data. An exception seems the most appropriate.

I kinda feel that instead of supporting this, it would be better to add some rules to the iCalendar validation/repair system to automatically correct these types of objects.

[PowerKiKi]

I forgot to mention my live example of this case. On this public Google Calendar, you'll find the event ss08pi2a01557ohq5ktg6u529k@google.com (at the end of the file), which is supposed to start on the 2017-03-16, but the UNTIL is 1 second before DTSTART. To me this was clearly done on purpose (most likely internally by Google rather than end-user).

So clearly there is no much point into creating something just to say it does not exists, but on the other hand I wouldn't say it is technically incorrect. Merging this PR would mean a better interoperability with Google Calendar and Apple Calendar. So of course I am in favor of merging it, but maybe we can find another opinion on the matter ? Would you know of any places where we could get interesting insights ?

[DominikTo]

Sounds a bit similar to another Google Calendar issue (recurrence rules where all instances are an exception == no valid event at all). In that case also an exception is thrown:

https://github.com/sabre-io/vobject/blob/master/lib/Recur/EventIterator.php#L203

[PowerKiKi]

I rebased, just for the sake of it.

Does the RFC specifies that a recurrence rule without any instance is invalid ?

If it does not specify that, then I'd still argue that an exception should not be thrown. Having empty collection of things is a rather common concept and it usually does not need to be treated as an exception.

[evert]

The RFC doesn't strictly imply anything in this case. It makes the most sense to me to throw the exception, because it's not really a collection of 0 items. What we're kind of talking about in practice, is a collection of type calendar event, and an occasional NULL that's invisible to the user. To stay consistent with the Google Calendar issue, I think an exception here too is the most appropriate.

[PowerKiKi]

invisible to the user

That's a very good point. IIRC my original case was a calendar with two instances, one before DSTART, and one after. So in that very specific case, one event was visible to the end-user. But clearly this might not always be the case. And I agree that it would be incorrect to have data that the end-user cannot see and thus cannot fix.

Should we modify this PR, or create a separate PR, to throw an exception ?