search

OrchardCMS / OrchardCore

Orchard Core is an open-source modular and multi-tenant application framework built with ASP.NET Core, and a content management system (CMS) built on top of that framework.
https://orchardcore.net
BSD 3-Clause "New" or "Revised" License
7.42k stars 2.4k forks source link

Datetime field cannot save value #10574

Open tropcicstefan opened 3 years ago

tropcicstefan commented 3 years ago

After returning back to this, I am certain this is a bug DateTimeField isn't able to save datetime value because of how LocalClock.ConvertToUtcAsync is implemented.

Specifically AtStrictly at line 62 https://github.com/OrchardCMS/OrchardCore/blob/387ded1a6429fbe10f8adfa941a33c4704395654/src/OrchardCore/OrchardCore/Modules/Services/LocalClock.cs#L62

Problem is NodaTime throws AmbiguousTimeException when using time value that is in period of change. As stated in discussion, for instance at UTC+2 Warsaw try enter 2021-10-31 2:05 AM.

One way to fix this is to change AtStrictly to AtLeniently but I am not certain of possible consequences throughout framework.

Originally posted by @tropcicstefan in https://github.com/OrchardCMS/OrchardCore/discussions/10535

Skrypt commented 3 years ago

I'm sorry, I did not have time to look at this issue but I think we should keep only one issue open for this.

Skrypt commented 3 years ago

You are right, and the issue is that we only store the UTC DateTime. This causes issues for when we try to convert a LocalDateTime which is in a saving daylight period. It makes this UTC DateTime ambiguous on whether it should be saved with the daylight saving time offset or not. Here, a daylight saving time can be 30 minutes in some regions, which creates this ambiguity. So, here, I need to see what does AtLenently allows and how it behaves. If you can make a comparison and show me the results that would save me time. Thanks.

Here, what Jon Skeet suggest, it is to use MapLocal(LocalDateTime):

MapLocal(LocalDateTime) will return a ZoneLocalMapping with complete information about whether the given local time occurs zero times, once or twice. This is the most fine-grained approach, which is the fiddliest to use but puts the caller in the most control.

Though, how would we store a ZoneLocalMapping in the database? https://nodatime.org/1.2.x/api/NodaTime.TimeZones.ZoneLocalMapping.html

Maybe, one solution would be to always store the DateTime value as UTC AND without daylight saving time offset. This way, we could keep the .AtStrictly(DateTime).

tropcicstefan commented 3 years ago

If you can make a comparison and show me the results that would save me time.

From noda documentation at the bottom under title Lenient resolver changes

For ambiguous values:

It now returns the earlier of the two possible instants. For example, if 01:00 is ambiguous, it used to return 1:00 standard time and it now will return 01:00 daylight time.

For skipped values:

It now returns the instant that would have occurred if the gap had not existed. This corresponds to a local time that is shifted forward by the duration of the gap. For example, if values from 02:00 to 02:59 were skipped, a value of 02:30 used to return 03:00 and it will now return 03:30.

This article is something to consider also

Skrypt commented 3 years ago

@tropcicstefan Now can we concentrate on this one?

Piedone commented 5 months ago

@tropcicstefan would you like to get back here?