Skip to content

Commit

Permalink
Fix Period and PeriodList to work with EXDATE and RDATE
Browse files Browse the repository at this point in the history
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 ical-org#590
- Resolves ical-org#591
- Resolves ical-org#614
- Resolves ical-org#676
  • Loading branch information
axunonb committed Jan 8, 2025
1 parent 6b933ce commit 753e478
Show file tree
Hide file tree
Showing 27 changed files with 954 additions and 347 deletions.
4 changes: 2 additions & 2 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 @@ -516,7 +516,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.Zero));
});

evt = new CalendarEvent
Expand Down
2 changes: 1 addition & 1 deletion 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 Down
101 changes: 19 additions & 82 deletions Ical.Net.Tests/EqualityAndHashingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Ical.Net.Tests;

public class EqualityAndHashingTests
{
private const string _someTz = "America/Los_Angeles";
private const string TzId = "America/Los_Angeles";
private static readonly DateTime _nowTime = DateTime.Parse("2016-07-16T16:47:02.9310521-04:00");
private static readonly DateTime _later = _nowTime.AddHours(1);

Expand All @@ -39,8 +39,8 @@ public static IEnumerable CalDateTime_TestCases()
var nowCalDt = new CalDateTime(_nowTime);
yield return new TestCaseData(nowCalDt, new CalDateTime(_nowTime)).SetName("Now, no time zone");

var nowCalDtWithTz = new CalDateTime(_nowTime, _someTz);
yield return new TestCaseData(nowCalDtWithTz, new CalDateTime(_nowTime, _someTz)).SetName("Now, with time zone");
var nowCalDtWithTz = new CalDateTime(_nowTime, TzId);
yield return new TestCaseData(nowCalDtWithTz, new CalDateTime(_nowTime, TzId)).SetName("Now, with time zone");
}

[Test]
Expand Down Expand Up @@ -281,7 +281,7 @@ public void Resources_Tests()
});
}

internal static (byte[] original, byte[] copy) GetAttachments()
private static (byte[] original, byte[] copy) GetAttachments()
{
var payload = Encoding.UTF8.GetBytes("This is an attachment!");
var payloadCopy = new byte[payload.Length];
Expand Down Expand Up @@ -347,112 +347,51 @@ public void PeriodListTests()
new DateTime(2017, 03, 02, 06, 00, 00),
new DateTime(2017, 03, 03, 06, 00, 00),
new DateTime(2017, 03, 06, 06, 00, 00),
new DateTime(2017, 03, 07, 06, 00, 00),
new DateTime(2017, 03, 08, 06, 00, 00),
new DateTime(2017, 03, 09, 06, 00, 00),
new DateTime(2017, 03, 10, 06, 00, 00),
new DateTime(2017, 03, 13, 06, 00, 00),
new DateTime(2017, 03, 14, 06, 00, 00),
new DateTime(2017, 03, 17, 06, 00, 00),
new DateTime(2017, 03, 20, 06, 00, 00),
new DateTime(2017, 03, 21, 06, 00, 00),
new DateTime(2017, 03, 22, 06, 00, 00),
new DateTime(2017, 03, 23, 06, 00, 00),
new DateTime(2017, 03, 24, 06, 00, 00),
new DateTime(2017, 03, 27, 06, 00, 00),
new DateTime(2017, 03, 28, 06, 00, 00),
new DateTime(2017, 03, 29, 06, 00, 00),
new DateTime(2017, 03, 30, 06, 00, 00),
new DateTime(2017, 03, 31, 06, 00, 00),
new DateTime(2017, 04, 03, 06, 00, 00),
new DateTime(2017, 04, 05, 06, 00, 00),
new DateTime(2017, 04, 06, 06, 00, 00),
new DateTime(2017, 04, 07, 06, 00, 00),
new DateTime(2017, 04, 10, 06, 00, 00),
new DateTime(2017, 04, 11, 06, 00, 00),
new DateTime(2017, 04, 12, 06, 00, 00),
new DateTime(2017, 04, 13, 06, 00, 00),
new DateTime(2017, 04, 17, 06, 00, 00),
new DateTime(2017, 04, 18, 06, 00, 00),
new DateTime(2017, 04, 19, 06, 00, 00),
new DateTime(2017, 04, 20, 06, 00, 00),
new DateTime(2017, 04, 21, 06, 00, 00),
new DateTime(2017, 04, 24, 06, 00, 00),
new DateTime(2017, 04, 25, 06, 00, 00),
new DateTime(2017, 04, 27, 06, 00, 00),
new DateTime(2017, 04, 28, 06, 00, 00),
new DateTime(2017, 05, 01, 06, 00, 00),
new DateTime(2017, 05, 01, 06, 00, 00)
}
.Select(dt => new Period(new CalDateTime(dt))).ToList();

