forked from doctrine/orm
-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Fix
@SequenceGeneratorDefinition inheritance, take 2
This is an alternative implementation to doctrine#11050. The difference is that with this PR here, once `@SequenceGeneratorDefinition` is used in an inheritance tree of entities and/or mapped superclasses, this definition (and in particular, the sequence name) will be inherited by all child entity/mapped superclasses. Before doctrine#10455, the rules how `@SequenceGeneratorDefinition` is inherited from parent entity/mapped superclasses were as follows: * Entity and mapped superclasses inherit the ID generator type (as given by `@GeneratedValue`) from their parent classes * `@SequenceGeneratorDefinition`, however, is not generally inherited * ... instead, a default sequence generator definition is created for every class when no explicit configuration is given. In this case, sequence names are based on the current class' table name. * Once a root entity has been identified, all subclasses inherit its sequence generator definition unchanged. But, this has to be considered together with the quirky mapping driver behaviour that was deprecated in doctrine#10455: The mapping driver would not report public and protected fields from mapped superclasses, so these were virtually "pushed down" to the next entity classes. That means `@SequenceGeneratorDefinition` on mapped superclasses would, in fact, be effective as-is for inheriting entity classes. This is what was covered by the tests in `BasicInheritanceMappingTest` that I marked with `@group doctrineGH-10927`. My guess is that this PR will make it possible to opt-in to `reportFieldsWhereDeclared` (see doctrine#10455) and still get the same behaviour for mapped superclasses using `@SequenceGeneratorDefinition` as before. But maybe I don't see the full picture and all edge cases, so 👀 requested. The `GH10927Test` test case validates the sequence names generated in a few cases. In fact, I wrote this test against the `2.15.x` branch to make sure we get results that are consistent with the previous behaviour.
- Loading branch information
Showing
5 changed files
with
142 additions
and
11 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
122 changes: 122 additions & 0 deletions
122
tests/Doctrine/Tests/ORM/Functional/Ticket/GH10927Test.php
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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 | ||
| { | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters