Skip to content

Fix @SequenceGeneratorDefinition inheritance, take 2 #11052

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 && ! isset($parent->sequenceGeneratorDefinition['implicit'])))) {
$this->inheritIdGeneratorMapping($class, $parent);
} else {
$this->completeIdGeneratorMapping($class);
}

Expand Down Expand Up @@ -677,6 +678,7 @@ private function completeIdGeneratorMapping(ClassMetadataInfo $class): void
'sequenceName' => $this->truncateSequenceName($sequenceName),
'allocationSize' => 1,
'initialValue' => 1,
'implicit' => true,
];

if ($quoted) {
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,7 @@ class ClassMetadataInfo implements ClassMetadata
* </code>
*
* @var array<string, mixed>|null
* @psalm-var array{sequenceName: string, allocationSize: string, initialValue: string, quoted?: mixed}|null
* @psalm-var array{sequenceName: string, allocationSize: string, initialValue: string, quoted?: mixed, implicit?: bool}|null
* @todo Merge with tableGeneratorDefinition into generic generatorDefinition
*/
public $sequenceGeneratorDefinition;
Expand Down Expand Up @@ -3433,7 +3433,7 @@ public function setCustomGeneratorDefinition(array $definition)
* )
* </code>
*
* @psalm-param array{sequenceName?: string, allocationSize?: int|string, initialValue?: int|string, quoted?: mixed} $definition
* @psalm-param array{sequenceName?: string, allocationSize?: int|string, initialValue?: int|string, quoted?: mixed, implicit?: bool} $definition
*
* @return void
*
Expand Down
6 changes: 3 additions & 3 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -536,12 +536,12 @@ parameters:
path: lib/Doctrine/ORM/Tools/Console/Command/MappingDescribeCommand.php

-
message: "#^Offset 'allocationSize' on array\\{sequenceName\\: string, allocationSize\\: string, initialValue\\: string, quoted\\?\\: mixed\\} in isset\\(\\) always exists and is not nullable\\.$#"
message: "#^Offset 'allocationSize' on array\\{sequenceName\\: string, allocationSize\\: string, initialValue\\: string, quoted\\?\\: mixed, implicit\\?\\: bool\\} in isset\\(\\) always exists and is not nullable\\.$#"
count: 1
path: lib/Doctrine/ORM/Tools/EntityGenerator.php

-
message: "#^Offset 'initialValue' on array\\{sequenceName\\: string, allocationSize\\: string, initialValue\\: string, quoted\\?\\: mixed\\} in isset\\(\\) always exists and is not nullable\\.$#"
message: "#^Offset 'initialValue' on array\\{sequenceName\\: string, allocationSize\\: string, initialValue\\: string, quoted\\?\\: mixed, implicit\\?\\: bool\\} in isset\\(\\) always exists and is not nullable\\.$#"
count: 1
path: lib/Doctrine/ORM/Tools/EntityGenerator.php

Expand All @@ -551,7 +551,7 @@ parameters:
path: lib/Doctrine/ORM/Tools/EntityGenerator.php

-
message: "#^Offset 'sequenceName' on array\\{sequenceName\\: string, allocationSize\\: string, initialValue\\: string, quoted\\?\\: mixed\\} in isset\\(\\) always exists and is not nullable\\.$#"
message: "#^Offset 'sequenceName' on array\\{sequenceName\\: string, allocationSize\\: string, initialValue\\: string, quoted\\?\\: mixed, implicit\\?\\: bool\\} in isset\\(\\) always exists and is not nullable\\.$#"
count: 1
path: lib/Doctrine/ORM/Tools/EntityGenerator.php

Expand Down
122 changes: 122 additions & 0 deletions tests/Doctrine/Tests/ORM/Functional/Ticket/GH10927Test.php
Original file line number Diff line number Diff line change
@@ -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
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ public function testMappedSuperclassWithId(): void
/**
* @group DDC-1156
* @group DDC-1218
* @group GH-10927
*/
public function testGeneratedValueFromMappedSuperclass(): void
{
Expand All @@ -179,6 +180,7 @@ public function testGeneratedValueFromMappedSuperclass(): void
/**
* @group DDC-1156
* @group DDC-1218
* @group GH-10927
*/
public function testSequenceDefinitionInHierarchyWithSandwichMappedSuperclass(): void
{
Expand All @@ -195,6 +197,7 @@ public function testSequenceDefinitionInHierarchyWithSandwichMappedSuperclass():
/**
* @group DDC-1156
* @group DDC-1218
* @group GH-10927
*/
public function testMultipleMappedSuperclasses(): void
{
Expand Down