Skip to content

Commit 4e8e3ef

Browse files
mpdudempdude
committed
Discover entity subclasses that need not be declared in the discriminator map
1 parent d68baef commit 4e8e3ef

File tree

4 files changed

+385
-1
lines changed

4 files changed

+385
-1
lines changed

‎lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php

+58
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
115115
$class->setVersioned($parent->isVersioned);
116116
$class->setVersionField($parent->versionField);
117117
$class->setDiscriminatorMap($parent->discriminatorMap);
118+
$class->addSubClasses($parent->subClasses);
118119
$class->setLifecycleCallbacks($parent->lifecycleCallbacks);
119120
$class->setChangeTrackingPolicy($parent->changeTrackingPolicy);
120121

@@ -219,11 +220,16 @@ protected function doLoadMetadata($class, $parent, $rootEntityFound, array $nonS
219220
$this->addDefaultDiscriminatorMap($class);
220221
}
221222

223+
// During the following event, there may also be updates to the discriminator map as per GH-1257/GH-8402.
224+
// So, we must not discover the missing subclasses before that.
225+
222226
if ($this->evm->hasListeners(Events::loadClassMetadata)) {
223227
$eventArgs = new LoadClassMetadataEventArgs($class, $this->em);
224228
$this->evm->dispatchEvent(Events::loadClassMetadata, $eventArgs);
225229
}
226230

231+
$this->findAbstractEntityClassesNotListedInDiscriminatorMap($class);
232+
227233
if ($class->changeTrackingPolicy === ClassMetadata::CHANGETRACKING_NOTIFY) {
228234
Deprecation::trigger(
229235
'doctrine/orm',
@@ -338,6 +344,58 @@ private function addDefaultDiscriminatorMap(ClassMetadata $class): void
338344
$class->setDiscriminatorMap($map);
339345
}
340346

347+
private function findAbstractEntityClassesNotListedInDiscriminatorMap(ClassMetadata $rootEntityClass): void
348+
{
349+
// Only root classes in inheritance hierarchies need contain a discriminator map,
350+
// so skip for other classes.
351+
if (! $rootEntityClass->isRootEntity() || $rootEntityClass->isInheritanceTypeNone()) {
352+
return;
353+
}
354+
355+
$processedClasses = [$rootEntityClass->name => true];
356+
foreach ($rootEntityClass->subClasses as $knownSubClass) {
357+
$processedClasses[$knownSubClass] = true;
358+
}
359+
360+
foreach ($rootEntityClass->discriminatorMap as $declaredClassName) {
361+
// This fetches non-transient parent classes only
362+
$parentClasses = $this->getParentClasses($declaredClassName);
363+
364+
foreach ($parentClasses as $parentClass) {
365+
if (isset($processedClasses[$parentClass])) {
366+
continue;
367+
}
368+
369+
$processedClasses[$parentClass] = true;
370+
371+
// All non-abstract entity classes must be listed in the discriminator map, and
372+
// this will be validated/enforced at runtime (possibly at a later time, when the
373+
// subclass is loaded, but anyways). Also, subclasses is about entity classes only.
374+
// That means we can ignore non-abstract classes here. The (expensive) driver
375+
// check for mapped superclasses need only be run for abstract candidate classes.
376+
if (! (new ReflectionClass($parentClass))->isAbstract() || $this->peekIfIsMappedSuperclass($parentClass)) {
377+
continue;
378+
}
379+
380+
// We have found a non-transient, non-mapped-superclass = an entity class (possibly abstract, but that does not matter)
381+
$rootEntityClass->addSubClass($parentClass);
382+
}
383+
}
384+
}
385+
386+
/** @param class-string $className */
387+
private function peekIfIsMappedSuperclass(string $className): bool
388+
{
389+
// TODO can we shortcut this for classes that have already been loaded? can that be the case at all?
390+
$reflService = $this->getReflectionService();
391+
$class = $this->newClassMetadataInstance($className);
392+
$this->initializeReflection($class, $reflService);
393+
394+
$this->driver->loadMetadataForClass($className, $class);
395+
396+
return $class->isMappedSuperclass;
397+
}
398+
341399
/**
342400
* Gets the lower-case short name of a class.
343401
*

‎lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php

+36-1
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,27 @@ class ClassMetadataInfo implements ClassMetadata
397397
public $parentClasses = [];
398398

399399
/**
400-
* READ-ONLY: The names of all subclasses (descendants).
400+
* READ-ONLY: For classes in inheritance mapping hierarchies, this field contains the names of all
401+
* <em>entity</em> subclasses of this class. These may also be abstract classes.
402+
*
403+
* This list is used, for example, to enumerate all necessary tables in JTI when querying for root
404+
* or subclass entities, or to gather all fields comprised in an entity inheritance tree.
405+
*
406+
* For classes that do not use STI/JTI, this list is empty.
407+
*
408+
* Implementation note:
409+
*
410+
* In PHP, there is no general way to discover all subclasses of a given class at runtime. For that
411+
* reason, the list of classes given in the discriminator map at the root entity is considered
412+
* authoritative. The discriminator map must contain all <em>concrete</em> classes that can
413+
* appear in the particular inheritance hierarchy tree. Since there can be no instances of abstract
414+
* entity classes, users are not required to list such classes with a discriminator value.
415+
*
416+
* The possibly remaining "gaps" for abstract entity classes are filled after the class metadata for the
417+
* root entity has been loaded.
418+
*
419+
* For subclasses of such root entities, the list can be reused/passed downwards, it only needs to
420+
* be filtered accordingly (only keep remaining subclasses)
401421
*
402422
* @psalm-var list<class-string>
403423
*/
@@ -3275,6 +3295,21 @@ public function addDiscriminatorMapClass($name, $className)
32753295
throw MappingException::invalidClassInDiscriminatorMap($className, $this->name);
32763296
}
32773297

