Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RecurrencePattern.Until should be of type IDateTime instead of DateTime #653

Open
axunonb opened this issue Nov 26, 2024 · 2 comments
Open

Comments

@axunonb
Copy link
Collaborator

axunonb commented Nov 26, 2024

Reasoning:
In the RecurrencePatternSerializer the RecurrencePattern.Until is necessarily converted to an IDateTime for serialization.
That's why it would be better to store it as an IDateTime from the beginning, instead of relying on RecurrencePattern.Until.Kind.

if (recur.Until != DateTime.MinValue)
{
    var serializer = factory.Build(typeof(IDateTime), SerializationContext) as IStringSerializer;
    if (serializer != null)
    {
        IDateTime until = new CalDateTime(recur.Until); // <<<<<<<<< The DateTimeKind is important here
        until.HasTime = true;
        values.Add("UNTIL=" + serializer.SerializeToString(until));
    }
}
@minichma
Copy link
Collaborator

Generally, using DateTime is a break in how date/time is represented throughout the library. In most (all?) other places, a date/time that represents a value from the RFC is implemented as IDateTime. Its not obvious, why this is different in the case of UNTIL. A reason might be, that UNTIL most often needs to be represented in UTC and sometimes as floating time (if DTSTART is floating), but never with timezone reference, so at first glance DateTime might seem sufficient. Nevertheless IDateTime aka CalDateTime would be the more natural fit, because

  • it can represent floating time vs UTC
  • it can represent DATE vs DATE-TIME
  • it is the way it is done throughout the lib
  • it doesn't have the ambiguity of DateTime, e.g. regarding DateTimeKind.Local.

@axunonb
Copy link
Collaborator Author

axunonb commented Nov 29, 2024

While serialization works (renders date/time as UTC), we can apply a fix to get the deserialized UTC value correctly
by replacing

if (dt != null)
{
r.Until = dt.Value;
}

with

r.Until = DateTime.SpecifyKind(dt.Value, dt.IsUtc ? DateTimeKind.Utc : DateTimeKind.Unspecified);

But it still means to rely on DateTimeKind.
This might also fix #406 at least partly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants