Skip to content
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

Add runtime validation that the discriminator map contains all entity classes, also abstract ones? #10389

Closed
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
9 changes: 5 additions & 4 deletions lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -268,17 +268,18 @@ protected function validateRuntimeMetadata($class, $parent)
throw MappingException::missingDiscriminatorColumn($class->name);
}

if (! in_array($class->name, $class->discriminatorMap, true)) {
throw MappingException::mappedClassNotPartOfDiscriminatorMap($class->name, $class->rootEntityName);
}
Comment on lines +271 to +273
Copy link
Contributor Author

@mpdude mpdude Jan 13, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This extra check is needed here to catch inheritance roots missing in the DM.


foreach ($class->subClasses as $subClass) {
if ((new ReflectionClass($subClass))->name !== $subClass) {
throw MappingException::invalidClassInDiscriminatorMap($subClass, $class->name);
}
}
} else {
assert($parent instanceof ClassMetadataInfo); // https://github.com/doctrine/orm/issues/8746
if (
! $class->reflClass->isAbstract()
&& ! in_array($class->name, $class->discriminatorMap, true)
) {
if (! in_array($class->name, $class->discriminatorMap, true)) {
throw MappingException::mappedClassNotPartOfDiscriminatorMap($class->name, $class->rootEntityName);
}
}
Expand Down
4 changes: 2 additions & 2 deletions lib/Doctrine/ORM/Mapping/MappingException.php
Original file line number Diff line number Diff line change
Expand Up @@ -762,8 +762,8 @@ public static function mappedClassNotPartOfDiscriminatorMap($className, $rootCla
{
return new self(
"Entity '" . $className . "' has to be part of the discriminator map of '" . $rootClassName . "' " .
"to be properly mapped in the inheritance hierarchy. Alternatively you can make '" . $className . "' an abstract class " .
'to avoid this exception from occurring.'
'to be properly mapped in the inheritance hierarchy. The discriminator map needs to contain all ' .
'entity classes from the hierarchy, including abstract ones.'
);
}

Expand Down
84 changes: 80 additions & 4 deletions tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,19 +131,48 @@ public function testUnmappedSuperclassInHierarchy(): void
}

/** @group DDC-1204 */
public function testUnmappedEntityInHierarchy(): void
public function testNonAbstractEntityNotInDiscriminatorMap(): void
{
// HierarchyE is in the DM, but the (non-abstract) parent class HierarchyBEntity is not
$this->expectException(MappingException::class);
$this->expectExceptionMessage(
'Entity \'Doctrine\Tests\ORM\Mapping\HierarchyBEntity\' has to be part of the discriminator map'
. ' of \'Doctrine\Tests\ORM\Mapping\HierarchyBase\' to be properly mapped in the inheritance hierarchy.'
. ' Alternatively you can make \'Doctrine\Tests\ORM\Mapping\HierarchyBEntity\' an abstract class to'
. ' avoid this exception from occurring.'
. ' The discriminator map needs to contain all entity classes from the hierarchy, including abstract ones.'
);

$this->cmf->getMetadataFor(HierarchyE::class);
}

public function testAbstractEntityNotInDiscriminatorMap(): void
{
// Like testNonAbstractEntityNotInDiscriminatorMap(), but this time the parent class
// HierarchyF is abstract (and not in the DM as well)
$this->expectException(MappingException::class);
$this->expectExceptionMessage(
'Entity \'Doctrine\Tests\ORM\Mapping\HierarchyF\' has to be part of the discriminator map'
. ' of \'Doctrine\Tests\ORM\Mapping\HierarchyBase\' to be properly mapped in the inheritance hierarchy.'
. ' The discriminator map needs to contain all entity classes from the hierarchy, including abstract ones.'
);

$this->cmf->getMetadataFor(HierarchyG::class);
}

public function testAbstractRootEntityNotInDiscriminatorMap(): void
{
// Inheritance root entities go through a different path during runtime validation, so
// we need to check root classes separately. Even when the root class is abstract, it may
// not be omitted from the DM.
$this->expectException(MappingException::class);
$this->expectExceptionMessage(
'Entity \'Doctrine\Tests\ORM\Mapping\AbstractRootClass\' has to be part of the discriminator map'
. ' of \'Doctrine\Tests\ORM\Mapping\AbstractRootClass\' to be properly mapped in the inheritance hierarchy.'
. ' The discriminator map needs to contain all entity classes from the hierarchy, including abstract ones.'
);

$this->cmf->getMetadataFor(AbstractRootClass::class);
}

/**
* @group DDC-1204
* @group DDC-1203
Expand Down Expand Up @@ -339,9 +368,11 @@ class EntityIndexSubClass extends MappedSuperclassBaseIndex
* @InheritanceType("SINGLE_TABLE")
* @DiscriminatorColumn(name="type", type="string", length=20)
* @DiscriminatorMap({
* "base" = "HierarchyBase",
* "c" = "HierarchyC",
* "d" = "HierarchyD",
* "e" = "HierarchyE"
* "e" = "HierarchyE",
* "g" = "HierarchyG"
* })
*/
abstract class HierarchyBase
Expand Down Expand Up @@ -406,6 +437,51 @@ class HierarchyE extends HierarchyBEntity
public $e;
}

/** @Entity */
abstract class HierarchyF extends HierarchyBase
{
/**
* @var string
* @Column(type="string", length=255)
*/
public $f;
}

/** @Entity */
class HierarchyG extends HierarchyF
{
/**
* @var string
* @Column(type="string", length=255)
*/
public $e;
}

/**
* @Entity
* @InheritanceType("SINGLE_TABLE")
* @DiscriminatorColumn(name="type", type="string", length=20)
* @DiscriminatorMap({
* "child" = "AbstractRootClassChild"
* })
*/
abstract class AbstractRootClass
{
/**
* @Column(type="integer")
* @Id
* @GeneratedValue
*
* @var int
*/
public $id;
}

/** @Entity */
class AbstractRootClassChild extends AbstractRootClass
{
}

/** @Entity */
class SuperclassEntity extends SuperclassBase
{
Expand Down