3298+
$this->addSubClass($className);
3299+
}
3300+
3301+
/** @param array<class-string> $classes */
3302+
public function addSubClasses(array $classes): void
3303+
{
3304+
foreach ($classes as $className) {
3305+
$this->addSubClass($className);
3306+
}
3307+
}
3308+
3309+
public function addSubClass(string $className): void
3310+
{
3311+
// By ignoring classes that are not subclasses of the current class, we simplify inheriting
3312+
// the subclass list from a parent class at the beginning of \Doctrine\ORM\Mapping\ClassMetadataFactory::doLoadMetadata.
32783313
if (is_subclass_of($className, $this->name) && ! in_array($className, $this->subClasses, true)) {
32793314
$this->subClasses[] = $className;
32803315
}

‎tests/Doctrine/Tests/ORM/Functional/Ticket/GH10387Test.php

+173
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Doctrine\Tests\ORM\Functional\Ticket;
6+
7+
use Doctrine\ORM\Mapping as ORM;
8+
use Doctrine\ORM\Tools\SchemaTool;
9+
use Doctrine\Tests\OrmTestCase;
10+
use Generator;
11+
12+
use function array_map;
13+
14+
/**
15+
* @group GH-10387
16+
*/
17+
class GH10387Test extends OrmTestCase
18+
{
19+
/**
20+
* @dataProvider classHierachies
21+
*/
22+
public function testSchemaToolCreatesColumnForFieldInTheMiddleClass(array $classes): void
23+
{
24+
$em = $this->getTestEntityManager();
25+
$schemaTool = new SchemaTool($em);
26+
$metadata = array_map(static function (string $class) use ($em) {
27+
return $em->getClassMetadata($class);
28+
}, $classes);
29+
$schema = $schemaTool->getSchemaFromMetadata([$metadata[0]]);
30+
31+
self::assertNotNull($schema->getTable('root')->getColumn('middle_class_field'));
32+
self::assertNotNull($schema->getTable('root')->getColumn('leaf_class_field'));
33+
}
34+
35+
public function classHierachies(): Generator
36+
{
37+
yield 'hierarchy with Entity classes only' => [[GH10387EntitiesOnlyRoot::class, GH10387EntitiesOnlyMiddle::class, GH10387EntitiesOnlyLeaf::class]];
38+
yield 'MappedSuperclass in the middle of the hierarchy' => [[GH10387MappedSuperclassRoot::class, GH10387MappedSuperclassMiddle::class, GH10387MappedSuperclassLeaf::class]];
39+
yield 'abstract entity the the root and in the middle of the hierarchy' => [[GH10387AbstractEntitiesRoot::class, GH10387AbstractEntitiesMiddle::class, GH10387AbstractEntitiesLeaf::class]];
40+
}
41+
}
42+
43+
/**
44+
* @ORM\Entity
45+
* @ORM\Table(name="root")
46+
* @ORM\InheritanceType("SINGLE_TABLE")
47+
* @ORM\DiscriminatorMap({ "A": "GH10387EntitiesOnlyRoot", "B": "GH10387EntitiesOnlyMiddle", "C": "GH10387EntitiesOnlyLeaf"})
48+
*/
49+
class GH10387EntitiesOnlyRoot
50+
{
51+
/**
52+
* @ORM\Id
53+
* @ORM\Column
54+
*
55+
* @var string
56+
*/
57+
private $id;
58+
}
59+
60+
/**
61+
* @ORM\Entity
62+
*/
63+
class GH10387EntitiesOnlyMiddle extends GH10387EntitiesOnlyRoot
64+
{
65+
/**
66+
* @ORM\Column(name="middle_class_field")
67+
*
68+
* @var string
69+
*/
70+
private $parentValue;
71+
}
72+
73+
/**
74+
* @ORM\Entity
75+
*/
76+
class GH10387EntitiesOnlyLeaf extends GH10387EntitiesOnlyMiddle
77+
{
78+
/**
79+
* @ORM\Column(name="leaf_class_field")
80+
*
81+
* @var string
82+
*/
83+
private $childValue;
84+
}
85+
86+
/**
87+
* @ORM\Entity
88+
* @ORM\Table(name="root")
89+
* @ORM\InheritanceType("SINGLE_TABLE")
90+
* @ORM\DiscriminatorMap({ "A": "GH10387MappedSuperclassRoot", "B": "GH10387MappedSuperclassLeaf"})
91+
* ^- This DiscriminatorMap contains the Entity classes only, not the Mapped Superclass
92+
*/
93+
class GH10387MappedSuperclassRoot
94+
{
95+
/**
96+
* @ORM\Id
97+
* @ORM\Column
98+
*
99+
* @var string
100+
*/
101+
private $id;
102+
}
103+
104+
/**
105+
* @ORM\MappedSuperclass
106+
*/
107+
class GH10387MappedSuperclassMiddle extends GH10387MappedSuperclassRoot
108+
{
109+
/**
110+
* @ORM\Column(name="middle_class_field")
111+
*
112+
* @var string
113+
*/
114+
private $parentValue;
115+
}
116+
117+
/**
118+
* @ORM\Entity
119+
*/
120+
class GH10387MappedSuperclassLeaf extends GH10387MappedSuperclassMiddle
121+
{
122+
/**
123+
* @ORM\Column(name="leaf_class_field")
124+
*
125+
* @var string
126+
*/
127+
private $childValue;
128+
}
129+
130+
131+
/**
132+
* @ORM\Entity
133+
* @ORM\Table(name="root")
134+
* @ORM\InheritanceType("SINGLE_TABLE")
135+
* @ORM\DiscriminatorMap({ "A": "GH10387AbstractEntitiesLeaf"})
136+
* ^- This DiscriminatorMap contains the single non-abstract Entity class only
137+
*/
138+
abstract class GH10387AbstractEntitiesRoot
139+
{
140+
/**
141+
* @ORM\Id
142+
* @ORM\Column
143+
*
144+
* @var string
145+
*/
146+
private $id;
147+
}
148+
149+
/**
150+
* @ORM\Entity
151+
*/
152+
abstract class GH10387AbstractEntitiesMiddle extends GH10387AbstractEntitiesRoot
153+
{
154+
/**
155+
* @ORM\Column(name="middle_class_field")
156+
*
157+
* @var string
158+
*/
159+
private $parentValue;
160+
}
161+
162+
/**
163+
* @ORM\Entity
164+
*/
165+
class GH10387AbstractEntitiesLeaf extends GH10387AbstractEntitiesMiddle
166+
{
167+
/**
168+
* @ORM\Column(name="leaf_class_field")
169+
*
170+
* @var string
171+
*/
172+
private $childValue;
173+
}

0 commit comments

Comments
 (0)