Skip to content

Commit

Permalink
Add abstraction layer to create EXDATE and RDATE properties (#684)
Browse files Browse the repository at this point in the history
* Fix `Period` and `PeriodList` to work with `EXDATE` and `RDATE`

Improve reliability and usability for `Period` and `PeriodList` to become less error-prone
by enforcing timezones being used consistently

Period
- Make parameterless CTOR `internal` to ensure proper initialization by users
- A period can be defined
  1. by a start time and an end time,
  2. by a start time and a duration,
  3. by a start time only, with the duration unspecified. This is for EXDATE and RDATE date-only and date/time.
- For cosistency, either the `EndTime` or the `Duration` can be set at a time. The last one set with a value not `null` will prevail, while the other will become `null`.
- Timezones of `StartTime`and (optional) `EndTime` must be the same
- `CompareTo` uses `AsUtc` for comparing the `StartTime`
- Remove returning a "magic" duration of 1 day, if `EndTime` is null and `StartTime` is date-only. This broke `EXDATE` and `RDATE` of an event, when the only a start time exists.
- Added `EffectiveEndTime` and `EffectiveDuration` properties to provide calculated values based on the set values.
- Update the `EndTime` and `Duration` properties to directly return the set values.
- Change the access modifiers of `GetEffectiveDuration()` and `GetEffectiveEndTime` methods from internal to private.

PeriodList
- `TzId`: `public` setter changed to `private`
- `EnsureConsistentTimezones`: The first period determines the timezone of the `PeriodList` and all other `Period`s added must have the same timezone
- Add `SetService(new PeriodListEvaluator(this))` for `StringReader` CTOR overload
- Add `static PeriodList FromStringReader(StringReader)`
- Add `static PeriodList FromDateTime(IDateTime)`
- Add `PeriodList AddPeriod(Period)` for chaining
- Add `PeriodList Add(IDateTime)` for chaining
- nullable enable

EventEvaluator:
- `EventEvaluator.WithEndTime(Period)` only sets the `EndTime`, as `Period.EffectiveDuration` returns the duration.

TodoEvaluator:
- Remove method `PeriodWithDuration(Period)` as it became redudant with the refactored `Period` class.

DataTypeSerializer and other serializers, CalendarObjectBase:
- `Activator.CreateInstance(TargetType, true)` allows for not `public` CTORs, so that parameterless CTORs can be excluded from the public API, if proper initialization can't be assured (like with `Period`).

PropertySerializer:
- TBD: Never set the UTC timezone ID (use appending 'Z')

- Resolves #590
- Resolves #591
- Resolves #614
- Resolves #676

* Add unit tests to `RecurrenceWithRDateTests`

* Insert values about failure to Exception.Messages

* Correct handling of iCalendar PERIOD

PeriodKind
- Add new enum

Period:
- Add internal `property string TzId`
- Add internal method `PeriodKind GetPeriodKind()`
- Ensure timeone consistence for setters of StartTime / EndTime

PeriodList:
- Add internal `property string TzId`
- Add internal property ``PeriodKind PeriodListKind`m
- Ensure added `Period`s have the same `TzId`and `PeriodKind` as the first one added

PeriodListSerialilzer
- Serializes TZID
- Serializes value type PERIOD

* Add abstraction layer to create EXDATE and RDATE

- Introduced `ExceptionDateCollection`, `RecurrencePeriodCollection` and PeriodCollectionBase` classes.
- Updated serializers to handle the modified `PeriodList` implementation.

These classes are an abstraction layer for creating `List<PeriodList>` objects.
`Period`s and `CalDateTime`s are accepted without having to care for RFC 5545 rules regarding serialization:

`ToRecurrenceDates` and `ToRecurrenceDates` methods aggregate and convert the exception or recurrence `CalDateTime` and `Period` objects into a list of `PeriodList` objects. Periods are grouped by their timezone IDs and period kinds in a way, that each `PeriodList` contains only distinct periods ready to serialize.

* `Period` StartTime, EndTime and Duration properties are immutable

- Adjust code in ' EventEvaluator` and `PeriodSerializer`
- Method `GetPeriodKind()` is replace with internal property `PeriodKind`
- Add internal `Period.Create` to centralize logic if end time and duration exist
- Modified unit test

* Remove source and test files related to RDATE/EXDATEcollections

Remove solution for RDATE/ESDATE introduced earlier in this PE

The commit removes several test and source files related to period and exception date collections in calendar events. Specifically, the following files have been completely removed:
- `PeriodCollectionBase.cs`
- `ExceptionDateCollection.cs`
- `RecurrencePeriodCollection.cs`
- `PeriodCollectionTests.cs`
- `RecurrenceWithExDateTests.cs`
- `RecurrenceWithRDateTests.cs`

* Add wrapper classes for `ExceptionDates` and `RecurrenceDates`

- Add internal properties `PeriodKind` and `TzId` to `PeriodList`.
- Modify `Period` indexer and `Insert` to ensure consistency for timezone and `PeriodKind`
- Prepare for `PeriodList` to become `internal`
- Update `PeriodListSerializer` and `PeriodSerializer` classes for `Period` serialization
- Introduce `ExceptionDates`, `RecurrenceDates`, and `PeriodListWrapperBase` to become wrappers around these properties of `RecurringComponent`
- Add unit tests in `PeriodListWrapperTests` for `ExceptionDates` and `RecurrenceDates`.

Note: The wrappers are not yet implemented for `IRecurringComponent`s.

* Migrate usage of `IList<PeriodList>` to `ExceptionDates` and `RecurrenceDates`

`IRecurrable`
- Change the type of `ExceptionDates` from `IList<PeriodList>` to `ExceptionDates`.
- Change the type of `RecurrenceDates` from `IList<PeriodList>` to `RecurrenceDates`.

`RecurringComponent`, `VTimeZoneInfo`
- Rename `IList<PeriodList> ExceptionDates` to `ExceptionDatesPeriodLists`. This serves as the backing storage for the new `ExceptionDates` class/property.
- Rename `IList<PeriodList> RecurrenceDates` to `RecurrenceDatesPeriodLists`. This serves as the backing storage for the new `RecurrenceDates` class/property.
- Add the creation of instances of `ExceptionDates` and `RecurrenceDates` to the `Initialize()` method.

`PeriodList` and `PeriodListEvaluator`
- Change the access modifier to `internal`.

`RecurringEvaluator` and `TodoEvaluator`
- Implement the new `ExceptionDates` and `RecurrenceDates` classes.

`VTimeZone`
- Enhanced serialization:
  - Before: Each `RDATE` date/time was serialized as a separate `RDATE` propery
  - Now: `RDATE` date/times which can be clustered go into one single `RDATE` property

Unit tests
- Refactor tests, using `ExceptionDates` and `RecurrenceDates` instead of `IList<PeriodList>`
- Remove the duplicate `SimpleDeserializationTests.RecurrenceDates1()` method, which exists in `DeserializationTests.RecurrenceDates1()`.

Todo:
- Replace the use of `PeriodList.GetGroupedPeriods` in `CalendarEvent.Equals()`.

* Add `PeriodListWrapperBase.GetAllPeriodsByKind`

Using this internal method stops recreating `Period`s from `IDateTime`
Update comments in PropertySerializer

* 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`

* Add EXDATE and RDATE unit tests mentioned in issues

* Updates after revview 2025-01-16
  • Loading branch information
axunonb authored Jan 17, 2025
1 parent 6b933ce commit 391c624
Show file tree
Hide file tree
Showing 44 changed files with 2,139 additions and 621 deletions.
19 changes: 9 additions & 10 deletions Ical.Net.Tests/CalendarEventTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ namespace Ical.Net.Tests;
[TestFixture]
public class CalendarEventTest
{
private static readonly DateTime _now = DateTime.UtcNow;
private static readonly DateTime _now = DateTime.SpecifyKind(DateTime.Now, DateTimeKind.Unspecified);
private static readonly DateTime _later = _now.AddHours(1);
private static readonly string _uid = Guid.NewGuid().ToString();

Expand Down Expand Up @@ -249,29 +249,29 @@ public void RrulesAreSignificantTests()
Assert.That(testRrule.GetHashCode(), Is.Not.EqualTo(simpleEvent.GetHashCode()));

var testRdate = GetSimpleEvent();
testRdate.RecurrenceDates = new List<PeriodList> { new PeriodList { new Period(new CalDateTime(_now)) } };
testRdate.RecurrenceDatesPeriodLists = new List<PeriodList> { new PeriodList { new Period(new CalDateTime(_now)) } };
Assert.That(testRdate, Is.Not.EqualTo(simpleEvent));
Assert.That(testRdate.GetHashCode(), Is.Not.EqualTo(simpleEvent.GetHashCode()));
}

private static List<RecurrencePattern> GetSimpleRecurrenceList()
=> new List<RecurrencePattern> { new RecurrencePattern(FrequencyType.Daily, 1) { Count = 5 } };
private static List<PeriodList> GetExceptionDates()
=> new List<PeriodList> { new PeriodList { new Period(new CalDateTime(_now.AddDays(1).Date)) } };
private static List<IDateTime> GetExceptionDates()
=> new List<IDateTime> { new CalDateTime(_now.AddDays(1).Date) };

[Test]
public void EventWithRecurrenceAndExceptionComparison()
{
var vEvent = GetSimpleEvent();
vEvent.RecurrenceRules = GetSimpleRecurrenceList();
vEvent.ExceptionDates = GetExceptionDates();
vEvent.ExceptionDates.AddRange(GetExceptionDates());

var calendar = new Calendar();
calendar.Events.Add(vEvent);

var vEvent2 = GetSimpleEvent();
vEvent2.RecurrenceRules = GetSimpleRecurrenceList();
vEvent2.ExceptionDates = GetExceptionDates();
vEvent2.ExceptionDates.AddRange(GetExceptionDates());

var cal2 = new Calendar();
cal2.Events.Add(vEvent2);
Expand All @@ -283,8 +283,7 @@ public void EventWithRecurrenceAndExceptionComparison()
{
Assert.That(eventB.RecurrenceRules.First(), Is.EqualTo(eventA.RecurrenceRules.First()));
Assert.That(eventB.RecurrenceRules.First().GetHashCode(), Is.EqualTo(eventA.RecurrenceRules.First().GetHashCode()));
Assert.That(eventB.ExceptionDates.First(), Is.EqualTo(eventA.ExceptionDates.First()));
Assert.That(eventB.ExceptionDates.First().GetHashCode(), Is.EqualTo(eventA.ExceptionDates.First().GetHashCode()));
Assert.That(eventB.ExceptionDates.GetAllDates().First(), Is.EqualTo(eventA.ExceptionDates.GetAllDates().First()));
Assert.That(eventB.GetHashCode(), Is.EqualTo(eventA.GetHashCode()));
Assert.That(eventB, Is.EqualTo(eventA));
Assert.That(cal2, Is.EqualTo(calendar));
Expand All @@ -309,7 +308,7 @@ public void AddingExdateToEventShouldNotBeEqualToOriginal()
var deserializedNoExDate = Calendar.Load(serialized);
Assert.That(deserializedNoExDate, Is.EqualTo(cal1));

vEvent.ExceptionDates = GetExceptionDates();
vEvent.ExceptionDates.AddRange(GetExceptionDates());
serialized = serializer.SerializeToString(cal1);
var deserializedWithExDate = Calendar.Load(serialized);

Expand Down Expand Up @@ -516,7 +515,7 @@ public void GetEffectiveDurationTests()
{
Assert.That(evt.DtStart.Value, Is.EqualTo(dt.Date));
Assert.That(evt.Duration, Is.Null);
Assert.That(evt.GetEffectiveDuration(), Is.EqualTo(Duration.FromDays(1)));
Assert.That(evt.GetEffectiveDuration(), Is.EqualTo(DataTypes.Duration.FromDays(1)));
});

evt = new CalendarEvent
Expand Down
2 changes: 1 addition & 1 deletion Ical.Net.Tests/CalendarPropertiesTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ public void PropertySetValueMustAllowNull()
var property = new CalendarProperty();
Assert.DoesNotThrow(() => property.SetValue(null));
}
}
}
4 changes: 2 additions & 2 deletions Ical.Net.Tests/CollectionHelpersTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace Ical.Net.Tests;

internal class CollectionHelpersTests
{
private static readonly DateTime _now = DateTime.UtcNow;
private static readonly DateTime _now = DateTime.SpecifyKind(DateTime.UtcNow, DateTimeKind.Unspecified);

private static List<PeriodList> GetExceptionDates()
=> new List<PeriodList> { new PeriodList { new Period(new CalDateTime(_now.AddDays(1).Date)) } };
Expand All @@ -29,7 +29,7 @@ public void ExDateTests()
});

var changedPeriod = GetExceptionDates();
changedPeriod.First().First().StartTime = new CalDateTime(_now.AddHours(-1));
changedPeriod[0][0] = new Period(new CalDateTime(_now.AddHours(-1)), changedPeriod[0][0].EndTime);

Assert.That(changedPeriod, Is.Not.EqualTo(GetExceptionDates()));
}
Expand Down
2 changes: 1 addition & 1 deletion Ical.Net.Tests/CopyComponentTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = Duration.FromDays(1), Status = FreeBusyStatus.Busy } }
Entries = { new FreeBusyEntry(new Period(new CalDateTime(2024, 10, 1), Duration.FromDays(1)), FreeBusyStatus.Busy) { Language = "English" }}
};

var copy = orig.Copy<FreeBusy>();
Expand Down
129 changes: 76 additions & 53 deletions Ical.Net.Tests/DeserializationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -267,36 +267,37 @@ public void Encoding3()
[Test]
public void Event8()
{
var sr = @"BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Apple Computer\, Inc//iCal 1.0//EN
CALSCALE:GREGORIAN
BEGIN:VEVENT
CREATED:20070404T211714Z
DTEND:20070407T010000Z
DTSTAMP:20070404T211714Z
DTSTART:20070406T230000Z
DURATION:PT2H
RRULE:FREQ=WEEKLY;UNTIL=20070801T070000Z;BYDAY=FR
SUMMARY:Friday Meetings
DTSTAMP:20040103T033800Z
SEQUENCE:1
UID:fd940618-45e2-4d19-b118-37fd7a8e3906
END:VEVENT
BEGIN:VEVENT
CREATED:20070404T204310Z
DTEND:20070416T030000Z
DTSTAMP:20070404T204310Z
DTSTART:20070414T200000Z
DURATION:P1DT7H
RRULE:FREQ=DAILY;COUNT=12;BYDAY=SA,SU
SUMMARY:Weekend Yea!
DTSTAMP:20040103T033800Z
SEQUENCE:1
UID:ebfbd3e3-cc1e-4a64-98eb-ced2598b3908
END:VEVENT
END:VCALENDAR
";
var sr = """
BEGIN:VCALENDAR
VERSION:2.0
PRODID:-//Apple Computer\, Inc//iCal 1.0//EN
CALSCALE:GREGORIAN
BEGIN:VEVENT
CREATED:20070404T211714Z
DTEND:20070407T010000Z
DTSTAMP:20070404T211714Z
DTSTART:20070406T230000Z
DURATION:PT2H
RRULE:FREQ=WEEKLY;UNTIL=20070801T070000Z;BYDAY=FR
SUMMARY:Friday Meetings
DTSTAMP:20040103T033800Z
SEQUENCE:1
UID:fd940618-45e2-4d19-b118-37fd7a8e3906
END:VEVENT
BEGIN:VEVENT
CREATED:20070404T204310Z
DTEND:20070416T030000Z
DTSTAMP:20070404T204310Z
DTSTART:20070414T200000Z
DURATION:P1DT7H
RRULE:FREQ=DAILY;COUNT=12;BYDAY=SA,SU
SUMMARY:Weekend Yea!
DTSTAMP:20040103T033800Z
SEQUENCE:1
UID:ebfbd3e3-cc1e-4a64-98eb-ced2598b3908
END:VEVENT
END:VCALENDAR
""";
var iCal = Calendar.Load(sr);
Assert.That(iCal.Events.Count == 2, Is.True, "There should be 2 events in the parsed calendar");
Assert.That(iCal.Events["fd940618-45e2-4d19-b118-37fd7a8e3906"], Is.Not.Null, "Event fd940618-45e2-4d19-b118-37fd7a8e3906 should exist in the calendar");
Expand Down Expand Up @@ -351,25 +352,46 @@ public void Google1()
public void RecurrenceDates1()
{
var iCal = Calendar.Load(IcsFiles.RecurrenceDates1);
Assert.That(iCal.Events, Has.Count.EqualTo(1));
Assert.That(iCal.Events.First().RecurrenceDates, Has.Count.EqualTo(3));

var expectedStartTimes = new List<CalDateTime>
{
// date are unique
new CalDateTime(1997, 7, 14, 12, 30, 0, CalDateTime.UtcTzId),
new CalDateTime(1996, 4, 3, 2, 0, 0, CalDateTime.UtcTzId),
new CalDateTime(1996, 4, 4, 1, 0, 0, CalDateTime.UtcTzId),
new CalDateTime(1997, 1, 1),
new CalDateTime(1997, 1, 20),
new CalDateTime(1997, 2, 17),
new CalDateTime(1997, 4, 21),
new CalDateTime(1997, 5, 26),
new CalDateTime(1997, 7, 4),
new CalDateTime(1997, 9, 1),
new CalDateTime(1997, 10, 14),
new CalDateTime(1997, 11, 28),
new CalDateTime(1997, 11, 29),
new CalDateTime(1997, 12, 25)
};

var expectedEndTime = new CalDateTime(new DateTime(1996, 4, 3, 4, 0, 0, DateTimeKind.Utc));

var actualStartTimes = iCal.Events[0].RecurrenceDates.GetAllPeriods()
.Select(p => p.StartTime)
.Union(iCal.Events[0].RecurrenceDates.GetAllDates())
.ToList();

Assert.Multiple(() =>
{
Assert.That(iCal.Events.First().RecurrenceDates[0][0].StartTime, Is.EqualTo((CalDateTime)new DateTime(1997, 7, 14, 12, 30, 0, DateTimeKind.Utc)));
Assert.That(iCal.Events.First().RecurrenceDates[1][0].StartTime, Is.EqualTo((CalDateTime)new DateTime(1996, 4, 3, 2, 0, 0, DateTimeKind.Utc)));
Assert.That(iCal.Events.First().RecurrenceDates[1][0].EndTime, Is.EqualTo((CalDateTime)new DateTime(1996, 4, 3, 4, 0, 0, DateTimeKind.Utc)));
Assert.That(iCal.Events.First().RecurrenceDates[2][0].StartTime, Is.EqualTo(new CalDateTime(1997, 1, 1)));
Assert.That(iCal.Events.First().RecurrenceDates[2][1].StartTime, Is.EqualTo(new CalDateTime(1997, 1, 20)));
Assert.That(iCal.Events.First().RecurrenceDates[2][2].StartTime, Is.EqualTo(new CalDateTime(1997, 2, 17)));
Assert.That(iCal.Events.First().RecurrenceDates[2][3].StartTime, Is.EqualTo(new CalDateTime(1997, 4, 21)));
Assert.That(iCal.Events.First().RecurrenceDates[2][4].StartTime, Is.EqualTo(new CalDateTime(1997, 5, 26)));
Assert.That(iCal.Events.First().RecurrenceDates[2][5].StartTime, Is.EqualTo(new CalDateTime(1997, 7, 4)));
Assert.That(iCal.Events.First().RecurrenceDates[2][6].StartTime, Is.EqualTo(new CalDateTime(1997, 9, 1)));
Assert.That(iCal.Events.First().RecurrenceDates[2][7].StartTime, Is.EqualTo(new CalDateTime(1997, 10, 14)));
Assert.That(iCal.Events.First().RecurrenceDates[2][8].StartTime, Is.EqualTo(new CalDateTime(1997, 11, 28)));
Assert.That(iCal.Events.First().RecurrenceDates[2][9].StartTime, Is.EqualTo(new CalDateTime(1997, 11, 29)));
Assert.That(iCal.Events.First().RecurrenceDates[2][10].StartTime, Is.EqualTo(new CalDateTime(1997, 12, 25)));
Assert.That(iCal.Events, Has.Count.EqualTo(1));
Assert.That(iCal.Events[0].RecurrenceDatesPeriodLists, Has.Count.EqualTo(3));
Assert.That(actualStartTimes, Has.Count.EqualTo(expectedStartTimes.Count));

foreach (var date in expectedStartTimes)
{
Assert.That(actualStartTimes.Single(dt => dt.Equals(date)),
Is.EqualTo(date), "Should contain " + date);
}

Assert.That(iCal.Events[0].RecurrenceDates.Contains(new Period(expectedStartTimes[1], expectedEndTime)));
});
}

Expand Down Expand Up @@ -549,13 +571,14 @@ public void Property1()
[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 calStr = $"""
BEGIN:VCALENDAR
BEGIN:VEVENT
DTSTART:20070406T230000Z
{(useDtEnd ? "DTEND:20070407T010000Z" : "DURATION:PT1H")}
END:VEVENT
END:VCALENDAR
""";

var calendar = Calendar.Load(calStr);

Expand Down
Loading

0 comments on commit 391c624

Please sign in to comment.