Skip to content

Commit dc4f248

Browse files
committed
Replace method PeriodList.GetGroupedPeriods
- No duplicate `Period` instances can be added or inserted into the `PeriodList`. - `RecurrenceDates.GetAllDates()`, `PeriodListWrapperBase.GetAllDates()` and `PeriodListWrapperBase.GetAllPeriodsByKind` return a flattened and ordered list of distinct objects - Update `CalendarEvent.Equals` and `CalendarEvent.GetHashCode` using `ExceptionDates` and `RecurrenceDates` instead of `PeriodList.GetGroupedPeriods` - Remove `PeriodList.GetGroupedPeriods`
1 parent f2e9c19 commit dc4f248

File tree

8 files changed

+93
-99
lines changed

8 files changed

+93
-99
lines changed

Ical.Net.Tests/PeriodListTest.cs

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public void GetSet_Period_ShouldReturnCorrectPeriod()
3939
var period2 = new Period(new CalDateTime(2025, 2, 1, 0, 0, 0), Duration.FromHours(1));
4040

4141
periodList.Add(period1);
42-
periodList.Add(period1);
42+
periodList.Add(period2);
4343

4444
// Act
4545
var retrievedPeriod = periodList[0];
@@ -54,6 +54,52 @@ public void GetSet_Period_ShouldReturnCorrectPeriod()
5454
});
5555
}
5656

57+
[Test]
58+
public void ExistingPeriod_ShouldNotBeAddedAgain()
59+
{
60+
// Arrange
61+
var periodList = new PeriodList();
62+
var period = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), Duration.FromHours(1));
63+
64+
periodList.Add(period);
65+
periodList.Add(period);
66+
periodList.Insert(0, period);
67+
68+
// Assert
69+
Assert.That(periodList, Has.Count.EqualTo(1));
70+
}
71+
72+
[Test]
73+
public void AddPeriodWithInconsistentTimezoneOrPeriodKind_ShouldThrow()
74+
{
75+
// Arrange
76+
var periodList = new PeriodList();
77+
var dateTimeUtcPeriod = new Period(new CalDateTime(2023, 1, 1, 12, 0, 0, CalDateTime.UtcTzId));
78+
var durationPeriod = new Period(new CalDateTime(2023, 1, 2, 12, 0, 0, CalDateTime.UtcTzId), new Duration(1, 0, 0, 0));
79+
var dateTimeLocalPeriod = new Period(new CalDateTime(2023, 1, 2, 12, 0, 0, "America/New_York"));
80+
var dateOnlyPeriod = new Period(new CalDateTime(2023, 1, 3));
81+
82+
// The first period determines timezone and period kind
83+
periodList.Add(dateTimeUtcPeriod);
84+
85+
// Act & Assert
86+
Assert.Multiple(() =>
87+
{
88+
Assert.That(periodList.Count, Is.EqualTo(1));
89+
90+
// Test adding a period with inconsistent PeriodKind
91+
Assert.That(() => periodList.Add(durationPeriod), Throws.ArgumentException);
92+
93+
// Test adding a period with inconsistent TzId
94+
Assert.That(() => periodList.Add(dateTimeLocalPeriod), Throws.ArgumentException);
95+
96+
// Test adding period with inconsistent PeriodKind
97+
Assert.That(() => periodList.Insert(0, dateOnlyPeriod), Throws.ArgumentException);
98+
;
99+
Assert.That(periodList.Count, Is.EqualTo(1));
100+
});
101+
102+
}
57103
[Test]
58104
public void Clear_ShouldRemoveAllPeriods()
59105
{

Ical.Net/CalendarComponents/CalendarEvent.cs

Lines changed: 10 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -285,29 +285,18 @@ protected bool Equals(CalendarEvent? other)
285285
return false;
286286
}
287287

