p.a.event: Cannot compare offset-naive and offset-aware datetimes

[sneridagh]

On the deserializer:

  Module plone.restapi.services.content.update, line 22, in reply
  Module plone.restapi.deserializer.dxcontent, line 78, in __call__
TypeError: can't compare offset-naive and offset-aware datetimes

In fact, you cannot feed a PATCH request with the GET response of the same object... it's broken with the same error.

[sneridagh]

Here we are flattening the TZ information of any Datetime:
https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/deserializer/dxfields.py#L46

@buchi @lukasgraf @tisto There is any reason for this? This is what is causing that the later comparision when doing a PATCH fails for p.a.event (since it forces it to have TZ):

What would be the best approach in this case?

[tisto]

@sneridagh I think we should create a test class that makes sure all content objects we GET from the API can be used in a POST to create an object.

@thet opinions on the datetime representation on plone.restapi?

[thet]

@tisto @sneridagh I'm glad you asked :)

I suggest representing dates as ISO8601 string converted to UTC and eventually transmitting the display timezone as a string in a separate field.

Example: 2017-03-08T15:18:52+00:00

But since plone.app.event 2.0, event type start and end dates are stored with their original timezone information and the timezone is a portal wide setting, which can be overwritten by user settings. So the target timezone can also be set as a site wide setting by the API client. However, the API client needs to know about a "default timezone" to display times.

Regarding timezones : The ISO representation only knows timezone offsets from UTC, which is not very helpful. E.g. you cannot know, which standard and daylight saving time rules apply for the offset. For that, you need to know the timezone name, like "Europe/Vienna".
But when we transmit dates converted to UTC and let the client/server decide into which zone to convert, this isn't actually a big deal.

JavaScript

// Parsing
var dt = new Date('2017-03-08T15:18:52+00:00')

// Serializing
dt = dt.toISOString()  // '2017-03-08T15:18:52.000Z'

// better bring in same format like python datetime isoformat and W3C examples
dt = dt.substring(0, dt.length-5) + '+00:00'  // '2017-03-08T15:18:52+00:00'

Python

from dateutil.parser import parse
dt = parse('2017-03-08T15:18:52.000Z')
# dt is now: datetime.datetime(2017, 3, 8, 15, 18, 52, 000000, tzinfo=tzutc())

from plone.app.event.base import default_timezone
from plone.event import utils

tz = default_timezone()  # E.g.: 'Europe/Vienna'
dt = utils.dt_to_zone(dt, tz)
# dt is now: datetime.datetime(2017, 3, 8, 16, 18, 52, tzinfo=<DstTzInfo 'Europe/Vienna' CET+1:00:00 STD>)

iso = dt.isoformat()  # '2017-03-08T16:18:52+01:00'

# Converting to utc before
dt = utils.utc(dt)
# dt is now: datetime.datetime(2017, 3, 8, 15, 18, 52, tzinfo=<UTC>)

dt.isoformat()  # '2017-03-08T15:18:52+00:00'

More info

• https://en.wikipedia.org/wiki/ISO_8601#Time_zone_designators
• https://www.w3.org/TR/NOTE-datetime
• https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString
• https://dateutil.readthedocs.io/en/stable/parser.html

[thet]

I just learned, that there is a JavaScript toJSON function on Date objects: http://stackoverflow.com/a/15952652/1337474 & https://xkcd.com/1179/
JavaScript:

dt = new Date('2017-03-08T15:18:52+00:00')
dt.toJSON()  // '2017-03-08T15:18:52.000Z'

Which can be easily parsed with python-dateutil:

parse('2017-03-08T15:18:52.000Z')  # datetime.datetime(2017, 3, 8, 15, 18, 52, tzinfo=tzutc())

Maybe you should just use these and replace the +00:00 with Z when serializing to JSON.

[sneridagh]

@thet Thanks for your advice!

[sneridagh]

@buchi we need some information about why you stripped the TZ info in the deserializer of any DataTime:
https://github.com/plone/plone.restapi/blob/master/src/plone/restapi/deserializer/dxfields.py#L46

Do you remember why we are doing this?

[buchi]

@sneridagh As far as I remember datetimes are stored without timezone in dexterity content types. At least for standard fields like effective, modified, etc. I need to check that in detail and will give you more information later (probably tomorrow).

[thet]

@sneridagh @buchi I would not strip the timezone information from the datetime object and make it a timezone-naive datetime that way. IMO thats wrong. Also, normally there are no naive DateTimes in Plone - even if they are only UTC/GMT offsets:

from DateTime import DateTime
DateTime()  # DateTime('2017/03/09 23:50:35.733336 GMT+1')
self.context.effective_date  # DateTime('2017/03/09 23:40:00 GMT+1')
self.context.effective_date.asdatetime()  # datetime.datetime(2017, 3, 9, 23, 40, tzinfo=<StaticTzInfo 'GMT+1'>)
DateTime(self.context.effective_date.asdatetime())  # DateTime('2017/03/09 23:40:00 GMT+1')

pytz also knows about those GMT offsets.

I suggest to not use naive datetimes, which lead to exactly the failure @sneridagh was reporting here.
I also suggest to look at the date/time/timezone helper methods in https://github.com/plone/plone.event/blob/master/plone/event/utils.py

