Skip to content

Don't skip anchors with strict matching in Calendar.RecurrenceRule. Resolve #881 #1000

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

Merged
merged 1 commit into from
Nov 1, 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
39 changes: 36 additions & 3 deletions Sources/FoundationEssentials/Calendar/Calendar_Recurrence.swift
Original file line number Diff line number Diff line change
Expand Up @@ -230,12 +230,42 @@ extension Calendar {
case .monthly: [.second, .minute, .hour, .day]
case .yearly: [.second, .minute, .hour, .day, .month]
}
let componentsForEnumerating = recurrence.calendar._dateComponents(components, from: start)
var componentsForEnumerating = recurrence.calendar._dateComponents(components, from: start)

let rangeForBaseRecurrence: Range<Date>? = nil
let expansionChangesDay = dayOfYearAction == .expand || dayOfMonthAction == .expand || weekAction == .expand || weekdayAction == .expand
let expansionChangesMonth = dayOfYearAction == .expand || monthAction == .expand || weekAction == .expand

if expansionChangesDay, componentsForEnumerating.day != nil {
// If we expand either the day of the month or weekday, then
// the day of month is likely to not match that of the start
// date. Reset it to 1 in the base recurrence as to not skip
// "invalid" anchors, such as February 30
componentsForEnumerating.day = 1
}
if expansionChangesMonth, componentsForEnumerating.month != nil {
// Likewise, if we will be changing the month, reset it to 1
// in case the start date falls on a leap month
componentsForEnumerating.month = 1
componentsForEnumerating.isLeapMonth = nil
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would we also need to handle nth-weekday and yearly combination?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, good catch! Added more exhaustive checks and cleaned up the logic a bit to be easier to read

if expansionChangesDay || expansionChangesMonth, weekAction == .expand, weekdayAction != .expand {
// If we are expanding weeks, all expansions in a given year
// will have the same weekday. Above we have reset the month
// or the day of the month, so we also changed that weekday.

// To specify a yearly recurrence which starts from the same
// weekday, and which doesn't start from a leap day / month,
// simply use `dayOfYear` of the start date
componentsForEnumerating.day = nil
componentsForEnumerating.month = nil
componentsForEnumerating.isLeapMonth = nil
let daysInWeek = recurrence.calendar.maximumRange(of: .weekday)!.count
componentsForEnumerating.dayOfYear = recurrence.calendar.component(.dayOfYear, from: start) % daysInWeek // mod 7 to get the same weekday in the beginning of the year, so it's guaranteed to always exist
}

baseRecurrence = Calendar.DatesByMatching(calendar: recurrence.calendar,
start: start,
range: rangeForBaseRecurrence,
range: nil,
matchingComponents: componentsForEnumerating,
matchingPolicy: recurrence.matchingPolicy,
repeatedTimePolicy: recurrence.repeatedTimePolicy,
Expand Down Expand Up @@ -335,6 +365,9 @@ extension Calendar {
componentCombinations.weekdays = recurrence.weekdays
componentCombinations.daysOfYear = nil
componentCombinations.daysOfMonth = nil
if recurrence.frequency == .yearly, monthAction != .expand {
componentCombinations.months = nil
}
} else if recurrence.frequency == .weekly || weekAction == .expand {
if let weekdayIdx = components.weekday, let weekday = Locale.Weekday(weekdayIdx) {
// In a weekly recurrence (or one that expands weeks of year), we want results to fall on the same weekday as the initial date
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -676,4 +676,67 @@ final class GregorianCalendarRecurrenceRuleTests: XCTestCase {

XCTAssertEqual(results, [])
}

func testFirstMondaysStrictMatching() {
let startDate = Date(timeIntervalSince1970: 1706659200.0) // 2024-01-31T00:00:00-0000

var rule = Calendar.RecurrenceRule(calendar: gregorian, frequency: .monthly, matchingPolicy: .strict)
rule.weekdays = [.nth(1, .monday)]

var dates = rule.recurrences(of: startDate).makeIterator()
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1707091200.0)) // 2024-02-05T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1709510400.0)) // 2024-03-04T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1711929600.0)) // 2024-04-01T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1714953600.0)) // 2024-05-06T00:00:00-0000
}

func testFifthFridaysStrictMatching() {
let startDate = Date(timeIntervalSince1970: 1706659200.0) // 2024-01-31T00:00:00-0000

var rule = Calendar.RecurrenceRule(calendar: gregorian, frequency: .monthly, matchingPolicy: .strict)
rule.weekdays = [.nth(5, .friday)]

var dates = rule.recurrences(of: startDate).makeIterator()
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1711670400.0)) // 2024-03-29T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1717113600.0)) // 2024-05-31T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1724976000.0)) // 2024-08-30T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1732838400.0)) // 2024-11-29T00:00:00-0000
}

func testYearlyRecurrenceWeekdayExpansionStrictMatching() {
let startDate = Date(timeIntervalSince1970: 1709164800.0) // 2024-02-29T00:00:00-0000

var rule = Calendar.RecurrenceRule(calendar: gregorian, frequency: .yearly, matchingPolicy: .strict)
rule.weekdays = [.nth(5, .friday)]

var dates = rule.recurrences(of: startDate).makeIterator()
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1738281600.0)) // 2025-01-31T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1769731200.0)) // 2026-01-30T00:00:00-0000
}

func testYearlyRecurrenceDayOfYearExpansionStrictMatching() {
let startDate = Date(timeIntervalSince1970: 1709164800.0) // 2024-02-29T00:00:00-0000

var rule = Calendar.RecurrenceRule(calendar: gregorian, frequency: .yearly, matchingPolicy: .strict)
rule.daysOfTheYear = [61]

var dates = rule.recurrences(of: startDate).makeIterator()
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1709251200.0)) // 2024-03-01T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1740873600.0)) // 2025-03-02T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1772409600.0)) // 2026-03-02T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1803945600.0)) // 2027-03-02T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1835481600.0)) // 2028-03-01T00:00:00-0000
}

func testYearlyRecurrenceWeekExpansionStrictMatching() {
let startDate = Date(timeIntervalSince1970: 1709164800.0) // 2024-02-29T00:00:00-0000

var rule = Calendar.RecurrenceRule(calendar: gregorian, frequency: .yearly, matchingPolicy: .strict)
rule.weeks = [2]

var dates = rule.recurrences(of: startDate).makeIterator()
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1736553600.0)) // 2025-01-11T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1767484800.0)) // 2026-01-04T00:00:00-0000
XCTAssertEqual(dates.next(), Date(timeIntervalSince1970: 1799020800.0)) // 2027-01-04T00:00:00-0000
}
}