[Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys #21288
Conversation
dmaicher
force-pushed
the
unique-entity-pk-objects
branch
from
3fd23b7
to
5aadce3
Compare
| $identifiers = array_map(function ($identifier) use ($em) { | ||
| // identifiers can be objects (without any __toString method) if its a composite PK | ||
| if (is_object($identifier) && !method_exists($identifier, '__toString')) { | ||
| return sprintf('(%s)', $this->buildInvalidValueString($em, $em->getClassMetadata(get_class($identifier)), $identifier)); |
There was a problem hiding this comment.
is it safe to use the same object manager here for the related class? Not sure if Doctrine supports relations between classes of different object managers?
|
|
||
| $this->validator->validate($newEntity, $constraint); | ||
|
|
||
| $expectedValue = 'Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\CompositeObjectNoToStringIdEntity" identified by "(Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "1"), (Object of class "Symfony\Bridge\Doctrine\Tests\Fixtures\SingleIntIdNoToStringEntity" identified by "2")"'; |
There was a problem hiding this comment.
should we find a shorter way to build this message? 😝
|
I've discussed this PR with @dmaicher in dmaicher#1, which is now merged. This looks good to me. ping @symfony/deciders |
nicolas-grekas
changed the title
|
Failure looks related to me (not the messages about getMock, but the one about CompositeObjectNoToStringIdEntity) - maybe a composer.json update needed somewhere? |
|
@nicolas-grekas I've opened dmaicher#2 which seems to have fixed the tests, waiting for @dmaicher to merge it. |
|
@dmaicher can you please squash the history? We can't merge (nor don't want to :) ) merge PRs with merges inside. |
|
(maybe keep just two commits, one for you and one for @HeahDude with rebase -i ?) |
|
@nicolas-grekas sure 😉 Let's wait for the build and hope its fixed |
|
Green 🎉 |
Fixed UniqueEntityValidator tests
dmaicher
force-pushed
the
unique-entity-pk-objects
branch
from
d942a96
to
b3ced86
Compare
|
Done 😉 |
|
I've not checked, but don't we have the same issue in 2.7 and 2.8? |
|
Thank you @dmaicher. |
…object primary keys (dmaicher, HeahDude) This PR was merged into the 3.1 branch. Discussion ---------- [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys | Q | A | ------------- | --- | Branch? | master / 3.1 | Bug fix? | yes | New feature? |no | BC breaks? | no | Deprecations? |no | Tests pass? | yes (fail on php 7.1 unrelated?) | Fixed tickets | #21274 | License | MIT | Doc PR | - This PR fixes an issue with the UniqueEntityValidator in case the entity being validated uses a composite primary key via relations to other entities whose classes do not have a `__toString` method. Commits ------- b3ced86 [DoctrineBridge] Fixed invalid unique value as composite key 5aadce3 [Doctrine Bridge] fix UniqueEntityValidator for composite object primary keys
This PR fixes an issue with the UniqueEntityValidator in case the entity being validated uses a composite primary key via relations to other entities whose classes do not have a
__toStringmethod.