From 384a5b4c8cd54feade72a2ccec643fdb428f7c6c Mon Sep 17 00:00:00 2001 From: Matthias Pigulla Date: Thu, 19 Jan 2023 09:00:30 +0000 Subject: [PATCH] Deprecate undeclared entity inheritance Inheritance has to be declared as soon as one entity class extends (directly or through middle classes) another one. This is also pointed out in the opening comment for #8348: > Entities are not allowed to extend from entities without an inheritence mapping relationship (Single Table or Joined Table inheritance). [...] While Doctrine so far allowed these things, they are fragile and will break on certain scenarios. Throwing an exception in case of this misconfiguration is nothing we should do light-heartedly, given that it may surprise users in a bugfix or feature release. So, we should start with a deprecation notice and make this an exception in 3.0. The documentation is updated accordingly at #10429. 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. In case you are affected by this, please understand that although things "previously worked" for you, you have been using the ORM outside of what it was designed to do. That may have worked in simple cases, but may also have caused invalid results (wrong or missing data after hydration?) that possibly went unnoticed in subtle cases. --- UPGRADE.md | 5 + .../ORM/Mapping/ClassMetadataFactory.php | 12 ++ lib/Doctrine/ORM/Mapping/MappingException.php | 13 ++ .../Tests/Models/DDC1590/DDC1590Entity.php | 2 - .../Tests/Models/DDC2372/DDC2372User.php | 6 + .../Models/DDC5934/DDC5934BaseContract.php | 6 +- .../Tests/Models/Forum/ForumAdministrator.php | 18 --- .../Mapping/BasicInheritanceMappingTest.php | 122 ++++++++++++++++++ 8 files changed, 161 insertions(+), 23 deletions(-) delete mode 100644 tests/Doctrine/Tests/Models/Forum/ForumAdministrator.php diff --git a/UPGRADE.md b/UPGRADE.md index 0346a4d2ad9..69101301659 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,10 @@ # Upgrade to 2.15 +## Deprecated undeclared entity inheritance + +As soon as an entity class inherits from another entity class, inheritance has to +be declared by adding the appropriate configuration for the root entity. + ## Deprecated stubs for "concrete table inheritance" This third way of mapping class inheritance was never implemented. Code stubs are diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index ab87f59813b..f1b4292d291 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -148,6 +148,18 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS } if (! $class->isMappedSuperclass) { + if ($rootEntityFound && $class->isInheritanceTypeNone()) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/10431', + "Entity class '%s' is a subclass of the root entity class '%s', but no inheritance mapping type was declared. This is a misconfiguration and will be an error in Doctrine ORM 3.0.", + $class->name, + end($nonSuperclassParents) + ); + // enable this in 3.0 + // 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 9fd3f032eda..4714047122d 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/Models/DDC1590/DDC1590Entity.php b/tests/Doctrine/Tests/Models/DDC1590/DDC1590Entity.php index 26d41781542..1078ffeb33c 100644 --- a/tests/Doctrine/Tests/Models/DDC1590/DDC1590Entity.php +++ b/tests/Doctrine/Tests/Models/DDC1590/DDC1590Entity.php @@ -6,13 +6,11 @@ use DateTime; use Doctrine\ORM\Mapping\Column; -use Doctrine\ORM\Mapping\Entity; use Doctrine\ORM\Mapping\GeneratedValue; use Doctrine\ORM\Mapping\Id; use Doctrine\ORM\Mapping\MappedSuperclass; /** - * @Entity * @MappedSuperclass */ abstract class DDC1590Entity diff --git a/tests/Doctrine/Tests/Models/DDC2372/DDC2372User.php b/tests/Doctrine/Tests/Models/DDC2372/DDC2372User.php index c2ec10e6a47..f11b248e2d3 100644 --- a/tests/Doctrine/Tests/Models/DDC2372/DDC2372User.php +++ b/tests/Doctrine/Tests/Models/DDC2372/DDC2372User.php @@ -5,15 +5,21 @@ namespace Doctrine\Tests\Models\DDC2372; use Doctrine\ORM\Mapping\Column; +use Doctrine\ORM\Mapping\DiscriminatorColumn; +use Doctrine\ORM\Mapping\DiscriminatorMap; use Doctrine\ORM\Mapping\Entity; use Doctrine\ORM\Mapping\GeneratedValue; use Doctrine\ORM\Mapping\Id; +use Doctrine\ORM\Mapping\InheritanceType; use Doctrine\ORM\Mapping\Table; use Doctrine\Tests\Models\DDC2372\Traits\DDC2372AddressAndAccessors; /** * @Entity * @Table(name="users") + * @InheritanceType("SINGLE_TABLE") + * @DiscriminatorColumn("type") + * @DiscriminatorMap({"user"="DDC2372User", "admin"="DDC2372Admin"}) */ class DDC2372User { diff --git a/tests/Doctrine/Tests/Models/DDC5934/DDC5934BaseContract.php b/tests/Doctrine/Tests/Models/DDC5934/DDC5934BaseContract.php index 5933d27f87e..13f7fda9e24 100644 --- a/tests/Doctrine/Tests/Models/DDC5934/DDC5934BaseContract.php +++ b/tests/Doctrine/Tests/Models/DDC5934/DDC5934BaseContract.php @@ -8,13 +8,13 @@ use Doctrine\Common\Collections\Collection; use Doctrine\ORM\Mapping\ClassMetadata; use Doctrine\ORM\Mapping\Column; -use Doctrine\ORM\Mapping\Entity; use Doctrine\ORM\Mapping\GeneratedValue; use Doctrine\ORM\Mapping\Id; use Doctrine\ORM\Mapping\ManyToMany; +use Doctrine\ORM\Mapping\MappedSuperclass; -/** @Entity */ -#[Entity] +/** @MappedSuperclass */ +#[MappedSuperclass] class DDC5934BaseContract { /** diff --git a/tests/Doctrine/Tests/Models/Forum/ForumAdministrator.php b/tests/Doctrine/Tests/Models/Forum/ForumAdministrator.php deleted file mode 100644 index 9ce94b9755f..00000000000 --- a/tests/Doctrine/Tests/Models/Forum/ForumAdministrator.php +++ /dev/null @@ -1,18 +0,0 @@ -table['uniqueConstraints']); self::assertArrayHasKey('IDX_MAPPED2_INDEX', $class->table['indexes']); } + + /** + * @dataProvider invalidHierarchyDeclarationClasses + */ + public function testUndeclaredHierarchyRejection(string $rootEntity, string $childClass): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/10431'); + + /* on 3.0, use this instead: */ + // 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 +478,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 +{ +}