UNTIL property in RRULE is supposed to be inclusive

[rvaidya]

The pattern I'm using: FREQ=WEEKLY;INTERVAL=1;BYDAY=WE;DTSTART=20160801T070000Z;UNTIL=20160831T070000Z

UNTIL is inclusive, unlike DTEND which is non-inclusive

iCal.NET drops 8/31 out of the date list, while DDay.iCal does not

[rvaidya]

For context, I am calling RecurrencePatternEvaluator directly to evaluate recurrence pattern strings. I'm passing in: "2016-08-01" as period start, "2016-08-31" + 1ms as period end (1ms hack since i want to include 8/31)

[rianjs]

OK there's a couple of things here:

• FREQ=WEEKLY;INTERVAL=1;BYDAY=WE;DTSTART=20160801T070000Z;UNTIL=20160831T070000Z is not a valid RRULE pattern. DTSTART is a property that goes with something like a VEVENT. It doesn't go with an RRULE.
• RRULEs can't be evaluated in a vacuum. RRULEs are properties that belong to components like VEVENTs, they can't be evaluated by themselves.
• There is no way to ask ical.net to do something like "Here is a rule set. Given search start and end times of X, what would be the recurrence set you find"

If I am misunderstanding your issue, please post a complete VEVENT text block.

[rianjs]

OK, I've replicated your problem. But it's not a bug, it's a problem with how you have formulated your search range. Have a look.

Here's the starting VEVENT:

BEGIN:VCALENDAR
PRODID:-//github.com/rianjs/ical.net//NONSGML ical.net 2.2//EN
VERSION:2.0
BEGIN:VEVENT
SUMMARY:This is an event
DTEND;TZID=UTC:20160801T080000
DTSTAMP:20160905T142724Z
DTSTART;TZID=UTC:20160801T070000
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=WE;UNTIL=20160901T070000
UID:abab717c-1786-4efc-87dd-6859c2b48eb6
END:VEVENT
END:VCALENDAR

Or, the programmatic equivalent, if you prefer:

var rrule = new RecurrencePattern(FrequencyType.Weekly, interval: 1)
{
    Until = DateTime.Parse("2016-08-31T07:00:00"),
    ByDay = new List<IWeekDay> { new WeekDay(DayOfWeek.Wednesday)},
};

var start = DateTime.Parse("2016-08-01T07:00:00");
var end = start.AddHours(1);
var e = new Event()
{
    DtStart = new CalDateTime(start, "UTC"),
    DtEnd = new CalDateTime(end, "UTC"),
    RecurrenceRules = new List<IRecurrencePattern> { rrule },
    Summary = "This is an event",
    Uid = "abab717c-1786-4efc-87dd-6859c2b48eb6",
};

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

//Read the ical text, and make sure the programmatic composition and the raw text produce
//equivalent Calendars
var collection = Calendar.LoadFromStream(new StringReader(ical));
var firstEvent = collection.First().Events.First();

Assert.AreEqual(e, firstEvent);

Here's the piece you care about, building on the above:

// Midnight on July 1, 2016 is start of the search range
var startSearch = new CalDateTime(DateTime.Parse("2016-07-01T00:00:00"), "UTC");

// You are constraining the end of your search to Aug 31 @ 00:00, which is before Aug 31 @ 08:00
// If you extend the range to 2016-08-31T07:00:01 and on, the occurrence you expect will show up
var endSearch = new CalDateTime(DateTime.Parse("2016-08-31T07:00:01"), "UTC");  //Yes

var occurrences = firstEvent.GetOccurrences(startSearch, endSearch)
    .Select(o => o.Period as Period)
    .OrderBy(p => p.StartTime)
    .ToList();

If you make endSearch any earlier than Aug 31 @ 07:00:01, the last occurrence will not show up.

A couple of thoughts:

• Evaluating the recurrence rule does indeed have UNTIL as an inclusive parameter
• Evaluating the search query (calling GetOccurrences()) should not change based on the RRULE being evaluated. I.e. search semantics should be stable.
• Right now, a Period must have at least some overlap with searchEnd when calling GetOccurrences() in order to show up in a recurrence set.

That last piece can be re-evaluated, but I think it's OK as-is. This is one of the problems with time. If you have an event that starts at Aug 5 @ 9:00am, and you ask about events that occur between midnight on Aug 1 and Aug 5 @ 9:00am, should the search result include the thing that starts at 9am? It depends who you ask, but I lean towards "no".

As a general rule of thumb, using dday.ical as a source of truth for anything is a really bad idea, because it's buggy throughout. It's a useful reference point, but not a source of truth.

[rianjs]

I added a unit test that asserts that there must be some overlap for an occurrence to show up in a recurrence set. There was no bug, but it doesn't hurt to assert implicit rules.

9487b13ce5b449de49603da4d596bf13f46646a5

Actually, I should probably add that detail to the GetOccurrences intellisense documentation...

[rianjs]

Better intellisense docs: 8f691ac8ef01dc10bdac7ad432952be859c0d15e

[rvaidya]

We use the library to express recurrence patterns for rules in our software's scheduling engine in a syntactically standard way that has supporting libraries on multiple platforms. We're only using the recurrence part, so we're not actually dealing with VEVENTs at all. Additionally, we also only deal with dates, so the 7AM UTC coming in is a bug on our side.

Here's what we're calling (this is based off a code sample from Doug Day for a similar scenario):

RecurrencePattern rp = new RecurrencePattern(recurrenceRule); RecurrencePatternEvaluator rpe = new RecurrencePatternEvaluator(rp); rpe.Evaluate(periodStart, periodStart, periodEnd, false);

It works if we strip the DTSTART and UNTIL out of the pattern - does this change how it's evaluated internally? The RRULE here is just "RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=WE" with searchStart and searchEnd, and reference date set to searchStart.

Since we're dealing with dates, when someone asks "I want dates from 8/1 to 8/31" the 8/31 is included.

I agree with how you've defined the boundary between events that have time, and I'm fine with none of this resulting in code changes since our use case is nonstandard (we're handling this by adding 1ms to the searchEnd date to make the end inclusive). It sounds like DDay.iCal returning 8/31 when DTSTART/UNTIL are specified in the string, even with times set at 12AM UTC, is a bug on their side.

I'd just like to make sure that stripping out DTSTART/DTEND and having 8/31 be returned is correct behavior from your perspective so that we can continue trying out this updated library. The main reason we want to switch is that DDay.iCal is extremely slow. We need to evaluate tens to thousands of recurrence pattern strings like this one within the scope of a user action.

To alleviate this, I've had to add a hack where I can evaluate very simple recurrence patterns (daily, weekly, with BYDAY and INTERVAL and nothing else) using my own custom code and for loops, but anytime a pattern doesn't fall into the simple subset the run time goes from being basically immeasureable to upwards of 30 seconds, since it starts going into DDay.iCal.