From f9a4dcb2d0b9ceab4766c8e812df603e5a43cd05 Mon Sep 17 00:00:00 2001 From: Benjamin Eberlei Date: Fri, 18 Nov 2011 18:33:03 +0100 Subject: [PATCH] Remove code that could allow users of xml and yaml to define orphan removal on the wrong association sides. --- .../ORM/Mapping/ClassMetadataInfo.php | 123 +++++++++--------- lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php | 8 -- .../ORM/Mapping/Driver/YamlDriver.php | 20 +-- lib/Doctrine/ORM/Mapping/MappingException.php | 12 +- 4 files changed, 80 insertions(+), 83 deletions(-) diff --git a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php index b8c4ef2f1..2734d3647 100644 --- a/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php +++ b/lib/Doctrine/ORM/Mapping/ClassMetadataInfo.php @@ -119,7 +119,7 @@ class ClassMetadataInfo implements ClassMetadata const FETCH_LAZY = 2; /** * Specifies that an association is to be fetched when the owner of the - * association is fetched. + * association is fetched. */ const FETCH_EAGER = 3; /** @@ -206,7 +206,7 @@ class ClassMetadataInfo implements ClassMetadata /** * READ-ONLY: The named queries allowed to be called directly from Repository. - * + * * @var array */ public $namedQueries = array(); @@ -361,7 +361,7 @@ class ClassMetadataInfo implements ClassMetadata * - mappedBy (string, required for bidirectional associations) * The name of the field that completes the bidirectional association on the owning side. * This key must be specified on the inverse side of a bidirectional association. - * + * * - inversedBy (string, required for bidirectional associations) * The name of the field that completes the bidirectional association on the inverse side. * This key must be specified on the owning side of a bidirectional association. @@ -388,7 +388,7 @@ class ClassMetadataInfo implements ClassMetadata * Specification of a field on target-entity that is used to index the collection by. * This field HAS to be either the primary key or a unique column. Otherwise the collection * does not contain all the entities that are actually related. - * + * * A join table definition has the following structure: *
      * array(
@@ -430,7 +430,7 @@ class ClassMetadataInfo implements ClassMetadata
     /**
      * READ-ONLY: The definition of the sequence generator of this class. Only used for the
      * SEQUENCE generation strategy.
-     * 
+     *
      * The definition has the following structure:
      * 
      * array(
@@ -774,15 +774,22 @@ class ClassMetadataInfo implements ClassMetadata
         // If targetEntity is unqualified, assume it is in the same namespace as
         // the sourceEntity.
         $mapping['sourceEntity'] = $this->name;
-        
+
         if (isset($mapping['targetEntity'])) {
             if (strlen($this->namespace) > 0 && strpos($mapping['targetEntity'], '\\') === false) {
                 $mapping['targetEntity'] = $this->namespace . '\\' . $mapping['targetEntity'];
             }
-            
+
             $mapping['targetEntity'] = ltrim($mapping['targetEntity'], '\\');
         }
 
+        if ( ($mapping['type'] & (self::MANY_TO_ONE|self::MANY_TO_MANY)) > 0 &&
+                isset($mapping['orphanRemoval']) &&
+                $mapping['orphanRemoval'] == true) {
+
+            throw MappingException::illegalOrphanRemoval($this->name, $mapping['fieldName']);
+        }
+
         // Complete id mapping
         if (isset($mapping['id']) && $mapping['id'] === true) {
             if (isset($mapping['orphanRemoval']) && $mapping['orphanRemoval'] == true) {
@@ -813,7 +820,7 @@ class ClassMetadataInfo implements ClassMetadata
         if ( ! isset($mapping['targetEntity'])) {
             throw MappingException::missingTargetEntity($mapping['fieldName']);
         }
-        
+
         // Mandatory and optional attributes for either side
         if ( ! $mapping['mappedBy']) {
             if (isset($mapping['joinTable']) && $mapping['joinTable']) {
@@ -829,7 +836,7 @@ class ClassMetadataInfo implements ClassMetadata
         if (isset($mapping['id']) && $mapping['id'] === true && $mapping['type'] & self::TO_MANY) {
             throw MappingException::illegalToManyIdentifierAssoaction($this->name, $mapping['fieldName']);
         }
-        
+
         // Fetch mode. Default fetch mode to LAZY, if not set.
         if ( ! isset($mapping['fetch'])) {
             $mapping['fetch'] = self::FETCH_LAZY;
@@ -837,18 +844,18 @@ class ClassMetadataInfo implements ClassMetadata
 
         // Cascades
         $cascades = isset($mapping['cascade']) ? array_map('strtolower', $mapping['cascade']) : array();
-        
+
         if (in_array('all', $cascades)) {
             $cascades = array('remove', 'persist', 'refresh', 'merge', 'detach');
         }
-        
+
         $mapping['cascade'] = $cascades;
         $mapping['isCascadeRemove'] = in_array('remove',  $cascades);
         $mapping['isCascadePersist'] = in_array('persist',  $cascades);
         $mapping['isCascadeRefresh'] = in_array('refresh',  $cascades);
         $mapping['isCascadeMerge'] = in_array('merge',  $cascades);
         $mapping['isCascadeDetach'] = in_array('detach',  $cascades);
-        
+
         return $mapping;
     }
 
@@ -862,11 +869,11 @@ class ClassMetadataInfo implements ClassMetadata
     protected function _validateAndCompleteOneToOneMapping(array $mapping)
     {
         $mapping = $this->_validateAndCompleteAssociationMapping($mapping);
-        
+
         if (isset($mapping['joinColumns']) && $mapping['joinColumns']) {
             $mapping['isOwningSide'] = true;
         }
-        
+
         if ($mapping['isOwningSide']) {
             if ( ! isset($mapping['joinColumns']) || ! $mapping['joinColumns']) {
                 // Apply default join column
@@ -933,7 +940,7 @@ class ClassMetadataInfo implements ClassMetadata
         if ( ! isset($mapping['mappedBy'])) {
             throw MappingException::oneToManyRequiresMappedBy($mapping['fieldName']);
         }
-        
+
         $mapping['orphanRemoval']   = isset($mapping['orphanRemoval']) ? (bool) $mapping['orphanRemoval'] : false;
         $mapping['isCascadeRemove'] = $mapping['orphanRemoval'] ? true : $mapping['isCascadeRemove'];
 
@@ -942,7 +949,7 @@ class ClassMetadataInfo implements ClassMetadata
                 throw new \InvalidArgumentException("'orderBy' is expected to be an array, not ".gettype($mapping['orderBy']));
             }
         }
-        
+
         return $mapping;
     }
 
@@ -960,7 +967,7 @@ class ClassMetadataInfo implements ClassMetadata
             } else {
                 $targetShortName = strtolower($mapping['targetEntity']);
             }
-            
+
             // owning side MUST have a join table
             if ( ! isset($mapping['joinTable']['name'])) {
                 $mapping['joinTable']['name'] = $sourceShortName .'_' . $targetShortName;
@@ -1112,24 +1119,24 @@ class ClassMetadataInfo implements ClassMetadata
     public function getIdentifierColumnNames()
     {
         $columnNames = array();
-        
+
         foreach ($this->identifier as $idProperty) {
             if (isset($this->fieldMappings[$idProperty])) {
                 $columnNames[] = $this->fieldMappings[$idProperty]['columnName'];
-                
+
                 continue;
             }
-            
+
             // Association defined as Id field
             $joinColumns      = $this->associationMappings[$idProperty]['joinColumns'];
             $assocColumnNames = array_map(function ($joinColumn) { return $joinColumn['name']; }, $joinColumns);
-            
+
             $columnNames = array_merge($columnNames, $assocColumnNames);
         }
-        
+
         return $columnNames;
     }
-    
+
     /**
      * Sets the type of Id generator to use for the mapped class.
      */
@@ -1369,11 +1376,11 @@ class ClassMetadataInfo implements ClassMetadata
                 $this->table['name'] = $table['name'];
             }
         }
