feat: Add evaluation layer to CalDateTime

[axunonb]

Add evaluation layer to CalDateTime (WIP!)

• Introduced CalDateTimeExtensions for extension methods on CalDateTime
• Moved CalDateTime arithmetic and conversion methods to CalDateTimeExtensions
• Adjusted related classes with new references

Note:

• CalDateTimeExtensions.ToTimeZone links the input CalDateTime with its ZonedDateTime representation, using a cache with weak references to CalDateTime. The cache is implemented with ConditionalWeakTable<CalDateTime, ZonedDateTime>.
• This way, once a CalDateTime is resolved to a ZonedDateTime, the ZonedDateTime will be used for further conversions.

Why cache ZonedDateTime?

• If a CalDateTime is instantiated from a lenient timezone conversion, this leniently determined value must not be converted again.
• Lenient conversions resolve non-existing or ambiguous times caused by DST transitions.

Resolves #737
Resolves #677

[codecov]

Codecov Report

Attention: Patch coverage is 93.58974% with 5 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
Ical.Net/DataTypes/CalDateTime.cs	62%	1 Missing and 2 partials ⚠️
Ical.Net/Evaluation/CalDateTimeExtensions.cs	97%	1 Missing and 1 partial ⚠️

❌ Your project status has failed because the head coverage (67%) is below the target coverage (80%). You can increase the head coverage or adjust the target coverage.

@@         Coverage Diff         @@
##           main   #805   +/-   ##
===================================
  Coverage    67%    67%           
===================================
  Files       106    107    +1     
  Lines      4430   4448   +18     
  Branches   1065   1069    +4     
===================================
+ Hits       2954   2976   +22     
+ Misses     1039   1038    -1     
+ Partials    437    434    -3     

Files with missing lines	Coverage Δ
Ical.Net/CalendarComponents/VTimeZone.cs	73% <ø> (ø)
Ical.Net/CalendarExtensions.cs	100% <ø> (ø)
Ical.Net/DataTypes/Duration.cs	77% <ø> (ø)
Ical.Net/DataTypes/Period.cs	85% <100%> (ø)
Ical.Net/DataTypes/Trigger.cs	44% <ø> (ø)
Ical.Net/Utility/DateUtil.cs	44% <100%> (ø)
Ical.Net/Evaluation/CalDateTimeExtensions.cs	97% <97%> (ø)
Ical.Net/DataTypes/CalDateTime.cs	87% <62%> (-<1%)	⬇️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[axunonb]

@minichma As long as we have CalDateTime ToTimeZone for the conversion this might be a simple fix for #737

[minichma]

@axunonb Not sure we should track the precise instant in CalDateTime. I.e. CalDateTime represents values from the ICAL format, which just doesn't provide any means of resolving this ambiguity. By tracking the instant (and the exact time zone object), we lose fundamental properties like equality after a serialization/deserialization round trip. I think to address #737 we should either represent recurrences via a data type other than Period, i.e. one that allows unambiguously tracking the end date (e.g. by specifying a TimeSpan rather than a end). Alternatively we could be waiting for what we discussed regarding v6, where we could consider separating the model layer from the evaluation layer. In the model layer CalDateTime would represent an ICAL date/time value, but in the evaluation layer we could work with ZonedDateTime. I feel resolving #805 is not worth this change.

[axunonb]

In the model layer CalDateTime would represent an ICAL date/time value, but in the evaluation layer we could work with ZonedDateTime.

If your time allows, I'd be interested in your opinion on this kind of "evaluation layer" which comes as extension methods for CalDateTime. FYI: Over all unit tests, were getting about 40% cache hits for CalDateTime to ZonedDateTime, and a bit more for GetOccurrences only.

TBD:

• Part of the operators make use of the CalDateTimeExtensions - move to the extensions?
• Remove IFormattable from CalDateTime? Or just remove the zone-aware ZonedDateTime-aware implementation?

recurrences via a data type other than Period

Needs to be covered separately.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[minichma]

@axunonb I love all the evaluation/arithmetics code being moved out of CalDateTime into extension methods, very nice! I'm not convinced by the caching code though for several reasons.

The outcome is very hard to reason upon. Consider the following test case:

    [Test]
    public void CalDateTime_AmbiguousTime_ResolvedCorrectly2()
    {
        var dt1 = new CalDateTime(2024, 10, 27, 1, 30, 0, "UTC").ToTimeZone("CET");
        var dt2 = new CalDateTime(2024, 10, 27, 2, 30, 0, "CET");

        // This is a fundamental assumption
        Assert.That(dt1 == dt2, Is.EqualTo(dt1.AsUtc() == dt2.AsUtc()));
    }

It's not quite obvious that this will fail. Or this one:

    [Test]
    public void CalDateTime_AmbiguousTime_ResolvedCorrectly()
    {
        TimeZoneResolvers.TimeZoneResolver = _ => NodaTime.DateTimeZoneProviders.Tzdb.GetZoneOrNull("UTC");

        var dt1 = new CalDateTime(2024, 10, 27, 1, 30, 0, "CET");
        var dt2 = new CalDateTime(2024, 10, 27, 1, 30, 0, "CET");
        var _ = dt1.AsUtc();

        TimeZoneResolvers.TimeZoneResolver = TimeZoneResolvers.Default;

        Assert.That(dt1, Is.EqualTo(dt2));
        Assert.That(dt1.AsUtc(), Is.EqualTo(dt2.AsUtc()));
    }

Its not obvious that the first assertion will succeed but the second will fail (clearly this is not a typical case, it rather shows the problems arising from TimeZoneResolvers.TimeZoneResolver being a global static).

By attaching the converted value via a cache does still attach it so the problems as outlined in my previous comment still exist but with the external cache its even harder to understand what's happening.

Regarding the serialization round-trip, the following test will still fail:

    [Test]
    public void CalDateTime_AmbiguousTime_ResolvedCorrectly3()
    {
        var dt1 = new CalDateTime(2024, 10, 27, 1, 30, 0, "UTC").ToTimeZone("CET");

        var serializer = new DateTimeSerializer();
        var stream = new System.IO.MemoryStream();
        serializer.Serialize(dt1, stream, System.Text.Encoding.UTF8);
        stream.Position = 0;
        var restored = ((CalDateTime)serializer.Deserialize(stream, System.Text.Encoding.UTF8)).ToTimeZone("CET");

        Assert.That(dt1.AsUtc(), Is.EqualTo(restored.AsUtc()));
    }

The evaluation layer as I was thinking about it would be a completely independent set of classes. They might be allowed to reference the model layer but not the other way round. The model layer would just represent the ICAL model as it is but without any evaluation/arithmetic operations on it, including adding, converting, ... Only after conversion to the evaluation layer all the magic would be available. The date-time classes there would probably operate on something like ZonedDateTime, including the concrete instant they represent. Then the ambiguity would be gone.

However, in the meantime it would be quite straight-forward to return something else than Period from GetOccurrences() et al. I.e. we could easily introduce a new type that doesn't contain an end time but only a TimeSpan representing the exact duration, instead. Then there wouldn't be any ambiguity either.

[axunonb]

Closed in favor of #809