Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properly deal with non-existent recurrences #681

Open
minichma opened this issue Dec 27, 2024 · 6 comments
Open

Properly deal with non-existent recurrences #681

minichma opened this issue Dec 27, 2024 · 6 comments

Comments

@minichma
Copy link
Collaborator

minichma commented Dec 27, 2024

RFC 5545 section 3.3.10 seems to specify conflicting rules regarding how to deal with non-existent recurrences.

  1. Recurrence rules may generate recurrence instances with an invalid
    date(e.g., February 30) or nonexistent local time(e.g., 1:30 AM
    on a day where the local time is moved forward by an hour at 1:00
    AM). Such recurrence instances MUST be ignored and MUST NOT be
    counted as part of the recurrence set.

  2. If the computed local start time of a recurrence instance does not
    exist, or occurs more than once, for the specified time zone, the
    time of the recurrence instance is interpreted in the same manner
    as an explicit DATE-TIME value describing that date and time, as
    specified in Section 3.3.5.

My interpretation would be that 1) applies to 'BY' rules, a rule like

DTSTART;TZID=Europe/Berlin:20250330T003000
RRULE:FREQ=HOURLY;BYHOUR=0,2,4;COUNT=2

would generate 20250330T003000,20250330T043000, because 20250330T023000 doesn't exist and MUST be ignored.

On the other hand something like

DTSTART;TZID=Europe/Berlin:20250329T023000
RRULE:FREQ=DAILY;COUNT=2

would generate 20250329T023000,20250330T033000, because 20250330T023000 must be treated according to section 3.3.5, which requires the previous tz offset to be applied.

According to my interpretation, individual nonexistent BY rules would be ignored but whole intervals wouldn't. But that's just a guess at this time.

Consider the following test case. Which values would be expected if DTSTART or DTEND are nonexistent?

    [TestCase("DTSTART;TZID=Europe/Vienna:20250316T023000", "DTEND;TZID=Europe/Vienna:20250323T023000", "20250316T023000/PT168H", "20250323T023000/PT168H", "20250330T033000/PT168H")]

    // TBD: Which are the right durations in cases where DTSTART or DTEND is non-existing?
    [TestCase("DTSTART;TZID=Europe/Vienna:20250316T023000", "DURATION:P1W",                             "20250316T023000/PT168H", "20250323T023000/PT???", "20250330T033000/PT???")]

    public void DurationOfRecurrencesOverDst(string dtStart, string dtEnd, string d1, string d2, string d3)
    {
        var iCal = Calendar.Load($"""
            BEGIN:VCALENDAR
            BEGIN:VEVENT
            {dtStart}
            {dtEnd}
            RRULE:FREQ=WEEKLY;COUNT=3
            END:VEVENT
            END:VCALENDAR
            """);

        var start = iCal.Events.First().Start;

        var periodSerializer = new PeriodSerializer();
        var expectedPeriods =
            new[] { d1, d2, d3 }
            .Where(x => x != null)
            .Select(x => (Period)periodSerializer.Deserialize(new StringReader(x)))
            .ToArray();

        foreach (var p in expectedPeriods)
        {
            p.StartTime = p.StartTime.ToTimeZone(start.TzId);
            p.EndTime = p.StartTime.Add(p.Duration.Value);
        }

        // date only cannot have a time zone
        EventOccurrenceTest(
            cal: iCal,
            fromDate: null,
            toDate: null,
            expectedPeriods: expectedPeriods,
            timeZones: null,
            eventIndex: 0
        );
    }
@axunonb
Copy link
Collaborator

axunonb commented Dec 27, 2024

I share your interpretation, and Python with the datetime and pytz libraries create the same results

@minichma
Copy link
Collaborator Author

I share your interpretation, and Python with the datetime and pytz libraries create the same results

Oh, good to hear. You mean regarding omitting vs ignoring recurrences, right? I removed the test case from the description though, because I think my interpretation regarding how to adjust nonexistent times could be wrong.

@axunonb
Copy link
Collaborator

axunonb commented Dec 27, 2024

regarding omitting vs ignoring recurrences

Yes

Although you remove the test case from the description, it would be good to include it in the repo together with the reasoning for this implementation. Very nice finding BTW

@axunonb
Copy link
Collaborator

axunonb commented Dec 27, 2024

Gave your examples another try with Python using icalendar library to double-check results. Here we are:

import icalendar
from datetime import datetime
from dateutil.rrule import rrulestr
import pytz

# Create a calendar and event
cal = icalendar.Calendar()
event = icalendar.Event()

# Define the event properties
berlin_tz = pytz.timezone('Europe/Berlin')
dtstart = berlin_tz.localize(datetime(2025, 3, 29, 2, 30))
event.add('dtstart', dtstart)
event.add('rrule', {'FREQ': 'DAILY', 'COUNT': 2})

# Add the event to the calendar
cal.add_component(event)

# Extract the RRULE and DTSTART
rrule = event.get('rrule').to_ical().decode()
dtstart = event.get('dtstart').dt

# Calculate occurrences
rule = rrulestr(rrule, dtstart=dtstart)
occurrences = list(rule)

# Print occurrences
for occ in occurrences:
    print(occ)

Output (in both cases):

2025-03-29 02:30:00+01:00
2025-03-30 02:30:00+01:00

[Edit]
I used some other test cases from us which return obviously wrong occurrences. We should neglect their results.
Still it would be good to have another library for double-checking.

@minichma
Copy link
Collaborator Author

Very interesting, thanks for testing! Obviously 2025-03-30 02:30:00+01:00 doesn't exist, so how to resolve this?

  • Ignore (aka discard) the entry as required in 1)? Not sure, but rather not.
  • Move to 2025-03-30 03:30:00+02:00 as required by 2)? Probably.
  • Anything else, e.g. 2025-03-30 03:00:00+02:00? Probably not.

@axunonb
Copy link
Collaborator

axunonb commented Dec 28, 2024

Yes, let's stick to "our way".
I also had a brief look at other Python libraries, and I think ical.net is meanwhile doing a pretty good job on recurrence and covering RFC 5545 in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants