Fields persisted by Doctrine ORM Persister have a wrong timezone

[souhail-5]

API Platform version(s) affected: 2.5.0

Description
Hello,

Given I am using Doctrine ORM Persister
And I have an Entity property $occuredAt typed asdatetime
When I POST the field occuredAt with this value 2019-10-28T18:37:00+04:00
Then the Entity is wrongly persisted with this value 2019-10-28 18:37:00
And the Resource is wrongly returned with this value 2019-10-28T18:37:00+00:00

It should be persisted with this value 2019-10-28 14:37:00 returned with this value 2019-10-28T14:37:00+00:00 (if no extra serialization was applied).

Am I missing any configuration option?

Possible Solution
I tried to remove this line in ...\Bridge\Doctrine\Common\DataPersister and the returned value was as expected.

[antograssiot]

I guess that without this line, the response to your POST/PUT request is as expected but actually sending a subsequent GET request on this resource would show the bad date too as it is not saved correctly in the DB.
If your PHP default timezone is the same than you DB, setting the correct default timezone in your setter for this field is a trick that worked for me. Even if not ideal, it avoid a custom Doctrine type and therefore let you continue to use core filters

[teohhanhui]

This is not a bug. It's just how Doctrine works.

I'm not sure what we could provide out of the box, but what you could do is:

$this->occuredAt->setTimezone(new \DateTimeZone('UTC'));

Ideally this should be done by decorating a normalizer. Unfortunately, there's still no way to have property-level serializer context: #1922

[teohhanhui]

Perhaps we could add a feature to regularize the timezone in the (Symfony) DateTimeNormalizer. WDYT @dunglas?

[souhail-5]

setting the correct default timezone in your setter for this field is a trick

Yes, I do that for now, thank you :)

This is not a bug. It's just how Doctrine works.

@teohhanhui, I agree and I found this issue, maybe it's related.
As it's related to Doctrine, It would be better to do the trick inside the DataPersister (or inside ApiPlatform\Core\Bridge\Doctrine), not in the Normalizer.

[teohhanhui]

As it's related to Doctrine, It would be better to do the trick in the DataPersister (for Doctrine) not in the Normalizer.

It's not possible, because the DataPersister does not do any traversal through the object graph (and we wouldn't want it to). Also, there's nothing wrong with doing it in the normalizer, it just means regularizing everything into a specific timezone (I'd recommended converting to UTC anyway, because the Daylight Saving Time / summer time problem is not fun).

[souhail-5]

it just means regularizing everything into a specific timezone

Doing this in the DateTimeNormalizer::denormalize() will drop the original timezone information. We may need the original entry information.

[teohhanhui]

We may need the original entry information.

Yeah, which is why it should be optional, and why I think it's not something we can fix out of the box.

Of course, if you have another idea, please share and we'll see if it's feasible. 😄

[bastnic]

FYI,

We have the same kind of trouble and we force the date to be in UTC in database (correct fix would have been to store the timezone in a second field, but we don't need it):

<?php

declare(strict_types=1);

namespace Lib\Core\Doctrine\Types;

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

class UTCDateTimeType extends DateTimeType
{
    /** @var \DateTimeZone|null */
    private static $utc;

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

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

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

        $converted = \DateTime::createFromFormat(
            $platform->getDateTimeFormatString(),
            $value,
            self::$utc ? self::$utc : self::$utc = new \DateTimeZone('UTC')
        );

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

        return $converted;
    }

    public static function getUtc(): \DateTimeZone
    {
        return self::$utc ? self::$utc : self::$utc = new \DateTimeZone('UTC');
    }

    public function requiresSQLCommentHint(AbstractPlatform $platform): bool
    {
        return true;
    }
}

doctrine:
    dbal:
        types:
          datetime: Lib\Core\Doctrine\Types\UTCDateTimeType

Then we force the DateTimeNormalizer to send data in our TimeZone:

    Symfony\Component\Serializer\Normalizer\DateTimeNormalizer:
        arguments:
            $defaultContext:
                datetime_timezone: 'Europe/Paris'

[souhail-5]

Thank you :) Just one point: as mentioned by @antograssiot, (currently?) the inconvenient with doing it this way is that you can't continue to use core filters.

@teohhanhui, is it a good/feasible idea to have UTCDateTimeType as suggested by @bastnic and by Doctrine inside Core\Bridge\Doctrine and modify Filters like Bridge\Doctrine\Orm\Filter\DateFilter::DOCTRINE_DATE_TYPES to accept UTCDateTimeType?
It will be documented to use UTCDateTimeType if we need to accept a "datetime+timezone" field.

[soyuka]

This could definitely be added to the documentation.

[teohhanhui]

I don't believe it should be done by a custom Doctrine type, even though it's indeed a solution suggested in the Doctrine docs: https://www.doctrine-project.org/projects/doctrine-orm/en/current/cookbook/working-with-datetime.html#handling-different-timezones-with-the-datetime-type

I think it would be cleaner to have an (optional) feature to regularize the timezone in Symfony Serializer's DateTimeNormalizer when denormalizing.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.