Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/1601 ensure utc in database #46

Merged
merged 8 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ See [keep a changelog] for information about writing changes to this log.
- Remove redundant CORS bundle
- Update FeedDefaultsMapper to set default values for empty nested properties
- Better error handling for import flow
- Force UTC for all timestamps persisted in the database

[keep a changelog]: https://keepachangelog.com/en/1.1.0/
[unreleased]: https://github.com/itk-dev/event-database-imports/compare/main...develop
3 changes: 3 additions & 0 deletions config/packages/doctrine.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
doctrine:
dbal:
url: '%env(resolve:DATABASE_URL)%'
types:
datetime: App\Doctrine\Extensions\DBAL\Types\UTCDateTimeType
datetime_immutable: App\Doctrine\Extensions\DBAL\Types\UTCDateTimeImmutableType

# IMPORTANT: You MUST configure your server version,
# either here or in the DATABASE_URL env var (see .env file)
Expand Down
53 changes: 53 additions & 0 deletions docs/adr/008-enforce-utc-in-database-and-orm.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# Enforce UTC in database and ORM layer

Date: 10-06-2024

## Status

Accepted

## Context

Doctrine ORM by design does NOT handle timezones when persisting DateTime objects. E.g. if you do for a
doctrine entity (note different timezones)

```php
$event->start = new \DateTimeImmutable('2024-06-04 17:17:17.000000', new \DateTimeZone('UTC'));
$event->end = new \DateTimeImmutable('2024-06-04 17:17:17.000000', new \DateTimeZone('Europe/Copenhagen'));

$this->entityManager->persist($event);
$this->entityManager->flush();
```

Then in the database these will have been persisted as

| id | start | end |
|-----|---------------------|---------------------|
| xyz | 2024-06-04 17:17:17 | 2024-06-04 17:17:17 |

Thereby discarding the timezone and timezone offset.

Because we accept any valid timestamp in the feed imports regardless of timezone this doctrine behavior will lead to
inconsistencies in the timestamps persisted in the database and exposed through the API.

## Decision

All datetime fields must be converted to UTC before they are persisted to database. This is done by overwriting the
standard doctrine `DateTime` and `DateTimeImmutable` types by custom types as specified by Doctrine
[Handling different Timezones with the DateTime Type](https://www.doctrine-project.org/projects/doctrine-orm/en/3.2/cookbook/working-with-datetime.html#handling-different-timezones-with-the-datetime-type)

```yaml
# config/packages/doctrine.yaml
doctrine:
dbal:
types:
datetime: App\Doctrine\Extensions\DBAL\Types\UTCDateTimeType
datetime_immutable: App\Doctrine\Extensions\DBAL\Types\UTCDateTimeImmutableType
```

## Consequences

All "view layers" need to be configured to factor in the model timezone. For the API this means that all datetime fields
must be serialized with timezone (already the default). For EasyAdmin all datetime fields must be configured with
correct "model" and "view" timezones. This is done with [field configurators](https://symfony.com/bundles/EasyAdminBundle/current/fields.html#field-configurators)
and filter configurators. See `App\EasyAdmin\DateTimeFieldConfigurator` and `App\EasyAdmin\DateTimeFilterConfigurator`
21 changes: 20 additions & 1 deletion src/Controller/Admin/DashboardController.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use App\Entity\Tag;
use App\Entity\User;
use App\Entity\Vocabulary;
use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
use EasyCorp\Bundle\EasyAdminBundle\Config\Dashboard;
use EasyCorp\Bundle\EasyAdminBundle\Config\MenuItem;
use EasyCorp\Bundle\EasyAdminBundle\Controller\AbstractDashboardController;
Expand All @@ -22,10 +23,15 @@

class DashboardController extends AbstractDashboardController
{
public const string MODEL_TIMEZONE = 'UTC';
public const string VIEW_TIMEZONE = 'Europe/Copenhagen';

// Default date time format used in the UI.
//
// @see https://unicode-org.github.io/icu/userguide/format_parse/datetime/#datetime-format-syntax
public const DATETIME_FORMAT = 'dd-MM-Y HH:mm:ss';
public const string DATETIME_FORMAT = 'dd-MM-Y HH:mm:ss';
public const string TIME_FORMAT = 'HH:mm:ss';
public const string DATE_FORMAT = 'dd-MM-Y';
Comment on lines +27 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting the “view” and “format” values from config (.env.local) would be really nice, but it may require a lot of effort.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are unlikely to change in the applications lifetime so will leave as is for now.


#[Route('/admin', name: 'admin')]
public function index(): Response
Expand Down Expand Up @@ -54,4 +60,17 @@ public function configureMenuItems(): iterable
yield MenuItem::linkToCrud(new TranslatableMessage('admin.link.organizations'), 'fa fa-sitemap', Organization::class);
yield MenuItem::linkToCrud(new TranslatableMessage('admin.link.users'), 'fa fa-user', User::class);
}

public function configureCrud(): Crud
{
// Default config for all cruds in this controller.
// Only impact index and detail actions.
// For Forms, use ->setFormTypeOption('view_timezone', '...') on all fields
// Done globally in App\EasyAdmin\DateTimeFieldConfigurator
return Crud::new()
->setTimezone(self::VIEW_TIMEZONE)
->setDateTimeFormat(self::DATETIME_FORMAT)
->setDateFormat(self::DATE_FORMAT)
->setTimeFormat(self::TIME_FORMAT);
}
}
13 changes: 13 additions & 0 deletions src/Controller/Admin/OccurrenceCrudController.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
use App\Entity\Occurrence;
use Doctrine\Common\Collections\Criteria;
use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
use EasyCorp\Bundle\EasyAdminBundle\Config\Filters;
use EasyCorp\Bundle\EasyAdminBundle\Field\AssociationField;
use EasyCorp\Bundle\EasyAdminBundle\Field\DateTimeField;
use EasyCorp\Bundle\EasyAdminBundle\Field\FormField;
use EasyCorp\Bundle\EasyAdminBundle\Field\IdField;
use EasyCorp\Bundle\EasyAdminBundle\Field\TextField;
use EasyCorp\Bundle\EasyAdminBundle\Filter\DateTimeFilter;
use Symfony\Component\Translation\TranslatableMessage;

class OccurrenceCrudController extends AbstractBaseCrudController
Expand Down Expand Up @@ -64,4 +66,15 @@ public function configureFields(string $pageName): iterable
->setFormat(DashboardController::DATETIME_FORMAT),
];
}

public function configureFilters(Filters $filters): Filters
{
return $filters
->add('event')
->add(DateTimeFilter::new('start'))
->add(DateTimeFilter::new('end'))
->add('ticketPriceRange')
->add('room')
;
}
}
47 changes: 47 additions & 0 deletions src/Doctrine/Extensions/DBAL/Types/UTCDateTimeImmutableType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php

namespace App\Doctrine\Extensions\DBAL\Types;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\DateTimeImmutableType;

class UTCDateTimeImmutableType extends DateTimeImmutableType
{
private static ?\DateTimeZone $utc;

public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
{
if ($value instanceof \DateTimeImmutable) {
if (self::getUtc()->getName() !== $value->getTimezone()->getName()) {
$value = $value->setTimezone(self::getUtc());
}
}

return parent::convertToDatabaseValue($value, $platform);
}

public function convertToPHPValue($value, AbstractPlatform $platform): ?\DateTimeImmutable
{
if (null === $value || $value instanceof \DateTimeImmutable) {
return $value;
}

$converted = \DateTimeImmutable::createFromFormat(
$platform->getDateTimeFormatString(),
$value,
self::getUtc()
);

if (false === $converted) {
throw ConversionException::conversionFailedFormat($value, $this->getName(), $platform->getDateTimeFormatString());
}

return $converted;
}

private static function getUtc(): \DateTimeZone
{
return self::$utc ??= new \DateTimeZone('UTC');
}
}
45 changes: 45 additions & 0 deletions src/Doctrine/Extensions/DBAL/Types/UTCDateTimeType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
<?php

