Skip to content

Conversation

suquant
Copy link
Member

@suquant suquant commented May 28, 2012

У нас начиная с PHP5.2 >= есть поддержка DateTime & DateTimeZone

Собственно теперь Timestamp просто контейнер с доп функциями.

Тесты все работают, BC не нарушенно.

@crazedr0m
Copy link
Contributor

добавьте тест для даты "1-04-03" ну или "0001-04-03"

@crazedr0m
Copy link
Contributor

хотя нет именно для разных вариантов. и для "1-04-03" и для "0001-04-03"

@crazedr0m
Copy link
Contributor

для года 2 и 1 знак, результаты будут разные а для 3 и 4 знака в значении года - результаты будут одинаковые в старой и новой версиях.
Прошу прощения не в новой и старой версии, а в старом Timestamp и штатном DateTime. В общем дополнительные тесты не помешают я думаю.

@neemah
Copy link
Contributor

neemah commented May 28, 2012

$this->dateTime = new DateTime(date($this->getFormat(), $date));

Должно быть

$this->dateTime = new DateTime("@$date");

Тогда DateTime не потеряет временную зону которая регламентированна настройками php.

Пруф из документации:

Замечание:

Значение аргумента $timezone равно как и текущая временная зона не будут учитываться, если в качестве аргумента $time передается метка времени UNIX (напрмер @9466848) или время, в котором временная зона уже содержится (например 2010-01-28T15:00:00+02:00)

@dovg
Copy link
Member

dovg commented May 28, 2012

sleep и wakeup не поломался? в текущей реализации сохраняется только int

@suquant
Copy link
Member Author

suquant commented May 28, 2012

хотя нет именно для разных вариантов. и для "1-04-03" и для "0001-04-03"

1-04-03 это можно трактовать как 2001-04-03 или же 1-04-03 что правильно ?
а 0001=04-03 можно трактовать как 1 год от Рождества Христова.

Как быть ? :-)

@suquant
Copy link
Member Author

suquant commented May 28, 2012

Значение аргумента $timezone равно как и текущая временная зона не будут учитываться, если в качестве аргумента $time передается метка времени UNIX (напрмер @9466848) или время, в котором временная зона уже содержится (например 2010-01-28T15:00:00+02:00)

Обошел это проставлением дефолтовой зоной в конструкторе :-)

@neemah
Copy link
Contributor

neemah commented May 28, 2012

Это не обошел, это выстрел в ногу.

@suquant
Copy link
Member Author

suquant commented May 28, 2012

Это не обошел, это выстрел в ногу.

Эт почему-же ?
Не люблю я просто всякие магические конструкции вида "@..."

@crazedr0m
Copy link
Contributor

"1-04-03" и для "0001-04-03"

я считаю что правильно текущее поведение, т.е. первый год

@AlexeyDsov
Copy link
Member

Помимо еще недосказанного @neemah, тут, видимо нужно сделать метод __clone, что бы protected $datetime нормально клонировался.

@suquant
Copy link
Member Author

suquant commented May 29, 2012

Помимо еще недосказанного @neemah, тут, видимо нужно сделать метод __clone, что бы protected $datetime нормально клонировался.

Согласен, сделаю.

@neemah
Copy link
Contributor

neemah commented May 29, 2012

По поводу моего коммента: я конечно же не настаиваю, но если есть документированная операция создания объекта DateTime на основе unix timestamp и гарантированно дающая правильный результат (даже тесты не нужны), то нет смысла в дополнительной операции date().

@suquant
Copy link
Member Author

suquant commented May 29, 2012

Ну я думаю что явно оно яснее, нагляднее, а date т.к. DateTime в явном виде не умеет работать с UNIX time stamp-ом по этому небольшой оверхеад, хотя тут можно поспорить мож @... дольше выполняется )))

@suquant
Copy link
Member Author

suquant commented May 31, 2012

Собсно подумал и пришел к тому что все-же даты вида [1-9, 01-99]-month-day
нужно расчитывать от текушего года, международная практика, а к датам типа [100-999] добаялется ведомый "0" т.е. 0123 год к примеру.

Собсно в тестах учел.

@suquant
Copy link
Member Author

suquant commented May 31, 2012

по поводу "@..." не стал этого делать, вместо этого у нас unixtimestamp приводится к дате с помощью функции date() а дефолотовая зона ставиться в самом конструкторе, если не пришол DateTimeZone

@neemah
Copy link
Contributor

neemah commented May 31, 2012

Интересная тема, собака с unix timestamp у меня по тестам реально работает медленнее: http://pastebin.com/Qebk3Li3
PHP 5.4.3 (cli) (built: May 8 2012 17:04:44)

В общем смотрите сами :)

@AlexeyDsov
Copy link
Member

Добрался до тестирования этого реквеста.

  1. Неправильно работает exportValue у примитивов timestamp[tz] в случае setMarried - в массиве возвращается только год, месяц день.
  2. В PrimitiveTimestampTZTest, как уже написал выше, не хватает тестов на exportValue + в тест testSingle использует primitiveTimestamp вместое primitiveTimestampTZ
  3. Надо б убрать наверное все таки из тестов отладочные @group ff :)

@AlexeyDsov
Copy link
Member

Все блин смотрю и смотрю по вечерам на этот реквест и никак не замержу.
Последнее пожалуй что смущает - можно ли как-нибудь, что б в примере

<?php
$prm = Primitive::timestampTZ('ttt');
$prm->import(array('ttt' => '2012-01-01 22:22:22-0004'));
$export = $prm->exportValue();

в export строке данные были в той же таймзоне что и во время импорта?

Ах да. Помоему для тестов в test/config.inc.php.tpl надо принудительно прописывать одну конкретную дефолтную таймзону.

@suquant
Copy link
Member Author

suquant commented Jul 10, 2012

Да дефолтная timezone должна быть, иначе кажеться будет взята зона с сервера по умолчанию.

в export строке данные были в той же таймзоне что и во время импорта?

мы вроде как уже это обсуждали, если мне не изменяет память, и пришли к тому что возвращаем в той timezone в котором сейчас сервер (с правильным смещением)

@AlexeyDsov
Copy link
Member

Отловил что на 32-битных системах не корректно работают sleep - wakeup в текущем варианте - через int.
Зато если вместо int сериализовывать dateTime, то все ок. Проигрышь в размере хранения, но работает вернее.
Кстати, если так сделать, то, кажется int вообще сам по себе уже не нужен будет.

@AlexeyDsov
Copy link
Member

Добавил что б в тестах ставилась дефолтная временная зона Europe/Moscow.

@AlexeyDsov
Copy link
Member

Еще разок сделал rebase и склеил в один коммит. Ну вроде можно нажимать кнопочку merge и писать doc/ChangeLog

AlexeyDsov added a commit that referenced this pull request Jul 19, 2012
Update Timestamp (+BC) + TimestampTZ
@AlexeyDsov AlexeyDsov merged commit 4d0cfc6 into onPHP:master Jul 19, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants