Fix @SequenceGeneratorDefinition inheritance, take 1

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,12 @@ 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 this class has a parent the id generator strategy is inherited.
+        // However this is only true if the hierarchy of parents contains the root entity,
+        // if it consists of mapped superclasses these don't necessarily include the id field.
+        if ($parent && $rootEntityFound) {
+            $this->inheritIdGeneratorMapping($class, $parent);
+        } else {
             $this->completeIdGeneratorMapping($class);
         }
 

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
+{
+}

tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php

@@ -163,22 +163,7 @@ public function testMappedSuperclassWithId(): void
     /**
      * @group DDC-1156
      * @group DDC-1218
-     */
-    public function testGeneratedValueFromMappedSuperclass(): void
-    {
-        $class = $this->cmf->getMetadataFor(SuperclassEntity::class);
-        assert($class instanceof ClassMetadata);
-
-        self::assertInstanceOf(IdSequenceGenerator::class, $class->idGenerator);
-        self::assertEquals(
-            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo'],
-            $class->sequenceGeneratorDefinition
-        );
-    }
-
-    /**
-     * @group DDC-1156
-     * @group DDC-1218
+     * @group GH-10927
      */
     public function testSequenceDefinitionInHierarchyWithSandwichMappedSuperclass(): void
     {
@@ -192,22 +177,6 @@ public function testSequenceDefinitionInHierarchyWithSandwichMappedSuperclass():
         );
     }
 
-    /**
-     * @group DDC-1156
-     * @group DDC-1218
-     */
-    public function testMultipleMappedSuperclasses(): void
-    {
-        $class = $this->cmf->getMetadataFor(MediumSuperclassEntity::class);
-        assert($class instanceof ClassMetadata);
-
-        self::assertInstanceOf(IdSequenceGenerator::class, $class->idGenerator);
-        self::assertEquals(
-            ['allocationSize' => 1, 'initialValue' => 10, 'sequenceName' => 'foo'],
-            $class->sequenceGeneratorDefinition
-        );
-    }
-
     /**
      * Ensure indexes are inherited from the mapped superclass.
      *