Allow multiple entities with closure strategy

CHANGELOG.md

@@ -78,6 +78,7 @@ a release.
 ### Fixed
 - References: fixed condition in `XML` Driver that did not allow to retrieve from the entity definition the `mappedBy` and `inversedBy` fields.
 - Fix bug collecting metadata for inherited mapped classes
+- Tree: Fix issue with null ids inserted into closure table when persisting more than one type of entity using the closure strategy (#2653)
 
 ## [3.12.0] - 2023-07-08
 ### Added

composer.json

@@ -57,19 +57,17 @@
         "doctrine/dbal": "^3.2",
         "doctrine/doctrine-bundle": "^2.3",
         "doctrine/mongodb-odm": "^2.3",
-        "doctrine/orm": "^2.14.0 || ^3.0",
-        "friendsofphp/php-cs-fixer": "^3.14.0",
-        "nesbot/carbon": "^2.71 || ^3.0",
-        "phpstan/phpstan": "^1.11",
-        "phpstan/phpstan-doctrine": "^1.4",
-        "phpstan/phpstan-phpunit": "^1.4",
-        "phpunit/phpunit": "^9.6",
-        "rector/rector": "^1.1",
-        "symfony/console": "^5.4 || ^6.0 || ^7.0",
-        "symfony/doctrine-bridge": "^5.4 || ^6.0 || ^7.0",
-        "symfony/phpunit-bridge": "^6.0 || ^7.0",
-        "symfony/uid": "^5.4 || ^6.0 || ^7.0",
-        "symfony/yaml": "^5.4 || ^6.0 || ^7.0"
+        "doctrine/orm": "^2.14.0",
+        "friendsofphp/php-cs-fixer": "^3.4.0 <3.10",
+        "nesbot/carbon": "^2.55",
+        "phpstan/phpstan": "^1.10.2",
+        "phpstan/phpstan-doctrine": "^1.0",
+        "phpstan/phpstan-phpunit": "^1.0",
+        "phpunit/phpunit": "^8.5 || ^9.5",
+        "rector/rector": "^0.15.20",
+        "symfony/console": "^4.4 || ^5.3 || ^6.0",
+        "symfony/phpunit-bridge": "^6.0",
+        "symfony/yaml": "^4.4 || ^5.3 || ^6.0"
     },
     "conflict": {
         "doctrine/annotations": "<1.13 || >=3.0",

src/Tree/Strategy/ORM/Closure.php

@@ -285,12 +285,20 @@ public function processPostPersist($em, $entity, AdapterInterface $ea)
         $uow = $em->getUnitOfWork();
         $emHash = spl_object_id($em);
 
-        while ($node = array_shift($this->pendingChildNodeInserts[$emHash])) {
+        foreach ($this->pendingChildNodeInserts[$emHash] as $node) {
             $meta = $em->getClassMetadata(get_class($node));
-            $config = $this->listener->getConfiguration($em, $meta->getName());
-
             $identifier = $meta->getSingleIdentifierFieldName();
             $nodeId = $meta->getReflectionProperty($identifier)->getValue($node);
+
+            if (null === $nodeId) {
+                // Do not update if the node has not been persisted yet.
+                continue;
+            }
+
+            // The closure for this node will now be inserted. Remove the node from the list of pending inserts to indicate this.
+            unset($this->pendingChildNodeInserts[$emHash][spl_object_id($node)]);
+
+            $config = $this->listener->getConfiguration($em, $meta->getName());
             $parent = $meta->getReflectionProperty($config['parent'])->getValue($node);
 
             $closureClass = $config['closure'];
@@ -349,6 +357,13 @@ public function processPostPersist($em, $entity, AdapterInterface $ea)
             }
 
             foreach ($entries as $closure) {
+                $existingClosure = $em->getRepository($closureClass)->findBy([
+                    $ancestorColumnName => $closure[$ancestorColumnName],
+                    $descendantColumnName => $closure[$descendantColumnName],
+                ]);
+                if ([] !== $existingClosure) {
+                    continue;
+                }
                 if (!$em->getConnection()->insert($closureTable, $closure)) {
                     throw new RuntimeException('Failed to insert new Closure record');
                 }
@@ -500,13 +515,15 @@ protected function getJoinColumnFieldName($association)
      */
     protected function setLevelFieldOnPendingNodes(ObjectManager $em)
     {
-        if (!empty($this->pendingNodesLevelProcess)) {
+        while (!empty($this->pendingNodesLevelProcess)) {
+            // Nodes need to be processed one class at a time. Each iteration through the while loop will process one type, starting with the first item on the list.
             $first = array_slice($this->pendingNodesLevelProcess, 0, 1);
             $first = array_shift($first);
 
             assert(null !== $first);
 
-            $meta = $em->getClassMetadata(get_class($first));
+            $className = get_class($first);
+            $meta = $em->getClassMetadata($className);
             unset($first);
             $identifier = $meta->getIdentifier();
             $mapping = $meta->getFieldMapping($identifier[0]);
@@ -516,6 +533,10 @@ protected function setLevelFieldOnPendingNodes(ObjectManager $em)
             $uow = $em->getUnitOfWork();
 
             foreach ($this->pendingNodesLevelProcess as $node) {
+                if (get_class($node) !== $className) {
+                    // Only process nodes of the same type as the first element
+                    continue;
+                }
                 $children = $em->getRepository($meta->getName())->children($node);
 
                 foreach ($children as $child) {
@@ -543,6 +564,14 @@ protected function setLevelFieldOnPendingNodes(ObjectManager $em)
 
             // Now we update levels
             foreach ($this->pendingNodesLevelProcess as $nodeId => $node) {
+                if (get_class($node) !== $className) {
+                    // Only process nodes of the same time as the first element
+                    continue;
+                }
+
+                // This node will now be processed. Therefore, remove it from the list of pending nodes.
+                unset($this->pendingNodesLevelProcess[$nodeId]);
+
                 // Update new level
                 $level = $levels[$nodeId];
                 $levelProp = $meta->getReflectionProperty($config['level']);
@@ -555,8 +584,6 @@ protected function setLevelFieldOnPendingNodes(ObjectManager $em)
                 $levelProp->setValue($node, $level);
                 $uow->setOriginalEntityProperty(spl_object_id($node), $config['level'], $level);
             }
-
-            $this->pendingNodesLevelProcess = [];
         }
     }
 

tests/Gedmo/Tree/Fixture/Issue2652/Category2.php

@@ -0,0 +1,119 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Doctrine Behavioral Extensions package.
+ * (c) Gediminas Morkevicius <gediminas.morkevicius@gmail.com> 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\Issue2652;
+
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\Common\Collections\Collection;
+use Doctrine\DBAL\Types\Types;
+use Doctrine\ORM\Mapping as ORM;
+use Gedmo\Mapping\Annotation as Gedmo;
+use Gedmo\Tree\Entity\Repository\ClosureTreeRepository;
+
+/**
+ * @Gedmo\Tree(type="closure")
+ * @Gedmo\TreeClosure(class="Category2Closure")
+ * 
+ * @ORM\Entity(repositoryClass="Gedmo\Tree\Entity\Repository\ClosureTreeRepository")
+ */
+#[ORM\Entity(repositoryClass: ClosureTreeRepository::class)]
+#[Gedmo\Tree(type: 'closure')]
+#[Gedmo\TreeClosure(class: Category2Closure::class)]
+class Category2
+{
+    /**
+     * @var int|null
+     *
+     * @ORM\Id
+     * @ORM\GeneratedValue
+     * @ORM\Column(type="integer")
+     */
+    #[ORM\Id]
+    #[ORM\GeneratedValue]
+    #[ORM\Column(type: Types::INTEGER)]
+    private $id;
+
+    /**
+     * @ORM\Column(name="title", type="string", length=64)
+     */
+    #[ORM\Column(name: 'title', type: Types::STRING, length: 64)]
+    private ?string $title = null;
+
+    /**
+     * @ORM\Column(name="level", type="integer", nullable=true)
+     * 
+     * @Gedmo\TreeLevel
+     */
+    #[ORM\Column(name: 'level', type: Types::INTEGER, nullable: true)]
+    #[Gedmo\TreeLevel]
+    private ?int $level = null;
+
+    /**
+     * @Gedmo\TreeParent
+     * 
+     * @ORM\JoinColumn(name="parent_id", referencedColumnName="id", onDelete="CASCADE")
+     * @ORM\ManyToOne(targetEntity="Category2", inversedBy="children")
+     */
+    #[ORM\ManyToOne(targetEntity: self::class, inversedBy: 'children')]
+    #[ORM\JoinColumn(name: 'category2_id', referencedColumnName: 'id', onDelete: 'CASCADE')]
+    #[Gedmo\TreeParent]
+    private ?\Gedmo\Tests\Tree\Fixture\Issue2652\Category2 $parent = null;
+
+    /**
+     * @var Collection<int, Category2Closure>
+     */
+    private $closures;
+
+    public function __construct()
+    {
+        $this->closures = new ArrayCollection();
+    }
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+
+    public function setTitle(?string $title): void
+    {
+        $this->title = $title;
+    }
+
+    public function getTitle(): ?string
+    {
+        return $this->title;
+    }
+
+    public function setParent(?self $parent = null): void
+    {
+        $this->parent = $parent;
+    }
+
+    public function getParent(): ?self
+    {
+        return $this->parent;
+    }
+
+    public function addClosure(Category2Closure $closure): void
+    {
+        $this->closures[] = $closure;
+    }
+
+    public function setLevel(?int $level): void
+    {
+        $this->level = $level;
+    }
+
+    public function getLevel(): ?int
+    {
+        return $this->level;
+    }
+}

tests/Gedmo/Tree/Fixture/Issue2652/Category2Closure.php

@@ -0,0 +1,46 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Doctrine Behavioral Extensions package.
+ * (c) Gediminas Morkevicius <gediminas.morkevicius@gmail.com> 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\Issue2652;
+
+use Doctrine\ORM\Mapping as ORM;
+use Gedmo\Tree\Entity\MappedSuperclass\AbstractClosure;
+
+/**
+ * @ORM\Entity
+ * @ORM\Table(
+ *     indexes={@ORM\Index(name="closure_category2_depth_idx", columns={"depth"})},
+ *     uniqueConstraints={@ORM\UniqueConstraint(name="closure_category2_unique_idx", columns={
+ *         "ancestor", "descendant"
+ *     })}
+ * )
+ */
+#[ORM\Entity]
+#[ORM\UniqueConstraint(name: 'closure_category2_unique_idx', columns: ['ancestor', 'descendant'])]
+#[ORM\Index(name: 'closure_category2_depth_idx', columns: ['depth'])]
+class Category2Closure extends AbstractClosure
+{
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Issue2652\Category2")
+     * @ORM\JoinColumn(name="ancestor", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Category2::class)]
+    #[ORM\JoinColumn(name: 'ancestor', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $ancestor;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Issue2652\Category2")
+     * @ORM\JoinColumn(name="descendant", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Category2::class)]
+    #[ORM\JoinColumn(name: 'descendant', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $descendant;
+}

tests/Gedmo/Tree/Issue/Issue2652Test.php

@@ -0,0 +1,94 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the Doctrine Behavioral Extensions package.
+ * (c) Gediminas Morkevicius <gediminas.morkevicius@gmail.com> 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;
+
+use Doctrine\Common\EventManager;
+use Gedmo\Tests\Tool\BaseTestCaseORM;
+use Gedmo\Tests\Tree\Fixture\Closure\Category;
+use Gedmo\Tests\Tree\Fixture\Closure\CategoryClosure;
+use Gedmo\Tests\Tree\Fixture\Issue2652\Category2;
+use Gedmo\Tests\Tree\Fixture\Issue2652\Category2Closure;
+use Gedmo\Tree\TreeListener;
+
+/**
+ * These are tests for Tree behavior
+ *
+ * @author Gustavo Adrian <comfortablynumb84@gmail.com>
+ * @author Gediminas Morkevicius <gediminas.morkevicius@gmail.com>
+ */
+final class Issue2652Test extends BaseTestCaseORM
+{
+    private const CATEGORY = Category::class;
+    private const CLOSURE = CategoryClosure::class;
+    private const CATEGORY2 = Category2::class;
+    private const CLOSURE2 = Category2Closure::class;
+
+    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 testAddMultipleEntityTypes(): void
+    {
+        $food = new Category();
+        $food->setTitle('Food');
+        $this->em->persist($food);
+
+        $monkey = new Category2();
+        $monkey->setTitle('Monkey');
+        $this->em->persist($monkey);
+
+        // Before issue #2652 was fixed, null values would be inserted into the closure table at this next line and an exception would be thrown
+        $this->em->flush();
+
+        // Ensure that the closures were correctly inserted
+        $repo = $this->em->getRepository(self::CATEGORY);
+
+        $food = $repo->findOneBy(['title' => 'Food']);
+        $dql = 'SELECT c FROM '.self::CLOSURE.' c';
+        $dql .= ' WHERE c.ancestor = :ancestor';
+        $query = $this->em->createQuery($dql);
+        $query->setParameter('ancestor', $food);
+
+        $foodClosures = $query->getResult();
+        static::assertCount(1, $foodClosures);
+
+        $repo = $this->em->getRepository(self::CATEGORY2);
+        $monkey = $repo->findOneBy(['title' => 'Monkey']);
+        $dql = 'SELECT c FROM '.self::CLOSURE2.' c';
+        $dql .= ' WHERE c.ancestor = :ancestor';
+        $query = $this->em->createQuery($dql);
+        $query->setParameter('ancestor', $monkey);
+
+        $monkeyClosures = $query->getResult();
+        static::assertCount(1, $monkeyClosures);
+    }
+
+    protected function getUsedEntityFixtures(): array
+    {
+        return [
+            self::CATEGORY,
+            self::CLOSURE,
+            self::CATEGORY2,
+            self::CLOSURE2,
+        ];
+    }
+}