From f50803ccb91c5c31aede16df73fa7560a42f02ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Michael=20Ol=C5=A1avsk=C3=BD?= <molsavsky1@gmail.com> Date: Wed, 2 Aug 2023 13:44:15 +0200 Subject: [PATCH] Fix UnitOfWork->originalEntityData is missing not-modified collections after computeChangeSet (#9301) * Fix original data incomplete after flush * Apply suggestions from code review Co-authored-by: Alexander M. Turek <me@derrabus.de> --------- Co-authored-by: Alexander M. Turek <me@derrabus.de> --- lib/Doctrine/ORM/UnitOfWork.php | 1 + .../Tests/Models/Issue9300/Issue9300Child.php | 44 ++++++++++ .../Models/Issue9300/Issue9300Parent.php | 30 +++++++ .../ORM/Functional/Ticket/Issue9300Test.php | 80 +++++++++++++++++++ .../Doctrine/Tests/OrmFunctionalTestCase.php | 4 + 5 files changed, 159 insertions(+) create mode 100644 tests/Doctrine/Tests/Models/Issue9300/Issue9300Child.php create mode 100644 tests/Doctrine/Tests/Models/Issue9300/Issue9300Parent.php create mode 100644 tests/Doctrine/Tests/ORM/Functional/Ticket/Issue9300Test.php diff --git a/lib/Doctrine/ORM/UnitOfWork.php b/lib/Doctrine/ORM/UnitOfWork.php index 5bf8a6b6085..f9b8cf2b892 100644 --- a/lib/Doctrine/ORM/UnitOfWork.php +++ b/lib/Doctrine/ORM/UnitOfWork.php @@ -692,6 +692,7 @@ public function computeChangeSet(ClassMetadata $class, $entity) if ($class->isCollectionValuedAssociation($name) && $value !== null) { if ($value instanceof PersistentCollection) { if ($value->getOwner() === $entity) { + $actualData[$name] = $value; continue; } diff --git a/tests/Doctrine/Tests/Models/Issue9300/Issue9300Child.php b/tests/Doctrine/Tests/Models/Issue9300/Issue9300Child.php new file mode 100644 index 00000000000..0919d6d33b4 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Issue9300/Issue9300Child.php @@ -0,0 +1,44 @@ +<?php + +declare(strict_types=1); + +namespace Doctrine\Tests\Models\Issue9300; + +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; +use Doctrine\ORM\Mapping\Column; +use Doctrine\ORM\Mapping\Entity; +use Doctrine\ORM\Mapping\GeneratedValue; +use Doctrine\ORM\Mapping\Id; +use Doctrine\ORM\Mapping\ManyToMany; + +/** + * @Entity + */ +class Issue9300Child +{ + /** + * @var int + * @Id + * @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @var Collection<int, Issue9300Parent> + * @ManyToMany(targetEntity="Issue9300Parent") + */ + public $parents; + + /** + * @var string + * @Column(type="string") + */ + public $name; + + public function __construct() + { + $this->parents = new ArrayCollection(); + } +} diff --git a/tests/Doctrine/Tests/Models/Issue9300/Issue9300Parent.php b/tests/Doctrine/Tests/Models/Issue9300/Issue9300Parent.php new file mode 100644 index 00000000000..f9b50b36c95 --- /dev/null +++ b/tests/Doctrine/Tests/Models/Issue9300/Issue9300Parent.php @@ -0,0 +1,30 @@ +<?php + +declare(strict_types=1); + +namespace Doctrine\Tests\Models\Issue9300; + +use Doctrine\ORM\Mapping\Column; +use Doctrine\ORM\Mapping\Entity; +use Doctrine\ORM\Mapping\GeneratedValue; +use Doctrine\ORM\Mapping\Id; + +/** + * @Entity + */ +class Issue9300Parent +{ + /** + * @var int + * @Id + * @Column(type="integer") + * @GeneratedValue + */ + public $id; + + /** + * @var string + * @Column(type="string") + */ + public $name; +} diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/Issue9300Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/Issue9300Test.php new file mode 100644 index 00000000000..5a6309e433c --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/Issue9300Test.php @@ -0,0 +1,80 @@ +<?php + +declare(strict_types=1); + +namespace Doctrine\Tests\ORM\Functional\Ticket; + +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Tests\Models\Issue9300\Issue9300Child; +use Doctrine\Tests\Models\Issue9300\Issue9300Parent; +use Doctrine\Tests\OrmFunctionalTestCase; + +/** + * @group GH-9300 + */ +class Issue9300Test extends OrmFunctionalTestCase +{ + protected function setUp(): void + { + $this->useModelSet('issue9300'); + + parent::setUp(); + } + + /** + * @group GH-9300 + */ + public function testPersistedCollectionIsPresentInOriginalDataAfterFlush(): void + { + $parent = new Issue9300Parent(); + $child = new Issue9300Child(); + $child->parents->add($parent); + + $parent->name = 'abc'; + $child->name = 'abc'; + + $this->_em->persist($parent); + $this->_em->persist($child); + $this->_em->flush(); + + $parent->name = 'abcd'; + $child->name = 'abcd'; + + $this->_em->flush(); + + self::assertArrayHasKey('parents', $this->_em->getUnitOfWork()->getOriginalEntityData($child)); + } + + /** + * @group GH-9300 + */ + public function testPersistingCollectionAfterFlushWorksAsExpected(): void + { + $parentOne = new Issue9300Parent(); + $parentTwo = new Issue9300Parent(); + $childOne = new Issue9300Child(); + + $parentOne->name = 'abc'; + $parentTwo->name = 'abc'; + $childOne->name = 'abc'; + $childOne->parents = new ArrayCollection([$parentOne]); + + $this->_em->persist($parentOne); + $this->_em->persist($parentTwo); + $this->_em->persist($childOne); + $this->_em->flush(); + + // Recalculate change-set -> new original data + $childOne->name = 'abcd'; + $this->_em->flush(); + + $childOne->parents = new ArrayCollection([$parentTwo]); + + $this->_em->flush(); + $this->_em->clear(); + + $childOneFresh = $this->_em->find(Issue9300Child::class, $childOne->id); + self::assertCount(1, $childOneFresh->parents); + self::assertEquals($parentTwo->id, $childOneFresh->parents[0]->id); + } +} diff --git a/tests/Doctrine/Tests/OrmFunctionalTestCase.php b/tests/Doctrine/Tests/OrmFunctionalTestCase.php index 07bf94eb654..4a64136f184 100644 --- a/tests/Doctrine/Tests/OrmFunctionalTestCase.php +++ b/tests/Doctrine/Tests/OrmFunctionalTestCase.php @@ -338,6 +338,10 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue5989\Issue5989Employee::class, Models\Issue5989\Issue5989Manager::class, ], + 'issue9300' => [ + Models\Issue9300\Issue9300Child::class, + Models\Issue9300\Issue9300Parent::class, + ], ]; /** @param class-string ...$models */