Fix @SequenceGeneratorDefinition inheritance

doctrine · Nov 6, 2023 · e87126c · e87126c
1 parent 609647a
commit e87126c
Show file tree

Hide file tree

Showing 4 changed files with 137 additions and 6 deletions.
diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
@@ -113,7 +113,7 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
         if ($parent) {
             $class->setInheritanceType($parent->inheritanceType);
             $class->setDiscriminatorColumn($parent->discriminatorColumn);
-            $this->inheritIdGeneratorMapping($class, $parent);
+            $class->setIdGeneratorType($parent->generatorType);
             $this->addInheritedFields($class, $parent);
             $this->addInheritedRelations($class, $parent);
             $this->addInheritedEmbeddedClasses($class, $parent);
@@ -141,8 +141,9 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
             throw MappingException::reflectionFailure($class->getName(), $e);
         }
 
-        // Complete id generator mapping when the generator was declared/added in this class
-        if ($class->identifier && (! $parent || ! $parent->identifier)) {
+        if ($parent && ($rootEntityFound || ($parent->sequenceGeneratorDefinition && $parent->sequenceGeneratorDefinition['explicit']))) {
+            $this->inheritIdGeneratorMapping($class, $parent);
+        } else {
             $this->completeIdGeneratorMapping($class);
         }
 
@@ -677,6 +678,7 @@ private function completeIdGeneratorMapping(ClassMetadataInfo $class): void
                         'sequenceName'      => $this->truncateSequenceName($sequenceName),
                         'allocationSize'    => 1,
                         'initialValue'      => 1,
+                        'explicit'          => false,
                     ];
 
                     if ($quoted) {

diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
@@ -3441,6 +3441,10 @@ public function setCustomGeneratorDefinition(array $definition)
      */
     public function setSequenceGeneratorDefinition(array $definition)
     {
+        if (! isset($definition['explicit'])) {
+            $definition['explicit'] = true;
+        }
+
         if (! isset($definition['sequenceName']) || trim($definition['sequenceName']) === '') {
             throw MappingException::missingSequenceName($this->name);
         }

diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10927Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH10927Test.php
@@ -0,0 +1,122 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Doctrine\Tests\ORM\Functional\Ticket;
+
+use Doctrine\DBAL\Platforms\PostgreSQLPlatform;
+use Doctrine\ORM\Mapping as ORM;
+use Doctrine\Tests\OrmFunctionalTestCase;
+
+/**
+ * @group GH-10927
+ */
+class GH10927Test extends OrmFunctionalTestCase
+{
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $platform = $this->_em->getConnection()->getDatabasePlatform();
+        if (! $platform instanceof PostgreSQLPlatform) {
+            self::markTestSkipped('The ' . self::class . ' requires the use of postgresql.');
+        }
+
+        $this->setUpEntitySchema([
+            GH10927RootMappedSuperclass::class,
+            GH10927InheritedMappedSuperclass::class,
+            GH10927EntityA::class,
+            GH10927EntityB::class,
+            GH10927EntityC::class,
+        ]);
+    }
+
+    public function testSequenceGeneratorDefinitionForRootMappedSuperclass(): void
+    {
+        $metadata = $this->_em->getClassMetadata(GH10927RootMappedSuperclass::class);
+
+        self::assertNull($metadata->sequenceGeneratorDefinition);
+    }
+
+    public function testSequenceGeneratorDefinitionForEntityA(): void
+    {
+        $metadata = $this->_em->getClassMetadata(GH10927EntityA::class);
+
+        self::assertSame('GH10927EntityA_id_seq', $metadata->sequenceGeneratorDefinition['sequenceName']);
+    }
+
+    public function testSequenceGeneratorDefinitionForInheritedMappedSuperclass(): void
+    {
+        $metadata = $this->_em->getClassMetadata(GH10927InheritedMappedSuperclass::class);
+
+        self::assertSame('GH10927InheritedMappedSuperclass_id_seq', $metadata->sequenceGeneratorDefinition['sequenceName']);
+    }
+
+    public function testSequenceGeneratorDefinitionForEntityB(): void
+    {
+        $metadata = $this->_em->getClassMetadata(GH10927EntityB::class);
+
+        self::assertSame('GH10927EntityB_id_seq', $metadata->sequenceGeneratorDefinition['sequenceName']);
+    }
+
+    public function testSequenceGeneratorDefinitionForEntityC(): void
+    {
+        $metadata = $this->_em->getClassMetadata(GH10927EntityC::class);
+
+        self::assertSame('GH10927EntityB_id_seq', $metadata->sequenceGeneratorDefinition['sequenceName']);
+    }
+}
+
+/**
+ * @ORM\MappedSuperclass()
+ */
+class GH10927RootMappedSuperclass
+{
+}
+
+/**
+ * @ORM\Entity()
+ */
+class GH10927EntityA extends GH10927RootMappedSuperclass
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue(strategy="SEQUENCE")
+     * @ORM\Column(type="integer")
+     *
+     * @var int|null
+     */
+    private $id = null;
+}
+
+/**
+ * @ORM\MappedSuperclass()
+ */
+class GH10927InheritedMappedSuperclass extends GH10927RootMappedSuperclass
+{
+    /**
+     * @ORM\Id
+     * @ORM\GeneratedValue(strategy="SEQUENCE")
+     * @ORM\Column(type="integer")
+     *
+     * @var int|null
+     */
+    private $id = null;
+}
+
+/**
+ * @ORM\Entity()
+ * @ORM\InheritanceType("JOINED")
+ * @ORM\DiscriminatorColumn(name="discr", type="string")
+ * @ORM\DiscriminatorMap({"B" = "GH10927EntityB", "C" = "GH10927EntityC"})
+ */
+class GH10927EntityB extends GH10927InheritedMappedSuperclass
+{
+}
+
+/**
+ * @ORM\Entity()
+ */
+class GH10927EntityC extends GH10927EntityB
+{
+}
diff --git a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php
@@ -163,6 +163,7 @@ public function testMappedSuperclassWithId(): void
     /**
      * @group DDC-1156
      * @group DDC-1218
+     * @group GH-10927
      */
     public function testGeneratedValueFromMappedSuperclass(): void
     {
@@ -171,14 +172,15 @@ public function testGeneratedValueFromMappedSuperclass(): void
 
         self::assertInstanceOf(IdSequenceGenerator::class, $class->idGenerator);
         self::assertEquals(
-            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo'],
+            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo', 'explicit' => true],
             $class->sequenceGeneratorDefinition
         );
     }
 
     /**
      * @group DDC-1156
      * @group DDC-1218
+     * @group GH-10927
      */
     public function testSequenceDefinitionInHierarchyWithSandwichMappedSuperclass(): void
     {
@@ -187,14 +189,15 @@ public function testSequenceDefinitionInHierarchyWithSandwichMappedSuperclass():
 
         self::assertInstanceOf(IdSequenceGenerator::class, $class->idGenerator);
         self::assertEquals(
-            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo'],
+            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo', 'explicit' => true],
             $class->sequenceGeneratorDefinition
         );
     }
 
     /**
      * @group DDC-1156
      * @group DDC-1218
+     * @group GH-10927
      */
     public function testMultipleMappedSuperclasses(): void
     {
@@ -203,7 +206,7 @@ public function testMultipleMappedSuperclasses(): void
 
         self::assertInstanceOf(IdSequenceGenerator::class, $class->idGenerator);
         self::assertEquals(
-            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo'],
+            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo', 'explicit' => true],
             $class->sequenceGeneratorDefinition
         );
     }