You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
UnitOfWork::tryGetById() and UnitOfWork::tryGetByIdHash() may receive an object as an id, for instance when using a custom id type like Rhumsaa\Uuid.
This PR adds a simple cast to string in these methods.
There are a couple of things I'm not sure about, tho.
First of all, I don't know if functional tests alone are deemed sufficient, but I couldn't figure out how to replicate the issue via unit tests only (apart from directly invoking the two methods above with an object as argument, which looks a bit pointless).
Furthermore, I don't know if the cast should be applied elsewhere; so far the only cases I stumbled upon where common find and fetch joins, but I saw other methods where it may be necessary (i.e. here and here), but I couldn't replicate the code path to reach those.
Also I wonder if an exception should be thrown if the id object doesn't implement **toString(), and in this case if the fix should be refactored into Doctrine\ORM\Utility\IdentifierFlattener, for a bit of SoC.
Let me know what you think.
The text was updated successfully, but these errors were encountered:
Jira issue originally created by user @doctrinebot:
This issue is created automatically through a Github pull request on behalf of stefanotorresi:
Url: #1336
Message:
UnitOfWork::tryGetById()
andUnitOfWork::tryGetByIdHash()
may receive an object as an id, for instance when using a custom id type like Rhumsaa\Uuid.This PR adds a simple cast to string in these methods.
There are a couple of things I'm not sure about, tho.
First of all, I don't know if functional tests alone are deemed sufficient, but I couldn't figure out how to replicate the issue via unit tests only (apart from directly invoking the two methods above with an object as argument, which looks a bit pointless).
Furthermore, I don't know if the cast should be applied elsewhere; so far the only cases I stumbled upon where common find and fetch joins, but I saw other methods where it may be necessary (i.e. here and here), but I couldn't replicate the code path to reach those.
Also I wonder if an exception should be thrown if the id object doesn't implement
**toString()
, and in this case if the fix should be refactored intoDoctrine\ORM\Utility\IdentifierFlattener
, for a bit of SoC.Let me know what you think.
The text was updated successfully, but these errors were encountered: