tomwalder / php-gds

Google Cloud Datastore Library for PHP
Apache License 2.0
164 stars 44 forks source link

Fixed issue in timezone being ignored by the protobuf mapper #169

Closed RobertG4M closed 4 years ago

RobertG4M commented 6 years ago

The createFromFormat function ignores the configured timezone when a timestamp is provided, and won't set the DateTime object to the current default timezone, unlike the other date functions.

This is inconsistent with the behaviour from the RestV1 mapper, and the less precise code which is currently commented out.

lifeofguenter commented 6 years ago

Would be interesting what the "official" docs suggest / or what other libraries are doing. Imo I would not passthrough any timezone and rather default to UTC for any reads and writes to the datastore to keep consistency.

Handling timezone calculations should be done at a higher level.

RobertG4M commented 6 years ago

I've just pushed some quick tests I wrote last week to demonstrate the issue a little better.

You make a fair point, the problem at the moment is that we've currently got inconsistent behaviour between the two mappers.

When using the DateTime constructor (as the Restv1 Mapper does), PHP will auto-map that to the currently configured default timezone, whereas the use of createFromFormat circumvents that and returns a date in UTC regardless of what the default timezone is set as.

I don't mind if we fix the inconsistency in the other way (by forcing the UTC timezone in both mappers), but either way this could affect those currently depending on the existing behaviour.

tomwalder commented 6 years ago

Sorry for the massive delay... just looking into this.

tomwalder commented 6 years ago

So, my understanding is that unix timestamps are timezone agnostic.

See here for more info: https://stackoverflow.com/questions/23062515/do-unix-timestamps-change-across-timezones

As such, I just did this quick test script.

<?php

$obj_d1 = \DateTime::createFromFormat('U.u', sprintf('%0.6F', '299925000'));

$obj_d1->setTimezone(new \DateTimeZone('UTC'));
echo $obj_d1->format('c'), PHP_EOL;

// 1979-07-04T08:30:00+00:00

$obj_d1->setTimezone(new \DateTimeZone('Europe/London'));
echo $obj_d1->format('c'), PHP_EOL;

// 1979-07-04T09:30:00+01:00

For me, those outputs are both the SAME time.

The only thing I affect by changing the timezone is the output format.

I think I need to review the test in more detail to understand where the problem is. I'm not 100% there is a problem with the Protobuf Mapper.

tomwalder commented 6 years ago

OK, I think I get this. The difference is not in the DATA, but in the assigned timezone of the two datetime objects?

RobertG4M commented 4 years ago

Closing this for the time being with it being such an old PR. We've since handled this internally in the application where this issue was being experienced