e.g. plone.event.utils.pydt creates a timezone aware datetime of any DateTime or datetime object.

[buchi]

@thet @sneridagh I would also not strip the timezone information, but unfortunately that's what Plone 5 does by default. If you create e.g. an event in a Plone 5 site through the web interface and look at it in the debug console you get the following:

>>> IDublinCore(obj).effective
datetime.datetime(2017, 3, 10, 14, 0)
>>> IEventBasic(obj).start
datetime.datetime(2017, 3, 14, 17, 30, tzinfo=<DstTzInfo 'Europe/Berlin' CET+1:00:00 STD>)

The effective date doesn't have a timezone info while the start date has one (╯°□°)╯︵ ┻━┻

The intention is to have the same behavior in the API as when using the web interface to create/edit objects but I wasn't aware that p.a.event does have tz info.

The problem now is that the zope.schema's datetime field itself doesn't contain any information how tz information is handled (offset-naive or offset-aware). This is something that's managed by the widget I guess. So I'm not yet sure how we want to determine if we need to deserialize into a datetime with or without tz info. But we need to handle that differently because it would break Plone's web interface I guess if effective would contain tz info or start would not contain tz info.

I'm open to suggestions, but the #258 does not fix that in a clean way. I need to think more about that and will come back later...

[thet]

@buchi you are right:
for effective and expired dates, the timezone information of the datetime object is ignored. Instead it's assumed it's an timezone naive datetime object and converted to a DateTime object with the server's timezone:
https://github.com/plone/plone.app.dexterity/blob/master/plone/app/dexterity/behaviors/metadata.py#L324

IMO that's conceptionally wrong and needs to be fixed upstream.

A fix could be using plone.event.pydt and even storing the value as datetime instead a DateTime or at least taking the original timezone information into account, if there was one.
This is related to: plone/Products.CMFPlone#1298

/cc @jensens

[jensens]

We really should get rid of Zope DateTime and use Python datetime with timezone here. Same for created/modified dates.

Well, this needs a migration step is some work.

It needs a PLIP and may go into Plone 5.2 or 6.0.

[mr-rob]

Does anyone have a time frame until when it's possible to post/patch datetime values?

[sneridagh]

@tisto @buchi @lukasgraf @thet @jensens @csenger
It's been a while since I'm trying to fix this in a reasonable way. These are my findings to the point.

After reviewing your opinions, finally decided to store the dates using the time zone everywhere while using p.restapi. The results are in #394. As you can see, it works for Plone 4 (with some caveats that should be addressed and tested before releasing as it is) but they fail miserably in Plone 5.

The reason of failing in Plone 5 is because of the implementation of how the timezones are handled in p.a.event are different. So different that they are completely irreconciliables if we want to make p.restapi work for both versions...

We had this issue before but we managed to overcome this differences so far. This time, we have to find a way to branch the code detecting if we are in Plone 4 or 5 for code and for tests.

Any elegant idea on how to do that? Any other idea on how to overcome this?

If we don't fix that, p.a.event Events are broken in p.restapi as:

• You can't patch (update) any Event since the comparision between the existing and the new date fails (the infamous offset-naive and offset-aware error)
• On getting the start and end times, the TZ offsets are wrong

[thet]

What exactly is the problem?

If you serialize dates converted to UTC and de-serialize to datetime objects, using the pytz UTC zone (or any other) or possibly convert the UTC datetime to the portals default zone then you shouldn't run into timezone-naive/timezone aware comparison problems.

plone.app.event.base and plone.event.utils provide a lot of helper functions to deal with that.

However, if there isn't a datetime data exchange standard for datetime objects in JSON which preserves the real zone - by that I mean the name of the timezone, not the offset - then there is something missing in the internets.

[sneridagh]

Problem is that the timezone offsets are computed differently.

Plone 4: timezone is a schema property, so you can set it to "Europe/Berlin" then the offset is computed using it.

Plone 5: timezone is no longer a schema property, but a method in the IEventAccessor adapter computing the offset given the start and end datetimes own offset.

The result of what is stored is basically different in both versions, probably the UI is adapting those to make them look the same.

[thet]

Ah, yes.

In plone.app.event 1.x we converted all dates to UTC and stored the timezone separately. That was not practical and counter-intuitive, as we also had to postprocess forms.

If you access start/end datetimes via the IEventAccessor, you get those with the correct timezone already applied.
For setting datetimes in plone.app.event 1.x you have to set the timezone, the dates (wether as local datetimes or with the correct timezone applied) and then throw the object modified event so that data_postprocessing is done. See: https://github.com/plone/plone.app.event/blob/1.2.x/plone/app/event/dx/behaviors.py#L460

In plone.app.event 2.x we store dates with their timezone appllied directly without modifying. This turned out to be much more practical and intuitive to work with.
So you always have the datetimes with the correct timezone applied whether you access it directly on the context, via the behavior adapter or the IEventAccessor adapter.

You probably have to maintain two code paths in order to support plone.app.event 1.x (Plone 4) and 2.x (Plone 5) or use an adapter against event objects which cares about the differences for you.

You can also use plone.app.event 2.0 in Plone 4, if that is an option. I do that for an other project.