DDC-704 - Added better validation of inheritence type constructs in Xml, Annotation and Yaml Drivers

lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php

@@ -169,23 +169,30 @@ class AnnotationDriver implements Driver
         if (isset($classAnnotations['Doctrine\ORM\Mapping\InheritanceType'])) {
             $inheritanceTypeAnnot = $classAnnotations['Doctrine\ORM\Mapping\InheritanceType'];
             $metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . $inheritanceTypeAnnot->value));
+
+            if ($metadata->inheritanceType != \Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_NONE) {
+                // Evaluate DiscriminatorColumn annotation
+                if (isset($classAnnotations['Doctrine\ORM\Mapping\DiscriminatorColumn'])) {
+                    $discrColumnAnnot = $classAnnotations['Doctrine\ORM\Mapping\DiscriminatorColumn'];
+                    $metadata->setDiscriminatorColumn(array(
+                        'name' => $discrColumnAnnot->name,
+                        'type' => $discrColumnAnnot->type,
+                        'length' => $discrColumnAnnot->length
+                    ));
+                } else {
+                    throw MappingException::missingDiscriminatorColumn($className);
+                }
+
+                // Evaluate DiscriminatorMap annotation
+                if (isset($classAnnotations['Doctrine\ORM\Mapping\DiscriminatorMap'])) {
+                    $discrMapAnnot = $classAnnotations['Doctrine\ORM\Mapping\DiscriminatorMap'];
+                    $metadata->setDiscriminatorMap($discrMapAnnot->value);
+                } else {
+                    throw MappingException::missingDiscriminatorMap($className);
+                }
+            }
         }
 
-        // Evaluate DiscriminatorColumn annotation
-        if (isset($classAnnotations['Doctrine\ORM\Mapping\DiscriminatorColumn'])) {
-            $discrColumnAnnot = $classAnnotations['Doctrine\ORM\Mapping\DiscriminatorColumn'];
-            $metadata->setDiscriminatorColumn(array(
-                'name' => $discrColumnAnnot->name,
-                'type' => $discrColumnAnnot->type,
-                'length' => $discrColumnAnnot->length
-            ));
-        }
-
-        // Evaluate DiscriminatorMap annotation
-        if (isset($classAnnotations['Doctrine\ORM\Mapping\DiscriminatorMap'])) {
-            $discrMapAnnot = $classAnnotations['Doctrine\ORM\Mapping\DiscriminatorMap'];
-            $metadata->setDiscriminatorMap($discrMapAnnot->value);
-        }
 
         // Evaluate DoctrineChangeTrackingPolicy annotation
         if (isset($classAnnotations['Doctrine\ORM\Mapping\ChangeTrackingPolicy'])) {

lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php

@@ -77,27 +77,34 @@ class XmlDriver extends AbstractFileDriver
         if (isset($xmlRoot['inheritance-type'])) {
             $inheritanceType = (string)$xmlRoot['inheritance-type'];
             $metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . $inheritanceType));
-        }
 
