From 4c59207abdf31de5c7b050b16177e3e9d056112e Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 19 Jan 2023 09:00:30 +0000 Subject: [PATCH] Catch missing inheritance declarations early on When my understanding is correct, inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one. Catching missing inheritance declarations early on is important to avoid weird errors further down the road, giving users a clear indication of the root cause. The documentation is updated accordingly at #10429. --- .../ORM/Mapping/ClassMetadataFactory.php | 4 + lib/Doctrine/ORM/Mapping/MappingException.php | 13 ++ .../ORM/Functional/Ticket/GH8127Test.php | 28 +++-- .../Mapping/BasicInheritanceMappingTest.php | 117 ++++++++++++++++++ 4 files changed, 150 insertions(+), 12 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index bda0cf236aa..7abc68a558b 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -145,6 +145,10 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS } if (! $class->isMappedSuperclass) { + if ($rootEntityFound && $class->isInheritanceTypeNone()) { + throw MappingException::missingInheritanceTypeDeclaration(end($nonSuperclassParents), $class->name); + } + foreach ($class->embeddedClasses as $property => $embeddableClass) { if (isset($embeddableClass['inherited'])) { continue; diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index f606e4afcdf..3e4e3b7045f 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/lib/Doctrine/ORM/Mapping/MappingException.php @@ -539,6 +539,19 @@ static function ($a, $b) { ); } + /** + * @param class-string $rootEntityClass + * @param class-string $childEntityClass + */ + public static function missingInheritanceTypeDeclaration(string $rootEntityClass, string $childEntityClass): self + { + return new self(sprintf( + "Entity class '%s' is a subclass of the root entity class '%s', but no inheritance mapping type was declared.", + $childEntityClass, + $rootEntityClass + )); + } + /** * @param string $className * diff --git a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8127Test.php b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8127Test.php index 439edfa1887..651e6e49503 100644 --- a/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8127Test.php +++ b/tests/Doctrine/Tests/ORM/Functional/Ticket/GH8127Test.php @@ -9,22 +9,26 @@ class GH8127Test extends OrmFunctionalTestCase { - protected function setUp(): void - { - parent::setUp(); - - $this->createSchemaForModels( - GH8127Root::class, - GH8127Middle::class, - GH8127Leaf::class - ); - } +// protected function setUp(): void +// { +// parent::setUp(); +// +// $this->createSchemaForModels( +// GH8127Root::class, +// GH8127Middle::class, +// GH8127Leaf::class +// ); +// } /** * @dataProvider queryClasses */ public function testLoadFieldsFromAllClassesInHierarchy(string $queryClass): void { + $cm = $this->_em->getClassMetadata($queryClass); + + return; + $entity = new GH8127Leaf(); $entity->root = 'root'; $entity->middle = 'middle'; @@ -53,8 +57,8 @@ public function queryClasses(): array /** * @ORM\Entity * @ORM\Table(name="root") - * @ORM\InheritanceType("JOINED") - * @ORM\DiscriminatorMap({ "leaf": "GH8127Leaf" }) + * @ ORM\InheritanceType("JOINED") + * @ ORM\DiscriminatorMap({ "leaf": "GH8127Leaf" }) */ abstract class GH8127Root { diff --git a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php index 86cbb041ea1..e8eb6bde082 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php @@ -29,9 +29,11 @@ use Doctrine\Tests\Models\DDC869\DDC869Payment; use Doctrine\Tests\Models\DDC869\DDC869PaymentRepository; use Doctrine\Tests\OrmTestCase; +use Generator; use function assert; use function serialize; +use function sprintf; use function unserialize; class BasicInheritanceMappingTest extends OrmTestCase @@ -218,6 +220,39 @@ public function testMappedSuperclassIndex(): void self::assertArrayHasKey('IDX_MAPPED1_INDEX', $class->table['uniqueConstraints']); self::assertArrayHasKey('IDX_MAPPED2_INDEX', $class->table['indexes']); } + + /** + * @dataProvider invalidHierarchyDeclarationClasses + */ + public function testUndeclaredHierarchyRejection(string $rootEntity, string $childClass): void + { + self::expectException(MappingException::class); + self::expectExceptionMessage(sprintf( + "Entity class '%s' is a subclass of the root entity class '%s', but no inheritance mapping type was declared.", + $childClass, + $rootEntity + )); + + $this->cmf->getMetadataFor($childClass); + } + + public function invalidHierarchyDeclarationClasses(): Generator + { + yield 'concrete Entity root and child class, direct inheritance' + => [InvalidEntityRoot::class, InvalidEntityRootChild::class]; + + yield 'concrete Entity root and abstract child class, direct inheritance' + => [InvalidEntityRoot::class, InvalidEntityRootAbstractChild::class]; + + yield 'abstract Entity root and concrete child class, direct inheritance' + => [InvalidAbstractEntityRoot::class, InvalidAbstractEntityRootChild::class]; + + yield 'abstract Entity root and abstract child class, direct inheritance' + => [InvalidAbstractEntityRoot::class, InvalidAbstractEntityRootAbstractChild::class]; + + yield 'complex example (Entity Root -> Mapped Superclass -> transient class -> Entity)' + => [InvalidComplexRoot::class, InvalidComplexEntity::class]; + } } class TransientBaseClass @@ -438,3 +473,85 @@ class MediumSuperclassEntity extends MediumSuperclassBase class SubclassWithRepository extends DDC869Payment { } + +/** + * @Entity + * + * This class misses the DiscriminatorMap declaration + */ +class InvalidEntityRoot +{ + /** + * @Column(type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + * @var int + */ + public $id; +} + +/** @Entity */ +class InvalidEntityRootChild extends InvalidEntityRoot +{ +} + +/** @Entity */ +abstract class InvalidEntityRootAbstractChild extends InvalidEntityRoot +{ +} + +/** + * @Entity + * + * This class misses the DiscriminatorMap declaration + */ +class InvalidAbstractEntityRoot +{ + /** + * @Column(type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + * @var int + */ + public $id; +} + +/** @Entity */ +class InvalidAbstractEntityRootChild extends InvalidAbstractEntityRoot +{ +} + +/** @Entity */ +abstract class InvalidAbstractEntityRootAbstractChild extends InvalidAbstractEntityRoot +{ +} + +/** + * @Entity + * + * This class misses the DiscriminatorMap declaration + */ +class InvalidComplexRoot +{ + /** + * @Column(type="integer") + * @Id + * @GeneratedValue(strategy="AUTO") + * @var int + */ + public $id; +} + +/** @MappedSuperclass */ +class InvalidComplexMappedSuperclass extends InvalidComplexRoot +{ +} + +class InvalidComplexTransientClass extends InvalidComplexMappedSuperclass +{ +} + +/** @Entity */ +class InvalidComplexEntity extends InvalidComplexTransientClass +{ +}