From 5155c91abdd72fdc6f8e6736392d860b25d53a79 Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Sun, 23 Feb 2025 15:33:27 +0100 Subject: [PATCH 1/6] CalDateTime: Remove `AsUtc` from `GetHashCode` as it is not required and to avoid expensive timezone operations. --- Ical.Net/DataTypes/CalDateTime.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/Ical.Net/DataTypes/CalDateTime.cs b/Ical.Net/DataTypes/CalDateTime.cs index 33d471813..f7f73cd3a 100644 --- a/Ical.Net/DataTypes/CalDateTime.cs +++ b/Ical.Net/DataTypes/CalDateTime.cs @@ -241,7 +241,6 @@ public override int GetHashCode() { var hashCode = Value.GetHashCode(); hashCode = (hashCode * 397) ^ HasTime.GetHashCode(); - hashCode = (hashCode * 397) ^ AsUtc.GetHashCode(); hashCode = (hashCode * 397) ^ (TzId != null ? TzId.GetHashCode() : 0); return hashCode; } From ff00276ab298e99faceb7d8f45d8ff82b1d71a7e Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Sun, 23 Feb 2025 15:35:12 +0100 Subject: [PATCH 2/6] CalDateTime: Avoid UTC conversion when comparing two instances having the same tzid. --- Ical.Net/DataTypes/CalDateTime.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Ical.Net/DataTypes/CalDateTime.cs b/Ical.Net/DataTypes/CalDateTime.cs index f7f73cd3a..1dfd0ec09 100644 --- a/Ical.Net/DataTypes/CalDateTime.cs +++ b/Ical.Net/DataTypes/CalDateTime.cs @@ -250,28 +250,28 @@ public override int GetHashCode() { return left != null && right != null - && ((left.IsFloating || right.IsFloating) ? left.Value < right.Value : left.AsUtc < right.AsUtc); + && ((left.IsFloating || right.IsFloating || left.TzId == right.TzId) ? left.Value < right.Value : left.AsUtc < right.AsUtc); } public static bool operator >(CalDateTime? left, CalDateTime? right) { return left != null && right != null - && ((left.IsFloating || right.IsFloating) ? left.Value > right.Value : left.AsUtc > right.AsUtc); + && ((left.IsFloating || right.IsFloating || left.TzId == right.TzId) ? left.Value > right.Value : left.AsUtc > right.AsUtc); } public static bool operator <=(CalDateTime? left, CalDateTime? right) { return left != null && right != null - && ((left.IsFloating || right.IsFloating) ? left.Value <= right.Value : left.AsUtc <= right.AsUtc); + && ((left.IsFloating || right.IsFloating || left.TzId == right.TzId) ? left.Value <= right.Value : left.AsUtc <= right.AsUtc); } public static bool operator >=(CalDateTime? left, CalDateTime? right) { return left != null && right != null - && ((left.IsFloating || right.IsFloating) ? left.Value >= right.Value : left.AsUtc >= right.AsUtc); + && ((left.IsFloating || right.IsFloating || left.TzId == right.TzId) ? left.Value >= right.Value : left.AsUtc >= right.AsUtc); } public static bool operator ==(CalDateTime? left, CalDateTime? right) From aa1f8f92b0c5b15180f9e7ddbe3a1eb70467b9a3 Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Sun, 23 Feb 2025 15:38:40 +0100 Subject: [PATCH 3/6] CalDateTime: Remove implicit conversion from `DateTime` to `CalDateTime` which can easily be confusing and error prone. --- Ical.Net.Benchmarks/OccurencePerfTests.cs | 4 ++-- Ical.Net.Tests/DocumentationExamples.cs | 9 ++++----- Ical.Net.Tests/RecurrenceTests.cs | 4 ++-- Ical.Net.Tests/SerializationTests.cs | 2 +- Ical.Net/DataTypes/CalDateTime.cs | 8 -------- .../DataTypes/RecurrencePatternSerializer.cs | 5 +---- 6 files changed, 10 insertions(+), 22 deletions(-) diff --git a/Ical.Net.Benchmarks/OccurencePerfTests.cs b/Ical.Net.Benchmarks/OccurencePerfTests.cs index 250b70999..5f0493a05 100644 --- a/Ical.Net.Benchmarks/OccurencePerfTests.cs +++ b/Ical.Net.Benchmarks/OccurencePerfTests.cs @@ -60,7 +60,7 @@ private static Calendar GetFourCalendarEventsWithUntilRule() { var rrule = new RecurrencePattern(FrequencyType.Daily, 1) { - Until = startTime.AddDays(10), + Until = new CalDateTime(startTime.AddDays(10)), }; var e = new CalendarEvent @@ -141,4 +141,4 @@ private static Calendar GetFourCalendarEventsWithCountRule() c.Events.AddRange(events); return c; } -} \ No newline at end of file +} diff --git a/Ical.Net.Tests/DocumentationExamples.cs b/Ical.Net.Tests/DocumentationExamples.cs index c76e20c54..c051fa276 100644 --- a/Ical.Net.Tests/DocumentationExamples.cs +++ b/Ical.Net.Tests/DocumentationExamples.cs @@ -21,14 +21,14 @@ public void Daily_Test() // We want it to recur through the end of July. var vEvent = new CalendarEvent { - DtStart = new CalDateTime(DateTime.Parse("2016-07-01T07:00")), - DtEnd = new CalDateTime(DateTime.Parse("2016-07-01T08:00")), + DtStart = new CalDateTime("20160701T070000"), + DtEnd = new CalDateTime("20160701T080000"), }; //Recur daily through the end of the day, July 31, 2016 var recurrenceRule = new RecurrencePattern(FrequencyType.Daily, 1) { - Until = DateTime.Parse("2016-07-31T23:59:59") + Until = new CalDateTime("20160731T235959") }; vEvent.RecurrenceRules = new List { recurrenceRule }; @@ -57,7 +57,7 @@ public void EveryOtherTuesdayUntilTheEndOfTheYear_Test() // Recurring every other Tuesday until Dec 31 var rrule = new RecurrencePattern(FrequencyType.Weekly, 2) { - Until = DateTime.Parse("2016-12-31T11:59:59") + Until = new CalDateTime("20161231T115959") }; vEvent.RecurrenceRules = new List { rrule }; @@ -88,7 +88,6 @@ public void FourthThursdayOfNovember_Tests() Interval = 1, ByMonth = new List { 11 }, ByDay = new List { new WeekDay { DayOfWeek = DayOfWeek.Thursday, Offset = 4 } }, - Until = DateTime.MaxValue }; vEvent.RecurrenceRules = new List { rrule }; diff --git a/Ical.Net.Tests/RecurrenceTests.cs b/Ical.Net.Tests/RecurrenceTests.cs index 9bde58ce5..e750a79b3 100644 --- a/Ical.Net.Tests/RecurrenceTests.cs +++ b/Ical.Net.Tests/RecurrenceTests.cs @@ -3134,7 +3134,7 @@ public void OccurrenceMustBeCompletelyContainedWithinSearchRange() var rrule = new RecurrencePattern(FrequencyType.Weekly, interval: 1) { - Until = DateTime.Parse("2016-08-31T07:00:00"), + Until = new CalDateTime("20160831T070000"), ByDay = new List { new WeekDay(DayOfWeek.Wednesday) }, }; @@ -3307,7 +3307,7 @@ public void ExDatesShouldGetMergedInOutput() { var start = _now.AddYears(-1); var end = start.AddHours(1); - var rrule = new RecurrencePattern(FrequencyType.Daily) { Until = start.AddYears(2) }; + var rrule = new RecurrencePattern(FrequencyType.Daily) { Until = new CalDateTime(start.AddYears(2)) }; var e = new CalendarEvent { DtStart = new CalDateTime(start), diff --git a/Ical.Net.Tests/SerializationTests.cs b/Ical.Net.Tests/SerializationTests.cs index c5b3f5937..3dae7c133 100644 --- a/Ical.Net.Tests/SerializationTests.cs +++ b/Ical.Net.Tests/SerializationTests.cs @@ -502,7 +502,7 @@ public void TestRRuleUntilSerialization() { var rrule = new RecurrencePattern(FrequencyType.Daily) { - Until = _nowTime.AddDays(7), + Until = new CalDateTime(_nowTime.AddDays(7)), }; const string someTz = "Europe/Volgograd"; var e = new CalendarEvent diff --git a/Ical.Net/DataTypes/CalDateTime.cs b/Ical.Net/DataTypes/CalDateTime.cs index 1dfd0ec09..10a891fd7 100644 --- a/Ical.Net/DataTypes/CalDateTime.cs +++ b/Ical.Net/DataTypes/CalDateTime.cs @@ -301,14 +301,6 @@ public override int GetHashCode() return !(left == right); } - /// - /// Creates a new instance of with for - /// - public static implicit operator CalDateTime(DateTime left) - { - return new CalDateTime(left); - } - /// /// Converts the date/time to UTC (Coordinated Universal Time) /// If == diff --git a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs index 60dce80fa..0a473e120 100644 --- a/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs +++ b/Ical.Net/Serialization/DataTypes/RecurrencePatternSerializer.cs @@ -426,10 +426,7 @@ private static void SerializeByValue(List aggregate, IList byValue, } else if ((match = RecurUntil.Match(item)).Success) { - var dt = DateTime.Parse(match.Groups["DateTime"].Value); - DateTime.SpecifyKind(dt, DateTimeKind.Utc); - - r.Until = dt; + r.Until = new CalDateTime(match.Groups["DateTime"].Value); } else if ((match = SpecificRecurrenceCount.Match(item)).Success) { From b942fbecb7e196d67f10da1c31b0128f9a8326eb Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Sun, 23 Feb 2025 15:43:36 +0100 Subject: [PATCH 4/6] Test: Reproduce incorrect handling of UNTIL if it falls into a DST change. Reproduces #736 --- Ical.Net.Tests/RecurrenceTests.cs | 46 +++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/Ical.Net.Tests/RecurrenceTests.cs b/Ical.Net.Tests/RecurrenceTests.cs index e750a79b3..675c16f5b 100644 --- a/Ical.Net.Tests/RecurrenceTests.cs +++ b/Ical.Net.Tests/RecurrenceTests.cs @@ -76,6 +76,52 @@ string[] timeZones EventOccurrenceTest(cal, fromDate, toDate, expectedPeriods, timeZones, 0); } + private static TestCaseData[] EventOccurrenceTestCases = new TestCaseData[] + { + new(""" + DTSTART;TZID=Europe/Amsterdam:20201024T023000 + DURATION:PT5M + RRULE:FREQ=DAILY;UNTIL=20201025T010000Z + """, + new [] + { + "20201024T023000/PT5M", + "20201025T023000/PT5M" + } + ), + }; + + [Test, Category("Recurrence")] + [TestCaseSource(nameof(EventOccurrenceTestCases))] + public void EventOccurrenceTest( + string eventIcal, + string[] expectedPeriods) + { + var eventSerializer = new EventSerializer(); + var calendarIcalStr = $""" + BEGIN:VCALENDAR + VERSION:2.0 + BEGIN:VEVENT + {eventIcal} + END:VEVENT + END:VCALENDAR + """; + + var cal = Calendar.Load(calendarIcalStr); + var tzid = cal.Events.Single().Start.TzId; + + var periodSerializer = new PeriodSerializer(); + var periods = expectedPeriods + .Select(p => (Period) periodSerializer.Deserialize(new StringReader(p))) + .Select(p => + p.Duration is null + ? new Period(p.StartTime.ToTimeZone(tzid), p.EndTime) + : new Period(p.StartTime.ToTimeZone(tzid), p.Duration.Value)) + .ToArray(); + + EventOccurrenceTest(cal, null, null, periods, null, 0); + } + /// /// See Page 45 of RFC 2445 - RRULE:FREQ=YEARLY;INTERVAL=2;BYMONTH=1;BYDAY=SU;BYHOUR=8,9;BYMINUTE=30 /// From 8eedb91f487e09b659ec702bee5eb71b0d8d3062 Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Sun, 23 Feb 2025 15:45:29 +0100 Subject: [PATCH 5/6] Fix #736: Check for `UNTIL` only *after* TZ conversion to avoid issues with DST changes. --- .../Evaluation/RecurrencePatternEvaluator.cs | 64 +++++-------------- 1 file changed, 15 insertions(+), 49 deletions(-) diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index fad1b3e89..04610524a 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -8,7 +8,6 @@ using System.Collections.Generic; using System.Linq; using Ical.Net.DataTypes; -using Ical.Net.Utility; namespace Ical.Net.Evaluation; @@ -28,12 +27,6 @@ private RecurrencePattern ProcessRecurrencePattern(CalDateTime referenceDate) var r = new RecurrencePattern(); r.CopyFrom(Pattern); - // Convert the UNTIL value to one that matches the same time information as the reference date - if (r.Until is not null) - { - r.Until = MatchTimeZone(referenceDate, r.Until); - } - if (referenceDate.HasTime) { if (r.Frequency > FrequencyType.Secondly && r.BySecond.Count == 0 && referenceDate.HasTime @@ -152,12 +145,19 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see { var expandBehavior = RecurrenceUtil.GetExpandBehaviorList(pattern); + // This value is only used for performance reasons to stop incrementing after + // until is passed, even if no recurrences are being found. + // As a safe heuristic we add 1d to the UNTIL value to cover any time shift and DST changes. + // It's just important that we don't miss any recurrences, not that we stop exactly at UNTIL. + // Precise UNTIL handling is done outside this method after TZ conversion. + var coarseUntil = pattern.Until?.Value.AddDays(1); + var noCandidateIncrementCount = 0; DateTime? candidate = null; var dateCount = 0; while (maxCount < 0 || dateCount < maxCount) { - if (pattern.Until is not null && candidate > pattern.Until) + if (seedCopy > coarseUntil) { break; } @@ -202,11 +202,10 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see continue; } - if (pattern.Until is null || candidate <= pattern.Until) - { - yield return candidate.Value; - dateCount++; - } + // UNTIL is applied outside of this method, after TZ conversion has been applied. + + yield return candidate.Value; + dateCount++; } } else @@ -903,42 +902,9 @@ public override IEnumerable Evaluate(CalDateTime referenceDate, CalDateT var periodQuery = GetDates(referenceDate, periodStart, periodEnd, -1, pattern, includeReferenceDateInResults) .Select(dt => CreatePeriod(dt, referenceDate)); - return periodQuery; - } + if (pattern.Until is not null) + periodQuery = periodQuery.TakeWhile(p => p.StartTime <= pattern.Until); - private static CalDateTime MatchTimeZone(CalDateTime reference, CalDateTime until) - { - /* - The value of the "UNTIL" rule part MUST have the same value type as the - "DTSTART" property. Furthermore, if the "DTSTART" property is - specified as a date with local time, then the UNTIL rule part MUST - also be specified as a date with local time. - - If the "DTSTART" property is specified as a date with UTC time or a date with local - time and time zone reference, then the UNTIL rule part MUST be - specified as a date with UTC time. - */ - string? untilTzId; - if (reference.IsFloating) - { - // If 'reference' is floating, then 'until' must be floating - untilTzId = null; - } - else - { - // If 'reference' has a timezone, 'until' MUST be UTC, - // but in case of UTC rule violation we fall back to the 'reference' timezone - untilTzId = until.TzId == CalDateTime.UtcTzId - ? CalDateTime.UtcTzId - : reference.TzId; - } - - var untilCalDt = new CalDateTime(until.Value, untilTzId, reference.HasTime); - - // If 'reference' is floating, then 'until' is floating, too - return reference.TzId is null - ? untilCalDt - // convert to the reference timezone and convert the value to Floating - : untilCalDt.ToTimeZone(reference.TzId).ToTimeZone(null); + return periodQuery; } } From 124cb51969b4e71282c7521d9586b1fe572973f5 Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Sun, 23 Feb 2025 16:14:03 +0100 Subject: [PATCH 6/6] RecurrencePatternEvaluator: Simplify inner inverval loop complexity. --- .../Evaluation/RecurrencePatternEvaluator.cs | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs index 04610524a..95ca9932a 100644 --- a/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs +++ b/Ical.Net/Evaluation/RecurrencePatternEvaluator.cs @@ -141,7 +141,7 @@ private IEnumerable GetDates(CalDateTime seed, CalDateTime? periodStar return EnumerateDates(originalDate, seedCopy, periodStartDt, periodEndDt, maxCount, pattern); } - private IEnumerable EnumerateDates(DateTime originalDate, DateTime seedCopy, DateTime? periodStart, DateTime? periodEnd, int maxCount, RecurrencePattern pattern) + private IEnumerable EnumerateDates(DateTime originalDate, DateTime intervalRefTime, DateTime? periodStart, DateTime? periodEnd, int maxCount, RecurrencePattern pattern) { var expandBehavior = RecurrenceUtil.GetExpandBehaviorList(pattern); @@ -153,16 +153,10 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see var coarseUntil = pattern.Until?.Value.AddDays(1); var noCandidateIncrementCount = 0; - DateTime? candidate = null; var dateCount = 0; while (maxCount < 0 || dateCount < maxCount) { - if (seedCopy > coarseUntil) - { - break; - } - - if (candidate > periodEnd) + if (intervalRefTime > coarseUntil) { break; } @@ -173,19 +167,19 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see } //No need to continue if the seed is after the periodEnd - if (seedCopy > periodEnd) + if (intervalRefTime > periodEnd) { break; } - var candidates = GetCandidates(seedCopy, pattern, expandBehavior); + var candidates = GetCandidates(intervalRefTime, pattern, expandBehavior); if (candidates.Count > 0) { noCandidateIncrementCount = 0; foreach (var t in candidates.Where(t => t >= originalDate)) { - candidate = t; + var candidate = t; // candidates MAY occur before periodStart // For example, FREQ=YEARLY;BYWEEKNO=1 could return dates @@ -204,7 +198,7 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see // UNTIL is applied outside of this method, after TZ conversion has been applied. - yield return candidate.Value; + yield return candidate; dateCount++; } } @@ -217,7 +211,7 @@ private IEnumerable EnumerateDates(DateTime originalDate, DateTime see } } - IncrementDate(ref seedCopy, pattern, pattern.Interval); + IncrementDate(ref intervalRefTime, pattern, pattern.Interval); } }