From 8d9c59668418dcd3333bcd4bdbcbcea8d865d5c5 Mon Sep 17 00:00:00 2001 From: mup Date: Thu, 6 Feb 2025 13:51:59 +0100 Subject: [PATCH] [iOS] Fix iOS tests and SDK event expansion The way that we count events and occurrences changed with the implementation of the advanced repeat rules, given that, we've had to fix tests that relies on these counts. This commit address one corner case for weekly events expansion on SDK and fixes the tests on Swift. --- .../Alarms/AlarmModel.swift | 5 +- app-ios/tutanotaTests/AlarmManagerTest.swift | 8 +- app-ios/tutanotaTests/AlarmModelTest.swift | 8 +- tuta-sdk/rust/sdk/src/date/event_facade.rs | 117 +++++++++++++++--- 4 files changed, 112 insertions(+), 26 deletions(-) diff --git a/app-ios/TutanotaSharedFramework/Alarms/AlarmModel.swift b/app-ios/TutanotaSharedFramework/Alarms/AlarmModel.swift index d3622639dc84..e3b619a8e412 100644 --- a/app-ios/TutanotaSharedFramework/Alarms/AlarmModel.swift +++ b/app-ios/TutanotaSharedFramework/Alarms/AlarmModel.swift @@ -129,6 +129,7 @@ private struct LazyEventSequence: Sequence, IteratorProtocol { mutating func next() -> EventOccurrence? { if case let .count(n) = repeatRule.endCondition, occurrenceNumber >= n { return nil } + if intervalNumber > EVENTS_SCHEDULED_AHEAD { return nil } if expandedEvents.isEmpty { let nextExpansionProgenitor = cal.date(byAdding: self.calendarComponent, value: repeatRule.interval * intervalNumber, to: calcEventStart)! @@ -136,7 +137,7 @@ private struct LazyEventSequence: Sequence, IteratorProtocol { let eventFacade = EventFacade() let byRules = repeatRule.advancedRules.map { $0.toSDKRule() } let generatedEvents = eventFacade.generateFutureInstances( - date: progenitorTime * 1000, + date: progenitorTime, repeatRule: EventRepeatRule(frequency: repeatRule.frequency.toSDKPeriod(), byRules: byRules) ) self.expandedEvents.append(contentsOf: generatedEvents) @@ -162,7 +163,7 @@ private struct LazyEventSequence: Sequence, IteratorProtocol { return true } - .map { (_, event) in event } + .map { (_, event) in event }.sorted { $1 < $0 } if let date = expandedEvents.popLast() { occurrenceNumber += 1 diff --git a/app-ios/tutanotaTests/AlarmManagerTest.swift b/app-ios/tutanotaTests/AlarmManagerTest.swift index 85c0bb2501f4..40a71156b589 100644 --- a/app-ios/tutanotaTests/AlarmManagerTest.swift +++ b/app-ios/tutanotaTests/AlarmManagerTest.swift @@ -126,15 +126,15 @@ class AlarmManagerTest: XCTestCase { [ ScheduledAlarmInfo( alarmTime: start2.advanced(by: 24, .hours).advanced(by: -10, .minutes), - occurrence: 1, - identifier: ocurrenceIdentifier(alarmIdentifier: alarm2.identifier, occurrence: 1), + occurrence: 2, + identifier: ocurrenceIdentifier(alarmIdentifier: alarm2.identifier, occurrence: 2), summary: alarm2.summary, eventDate: start2.advanced(by: 24, .hours) ), ScheduledAlarmInfo( alarmTime: start2.advanced(by: -10, .minutes), - occurrence: 0, - identifier: ocurrenceIdentifier(alarmIdentifier: alarm2.identifier, occurrence: 0), + occurrence: 1, + identifier: ocurrenceIdentifier(alarmIdentifier: alarm2.identifier, occurrence: 1), summary: alarm2.summary, eventDate: start2 ), diff --git a/app-ios/tutanotaTests/AlarmModelTest.swift b/app-ios/tutanotaTests/AlarmModelTest.swift index c50b1a29e24c..46075519cb25 100644 --- a/app-ios/tutanotaTests/AlarmModelTest.swift +++ b/app-ios/tutanotaTests/AlarmModelTest.swift @@ -72,7 +72,7 @@ class AlarmModelTest: XCTestCase { let result = plan(alarms: [alarm]) XCTAssertEqual(result.count, 3) - XCTAssertEqual(result[2].occurrenceNumber, 2) + XCTAssertEqual(result[2].occurrenceNumber, 3) } func testWhenRepeatedAlarmStartsBeforeNowOnlyFutureOcurrencesArePlanned() { @@ -93,7 +93,7 @@ class AlarmModelTest: XCTestCase { let result = plan(alarms: [alarm]) XCTAssertEqual(result.count, 2) - XCTAssertEqual(result[1].occurrenceNumber, 2) + XCTAssertEqual(result[1].occurrenceNumber, 3) } func testWhenMultipleAlarmsArePresentOnlyTheNewestOccurrencesArePlanned() { @@ -169,8 +169,8 @@ class AlarmModelTest: XCTestCase { let occurrences = prefix(seq: seq, 5).map { $0.eventOccurrenceTime } let expected = [ - date(2025, 2, 2, 11, timeZone), date(2025, 2, 3, 11, timeZone), date(2025, 2, 4, 11, timeZone), date(2025, 2, 10, 11, timeZone), - date(2025, 2, 11, 11, timeZone), + date(2025, 2, 2, 12, timeZone), date(2025, 2, 3, 12, timeZone), date(2025, 2, 4, 12, timeZone), date(2025, 2, 10, 12, timeZone), + date(2025, 2, 11, 12, timeZone), ] XCTAssertEqual(occurrences, expected) } diff --git a/tuta-sdk/rust/sdk/src/date/event_facade.rs b/tuta-sdk/rust/sdk/src/date/event_facade.rs index 84e21141caf3..066024eca4c5 100644 --- a/tuta-sdk/rust/sdk/src/date/event_facade.rs +++ b/tuta-sdk/rust/sdk/src/date/event_facade.rs @@ -538,31 +538,51 @@ impl EventFacade { { new_dates.push(*date) } else if frequency == &RepeatPeriod::Weekly && target_week_day.is_some() { - let mut new_date = date.replace_date( - Date::from_iso_week_date( - date.year(), - date.iso_week(), - Weekday::from_short(target_week_day.unwrap().as_str()), - ) - .unwrap(), - ); - let interval_start = date.replace_date( - Date::from_iso_week_date(date.year(), date.iso_week(), week_start).unwrap(), - ); + let parsed_target_week_day = + Weekday::from_short(target_week_day.unwrap().as_str()); + let mut interval_start = *date; + while interval_start.date().weekday() != week_start { + interval_start = interval_start.sub(Duration::days(1)); + } + + let mut new_date = interval_start; + while new_date.weekday() != parsed_target_week_day { + new_date = new_date.add(Duration::days(1)) + } + + /* + if interval_start.assume_utc().unix_timestamp() + < date.assume_utc().unix_timestamp() + { + interval_start = interval_start.add(Duration::weeks(1)) + } + */ + let next_event = date.add(Duration::weeks(1)).assume_utc().unix_timestamp(); if new_date.assume_utc().unix_timestamp() - > interval_start + >= interval_start .add(Duration::weeks(1)) .assume_utc() .unix_timestamp() { continue; } else if new_date.assume_utc().unix_timestamp() - < interval_start.assume_utc().unix_timestamp() + < date.assume_utc().unix_timestamp() { new_date = new_date.add(Duration::weeks(1)); } + if (new_date.assume_utc().unix_timestamp() >= next_event) + || (week_start != Weekday::Monday // We have WKST + && new_date.assume_utc().unix_timestamp() + >= interval_start + .add(Duration::weeks(1)) + .assume_utc() + .unix_timestamp()) + { + continue; + } + if valid_months.is_empty() || valid_months.contains(&new_date.month().to_number()) { @@ -2221,7 +2241,39 @@ mod tests { } #[test] - fn test_flow_weekly_with_by_day_and_wkst() { + fn test_flow_weekly_with_by_day_edge() { + let time = Time::from_hms(13, 23, 00).unwrap(); + let date = PrimitiveDateTime::new( + Date::from_calendar_date(2025, Month::February, 2).unwrap(), + time, + ); + + let repeat_rule = EventRepeatRule { + frequency: RepeatPeriod::Weekly, + by_rules: vec![ + ByRule { + by_rule: ByRuleType::ByDay, + interval: "MO".to_string(), + }, + ByRule { + by_rule: ByRuleType::ByDay, + interval: "TU".to_string(), + }, + ], + }; + + let event_recurrence = EventFacade {}; + assert_eq!( + event_recurrence.generate_future_instances(date.to_date_time(), repeat_rule.clone()), + [ + date.replace_day(3).unwrap().to_date_time(), + date.replace_day(4).unwrap().to_date_time() + ] + ); + } + + #[test] + fn test_flow_weekly_with_by_day_and_wkst_edge() { let time = Time::from_hms(13, 23, 00).unwrap(); let date = PrimitiveDateTime::new( Date::from_calendar_date(2025, Month::February, 10).unwrap(), @@ -2246,12 +2298,45 @@ mod tests { ], }; + let event_recurrence = EventFacade {}; + assert_eq!( + event_recurrence.generate_future_instances(date.to_date_time(), repeat_rule.clone()), + [date.replace_day(13).unwrap().to_date_time(),] + ); + } + + #[test] + fn test_flow_weekly_with_by_day_and_wkst() { + let time = Time::from_hms(13, 23, 00).unwrap(); + let date = PrimitiveDateTime::new( + Date::from_calendar_date(2025, Month::February, 7).unwrap(), + time, + ); + + let repeat_rule = EventRepeatRule { + frequency: RepeatPeriod::Weekly, + by_rules: vec![ + ByRule { + by_rule: ByRuleType::Wkst, + interval: "FR".to_string(), + }, + ByRule { + by_rule: ByRuleType::ByDay, + interval: "TH".to_string(), + }, + ByRule { + by_rule: ByRuleType::ByDay, + interval: "FR".to_string(), + }, + ], + }; + let event_recurrence = EventFacade {}; assert_eq!( event_recurrence.generate_future_instances(date.to_date_time(), repeat_rule.clone()), [ - date.replace_day(14).unwrap().to_date_time(), - date.replace_day(20).unwrap().to_date_time() + date.replace_day(7).unwrap().to_date_time(), + date.replace_day(13).unwrap().to_date_time(), ] ); }