[Serializer] fix: deserialization union type of enum #47803
Conversation
| if ($missingConstructorArgumentException) { | ||
| throw $missingConstructorArgumentException; | ||
| if ($exception) { | ||
| throw $exception; |
There was a problem hiding this comment.
This will now re-throw a NotNormalizableValueException before the DISABLE_TYPE_ENFORCEMENT check
There was a problem hiding this comment.
@stof I did not understand quite well.
Is it better not to store the NotNormalizableValueException?
Like this :
if (!$exception && !$e instanceof NotNormalizableValueException) {
$exception = $e;
}|
@stof can you review please ? |
| @@ -63,7 +63,7 @@ public function denormalize($data, string $type, string $format = null, array $c | |||
|
|
|||
| try { | |||
| return $type::from($data); | |||
| } catch (\ValueError $e) { | |||
| } catch (\ValueError|\TypeError $e) { | |||
There was a problem hiding this comment.
To me, the TypeError should result in a NotNormalizableValueException, as it is a more precise case of the check above (as an integer-backed enum won't support strings but the normalizer does not distinguish the backing type)
There was a problem hiding this comment.
@stof Now, TypeError result in a NotNormalizableValueException.
I'm not sure how to throw the exception, your knowledge is welcome.
| @@ -63,7 +63,7 @@ public function denormalize($data, string $type, string $format = null, array $c | |||
|
|
|||
| try { | |||
| return $type::from($data); | |||
| } catch (\ValueError $e) { | |||
| } catch (\ValueError|\TypeError $e) { | |||
| throw new InvalidArgumentException('The data must belong to a backed enumeration of type '.$type); | |||
There was a problem hiding this comment.
This case should probably use NotNormalizableValueException even for invalid strings btw.
There was a problem hiding this comment.
The exception was changed here #47128
There was a problem hiding this comment.
Yes this commit is the source of the bug.
There was a problem hiding this comment.
Yet it was also fixing a bug, please make sure that test added in #47128 keeps passing and/or that the behavior it fixed remains the expected one.
There was a problem hiding this comment.
All Serializer-related tests, including those in this commit, pass.
|
@nicolas-grekas can you request available reviewers to review this ? |
|
Should be fixed by #51475 |
Fix deserialization of union type of BackedEnum