Skip to content

Commit

Permalink
Fix #729: incorrect expansion behaviour after BYWEEKNO (#731)
Browse files Browse the repository at this point in the history 
* Fix #729: incorrect expansion behaviour after `BYWEEKNO`

I.e. fix `BYMONTH`, `BYMONTHDAY`, `BYYEARDAY` in relation to `BYWEEKNO`

* Reduce cyclomatic complexity of `GetWeekNoVariants` to make CodeCov a little happier.

* Make method static to avoid CodeCov complaining.

---------

Co-authored-by: axunonb <[email protected]>
  • Loading branch information
@minichma @axunonb
minichma and axunonb authored Feb 20, 2025
1 parent 76a7aa7 commit 9725f2c
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 41 deletions.
61 changes: 61 additions & 0 deletions Ical.Net.Tests/Calendars/Recurrence/RecurrenceTestCases.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,67 @@ RRULE:FREQ=YEARLY;BYWEEKNO=1;BYDAY=MO,TU;INTERVAL=3;UNTIL=20320101
DTSTART:20241231
INSTANCES:20241231,20280103,20280104,20301230,20301231


# 30.12 in WEEK 1 with expansion on BYMONTH, BYMONTHDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYMONTH=12;BYMONTHDAY=1,30;BYWEEKNO=1;UNTIL=20320101
DTSTART:20241230
INSTANCES:20241230,20251230,20301230,20311230

# 30.12 in WEEK 1 with expansion on BYYEARDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYYEARDAY=-2;BYWEEKNO=1;UNTIL=20320101
DTSTART:20241230
INSTANCES:20241230,20251230,20301230,20311230

# 30.12 in WEEK 52 with expansion on BYMONTH, BYMONTHDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYMONTH=12;BYMONTHDAY=1,30;BYWEEKNO=52;UNTIL=20340101
DTSTART:20271230
INSTANCES:20271230,20281230,20291230,20331230

# 30.12 in WEEK 52 with expansion on BYYEARDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYYEARDAY=-2;BYWEEKNO=52;UNTIL=20340101
DTSTART:20271230
INSTANCES:20271230,20281230,20291230,20331230

# 30.12 in WEEK 53 with expansion on BYMONTH, BYMONTHDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYMONTH=12;BYMONTHDAY=1,30;BYWEEKNO=53;UNTIL=20380101
DTSTART:20261230
INSTANCES:20261230,20321230,20371230

# 30.12 in WEEK 53 with expansion on BYYEARDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYYEARDAY=-2;BYWEEKNO=53;UNTIL=20380101
DTSTART:20261230
INSTANCES:20261230,20321230,20371230

# 2.1 in WEEK 1 with expansion on BYMONTH, BYMONTHDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYMONTH=1;BYMONTHDAY=2,31;BYWEEKNO=1;UNTIL=20290102
DTSTART:20250102
INSTANCES:20250102,20260102,20290102

# 2.1 in WEEK 1 with expansion on BYYEARDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYYEARDAY=2;BYWEEKNO=1;UNTIL=20290102
DTSTART:20250102
INSTANCES:20250102,20260102,20290102

# 2.1 in WEEK 52 with expansion on BYMONTH, BYMONTHDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYMONTH=1;BYMONTHDAY=2,31;BYWEEKNO=52;UNTIL=20390102
DTSTART:20280102
INSTANCES:20280102,20390102

# 2.1 in WEEK 52 with expansion on BYYEARDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYYEARDAY=2;BYWEEKNO=52;UNTIL=20390102
DTSTART:20280102
INSTANCES:20280102,20390102

# 2.1 in WEEK 53 with expansion on BYMONTH, BYMONTHDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYMONTH=1;BYMONTHDAY=2,31;BYWEEKNO=53;UNTIL=20380102
DTSTART:20270102
INSTANCES:20270102,20330102,20380102

# 2.1 in WEEK 53 with expansion on BYYEARDAY, BYWEEKNO
RRULE:FREQ=YEARLY;BYYEARDAY=2;BYWEEKNO=53;UNTIL=20380102
DTSTART:20270102
INSTANCES:20270102,20330102,20380102

# BYMONTHDAY with limit behaviour; reproduces #728 as reported by pinkfloydx33
RRULE:FREQ=DAILY;BYMONTHDAY=20,-2;UNTIL=20250401
DTSTART:20250220
Expand Down
101 changes: 60 additions & 41 deletions Ical.Net/Evaluation/RecurrencePatternEvaluator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,18 @@ private IEnumerable<DateTime> EnumerateDates(DateTime originalDate, DateTime see
}
}