288-
// RDATEs and EXDATEs are all List<PeriodList>, because the spec allows for multiple declarations of collections.
289-
// Consequently we have to contrive a normalized representation before we can determine whether two events are equal
290-
291-
var exDates = PeriodList.GetGroupedPeriods(ExceptionDatesPeriodLists);
292-
var otherExDates = PeriodList.GetGroupedPeriods(other.ExceptionDatesPeriodLists);
293-
if (exDates.Keys.Count != otherExDates.Keys.Count || !exDates.Keys.OrderBy(k => k).SequenceEqual(otherExDates.Keys.OrderBy(k => k)))
294-
{
295-
return false;
296-
}
297-
298-
if (exDates.Any(exDate => !exDate.Value.OrderBy(d => d).SequenceEqual(otherExDates[exDate.Key].OrderBy(d => d))))
299-
{
300-
return false;
301-
}
302-
303-
var rDates = PeriodList.GetGroupedPeriods(RecurrenceDatesPeriodLists);
304-
var otherRDates = PeriodList.GetGroupedPeriods(other.RecurrenceDatesPeriodLists);
305-
if (rDates.Keys.Count != otherRDates.Keys.Count || !rDates.Keys.OrderBy(k => k).SequenceEqual(otherRDates.Keys.OrderBy(k => k)))
288+
// exDates and otherExDates are filled with a sorted list of distinct periods
289+
var exDates = ExceptionDates.GetAllPeriodsByKind(PeriodKind.Period, PeriodKind.DateOnly, PeriodKind.DateTime).ToList();
290+
var otherExDates = other.ExceptionDates.GetAllPeriodsByKind(PeriodKind.Period, PeriodKind.DateOnly, PeriodKind.DateTime).ToList();
291+
if (exDates.Count != otherExDates.Count || !exDates.SequenceEqual(otherExDates))
306292
{
307293
return false;
308294
}
309295

310-
if (rDates.Any(exDate => !exDate.Value.OrderBy(d => d).SequenceEqual(otherRDates[exDate.Key].OrderBy(d => d))))
296+
// rDates and otherRDates are filled with a sorted list of distinct periods
297+
var rDates = RecurrenceDates.GetAllPeriodsByKind(PeriodKind.Period, PeriodKind.DateOnly, PeriodKind.DateTime).ToList();
298+
var otherRDates = other.RecurrenceDates.GetAllPeriodsByKind(PeriodKind.Period, PeriodKind.DateOnly, PeriodKind.DateTime).ToList();
299+
if (rDates.Count != otherRDates.Count && !rDates.SequenceEqual(otherRDates))
311300
{
312301
return false;
313302
}
@@ -339,9 +328,9 @@ public override int GetHashCode()
339328
hashCode = (hashCode * 397) ^ Transparency?.GetHashCode() ?? 0;
340329
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(Attachments);
341330
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(Resources);
342-
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCodeForNestedCollection(ExceptionDatesPeriodLists);
331+
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(ExceptionDates.GetAllPeriodsByKind(PeriodKind.Period, PeriodKind.DateOnly, PeriodKind.DateTime));
343332
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(ExceptionRules);
344-
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCodeForNestedCollection(RecurrenceDatesPeriodLists);
333+
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(RecurrenceDates.GetAllPeriodsByKind(PeriodKind.Period, PeriodKind.DateOnly, PeriodKind.DateTime));
345334
hashCode = (hashCode * 397) ^ CollectionHelpers.GetHashCode(RecurrenceRules);
346335
return hashCode;
347336
}

Ical.Net/CalendarComponents/VTimeZone.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,29 +22,29 @@ public class VTimeZone : CalendarComponent
2222
public static VTimeZone FromLocalTimeZone()
2323
=> FromDateTimeZone(DateUtil.LocalDateTimeZone.Id);
2424

25-
public static VTimeZone FromLocalTimeZone(DateTime earlistDateTimeToSupport, bool includeHistoricalData)
26-
=> FromDateTimeZone(DateUtil.LocalDateTimeZone.Id, earlistDateTimeToSupport, includeHistoricalData);
25+
public static VTimeZone FromLocalTimeZone(DateTime earliestDateTimeToSupport, bool includeHistoricalData)
26+
=> FromDateTimeZone(DateUtil.LocalDateTimeZone.Id, earliestDateTimeToSupport, includeHistoricalData);
2727

2828
public static VTimeZone FromSystemTimeZone(TimeZoneInfo tzinfo)
2929
=> FromSystemTimeZone(tzinfo, new DateTime(DateTime.Now.Year, 1, 1), false);
3030

31-
public static VTimeZone FromSystemTimeZone(TimeZoneInfo tzinfo, DateTime earlistDateTimeToSupport, bool includeHistoricalData)
32-
=> FromDateTimeZone(tzinfo.Id, earlistDateTimeToSupport, includeHistoricalData);
31+
public static VTimeZone FromSystemTimeZone(TimeZoneInfo tzinfo, DateTime earliestDateTimeToSupport, bool includeHistoricalData)
32+
=> FromDateTimeZone(tzinfo.Id, earliestDateTimeToSupport, includeHistoricalData);
3333

