ical-org / ical.net

ical.net - an open source iCal library for .Net
Other
790 stars 231 forks source link

Get Occurrences fails on a RRULE FREQ=SECONDLY #622

Open axunonb opened 1 week ago

axunonb commented 1 week ago

Discussed in https://github.com/ical-org/ical.net/discussions/615

Originally posted by **DaleyKD** October 18, 2024 Does this library support getting occurrences if we use a secondly recurrence pattern? Our project doesn't use this for calendar events, but more to help us determine when to run the next job (as part of our job scheduler). And SOME jobs need to run every 45 seconds or so. I've been trying to get it to work, but have had no luck. It seems to work for FREQ=MINUTELY.
axunonb commented 1 week ago

@DaleyKD, @minichma

Here is temporary workaround for RRULE FREQ=SECONDLY

[Test]
public void SecondlyRecurrence()
{
    var dateTime = DateTime.UtcNow;

    // Create a new calendar
    var calendar = new Calendar
    {
        RecurrenceRestriction = RecurrenceRestrictionType.NoRestriction
    };

    var calendarEvent = new CalendarEvent
    {
        Summary = "Recurring Event Every 45 Seconds",
        DtStart = new CalDateTime(dateTime),
        DtEnd = new CalDateTime(dateTime.AddHours(1)),
        RecurrenceRules = new List<RecurrencePattern>
        {
            new RecurrencePattern(FrequencyType.Secondly, 45)
        }
    };

    // This does not seem to add any reference to the parent `Calendar`,
    // Services inside `CalendarEvent` and its children should be updated
    calendar.Events.Add(calendarEvent);

    // Serialize the calendar to an iCalendar string
    var serializer = new CalendarSerializer();
    var serializedCalendar = serializer.SerializeToString(calendar);

    /*************************************************************
     *  This is a temporary workaround to make GetOccurrences work
     *  with the RecurrenceRestriction set to NoRestriction.
     *************************************************************/
    calendar = Calendar.Load(serializedCalendar);
    // This setting does not deserialize, so we have to set again
    calendar.RecurrenceRestriction = RecurrenceRestrictionType.NoRestriction;

    // Evaluate the recurrences
    var occurrences = calendar.GetOccurrences(dateTime, dateTime.AddHours(1));

    Assert.That(occurrences, Has.Count.EqualTo(80));
}
DaleyKD commented 1 week ago

Wow! I didn't expect a response so quickly.

I'm currently only using the RecurrencePatternEvaluator to get the occurrences. I'll try to use the pattern's restriction policy (I didn't even realize there was one!) when I get back to writing unit tests.

You guys are absolutely knocking it out of the park. Thank you so very much.

minichma commented 1 week ago

Btw, there's one more limitation that could play a role with SECONDLY freq:

https://github.com/ical-org/ical.net/blob/a0179f811b425fe5bde004c102da209907681a8f/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs#L52

The number of increments between occurrences must be < (or <=) 1000, so something like this might not work as expected:

FREQ=SECONDLY;BYHOUR=1,2,3

because after 3:59:59 the number of second increments until 1:00:00 is > 1000. Shouldn't play a role in case of 45s intervals though.

axunonb commented 1 week ago

@minichma The recurrence related properties on Calender tend to complicate things and are not fully implemented. When using them, CalendarEvent can't be used standalone. Also, could we get rid of RecurrenceRestrictionType when _maxIncrementCount was configurable? We should discuss/evaluate how to handle that.

DaleyKD commented 1 week ago

FWIW, for me, this can be a lower priority than whatever else you have in the pipeline. The specific product I'm on right now is only days+. However, when we migrate other products to this, we'll need seconds.

minichma commented 1 week ago

The recurrence related properties on Calender tend to complicate things and are not fully implemented. When using them, CalendarEvent can't be used standalone. Also, could we get rid of RecurrenceRestrictionType when _maxIncrementCount was configurable? We should discuss/evaluate how to handle that.

Fully agree. I tend to think that its not the lib's task to define which kind of restrictions on performance are acceptable for the user. So I would probably remove the RecurrenceRestrictionType altogether. Its also not quite easy to understand what it is doing. The maxIncrement probably makes sense but in my opinion should default to indefinite.