Joined strategy bugfix and cosmetics.

lib/Doctrine/Mapper/Abstract.php

@@ -425,7 +425,7 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
                 . ' WHERE ' . implode(' = ? && ', $identifierColumnNames) . ' = ?';
             $query = $this->applyInheritance($query);
 
-            $params = array_merge(array($id), array_values($this->getDiscriminatorColumn()));
+            $params = array_merge(array($id),array());
 
             $data = $this->_conn->execute($query, $params)->fetch(PDO::FETCH_ASSOC);
 
@@ -804,11 +804,6 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
         }
     }
     
-    protected function _fireEvent($type, $callback, $invoker)
-    {
-        
-    }
-    
     /**
      * saves the given record
      *
@@ -817,7 +812,6 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
      */
     public function saveSingleRecord(Doctrine_Record $record)
     {
-        //$this->_fireEvent(Doctrine_Event::RECORD_SAVE, 'preSave', $record);
         $event = new Doctrine_Event($record, Doctrine_Event::RECORD_SAVE);
         $record->preSave($event);
         $this->getRecordListener()->preSave($event);
@@ -926,7 +920,7 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
     }
     
     /**
-     * updates given record
+     * Updates an entity.
      *
      * @param Doctrine_Record $record   record to be updated
      * @return boolean                  whether or not the update was successful
@@ -949,6 +943,9 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
         return true;
     }
     
+    /**
+     * Updates an entity.
+     */
     protected function _doUpdate(Doctrine_Record $record)
     {
         $identifier = $record->identifier();
@@ -958,7 +955,7 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
     }
     
     /**
-     * inserts a record into database
+     * Inserts an entity.
      *
      * @param Doctrine_Record $record   record to be inserted
      * @return boolean
@@ -982,6 +979,9 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
         return true;
     }
     
+    /**
+     * Inserts an entity.
+     */
     protected function _doInsert(Doctrine_Record $record)
     {
         $this->insertSingleRecord($record);
@@ -1033,11 +1033,14 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
         return true;
     }
     