var a = new PeriodList();
foreach (var period in startTimesA)
{
a.Add(period);
}

//Difference from A: first element became the second, and last element became the second-to-last element
// Difference from A: first element became the second,
// and last element became the second-to-last element
var startTimesB = new List<DateTime>
{
new DateTime(2017, 03, 03, 06, 00, 00),
new DateTime(2017, 03, 02, 06, 00, 00),
new DateTime(2017, 03, 06, 06, 00, 00),
new DateTime(2017, 03, 07, 06, 00, 00),
new DateTime(2017, 03, 08, 06, 00, 00),
new DateTime(2017, 03, 09, 06, 00, 00),
new DateTime(2017, 03, 10, 06, 00, 00),
new DateTime(2017, 03, 13, 06, 00, 00),
new DateTime(2017, 03, 14, 06, 00, 00),
new DateTime(2017, 03, 17, 06, 00, 00),
new DateTime(2017, 03, 20, 06, 00, 00),
new DateTime(2017, 03, 21, 06, 00, 00),
new DateTime(2017, 03, 22, 06, 00, 00),
new DateTime(2017, 03, 23, 06, 00, 00),
new DateTime(2017, 03, 24, 06, 00, 00),
new DateTime(2017, 03, 27, 06, 00, 00),
new DateTime(2017, 03, 28, 06, 00, 00),
new DateTime(2017, 03, 29, 06, 00, 00),
new DateTime(2017, 03, 30, 06, 00, 00),
new DateTime(2017, 03, 31, 06, 00, 00),
new DateTime(2017, 04, 03, 06, 00, 00),
new DateTime(2017, 04, 05, 06, 00, 00),
new DateTime(2017, 04, 06, 06, 00, 00),
new DateTime(2017, 04, 07, 06, 00, 00),
new DateTime(2017, 04, 10, 06, 00, 00),
new DateTime(2017, 04, 11, 06, 00, 00),
new DateTime(2017, 04, 12, 06, 00, 00),
new DateTime(2017, 04, 13, 06, 00, 00),
new DateTime(2017, 04, 17, 06, 00, 00),
new DateTime(2017, 04, 18, 06, 00, 00),
new DateTime(2017, 04, 19, 06, 00, 00),
new DateTime(2017, 04, 20, 06, 00, 00),
new DateTime(2017, 04, 21, 06, 00, 00),
new DateTime(2017, 04, 24, 06, 00, 00),
new DateTime(2017, 04, 25, 06, 00, 00),
new DateTime(2017, 04, 27, 06, 00, 00),
new DateTime(2017, 05, 01, 06, 00, 00),
new DateTime(2017, 04, 28, 06, 00, 00),
new DateTime(2017, 04, 28, 06, 00, 00)
}
.Select(dt => new Period(new CalDateTime(dt))).ToList();

var b = new PeriodList();

foreach (var period in startTimesB)
{
b.Add(period);
}

var collectionEqual = CollectionHelpers.Equals(a, b);
Assert.Multiple(() =>
{
Assert.That(collectionEqual, Is.EqualTo(true));
Assert.That(b.GetHashCode(), Is.EqualTo(a.GetHashCode()));
});

var listOfListA = new List<PeriodList> { a };
var listOfListB = new List<PeriodList> { b };
Assert.That(CollectionHelpers.Equals(listOfListA, listOfListB), Is.True);

var aThenB = new List<PeriodList> { a, b };
var bThenA = new List<PeriodList> { b, a };
Assert.That(CollectionHelpers.Equals(aThenB, bThenA), Is.True);

Assert.Multiple(() =>
{
Assert.That(collectionEqual, Is.EqualTo(true));
Assert.That(b.GetHashCode(), Is.EqualTo(a.GetHashCode()));
Assert.That(CollectionHelpers.Equals(listOfListA, listOfListB), Is.True);
Assert.That(CollectionHelpers.Equals(aThenB, bThenA), Is.True);
});
}

