Make users add mapping to tree closure tables

CHANGELOG.md

@@ -34,6 +34,10 @@ a release.
 - Blameable, IpTraceable, Timestampable: Type handling for the tracked field values configured in the origin field.
 - Loggable: Using only PHP 8 attributes.
 - References: Avoid deprecations using LazyCollection with PHP 8.1
+- Tree: Association mapping problems using Closure tree strategy (by manually defining mapping on the closure entity).
+
+### Deprecated
+- Tree: When using Closure tree strategy, it is deprecated not defining the mapping associations of the closure entity.
 
 ## [3.4.0] - 2021-12-05
 ### Added

doc/tree.md

@@ -1249,7 +1249,7 @@ You must pass a value in seconds to this parameter.
 ## Closure Table
 
 To be able to use this strategy, you'll need an additional entity which represents the closures. We already provide you an abstract
-entity, so you only need to extend it.
+entity, so you need to extend from it and add mapping information for ancestor and descendant.
 
 ### Closure Entity
 
@@ -1263,9 +1263,29 @@ use Doctrine\ORM\Mapping as ORM;
 
 /**
  * @ORM\Entity
+ * @ORM\UniqueConstraint(name="closure_unique_idx", columns={"ancestor", "descendant"})
+ * @ORM\Index(name="closure_depth_idx", columns={"depth"})
  */