private struct ExpandContext
{
/// <summary>
/// Indicates whether the dates have been fully expanded. If true, subsequent parts should only limit, not expand.
/// </summary>
/// <remarks>
/// This makes a difference in case of BYWEEKNO, which might span months and years. After it was applied (BYWEEKNO would
/// always expand), the subsequent parts mustn't expand.
/// </remarks>
public bool DatesFullyExpanded { get; set; }
}

/// <summary>
/// Returns a list of possible dates generated from the applicable BY* rules, using the specified date as a seed.
/// </summary>
Expand All @@ -231,12 +243,14 @@ private IEnumerable<DateTime> EnumerateDates(DateTime originalDate, DateTime see
/// <returns>A list of possible dates.</returns>
private ISet<DateTime> GetCandidates(DateTime date, RecurrencePattern pattern, bool?[] expandBehaviors)
{
var expandContext = new ExpandContext() { DatesFullyExpanded = false };

var dates = new List<DateTime> { date };
dates = GetMonthVariants(dates, pattern, expandBehaviors[0]);
dates = GetWeekNoVariants(dates, pattern, expandBehaviors[1]);
dates = GetYearDayVariants(dates, pattern, expandBehaviors[2]);
dates = GetMonthDayVariants(dates, pattern, expandBehaviors[3]);
dates = GetDayVariants(dates, pattern, expandBehaviors[4]);
dates = GetWeekNoVariants(dates, pattern, expandBehaviors[1], ref expandContext);
dates = GetYearDayVariants(dates, pattern, expandBehaviors[2], ref expandContext);
dates = GetMonthDayVariants(dates, pattern, expandBehaviors[3], ref expandContext);
dates = GetDayVariants(dates, pattern, expandBehaviors[4], ref expandContext);
dates = GetHourVariants(dates, pattern, expandBehaviors[5]);
dates = GetMinuteVariants(dates, pattern, expandBehaviors[6]);
dates = GetSecondVariants(dates, pattern, expandBehaviors[7]);
Expand Down Expand Up @@ -306,7 +320,7 @@ private List<DateTime> GetMonthVariants(List<DateTime> dates, RecurrencePattern
/// </summary>
/// <param name="dates">The list of dates to which the BYWEEKNO rules will be applied.</param>
/// <returns>The modified list of dates after applying the BYWEEKNO rules.</returns>
private List<DateTime> GetWeekNoVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand)
private List<DateTime> GetWeekNoVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext)
{
if (expand == null || pattern.ByWeekNo.Count == 0)
{
Expand All @@ -320,47 +334,47 @@ private List<DateTime> GetWeekNoVariants(List<DateTime> dates, RecurrencePattern

// Expand behavior
var weekNoDates = new List<DateTime>();
foreach (var t in dates)
foreach ((var t, var weekNo) in dates.SelectMany(t => GetByWeekNoForYearNormalized(pattern, t.Year), (t, weekNo) => (t, weekNo)))
{
foreach (var weekNo in GetByWeekNoForYearNormalized(pattern, t.Year))
{
var date = t;
var date = t;

// Make sure we start from a reference date that is in a week that belongs to the current year.
// Its not important that the date lies in a certain week, but that the week belongs to the
// current year and that the week day is preserved.
if (date.Month == 1)
date = date.AddDays(7);
else if (date.Month >= 12)
date = date.AddDays(-7);
// Make sure we start from a reference date that is in a week that belongs to the current year.
// Its not important that the date lies in a certain week, but that the week belongs to the
// current year and that the week day is preserved.
if (date.Month == 1)
date = date.AddDays(7);
else if (date.Month >= 12)
date = date.AddDays(-7);

// Determine our current week number
var currWeekNo = Calendar.GetIso8601WeekOfYear(date, pattern.FirstDayOfWeek);
// Determine our current week number
var currWeekNo = Calendar.GetIso8601WeekOfYear(date, pattern.FirstDayOfWeek);

// Move ahead to the correct week of the year
date = date.AddDays((weekNo - currWeekNo) * 7);
// Move ahead to the correct week of the year
date = date.AddDays((weekNo - currWeekNo) * 7);

// Ignore the week if it doesn't belong to the current year.
if (Calendar.GetIso8601YearOfWeek(date, pattern.FirstDayOfWeek) == t.Year)
{
// Step backward single days until we're at the correct DayOfWeek
while (date.DayOfWeek != pattern.FirstDayOfWeek)
{
date = date.AddDays(-1);
}
// Ignore the week if it doesn't belong to the current year.
if (Calendar.GetIso8601YearOfWeek(date, pattern.FirstDayOfWeek) != t.Year)
continue;

for (var k = 0; k < 7; k++)
{
weekNoDates.Add(date);
date = date.AddDays(1);
}
}
}
// Step backward single days until we're at the correct DayOfWeek
date = GetFirstDayOfWeekDate(date, pattern.FirstDayOfWeek);

weekNoDates.AddRange(Enumerable.Range(0, 7).Select(i => date.AddDays(i)));
}

// subsequent parts should only limit, not expand
expandContext.DatesFullyExpanded = true;

// Apply BYMONTH limit behavior, as we might have expanded over month/year boundaries
// in this method and BYMONTH has already been applied before, so wouldn't be again.
weekNoDates = GetMonthVariants(weekNoDates, pattern, expand: false);

return weekNoDates;
}

private static DateTime GetFirstDayOfWeekDate(DateTime date, DayOfWeek firstDayOfWeek)
=> date.AddDays(-((int) date.DayOfWeek + 7 - (int) firstDayOfWeek) % 7);

/// <summary>
/// Normalize the BYWEEKNO values to be positive integers.
/// </summary>
Expand All @@ -378,14 +392,14 @@ private List<int> GetByWeekNoForYearNormalized(RecurrencePattern pattern, int ye
/// </summary>
/// <param name="dates">The list of dates to which the BYYEARDAY rules will be applied.</param>
/// <returns>The modified list of dates after applying the BYYEARDAY rules.</returns>
private List<DateTime> GetYearDayVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand)
private static List<DateTime> GetYearDayVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext)
{
if (expand == null || pattern.ByYearDay.Count == 0)
{
return dates;
}

if (expand.Value)
if (expand.Value && !expandContext.DatesFullyExpanded)
{
var yearDayDates = new List<DateTime>(dates.Count);
foreach (var date in dates)
Expand All @@ -397,6 +411,8 @@ private List<DateTime> GetYearDayVariants(List<DateTime> dates, RecurrencePatter
// Ignore the BY values that don't fit into the current year (i.e. +-366 in non-leap-years).
.Where(d => d.Year == date1.Year));
}

expandContext.DatesFullyExpanded = true;
return yearDayDates;
}
// Limit behavior
Expand Down Expand Up @@ -434,14 +450,14 @@ private List<DateTime> GetYearDayVariants(List<DateTime> dates, RecurrencePatter
/// </summary>
/// <param name="dates">The list of dates to which the BYMONTHDAY rules will be applied.</param>
/// <returns>The modified list of dates after applying the BYMONTHDAY rules.</returns>
private List<DateTime> GetMonthDayVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand)
private List<DateTime> GetMonthDayVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext)
{
if (expand == null || pattern.ByMonthDay.Count == 0)
{
return dates;
}

if (expand.Value)
if (expand.Value && !expandContext.DatesFullyExpanded)
{
var monthDayDates = new List<DateTime>();
foreach (var date in dates)
Expand All @@ -455,6 +471,8 @@ select monthDay > 0
: date.AddDays(-date.Day + 1).AddMonths(1).AddDays(monthDay)
);
}

expandContext.DatesFullyExpanded = true;
return monthDayDates;
}

Expand Down Expand Up @@ -500,14 +518,14 @@ select monthDay > 0
/// </summary>
/// <param name="dates">The list of dates to which BYDAY rules will be applied.</param>
/// <returns>The modified list of dates after applying BYDAY rules, or the original list if no BYDAY rules are specified.</returns>
private List<DateTime> GetDayVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand)
private List<DateTime> GetDayVariants(List<DateTime> dates, RecurrencePattern pattern, bool? expand, ref ExpandContext expandContext)
{
if (expand == null || pattern.ByDay.Count == 0)
{
return dates;
}

if (expand.Value)
if (expand.Value && !expandContext.DatesFullyExpanded)
{
// Expand behavior
var weekDayDates = new List<DateTime>();
Expand All @@ -519,6 +537,7 @@ private List<DateTime> GetDayVariants(List<DateTime> dates, RecurrencePattern pa
}
}

expandContext.DatesFullyExpanded = true;
return weekDayDates;
}

Expand Down

0 comments on commit 9725f2c

Please sign in to comment.