Skip to content

Commit

Permalink
wip: Use UTC internally
Browse files Browse the repository at this point in the history
According to https://dev.mysql.com/doc/refman/8.0/en/time-zone-support.html

> MySQL converts `TIMESTAMP` values from the current time zone to UTC for storage, and back from UTC to the current time zone for retrieval. (This does not occur for other types such as `DATETIME`.)

That is bad.

- [ ] Check that we do not use TIMESTAMP anywhere or set MySQL timezone to UTC.
- [ ] Verify that PostgreSQL includes timezone offsets when selecting datetime values with timezone.
- [ ] Check that MySQL correctly compares datetimes in db with values containing tz offset as produced by `daos\mysql\statements::datetime()`.
- [ ] Add migrations for sqlite local → UTC when `date.timezone` is not UTC.
  • Loading branch information
jtojnar committed Jul 24, 2023
1 parent f14546b commit 48f6780
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 16 deletions.
6 changes: 2 additions & 4 deletions src/controllers/Items/Sync.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ public function sync(): void {
$this->view->jsonError(['sync' => 'missing since argument']);
}

$since = new \DateTimeImmutable($params['since']);
$since = $since->setTimeZone(new \DateTimeZone(date_default_timezone_get()));
// The client should include a timezone offset but let’s default to UTC in case it does not.
$since = new \DateTimeImmutable($params['since'], new \DateTimeZone('UTC'));

$lastUpdate = $this->itemsDao->lastUpdate();

Expand All @@ -66,9 +66,7 @@ public function sync(): void {
$sinceId = $this->itemsDao->lowestIdOfInterest() - 1;
// only send 1 day worth of items
$notBefore = new \DateTimeImmutable();
$notBefore = $notBefore->setTimeZone(new \DateTimeZone(date_default_timezone_get()));
$notBefore = $notBefore->sub(new \DateInterval('P1D'));
$notBefore = $notBefore->setTimeZone(new \DateTimeZone(date_default_timezone_get()));
}

$itemsHowMany = $this->configuration->itemsPerpage;
Expand Down
6 changes: 4 additions & 2 deletions src/daos/ItemOptions.php
Original file line number Diff line number Diff line change
Expand Up @@ -71,15 +71,17 @@ public static function fromUser(array $data): self {
}

if (isset($data['fromDatetime']) && is_string($data['fromDatetime']) && strlen($data['fromDatetime']) > 0) {
$options->fromDatetime = new \DateTimeImmutable($data['fromDatetime']);
// The client should include a timezone offset but let’s default to UTC in case it does not.
$options->fromDatetime = new \DateTimeImmutable($data['fromDatetime'], new \DateTimeZone('UTC'));
}

if (isset($data['fromId']) && is_numeric($data['fromId'])) {
$options->fromId = (int) $data['fromId'];
}

if (isset($data['updatedsince']) && is_string($data['updatedsince']) && strlen($data['updatedsince']) > 0) {
$options->updatedSince = new \DateTimeImmutable($data['updatedsince']);
// The client should include a timezone offset but let’s default to UTC in case it does not.
$options->updatedSince = new \DateTimeImmutable($data['updatedsince'], new \DateTimeZone('UTC'));
}

if (isset($data['tag']) && is_string($data['tag']) && strlen($tag = trim($data['tag'])) > 0) {
Expand Down
7 changes: 5 additions & 2 deletions src/daos/mysql/Items.php
Original file line number Diff line number Diff line change
Expand Up @@ -569,7 +569,9 @@ public function lastUpdate(): ?DateTimeImmutable {
FROM ' . $this->configuration->dbPrefix . 'items;');
$lastUpdate = $res[0]['last_update_time'];

return $lastUpdate !== null ? new DateTimeImmutable($lastUpdate) : null;
// MySQL and SQLite do not support timezones, load it as UTC.
// PostgreSQL will include the timezone offset as the part of the returned value and it will take precedence.
return $lastUpdate !== null ? new DateTimeImmutable($lastUpdate, new \DateTimeZone('UTC')) : null;
}

/**
Expand Down Expand Up @@ -627,7 +629,8 @@ public function bulkStatusUpdate(array $statuses): void {

// sanitize update time
if (array_key_exists('datetime', $status)) {
$updateDate = new \DateTimeImmutable($status['datetime']);
// The client should include a timezone offset but let’s default to UTC in case it does not.
$updateDate = new \DateTimeImmutable($status['datetime'], new \DateTimeZone('UTC'));
} else {
$updateDate = null;
}
Expand Down
3 changes: 2 additions & 1 deletion src/daos/mysql/Statements.php
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ public static function ensureRowTypes(array $rows, array $expectedRowTypes): arr
}
break;
case DatabaseInterface::PARAM_DATETIME:
$value = new \DateTimeImmutable($row[$columnIndex]);
// MySQL and SQLite do not support timezones, load it as UTC.
$value = new \DateTimeImmutable($row[$columnIndex], new \DateTimeZone('UTC'));
break;
default:
$value = null;
Expand Down
1 change: 1 addition & 0 deletions src/daos/pgsql/Statements.php
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ public static function ensureRowTypes(array $rows, array $expectedRowTypes): arr
}
break;
case DatabaseInterface::PARAM_DATETIME:
// PostgreSQL will include the timezone offset as the part of the returned value.
$value = new \DateTimeImmutable($row[$columnIndex]);
break;
default:
Expand Down
10 changes: 3 additions & 7 deletions src/daos/sqlite/Statements.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,10 @@ public static function bool(bool $bool): string {
* @return string representation of datetime
*/
public static function datetime(\DateTimeImmutable $date): string {
// SQLite does not support timezones.
// The client previously sent the local timezone
// but now it sends UTC time so we need to adjust it here
// to avoid fromDatetime mismatch.
// TODO: Switch to UTC everywhere.
$date = $date->setTimeZone((new \DateTimeImmutable())->getTimeZone());
// SQLite does not support timezones so we store all dates in UTC.
$utcDate = $date->setTimeZone(new \DateTimeZone('UTC'));

return $date->format('Y-m-d H:i:s');
return $utcDate->format('Y-m-d H:i:s');
}

/**
Expand Down

0 comments on commit 48f6780

Please sign in to comment.