From 69e9fd3145928960868a31a53bb05d7f54ee47b6 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Sun, 8 Aug 2010 12:29:14 +0200 Subject: [PATCH] DDC-704 - Added better validation of inheritence type constructs in Xml, Annotation and Yaml Drivers --- .../ORM/Mapping/Driver/AnnotationDriver.php | 37 +++++++++------- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 39 +++++++++------- .../ORM/Mapping/Driver/YamlDriver.php | 35 +++++++++------ lib/Doctrine/ORM/Mapping/MappingException.php | 10 +++++ .../ORM/Mapping/AnnotationDriverTest.php | 44 +++++++++++++++++-- .../Doctrine.Tests.ORM.Mapping.CTI.dcm.xml | 2 +- 6 files changed, 118 insertions(+), 49 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index 6a61772a7..7d0c1f778 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/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'])) { diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php index da2885b32..e8f97d835 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php +++ b/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 - if (isset($xmlRoot->{'discriminator-column'})) { - $discrColumn = $xmlRoot->{'discriminator-column'}; - $metadata->setDiscriminatorColumn(array( - 'name' => (string)$discrColumn['name'], - 'type' => (string)$discrColumn['type'], - 'length' => (string)$discrColumn['length'] - )); - } + if ($metadata->inheritanceType != \Doctrine\ORM\Mapping\ClassMetadata::INHERITANCE_TYPE_NONE) { + // Evaluate + if (isset($xmlRoot->{'discriminator-column'})) { + $discrColumn = $xmlRoot->{'discriminator-column'}; + $metadata->setDiscriminatorColumn(array( + 'name' => (string)$discrColumn['name'], + 'type' => (string)$discrColumn['type'], + 'length' => (string)$discrColumn['length'] + )); + } else { + throw MappingException::missingDiscriminatorColumn($className); + } - // Evaluate - if (isset($xmlRoot->{'discriminator-map'})) { - $map = array(); - foreach ($xmlRoot->{'discriminator-map'}->{'discriminator-mapping'} AS $discrMapElement) { - $map[(string)$discrMapElement['value']] = (string)$discrMapElement['class']; + // Evaluate + if (isset($xmlRoot->{'discriminator-map'})) { + $map = array(); + foreach ($xmlRoot->{'discriminator-map'}->{'discriminator-mapping'} AS $discrMapElement) { + $map[(string)$discrMapElement['value']] = (string)$discrMapElement['class']; + } + $metadata->setDiscriminatorMap($map); + } else { + throw MappingException::missingDiscriminatorMap($className); + } } - $metadata->setDiscriminatorMap($map); } + // Evaluate if (isset($xmlRoot['change-tracking-policy'])) { $metadata->setChangeTrackingPolicy(constant('Doctrine\ORM\Mapping\ClassMetadata::CHANGETRACKING_' diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php index 79c659814..967760785 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php +++ b/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'])) { diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php index 4e7247ab3..9fa5906bf 100644 --- a/lib/Doctrine/ORM/Mapping/MappingException.php +++ b/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 diff --git a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php index 484ce01ba..1c3d91eff 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php +++ b/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()); - $reader->setDefaultAnnotationNamespace('Doctrine\ORM\Mapping\\'); - $annotationDriver = new \Doctrine\ORM\Mapping\Driver\AnnotationDriver($reader); + $annotationDriver = $this->_loadDriver(); $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; +} \ No newline at end of file diff --git a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.CTI.dcm.xml b/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.CTI.dcm.xml index e6459afae..14abaef73 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/xml/Doctrine.Tests.ORM.Mapping.CTI.dcm.xml +++ b/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"> - +