-        // Evaluate <discriminator-column...>
+            if ($metadata->inheritanceType != \Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_NONE) {
-        if (isset($xmlRoot->{'discriminator-column'})) {
+                // Evaluate <discriminator-column...>
-            $discrColumn = $xmlRoot->{'discriminator-column'};
+                if (isset($xmlRoot->{'discriminator-column'})) {
-            $metadata->setDiscriminatorColumn(array(
+                    $discrColumn = $xmlRoot->{'discriminator-column'};
-                'name' => (string)$discrColumn['name'],
+                    $metadata->setDiscriminatorColumn(array(
-                'type' => (string)$discrColumn['type'],
+                        'name' => (string)$discrColumn['name'],
-                'length' => (string)$discrColumn['length']
+                        'type' => (string)$discrColumn['type'],
-            ));
+                        'length' => (string)$discrColumn['length']
-        }
+                    ));
+                } else {
+                    throw MappingException::missingDiscriminatorColumn($className);
+                }
 
-        // Evaluate <discriminator-map...>
+                // Evaluate <discriminator-map...>
-        if (isset($xmlRoot->{'discriminator-map'})) {
+                if (isset($xmlRoot->{'discriminator-map'})) {
-            $map = array();
+                    $map = array();
-            foreach ($xmlRoot->{'discriminator-map'}->{'discriminator-mapping'} AS $discrMapElement) {
+                    foreach ($xmlRoot->{'discriminator-map'}->{'discriminator-mapping'} AS $discrMapElement) {
-                $map[(string)$discrMapElement['value']] = (string)$discrMapElement['class'];
+                        $map[(string)$discrMapElement['value']] = (string)$discrMapElement['class'];
+                    }
+                    $metadata->setDiscriminatorMap($map);
+                } else {
+                    throw MappingException::missingDiscriminatorMap($className);
+                }
             }
-            $metadata->setDiscriminatorMap($map);
         }
 
+
         // Evaluate <change-tracking-policy...>
         if (isset($xmlRoot['change-tracking-policy'])) {
             $metadata->setChangeTrackingPolicy(constant('Doctrine\ORM\Mapping\ClassMetadata::CHANGETRACKING_'

lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php

@@ -69,22 +69,29 @@ class YamlDriver extends AbstractFileDriver
 
         if (isset($element['inheritanceType'])) {
             $metadata->setInheritanceType(constant('Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_' . strtoupper($element['inheritanceType'])));
+
+            if ($metadata->inheritanceType != \Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_NONE) {
+                // Evaluate discriminatorColumn
+                if (isset($element['discriminatorColumn'])) {
+                    $discrColumn = $element['discriminatorColumn'];
+                    $metadata->setDiscriminatorColumn(array(
+                        'name' => $discrColumn['name'],
+                        'type' => $discrColumn['type'],
+                        'length' => $discrColumn['length']
+                    ));
+                } else {
+                    throw MappingException::missingDiscriminatorColumn($className);
+                }
+
+                // Evaluate discriminatorMap
+                if (isset($element['discriminatorMap'])) {
+                    $metadata->setDiscriminatorMap($element['discriminatorMap']);
+                } else {
+                    throw MappingException::missingDiscriminatorMap($className);
+                }
+            }
         }
 
-        // Evaluate discriminatorColumn
-        if (isset($element['discriminatorColumn'])) {
-            $discrColumn = $element['discriminatorColumn'];
-            $metadata->setDiscriminatorColumn(array(
-                'name' => $discrColumn['name'],
-                'type' => $discrColumn['type'],
-                'length' => $discrColumn['length']
-            ));
-        }
-
-        // Evaluate discriminatorMap
-        if (isset($element['discriminatorMap'])) {
-            $metadata->setDiscriminatorMap($element['discriminatorMap']);
-        }
 
         // Evaluate changeTrackingPolicy
         if (isset($element['changeTrackingPolicy'])) {

lib/Doctrine/ORM/Mapping/MappingException.php

@@ -190,6 +190,16 @@ class MappingException extends \Doctrine\ORM\ORMException
         );
     }
 
+    public static function missingDiscriminatorMap($className)
+    {
+        return new self("Entity class '$className' is using inheritance but no discriminator map was defined.");
+    }
+
+    public static function missingDiscriminatorColumn($className)
+    {
+        return new self("Entity class '$className' is using inheritance but no discriminator column was defined.");
+    }
+
     /**
      * @param  string $className
      * @param  string $columnName

tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php

@@ -28,9 +28,7 @@ class AnnotationDriverTest extends AbstractMappingDriverTest
     public function testColumnWithMissingTypeDefaultsToString()
     {
         $cm = new ClassMetadata('Doctrine\Tests\ORM\Mapping\ColumnWithoutType');
-        $reader = new \Doctrine\Common\Annotations\AnnotationReader(new \Doctrine\Common\Cache\ArrayCache());
+        $annotationDriver = $this->_loadDriver();
-        $reader->setDefaultAnnotationNamespace('Doctrine\ORM\Mapping\\');
-        $annotationDriver = new \Doctrine\ORM\Mapping\Driver\AnnotationDriver($reader);
 
         $annotationDriver->loadMetadataForClass('Doctrine\Tests\ORM\Mapping\InvalidColumn', $cm);
         $this->assertEquals('string', $cm->fieldMappings['id']['type']);
@@ -92,6 +90,24 @@ class AnnotationDriverTest extends AbstractMappingDriverTest
         $this->assertNotContains($extraneousClassName, $classes);
     }
 
+    public function testInheritenceWithoutDiscriminatorMap()
+    {
+        $cm = new ClassMetadata('Doctrine\Tests\ORM\Mapping\ClassWithoutDiscriminatorMap');
+        $annotationDriver = $this->_loadDriver();
+
+        $this->setExpectedException("Doctrine\ORM\Mapping\MappingException");
+        $annotationDriver->loadMetadataForClass($cm->name, $cm);
+    }
+
+    public function testInheritenceWithoutDiscriminatorColumn()
+    {
+        $cm = new ClassMetadata('Doctrine\Tests\ORM\Mapping\ClassWithoutDiscriminatorColumn');
+        $annotationDriver = $this->_loadDriver();
+
+        $this->setExpectedException("Doctrine\ORM\Mapping\MappingException");
+        $annotationDriver->loadMetadataForClass($cm->name, $cm);
+    }
+
     protected function _loadDriverForCMSModels()
     {
         $annotationDriver = $this->_loadDriver();
@@ -121,3 +137,25 @@ class ColumnWithoutType
     /** @Id @Column */
     public $id;
 }
+
+/**
+ * @Entity
+ * @InheritanceType("SINGLE_TABLE")
+ * @DiscriminatorMap({"a" = "ClassWithoutDiscriminatorColumn"})
+ */
+class ClassWithoutDiscriminatorColumn
+{
+    /** @Id @Column */
+    public $id;
+}
+
+/**
+ * @Entity
+ * @InheritanceType("SINGLE_TABLE")
+ * @DiscriminatorColumn(name="discr", type="string")
+ */
+class ClassWithoutDiscriminatorMap
+{
+    /** @Id @Column */
+    public $id;
+}

tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.CTI.dcm.xml

@@ -3,7 +3,7 @@
       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
       xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
                           http://www.doctrine-project.org/schemas/orm/doctrine-mapping.xsd">
-    <entity name="Doctrine\Tests\ORM\Mapping\CTI">
+    <entity name="Doctrine\Tests\ORM\Mapping\CTI" inheritance-type="JOINED">
         <discriminator-column name="discr" type="string" length="60"/>
         <discriminator-map>
             <discriminator-mapping value="foo" class="CTIFoo"/>