rianjs / ical.net

ical.NET - an open source iCal library for .NET
MIT License
776 stars 230 forks source link

Event with recurrence rule generates occurence with DTSTART - DTEND #431

Open liborpansky opened 5 years ago

liborpansky commented 5 years ago

Description

We use Kendo Scheduler and the component generates events in the following way:

  1. Set Range <2018/11/30 - 2018/12/07>
  2. Set Recurrence rule: MONDAY, TUESDAY, WEDNESDAY
  3. Event Time: 10:00 UTC - 11:30 UTC

Event is set with start date retrieved as a first date from range: 2018/11/30 at 10:00 DTSTART:20181130T100000Z and end date retrieved from range: 2018/11/30 at 11:30 DTEND:20181130T113000Z

Kendo Scheduler generates 3 events repeated on monday, tuesday and wednesday:

iCal.Net returns following occurences:

Is it correct behaviour? I read RFC 5545 and didn't find any mention about how should iCalendar behave. Is it correct if I specifically set to repeat the event ONLY on monday, tuesday and wednesday it finds an occurence also on DTSTART and DTEND?

NuGet version

latest 4.1.9

Observed behavior

Event with recurrence rule generates occurence with DTSTART - DTEND, even if it should be ignored.

Expected behavior

Event with recurrence rule should apply only recurrence rules and ignore DTSTART and DTEND date.

Example

  [Test, Category("Recurrence")]
  public static void VEventShouldRetrieveOccurencesBasedOnRRuleOnly()
  {
      const string iCal =
        @"BEGIN:VCALENDAR
        PRODID:-//github.com/rianjs/ical.net//NONSGML ical.net 4.0//EN
        VERSION:2.0
        BEGIN:VEVENT
        DTEND:20181130T113000Z
        DTSTAMP:20181130T095036Z
        DTSTART:20181130T100000Z
        RRULE:FREQ=WEEKLY;UNTIL=20181207T120000Z;BYDAY=MO,TU,WE
        SEQUENCE:0
        UID:7357c0d9-68e2-4682-8f76-611d5ab770f3
        END:VEVENT
        END:VCALENDAR";

      var calendar = Calendar.Load(iCal);
      DateTime startDate = new DateTime(2018, 11, 30, 0, 0, 0, DateTimeKind.Utc);

      var startSearchTwoDays = new CalDateTime(startDate);
      var endSearchTwoDays = new CalDateTime(startDate.AddDays(1));

      var occurrencesDay = calendar
          .Events
          .First()
          .GetOccurrences(startSearchTwoDays, endSearchTwoDays)
          .Select(o => o.Period as Period)
          .OrderBy(p => p.StartTime)
          .ToList();

      Assert.IsTrue(occurrencesDay.Count == 0);

      var startSearchWeek = new CalDateTime(startDate);
      var endSearchWeek = new CalDateTime(startDate.AddDays(6));

      var occurrencesWeek = calendar
          .Events
          .First()
          .GetOccurrences(startSearchWeek, endSearchWeek)
          .Select(o => o.Period as Period)
          .OrderBy(p => p.StartTime)
          .ToList();

      Assert.IsTrue(occurrencesWeek.Count == 3);
  }
minichma commented 5 years ago

The case you specified is undefined. RFC 5545 states, that

The recurrence set generated with a "DTSTART" property value that doesn't match the pattern of the rule is undefined.

Issue #407 requests a feature, which helps at least detecting such cases.

gvas commented 2 years ago

Whether or not an event is generated at the reference date is controlled by the CalendarEvent class's EvaluationIncludesReferenceDate property. Unfortunately it is protected, but as a workaround you can create a derived class which overrides only that property. As far as I can tell this has no other side effect.

public class MyCalendarEvent: CalendarEvent
{
    protected override bool EvaluationIncludesReferenceDate => false;
}

After that you can use it like this:

...

var calendarEvent = new MyCalendarEvent
{
    Description = ev.Description,
    DtEnd = new CalDateTime(TimeZoneInfo.ConvertTimeToUtc(DateTime.SpecifyKind(ev.EndDateLocal, DateTimeKind.Unspecified), tzi)),
    DtStart = new CalDateTime(TimeZoneInfo.ConvertTimeToUtc(DateTime.SpecifyKind(ev.StartDateLocal, DateTimeKind.Unspecified), tzi)),
    IsAllDay = ev.AllDay,
    Summary = ev.Summary,
};

if (!string.IsNullOrWhiteSpace(ev.RecurrenceException))
{
    calendarEvent.ExceptionDates = new List<PeriodList> { new PeriodList(ev.RecurrenceException) };
}

if (!string.IsNullOrWhiteSpace(ev.RecurrenceRule))
{
    calendarEvent.RecurrenceRules = new List<RecurrencePattern> { new RecurrencePattern(ev.RecurrenceRule) };
}

var calendar = new Calendar();
calendar.Events.Add(calendarEvent);

HashSet<Occurrence> occurrences = calendar.GetOccurrences(start, end);

...

My use case didn't require the deserialization of an iCal string. I believe the simplest thing would be to deserialize it as usual then replace the regular CalendarEvent instances with the new MyCalendarEvent instances (after copying its properties).

EugeneBeanblossom commented 1 year ago

Whether or not an event is generated at the reference date is controlled by the CalendarEvent class's EvaluationIncludesReferenceDate property. Unfortunately it is protected, but as a workaround you can create a derived class which overrides only that property. As far as I can tell this has no other side effect.

public class MyCalendarEvent: CalendarEvent
{
    protected override bool EvaluationIncludesReferenceDate => false;
}

My use case didn't require the deserialization of an iCal string. I believe the simplest thing would be to deserialize it as usual then replace the regular CalendarEvent instances with the new MyCalendarEvent instances (after copying its properties).

I went ahead and tried implementing this, but I found two problems with it. One: any single-occurrence event will always return nothing, because it uses this property for that one occurrence. Two: it fails deserialization because it deserializes to a specific type based on the object name so it can't deserialize to your derived type meaning you'll have to copy over all properties of all events to a new instance of the derived type. Ew.

public class CalendarComponentFactory
{
    public virtual ICalendarComponent Build(string objectName)
    {
         ICalendarComponent c;
         var name = objectName.ToUpper();

        switch (name)
        {        
           \\ ...
            case EventStatus.Name:
                c = new CalendarEvent();
                break;

           \\ ...
        }
        c.Name = name;
        return c;
    }
}

The solution I found that addresses both is the following code change in CalendarEvent.cs.

protected override bool EvaluationIncludesReferenceDate 
{
    get
    {
        // If this is a one-time event then you need the reference date in the results otherwise only include those that follow the pattern
        return (RecurrenceRules?.Count ?? 0) == 0;
    }
}

It appears to work so far. This assumes the recurrence rules are only populated when there is an actual recurrence pattern. I found that having a FrequencyType of None in the rule will throw an error so it seems to support my thinking. If this is a viable solution, and if this is still being maintained, can it be implemented? Any adverse side effects you can think of? I don't mind submitting the change if-necessary.