+#[ORM\Entity]
+#[ORM\UniqueConstraint(name: 'closure_unique_idx', columns: ['ancestor', 'descendant'])]
+#[ORM\Index(name: 'closure_depth_idx', columns: ['depth'])]
 class CategoryClosure extends AbstractClosure
 {
+    /**
+     * @ORM\ManyToOne(targetEntity="YourNamespace\Entity\Category")
+     * @ORM\JoinColumn(name="ancestor", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Category::class)]
+    #[ORM\JoinColumn(name: 'ancestor', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $ancestor;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="YourNamespace\Entity\Category")
+     * @ORM\JoinColumn(name="descendant", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Category::class)]
+    #[ORM\JoinColumn(name: 'descendant', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $descendant;
 }
 ```
 

src/Tree/Strategy/ORM/Closure.php

@@ -13,6 +13,7 @@
 use Doctrine\DBAL\Connection;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\Mapping\ClassMetadataInfo;
+use Doctrine\Persistence\Mapping\ClassMetadata;
 use Doctrine\Persistence\ObjectManager;
 use Gedmo\Exception\RuntimeException;
 use Gedmo\Mapping\Event\AdapterInterface;
@@ -83,11 +84,22 @@ public function getName()
      */
     public function processMetadataLoad($em, $meta)
     {
+        // TODO: Remove the body of this method in the next major version.
         $config = $this->listener->getConfiguration($em, $meta->getName());
         $closureMetadata = $em->getClassMetadata($config['closure']);
         $cmf = $em->getMetadataFactory();
 
+        $hasTheUserExplicitlyDefinedMapping = true;
+
         if (!$closureMetadata->hasAssociation('ancestor')) {
+            @trigger_error(sprintf(
+                'Not adding mapping explicitly to "ancestor" property in "%s" is deprecated and will not work in'
+                .' version 4.0. You MUST explicitly set the mapping as in our docs: https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/tree.md#closure-table',
+                $closureMetadata->getName()
+            ), E_USER_DEPRECATED);
+
+            $hasTheUserExplicitlyDefinedMapping = false;
+
             // create ancestor mapping
             $ancestorMapping = [
                 'fieldName' => 'ancestor',
@@ -116,6 +128,14 @@ public function processMetadataLoad($em, $meta)
         }
 
         if (!$closureMetadata->hasAssociation('descendant')) {
+            @trigger_error(sprintf(
+                'Not adding mapping explicitly to "descendant" property in "%s" is deprecated and will not work in'
+                .' version 4.0. You MUST explicitly set the mapping as in our docs: https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/tree.md#closure-table',
+                $closureMetadata->getName()
+            ), E_USER_DEPRECATED);
+
+            $hasTheUserExplicitlyDefinedMapping = false;
+
             // create descendant mapping
             $descendantMapping = [
                 'fieldName' => 'descendant',
@@ -142,24 +162,48 @@ public function processMetadataLoad($em, $meta)
                 ->getAccessibleProperty($closureMetadata->getName(), 'descendant')
             ;
         }
-        // create unique index on ancestor and descendant
-        $indexName = substr(strtoupper('IDX_'.md5($closureMetadata->getName())), 0, 20);
-        $closureMetadata->table['uniqueConstraints'][$indexName] = [
-            'columns' => [
-                $this->getJoinColumnFieldName($em->getClassMetadata($config['closure'])->getAssociationMapping('ancestor')),
-                $this->getJoinColumnFieldName($em->getClassMetadata($config['closure'])->getAssociationMapping('descendant')),
-            ],
-        ];
-        // this one may not be very useful
-        $indexName = substr(strtoupper('IDX_'.md5($meta->getName().'depth')), 0, 20);
-        $closureMetadata->table['indexes'][$indexName] = [
-            'columns' => ['depth'],
-        ];
-
-        $cacheDriver = $cmf->getCacheDriver();
-
-        if ($cacheDriver instanceof Cache) {
-            $cacheDriver->save($closureMetadata->getName().'$CLASSMETADATA', $closureMetadata);
+
+        if (!$this->hasClosureTableUniqueConstraint($closureMetadata)) {
+            @trigger_error(sprintf(
+                'Not adding a unique constraint explicitly to "%s" is deprecated and will not be automatically'
+                .' added in version 4.0. You SHOULD explicitly add the unique constraint as in our docs: https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/tree.md#closure-table',
+                $closureMetadata->getName()
+            ), E_USER_DEPRECATED);
+
+            $hasTheUserExplicitlyDefinedMapping = false;
+
+            // create unique index on ancestor and descendant
+            $indexName = substr(strtoupper('IDX_'.md5($closureMetadata->getName())), 0, 20);
+            $closureMetadata->table['uniqueConstraints'][$indexName] = [
+                'columns' => [
+                    $this->getJoinColumnFieldName($em->getClassMetadata($config['closure'])->getAssociationMapping('ancestor')),
+                    $this->getJoinColumnFieldName($em->getClassMetadata($config['closure'])->getAssociationMapping('descendant')),
+                ],
+            ];
+        }
+
+        if (!$this->hasClosureTableDepthIndex($closureMetadata)) {
+            @trigger_error(sprintf(
+                'Not adding an index with "depth" column explicitly to "%s" is deprecated and will not be automatically'
+                .' added in version 4.0. You SHOULD explicitly add the index as in our docs: https://github.com/doctrine-extensions/DoctrineExtensions/blob/main/doc/tree.md#closure-table',
+                $closureMetadata->getName()
+            ), E_USER_DEPRECATED);
+
+            $hasTheUserExplicitlyDefinedMapping = false;
+
+            // this one may not be very useful
+            $indexName = substr(strtoupper('IDX_'.md5($meta->getName().'depth')), 0, 20);
+            $closureMetadata->table['indexes'][$indexName] = [
+                'columns' => ['depth'],
+            ];
+        }
+
+        if (!$hasTheUserExplicitlyDefinedMapping) {
+            $cacheDriver = $cmf->getCacheDriver();
+
+            if ($cacheDriver instanceof Cache) {
+                $cacheDriver->save($closureMetadata->getName().'$CLASSMETADATA', $closureMetadata);
+            }
         }
     }
 
@@ -467,4 +511,34 @@ protected function setLevelFieldOnPendingNodes(ObjectManager $em)
             $this->pendingNodesLevelProcess = [];
         }
     }
+
+    private function hasClosureTableUniqueConstraint(ClassMetadata $closureMetadata): bool
+    {
+        if (!isset($closureMetadata->table['uniqueConstraints'])) {
+            return false;
+        }
+
+        foreach ($closureMetadata->table['uniqueConstraints'] as $uniqueConstraint) {
+            if ([] === array_diff(['ancestor', 'descendant'], $uniqueConstraint['columns'])) {
+                return true;
+            }
+        }
+
+        return false;
+    }
+
+    private function hasClosureTableDepthIndex(ClassMetadata $closureMetadata): bool
+    {
+        if (!isset($closureMetadata->table['indexes'])) {
+            return false;
+        }
+
+        foreach ($closureMetadata->table['indexes'] as $uniqueConstraint) {
+            if ([] === array_diff(['depth'], $uniqueConstraint['columns'])) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }

tests/Gedmo/Mapping/Driver/Yaml/Gedmo.Tests.Mapping.Fixture.Yaml.ClosureCategory.dcm.yml

@@ -10,7 +10,7 @@ Gedmo\Tests\Mapping\Fixture\Yaml\ClosureCategory:
   gedmo:
     tree:
       type: closure
-      closure: Gedmo\Tests\Tree\Fixture\Closure\CategoryClosure
+      closure: Gedmo\Tests\Tree\Fixture\Closure\CategoryClosureWithoutMapping
   fields:
     title:
       type: string

tests/Gedmo/Mapping/Fixture/ClosureTreeClosure.php

@@ -12,11 +12,36 @@
 namespace Gedmo\Tests\Mapping\Fixture;
 
 use Doctrine\ORM\Mapping as ORM;
+use Gedmo\Tests\Mapping\Fixture\Xml\ClosureTree;
 use Gedmo\Tree\Entity\MappedSuperclass\AbstractClosure;
 
 /**
  * @ORM\Entity
+ * @ORM\Table(
+ *         indexes={@ORM\Index(name="closure_tree_depth_idx", columns={"depth"})},
+ *         uniqueConstraints={@ORM\UniqueConstraint(name="closure_tree_unique_idx", columns={
+ *             "ancestor", "descendant"
+ *         })}
+ * )
  */
+#[ORM\Entity]
+#[ORM\UniqueConstraint(name: 'closure_tree_unique_idx', columns: ['ancestor', 'descendant'])]
+#[ORM\Index(name: 'closure_tree_depth_idx', columns: ['depth'])]
 class ClosureTreeClosure extends AbstractClosure
 {
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Mapping\Fixture\Xml\ClosureTree")
+     * @ORM\JoinColumn(name="ancestor", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: ClosureTree::class)]
+    #[ORM\JoinColumn(name: 'ancestor', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $ancestor;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Mapping\Fixture\Xml\ClosureTree")
+     * @ORM\JoinColumn(name="descendant", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: ClosureTree::class)]
+    #[ORM\JoinColumn(name: 'descendant', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $descendant;
 }

tests/Gedmo/Mapping/TreeMappingTest.php

@@ -17,7 +17,7 @@
 use Gedmo\Tests\Mapping\Fixture\Yaml\Category;
 use Gedmo\Tests\Mapping\Fixture\Yaml\ClosureCategory;
 use Gedmo\Tests\Mapping\Fixture\Yaml\MaterializedPathCategory;
-use Gedmo\Tests\Tree\Fixture\Closure\CategoryClosure;
+use Gedmo\Tests\Tree\Fixture\Closure\CategoryClosureWithoutMapping;
 use Gedmo\Tree\TreeListener;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 
@@ -78,10 +78,10 @@ protected function setUp(): void
     public function testApcCached(): void
     {
         $this->em->getClassMetadata(self::YAML_CLOSURE_CATEGORY);
-        $this->em->getClassMetadata(CategoryClosure::class);
+        $this->em->getClassMetadata(CategoryClosureWithoutMapping::class);
 
         $meta = $this->em->getMetadataFactory()->getCacheDriver()->fetch(
-            'Gedmo\\Tests\\Tree\\Fixture\\Closure\\CategoryClosure$CLASSMETADATA'
+            'Gedmo\\Tests\\Tree\\Fixture\\Closure\\CategoryClosureWithoutMapping$CLASSMETADATA'
         );
         static::assertTrue($meta->hasAssociation('ancestor'));
         static::assertTrue($meta->hasAssociation('descendant'));
@@ -120,7 +120,7 @@ public function testYamlClosureMapping(): void
         static::assertArrayHasKey('strategy', $config);
         static::assertSame('closure', $config['strategy']);
         static::assertArrayHasKey('closure', $config);
-        static::assertSame(CategoryClosure::class, $config['closure']);
+        static::assertSame(CategoryClosureWithoutMapping::class, $config['closure']);
     }
 
     public function testYamlMaterializedPathMapping(): void

tests/Gedmo/Tree/Fixture/Closure/CategoryClosure.php

@@ -16,8 +16,31 @@
 
 /**
  * @ORM\Entity
+ * @ORM\Table(
+ *         indexes={@ORM\Index(name="closure_category_depth_idx", columns={"depth"})},
+ *         uniqueConstraints={@ORM\UniqueConstraint(name="closure_category_unique_idx", columns={
+ *             "ancestor", "descendant"
+ *         })}
+ * )
  */
 #[ORM\Entity]
+#[ORM\UniqueConstraint(name: 'closure_category_unique_idx', columns: ['ancestor', 'descendant'])]
+#[ORM\Index(name: 'closure_category_depth_idx', columns: ['depth'])]
 class CategoryClosure extends AbstractClosure
 {
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Closure\Category")
+     * @ORM\JoinColumn(name="ancestor", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Category::class)]
+    #[ORM\JoinColumn(name: 'ancestor', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $ancestor;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Closure\Category")
+     * @ORM\JoinColumn(name="descendant", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Category::class)]
+    #[ORM\JoinColumn(name: 'descendant', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $descendant;
 }