namespace App\Doctrine\Extensions\DBAL\Types;

use Doctrine\DBAL\Platforms\AbstractPlatform;
use Doctrine\DBAL\Types\ConversionException;
use Doctrine\DBAL\Types\DateTimeType;

class UTCDateTimeType extends DateTimeType
{
private static ?\DateTimeZone $utc;

public function convertToDatabaseValue($value, AbstractPlatform $platform): ?string
{
if ($value instanceof \DateTime) {
$value->setTimezone(self::getUtc());
}

return parent::convertToDatabaseValue($value, $platform);
}

public function convertToPHPValue($value, AbstractPlatform $platform): \DateTime|\DateTimeInterface|null
{
if (null === $value || $value instanceof \DateTime) {
return $value;
}

$converted = \DateTime::createFromFormat(
$platform->getDateTimeFormatString(),
$value,
self::getUtc()
);

if (false === $converted) {
throw ConversionException::conversionFailedFormat($value, $this->getName(), $platform->getDateTimeFormatString());
}

return $converted;
}

private static function getUtc(): \DateTimeZone
{
return self::$utc ??= new \DateTimeZone('UTC');
}
}
31 changes: 31 additions & 0 deletions src/EasyAdmin/DateTimeFieldConfigurator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?php

namespace App\EasyAdmin;

use App\Controller\Admin\DashboardController;
use EasyCorp\Bundle\EasyAdminBundle\Context\AdminContext;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Field\FieldConfiguratorInterface;
use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto;
use EasyCorp\Bundle\EasyAdminBundle\Dto\FieldDto;
use EasyCorp\Bundle\EasyAdminBundle\Field\DateField;
use EasyCorp\Bundle\EasyAdminBundle\Field\DateTimeField;
use EasyCorp\Bundle\EasyAdminBundle\Field\TimeField;

class DateTimeFieldConfigurator implements FieldConfiguratorInterface
{
public function supports(FieldDto $field, EntityDto $entityDto): bool
{
return
DateTimeField::class === $field->getFieldFqcn()
|| DateField::class === $field->getFieldFqcn()
|| TimeField::class === $field->getFieldFqcn();
}

public function configure(FieldDto $field, EntityDto $entityDto, AdminContext $context): void
{
$field->setFormTypeOptions([
'model_timezone' => DashboardController::MODEL_TIMEZONE,
'view_timezone' => DashboardController::VIEW_TIMEZONE,
]);
}
}
30 changes: 30 additions & 0 deletions src/EasyAdmin/DateTimeFilterConfigurator.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
<?php

namespace App\EasyAdmin;

use App\Controller\Admin\DashboardController;
use EasyCorp\Bundle\EasyAdminBundle\Context\AdminContext;
use EasyCorp\Bundle\EasyAdminBundle\Contracts\Filter\FilterConfiguratorInterface;
use EasyCorp\Bundle\EasyAdminBundle\Dto\EntityDto;
use EasyCorp\Bundle\EasyAdminBundle\Dto\FieldDto;
use EasyCorp\Bundle\EasyAdminBundle\Dto\FilterDto;
use EasyCorp\Bundle\EasyAdminBundle\Filter\DateTimeFilter;

class DateTimeFilterConfigurator implements FilterConfiguratorInterface
{
public function supports(FilterDto $filterDto, ?FieldDto $fieldDto, EntityDto $entityDto, AdminContext $context): bool
{
return DateTimeFilter::class === $filterDto->getFqcn();
}

public function configure(FilterDto $filterDto, ?FieldDto $fieldDto, EntityDto $entityDto, AdminContext $context): void
{
$filterDto->setFormTypeOptions([
'value_type_options' => [
// @see https://symfony.com/doc/current/reference/forms/types/date.html#field-options
'model_timezone' => DashboardController::MODEL_TIMEZONE,
'view_timezone' => DashboardController::VIEW_TIMEZONE,
],
]);
}
}