filicious / core

Filicious is a high level object oriented filesystem abstraction for PHP.
https://filicious.github.io/
31 stars 9 forks source link

Do not use `new DateTime('@' . $timestamp)` #67

Closed tristanlins closed 10 years ago

tristanlins commented 11 years ago

I know somewhere in the code new DateTime('@' . $timestamp) is used. But using this the timezone information get lost! As we are using Namespaces = PHP 5.3 we can use DateTime::setTimestamp instead.

backbone87 commented 11 years ago

Unix timestamp are always GMT Using DateTime::setTimestamp resets TZ to GMT afaik.

tristanlins commented 11 years ago

Using new DateTime('@' . $timestamp) will reset the timezone of the new object to UTC (=> timezone is lost). But with DateTime::setTimestamp the correct timezone is set in the new object.

$ php -r 'var_dump(new DateTime("@" . time()));'
class DateTime#1 (3) {
  public $date =>
  string(19) "2013-04-09 10:35:39"
  public $timezone_type =>
  int(1)
  public $timezone =>
  string(6) "+00:00"
}
$ php -r '$d = new DateTime(); var_dump($d->setTimestamp(time()));'
class DateTime#1 (3) {
  public $date =>
  string(19) "2013-04-09 12:35:39"
  public $timezone_type =>
  int(3)
  public $timezone =>
  string(13) "Europe/Berlin"
}

Yes you can also use a DateTime::setTimestamp after new DateTime('@' . $timestamp), but it make no sense if DateTime::setTimestamp do it right.

tristanlins commented 11 years ago

Okay, a lot confusion around this, just see the note on http://de.php.net/manual/en/datetime.construct.php

The $timezone parameter and the current timezone are ignored when the $time parameter either is a UNIX timestamp (e.g. @946684800) or specifies a timezone (e.g. 2010-01-28T15:00:00+02:00).

One argument more to use DateTime::setTimestamp to be sure the new DateTime object use the local timezone, and not UTC :-)

discordier commented 11 years ago

We should add an optional parameter that allows to retrieve the Timestamp in (at least) UTC.

maybe:

tbd

backbone87 commented 11 years ago

Unix TS is always GMT. Using new DateTime("@" . $unixTS); will yield a DateTime object with TZ set to GMT. Using $dt = new DateTime(); $dt->setTimestamp($unixTS); will yield a DateTime object with TZ set to the PHP default TZ. But both DateTimes return the same unix TS when using $dt->getTimestamp();. We will use the second approach in Filicious. If you want to change the TZ of the returned DateTime just use $dt->setDateTimeZone($tz); (or something like that). No need to provide additional params.

tristanlins commented 10 years ago

Fixed in https://github.com/filicious/core/commit/43ba0dccfd498f42920113d129c104efa7325666 and https://github.com/filicious/core/commit/6293ba945ec1261fa18a2a8e2f1d0c106842d6e2