tests/Gedmo/Tree/Fixture/Closure/CategoryClosureWithoutMapping.php

@@ -0,0 +1,23 @@
+<?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\Closure;
+
+use Doctrine\ORM\Mapping as ORM;
+use Gedmo\Tree\Entity\MappedSuperclass\AbstractClosure;
+
+/**
+ * @ORM\Entity
+ */
+#[ORM\Entity]
+class CategoryClosureWithoutMapping extends AbstractClosure
+{
+}

tests/Gedmo/Tree/Fixture/Closure/CategoryWithoutLevelClosure.php

@@ -16,8 +16,31 @@
 
 /**
  * @ORM\Entity
+ * @ORM\Table(
+ *         indexes={@ORM\Index(name="closure_category_without_level_depth_idx", columns={"depth"})},
+ *         uniqueConstraints={@ORM\UniqueConstraint(name="closure_category_without_level_unique_idx", columns={
+ *             "ancestor", "descendant"
+ *         })}
+ * )
  */
 #[ORM\Entity]
+#[ORM\UniqueConstraint(name: 'closure_category_without_level_unique_idx', columns: ['ancestor', 'descendant'])]
+#[ORM\Index(name: 'closure_category_without_level_depth_idx', columns: ['depth'])]
 class CategoryWithoutLevelClosure extends AbstractClosure
 {
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Closure\CategoryWithoutLevel")
+     * @ORM\JoinColumn(name="ancestor", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: CategoryWithoutLevel::class)]
+    #[ORM\JoinColumn(name: 'ancestor', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $ancestor;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Closure\CategoryWithoutLevel")
+     * @ORM\JoinColumn(name="descendant", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: CategoryWithoutLevel::class)]
+    #[ORM\JoinColumn(name: 'descendant', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $descendant;
 }

