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

Fix some recurrence evaluation issues #645

Merged
merged 3 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions Ical.Net.Tests/Calendars/Recurrence/RecurrenceTestCases.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@
RRULE:FREQ=WEEKLY;BYDAY=MO,TH;COUNT=3
DTSTART:20241024
INSTANCES:20241024,20241028,20241031

# Illegal rule part with multiple '='
RRULE:FREQ=WEEKLY;BYDAY=MO=;COUNT=3
EXCEPTION:System.ArgumentException
16 changes: 15 additions & 1 deletion Ical.Net.Tests/RecurrenceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3631,6 +3631,8 @@ public class RecurrenceTestCase

public IReadOnlyList<CalDateTime> Instances { get; set; }

public string Exception { get; set; }

public override string ToString()
=> $"Line {LineNumber}: {DtStart}, {RRule}";
}
Expand Down Expand Up @@ -3686,6 +3688,10 @@ private static IEnumerable<RecurrenceTestCase> ParseTestCaseFile(string fileCont
case "INSTANCES":
current.Instances = val.Split(',').Select(dt => new CalDateTime(dt) { TzId = "UTC" }).ToList();
break;

case "EXCEPTION":
current.Exception = val;
break;
}
}

Expand Down Expand Up @@ -3718,6 +3724,14 @@ public void ExecuteRecurrenceTestCase(RecurrenceTestCase testCase)

// Start at midnight, UTC time
evt.Start = testCase.DtStart;

if (testCase.Exception != null)
{
var exceptionType = Type.GetType(testCase.Exception);
Assert.Throws(exceptionType, () => new RecurrencePattern(testCase.RRule));
return;
}

evt.RecurrenceRules.Add(new RecurrencePattern(testCase.RRule));

var occurrences = evt.GetOccurrences(testCase.StartAt?.Value ?? DateTime.MinValue, DateTime.MaxValue)
Expand All @@ -3728,4 +3742,4 @@ public void ExecuteRecurrenceTestCase(RecurrenceTestCase testCase)

Assert.That(startDates, Is.EqualTo(testCase.Instances));
}
}
}
33 changes: 15 additions & 18 deletions Ical.Net.Tests/contrib/libical/icalrecur_test.out
Original file line number Diff line number Diff line change
Expand Up @@ -189,11 +189,10 @@ DTSTART:19970805T090000
INSTANCES:19970805T090000,19970817T090000,19970819T090000,19970831T090000
PREV-INSTANCES:19970819T090000,19970817T090000,19970805T090000

# TODO: FIX (see https://github.com/ical-org/ical.net/issues/618)
# RRULE:FREQ=WEEKLY;INTERVAL=1;COUNT=2;BYDAY=MO;
# DTSTART:20141006T090000
# INSTANCES:20141006T090000,20141013T090000
# PREV-INSTANCES:20141006T090000
RRULE:FREQ=WEEKLY;INTERVAL=1;COUNT=2;BYDAY=MO;
DTSTART:20141006T090000
INSTANCES:20141006T090000,20141013T090000
PREV-INSTANCES:20141006T090000

RRULE:FREQ=DAILY;COUNT=10
DTSTART:19970902T090000
Expand Down Expand Up @@ -307,19 +306,17 @@ START-AT:20170915T090000
INSTANCES:20171006T090000,20171103T090000,20171201T090000
PREV-INSTANCES:20170901T090000

# TODO: FIX (see https://github.com/ical-org/ical.net/issues/618)
# RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
# DTSTART:20161229T090000
# START-AT:20161231T090000
# INSTANCES:20170101T090000,20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
# PREV-INSTANCES:20161229T090000

# TODO: FIX (see https://github.com/ical-org/ical.net/issues/618)
# RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
# DTSTART:20161229T090000
# START-AT:20170102T090000
# INSTANCES:20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
# PREV-INSTANCES:20170101T090000,20161229T090000
RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
DTSTART:20161229T090000
START-AT:20161231T090000
INSTANCES:20170101T090000,20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
PREV-INSTANCES:20161229T090000