3434
public static VTimeZone FromDateTimeZone(string tzId)
3535
=> FromDateTimeZone(tzId, new DateTime(DateTime.Now.Year, 1, 1), includeHistoricalData: false);
3636

37-
public static VTimeZone FromDateTimeZone(string tzId, DateTime earlistDateTimeToSupport, bool includeHistoricalData)
37+
public static VTimeZone FromDateTimeZone(string tzId, DateTime earliestDateTimeToSupport, bool includeHistoricalData)
3838
{
3939
var vTimeZone = new VTimeZone(tzId);
4040

4141
var earliestYear = 1900;
42-
var earliestMonth = earlistDateTimeToSupport.Month;
43-
var earliestDay = earlistDateTimeToSupport.Day;
42+
var earliestMonth = earliestDateTimeToSupport.Month;
43+
var earliestDay = earliestDateTimeToSupport.Day;
4444
// Support date/times for January 1st of the previous year by default.
45-
if (earlistDateTimeToSupport.Year > 1900)
45+
if (earliestDateTimeToSupport.Year > 1900)
4646
{
47-
earliestYear = earlistDateTimeToSupport.Year - 1;
47+
earliestYear = earliestDateTimeToSupport.Year - 1;
4848
// Since we went back a year, we can't still be in a leap-year
4949
if (earliestMonth == 2 && earliestDay == 29)
5050
earliestDay = 28;
@@ -56,7 +56,7 @@ public static VTimeZone FromDateTimeZone(string tzId, DateTime earlistDateTimeTo
5656
earliestDay = 28;
5757
}
5858
var earliest = Instant.FromUtc(earliestYear, earliestMonth, earliestDay,
59-
earlistDateTimeToSupport.Hour, earlistDateTimeToSupport.Minute);
59+
earliestDateTimeToSupport.Hour, earliestDateTimeToSupport.Minute);
6060

6161
// Only include historical data if asked to do so. Otherwise,
6262
// use only the most recent adjustment rules available.

Ical.Net/DataTypes/ExceptionDates.cs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
#nullable enable
77
using System.Collections.Generic;
8-
using System.Linq;
98

109
namespace Ical.Net.DataTypes;
1110

@@ -29,10 +28,6 @@ public ExceptionDates Add(IDateTime dt)
2928
{
3029
var periodList = GetOrCreatePeriodList(dt);
3130

32-
// Don't add the same date twice.
33-
if (periodList.FirstOrDefault(period => Equals(period.StartTime, dt)) != null)
34-
return this;
35-
3631
var dtPeriod = new Period(dt);
3732
periodList.Add(dtPeriod);
3833

Ical.Net/DataTypes/Period.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,8 @@ public override int GetHashCode()
156156
public virtual IDateTime? EndTime => _endTime;
157157

158158
/// <summary>
159-
/// Gets the nominal duration of the period that was set, or - if this is <see langword="null"/> -
160-
/// calculates the exact duration based on the duration.
159+
/// Gets the end time of the period that was set, or - if this is <see langword="null"/> -
160+
/// calculates the end time based by adding <see cref="EffectiveDuration"/> to the <see cref="StartTime"/>.
161161
/// If <see cref="Duration"/> and <see cref="EndTime"/> are both <see langword="null"/>, the method returns <see langword="null"/>.
162162
/// </summary>
163163
public virtual IDateTime? EffectiveEndTime
@@ -183,7 +183,7 @@ public virtual IDateTime? EffectiveEndTime
183183

184184
/// <summary>
185185
/// Gets the duration of the period that was set, or - if this is <see langword="null"/> -
186-
/// calculates the exact duration based on the end time.
186+
/// calculates the exact duration by subtracting <see cref="StartTime"/> from <see cref="EndTime"/>.
187187
/// If <see cref="Duration"/> and <see cref="EndTime"/> are both <see langword="null"/>, the method returns <see langword="null"/>.
188188
/// </summary>
189189
public virtual Duration? EffectiveDuration

Ical.Net/DataTypes/PeriodList.cs

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
using System.Collections;
99
using System.Collections.Generic;
1010
using System.IO;
11-
using System.Linq;
1211
using Ical.Net.Evaluation;
1312
using Ical.Net.Serialization.DataTypes;
1413
using Ical.Net.Utility;
@@ -85,43 +84,6 @@ public override void CopyFrom(ICopyable obj)
8584
/// <returns></returns>
8685
public override string? ToString() => new PeriodListSerializer().SerializeToString(this);
8786

88-
/// <summary>
89-
/// Used for equality comparison of two lists of periods.
90-
/// </summary>
91-
public static Dictionary<string, IList<Period>> GetGroupedPeriods(IList<PeriodList> periodLists)
92-
{
93-
// In order to know if two events are equal, a semantic understanding of exdates, rdates, rrules, and exrules is required. This could be done by
94-
// computing the complete recurrence set (expensive) while being time-zone sensitive, or by comparing each List<Period> in each IPeriodList.
95-
96-
// For example, events containing these rules generate the same recurrence set, including having the same time zone for each occurrence, so
97-
// they're the same:
98-
// Event A:
99-
// RDATE:20170302T060000Z,20170303T060000Z
100-
// Event B:
101-
// RDATE:20170302T060000Z
102-
// RDATE:20170303T060000Z
103-
104-
var grouped = new Dictionary<string, HashSet<Period>>(StringComparer.OrdinalIgnoreCase);
105-
foreach (var periodList in periodLists)
106-
{
107-
foreach (var period in periodList)
108-
{
109-
// Dictionary key cannot be null, so an empty string is used for the default bucket
110-
var bucketTzId = period.StartTime.TzId ?? string.Empty;
111-
112-
if (!grouped.TryGetValue(bucketTzId, out var periods))
113-
{
114-
periods = new HashSet<Period>();
115-
grouped.Add(bucketTzId, periods);
116-
}
117-
118-
periods.Add(period);
119-
}
120-
}
121-
122-
return grouped.ToDictionary(k => k.Key, v => (IList<Period>) v.Value.OrderBy(d => d.StartTime).ToList());
123-
}
124-
12587
protected bool Equals(PeriodList other) => CollectionHelpers.Equals(Periods, other.Periods);
12688

12789
/// <inheritdoc/>
@@ -155,39 +117,44 @@ public Period this[int index]
155117
/// <inheritdoc/>
156118
public int IndexOf(Period item) => Periods.IndexOf(item);
157119

158-
/// <inheritdoc/>
120+
/// <summary>
121+
/// Inserts a <see cref="Period"/> to the list if it does not already exist.<br/>
122+
/// The timezone period kind of the first value added determines the timezone for the whole list.
123+
/// </summary>
124+
/// <exception cref="ArgumentException"></exception>
159125
public void Insert(int index, Period item)
160126
{
161127
EnsureConsistentTimezoneAndPeriodKind(item);
128+
if (Periods.Contains(item)) return;
162129
Periods.Insert(index, item);
163130
}
164131

165132
/// <inheritdoc/>
166133
public void RemoveAt(int index) => Periods.RemoveAt(index);
167134

168135
/// <summary>
169-
/// Adds a <see cref="Period"/> to the list.<br/>
136+
/// Adds a <see cref="Period"/> to the list if it does not already exist.<br/>
170137
/// The timezone period kind of the first value added determines the timezone for the whole list.
171138
/// </summary>
172139
/// <param name="item">The <see cref="Period"/> for an 'RDATE'.</param>
173140
/// <exception cref="ArgumentException"></exception>
174141
public void Add(Period item)
175142
{
176143
EnsureConsistentTimezoneAndPeriodKind(item);
144+
if (Periods.Contains(item)) return;
177145
Periods.Add(item);
178146
}
179147

180148
/// <summary>
181-
/// Adds a DATE or DATE-TIME value for an 'EXDATE' or 'RDATE' to the list.<br/>
149+
/// Adds a DATE or DATE-TIME value for an 'EXDATE' or 'RDATE' to the list if it does not already exist.<br/>
182150
/// The timezone period kind of the first value added determines the timezone for the whole list.
183151
/// </summary>
184152
/// <param name="dt"></param>
185153
/// <exception cref="ArgumentException"></exception>
186154
public void Add(IDateTime dt)
187155
{
188156
var p = new Period(dt);
189-
EnsureConsistentTimezoneAndPeriodKind(p);
190-
Periods.Add(p);
157+
Add(p);
191158
}
192159

193160
/// <inheritdoc/>

Ical.Net/DataTypes/PeriodListWrapperBase.cs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#nullable enable
77
using System.Collections.Generic;
88
using System.Linq;
9+
using Ical.Net.Utility;
910

1011
namespace Ical.Net.DataTypes;
1112

@@ -22,10 +23,13 @@ public abstract class PeriodListWrapperBase
2223
private protected PeriodListWrapperBase(IList<PeriodList> periodList) => ListOfPeriodList = periodList;
2324

2425
/// <summary>
25-
/// Gets a flattened list of all dates in the list.
26+
/// Gets a flattened and ordered list of all distinct dates in the list
2627
/// </summary>
2728
public IEnumerable<IDateTime> GetAllDates()
28-
=> ListOfPeriodList.SelectMany(pl => pl.Where(p => p.PeriodKind is PeriodKind.DateOnly or PeriodKind.DateTime).Select(p => p.StartTime));
29+
=> ListOfPeriodList.SelectMany(pl =>
30+
pl.Where(p => p.PeriodKind is PeriodKind.DateOnly or PeriodKind.DateTime)
31+
.Select(p => p.StartTime))
32+
.OrderedDistinct();
2933

3034
/// <summary>
3135
/// Clears all elements from the list.
@@ -97,8 +101,9 @@ private protected PeriodList GetOrCreatePeriodList(Period period)
97101
}
98102

99103
/// <summary>
100-
/// Gets a flattened list of all periods with <see cref="PeriodKind.Period"/>, <see cref="PeriodKind.DateOnly"/> and <see cref="PeriodKind.DateTime"/>.
104+
/// Gets a flattened and ordered list of all distinct periods with
105+
/// <see cref="PeriodKind.Period"/>, <see cref="PeriodKind.DateOnly"/> and <see cref="PeriodKind.DateTime"/>.
101106
/// </summary>
102107
internal IEnumerable<Period> GetAllPeriodsByKind(params PeriodKind[] periodKinds)
103-
=> ListOfPeriodList.SelectMany(pl => pl.Where(p => periodKinds.Contains(p.PeriodKind)));
108+
=> ListOfPeriodList.SelectMany(pl => pl.Where(p => periodKinds.Contains(p.PeriodKind))).OrderedDistinct();
104109
}