-        
+
         if (isset($table['indexes'])) {
             $this->table['indexes'] = $table['indexes'];
         }
-        
+
         if (isset($table['uniqueConstraints'])) {
             $this->table['uniqueConstraints'] = $table['uniqueConstraints'];
         }
@@ -1448,7 +1455,7 @@ class ClassMetadataInfo implements ClassMetadata
         if (isset($this->namedQueries[$queryMapping['name']])) {
             throw MappingException::duplicateQueryMapping($this->name, $queryMapping['name']);
         }
-        
+
         $name   = $queryMapping['name'];
         $query  = $queryMapping['query'];
         $dql    = str_replace('__CLASS__', $this->name, $query);
@@ -1516,11 +1523,11 @@ class ClassMetadataInfo implements ClassMetadata
     protected function _storeAssociationMapping(array $assocMapping)
     {
         $sourceFieldName = $assocMapping['fieldName'];
-        
+
         if (isset($this->fieldMappings[$sourceFieldName]) || isset($this->associationMappings[$sourceFieldName])) {
             throw MappingException::duplicateFieldMapping($this->name, $sourceFieldName);
         }
-        
+
         $this->associationMappings[$sourceFieldName] = $assocMapping;
     }
 
@@ -1531,7 +1538,7 @@ class ClassMetadataInfo implements ClassMetadata
      */
     public function setCustomRepositoryClass($repositoryClassName)
     {
-        if ($repositoryClassName !== null && strpos($repositoryClassName, '\\') === false 
+        if ($repositoryClassName !== null && strpos($repositoryClassName, '\\') === false
                 && strlen($this->namespace) > 0) {
             $repositoryClassName = $this->namespace . '\\' . $repositoryClassName;
         }
@@ -1731,7 +1738,7 @@ class ClassMetadataInfo implements ClassMetadata
 
     /**
      * Return the single association join column (if any).
-     * 
+     *
      * @param string $fieldName
      * @return string
      */
@@ -1773,7 +1780,7 @@ class ClassMetadataInfo implements ClassMetadata
             foreach ($this->associationMappings AS $assocName => $mapping) {
                 if ($this->isAssociationWithSingleJoinColumn($assocName) &&
                     $this->associationMappings[$assocName]['joinColumns'][0]['name'] == $columnName) {
-                    
+
                     return $assocName;
                 }
             }
@@ -1863,34 +1870,34 @@ class ClassMetadataInfo implements ClassMetadata
     {
         $this->isReadOnly = true;
     }
-    
+
     /**
      * A numerically indexed list of field names of this persistent class.
-     * 
+     *
      * This array includes identifier fields if present on this class.
-     * 
+     *
      * @return array
      */
     public function getFieldNames()
     {
         return array_keys($this->fieldMappings);
     }
-    
+
     /**
      * A numerically indexed list of association names of this persistent class.
-     * 
+     *
      * This array includes identifier associations if present on this class.
-     * 
+     *
      * @return array
      */
     public function getAssociationNames()
     {
         return array_keys($this->associationMappings);
     }
-    
+
     /**
      * Returns the target class name of the given association.
-     * 
+     *
      * @param string $assocName
      * @return string
      */
@@ -1899,13 +1906,13 @@ class ClassMetadataInfo implements ClassMetadata
         if ( ! isset($this->associationMappings[$assocName])) {
             throw new \InvalidArgumentException("Association name expected, '" . $assocName ."' is not an association.");
         }
-        
+
         return $this->associationMappings[$assocName]['targetEntity'];
     }
-    
+
     /**
      * Get fully-qualified class name of this persistent class.
-     * 
+     *
      * @return string
      */
     public function getName()
@@ -1915,59 +1922,59 @@ class ClassMetadataInfo implements ClassMetadata
 
     /**
      * Gets the (possibly quoted) identifier column names for safe use in an SQL statement.
-     * 
+     *
      * @param AbstractPlatform $platform
      * @return array
      */
     public function getQuotedIdentifierColumnNames($platform)
     {
         $quotedColumnNames = array();
-        
+
         foreach ($this->identifier as $idProperty) {
             if (isset($this->fieldMappings[$idProperty])) {
-                $quotedColumnNames[] = isset($this->fieldMappings[$idProperty]['quoted']) 
-                    ? $platform->quoteIdentifier($this->fieldMappings[$idProperty]['columnName']) 
+                $quotedColumnNames[] = isset($this->fieldMappings[$idProperty]['quoted'])
+                    ? $platform->quoteIdentifier($this->fieldMappings[$idProperty]['columnName'])
                     : $this->fieldMappings[$idProperty]['columnName'];
-                
+
                 continue;
             }
-            
+
             // Association defined as Id field
             $joinColumns            = $this->associationMappings[$idProperty]['joinColumns'];
             $assocQuotedColumnNames = array_map(
                 function ($joinColumn) {
-                    return isset($joinColumn['quoted']) 
-                        ? $platform->quoteIdentifier($joinColumn['name']) 
+                    return isset($joinColumn['quoted'])
+                        ? $platform->quoteIdentifier($joinColumn['name'])
                         : $joinColumn['name'];
-                }, 
+                },
                 $joinColumns
             );
-            
+
             $quotedColumnNames = array_merge($quotedColumnNames, $assocQuotedColumnNames);
         }
-        
+
         return $quotedColumnNames;
     }
 
     /**
      * Gets the (possibly quoted) column name of a mapped field for safe use
      * in an SQL statement.
-     * 
+     *
      * @param string $field
      * @param AbstractPlatform $platform
      * @return string
      */
     public function getQuotedColumnName($field, $platform)
     {
-        return isset($this->fieldMappings[$field]['quoted']) 
-            ? $platform->quoteIdentifier($this->fieldMappings[$field]['columnName']) 
+        return isset($this->fieldMappings[$field]['quoted'])
+            ? $platform->quoteIdentifier($this->fieldMappings[$field]['columnName'])
             : $this->fieldMappings[$field]['columnName'];
     }