tests/Gedmo/Tree/Fixture/Closure/PersonClosure.php

@@ -16,8 +16,31 @@
 
 /**
  * @ORM\Entity
+ * @ORM\Table(
+ *         indexes={@ORM\Index(name="closure_person_depth_idx", columns={"depth"})},
+ *         uniqueConstraints={@ORM\UniqueConstraint(name="closure_person_unique_idx", columns={
+ *             "ancestor", "descendant"
+ *         })}
+ * )
  */
 #[ORM\Entity]
+#[ORM\UniqueConstraint(name: 'closure_person_unique_idx', columns: ['ancestor', 'descendant'])]
+#[ORM\Index(name: 'closure_person_depth_idx', columns: ['depth'])]
 class PersonClosure extends AbstractClosure
 {
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Closure\Person")
+     * @ORM\JoinColumn(name="ancestor", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Person::class)]
+    #[ORM\JoinColumn(name: 'ancestor', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $ancestor;
+
+    /**
+     * @ORM\ManyToOne(targetEntity="Gedmo\Tests\Tree\Fixture\Closure\Person")
+     * @ORM\JoinColumn(name="descendant", referencedColumnName="id", nullable=false, onDelete="CASCADE")
+     */
+    #[ORM\ManyToOne(targetEntity: Person::class)]
+    #[ORM\JoinColumn(name: 'descendant', referencedColumnName: 'id', nullable: false, onDelete: 'CASCADE')]
+    protected $descendant;
 }