Ical.Net/DataTypes/RecurrenceDates.cs

Lines changed: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#nullable enable
77
using System.Collections.Generic;
88
using System.Linq;
9+
using Ical.Net.Utility;
910

1011
namespace Ical.Net.DataTypes;
1112

@@ -28,11 +29,6 @@ internal RecurrenceDates(IList<PeriodList> listOfPeriodList) : base(listOfPeriod
2829
public RecurrenceDates Add(IDateTime dt)
2930
{
3031
var periodList = GetOrCreatePeriodList(dt);
31-
32-
// Don't add the same date twice.
33-
if (periodList.FirstOrDefault(period => Equals(period.StartTime, dt)) != null)
34-
return this;
35-
3632
var dtPeriod = new Period(dt);
3733
periodList.Add(dtPeriod);
3834

@@ -45,11 +41,6 @@ public RecurrenceDates Add(IDateTime dt)
4541
public RecurrenceDates Add(Period period)
4642
{
4743
var periodList = GetOrCreatePeriodList(period);
48-
49-
// Don't add the same period twice.
50-
if (periodList.FirstOrDefault(p => Equals(p, period)) != null)
51-
return this;
52-
5344
periodList.Add(period);
5445

5546
return this;
@@ -103,8 +94,9 @@ public bool Remove(Period period)
10394
}
10495

10596
/// <summary>
106-
/// Gets a flattened list of all periods in the list.
97+
/// Gets a flattened and ordered list of all distinct periods in the list.
10798
/// </summary>
10899
public IEnumerable<Period> GetAllPeriods()
109-
=> ListOfPeriodList.SelectMany(pl => pl.Where(p => p.PeriodKind is PeriodKind.Period));
100+
=> ListOfPeriodList.
101+
SelectMany(pl => pl.Where(p => p.PeriodKind is PeriodKind.Period)).OrderedDistinct();
110102
}

0 commit comments

Comments
 (0)