-    
+
     /**
      * Gets the (possibly quoted) primary table name of this class for safe use
      * in an SQL statement.
-     * 
+     *
      * @param AbstractPlatform $platform
      * @return string
      */
diff --git a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
index 6f66337db..6f655d0db 100644
--- a/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
+++ b/lib/Doctrine/ORM/Mapping/Driver/XmlDriver.php
@@ -377,10 +377,6 @@ class XmlDriver extends AbstractFileDriver
                     $mapping['cascade'] = $this->_getCascadeMappings($manyToOneElement->cascade);
                 }
 
-                if (isset($manyToOneElement->{'orphan-removal'})) {
-                    $mapping['orphanRemoval'] = (bool)$manyToOneElement->{'orphan-removal'};
-                }
-
                 $metadata->mapManyToOne($mapping);
             }
         }
@@ -428,10 +424,6 @@ class XmlDriver extends AbstractFileDriver
                     $mapping['cascade'] = $this->_getCascadeMappings($manyToManyElement->cascade);
                 }
 
-                if (isset($manyToManyElement->{'orphan-removal'})) {
-                    $mapping['orphanRemoval'] = (bool)$manyToManyElement->{'orphan-removal'};
-                }
-
                 if (isset($manyToManyElement->{'order-by'})) {
                     $orderBy = array();
                     foreach ($manyToManyElement->{'order-by'}->{'order-by-field'} AS $orderByField) {
diff --git a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
index 5979ae73f..a33c19b6b 100644
--- a/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
+++ b/lib/Doctrine/ORM/Mapping/Driver/YamlDriver.php
@@ -47,7 +47,7 @@ class YamlDriver extends AbstractFileDriver
 
         if ($element['type'] == 'entity') {
             if (isset($element['repositoryClass'])) {
-                $metadata->setCustomRepositoryClass($element['repositoryClass']);  
+                $metadata->setCustomRepositoryClass($element['repositoryClass']);
             }
             if (isset($element['readOnly']) && $element['readOnly'] == true) {
                 $metadata->markReadOnly();
@@ -169,7 +169,7 @@ class YamlDriver extends AbstractFileDriver
                     'id' => true,
                     'fieldName' => $name
                 );
-                
+
                 if (isset($idElement['type'])) {
                     $mapping['type'] = $idElement['type'];
                 }
@@ -200,21 +200,21 @@ class YamlDriver extends AbstractFileDriver
         // Evaluate fields
         if (isset($element['fields'])) {
             foreach ($element['fields'] as $name => $fieldMapping) {
-                
+
                 $mapping = array(
                     'fieldName' => $name
                 );
-                
+
                 if (isset($fieldMapping['type'])) {
                     $e = explode('(', $fieldMapping['type']);
                     $fieldMapping['type'] = $e[0];
                     $mapping['type']      = $fieldMapping['type'];
-                    
+
                     if (isset($e[1])) {
                         $fieldMapping['length'] = substr($e[1], 0, strlen($e[1]) - 1);
                     }
                 }
-                
+
                 if (isset($fieldMapping['id'])) {
                     $mapping['id'] = true;
                     if (isset($fieldMapping['generator']['strategy'])) {
@@ -379,10 +379,6 @@ class YamlDriver extends AbstractFileDriver
                     $mapping['cascade'] = $manyToOneElement['cascade'];
                 }
 
-                if (isset($manyToOneElement['orphanRemoval'])) {
-                    $mapping['orphanRemoval'] = (bool)$manyToOneElement['orphanRemoval'];
-                }
-
                 $metadata->mapManyToOne($mapping);
             }
         }
@@ -438,10 +434,6 @@ class YamlDriver extends AbstractFileDriver
                     $mapping['cascade'] = $manyToManyElement['cascade'];
                 }
 
-                if (isset($manyToManyElement['orphanRemoval'])) {
-                    $mapping['orphanRemoval'] = (bool)$manyToManyElement['orphanRemoval'];
-                }
-
                 if (isset($manyToManyElement['orderBy'])) {
                     $mapping['orderBy'] = $manyToManyElement['orderBy'];
                 }
diff --git a/lib/Doctrine/ORM/Mapping/MappingException.php b/lib/Doctrine/ORM/Mapping/MappingException.php
index e32e34c16..e89347d42 100644
--- a/lib/Doctrine/ORM/Mapping/MappingException.php
+++ b/lib/Doctrine/ORM/Mapping/MappingException.php
@@ -184,7 +184,7 @@ class MappingException extends \Doctrine\ORM\ORMException
         if ( ! empty($path)) {
             $path = '[' . $path . ']';
         }
-        
+
         return new self(
             'File mapping drivers must have a valid directory path, ' .
             'however the given path ' . $path . ' seems to be incorrect!'
@@ -270,6 +270,12 @@ class MappingException extends \Doctrine\ORM\ORMException
             "part of the identifier in '$className#$field'.");
     }
 
+    public static function illegalOrphanRemoval($className, $field)
+    {
+        return new self("Orphan removal is only allowed on one-to-one and one-to-many ".
+                "associations, but " . $className."#" .$field . " is not.");
+    }
+
     public static function illegalInverseIdentifierAssocation($className, $field)
     {
         return new self("An inverse association is not allowed to be identifier in '$className#$field'.");
@@ -279,12 +285,12 @@ class MappingException extends \Doctrine\ORM\ORMException
     {
         return new self("Many-to-many or one-to-many associations are not allowed to be identifier in '$className#$field'.");
     }
-    
+
     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(