DDC-1203, DDC-1204 - Fix problems with mapped superclasses in midth o…

…f inheritance hierachy and entities not mapped in discriminator map.
ajohnstone · Jun 15, 2011 · 5ff44b5 · 5ff44b5
1 parent dd329c9
commit 5ff44b5
Show file tree

Hide file tree

Showing 3 changed files with 111 additions and 15 deletions.
diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php
@@ -261,19 +261,15 @@ protected function loadMetadata($name)
             $class = $this->newClassMetadataInstance($className);
 
             if ($parent) {
-                if (!$parent->isMappedSuperclass) {
-                    $class->setInheritanceType($parent->inheritanceType);
-                    $class->setDiscriminatorColumn($parent->discriminatorColumn);
-                }
+                $class->setInheritanceType($parent->inheritanceType);
+                $class->setDiscriminatorColumn($parent->discriminatorColumn);
                 $class->setIdGeneratorType($parent->generatorType);
                 $this->addInheritedFields($class, $parent);
                 $this->addInheritedRelations($class, $parent);
                 $class->setIdentifier($parent->identifier);
                 $class->setVersioned($parent->isVersioned);
                 $class->setVersionField($parent->versionField);
-                if (!$parent->isMappedSuperclass) {
-                    $class->setDiscriminatorMap($parent->discriminatorMap);
-                }
+                $class->setDiscriminatorMap($parent->discriminatorMap);
                 $class->setLifecycleCallbacks($parent->lifecycleCallbacks);
                 $class->setChangeTrackingPolicy($parent->changeTrackingPolicy);
             }
@@ -285,7 +281,7 @@ protected function loadMetadata($name)
                 throw MappingException::reflectionFailure($className, $e);
             }
 
-            if ($parent && ! $parent->isMappedSuperclass) {
+            if ($parent) {
                 if ($parent->isIdGeneratorSequence()) {
                     $class->setSequenceGeneratorDefinition($parent->sequenceGeneratorDefinition);
                 } else if ($parent->isIdGeneratorTable()) {
@@ -318,18 +314,23 @@ protected function loadMetadata($name)
             }
 
             // verify inheritance
-            if (!$parent && !$class->isMappedSuperclass && !$class->isInheritanceTypeNone()) {
-                if (count($class->discriminatorMap) == 0) {
-                    throw MappingException::missingDiscriminatorMap($class->name);
-                }
-                if (!$class->discriminatorColumn) {
-                    throw MappingException::missingDiscriminatorColumn($class->name);
+            if (!$class->isMappedSuperclass && !$class->isInheritanceTypeNone()) {
+                if (!$parent) {
+                    if (count($class->discriminatorMap) == 0) {
+                        throw MappingException::missingDiscriminatorMap($class->name);
+                    }
+                    if (!$class->discriminatorColumn) {
+                        throw MappingException::missingDiscriminatorColumn($class->name);
+                    }
+                } else if ($parent && !in_array($class->name, array_values($class->discriminatorMap))) {
+                    // enforce discriminator map for all entities of an inheritance hierachy, otherwise problems will occur.
+                    throw MappingException::mappedClassNotPartOfDiscriminatorMap($class->name, $class->rootEntityName);
                 }
             } else if ($class->isMappedSuperclass && $class->name == $class->rootEntityName && (count($class->discriminatorMap) || $class->discriminatorColumn)) {
                 // second condition is necessary for mapped superclasses in the middle of an inheritance hierachy
                 throw MappingException::noInheritanceOnMappedSuperClass($class->name);
             }
-
+            
             $this->loadedMetadata[$className] = $class;
 
             $parent = $class;

diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php
@@ -284,4 +284,12 @@ public static function noInheritanceOnMappedSuperClass($className)
     {
         return new self("Its not supported to define inheritance information on a mapped superclass '" . $className . "'.");
     }
+
+    public static function mappedClassNotPartOfDiscriminatorMap($className, $rootClassName)
+    {
+        return new self(
+            "Entity '" . $className . "' has to be part of the descriminator map of '" . $rootClassName . "' " .
+            "to be properly mapped in the inheritance hierachy. If you want to avoid instantiation of this type mark it abstract."
+        );
+    }
 }
diff --git a/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php b/tests/Doctrine/Tests/ORM/Mapping/BasicInheritanceMappingTest.php
@@ -3,6 +3,7 @@
 namespace Doctrine\Tests\ORM\Mapping;
 
 use Doctrine\ORM\Mapping\ClassMetadataFactory;
+use Doctrine\ORM\Tools\SchemaTool;
 
 require_once __DIR__ . '/../../TestInit.php';
 
@@ -66,6 +67,28 @@ public function testSerializationWithPrivateFieldsFromMappedSuperclass()
         $this->assertTrue(isset($class2->reflFields['mapped2']));
         $this->assertTrue(isset($class2->reflFields['mappedRelated1']));
     }
+
+    /**
+     * @group DDC-1203
+     */
+    public function testUnmappedSuperclassInHierachy()
+    {
+        $class = $this->_factory->getMetadataFor(__NAMESPACE__ . '\\HierachyD');
+
+        $this->assertTrue(isset($class->fieldMappings['id']));
+        $this->assertTrue(isset($class->fieldMappings['a']));
+        $this->assertTrue(isset($class->fieldMappings['d']));
+    }
+
+    /**
+     * @group DDC-1204
+     */
+    public function testUnmappedEntityInHierachy()
+    {
+        $this->setExpectedException('Doctrine\ORM\Mapping\MappingException', "Entity 'Doctrine\Tests\ORM\Mapping\HierachyBEntity' has to be part of the descriminator map of 'Doctrine\Tests\ORM\Mapping\HierachyBase' to be properly mapped in the inheritance hierachy. If you want to avoid instantiation of this type mark it abstract.");
+
+        $class = $this->_factory->getMetadataFor(__NAMESPACE__ . '\\HierachyE');
+    }
 }
 
 class TransientBaseClass {
@@ -103,3 +126,67 @@ class EntitySubClass2 extends MappedSuperclassBase {
     /** @Column(type="string") */
     private $name;
 }
+
+/**
+ * @Entity
+ * @InheritanceType("SINGLE_TABLE")
+ * @DiscriminatorColumn(name="type", type="string", length=20)
+ * @DiscriminatorMap({
+ *     "c"   = "HierachyC",
+ *     "d"   = "HierachyD",
+ *     "e"   = "HierachyE"
+ * })
+ */
+abstract class HierachyBase
+{
+    /**
+     * @Column(type="integer") @Id @GeneratedValue
+     * @var int
+     */
+    public $id;
+}
+
+/**
+ * @MappedSuperclass
+ */
+abstract class HierachyASuperclass extends HierachyBase
+{
+    /** @Column(type="string") */
+    public $a;
+}
+
+/**
+ * @Entity
+ */
+abstract class HierachyBEntity extends HierachyBase
+{
+    /** @Column(type="string") */
+    public $b;
+}
+
+/**
+ * @Entity
+ */
+class HierachyC extends HierachyBase
+{
+    /** @Column(type="string") */
+    public $c;
+}
+
+/**
+ * @Entity
+ */
+class HierachyD extends HierachyASuperclass
+{
+    /** @Column(type="string") */
+    public $d;
+}
+
+/**
+ * @Entity
+ */
+class HierachyE extends HierachyBEntity
+{
+    /** @Column(type="string") */
+    public $e;
+}