Serialization of DateTimeInterfaces

[andrei-dascalu]

Q	A
Bug report?	yes
Feature request?	no
BC Break report?	no
RFC?	no

Steps required to reproduce the problem

sample code

use JMS\Serializer\Annotation as JMS;
use Cake\Chronos\Chronos;
use DateTimeInterface;

class SerializeDate
{
    /**
     * @var DateTimeInterface
     * @JMS\Type("DateTimeInterface<'Y-m-d'>")
     * @JMS\SerializedName("geboortedatum")
    */
    public $dateTime;
}

$sendData = new SerializeDate();
$sendData->dateTime = Chronos::now();

/** config for annotations */
$serializer = JMS\Serializer\SerializerBuilder::create()->build();
$serializer->serialize($object, 'json');

Expected Result

{
	"geboortedatum": "2021-08-21"
}

Actual Result

{
	"geboortedatum": {
		"testNow": null,
		"toStringFormat": "Y-m-d H:i:s",
		"weekendDays": [6, 7],
		"diffFormatter": null,
		"_lastErrors": [],
		"days": {
			"1": "Monday",
			"2": "Tuesday",
			"3": "Wednesday",
			"4": "Thursday",
			"5": "Friday",
			"6": "Saturday",
			"7": "Sunday"
		},
		"weekStartsAt": 1,
		"weekEndsAt": 7,
		"relativePattern": "\/this|next|last|tomorrow|yesterday|midnight|today|[+-]|first|last|ago\/i"
	}
}

Chronos objects implement DateTimeInterface so I believe they should be serialised accordingly.

If I use a regular DateTime (that also implements DateTimeInterface), the result is as expected.

The behaviour is at the very least inconsistent:

• if Chronos is recognised as a DateTimeInterface (which it seems to be, since there's no type-related error), then it should be formatted accordingly
• if not, then there should be a clear type-related error, rather than silently serializing as an object when it's clearly against the desired configuration.

Cheers!

[goetas]

which version of the serialization library are you using? can you please provide a failing test case?

[andrei-dascalu]

Hi,

I'm using php 8.0.9 with JMS 3.14.0 & Chronos 2.2.0

Simple code to reproduce:

<?php

use JMS\Serializer\Annotation as JMS;
use Cake\Chronos\Chronos;
use DateTimeInterface;

require_once('./vendor/autoload.php');

class SerializeDate
{
    /**
     * @var DateTimeInterface
     * @JMS\Type("DateTimeInterface<'Y-m-d'>")
     * @JMS\SerializedName("birthdate")
    */
    public $dateTime;
}

$sendData = new SerializeDate();
$sendData->dateTime = Chronos::now();

$build = \JMS\Serializer\SerializerBuilder::create()->build();

print_r($build->serialize($sendData, 'json'));

[Majkl578]

I suspect the problem is in DateHandler::getSubscribingMethods() which only registers deserialization handler for DateTimeInterface but for some reason no serialization handler.
It works fine with built-in DateTime/DateTimeImmutable because they're handled explicitly.

Reporoducer without external library:

<?php

require __DIR__ . '/vendor/autoload.php';

$serializer = JMS\Serializer\SerializerBuilder::create()->build();

class CustomDateTimeImmutable extends DateTimeImmutable
{
	private $foo = 123;
}

$customDateTime = new CustomDateTimeImmutable();

echo $serializer->serialize(['time' => $customDateTime], 'json'); // {"time":{"foo":123}}

[andrei-dascalu]

Handlers are organised in an array based on the type. The type gets passed to the HandlerRegistry::getHandler, however the type is the concrete class rather than any potential superclass or satisfied interface (duh) so it seems that by design only concrete classes can be serialized (so asking to serialize a DateTimeInterface will not work unless it's specifically in the list).

Similar for deserializing, you can ask for the interface but this will only result in DateTime object (handler points to deserializeDateTimeFrom (either Yaml or Json, which produce the mutable DateTime).

@goetas @Majkl578
I am not quite sure how this can be fixed in a way that's clear cut and performant (and doesn't lead down the rabbit hole of "what if X implements interface M and N, which way to serialize?").

Perhaps a solution limited to DateTimeInterface where implemented interfaces could be loaded by reflection and then treat the target as a plain DateTime ?

[goetas]

I think that this line (https://github.com/schmittjoh/serializer/blob/master/src/GraphNavigator/SerializationGraphNavigator.php#L188) is the suspect. it changes the type at runtime...

[andrei-dascalu]

It does, but at the same time it looks like that's what it needs to do. It correctly finds that a custom implementation of DatetimeInterface is a subclass of DateTimeInterface and then replaces the type (initially DateTimeInterface) with the concrete class.

If I remove that check and the $type['name'] remains the interface, the resulting item is empty (as there's no handler for DateTimeInterface).

However, if I make the serializeDateTimeInterface method public and add the DateTimeInterface to the list of supported types, this behaves as expected.

I wonder though about the reason for changing the type at runtime? It might make sense to check the concrete type if there's no handler for the superclass (which, incidentally, it's also the case for DateTimeInterface).

After doing this I see that the nested interfaces tests fail, since there's no handler for the interface.

Maybe it makes sense to add a check, if there's a handler for the current type (aka interface) then don't switch ?

[andrei-dascalu]

@goetas @Majkl578 made a proposal for a solution, if you have time, let me know what you think.

[andrei-dascalu]

Hi @goetas I'm wondering if there's anything else I can do for the merge request to have it approved and accepted? Is the solution ok?

Thanks

[goetas]

fixed in #1344