Skip to content

Commit 101a1e0

Browse files
axunonbaxunonb
authored
Fix: Calculation for occurrences include DST transitions correctly (#673)
* Fix: Calculation for occurrences include DST transitions correctly Updated `CalendarEvent` and `EventEvaluator` to enhance event handling, particularly around time zones and daylight saving time transitions. Key changes include: - Enabled nullable reference types in `CalendarEvent` and `EventEvaluator.` - Updated `CalendarEvent` to throw `InvalidOperationException` when trying to set both `DtEnd` and `Duration`. - Event end time and duration calculations in `EventEvaluator` reflect any DST transitions. - Added new test class `RecurrenceTests_From_Issues` to validate changes with various edge cases and scenarios. Resolves #660 Resolves #623 Resolves #671 Resolves #567 Resolves #342 Resolves #298 * Refactor and improve Period class and related usages `CalendarEvent` - Renamed `CalcFirstNominalDuration` to `GetTimeSpanToAddToPeriodStartTime`. The logic remains unchanged. `Period` - Removed private `ExtrapolateTimes()` method - Enabled nullable reference types - Refactored constructors and properties - Added `GetOriginalValues` method - Enhanced documentation and exception handling - Fixed bug in `CollidesWith(Period)` - Added PeriodTests.cs to cover unit tests for `Period` class. - Shortcoming: `Period.Duration` may still implicitly return a nominal or an exact duration. * Make `CalendarEvent.GetTimeSpanToAddToPeriodStartTime()` internal
1 parent 4c1de35 commit 101a1e0

File tree

11 files changed

+1016
-288
lines changed

11 files changed

+1016
-288
lines changed

‎Ical.Net.Tests/CalendarEventTest.cs‎

Lines changed: 77 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -427,34 +427,37 @@ public void EventResourcesCanBeZeroedOut()
427427
public void HourMinuteSecondOffsetParsingTest()
428428
{
429429
const string ical =
430-
@"BEGIN:VCALENDAR
431-
PRODID:-//1&1 Mail & Media GmbH/GMX Kalender Server 3.10.0//NONSGML//DE
432-
VERSION:2.0
433-
CALSCALE:GREGORIAN
434-
METHOD:REQUEST
435-
BEGIN:VTIMEZONE
436-
TZID:Europe/Brussels
437-
TZURL:http://tzurl.org/zoneinfo/Europe/Brussels
438-
X-LIC-LOCATION:Europe/Brussels
439-
BEGIN:DAYLIGHT
440-
TZOFFSETFROM:-001730
441-
TZOFFSETTO:-001730
442-
TZNAME:CEST
443-
DTSTART:19810329T020000
444-
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU
445-
END:DAYLIGHT
446-
BEGIN:STANDARD
447-
TZOFFSETFROM:+001730
448-
TZOFFSETTO:+001730
449-
TZNAME:BMT
450-
DTSTART:18800101T000000
451-
RDATE:18800101T000000
452-
END:STANDARD
453-
END:VTIMEZONE
454-
END:VCALENDAR";
430+
"""
431+
BEGIN:VCALENDAR
432+
PRODID:-//1&1 Mail & Media GmbH/GMX Kalender Server 3.10.0//NONSGML//DE
433+
VERSION:2.0
434+
CALSCALE:GREGORIAN
435+
METHOD:REQUEST
436+
BEGIN:VTIMEZONE
437+
TZID:Europe/Brussels
438+
TZURL:http://tzurl.org/zoneinfo/Europe/Brussels
439+
X-LIC-LOCATION:Europe/Brussels
440+
BEGIN:DAYLIGHT
441+
TZOFFSETFROM:-001730
442+
TZOFFSETTO:-001730
443+
TZNAME:CEST
444+
DTSTART:19810329T020000
445+
RRULE:FREQ=YEARLY;BYMONTH=3;BYDAY=-1SU
446+
END:DAYLIGHT
447+
BEGIN:STANDARD
448+
TZOFFSETFROM:+001730
449+
TZOFFSETTO:+001730
450+
TZNAME:BMT
451+
DTSTART:18800101T000000
452+
RDATE:18800101T000000
453+
END:STANDARD
454+
END:VTIMEZONE
455+
END:VCALENDAR
456+
""";
455457
var timezones = Calendar.Load(ical)
456458
.TimeZones.First()
457-
.Children.Cast<CalendarComponent>();
459+
.Children.Cast<CalendarComponent>()
460+
.ToArray();
458461

459462
var positiveOffset = timezones
460463
.Skip(1).Take(1).First()
@@ -472,78 +475,99 @@ public void HourMinuteSecondOffsetParsingTest()
472475

473476

474477
[Test, Category("CalendarEvent")]
475-
public void TestGetEffectiveDuration()
478+
public void GetNominalDurationTests()
476479
{
477-
var now = _now.Subtract(TimeSpan.FromTicks(_now.Ticks % TimeSpan.TicksPerSecond));
480+
var dt = new DateTime(2025, 3, 1, 14, 30, 0);
481+
const string tzIdStart = "America/New_York";
482+
const string tzIdEnd = "Europe/London";
478483

479-
var evt = new CalendarEvent()
484+
var evt = new CalendarEvent
480485
{
481-
DtStart = new CalDateTime(DateOnly.FromDateTime(now), TimeOnly.FromDateTime(now)),
482-
DtEnd = new CalDateTime(DateOnly.FromDateTime(now.AddHours(1)), TimeOnly.FromDateTime(now.AddHours(1)))
486+
DtStart = new CalDateTime(DateOnly.FromDateTime(dt), TimeOnly.FromDateTime(dt), tzIdStart),
487+
DtEnd = new CalDateTime(DateOnly.FromDateTime(dt.AddHours(1)), TimeOnly.FromDateTime(dt.AddHours(1)), tzIdEnd)
483488
};
484489

485490
Assert.Multiple(() =>
486491
{
487-
Assert.That(evt.DtStart.Value, Is.EqualTo(now));
488-
Assert.That(evt.DtEnd.Value, Is.EqualTo(now.AddHours(1)));
489-
Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(1)));
492+
Assert.That(evt.DtStart.Value, Is.EqualTo(dt));
493+
Assert.That(evt.DtEnd.Value, Is.EqualTo(dt.AddHours(1)));
494+
Assert.That(evt.GetTimeSpanToAddToPeriodStartTime(), Is.EqualTo(TimeSpan.FromHours(1)));
490495
});
491496

492-
evt = new CalendarEvent()
497+
evt = new CalendarEvent
493498
{
494-
DtStart = new CalDateTime(DateOnly.FromDateTime(now.Date), TimeOnly.FromDateTime(now.Date)),
495-
DtEnd = new CalDateTime(DateOnly.FromDateTime(now.Date.AddHours(1)), TimeOnly.FromDateTime(now.Date.AddHours(1)))
499+
DtStart = new CalDateTime(DateOnly.FromDateTime(dt.Date)),
500+
DtEnd = new CalDateTime(DateOnly.FromDateTime(dt.Date))
496501
};
497502

498503
Assert.Multiple(() =>
499504
{
500-
Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date));
501-
Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(1)));
505+
Assert.That(evt.DtStart.Value, Is.EqualTo(dt.Date));
506+
Assert.That(evt.GetTimeSpanToAddToPeriodStartTime(), Is.EqualTo(TimeSpan.Zero));
502507
});
503508

