From 883f84d1bcfdd25642234163b056a6c926972d12 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Tue, 18 Mar 2025 13:42:55 +0100 Subject: [PATCH 1/9] adds testcase to reproduce https://github.com/doctrine-extensions/DoctrineExtensions/issues/2582 --- tests/Gedmo/Tree/Fixture/Issue2582/OU.php | 116 ++++++++++++++++ tests/Gedmo/Tree/Issue/Issue2582Test.php | 156 ++++++++++++++++++++++ 2 files changed, 272 insertions(+) create mode 100644 tests/Gedmo/Tree/Fixture/Issue2582/OU.php create mode 100644 tests/Gedmo/Tree/Issue/Issue2582Test.php diff --git a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php new file mode 100644 index 0000000000..51eb133709 --- /dev/null +++ b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php @@ -0,0 +1,116 @@ + true])] + #[Gedmo\TreeLeft] + private int $left = 1; + + /** + * @Gedmo\TreeLevel() + * + * @ORM\Column(name="lvl", type="integer", options={"unsigned"=true}) + */ + #[ORM\Column(name: 'lvl', type: 'integer', options: ['unsigned' => true])] + #[Gedmo\TreeLevel] + private int $level = 0; + + /** + * @Gedmo\TreeRight() + * + * @ORM\Column(name="rgt", type="integer", options={"unsigned"=true}) + */ + #[ORM\Column(name: 'rgt', type: 'integer', options: ['unsigned' => true])] + #[Gedmo\TreeRight] + private int $right = 2; + + /** + * @ORM\OneToMany(targetEntity="\Gedmo\Tests\Tree\Fixture\Issue2582\OU", mappedBy="parent") + * @ORM\OrderBy({"left" = "ASC"}) + */ + #[ORM\OneToMany(targetEntity: self::class, mappedBy: 'parent')] + #[ORM\OrderBy(['left' => 'ASC'])] + private Collection $children; + + public function __construct(string $id, ?self $parent = null) + { + $this->id = $id; + $this->children = new ArrayCollection(); + $this->parent = $parent; + if ($parent) { + $parent->children->add($this); + } + } + + public function getId(): string + { + return $this->id; + } + + public function getParent(): ?OU + { + return $this->parent; + } + + public function getLeft(): int + { + return $this->left; + } + + public function getLevel(): int + { + return $this->level; + } + + public function getRight(): int + { + return $this->right; + } + + public function getChildren(): Collection + { + return $this->children; + } +} diff --git a/tests/Gedmo/Tree/Issue/Issue2582Test.php b/tests/Gedmo/Tree/Issue/Issue2582Test.php new file mode 100644 index 0000000000..0e56683f08 --- /dev/null +++ b/tests/Gedmo/Tree/Issue/Issue2582Test.php @@ -0,0 +1,156 @@ + http://www.gediminasm.org + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Gedmo\Tests\Tree\Issue; + +use Doctrine\Common\EventManager; +use Gedmo\Tests\Tool\BaseTestCaseORM; +use Gedmo\Tests\Tree\Fixture\Issue2582\OU; +use Gedmo\Tree\TreeListener; + +class Issue2582Test extends BaseTestCaseORM +{ + private TreeListener $listener; + + protected function setUp(): void + { + parent::setUp(); + + $this->listener = new TreeListener(); + + $evm = new EventManager(); + $evm->addEventSubscriber($this->listener); + + $this->getDefaultMockSqliteEntityManager($evm); + } + + public function testInsertTwoRootsInOneFlush(): void + { + $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); + $ou11 = new OU('00000000-0000-0000-0000-000000000011', $ou1); + $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); + $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); + + + $this->em->persist($ou1); + $this->em->persist($ou11); + $this->em->persist($ou2); + $this->em->persist($ou21); + $this->em->flush(); + + $this->em->clear(); + + $expected = [ + ['00000000-0000-0000-0000-000000000001', null, 1, 0, 4], + ['00000000-0000-0000-0000-000000000011', '00000000-0000-0000-0000-000000000001', 2, 1, 3], + ['00000000-0000-0000-0000-000000000002', null, 5, 0, 8], + ['00000000-0000-0000-0000-000000000021', '00000000-0000-0000-0000-000000000002', 6, 1, 7], + ]; + foreach ($this->fetchAllOUs() as $i => $a) { + self::assertSame( + $expected[$i], + [ + $a->getId(), + $a->getParent()?->getId(), + $a->getLeft(), + $a->getLevel(), + $a->getRight(), + ], + ); + } + } + + public function testInsertTwoRootsInOneFlushRootsFirst(): void + { + $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); + $ou11 = new OU('00000000-0000-0000-0000-000000000011', $ou1); + $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); + $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); + + + $this->em->persist($ou1); + $this->em->persist($ou2); + $this->em->persist($ou11); + $this->em->persist($ou21); + $this->em->flush(); + + $this->em->clear(); + + $expected = [ + ['00000000-0000-0000-0000-000000000001', null, 1, 0, 4], + ['00000000-0000-0000-0000-000000000011', '00000000-0000-0000-0000-000000000001', 2, 1, 3], + ['00000000-0000-0000-0000-000000000002', null, 5, 0, 8], + ['00000000-0000-0000-0000-000000000021', '00000000-0000-0000-0000-000000000002', 6, 1, 7], + ]; + foreach ($this->fetchAllOUs() as $i => $a) { + self::assertSame( + $expected[$i], + [ + $a->getId(), + $a->getParent()?->getId(), + $a->getLeft(), + $a->getLevel(), + $a->getRight(), + ], + ); + } + } + + public function testInsertTwoRootsInTwoFlushes(): void + { + $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); + $ou11 = new OU('00000000-0000-0000-0000-000000000011', $ou1); + $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); + $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); + + + $this->em->persist($ou1); + $this->em->persist($ou11); + $this->em->flush(); + $this->em->persist($ou2); + $this->em->persist($ou21); + $this->em->flush(); + + $this->em->clear(); + + $expected = [ + ['00000000-0000-0000-0000-000000000001', null, 1, 0, 4], + ['00000000-0000-0000-0000-000000000011', '00000000-0000-0000-0000-000000000001', 2, 1, 3], + ['00000000-0000-0000-0000-000000000002', null, 5, 0, 8], + ['00000000-0000-0000-0000-000000000021', '00000000-0000-0000-0000-000000000002', 6, 1, 7], + ]; + foreach ($this->fetchAllOUs() as $i => $a) { + self::assertSame( + $expected[$i], + [ + $a->getId(), + $a->getParent()?->getId(), + $a->getLeft(), + $a->getLevel(), + $a->getRight(), + ], + ); + } + } + + private function fetchAllOUs(): array + { + $categoryRepo = $this->em->getRepository(OU::class); + return $categoryRepo + ->createQueryBuilder('ou') + ->orderBy('ou.left', 'ASC') + ->getQuery() + ->getResult(); + } + + protected function getUsedEntityFixtures(): array + { + return [OU::class]; + } +} From 5abc6a4b1a3eacec0b681bca88783f0cfe5ed7f1 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Tue, 18 Mar 2025 14:44:17 +0100 Subject: [PATCH 2/9] this fixes the issue although I am not 100% sure why Signed-off-by: Stefan Gehrig --- src/Tree/Strategy/ORM/Nested.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/Tree/Strategy/ORM/Nested.php b/src/Tree/Strategy/ORM/Nested.php index becef5a70e..a6fb33cb24 100644 --- a/src/Tree/Strategy/ORM/Nested.php +++ b/src/Tree/Strategy/ORM/Nested.php @@ -407,6 +407,11 @@ public function updateNode(EntityManagerInterface $em, $node, $parent, $position $wrapped->setPropertyValue($config['right'], $right); } $newRoot = $parentRoot; + + if (!isset($this->treeEdges[$meta->getName()])) { + $this->treeEdges[$meta->getName()] = $this->max($em, $config['useObjectClass'], $newRoot) + 1; + } + $this->treeEdges[$meta->getName()] += 2; } elseif (!isset($config['root']) || ($meta->isSingleValuedAssociation($config['root']) && null !== $parent && ($newRoot = $meta->getFieldValue($node, $config['root'])))) { if (!isset($this->treeEdges[$meta->getName()])) { From 43cfbe92822309c88afb10d278cfed4e542f5944 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Mon, 24 Mar 2025 21:17:34 +0100 Subject: [PATCH 3/9] fixes PHP 7.4 compatibility --- tests/Gedmo/Tree/Issue/Issue2582Test.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/Gedmo/Tree/Issue/Issue2582Test.php b/tests/Gedmo/Tree/Issue/Issue2582Test.php index 0e56683f08..4013f2a614 100644 --- a/tests/Gedmo/Tree/Issue/Issue2582Test.php +++ b/tests/Gedmo/Tree/Issue/Issue2582Test.php @@ -57,7 +57,7 @@ public function testInsertTwoRootsInOneFlush(): void $expected[$i], [ $a->getId(), - $a->getParent()?->getId(), + $a->getParent() ? $a->getParent()->getId() : null, $a->getLeft(), $a->getLevel(), $a->getRight(), @@ -93,7 +93,7 @@ public function testInsertTwoRootsInOneFlushRootsFirst(): void $expected[$i], [ $a->getId(), - $a->getParent()?->getId(), + $a->getParent() ? $a->getParent()->getId() : null, $a->getLeft(), $a->getLevel(), $a->getRight(), @@ -130,7 +130,7 @@ public function testInsertTwoRootsInTwoFlushes(): void $expected[$i], [ $a->getId(), - $a->getParent()?->getId(), + $a->getParent() ? $a->getParent()->getId() : null, $a->getLeft(), $a->getLevel(), $a->getRight(), From f2e8978e2365e77c812d8b7c821efe46e1135631 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Mon, 24 Mar 2025 21:18:55 +0100 Subject: [PATCH 4/9] fixes double ) in annotation --- tests/Gedmo/Tree/Fixture/Issue2582/OU.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php index 51eb133709..847b023feb 100644 --- a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php +++ b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php @@ -12,7 +12,7 @@ * * @ORM\Table(name="ous") * @ORM\Index(name="idx_tree", columns={"lft", "rgt"}) - * @ORM\Entity()) + * @ORM\Entity() */ #[ORM\Table(name: 'ous')] #[ORM\Index(name: 'idx_tree', columns: ['lft', 'rgt'])] From cd126dbc6a8949cb450b413d20823f6a6e34259b Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Tue, 25 Mar 2025 12:37:28 +0100 Subject: [PATCH 5/9] fixes index for doctrine orm annotations Signed-off-by: Stefan Gehrig --- tests/Gedmo/Tree/Fixture/Issue2582/OU.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php index 847b023feb..80dc77a2df 100644 --- a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php +++ b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php @@ -10,8 +10,12 @@ /** * @Gedmo\Tree(type="nested") * - * @ORM\Table(name="ous") - * @ORM\Index(name="idx_tree", columns={"lft", "rgt"}) + * @ORM\Table( + * name="ous", + * indexes={ + * @ORM\Index(name="idx_tree", fields={"left", "right"}) + * } + * ) * @ORM\Entity() */ #[ORM\Table(name: 'ous')] From d9d04c0787b7cdbed0e5376e98d0493b7d399354 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Wed, 26 Mar 2025 10:49:55 +0100 Subject: [PATCH 6/9] fixes PHP-CS and PHPStan reportings Signed-off-by: Stefan Gehrig --- tests/Gedmo/Tree/Fixture/Issue2582/OU.php | 18 ++++++++++--- tests/Gedmo/Tree/Issue/Issue2582Test.php | 31 +++++++++++++---------- 2 files changed, 32 insertions(+), 17 deletions(-) diff --git a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php index 80dc77a2df..0c4efc152a 100644 --- a/tests/Gedmo/Tree/Fixture/Issue2582/OU.php +++ b/tests/Gedmo/Tree/Fixture/Issue2582/OU.php @@ -1,5 +1,12 @@ http://www.gediminasm.org + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + namespace Gedmo\Tests\Tree\Fixture\Issue2582; use Doctrine\Common\Collections\ArrayCollection; @@ -73,6 +80,8 @@ class OU /** * @ORM\OneToMany(targetEntity="\Gedmo\Tests\Tree\Fixture\Issue2582\OU", mappedBy="parent") * @ORM\OrderBy({"left" = "ASC"}) + * + * @var Collection */ #[ORM\OneToMany(targetEntity: self::class, mappedBy: 'parent')] #[ORM\OrderBy(['left' => 'ASC'])] @@ -80,9 +89,9 @@ class OU public function __construct(string $id, ?self $parent = null) { - $this->id = $id; + $this->id = $id; $this->children = new ArrayCollection(); - $this->parent = $parent; + $this->parent = $parent; if ($parent) { $parent->children->add($this); } @@ -93,7 +102,7 @@ public function getId(): string return $this->id; } - public function getParent(): ?OU + public function getParent(): ?self { return $this->parent; } @@ -113,6 +122,9 @@ public function getRight(): int return $this->right; } + /** + * @return Collection + */ public function getChildren(): Collection { return $this->children; diff --git a/tests/Gedmo/Tree/Issue/Issue2582Test.php b/tests/Gedmo/Tree/Issue/Issue2582Test.php index 4013f2a614..d15b1883f7 100644 --- a/tests/Gedmo/Tree/Issue/Issue2582Test.php +++ b/tests/Gedmo/Tree/Issue/Issue2582Test.php @@ -32,9 +32,9 @@ protected function setUp(): void public function testInsertTwoRootsInOneFlush(): void { - $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); + $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); $ou11 = new OU('00000000-0000-0000-0000-000000000011', $ou1); - $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); + $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); @@ -53,7 +53,7 @@ public function testInsertTwoRootsInOneFlush(): void ['00000000-0000-0000-0000-000000000021', '00000000-0000-0000-0000-000000000002', 6, 1, 7], ]; foreach ($this->fetchAllOUs() as $i => $a) { - self::assertSame( + static::assertSame( $expected[$i], [ $a->getId(), @@ -68,9 +68,9 @@ public function testInsertTwoRootsInOneFlush(): void public function testInsertTwoRootsInOneFlushRootsFirst(): void { - $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); + $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); $ou11 = new OU('00000000-0000-0000-0000-000000000011', $ou1); - $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); + $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); @@ -89,7 +89,7 @@ public function testInsertTwoRootsInOneFlushRootsFirst(): void ['00000000-0000-0000-0000-000000000021', '00000000-0000-0000-0000-000000000002', 6, 1, 7], ]; foreach ($this->fetchAllOUs() as $i => $a) { - self::assertSame( + static::assertSame( $expected[$i], [ $a->getId(), @@ -104,9 +104,9 @@ public function testInsertTwoRootsInOneFlushRootsFirst(): void public function testInsertTwoRootsInTwoFlushes(): void { - $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); + $ou1 = new OU('00000000-0000-0000-0000-000000000001', null); $ou11 = new OU('00000000-0000-0000-0000-000000000011', $ou1); - $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); + $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); @@ -126,7 +126,7 @@ public function testInsertTwoRootsInTwoFlushes(): void ['00000000-0000-0000-0000-000000000021', '00000000-0000-0000-0000-000000000002', 6, 1, 7], ]; foreach ($this->fetchAllOUs() as $i => $a) { - self::assertSame( + static::assertSame( $expected[$i], [ $a->getId(), @@ -139,6 +139,14 @@ public function testInsertTwoRootsInTwoFlushes(): void } } + protected function getUsedEntityFixtures(): array + { + return [OU::class]; + } + + /** + * @return list + */ private function fetchAllOUs(): array { $categoryRepo = $this->em->getRepository(OU::class); @@ -148,9 +156,4 @@ private function fetchAllOUs(): array ->getQuery() ->getResult(); } - - protected function getUsedEntityFixtures(): array - { - return [OU::class]; - } } From cf528c87303a75cb550ebbfcf8dd435399950f30 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Sat, 29 Mar 2025 18:52:41 +0100 Subject: [PATCH 7/9] fixes sone more PHP-CS reportings Signed-off-by: Stefan Gehrig --- tests/Gedmo/Tree/Issue/Issue2582Test.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/Gedmo/Tree/Issue/Issue2582Test.php b/tests/Gedmo/Tree/Issue/Issue2582Test.php index d15b1883f7..cd889a6cb1 100644 --- a/tests/Gedmo/Tree/Issue/Issue2582Test.php +++ b/tests/Gedmo/Tree/Issue/Issue2582Test.php @@ -37,7 +37,6 @@ public function testInsertTwoRootsInOneFlush(): void $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); - $this->em->persist($ou1); $this->em->persist($ou11); $this->em->persist($ou2); @@ -73,7 +72,6 @@ public function testInsertTwoRootsInOneFlushRootsFirst(): void $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); - $this->em->persist($ou1); $this->em->persist($ou2); $this->em->persist($ou11); @@ -109,7 +107,6 @@ public function testInsertTwoRootsInTwoFlushes(): void $ou2 = new OU('00000000-0000-0000-0000-000000000002', null); $ou21 = new OU('00000000-0000-0000-0000-000000000021', $ou2); - $this->em->persist($ou1); $this->em->persist($ou11); $this->em->flush(); @@ -150,6 +147,7 @@ protected function getUsedEntityFixtures(): array private function fetchAllOUs(): array { $categoryRepo = $this->em->getRepository(OU::class); + return $categoryRepo ->createQueryBuilder('ou') ->orderBy('ou.left', 'ASC') From d29bde7df2cc10b8112200bd7eece32bf290e348 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Thu, 25 Sep 2025 10:43:08 +0200 Subject: [PATCH 8/9] made test class final as per suggestion Signed-off-by: Stefan Gehrig --- tests/Gedmo/Tree/Issue/Issue2582Test.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Gedmo/Tree/Issue/Issue2582Test.php b/tests/Gedmo/Tree/Issue/Issue2582Test.php index cd889a6cb1..fe481adf86 100644 --- a/tests/Gedmo/Tree/Issue/Issue2582Test.php +++ b/tests/Gedmo/Tree/Issue/Issue2582Test.php @@ -14,7 +14,7 @@ use Gedmo\Tests\Tree\Fixture\Issue2582\OU; use Gedmo\Tree\TreeListener; -class Issue2582Test extends BaseTestCaseORM +final class Issue2582Test extends BaseTestCaseORM { private TreeListener $listener; From ce527f3a9ca6a03c78cdac19f61d27fc62d4bab9 Mon Sep 17 00:00:00 2001 From: Stefan Gehrig Date: Thu, 25 Sep 2025 10:49:30 +0200 Subject: [PATCH 9/9] [#2582] added required changelog entry Signed-off-by: Stefan Gehrig --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e6fd39c05c..6bdc3fcd21 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,8 @@ a release. --- ## [Unreleased] +### Fixed +- Tree: Fixed inserting multiple root nodes in one flush operation with the nested set strategy in certain circumstances (#2582) ## [3.20.0] - 2025-04-04 ### Fixed