Fix id hash of entity with enum as identifier #10508
Conversation
Gwemox
force-pushed
the
fix-enum-identity
branch
11 times, most recently
from
c7ddd5b to
63afbeb
Compare
|
Can someone validate the workflow? |
|
Right away, sorry for the delay. 😓 |
Gwemox
force-pushed
the
fix-enum-identity
branch
from
63afbeb to
91ede87
Compare
|
Thank's @derrabus ! |
Gwemox
force-pushed
the
fix-enum-identity
branch
from
91ede87 to
c4df297
Compare
|
@derrabus Does this seem correct to you? |
|
@greg0ire can you review this ? Do you expect anything from me? |
lib/Doctrine/ORM/UnitOfWork.php
Outdated
| /** | ||
| * Gets the id hash of an entity by its identifier. | ||
| * | ||
| * @param array<mixed, mixed> $identifier The identifier of an entity |
There was a problem hiding this comment.
What's the key type for this array? It should be at least array-key, but maybe you can narrow it down even more. Maybe this is even a list<mixed>
There was a problem hiding this comment.
PHPStan tells me it's called with an int or string key.
There was a problem hiding this comment.
Ok, then use array-key I guess.
lib/Doctrine/ORM/UnitOfWork.php
Outdated
| * | ||
| * @return string The entity id hash. | ||
| */ | ||
| public static function getIdHashByIdentifer($identifier) |
There was a problem hiding this comment.
Let's make this method final please.
There was a problem hiding this comment.
Also, it should be "Identifier" with an i
|
|
||
| /** | ||
| * @OneToMany(targetEntity="GH10334Foo", mappedBy="collection", cascade={"persist", "remove"}) | ||
| * @var Collection $foos |
There was a problem hiding this comment.
| * @var Collection $foos | |
| * @var Collection<GH10334Foo> $foos |
| return $this->name; | ||
| } | ||
|
|
||
| public function setName(string $name): void |
There was a problem hiding this comment.
Please remove any unused getter/setter
When an entity have a backed enum as identifier, `UnitOfWork` tries to cast to string when generating the hash of the id. This fix calls `->value` when identifier is a `BackedEnum`. Fixes doctrine#10471 Fixes doctrine#10334
|
Thank you for this quick review, I made the requested changes. |
Gwemox
force-pushed
the
fix-enum-identity
branch
from
c4df297 to
09b4a75
Compare
|
Ping @SenseException @derrabus |
|
@greg0ire Could you please tag a release containing this bugfix? It is cause for major locking issues on our systems and blocking releases of our software.
|
Current situation
When an entity have a backed enum as identifier, Unit Of Work tries to cast to string when generating the hash of the id.
Error "Object of class MyEnum could not be converted to string"Suggested fix
This PR fixes the problem by adding function to get id of entity with backed enum check.
Tests added
It seems tests for this feature were completely missing, so I added them.
Fixes #10471
Fixes #10334