-
-
Notifications
You must be signed in to change notification settings - Fork 9.7k
Description
Symfony version(s) affected
7.2.5
Description
Hello !
First of all, I saw quite a few issues around denormalization of union types, but didn't find one that matches my issue so I decided to create a proper one. Sorry if this is a duplicate.
That being said, I am having an issue while denormalizing a value of type null|A|B
where :
A
is an object with a custom Denormalizer to handle itB
is a random DTO
While writing my tests, I realized that my value was always denormalized as a B
instance, even if my value corresponded to the supports
method of my Denormalizer. I originally thought it was about services priority, but after investigating, I found out that the AbstractObjectNormalizer
does not behave like I would like here.
At line 662, the types are resolved accordingly to the fact that the type can be a union type (which is my case) :
$types = match (true) {
$type instanceof IntersectionType => throw new LogicException('Unable to handle intersection type.'),
$type instanceof UnionType => $type->getTypes(),
default => [$type],
};
However, it then iterates over every type, in search of a value to use for the result of the denormalization. As we can see at line 837, for object types with a defined class, the AbstractObjectNormalizer
will find the first supporting Denormalizer and return the denormalized value :
if (TypeIdentifier::OBJECT === $typeIdentifier && null !== $class) {
if (!$this->serializer instanceof DenormalizerInterface) {
throw new LogicException(\sprintf('Cannot denormalize attribute "%s" for class "%s" because injected serializer is not a denormalizer.', $attribute, $class));
}
$childContext = $this->createChildContext($context, $attribute, $format);
if ($this->serializer->supportsDenormalization($data, $class, $format, $childContext)) {
return $this->serializer->denormalize($data, $class, $format, $childContext);
}
}
All of this means that the order in which the types are returned by the TypeInfo
component matters. I think the problem is here, since it creates "randomness" in the way your value is going to be denormalized. You do not seem to be able to control it. Also, it prevents the dev from having flexibility in the type declaration.
How to reproduce
Create an object with its denormalizer :
class A
{
public function __construct(
public string $value,
) {}
}
class ADenormalizer implements DenormalizerInterface
{
public function supportsDenormalization(mixed $data, string $type, ?string $format = null, array $context = []): bool
{
return A::class === $type;
}
}
Create an object :
class MyObject
{
public function __construct() {
public null|A|B $prop = null,
} {}
}
Make sure here that B
will be retrieved before A
on type resolution.
B
can just be random DTO containing random properties.
Denormalize into that object :
{
"prop": "test"
}
$result = $this->serializer->deserialize($json, MyObject::class, 'json');
Check that $prop
is type A
, but realize that it's type B
:
\assert($result->prop instanceof A);
Possible Solution
I don't know whether keeping the declaration order should be the default behavior or not, but this is the first thing that comes to my mind for such an issue right now. It would devs to put more "restrictive" types before the generic ones I guess.
Additional Context
No response