From 6779452c7264f01e3541fd6ca729ac17ff423c55 Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Mon, 14 Oct 2024 15:52:36 +0200 Subject: [PATCH] CalendarEvent: Don't set `DURATION` resp. `DTEND` from each other. Only calculate the duration on the fly when needed. This way we don't overwrite the information, which of both was originally set and can adjust calculations accordingly (in the future). See https://github.com/ical-org/ical.net/issues/574. --- Ical.Net.Tests/CalendarEventTest.cs | 79 +++++++++++++++- Ical.Net.Tests/DeserializationTests.cs | 24 ++++- Ical.Net.Tests/SymmetricSerializationTests.cs | 29 +++++- Ical.Net/CalendarComponents/CalendarEvent.cs | 93 ++++++++++--------- Ical.Net/Evaluation/EventEvaluator.cs | 8 +- 5 files changed, 179 insertions(+), 54 deletions(-) diff --git a/Ical.Net.Tests/CalendarEventTest.cs b/Ical.Net.Tests/CalendarEventTest.cs index a0e96060..de1522b0 100644 --- a/Ical.Net.Tests/CalendarEventTest.cs +++ b/Ical.Net.Tests/CalendarEventTest.cs @@ -469,4 +469,81 @@ public void HourMinuteSecondOffsetParsingTest() var expectedNegative = TimeSpan.FromMinutes(-17.5); Assert.That(negativeOffset?.Offset, Is.EqualTo(expectedNegative)); } -} \ No newline at end of file + + + [Test, Category("CalendarEvent")] + public void TestGetEffectiveDuration() + { + var now = _now.Subtract(TimeSpan.FromTicks(_now.Ticks % TimeSpan.TicksPerSecond)); + + var evt = new CalendarEvent() + { + DtStart = new CalDateTime(now) { HasTime = true }, + DtEnd = new CalDateTime(now.AddHours(1)) { HasTime = true }, + }; + + Assert.Multiple(() => + { + Assert.That(evt.DtStart.Value, Is.EqualTo(now)); + Assert.That(evt.DtEnd.Value, Is.EqualTo(now.AddHours(1))); + Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(1))); + }); + + evt = new CalendarEvent() + { + DtStart = new CalDateTime(now.Date) { HasTime = true }, + DtEnd = new CalDateTime(now.Date.AddHours(1)) { HasTime = true }, + }; + + Assert.Multiple(() => + { + Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date)); + Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(1))); + }); + + evt = new CalendarEvent() + { + DtStart = new CalDateTime(now.Date) { HasTime = false }, + }; + + Assert.Multiple(() => + { + Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date)); + Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromDays(1))); + }); + + evt = new CalendarEvent() + { + DtStart = new CalDateTime(now) { HasTime = true }, + Duration = TimeSpan.FromHours(2), + }; + + Assert.Multiple(() => { + Assert.That(evt.DtStart.Value, Is.EqualTo(now)); + Assert.That(evt.DtEnd, Is.Null); + Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(2))); + }); + + evt = new CalendarEvent() + { + DtStart = new CalDateTime(now.Date) { HasTime = true }, + Duration = TimeSpan.FromHours(2), + }; + + Assert.Multiple(() => { + Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date)); + Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromHours(2))); + }); + + evt = new CalendarEvent() + { + DtStart = new CalDateTime(now.Date) { HasTime = false }, + Duration = TimeSpan.FromDays(1), + }; + + Assert.Multiple(() => { + Assert.That(evt.DtStart.Value, Is.EqualTo(now.Date)); + Assert.That(evt.GetFirstDuration(), Is.EqualTo(TimeSpan.FromDays(1))); + }); + } +} diff --git a/Ical.Net.Tests/DeserializationTests.cs b/Ical.Net.Tests/DeserializationTests.cs index 57ca53aa..2c45739e 100644 --- a/Ical.Net.Tests/DeserializationTests.cs +++ b/Ical.Net.Tests/DeserializationTests.cs @@ -552,4 +552,26 @@ public void Property1() Assert.That(props[i].Value, Is.EqualTo("2." + i)); } } -} \ No newline at end of file + + [Test] + [TestCase(true)] + [TestCase(false)] + public void KeepApartDtEndAndDuration_Tests(bool useDtEnd) + { + var calStr = $@"BEGIN:VCALENDAR +BEGIN:VEVENT +DTSTART:20070406T230000Z +{(useDtEnd ? "DTEND:20070407T010000Z" : "DURATION:PT1H")} +END:VEVENT +END:VCALENDAR +"; + + var calendar = Calendar.Load(calStr); + + Assert.Multiple(() => + { + Assert.That(calendar.Events.Single().DtEnd != null, Is.EqualTo(useDtEnd)); + Assert.That(calendar.Events.Single().Duration != default, Is.EqualTo(!useDtEnd)); + }); + } +} diff --git a/Ical.Net.Tests/SymmetricSerializationTests.cs b/Ical.Net.Tests/SymmetricSerializationTests.cs index 66b83aad..ffaf6ac4 100644 --- a/Ical.Net.Tests/SymmetricSerializationTests.cs +++ b/Ical.Net.Tests/SymmetricSerializationTests.cs @@ -25,7 +25,17 @@ public class SymmetricSerializationTests private static readonly DateTime _later = _nowTime.AddHours(1); private static CalendarSerializer GetNewSerializer() => new CalendarSerializer(); private static string SerializeToString(Calendar c) => GetNewSerializer().SerializeToString(c); - private static CalendarEvent GetSimpleEvent() => new CalendarEvent { DtStart = new CalDateTime(_nowTime), DtEnd = new CalDateTime(_later) }; + private static CalendarEvent GetSimpleEvent(bool useDtEnd = true) + { + var evt = new CalendarEvent { DtStart = new CalDateTime(_nowTime) }; + if (useDtEnd) + evt.DtEnd = new CalDateTime(_later); + else + evt.Duration = _later - _nowTime; + + return evt; + } + private static Calendar UnserializeCalendar(string s) => Calendar.Load(s); [Test, TestCaseSource(nameof(Event_TestCases))] @@ -47,24 +57,33 @@ public void Event_Tests(Calendar iCalendar) } public static IEnumerable Event_TestCases() + { + return Event_TestCasesInt(true).Concat(Event_TestCasesInt(false)); + } + + private static IEnumerable Event_TestCasesInt(bool useDtEnd) { var rrule = new RecurrencePattern(FrequencyType.Daily, 1) { Count = 5 }; var e = new CalendarEvent { DtStart = new CalDateTime(_nowTime), - DtEnd = new CalDateTime(_later), RecurrenceRules = new List { rrule }, }; + if (useDtEnd) + e.DtEnd = new CalDateTime(_later); + else + e.Duration = _later - _nowTime; + var calendar = new Calendar(); calendar.Events.Add(e); - yield return new TestCaseData(calendar).SetName("readme.md example"); + yield return new TestCaseData(calendar).SetName($"readme.md example with {(useDtEnd ? "DTEND" : "DURATION")}"); - e = GetSimpleEvent(); + e = GetSimpleEvent(useDtEnd); e.Description = "This is an event description that is really rather long. Hopefully the line breaks work now, and it's serialized properly."; calendar = new Calendar(); calendar.Events.Add(e); - yield return new TestCaseData(calendar).SetName("Description serialization isn't working properly. Issue #60"); + yield return new TestCaseData(calendar).SetName($"Description serialization isn't working properly. Issue #60 {(useDtEnd ? "DTEND" : "DURATION")}"); } [Test] diff --git a/Ical.Net/CalendarComponents/CalendarEvent.cs b/Ical.Net/CalendarComponents/CalendarEvent.cs index 78be3710..ac7fc040 100644 --- a/Ical.Net/CalendarComponents/CalendarEvent.cs +++ b/Ical.Net/CalendarComponents/CalendarEvent.cs @@ -31,27 +31,6 @@ public class CalendarEvent : RecurringComponent, IAlarmContainer, IComparable - /// The start date/time of the event. - /// - /// If the duration has not been set, but - /// the start/end time of the event is available, - /// the duration is automatically determined. - /// Likewise, if the end date/time has not been - /// set, but a start and duration are available, - /// the end date/time will be extrapolated. - /// - /// - public override IDateTime DtStart - { - get => base.DtStart; - set - { - base.DtStart = value; - ExtrapolateTimes(2); - } - } - /// /// The end date/time of the event. /// @@ -71,7 +50,6 @@ public virtual IDateTime DtEnd if (!Equals(DtEnd, value)) { Properties.Set("DTEND", value); - ExtrapolateTimes(0); } } } @@ -105,11 +83,58 @@ public virtual TimeSpan Duration if (!Equals(Duration, value)) { Properties.Set("DURATION", value); - ExtrapolateTimes(1); } } } + /// + /// Calculates and returns the duration of the first occurrence of this event. + /// + /// + /// If the 'DURATION' property is set, this method will return its value. + /// Otherwise, if DTSTART and DTEND are set, it will return DTSTART minus DTEND. + /// Otherwise it will return `default(TimeSpan)`. + /// Note that for recurring events, the duration of individual occurrences may vary + /// if they span a DST change. + /// + /// The effective duration of this event. + public virtual TimeSpan GetFirstDuration() + { + if (Properties.ContainsKey("DURATION")) + return Duration; + + if (DtStart is not null) + { + if (DtEnd is not null) + { + // The "DTEND" property + // for a "VEVENT" calendar component specifies the non-inclusive end + // of the event. + return DtEnd.Subtract(DtStart); + } + else if (!DtStart.HasTime) + { + // For cases where a "VEVENT" calendar component + // specifies a "DTSTART" property with a DATE value type but no + // "DTEND" nor "DURATION" property, the event's duration is taken to + // be one day. + return TimeSpan.FromDays(1); + } + else + { + // For cases where a "VEVENT" calendar component + // specifies a "DTSTART" property with a DATE-TIME value type but no + // "DTEND" property, the event ends on the same calendar date and + // time of day specified by the "DTSTART" property. + return TimeSpan.Zero; + } + } + + // This is an illegal state. We return zero for compatibility reasons. + return TimeSpan.Zero; + } + + /// /// An alias to the DtEnd field (i.e. end date/time). /// @@ -264,26 +289,6 @@ protected override void OnDeserializing(StreamingContext context) protected override void OnDeserialized(StreamingContext context) { base.OnDeserialized(context); - - ExtrapolateTimes(-1); - } - - private void ExtrapolateTimes(int source) - { - /* - * Source values, a fix introduced to prevent stack overflow exceptions from occuring. - * -1 = Anybody, stack overflow could maybe still occur in this case? - * 0 = End - * 1 = Duration - */ - if (DtEnd == null && DtStart != null && Duration != default(TimeSpan) && source != 0) - { - DtEnd = DtStart.Add(Duration); - } - else if (Duration == default(TimeSpan) && DtStart != null && DtEnd != null && source != 1) - { - Duration = DtEnd.Subtract(DtStart); - } } protected bool Equals(CalendarEvent other) @@ -296,6 +301,7 @@ protected bool Equals(CalendarEvent other) && string.Equals(Summary, other.Summary, StringComparison.OrdinalIgnoreCase) && string.Equals(Description, other.Description, StringComparison.OrdinalIgnoreCase) && Equals(DtEnd, other.DtEnd) + && Equals(Duration, other.Duration) && string.Equals(Location, other.Location, StringComparison.OrdinalIgnoreCase) && resourcesSet.SetEquals(other.Resources) && string.Equals(Status, other.Status, StringComparison.Ordinal) @@ -356,6 +362,7 @@ public override int GetHashCode() hashCode = (hashCode * 397) ^ (Summary?.GetHashCode() ?? 0); hashCode = (hashCode * 397) ^ (Description?.GetHashCode() ?? 0); hashCode = (hashCode * 397) ^ (DtEnd?.GetHashCode() ?? 0); + hashCode = (hashCode * 397) ^ Duration.GetHashCode(); hashCode = (hashCode * 397) ^ (Location?.GetHashCode() ?? 0); hashCode = (hashCode * 397) ^ Status?.GetHashCode() ?? 0; hashCode = (hashCode * 397) ^ IsActive.GetHashCode(); diff --git a/Ical.Net/Evaluation/EventEvaluator.cs b/Ical.Net/Evaluation/EventEvaluator.cs index d0e07c98..35050210 100644 --- a/Ical.Net/Evaluation/EventEvaluator.cs +++ b/Ical.Net/Evaluation/EventEvaluator.cs @@ -45,19 +45,19 @@ public override HashSet Evaluate(IDateTime referenceTime, DateTime perio foreach (var period in Periods) { - period.Duration = CalendarEvent.Duration; + period.Duration = CalendarEvent.GetFirstDuration(); period.EndTime = period.Duration == default ? period.StartTime - : period.StartTime.Add(CalendarEvent.Duration); + : period.StartTime.Add(CalendarEvent.GetFirstDuration()); } // Ensure each period has a duration foreach (var period in Periods.Where(p => p.EndTime == null)) { - period.Duration = CalendarEvent.Duration; + period.Duration = CalendarEvent.GetFirstDuration(); period.EndTime = period.Duration == default ? period.StartTime - : period.StartTime.Add(CalendarEvent.Duration); + : period.StartTime.Add(CalendarEvent.GetFirstDuration()); } return Periods;