From d115f7af4f35c069be8522e1f7fbdd628644bf4e Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 27 Aug 2010 22:14:48 +0200 Subject: [PATCH] DDC-752 - Postpone Inheritance Related Metadata Validation into CMF --- .../ORM/Mapping/ClassMetadataFactory.php | 159 +++++++++++------- .../ORM/Mapping/Driver/AnnotationDriver.php | 4 - .../DisconnectedClassMetadataFactory.php | 4 +- .../ORM/Mapping/AnnotationDriverTest.php | 40 ----- .../ORM/Mapping/ClassMetadataFactoryTest.php | 16 +- 5 files changed, 106 insertions(+), 117 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php index 7da64480b..400970d6d 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php @@ -38,17 +38,40 @@ use ReflectionException, */ class ClassMetadataFactory { - private $_em; - /** The targeted database platform. */ - private $_targetPlatform; - /** The used metadata driver. */ - private $_driver; - /** The event manager instance */ - private $_evm; - /** The used cache driver. */ - private $_cacheDriver; - private $_loadedMetadata = array(); - private $_initialized = false; + /** + * @var EntityManager + */ + private $em; + + /** + * @var AbstractPlatform + */ + private $targetPlatform; + + /** + * @var Driver\Driver + */ + private $driver; + + /** + * @var \Doctrine\Common\EventManager + */ + private $evm; + + /** + * @var \Doctrine\Common\Cache\Cache + */ + private $cacheDriver; + + /** + * @var array + */ + private $loadedMetadata = array(); + + /** + * @var bool + */ + private $initialized = false; /** * Creates a new factory instance that uses the given metadata driver implementation. @@ -57,7 +80,7 @@ class ClassMetadataFactory */ public function __construct(EntityManager $em) { - $this->_em = $em; + $this->em = $em; } /** @@ -67,7 +90,7 @@ class ClassMetadataFactory */ public function setCacheDriver($cacheDriver) { - $this->_cacheDriver = $cacheDriver; + $this->cacheDriver = $cacheDriver; } /** @@ -77,12 +100,12 @@ class ClassMetadataFactory */ public function getCacheDriver() { - return $this->_cacheDriver; + return $this->cacheDriver; } public function getLoadedMetadata() { - return $this->_loadedMetadata; + return $this->loadedMetadata; } /** @@ -93,12 +116,12 @@ class ClassMetadataFactory */ public function getAllMetadata() { - if ( ! $this->_initialized) { - $this->_initialize(); + if ( ! $this->initialized) { + $this->initialize(); } $metadata = array(); - foreach ($this->_driver->getAllClassNames() as $className) { + foreach ($this->driver->getAllClassNames() as $className) { $metadata[] = $this->getMetadataFor($className); } @@ -109,12 +132,12 @@ class ClassMetadataFactory * Lazy initialization of this stuff, especially the metadata driver, * since these are not needed at all when a metadata cache is active. */ - private function _initialize() + private function initialize() { - $this->_driver = $this->_em->getConfiguration()->getMetadataDriverImpl(); - $this->_targetPlatform = $this->_em->getConnection()->getDatabasePlatform(); - $this->_evm = $this->_em->getEventManager(); - $this->_initialized = true; + $this->driver = $this->em->getConfiguration()->getMetadataDriverImpl(); + $this->targetPlatform = $this->em->getConnection()->getDatabasePlatform(); + $this->evm = $this->em->getEventManager(); + $this->initialized = true; } /** @@ -125,43 +148,43 @@ class ClassMetadataFactory */ public function getMetadataFor($className) { - if ( ! isset($this->_loadedMetadata[$className])) { + if ( ! isset($this->loadedMetadata[$className])) { $realClassName = $className; // Check for namespace alias if (strpos($className, ':') !== false) { list($namespaceAlias, $simpleClassName) = explode(':', $className); - $realClassName = $this->_em->getConfiguration()->getEntityNamespace($namespaceAlias) . '\\' . $simpleClassName; + $realClassName = $this->em->getConfiguration()->getEntityNamespace($namespaceAlias) . '\\' . $simpleClassName; - if (isset($this->_loadedMetadata[$realClassName])) { + if (isset($this->loadedMetadata[$realClassName])) { // We do not have the alias name in the map, include it - $this->_loadedMetadata[$className] = $this->_loadedMetadata[$realClassName]; + $this->loadedMetadata[$className] = $this->loadedMetadata[$realClassName]; - return $this->_loadedMetadata[$realClassName]; + return $this->loadedMetadata[$realClassName]; } } - if ($this->_cacheDriver) { - if (($cached = $this->_cacheDriver->fetch("$realClassName\$CLASSMETADATA")) !== false) { - $this->_loadedMetadata[$realClassName] = $cached; + if ($this->cacheDriver) { + if (($cached = $this->cacheDriver->fetch("$realClassName\$CLASSMETADATA")) !== false) { + $this->loadedMetadata[$realClassName] = $cached; } else { - foreach ($this->_loadMetadata($realClassName) as $loadedClassName) { - $this->_cacheDriver->save( - "$loadedClassName\$CLASSMETADATA", $this->_loadedMetadata[$loadedClassName], null + foreach ($this->loadMetadata($realClassName) as $loadedClassName) { + $this->cacheDriver->save( + "$loadedClassName\$CLASSMETADATA", $this->loadedMetadata[$loadedClassName], null ); } } } else { - $this->_loadMetadata($realClassName); + $this->loadMetadata($realClassName); } if ($className != $realClassName) { // We do not have the alias name in the map, include it - $this->_loadedMetadata[$className] = $this->_loadedMetadata[$realClassName]; + $this->loadedMetadata[$className] = $this->loadedMetadata[$realClassName]; } } - return $this->_loadedMetadata[$className]; + return $this->loadedMetadata[$className]; } /** @@ -172,7 +195,7 @@ class ClassMetadataFactory */ public function hasMetadataFor($className) { - return isset($this->_loadedMetadata[$className]); + return isset($this->loadedMetadata[$className]); } /** @@ -185,7 +208,7 @@ class ClassMetadataFactory */ public function setMetadataFor($className, $class) { - $this->_loadedMetadata[$className] = $class; + $this->loadedMetadata[$className] = $class; } /** @@ -194,12 +217,12 @@ class ClassMetadataFactory * @param string $name * @return array $parentClasses */ - protected function _getParentClasses($name) + protected function getParentClasses($name) { // Collect parent classes, ignoring transient (not-mapped) classes. $parentClasses = array(); foreach (array_reverse(class_parents($name)) as $parentClass) { - if ( ! $this->_driver->isTransient($parentClass)) { + if ( ! $this->driver->isTransient($parentClass)) { $parentClasses[] = $parentClass; } } @@ -213,37 +236,37 @@ class ClassMetadataFactory * @param string $name The name of the class for which the metadata should get loaded. * @param array $tables The metadata collection to which the loaded metadata is added. */ - protected function _loadMetadata($name) + protected function loadMetadata($name) { - if ( ! $this->_initialized) { - $this->_initialize(); + if ( ! $this->initialized) { + $this->initialize(); } $loaded = array(); - $parentClasses = $this->_getParentClasses($name); + $parentClasses = $this->getParentClasses($name); $parentClasses[] = $name; // Move down the hierarchy of parent classes, starting from the topmost class $parent = null; $visited = array(); foreach ($parentClasses as $className) { - if (isset($this->_loadedMetadata[$className])) { - $parent = $this->_loadedMetadata[$className]; + if (isset($this->loadedMetadata[$className])) { + $parent = $this->loadedMetadata[$className]; if ( ! $parent->isMappedSuperclass) { array_unshift($visited, $className); } continue; } - $class = $this->_newClassMetadataInstance($className); + $class = $this->newClassMetadataInstance($className); if ($parent) { $class->setInheritanceType($parent->inheritanceType); $class->setDiscriminatorColumn($parent->discriminatorColumn); $class->setIdGeneratorType($parent->generatorType); - $this->_addInheritedFields($class, $parent); - $this->_addInheritedRelations($class, $parent); + $this->addInheritedFields($class, $parent); + $this->addInheritedRelations($class, $parent); $class->setIdentifier($parent->identifier); $class->setVersioned($parent->isVersioned); $class->setVersionField($parent->versionField); @@ -254,7 +277,7 @@ class ClassMetadataFactory // Invoke driver try { - $this->_driver->loadMetadataForClass($className, $class); + $this->driver->loadMetadataForClass($className, $class); } catch (ReflectionException $e) { throw MappingException::reflectionFailure($className, $e); } @@ -276,7 +299,17 @@ class ClassMetadataFactory $class->setIdGenerator($parent->idGenerator); } } else { - $this->_completeIdGeneratorMapping($class); + $this->completeIdGeneratorMapping($class); + } + + // 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 ($parent && $parent->isInheritanceTypeSingleTable()) { @@ -285,12 +318,12 @@ class ClassMetadataFactory $class->setParentClasses($visited); - if ($this->_evm->hasListeners(Events::loadClassMetadata)) { + if ($this->evm->hasListeners(Events::loadClassMetadata)) { $eventArgs = new \Doctrine\ORM\Event\LoadClassMetadataEventArgs($class); - $this->_evm->dispatchEvent(Events::loadClassMetadata, $eventArgs); + $this->evm->dispatchEvent(Events::loadClassMetadata, $eventArgs); } - $this->_loadedMetadata[$className] = $class; + $this->loadedMetadata[$className] = $class; $parent = $class; @@ -310,7 +343,7 @@ class ClassMetadataFactory * @param string $className * @return Doctrine\ORM\Mapping\ClassMetadata */ - protected function _newClassMetadataInstance($className) + protected function newClassMetadataInstance($className) { return new ClassMetadata($className); } @@ -321,7 +354,7 @@ class ClassMetadataFactory * @param Doctrine\ORM\Mapping\ClassMetadata $subClass * @param Doctrine\ORM\Mapping\ClassMetadata $parentClass */ - private function _addInheritedFields(ClassMetadata $subClass, ClassMetadata $parentClass) + private function addInheritedFields(ClassMetadata $subClass, ClassMetadata $parentClass) { foreach ($parentClass->fieldMappings as $fieldName => $mapping) { if ( ! isset($mapping['inherited']) && ! $parentClass->isMappedSuperclass) { @@ -343,7 +376,7 @@ class ClassMetadataFactory * @param Doctrine\ORM\Mapping\ClassMetadata $subClass * @param Doctrine\ORM\Mapping\ClassMetadata $parentClass */ - private function _addInheritedRelations(ClassMetadata $subClass, ClassMetadata $parentClass) + private function addInheritedRelations(ClassMetadata $subClass, ClassMetadata $parentClass) { foreach ($parentClass->associationMappings as $field => $mapping) { //$subclassMapping = $mapping; @@ -363,13 +396,13 @@ class ClassMetadataFactory * * @param Doctrine\ORM\Mapping\ClassMetadata $class */ - private function _completeIdGeneratorMapping(ClassMetadataInfo $class) + private function completeIdGeneratorMapping(ClassMetadataInfo $class) { $idGenType = $class->generatorType; if ($idGenType == ClassMetadata::GENERATOR_TYPE_AUTO) { - if ($this->_targetPlatform->prefersSequences()) { + if ($this->targetPlatform->prefersSequences()) { $class->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_SEQUENCE); - } else if ($this->_targetPlatform->prefersIdentityColumns()) { + } else if ($this->targetPlatform->prefersIdentityColumns()) { $class->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_IDENTITY); } else { $class->setIdGeneratorType(ClassMetadata::GENERATOR_TYPE_TABLE); @@ -382,7 +415,7 @@ class ClassMetadataFactory // For PostgreSQL IDENTITY (SERIAL) we need a sequence name. It defaults to // __seq in PostgreSQL for SERIAL columns. // Not pretty but necessary and the simplest solution that currently works. - $seqName = $this->_targetPlatform instanceof Platforms\PostgreSQLPlatform ? + $seqName = $this->targetPlatform instanceof Platforms\PostgreSQLPlatform ? $class->table['name'] . '_' . $class->columnNames[$class->identifier[0]] . '_seq' : null; $class->setIdGenerator(new \Doctrine\ORM\Id\IdentityGenerator($seqName)); @@ -392,7 +425,7 @@ class ClassMetadataFactory $definition = $class->sequenceGeneratorDefinition; if ( ! $definition) { $sequenceName = $class->getTableName() . '_' . $class->getSingleIdentifierColumnName() . '_seq'; - $definition['sequenceName'] = $this->_targetPlatform->fixSchemaElementName($sequenceName); + $definition['sequenceName'] = $this->targetPlatform->fixSchemaElementName($sequenceName); $definition['allocationSize'] = 1; $definition['initialValue'] = 1; $class->setSequenceGeneratorDefinition($definition); diff --git a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php index f6faeb891..d62370559 100644 --- a/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php +++ b/lib/Doctrine/ORM/Mapping/Driver/AnnotationDriver.php @@ -179,16 +179,12 @@ class AnnotationDriver implements Driver '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); } } } diff --git a/lib/Doctrine/ORM/Tools/DisconnectedClassMetadataFactory.php b/lib/Doctrine/ORM/Tools/DisconnectedClassMetadataFactory.php index 4957b2c94..436c04b4f 100644 --- a/lib/Doctrine/ORM/Tools/DisconnectedClassMetadataFactory.php +++ b/lib/Doctrine/ORM/Tools/DisconnectedClassMetadataFactory.php @@ -44,7 +44,7 @@ class DisconnectedClassMetadataFactory extends ClassMetadataFactory /** * @override */ - protected function _newClassMetadataInstance($className) + protected function newClassMetadataInstance($className) { return new ClassMetadataInfo($className); } @@ -52,7 +52,7 @@ class DisconnectedClassMetadataFactory extends ClassMetadataFactory /** * @override */ - protected function _getParentClasses($name) + protected function getParentClasses($name) { return array(); } diff --git a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php index 1c3d91eff..468ef680d 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/AnnotationDriverTest.php @@ -90,24 +90,6 @@ 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(); @@ -137,25 +119,3 @@ 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/ClassMetadataFactoryTest.php b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php index 5236bdf91..cbd9ce5e5 100644 --- a/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php +++ b/tests/Doctrine/Tests/ORM/Mapping/ClassMetadataFactoryTest.php @@ -102,27 +102,27 @@ class ClassMetadataFactoryTest extends \Doctrine\Tests\OrmTestCase /* Test subject class with overriden factory method for mocking purposes */ class ClassMetadataFactoryTestSubject extends \Doctrine\ORM\Mapping\ClassMetadataFactory { - private $_mockMetadata = array(); - private $_requestedClasses = array(); + private $mockMetadata = array(); + private $requestedClasses = array(); /** @override */ - protected function _newClassMetadataInstance($className) + protected function newClassMetadataInstance($className) { - $this->_requestedClasses[] = $className; - if ( ! isset($this->_mockMetadata[$className])) { + $this->requestedClasses[] = $className; + if ( ! isset($this->mockMetadata[$className])) { throw new InvalidArgumentException("No mock metadata found for class $className."); } - return $this->_mockMetadata[$className]; + return $this->mockMetadata[$className]; } public function setMetadataForClass($className, $metadata) { - $this->_mockMetadata[$className] = $metadata; + $this->mockMetadata[$className] = $metadata; } public function getRequestedClasses() { - return $this->_requestedClasses; + return $this->requestedClasses; } }