RRULE:FREQ=WEEKLY;UNTIL=20170127T000000Z;WKST=MO;BYDAY=SU,TU,TH;INTERVAL=2
DTSTART:20161229T090000
START-AT:20170102T090000
INSTANCES:20170110T090000,20170112T090000,20170115T090000,20170124T090000,20170126T090000
PREV-INSTANCES:20170101T090000,20161229T090000

RRULE:FREQ=DAILY;UNTIL=20170131T140000Z;BYMONTH=1;INTERVAL=3
DTSTART:20170101T090000
Expand Down
9 changes: 9 additions & 0 deletions Ical.Net/Evaluation/RecurrencePatternEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@
}
}
}
#pragma warning 0618 restore

Check warning on line 238 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View workflow job for this annotation

GitHub Actions / tests

Expected 'disable' or 'restore'

Check warning on line 238 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View workflow job for this annotation

GitHub Actions / tests

Expected 'disable' or 'restore'

Check warning on line 238 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View workflow job for this annotation

GitHub Actions / tests

Expected 'disable' or 'restore'

Check warning on line 238 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View workflow job for this annotation

GitHub Actions / coverage

Expected 'disable' or 'restore'

Check warning on line 238 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View workflow job for this annotation

GitHub Actions / coverage

Expected 'disable' or 'restore'

Check warning on line 238 in Ical.Net/Evaluation/RecurrencePatternEvaluator.cs

View workflow job for this annotation

GitHub Actions / coverage

Expected 'disable' or 'restore'
/// <summary>
/// Returns a list of start dates in the specified period represented by this recurrence pattern.
/// This method includes a base date argument, which indicates the start of the first occurrence of this recurrence.
Expand Down Expand Up @@ -658,6 +658,9 @@
{
var weekNo = Calendar.GetIso8601WeekOfYear(date, CalendarWeekRule.FirstFourDayWeek, pattern.FirstDayOfWeek);

// Go to the first day of the week
date = date.AddDays(-GetWeekDayOffset(date, pattern.FirstDayOfWeek));

// construct a list of possible week days..
while (date.DayOfWeek != dayOfWeek)
{
Expand Down Expand Up @@ -725,6 +728,12 @@
return GetOffsetDates(days, weekDay.Offset);
}

/// <summary>
/// Returns the days since the start of the week, 0 if the date is on the first day of the week.
/// </summary>
private static int GetWeekDayOffset(DateTime date, DayOfWeek startOfWeek)
=> date.DayOfWeek + ((date.DayOfWeek < startOfWeek) ? 7 : 0) - startOfWeek;

/// <summary>
/// Returns a single-element sublist containing the element of <paramref name="dates"/> at <paramref name="offset"/>.
/// Valid offsets are from 1 to the size of the list. If an invalid offset is supplied, all elements from <paramref name="dates"/>
Expand Down
15 changes: 14 additions & 1 deletion Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,20 @@ public override object Deserialize(TextReader tr)
var keywordPairs = match.Groups[2].Value.Split(';');
foreach (var keywordPair in keywordPairs)
{
if (keywordPair.Length == 0)
{
// This is illegal but ignore for now.
continue;
}

var keyValues = keywordPair.Split('=');
if (keyValues.Length != 2)
{
// ArgumentExceptions seem to be the Exception of choise for this class. Should
minichma marked this conversation as resolved.
Show resolved Hide resolved
// probably be changed to a more specific exception type.
throw new ArgumentException($"The recurrence rule part '{keywordPair}' is invalid.");
}

var keyword = keyValues[0];
var keyValue = keyValues[1];

Expand Down Expand Up @@ -493,4 +506,4 @@ public override object Deserialize(TextReader tr)

return r;
}
}
}
Loading