504-
evt = new CalendarEvent()
509+
evt = new CalendarEvent
505510
{
506-
DtStart = new CalDateTime(DateOnly.FromDateTime(now)),
511+
DtStart = new CalDateTime(DateOnly.FromDateTime(dt)),
507512
};
508513

509514
Assert.Multiple(() =>
510515
{
511-
Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date));
512-
Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromDays(1)));
516+
Assert.That(evt.DtStart.Value, Is.EqualTo(dt.Date));
517+
Assert.That(evt.Duration, Is.Null);
518+
Assert.That(evt.GetTimeSpanToAddToPeriodStartTime(), Is.EqualTo(TimeSpan.FromDays(1)));
513519
});
514520

515-
evt = new CalendarEvent()
521+
evt = new CalendarEvent
516522
{
517-
DtStart = new CalDateTime(DateOnly.FromDateTime(now), TimeOnly.FromDateTime(now)),
523+
DtStart = new CalDateTime(DateOnly.FromDateTime(dt), TimeOnly.FromDateTime(dt)),
518524
Duration = TimeSpan.FromHours(2),
519525
};
520526

521527
Assert.Multiple(() => {
522-
Assert.That(evt.DtStart.Value, Is.EqualTo(now));
528+
Assert.That(evt.DtStart.Value, Is.EqualTo(dt));
523529
Assert.That(evt.DtEnd, Is.Null);
524-
Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(2)));
530+
Assert.That(evt.GetTimeSpanToAddToPeriodStartTime(), Is.EqualTo(TimeSpan.FromHours(2)));
525531
});
526532