[Test]
Expand Down Expand Up @@ -489,8 +428,6 @@ private void TestComparison(Func<CalDateTime, IDateTime, bool> calOp, Func<int?,
[Test]
public void CalDateTimeComparisonOperatorTests()
{
// Assumption: comparison operators on CalDateTime are expected to
// work like operators on Nullable<int>.
TestComparison((dt1, dt2) => dt1 == dt2, (i1, i2) => i1 == i2);
TestComparison((dt1, dt2) => dt1 != dt2, (i1, i2) => i1 != i2);
TestComparison((dt1, dt2) => dt1 > dt2, (i1, i2) => i1 > i2);
Expand Down
140 changes: 140 additions & 0 deletions Ical.Net.Tests/PeriodListTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
//
// Copyright ical.net project maintainers and contributors.
// Licensed under the MIT license.
//

#nullable enable
using System;
using System.IO;
using Ical.Net.DataTypes;
using NUnit.Framework;

namespace Ical.Net.Tests;

[TestFixture]
public class PeriodListTests
{
[Test]
public void RemovePeriod_ShouldDecreaseCount()
{
// Arrange
var periodList = new PeriodList();
var period = new Period(new CalDateTime(2023, 1, 1, 0, 0, 0), Duration.FromHours(1));
periodList.Add(period);

// Act
periodList.Remove(period);

// Assert
Assert.That(periodList, Has.Count.EqualTo(0));
}

[Test]
public void GetSet_Period_ShouldReturnCorrectPeriod()
{
// Arrange
var periodList = new PeriodList();
var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), Duration.FromHours(1));
var period2 = new Period(new CalDateTime(2025, 2, 1, 0, 0, 0), Duration.FromHours(1));

periodList.AddPeriod(period1).AddPeriod(period1);

// Act
var retrievedPeriod = periodList[0];
periodList[1] = period2;

// Assert
Assert.Multiple(() =>
{
Assert.That(period1, Is.EqualTo(retrievedPeriod));
Assert.That(periodList.Contains(period1), Is.True);
Assert.That(periodList[periodList.IndexOf(period2)], Is.EqualTo(period2));
});
}

[Test]
public void Clear_ShouldRemoveAllPeriods()
{
// Arrange
var periodList = new PeriodList();
var pl = PeriodList
.FromDateTime(new CalDateTime(2025, 1, 2))
.Add(new CalDateTime(2025, 1, 3));

var count = pl.Count;

// Act
periodList.Clear();

// Assert
Assert.Multiple(() =>
{
Assert.That(count, Is.EqualTo(2));
Assert.That(periodList, Has.Count.EqualTo(0));
});
}

[Test]
public void Create_FromStringReader_ShouldSucceed()
{
// Arrange
const string periodString = "20250101T000000Z/20250101T010000Z,20250102T000000Z/20250102T010000Z";
using var reader = new StringReader(periodString);

// Act
var periodList = PeriodList.FromStringReader(reader);

// Assert
Assert.Multiple(() =>
{
Assert.That(periodList, Has.Count.EqualTo(2));
Assert.That(periodList[0].StartTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 0, 0, 0, "UTC")));
Assert.That(periodList[0].EndTime, Is.EqualTo(new CalDateTime(2025, 1, 1, 1, 0, 0, "UTC")));
Assert.That(periodList[1].StartTime, Is.EqualTo(new CalDateTime(2025, 1, 2, 0, 0, 0, "UTC")));
Assert.That(periodList[1].EndTime, Is.EqualTo(new CalDateTime(2025, 1, 2, 1, 0, 0, "UTC")));
Assert.That(periodList.IsReadOnly, Is.EqualTo(false));
});
}

[Test]
public void InsertAt_ShouldInsertPeriodAtCorrectPosition()
{
// Arrange
var periodList = new PeriodList();
var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), Duration.FromHours(1));
var period2 = new Period(new CalDateTime(2025, 1, 2, 0, 0, 0), Duration.FromHours(1));
var period3 = new Period(new CalDateTime(2025, 1, 3, 0, 0, 0), Duration.FromHours(1));
periodList.AddPeriod(period1).AddPeriod(period3);

// Act
periodList.Insert(1, period2);

// Assert
Assert.Multiple(() =>
{
Assert.That(periodList, Has.Count.EqualTo(3));
Assert.That(periodList[1], Is.EqualTo(period2));
});
}

[Test]
public void RemoveAt_ShouldRemovePeriodAtCorrectPosition()
{
// Arrange
var periodList = new PeriodList();
var period1 = new Period(new CalDateTime(2025, 1, 1, 0, 0, 0), Duration.FromHours(1));
var period2 = new Period(new CalDateTime(2025, 1, 2, 0, 0, 0), Duration.FromHours(1));
var period3 = new Period(new CalDateTime(2025, 1, 3, 0, 0, 0), Duration.FromHours(1));
periodList.AddPeriod(period1).AddPeriod(period2).AddPeriod(period3);

// Act
periodList.RemoveAt(1);

// Assert
Assert.Multiple(() =>
{
Assert.That(periodList, Has.Count.EqualTo(2));
Assert.That(periodList[1], Is.EqualTo(period3));
});
}
}
Loading

0 comments on commit 753e478

Please sign in to comment.