+    /**
+     * Deletes an entity.
+     */
     protected function _doDelete(Doctrine_Record $record, Doctrine_Connection $conn)
     {
         try {
             $conn->beginInternalTransaction();
-            $this->deleteComposites($record);
+            $this->_deleteComposites($record);
 
             $record->state(Doctrine_Record::STATE_TDIRTY);
 
@@ -1060,7 +1063,7 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
      * @throws PDOException         if something went wrong at database level
      * @return void
      */
-    protected function deleteComposites(Doctrine_Record $record)
+    protected function _deleteComposites(Doctrine_Record $record)
     {
         foreach ($this->_classMetadata->getRelations() as $fk) {
             if ($fk->isComposite()) {
@@ -1125,18 +1128,19 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
     
     /* Hooks used during SQL query construction to manipulate the query. */
     
+    /**
+     * Callback that is invoked during the SQL construction process.
+     */
     public function getCustomJoins()
     {
         return array();
     }
     
+    /**
+     * Callback that is invoked during the SQL construction process.
+     */
     public function getCustomFields()
     {
         return array();
     }
-    
-    public function getDiscriminatorColumn()
-    {
-        return array();
-    }
 }

lib/Doctrine/Mapper/Joined.php

@@ -5,7 +5,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
     protected $_columnNameFieldNameMap = array();
     
     /**
-     * inserts a record into database
+     * Inserts an entity that is part of a Class Table Inheritance hierarchy.
      *
      * @param Doctrine_Record $record   record to be inserted
      * @return boolean
@@ -18,7 +18,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
         $dataSet = $this->_formatDataSet($record);
         $component = $table->getClassName();
 
-        $classes = $table->getOption('parents');
+        $classes = $table->getParentClasses();
         array_unshift($classes, $component);
         
         try {
@@ -59,7 +59,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
     }
     
     /**
-     * updates given record
+     * Updates an entity that is part of a Class Table Inheritance hierarchy.
      *
      * @param Doctrine_Record $record   record to be updated
      * @return boolean                  whether or not the update was successful
@@ -93,7 +93,10 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
         return true;
     }
     
-
+    /**
+     * Deletes an entity that is part of a Class Table Inheritance hierarchy.
+     *
+     */
     protected function _doDelete(Doctrine_Record $record, Doctrine_Connection $conn)
     {
         try {
@@ -122,49 +125,49 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
     }
     
     /**
+     * Adds all parent classes as INNER JOINs and subclasses as OUTER JOINs
+     * to the query.
      *
+     * Callback that is invoked during the SQL construction process.
      *
+     * @return array  The custom joins in the format <className> => <joinType>
      */
     public function getCustomJoins()
     {
         $customJoins = array();
-        foreach ($this->_classMetadata->getOption('parents') as $parentClass) {
+        foreach ($this->_classMetadata->getParentClasses() as $parentClass) {
             $customJoins[$parentClass] = 'INNER';
         }
-        foreach ((array)$this->_classMetadata->getOption('subclasses') as $subClass) {
+        foreach ((array)$this->_classMetadata->getSubclasses() as $subClass) {
             if ($subClass != $this->_domainClassName) {
                 $customJoins[$subClass] = 'LEFT';
             }
         }
+        
         return $customJoins;
     }
     
+    /**
+     * Adds the discriminator column to the selected fields in a query as well as
+     * all fields of subclasses. In Class Table Inheritance the default behavior is that
+     * all subclasses are joined in through OUTER JOINs when querying a base class.
+     *
+     * Callback that is invoked during the SQL construction process.
+     *
+     * @return array  An array with the field names that will get added to the query.
+     */
     public function getCustomFields()
     {
         $fields = array($this->_classMetadata->getInheritanceOption('discriminatorColumn'));
-        //$fields = array();
-        if ($this->_classMetadata->getOption('subclasses')) {
-            foreach ($this->_classMetadata->getOption('subclasses') as $subClass) {
+        if ($this->_classMetadata->getSubclasses()) {
+            foreach ($this->_classMetadata->getSubclasses() as $subClass) {
                 $fields = array_merge($this->_conn->getMetadata($subClass)->getFieldNames(), $fields);
             }
         }
+        
         return array_unique($fields);
     }
     
-    /**
-     *
-     */
-    public function getDiscriminatorColumn()
-    {
-        $joinedParents = $this->_classMetadata->getOption('joinedParents');
-        if (count($joinedParents) <= 0) {
-            $inheritanceMap = $this->_classMetadata->getOption('inheritanceMap');
-        } else {
-            $inheritanceMap = $this->_conn->getTable(array_pop($joinedParents))->getOption('inheritanceMap');
-        }
-        return isset($inheritanceMap[$this->_domainClassName]) ? $inheritanceMap[$this->_domainClassName] : array();
-    }
-    
     /**
      *
      */
@@ -180,6 +183,9 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
         return $fieldNames;
     }
     
+    /**
+     * 
+     */
     public function getFieldName($columnName)
     {
         if (isset($this->_columnNameFieldNameMap[$columnName])) {
@@ -191,7 +197,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
             return $this->_columnNameFieldNameMap[$columnName];
         }
         
-        foreach ($this->_classMetadata->getOption('parents') as $parentClass) {
+        foreach ($this->_classMetadata->getParentClasses() as $parentClass) {
             $parentTable = $this->_conn->getMetadata($parentClass);
             if ($parentTable->hasColumn($columnName)) {
                 $this->_columnNameFieldNameMap[$columnName] = $parentTable->getFieldName($columnName);
@@ -199,7 +205,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
             }
         }
         
-        foreach ((array)$this->_classMetadata->getOption('subclasses') as $subClass) {
+        foreach ((array)$this->_classMetadata->getSubclasses() as $subClass) {
             $subTable = $this->_conn->getMetadata($subClass);
             if ($subTable->hasColumn($columnName)) {
                 $this->_columnNameFieldNameMap[$columnName] = $subTable->getFieldName($columnName);
@@ -210,20 +216,24 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
         throw new Doctrine_Mapper_Exception("No field name found for column name '$columnName'.");
     }
     
+    /**
+     * 
+     * @todo Looks like this better belongs into the ClassMetadata class.
+     */
     public function getOwningTable($fieldName)
     {        
         if ($this->_classMetadata->hasField($fieldName) && ! $this->_classMetadata->isInheritedField($fieldName)) {
             return $this->_classMetadata;
         }
         
-        foreach ($this->_classMetadata->getOption('parents') as $parentClass) {
+        foreach ($this->_classMetadata->getParentClasses() as $parentClass) {
             $parentTable = $this->_conn->getMetadata($parentClass);
             if ($parentTable->hasField($fieldName) && ! $parentTable->isInheritedField($fieldName)) {
                 return $parentTable;
             }
         }
         
-        foreach ((array)$this->_classMetadata->getOption('subclasses') as $subClass) {
+        foreach ((array)$this->_classMetadata->getSubclasses() as $subClass) {
             $subTable = $this->_conn->getMetadata($subClass);
             if ($subTable->hasField($fieldName) && ! $subTable->isInheritedField($fieldName)) {
                 return $subTable;
@@ -234,7 +244,9 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
     }
     
     /**
-     * 
+     * Analyzes the fields of the entity and creates a map in which the field names
+     * are grouped by the class names they belong to. 
+     *
      */
     protected function _formatDataSet(Doctrine_Record $record)
     {
@@ -246,6 +258,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
         $classes = array_merge(array($component), $this->_classMetadata->getParentClasses());
         
         foreach ($classes as $class) {
+            $dataSet[$class] = array();            
             $metadata = $this->_conn->getMetadata($class);
             foreach ($metadata->getColumns() as $columnName => $definition) {
                 if ((isset($definition['primary']) && $definition['primary'] === true) ||
@@ -253,7 +266,10 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
                     continue;
                 }
                 $fieldName = $table->getFieldName($columnName);
-                $dataSet[$class][$fieldName] = isset($array[$fieldName]) ? $array[$fieldName] : null;
+                if ( ! array_key_exists($fieldName, $array)) {
+                    continue;
+                }
+                $dataSet[$class][$fieldName] = $array[$fieldName];
             }
         }
         

lib/Doctrine/Mapper/SingleTable.php

@@ -3,12 +3,6 @@
 class Doctrine_Mapper_SingleTable extends Doctrine_Mapper_Abstract
 {
     
-    public function getDiscriminatorColumn()
-    {
-        $inheritanceMap = $this->_classMetadata->getOption('inheritanceMap');
-        return isset($inheritanceMap[$this->_domainClassName]) ? $inheritanceMap[$this->_domainClassName] : array();
-    }
-    
     /*public function addToWhere($componentAlias, array &$sqlWhereParts, Doctrine_Query $query)
     {
         $array = array();

tests/Inheritance/JoinedTestCase.php

@@ -102,6 +102,19 @@ class Doctrine_Inheritance_Joined_TestCase extends Doctrine_UnitTestCase
         $this->assertTrue($superManager instanceof CTI_SuperManager);
     }
     
+    public function testUpdateUpdatesOnlyChangedFields()
+    {
+        $manager = $this->_createManager();        
+        try {
+            $manager->salary = 12;
+            $manager->save();
+            $this->pass();
+        } catch (Exception $e) {
+            $this->fail("Update failed [{$e->getMessage()}].");
+        }
+        
+    }
+    
     public function testUpdateUpdatesDataAcrossJoinedTablesTransparently()
     {
         $manager = $this->_createManager();
@@ -183,7 +196,7 @@ class CTI_User extends Doctrine_Record
         $class->setTableName('cti_user');
         $class->setColumn('cti_id as id', 'integer', 4, array('primary' => true, 'autoincrement' => true));
         $class->setColumn('cti_foo as foo', 'integer', 4);
-        $class->setColumn('cti_name as name', 'string', 50);
+        $class->setColumn('cti_name as name', 'string', 50, array('notnull' => true));
         $class->setColumn('dtype', 'integer', 2);
     }    
 }