527533
evt = new CalendarEvent()
528534
{
529-
DtStart = new CalDateTime(DateOnly.FromDateTime(now.Date), TimeOnly.FromDateTime(now.Date)),
535+
DtStart = new CalDateTime(DateOnly.FromDateTime(dt.Date), TimeOnly.FromDateTime(dt.Date)),
530536
Duration = TimeSpan.FromHours(2),
531537
};
532538

533539
Assert.Multiple(() => {
534-
Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date));
535-
Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(2)));
540+
Assert.That(evt.DtStart.Value, Is.EqualTo(dt.Date));
541+
Assert.That(evt.GetTimeSpanToAddToPeriodStartTime(), Is.EqualTo(TimeSpan.FromHours(2)));
536542
});
537543

538544
evt = new CalendarEvent()
539545
{
540-
DtStart = new CalDateTime(DateOnly.FromDateTime(now)),
546+
DtStart = new CalDateTime(DateOnly.FromDateTime(dt)),
541547
Duration = TimeSpan.FromDays(1),
542548
};
543549

544550
Assert.Multiple(() => {
545-
Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date));
546-
Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromDays(1)));
551+
Assert.That(evt.DtStart.Value, Is.EqualTo(dt.Date));
552+
Assert.That(evt.GetTimeSpanToAddToPeriodStartTime(), Is.EqualTo(TimeSpan.FromDays(1)));
553+
});
554+
}
555+
556+
[Test]
557+
public void EitherEndTime_OrDuraction_CanBeSet()
558+
{
559+
var evt = new CalendarEvent()
560+
{
561+
DtStart = new CalDateTime(2025, 10, 11, 12, 13, 14, CalDateTime.UtcTzId)
562+
};
563+
564+
Assert.Multiple(() =>
565+
{
566+
Assert.That(() => evt.DtEnd = new CalDateTime(2025, 12, 11), Throws.Nothing);
567+
Assert.That(() => evt.Duration = TimeSpan.FromDays(1), Throws.InvalidOperationException);
568+
Assert.That(() => evt.DtEnd = null, Throws.Nothing);
569+
Assert.That(() => evt.Duration = TimeSpan.FromDays(1), Throws.Nothing);
570+
Assert.That(() => evt.DtEnd = new CalDateTime(2025, 12, 11), Throws.InvalidOperationException);
547571
});
548572
}
549573
}

‎Ical.Net.Tests/CollectionHelpersTests.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ public void ExDateTests()
2424
{
2525
Assert.Multiple(() =>
2626
{
27-
Assert.That(GetExceptionDates(), Is.EqualTo(GetExceptionDates()));
2827
Assert.That(GetExceptionDates(), Is.Not.Null);
2928
Assert.That(GetExceptionDates(), Is.Not.EqualTo(null));
3029
});

‎Ical.Net.Tests/CopyComponentTests.cs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ public static IEnumerable CopyCalendarTest_TestCases()
6060
{
6161
DtStart = new CalDateTime(_now),
6262
DtEnd = new CalDateTime(_later),
63-
Duration = TimeSpan.FromHours(1),
6463
};
6564

6665
private static string SerializeEvent(CalendarEvent e) => new CalendarSerializer().SerializeToString(new Calendar { Events = { e } });
@@ -199,4 +198,4 @@ public void CopyJournalTest()
199198
Assert.That(copy.Status, Is.EqualTo(orig.Status));
200199
});
201200
}
202-
}
201+
}

‎Ical.Net.Tests/EqualityAndHashingTests.cs‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ public void Event_Tests(CalendarEvent incoming, CalendarEvent expected)
7979
{
8080
DtStart = new CalDateTime(_nowTime),
8181
DtEnd = new CalDateTime(_later),
82-
Duration = TimeSpan.FromHours(1),
8382
};
8483

8584
private static string SerializeEvent(CalendarEvent e) => new CalendarSerializer().SerializeToString(new Calendar { Events = { e } });
@@ -112,7 +111,6 @@ public void Calendar_Tests()
112111
var e = new CalendarEvent
113112
{
114113
DtStart = new CalDateTime(_nowTime),
115-
DtEnd = new CalDateTime(_later),
116114
Duration = TimeSpan.FromHours(1),
117115
RecurrenceRules = new List<RecurrencePattern> { rruleA },
118116
};
@@ -130,7 +128,6 @@ public void Calendar_Tests()
130128
expectedCalendar.Events.Add(new CalendarEvent
131129
{
132130
DtStart = new CalDateTime(_nowTime),
133-
DtEnd = new CalDateTime(_later),
134131
Duration = TimeSpan.FromHours(1),
135132
RecurrenceRules = new List<RecurrencePattern> { rruleB },
136133
});
@@ -501,4 +498,4 @@ public void CalDateTimeComparisonOperatorTests()
501498
TestComparison((dt1, dt2) => dt1 < dt2, (i1, i2) => i1 < i2);
502499
TestComparison((dt1, dt2) => dt1 <= dt2, (i1, i2) => i1 <= i2);
503500
}
504-
}
501+
}

