1

Resolved

DateTime field; default Time always on 12:00AM!

description

When using the field: 'DateTime' (defined as date only!) the time is always set as 12 mid-day i.e. 1200(24h)!
see below record (it shows correctly 10:00 because I'm on summertime and GMT+1):

Orchard_Framework_ContentItemVersionRecord: <dateend>2014-07-21T10:00:00.0000000Z</dateend> -->(24h!)

When using this 'DateTime' field in a query the day is gone at noon that day !!!!!

I can repeat it! I help myself now and use also the time option to be able to set the time at 2400/0000 (24h)! which then ends the day correctly.
discussion see '522398'
thanks ed

comments

Decorum wrote Jul 23 at 2:25 PM

I will investigate.

edikaufmann wrote Jul 23 at 2:33 PM

thanks for already attacking it! Another crazy thing I observe ... but this might not be in the same context:

when trying to add 2400 or 0000 for that matter I observe:
using 24:00: is not accepted
using 0000: makes GMT+2 22:00 but on the previous day - crazy!!
PS: this needs some more testing!?

Decorum wrote Jul 23 at 7:21 PM

I have investigated and come to the conclusion that I do not consider this to be a bug in DefaultDateServices after all. The reason is quite complicated but I will try to explain it.

Orchard has no such thing as a pure date field storage (i.e. without a time component). All dates are stored as DateTime values in UTC time zone, and these always include a time component.

The Orchard DateTime field on the other hand has a date-only mode. When storing that date, some value must be assumed for the time component. Later when some piece of code retrieves the value, it does not necessarily know that it's part of a date-only field, so the time component will be "activated".

Combined with time zone conversion, this causes some issues. Imagine the following two possible implementations and their result:
  1. The implementation you are requesting: Let's say you are in the time zone UTC-3. You select the date 2014-05-05, Orchard ignores the time zone conversion and simply stores it as 2014-05-05 00:00:00. Later some code retrieves the value and wants to manage it in the user's locale and time zone, and so it gets converted to 2014-05-04 21:00:00. Extremely bad - the date component has mutated!
  2. The current implementation: Let's say you are in the time zone UTC+2. You select the date 2014-05-05 and Orchard infers 12:00:00 for the date component, converts it to UTC and hence stores it as 2014-05-05 10:00:00. Later some code retrieves the value and wants to manage it in the user's locale and time zone, and so it gets converted to 2014-05-05 12:00:00. Not as bad as the above scenario - at least the date component is intact.
The only value for the time component that ensures that a DateTime always survives time zone conversion without mutating the date component is 12:00. This is the reason why DefaultDateServices uses this value for the time component if no time component is specified when converting a date from a string in local format and local time zone, to a stored DateTime in UTC.

So, in my opinion, it must be up to the code consuming the DateTime field to know that it should ignore the time component. I think this is reasonable, because the consuming code must anyway possess the knowledge of whether a certain date-only field should be treated as inclusive or exclusive. For example, if you have two date-only fields, MinDate and MaxDate, and you want to query on that interval, the MaxDate should be treated as MaxDate+1d for the purposes of the query. The querying code must know this. So it stands to reason, that code must also know to discard any time component of the DateTime.

In any case, even if we should want to change this behavior in Orchard, IDateServices is not the place to change this logic because it does not deal with date-only values. If anything, the driver for DateTimeField should be responsible for doing conversion and storage with respect to the fact that the field is date-only. One option would be to have DateTimeFieldDriver completely bypass time zone conversion when the field is in date-only mode, and always store and provide 00:00:00 for the time component. But this assumes that the database column are only read through the DateTimeField abstraction, and never directly by code that does not know about the date-only aspect. I will investigate and see if this would be possible to do in a non-breaking way.

edikaufmann wrote Jul 24 at 6:01 AM

well, I obviously opened a can of worms! I certainly appreciate all your thoughts and realize (again) that all this DateTime business is complex. I certainly cannot contribute substantially to a solution - I fear: only moral support!

One thought though:
today I help myself by using the 'Date AND Time' option - i.e. I set it explicitly (mostly 23:59 (24h) or 11:59PM) to 'end' the day.

In other words; DO NOT even offer the 'Date only' option at all and change the 'Time' when absolutely needed e.g. in a query!
I think this would be a rather small restriction - after all - if you are dependent on the 'Time' (e.g. in a query) you have to set it anyway - otherwise the default setting12:00(24h) doesn't hurt.! In the end it might be a documentation issue only!

PS: The annoying thing was - in the 'Date only' option - NOT to know that the time is set to 12:00 (24h).

MY Bottom line: now where I know (the implementation) I certainly can live with it!
Decorum, let me know if I can be of any additional (philosophical ..) help
regards ed

JasperD wrote Jul 24 at 8:52 PM

@Daniel: About your first example with the changed date: Am I right that this is only problem when using a date time field without a (user-visible) time portion? I'm assuming that localized date/time is only used when displaying or entering a date and UTC is used in all other cases (internal stuff, not visible to users).

What do you think about replacing date only DateTime field with a new Date field that is just a date without time portion and time zone (pretty much what LocalDate in NodaTime is)? A value of such a date field wouldn't be comparable with a DateTime field, but I consider this a consequent restriction.

Decorum wrote Jul 24 at 11:32 PM

@Edi: Yes I have been thinking along the same lines - it's problematic to begin with to even offer a date-only value on a platform that lacks inherent support for it. But I'm afraid it would be too much of a braking change to remove it.

@Jasper: The problem with that approach is that there is no such data type in the .NET Framework. How do you represent it in memory? Inventing a new CLR type does not seem feasible, all the date management logic from System.DateTime would have to be reimplemented...

The more I think about this, the more I realize the right way to go is to handle this in the DateTimeField. Essentially, when in date-only mode, the DateTimeField should always store 00:00:00 for the time component, and never perform any time zone conversion. I think this can be implemented as a non-breaking change, without too much effort.

edikaufmann wrote Jul 25 at 5:44 AM

@Decorum: I understand what you are going at. Only one practical thing (e.g. for a casual 'Editor'). When setting the 'Time' (DateOnly mode) to 00:00:00 does that mean the 'Time' is not at all looked at (e.g. in a query)?
  • Does 00:00:00 represent the end-of-the-day?
  • Because TODAY in DateOnly mode the time is obviously looked at!
  • In the NEW implementation in DateOnly mode: is the time still looked at and, if yes: is 00:00:00 really the end of that day? Otherwise we have not achieved anything!
PS: that's why ALWAYS consciously setting the time was my proposal!
hope I did not confuse this thing even more
edi

Decorum wrote Jul 25 at 11:22 AM

@Edi: To answer your question, the storage of 00:00:00 as the time component is one thing, and the interpretation of that value in a query is anotheng thing. In fact the two are completely separate things.

When a date-only field is being used for example in a query, that query does not take into account whether it is a date-only field or not. It only sees a DateTime field with 00:00:00 for the time component. And that is the problem. When constructing a query you have to be aware. You also have to be aware of the semantic meaning of that date. Is it an end date? Is it a start date? Is it inclusive or exclusive? Those all depends on the semantic definition of the field. There is no correct generic interpretation.

The only sensible thing to do is to always store 00:00:00 in the time component. This way at least it's predictable and queries can know what to expect and how to filter.

Decorum wrote Jul 25 at 11:37 AM

I've determined that this requires significant refactoring of the IDateServices abstraction and its default implementation. This coincides with some other improvements that also need to be made regarding the support for non-gregorian calendars. It would be best to kill all those birds with one stone. I would not be comfortable doing this in 1.8.x so I'm going to create a feature branch for it from 1.x.

edikaufmann wrote Jul 25 at 3:13 PM

@decorum: thanks for the explanation. It's definitely more complex than I thought!
As said, now where I know today's implementation, I can live with it comfortably! That said I appreciate your effort even more, thanks.
.... knowing what I know now and appreciating the effort you have to put into a 'final' solution ... I probably would never have brought it up!
again thanks and happy coding, edi

JasperD wrote Jul 26 at 12:06 AM

@Daniel: It wasn’t my intention to propose the implementation of a new date and time lib, I just got a little bit off the track when I thought about NodaTime (which is in fact an alternative date and time implementation for .NET). What I wanted to suggest is to remove the date-only option (and consequently the time-only option) from the DateTime field and implement distinct date and time fields (irrespective of the underlying CLR type for now) in addition to the existing DateTime field since date and time (and datetime) are different things that should be treated in their own way (imho).

As a question: You mentioned that interpreting a DateTime field is fraught with problems since we do not know if is a date only field or not. Would it help to have different fields (even if they use the same CRL type) or do we just have the type (e.g. System.DateTime) at some point?

Further thought, if we would have distinct fields for Date, Time and DateTime and always know the field type, wouldn’t it be right not to compare these fields (their values) with each other?

Decorum wrote Jul 26 at 12:51 AM

@Jasper: Ah, I see. I was not aware of NodaTime - looks interesting, will take a look.
As a question: You mentioned that interpreting a DateTime field is fraught with problems since we do not know if is a date only field or not. Would it help to have different fields (even if they use the same CRL type) or do we just have the type (e.g. System.DateTime) at some point?
Well it would help to some extent I guess. But as I mentioned above, when querying, performing arithmetics and performing comparisons based on such a date, you in any case need to know the semantic meaning of the date in order to carry out meaningful operations on it. Given that need, we may as well also make consumers responsible for knowing whether a given field is date-only or not. So bottom line is, I don't think it would add much value to differentiate them by type.

We should however IMO surface information on whether a field is date-only or not, preferably as a property on the field itself. I intend to add this in the branch I created.
Further thought, if we would have distinct fields for Date, Time and DateTime and always know the field type, wouldn’t it be right not to compare these fields (their values) with each other?
I don't think so. For example, a common scenario is to find out if a certain point in time (date and time) is within a range of days (min date and max date).

Decorum wrote Aug 7 at 1:27 AM

This has now been fixed for the upcoming 1.9 release. More information here:
https://orchard.codeplex.com/discussions/560347