Skip to content

Commit

Permalink
Refactor to use DateOnly and TimeOnly for date/time handling (ical-or…
Browse files Browse the repository at this point in the history
…g#658)

Increased `CalDateTime` immutability

CalDateTime:
* `Date` is of type `DateOnly` (breaking vs. v4)
* Make `UtzTzId` a public string
* Remove `DateOnlyValue`
* Remove `HasDate` (always true) (breaking vs. v4)
* Remove `Millisecond` (breaking vs. v4)
* Remove `Ticks` (breaking vs. v4)
* Remove overload CalDateTime(int year, int month, int day, string? tzId) (breaking vs. v4)
* `HasTime` only has a getter (breaking vs. v4)
* `Time` is a getter of type `TimeOnly`. Values are always rounded to the nearest second (breaking vs. v4)
* `AddMilliseconds` takes `double` as argument, like `DateTime.AddMilliseconds` (breaking vs. v4)
* Remove `TimeOnlyValue`
* `Value` only has a getter (breaking vs. v4)
* `TzId` only has a getter (breaking vs. v4)
* Remove `AsSystemLocal` (breaking vs. v4)
* Remove `AsDateTimeOffset` (breaking vs. v4)
* Remove `AddMilliseconds` (breaking vs. v4)
* Remove `AddTicks` (breaking vs. v4)
* Remove unused `TimeSpan? operator -(CalDateTime? left, IDateTime? right)` (breaking vs. v4)
* Update Equality operators for comparing to floating date/time (breaking vs. v4)

IDateTime (breaking vs. v4):
* Remove `AsSystemLocal` (breaking vs. v4)`
* Remove `AsDateTimeOffset` (breaking vs. v4)`
* Remove `AddMilliseconds` (breaking vs. v4)
* Remove `AddTicks` (breaking vs. v4)
* Remove `Millisecond` (breaking vs. v4)
* Remove `Ticks` (breaking vs. v4)
* Add explicit bool IsFloating (implicit TzId == null)
* `Date` is a getter of type `DateOnly`
* `Time` is a getter of type `TimeOnly`
* `HasTime` only has a getter
* Remove `HasDate` (always true) (breaking vs. v4)
* `Value` only has a getter
* `TzId` only has a getter
* `ToTimeZone(string?)` argument is a NRT

Miscellaneous:
* CTORs with `DateTime` arguments persist. Actually they are also comfortable, and now consistently processed
* Keep `IDateTime` - maybe in another PR
* Transitioned from DateTime to DateOnly and TimeOnly across the codebase for improved date and time handling.
* Updated methods, properties, and tests to ensure compatibility with the new approach.
* Updated xmldoc

Resolves ical-org#656
Resolves ical-org#662 

* Remove AsDateTimeOffset from IDateTime and CalDateTime

* Floating date/time can convert to any timezone ID, keeping Value unchanged

Remove fallback to system's local timezone for floating date/time

* Added unit test GetOccurrenceShouldExcludeDtEndZoned

Resolves ical-org#660

* Fix: Initialize CalDateTime with timezone UTC only, if TimeOnly is not null

Reasoning: DATE cannot have a timezone

* Refactor time handling and remove millisecond/ticks methods

* Updated CalDateTime to truncate time to seconds instead of rounding.
* Removed AddMilliseconds and AddTicks methods and related test cases.
* Fix: minus and plus operators for cases where time parts are equal
* Adjusted EndOfDay method to use AddSeconds(-1) instead of AddTicks(-1)
* Removed GetOccurrenceShouldExcludeDtEndZoned test.

* Remove setter from `CalendarEvent.AllDay`

* Made `IsAllDay` in `CalendarEvent.cs` read-only,
* Updated `Issue432_AllDay` unit test in `RecurrenceTests.cs` to remove time component from `Start` property and eliminate setting the `IsAllDay` property. (The only place where the setter was used.)
* Simplified `CalDateTime.cs` initialization by removing condition related to `UtcTzId` when `timeOnly` is not provided.

* Ensure transitive equality for CalDateTime

* `operator ==`: if (left.IsFloating != right.IsFloating) return false;
* Add unit test

* Remove properties `Ticks` and `Millisecond`

* Remove redundant if clause

* Simplify end period creation in GetOccurrences method

Removed the `endTimeOnly` variable and its usage in the `GetOccurrences` method. The code now directly creates the `CalDateTime` object for the end of the period without adjusting the time component. This change streamlines the code and avoids potential issues related to time adjustments.

* Simplify GetOccurrences methods by removing time adjustments

The `GetOccurrences` methods in `Calendar.cs` have been simplified
by removing the code that adjusted the end time by subtracting one
second or one tick. The methods now directly use the start date
and the date one day after the start date without adjusting the
time component. This change simplifies the logic and removes
unnecessary time adjustments.

The adjustments are redudant, because
public static HashSet<Occurrence> RecurrenceUtil.GetOccurrences(IRecurrable recurrable, IDateTime periodStart, IDateTime periodEnd, bool includeReferenceDateInResults)
uses precise "LessThan" for comparing end date/times

Resolves  ical-org#662
  • Loading branch information
axunonb authored Dec 6, 2024
1 parent add58a9 commit 71a4831
Show file tree
Hide file tree
Showing 21 changed files with 612 additions and 677 deletions.
285 changes: 140 additions & 145 deletions Ical.Net.Tests/CalDateTimeTests.cs

Large diffs are not rendered by default.

16 changes: 8 additions & 8 deletions Ical.Net.Tests/CalendarEventTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -478,8 +478,8 @@ public void TestGetEffectiveDuration()

var evt = new CalendarEvent()
{
DtStart = new CalDateTime(now) { HasTime = true },
DtEnd = new CalDateTime(now.AddHours(1)) { HasTime = true },
DtStart = new CalDateTime(DateOnly.FromDateTime(now), TimeOnly.FromDateTime(now)),
DtEnd = new CalDateTime(DateOnly.FromDateTime(now.AddHours(1)), TimeOnly.FromDateTime(now.AddHours(1)))
};

Assert.Multiple(() =>
Expand All @@ -491,8 +491,8 @@ public void TestGetEffectiveDuration()

evt = new CalendarEvent()
{
DtStart = new CalDateTime(now.Date) { HasTime = true },
DtEnd = new CalDateTime(now.Date.AddHours(1)) { HasTime = true },
DtStart = new CalDateTime(DateOnly.FromDateTime(now.Date), TimeOnly.FromDateTime(now.Date)),
DtEnd = new CalDateTime(DateOnly.FromDateTime(now.Date.AddHours(1)), TimeOnly.FromDateTime(now.Date.AddHours(1)))
};

Assert.Multiple(() =>
Expand All @@ -503,7 +503,7 @@ public void TestGetEffectiveDuration()

evt = new CalendarEvent()
{
DtStart = new CalDateTime(now.Date) { HasTime = false },
DtStart = new CalDateTime(DateOnly.FromDateTime(now)),
};

Assert.Multiple(() =>
Expand All @@ -514,7 +514,7 @@ public void TestGetEffectiveDuration()

evt = new CalendarEvent()
{
DtStart = new CalDateTime(now) { HasTime = true },
DtStart = new CalDateTime(DateOnly.FromDateTime(now), TimeOnly.FromDateTime(now)),
Duration = TimeSpan.FromHours(2),
};

Expand All @@ -526,7 +526,7 @@ public void TestGetEffectiveDuration()

evt = new CalendarEvent()
{
DtStart = new CalDateTime(now.Date) { HasTime = true },
DtStart = new CalDateTime(DateOnly.FromDateTime(now.Date), TimeOnly.FromDateTime(now.Date)),
Duration = TimeSpan.FromHours(2),
};

Expand All @@ -537,7 +537,7 @@ public void TestGetEffectiveDuration()

evt = new CalendarEvent()
{
DtStart = new CalDateTime(now.Date) { HasTime = false },
DtStart = new CalDateTime(DateOnly.FromDateTime(now)),
Duration = TimeSpan.FromDays(1),
};

Expand Down
27 changes: 1 addition & 26 deletions Ical.Net.Tests/ComponentTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,29 +22,4 @@ public void UniqueComponent1()
Assert.That(evt.Created, Is.Null); // We don't want this to be set automatically
Assert.That(evt.DtStamp, Is.Not.Null);
}

[Test, Category("Components")]
public void ChangeCalDateTimeValue()
{
var e = new CalendarEvent
{
Start = new CalDateTime(2017, 11, 22, 11, 00, 01),
End = new CalDateTime(2017, 11, 22, 11, 30, 01),
};

var firstStartAsUtc = e.Start.AsUtc;
var firstEndAsUtc = e.End.AsUtc;

e.Start.Value = new DateTime(2017, 11, 22, 11, 30, 01);
e.End.Value = new DateTime(2017, 11, 22, 12, 00, 01);

var secondStartAsUtc = e.Start.AsUtc;
var secondEndAsUtc = e.End.AsUtc;

Assert.Multiple(() =>
{
Assert.That(secondStartAsUtc, Is.Not.EqualTo(firstStartAsUtc));
Assert.That(secondEndAsUtc, Is.Not.EqualTo(firstEndAsUtc));
});
}
}
}
4 changes: 2 additions & 2 deletions Ical.Net.Tests/DeserializationTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,8 @@ public void Google1()
var evt = iCal.Events["594oeajmftl3r9qlkb476rpr3c@google.com"];
Assert.That(evt, Is.Not.Null);

IDateTime dtStart = new CalDateTime(2006, 12, 18, tzId);
IDateTime dtEnd = new CalDateTime(2006, 12, 23, tzId);
IDateTime dtStart = new CalDateTime(2006, 12, 18);
IDateTime dtEnd = new CalDateTime(2006, 12, 23);
var occurrences = iCal.GetOccurrences(dtStart, dtEnd).OrderBy(o => o.Period.StartTime).ToList();

var dateTimes = new[]
Expand Down
10 changes: 5 additions & 5 deletions Ical.Net.Tests/ProgramTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,8 @@ public void Merge1()

// Get occurrences for the first event
var occurrences = evt1.GetOccurrences(
new CalDateTime(1996, 1, 1, _tzid),
new CalDateTime(2000, 1, 1, _tzid)).OrderBy(o => o.Period.StartTime).ToList();
new CalDateTime(1996, 1, 1),
new CalDateTime(2000, 1, 1)).OrderBy(o => o.Period.StartTime).ToList();

var dateTimes = new[]
{
Expand Down Expand Up @@ -104,8 +104,8 @@ public void Merge1()

// Get occurrences for the 2nd event
occurrences = evt2.GetOccurrences(
new CalDateTime(1996, 1, 1, _tzid),
new CalDateTime(1998, 4, 1, _tzid)).OrderBy(o => o.Period.StartTime).ToList();
new CalDateTime(1996, 1, 1),
new CalDateTime(1998, 4, 1)).OrderBy(o => o.Period.StartTime).ToList();

var dateTimes1 = new[]
{
Expand Down Expand Up @@ -180,4 +180,4 @@ public void SystemTimeZone3()
}, Throws.Nothing, "Time zone should be found.");
}
}
}
}
Loading

0 comments on commit 71a4831

Please sign in to comment.