‎Ical.Net.Tests/PeriodTests.cs‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
//
2+
// Copyright ical.net project maintainers and contributors.
3+
// Licensed under the MIT license.
4+
//
5+
6+
#nullable enable
7+
using System;
8+
using Ical.Net.DataTypes;
9+
using NUnit.Framework;
10+
11+
namespace Ical.Net.Tests;
12+
13+
[TestFixture]
14+
public class PeriodTests
15+
{
16+
[Test]
17+
public void CreatePeriodWithArguments()
18+
{
19+
var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"));
20+
var periodWithEndTime = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York"));
21+
var periodWithDuration = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new TimeSpan(1, 0, 0));
22+
23+
Assert.Multiple(() =>
24+
{
25+
Assert.That(period.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
26+
Assert.That(period.EndTime, Is.Null);
27+
Assert.That(period.Duration, Is.Null);
28+
29+
Assert.That(periodWithEndTime.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
30+
Assert.That(periodWithEndTime.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York")));
31+
Assert.That(periodWithEndTime.Duration, Is.EqualTo(new TimeSpan(1, 0, 0)));
32+
33+
Assert.That(periodWithDuration.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
34+
Assert.That(periodWithDuration.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York")));
35+
Assert.That(periodWithDuration.Duration, Is.EqualTo(new TimeSpan(1, 0, 0)));
36+
});
37+
}
38+
39+
[Test]
40+
public void CreatePeriodWithInvalidArgumentsShouldThrow()
41+
{
42+
Assert.Throws<ArgumentException>(() => _ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"),
43+
new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")));
44+
Assert.Throws<ArgumentException>(() =>
45+
_ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new TimeSpan(-1, 0, 0)));
46+
}
47+
[Test]
48+
public void SetAndGetStartTime()
49+
{
50+
var period = new Period(new CalDateTime(DateTime.UtcNow));
51+
var startTime = new CalDateTime(2025, 1, 1, 0, 0, 0);
52+
period.StartTime = startTime;
53+
54+
Assert.That(period.StartTime, Is.EqualTo(startTime));
55+
}
56+
57+
[Test]
58+
public void SetEndTime_GetDuration()
59+
{
60+
var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0));
61+
var endTime = new CalDateTime(2025, 1, 31, 0, 0, 0);
62+
period.EndTime = endTime;
63+
64+
Assert.That(period.EndTime, Is.EqualTo(endTime));
65+
Assert.That(period.GetOriginalValues().EndTime, Is.EqualTo(endTime));
66+
Assert.That(period.GetOriginalValues().Duration, Is.Null);
67+
Assert.That(period.Duration, Is.EqualTo(new TimeSpan(30, 0, 0, 0)));
68+
}
69+
70+
[Test]
71+
public void SetDuration_GetEndTime()
72+
{
73+
var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0));
74+
var duration = new TimeSpan(1, 0, 0);
75+
period.Duration = duration;
76+
77+
Assert.That(period.Duration, Is.EqualTo(duration));
78+
Assert.That(period.GetOriginalValues().Duration, Is.EqualTo(duration));
79+
Assert.That(period.GetOriginalValues().EndTime, Is.Null);
80+
Assert.That(period.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0)));
81+
}
82+
83+
[Test]
84+
public void CollidesWithPeriod()
85+
{
86+
var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), new TimeSpan(1, 0, 0));
87+
var period2 = new Period(new CalDateTime(2025, 1, 1, 0, 30, 0), new TimeSpan(1, 0, 0));
88+
var period3 = new Period(new CalDateTime(2025, 1, 1, 1, 30, 0), new TimeSpan(1, 0, 0));
89+
90+
Assert.Multiple(() =>
91+
{
92+
Assert.That(period1.CollidesWith(period2), Is.True);
93+
Assert.That(period1.CollidesWith(period3), Is.False);
94+
Assert.That(period2.CollidesWith(period3), Is.True);
95+
});
96+
}
97+
}

0 commit comments

Comments
 (0)