refactorings

lib/Doctrine/Connection.php

@@ -510,26 +510,26 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @throws PDOException                         if something fails at PDO level
      * @return integer                              number of rows affected
      */
-    public function replace(Doctrine_Table $table, array $fields, array $keys)
+    public function replace($tableName, array $data, array $keys)
     {
         if (empty($keys)) {
             throw new Doctrine_Connection_Exception('Not specified which fields are keys');
         }
         $condition = $values = array();
 
-        foreach ($fields as $fieldName => $value) {
-            $values[$fieldName] = $value;
+        foreach ($data as $columnName => $value) {
+            $values[$columnName] = $value;
 
-            if (in_array($fieldName, $keys)) {
+            if (in_array($columnName, $keys)) {
                 if ($value === null)
-                    throw new Doctrine_Connection_Exception('key value '.$fieldName.' may not be null');
+                    throw new Doctrine_Connection_Exception('key value '.$columnName.' may not be null');
 
-                $condition[] = $table->getColumnName($fieldName) . ' = ?';
+                $condition[] = $columnName . ' = ?';
                 $conditionValues[] = $value;
             }
         }
 
-        $query = 'DELETE FROM ' . $this->quoteIdentifier($table->getTableName())
+        $query = 'DELETE FROM ' . $this->quoteIdentifier($tableName)
                 . ' WHERE ' . implode(' AND ', $condition);
         $affectedRows = $this->exec($query, $conditionValues);
 
@@ -541,25 +541,22 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
     }
 
     /**
-     * deletes table row(s) matching the specified identifier
+     * Deletes table row(s) matching the specified identifier.
      *
      * @throws Doctrine_Connection_Exception    if something went wrong at the database level
      * @param string $table         The table to delete data from
      * @param array $identifier     An associateve array containing identifier fieldname-value pairs.
      * @return integer              The number of affected rows
-     *
-     * @todo First argument should just be a table name. Move the conversion from
-     *       field to column names one layer up.
      */
-    public function delete(Doctrine_ClassMetadata $table, array $identifier)
+    public function delete($tableName, array $identifier)
     {
         $criteria = array();
         foreach (array_keys($identifier) as $id) {
-            $criteria[] = $table->getColumnName($id) . ' = ?';
+            $criteria[] = $id . ' = ?';
         }
 
         $query = 'DELETE FROM '
-               . $this->quoteIdentifier($table->getTableName())
+               . $this->quoteIdentifier($tableName)
                . ' WHERE ' . implode(' AND ', $criteria);
 
         return $this->exec($query, array_values($identifier));
@@ -573,31 +570,28 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @param array $values     An associateve array containing column-value pairs.
      * @return mixed            boolean false if empty value array was given,
      *                          otherwise returns the number of affected rows
-     *
-     * @todo First argument should just be a table name. Move the conversion from
-     *       field to column names one layer up.
      */
-    public function update(Doctrine_ClassMetadata $table, array $fields, array $identifier)
+    public function update($tableName, array $data, array $identifier)
     {
-        if (empty($fields)) {
+        if (empty($data)) {
             return false;
         }
 
         $set = array();
-        foreach ($fields as $fieldName => $value) {
+        foreach ($data as $columnName => $value) {
             if ($value instanceof Doctrine_Expression) {
-                $set[] = $table->getColumnName($fieldName) . ' = ' . $value->getSql();
-                unset($fields[$fieldName]);
+                $set[] = $columnName . ' = ' . $value->getSql();
+                unset($data[$columnName]);
             } else {
-                $set[] = $table->getColumnName($fieldName) . ' = ?';
+                $set[] = $columnName . ' = ?';
             }
         }
 
-        $params = array_merge(array_values($fields), array_values($identifier));
+        $params = array_merge(array_values($data), array_values($identifier));
 
-        $sql  = 'UPDATE ' . $this->quoteIdentifier($table->getTableName())
+        $sql  = 'UPDATE ' . $this->quoteIdentifier($tableName)
               . ' SET ' . implode(', ', $set)
-              . ' WHERE ' . implode(' = ? AND ', $table->getIdentifierColumnNames())
+              . ' WHERE ' . implode(' = ? AND ', array_keys($identifier))
               . ' = ?';
 
         return $this->exec($sql, $params);
@@ -610,27 +604,22 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @param array $fields     An associateve array containing fieldname-value pairs.
      * @return mixed            boolean false if empty value array was given,
      *                          otherwise returns the number of affected rows
-     * 
-     * @todo First argument should just be a table name. Move the conversion from
-     *       field to column names one layer up.
      */
-    public function insert(Doctrine_ClassMetadata $table, array $fields)
+    public function insert($tableName, array $data)
     {
-        if (empty($fields)) {
+        if (empty($data)) {
             return false;
         }
 
-        $tableName = $table->getTableName();
-
         // column names are specified as array keys
         $cols = array();
         // the query VALUES will contain either expresions (eg 'NOW()') or ?
         $a = array();
-        foreach ($fields as $fieldName => $value) {
-            $cols[] = $this->quoteIdentifier($table->getColumnName($fieldName));
+        foreach ($data as $columnName => $value) {
+            $cols[] = $this->quoteIdentifier($columnName);
             if ($value instanceof Doctrine_Expression) {
                 $a[] = $value->getSql();
-                unset($fields[$fieldName]);
+                unset($data[$columnName]);
             } else {
                 $a[] = '?';
             }
@@ -643,8 +632,8 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
 
         $query .= implode(', ', $a) . ')';
         // prepare and execute the statement
-
-        return $this->exec($query, array_values($fields));
+        
+        return $this->exec($query, array_values($data));
     }
 
     /**

lib/Doctrine/Connection/Mysql.php

@@ -166,43 +166,43 @@ class Doctrine_Connection_Mysql extends Doctrine_Connection_Common
      *
      * @return integer      the number of affected rows
      */
-    public function replace(Doctrine_Table $table, array $fields, array $keys)
+    public function replace($tableName, array $data, array $keys)
     {
-        $count = count($fields);
+        $count = count($data);
         $query = $values = '';
         $keys = $colnum = 0;
 
-        for (reset($fields); $colnum < $count; next($fields), $colnum++) {
-            $name = key($fields);
+        for (reset($data); $colnum < $count; next($data), $colnum++) {
+            $name = key($data);
 
             if ($colnum > 0) {
                 $query .= ',';
                 $values.= ',';
             }
 
-            $query .= $table->getColumnName($name);
+            $query .= $name;
 
-            if (isset($fields[$name]['null']) && $fields[$name]['null']) {
+            if (isset($data[$name]['null']) && $data[$name]['null']) {
                 $value = 'NULL';
             } else {
-                $type = isset($fields[$name]['type']) ? $fields[$name]['type'] : null;
-                $value = $this->quote($fields[$name]['value'], $type);
+                $type = isset($data[$name]['type']) ? $data[$name]['type'] : null;
+                $value = $this->quote($data[$name]['value'], $type);
             }
 
             $values .= $value;
 
-            if (isset($fields[$name]['key']) && $fields[$name]['key']) {
+            if (isset($data[$name]['key']) && $data[$name]['key']) {
                 if ($value === 'NULL') {
-                    throw new Doctrine_Connection_Mysql_Exception('key value '.$name.' may not be NULL');
+                    throw new Doctrine_Connection_Mysql_Exception('key value '.$name.' may not be NULL.');
                 }
                 $keys++;
             }
         }
 
         if ($keys == 0) {
-            throw new Doctrine_Connection_Mysql_Exception('not specified which fields are keys');
+            throw new Doctrine_Connection_Mysql_Exception('Not specified which fields are keys.');
         }
-        $query = 'REPLACE INTO ' . $table->getTableName() . ' (' . $query . ') VALUES (' . $values . ')';
+        $query = 'REPLACE INTO ' . $tableName . ' (' . $query . ') VALUES (' . $values . ')';
 
         return $this->exec($query);
     }

lib/Doctrine/Mapper/Abstract.php

@@ -773,23 +773,24 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
             return false;
         }
         
-        $table = $record->getTable();
-        $identifier = (array) $table->getIdentifier();
+        $class = $record->getClassMetadata();
+        $identifier = (array) $class->getIdentifier();
+        $fields = $this->_convertFieldToColumnNames($fields, $class);
 
-        $seq = $table->getOption('sequenceName');
+        $seq = $class->getTableOption('sequenceName');
         if ( ! empty($seq)) {
             $id = $this->_conn->sequence->nextId($seq);
-            $seqName = $table->getIdentifier();
+            $seqName = $class->getIdentifier();
             $fields[$seqName] = $id;
             $record->assignIdentifier($id);
         }
 
-        $this->_conn->insert($table, $fields);
+        $this->_conn->insert($class->getTableName(), $fields);
 
-        if (empty($seq) && count($identifier) == 1 && $identifier[0] == $table->getIdentifier() &&
-                $table->getIdentifierType() != Doctrine::IDENTIFIER_NATURAL) {
+        if (empty($seq) && count($identifier) == 1 && $identifier[0] == $class->getIdentifier() &&
+                $class->getIdentifierType() != Doctrine::IDENTIFIER_NATURAL) {
             if (strtolower($this->_conn->getName()) == 'pgsql') {
-                $seq = $table->getTableName() . '_' . $identifier[0];
+                $seq = $class->getTableName() . '_' . $identifier[0];
             }
 
             $id = $this->_conn->sequence->lastInsertId($seq);
@@ -804,6 +805,16 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
         }
     }
     
+    protected function _convertFieldToColumnNames(array $fields, Doctrine_ClassMetadata $class)
+    {
+        $converted = array();
+        foreach ($fields as $fieldName => $value) {
+            $converted[$class->getColumnName($fieldName)] = $value;
+        }
+        
+        return $converted;
+    }
+    
     /**
      * saves the given record
      *
@@ -948,9 +959,9 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
      */
     protected function _doUpdate(Doctrine_Record $record)
     {
-        $identifier = $record->identifier();
-        $array = $record->getPrepared();
-        $this->_conn->update($this->_classMetadata, $array, $identifier);
+        $identifier = $this->_convertFieldToColumnNames($record->identifier(), $this->_classMetadata);
+        $data = $this->_convertFieldToColumnNames($record->getPrepared(), $this->_classMetadata);
+        $this->_conn->update($this->_classMetadata->getTableName(), $data, $identifier);
         $record->assignIdentifier(true);
     }
     
@@ -1043,8 +1054,9 @@ abstract class Doctrine_Mapper_Abstract extends Doctrine_Configurable implements
             $this->_deleteComposites($record);
 
             $record->state(Doctrine_Record::STATE_TDIRTY);
-
-            $conn->delete($this->_classMetadata, $record->identifier());
+            
+            $identifier = $this->_convertFieldToColumnNames($record->identifier(), $this->_classMetadata);
+            $conn->delete($this->_classMetadata->getTableName(), $identifier);
             $record->state(Doctrine_Record::STATE_TCLEAN);
 
             $this->removeRecord($record);

lib/Doctrine/Mapper/Joined.php

@@ -9,34 +9,32 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
      *
      * @param Doctrine_Record $record   record to be inserted
      * @return boolean
-     * @todo Move to Doctrine_Table (which will become Doctrine_Mapper).
      */
     protected function _doInsert(Doctrine_Record $record)
     {
-        $table = $this->_classMetadata;
+        $class = $this->_classMetadata;
                     
         $dataSet = $this->_formatDataSet($record);
-        $component = $table->getClassName();
-
-        $classes = $table->getParentClasses();
+        $component = $class->getClassName();
+        $classes = $class->getParentClasses();
         array_unshift($classes, $component);
         
         try {
             $this->_conn->beginInternalTransaction();
             $identifier = null;
             foreach (array_reverse($classes) as $k => $parent) {
-                $parentTable = $this->_conn->getMetadata($parent);
+                $parentClass = $this->_conn->getClassMetadata($parent);
                 if ($k == 0) {
-                    $identifierType = $parentTable->getIdentifierType();
+                    $identifierType = $parentClass->getIdentifierType();
                     if ($identifierType == Doctrine::IDENTIFIER_AUTOINC) {
-                        $this->_conn->insert($parentTable, $dataSet[$parent]);
+                        $this->_conn->insert($parentClass->getTableName(), $dataSet[$parent]);
                         $identifier = $this->_conn->sequence->lastInsertId();
                     } else if ($identifierType == Doctrine::IDENTIFIER_SEQUENCE) {
-                        $seq = $record->getTable()->getOption('sequenceName');
+                        $seq = $record->getClassMetadata()->getTableOption('sequenceName');
                         if ( ! empty($seq)) {
                             $identifier = $this->_conn->sequence->nextId($seq);
-                            $dataSet[$parent][$parentTable->getIdentifier()] = $identifier;
-                            $this->_conn->insert($parentTable, $dataSet[$parent]);
+                            $dataSet[$parent][$parentClass->getIdentifier()] = $identifier;
+                            $this->_conn->insert($parentClass->getTableName(), $dataSet[$parent]);
                         }
                     } else {
                         throw new Doctrine_Mapper_Exception("Unsupported identifier type '$identifierType'.");
@@ -44,9 +42,9 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
                     $record->assignIdentifier($identifier);
                 } else {
                     foreach ((array) $record->identifier() as $id => $value) {
-                        $dataSet[$parent][$id] = $value;
+                        $dataSet[$parent][$parentClass->getColumnName($id)] = $value;
                     }
-                    $this->_conn->insert($parentTable, $dataSet[$parent]);
+                    $this->_conn->insert($parentClass->getTableName(), $dataSet[$parent]);
                 }
             }
             $this->_conn->commit();
@@ -68,10 +66,10 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
     protected function _doUpdate(Doctrine_Record $record)
     {
         $table = $this->_classMetadata;
-        $identifier = $record->identifier();                     
+        $identifier = $this->_convertFieldToColumnNames($record->identifier(), $this->_classMetadata);
         $dataSet = $this->_formatDataSet($record);
         $component = $table->getClassName();
-        $classes = $table->getOption('parents');
+        $classes = $table->getParentClasses();
         array_unshift($classes, $component);
 
         foreach ($record as $field => $value) {
@@ -84,8 +82,8 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
         }
 
         foreach (array_reverse($classes) as $class) {
-            $parentTable = $this->_conn->getMetadata($class);
-            $this->_conn->update($parentTable, $dataSet[$class], $identifier);
+            $parentTable = $this->_conn->getClassMetadata($class);
+            $this->_conn->update($parentTable->getTableName(), $dataSet[$class], $identifier);
         }
         
         $record->assignIdentifier(true);
@@ -100,18 +98,20 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
     protected function _doDelete(Doctrine_Record $record, Doctrine_Connection $conn)
     {
         try {
-            $table = $this->_classMetadata;
+            $class = $this->_classMetadata;
             $conn->beginInternalTransaction();
             $this->deleteComposites($record);
 
             $record->state(Doctrine_Record::STATE_TDIRTY);
 
-            foreach ($table->getParentClasses() as $parent) {
-                $parentTable = $conn->getClassMetadata($parent);
-                $conn->delete($parentTable, $record->identifier());
+            $identifier = $this->_convertFieldToColumnNames($record->identifier(), $class);
+
+            foreach ($class->getParentClasses() as $parent) {
+                $parentClass = $conn->getClassMetadata($parent);
+                $conn->delete($parentClass->getTableName(), $identifier);
             }
 
-            $conn->delete($table, $record->identifier());
+            $conn->delete($class->getTableName(), $identifier);
             $record->state(Doctrine_Record::STATE_TCLEAN);
 
             $this->removeRecord($record);
@@ -269,7 +269,7 @@ class Doctrine_Mapper_Joined extends Doctrine_Mapper_Abstract
                 if ( ! array_key_exists($fieldName, $array)) {
                     continue;
                 }
-                $dataSet[$class][$fieldName] = $array[$fieldName];
+                $dataSet[$class][$columnName] = $array[$fieldName];
             }
         }
         

tests/lib/Doctrine_OrmTestCase.php

@@ -82,7 +82,7 @@ class Doctrine_OrmTestCase extends Doctrine_TestCase
         }
         
         foreach ($fixture['rows'] as $row) {
-            $conn->insert($classMetadata, $row);
+            $conn->insert($tableName, $row);
         }
     }