From 19e98d783eb9d02f5272cef78a6ff4bbcc48127a Mon Sep 17 00:00:00 2001 From: Markus Minichmayr Date: Thu, 26 Dec 2024 23:03:37 +0100 Subject: [PATCH] Change `Period.Duration` from `TimeSpan` to `Duration`. Remove extrapolation and introduce `GetEffectiveDuration()`, `GetEffectiveEndTime()`. --- Ical.Net.Tests/CopyComponentTests.cs | 2 +- Ical.Net.Tests/PeriodTests.cs | 32 +++---- Ical.Net/CalendarComponents/Alarm.cs | 9 +- Ical.Net/DataTypes/Period.cs | 93 ++++++++----------- Ical.Net/Evaluation/EventEvaluator.cs | 3 +- .../DataTypes/PeriodSerializer.cs | 2 +- 6 files changed, 62 insertions(+), 79 deletions(-) diff --git a/Ical.Net.Tests/CopyComponentTests.cs b/Ical.Net.Tests/CopyComponentTests.cs index 12ae3e71..24d77f30 100644 --- a/Ical.Net.Tests/CopyComponentTests.cs +++ b/Ical.Net.Tests/CopyComponentTests.cs @@ -110,7 +110,7 @@ public void CopyFreeBusyTest() { Start = new CalDateTime(_now), End = new CalDateTime(_later), - Entries = { new FreeBusyEntry { Language = "English", StartTime = new CalDateTime(2024, 10, 1), Duration = TimeSpan.FromDays(1), Status = FreeBusyStatus.Busy } } + Entries = { new FreeBusyEntry { Language = "English", StartTime = new CalDateTime(2024, 10, 1), Duration = Duration.FromDays(1), Status = FreeBusyStatus.Busy } } }; var copy = orig.Copy(); diff --git a/Ical.Net.Tests/PeriodTests.cs b/Ical.Net.Tests/PeriodTests.cs index 04d54d44..3ae3021d 100644 --- a/Ical.Net.Tests/PeriodTests.cs +++ b/Ical.Net.Tests/PeriodTests.cs @@ -18,7 +18,7 @@ public void CreatePeriodWithArguments() { var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York")); 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")); - var periodWithDuration = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new TimeSpan(1, 0, 0)); + var periodWithDuration = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), Duration.FromHours(1)); Assert.Multiple(() => { @@ -27,12 +27,12 @@ public void CreatePeriodWithArguments() Assert.That(period.Duration, Is.Null); Assert.That(periodWithEndTime.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"))); - Assert.That(periodWithEndTime.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York"))); - Assert.That(periodWithEndTime.Duration, Is.EqualTo(new TimeSpan(1, 0, 0))); + Assert.That(periodWithEndTime.GetEffectiveEndTime(), Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York"))); + Assert.That(periodWithEndTime.GetEffectiveDuration(), Is.EqualTo(Duration.FromHours(1))); Assert.That(periodWithDuration.StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"))); - Assert.That(periodWithDuration.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York"))); - Assert.That(periodWithDuration.Duration, Is.EqualTo(new TimeSpan(1, 0, 0))); + Assert.That(periodWithDuration.GetEffectiveEndTime(), Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "America/New_York"))); + Assert.That(periodWithDuration.GetEffectiveDuration(), Is.EqualTo(Duration.FromHours(1))); }); } @@ -42,7 +42,7 @@ public void CreatePeriodWithInvalidArgumentsShouldThrow() Assert.Throws(() => _ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"))); Assert.Throws(() => - _ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), new TimeSpan(-1, 0, 0))); + _ = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0, "America/New_York"), Duration.FromHours(-1))); } [Test] public void SetAndGetStartTime() @@ -61,31 +61,31 @@ public void SetEndTime_GetDuration() var endTime = new CalDateTime(2025, 1, 31, 0, 0, 0); period.EndTime = endTime; + Assert.That(period.GetEffectiveEndTime(), Is.EqualTo(endTime)); Assert.That(period.EndTime, Is.EqualTo(endTime)); - Assert.That(period.GetOriginalValues().EndTime, Is.EqualTo(endTime)); - Assert.That(period.GetOriginalValues().Duration, Is.Null); - Assert.That(period.Duration, Is.EqualTo(new TimeSpan(30, 0, 0, 0))); + Assert.That(period.Duration, Is.Null); + Assert.That(period.GetEffectiveDuration(), Is.EqualTo(Duration.FromDays(30))); } [Test] public void SetDuration_GetEndTime() { var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0)); - var duration = new TimeSpan(1, 0, 0); + var duration = Duration.FromHours(1); period.Duration = duration; + Assert.That(period.GetEffectiveDuration(), Is.EqualTo(duration)); Assert.That(period.Duration, Is.EqualTo(duration)); - Assert.That(period.GetOriginalValues().Duration, Is.EqualTo(duration)); - Assert.That(period.GetOriginalValues().EndTime, Is.Null); - Assert.That(period.EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0))); + Assert.That(period.EndTime, Is.Null); + Assert.That(period.GetEffectiveEndTime(), Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0))); } [Test] public void CollidesWithPeriod() { - var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), new TimeSpan(1, 0, 0)); - var period2 = new Period(new CalDateTime(2025, 1, 1, 0, 30, 0), new TimeSpan(1, 0, 0)); - var period3 = new Period(new CalDateTime(2025, 1, 1, 1, 30, 0), new TimeSpan(1, 0, 0)); + var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), Duration.FromHours(1)); + var period2 = new Period(new CalDateTime(2025, 1, 1, 0, 30, 0), Duration.FromHours(1)); + var period3 = new Period(new CalDateTime(2025, 1, 1, 1, 30, 0), Duration.FromHours(1)); Assert.Multiple(() => { diff --git a/Ical.Net/CalendarComponents/Alarm.cs b/Ical.Net/CalendarComponents/Alarm.cs index 625512c2..531e1747 100644 --- a/Ical.Net/CalendarComponents/Alarm.cs +++ b/Ical.Net/CalendarComponents/Alarm.cs @@ -93,7 +93,7 @@ public virtual IList GetOccurrences(IRecurringComponent rc, IDa fromDate = rc.Start.Copy(); } - var d = default(TimeSpan); + Duration? d = null; foreach (var o in rc.GetOccurrences(fromDate, toDate)) { var dt = o.Period.StartTime; @@ -102,16 +102,15 @@ public virtual IList GetOccurrences(IRecurringComponent rc, IDa if (o.Period.EndTime != null) { dt = o.Period.EndTime; - if (d == default) + if (d == null) { d = o.Period.Duration!.Value; // the getter always returns a value } } // Use the "last-found" duration as a reference point - else if (d != default(TimeSpan)) + else if (d != null) { - // TODO: Nominal or exact? - dt = o.Period.StartTime.Add(d.ToDuration()); + dt = o.Period.StartTime.Add(d.Value); } else { diff --git a/Ical.Net/DataTypes/Period.cs b/Ical.Net/DataTypes/Period.cs index 5ec965bd..eab9b967 100644 --- a/Ical.Net/DataTypes/Period.cs +++ b/Ical.Net/DataTypes/Period.cs @@ -36,7 +36,7 @@ public Period(IDateTime start, IDateTime? end = null) } StartTime = start ?? throw new ArgumentNullException(nameof(start)); - _endTime = end; + EndTime = end; } /// @@ -46,15 +46,15 @@ public Period(IDateTime start, IDateTime? end = null) /// /// /// - public Period(IDateTime start, TimeSpan duration) + public Period(IDateTime start, Duration duration) { - if (duration < TimeSpan.Zero) + if (duration.IsNegative) { throw new ArgumentException("Duration must be greater than or equal to zero.", nameof(duration)); } StartTime = start; - _duration = duration; + Duration = duration; } /// @@ -65,8 +65,8 @@ public override void CopyFrom(ICopyable obj) if (obj is not Period p) return; StartTime = p.StartTime.Copy(); - _endTime = p._endTime?.Copy(); - _duration = p._duration; + EndTime = p.EndTime?.Copy(); + Duration = p.Duration; } protected bool Equals(Period other) => Equals(StartTime, other.StartTime) && Equals(EndTime, other.EndTime) && Duration.Equals(other.Duration); @@ -103,8 +103,6 @@ public override string ToString() /// public virtual IDateTime StartTime { get; set; } = null!; - private IDateTime? _endTime; - /// /// Gets either the end time of the period that was set, /// or calculates the exact end time based on the nominal duration. @@ -113,20 +111,7 @@ public override string ToString() /// Either the or the can be set at a time. /// The last one set will be stored, and the other will be calculated. /// - public virtual IDateTime? EndTime - { - get => _endTime ?? (_duration != null ? StartTime.Add(_duration.Value.ToDuration()) : null); - set - { - _endTime = value; - if (_endTime != null) - { - _duration = null; - } - } - } - - private TimeSpan? _duration; + public virtual IDateTime? EndTime { get; set; } /// /// Gets either the nominal duration of the period that was set, @@ -139,34 +124,38 @@ public virtual IDateTime? EndTime /// A that has a date-only , no /// and no duration set, is considered to last for one day. /// - public virtual TimeSpan? Duration - { - get - { - if (_endTime == null && !StartTime.HasTime) - { - return TimeSpan.FromDays(1); - } - return _duration ?? _endTime?.Subtract(StartTime).ToTimeSpan(); - } - set - { - _duration = value; - if (_duration != null) - { - _endTime = null; - } - } - } + public virtual Duration? Duration { get; set; } /// /// Gets the original start time, end time, and duration as they were set, /// while or properties may be calculated. /// /// - public (IDateTime StartTime, IDateTime? EndTime, TimeSpan? Duration) GetOriginalValues() + public (IDateTime StartTime, IDateTime? EndTime, Duration? Duration) GetOriginalValues() + { + return (StartTime, EndTime, Duration); + } + + internal Duration GetEffectiveDuration() { - return (StartTime, _endTime, _duration); + if (Duration is { } d) + return d; + + if (EndTime is { } endTime) + return endTime.Subtract(StartTime); + + if (!StartTime.HasTime) + return DataTypes.Duration.FromDays(1); + + return DataTypes.Duration.Zero; + } + + internal IDateTime GetEffectiveEndTime() + { + if (EndTime is { } endTime) + return endTime; + + return StartTime.Add(GetEffectiveDuration()); } /// @@ -184,8 +173,9 @@ public virtual bool Contains(IDateTime? dt) return false; } + var endTime = GetEffectiveEndTime(); // End time is exclusive - return EndTime == null || EndTime.GreaterThan(dt); + return endTime == null || endTime.GreaterThan(dt); } /// @@ -199,17 +189,10 @@ public virtual bool Contains(IDateTime? dt) /// /// public virtual bool CollidesWith(Period period) - { - // Check if the start or end time of the given period is within the current period - var startContained = Contains(period.StartTime); - var endContained = period.EndTime != null && Contains(period.EndTime); - - // Check if the current period is completely within the given period - var currentStartContained = period.Contains(StartTime); - var currentEndContained = EndTime != null && period.Contains(EndTime); - - return startContained || endContained || currentStartContained || currentEndContained; - } + => Contains(period.StartTime) + || period.Contains(StartTime) + || Contains(period.GetEffectiveEndTime()) + || period.Contains(GetEffectiveEndTime()); public int CompareTo(Period? other) { diff --git a/Ical.Net/Evaluation/EventEvaluator.cs b/Ical.Net/Evaluation/EventEvaluator.cs index cefa9594..09b44388 100644 --- a/Ical.Net/Evaluation/EventEvaluator.cs +++ b/Ical.Net/Evaluation/EventEvaluator.cs @@ -91,7 +91,8 @@ and it may differ from the time span added to the period start time. } // Return the Period object with the calculated end time and duration. - period.Duration = endTime.SubtractExact(period.StartTime); // exact duration + // TODO: Should we preserve both, endTime and duration? + period.Duration = endTime.Subtract(period.StartTime); // exact duration period.EndTime = endTime; // Only EndTime is relevant for further processing. return period; diff --git a/Ical.Net/Serialization/DataTypes/PeriodSerializer.cs b/Ical.Net/Serialization/DataTypes/PeriodSerializer.cs index 2cb8049d..b7b4ac7a 100644 --- a/Ical.Net/Serialization/DataTypes/PeriodSerializer.cs +++ b/Ical.Net/Serialization/DataTypes/PeriodSerializer.cs @@ -99,7 +99,7 @@ public override object Deserialize(TextReader tr) p.EndTime = dtSerializer.Deserialize(new StringReader(values[1])) as IDateTime; if (p.EndTime == null) { - p.Duration = ((Duration)durationSerializer.Deserialize(new StringReader(values[1]))).ToTimeSpan(); + p.Duration = (Duration)durationSerializer.Deserialize(new StringReader(values[1])); } // Only return an object if it has been deserialized correctly.