[2.0] A few fixes and clean up to the Optimistic Locking implementation.

lib/Doctrine/ORM/Persisters/JoinedSubclassPersister.php

@@ -58,6 +58,26 @@ class JoinedSubclassPersister extends StandardEntityPersister
         }
     }
 
+    /**
+     * This function finds the ClassMetadata instance in a inheritance hierarchy
+     * that is responsible for enabling versioning.
+     *
+     * @return mixed $versionedClass  ClassMetadata instance or false if versioning is not enabled
+     */
+    private function _getVersionedClassMetadata()
+    {
+        if ($isVersioned = $this->_class->isVersioned) {
+            if (isset($this->_class->fieldMappings[$this->_class->versionField]['inherited'])) {
+                $definingClassName = $this->_class->fieldMappings[$this->_class->versionField]['inherited'];
+                $versionedClass = $this->_em->getClassMetadata($definingClassName);
+            } else {
+                $versionedClass = $this->_class;
+            }
+            return $versionedClass;
+        }
+        return false;
+    }
+
     /**
      * {@inheritdoc}
      *
@@ -95,12 +115,7 @@ class JoinedSubclassPersister extends StandardEntityPersister
         }
 
         if ($isVersioned = $this->_class->isVersioned) {
-            if (isset($this->_class->fieldMappings[$this->_class->versionField]['inherited'])) {
+            $versionedClass = $this->_getVersionedClassMetadata();
-                $definingClassName = $this->_class->fieldMappings[$this->_class->versionField]['inherited'];
-                $versionedClass = $this->_em->getClassMetadata($definingClassName);
-            } else {
-                $versionedClass = $this->_class;
-            }
         }
 
         $postInsertIds = array();
@@ -204,8 +219,22 @@ class JoinedSubclassPersister extends StandardEntityPersister
             $this->_em->getUnitOfWork()->getEntityIdentifier($entity)
         );
 
-        foreach ($updateData as $tableName => $data) {
+        if ($isVersioned = $this->_class->isVersioned) {
-            $this->_doUpdate($entity, $tableName, $updateData[$tableName], $id);
+            $versionedClass = $this->_getVersionedClassMetadata();
+            $versionedTable = $versionedClass->primaryTable['name'];
+        }
+
+        if ($updateData) {
+            foreach ($updateData as $tableName => $data) {
+                if ($isVersioned && $versionedTable == $tableName) {
+                    $this->_doUpdate($entity, $tableName, $data, $id);
+                } else {
+                    $this->_conn->update($tableName, $data, $id);
+                }
+            }
+            if ($isVersioned && ! isset($updateData[$versionedTable])) {
+                $this->_doUpdate($entity, $versionedTable, array(), $id);
+            }
         }
     }
 

lib/Doctrine/ORM/Persisters/StandardEntityPersister.php

@@ -182,6 +182,13 @@ class StandardEntityPersister
         return $postInsertIds;
     }
 
+    /**
+     * This function retrieves the default version value which was created
+     * by the DBMS INSERT statement. The value is assigned back in to the 
+     * $entity versionField property.
+     *
+     * @return void
+     */
     protected function _assignDefaultVersionValue($class, $entity, $id)
     {
         $versionField = $this->_class->getVersionField();
@@ -222,6 +229,16 @@ class StandardEntityPersister
         }
     }
 
+    /**
+     * Perform UPDATE statement for an entity. This function has support for
+     * optimistic locking if the entities ClassMetadata has versioning enabled.
+     *
+     * @param object $entity        The entity object being updated
+     * @param string $tableName     The name of the table being updated
+     * @param array $data           The array of data to set
+     * @param array $where          The condition used to update
+     * @return void
+     */
     protected function _doUpdate($entity, $tableName, $data, $where)
     {
         $set = array();
@@ -229,7 +246,7 @@ class StandardEntityPersister
             $set[] = $this->_conn->quoteIdentifier($columnName) . ' = ?';
         }
 
-        if ($versioned = $this->_class->isVersioned()) {
+        if ($isVersioned = $this->_class->isVersioned()) {
             $versionField = $this->_class->getVersionField();
             $identifier = $this->_class->getIdentifier();
             $versionFieldColumnName = $this->_class->getColumnName($versionField);
@@ -237,6 +254,7 @@ class StandardEntityPersister
             $set[] = $this->_conn->quoteIdentifier($versionFieldColumnName) . ' = ' .
                      $this->_conn->quoteIdentifier($versionFieldColumnName) . ' + 1';
         }
+
         $params = array_merge(array_values($data), array_values($where));
 
         $sql  = 'UPDATE ' . $this->_conn->quoteIdentifier($tableName)
@@ -246,7 +264,7 @@ class StandardEntityPersister
 
         $result = $this->_conn->exec($sql, $params);
 
-        if ($versioned && ! $result) {
+        if ($isVersioned && ! $result) {
             throw \Doctrine\ORM\OptimisticLockException::optimisticLockFailed();
         }
     }

tests/Doctrine/Tests/ORM/Locking/OptimisticTest.php

@@ -34,10 +34,11 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase
         $this->_conn = $this->_em->getConnection();
     }
 
-    public function testJoinedInsertSetsInitialVersionValue()
+    public function testJoinedChildInsertSetsInitialVersionValue()
     {
-        $test = new OptimisticJoinedParent();
+        $test = new OptimisticJoinedChild();
-        $test->name = 'test';
+        $test->name = 'child';
+        $test->whatever = 'whatever';
         $this->_em->save($test);
         $this->_em->flush();
 
@@ -47,10 +48,39 @@ class OptimisticTest extends \Doctrine\Tests\OrmFunctionalTestCase
     /**
      * @expectedException Doctrine\ORM\OptimisticLockException
      */
-    public function testJoinedFailureThrowsException()
+    public function testJoinedChildFailureThrowsException()
+    {
+        $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Locking\OptimisticJoinedChild t WHERE t.name = :name');
+        $q->setParameter('name', 'child');
+        $test = $q->getSingleResult();
+
+        // Manually update/increment the version so we can try and save the same
+        // $test and make sure the exception is thrown saying the record was 
+        // changed or updated since you read it
+        $this->_conn->execute('UPDATE optimistic_joined_parent SET version = ? WHERE id = ?', array(2, $test->id));
+
+        // Now lets change a property and try and save it again
+        $test->whatever = 'ok';
+        $this->_em->flush();
+    }
+
+    public function testJoinedParentInsertSetsInitialVersionValue()
+    {
+        $test = new OptimisticJoinedParent();
+        $test->name = 'parent';
+        $this->_em->save($test);
+        $this->_em->flush();
+
+        $this->assertEquals(1, $test->version);
+    }
+
+    /**
+     * @expectedException Doctrine\ORM\OptimisticLockException
+     */
+    public function testJoinedParentFailureThrowsException()
     {
         $q = $this->_em->createQuery('SELECT t FROM Doctrine\Tests\ORM\Locking\OptimisticJoinedParent t WHERE t.name = :name');
-        $q->setParameter('name', 'test');
+        $q->setParameter('name', 'parent');
         $test = $q->getSingleResult();
 
         // Manually update/increment the version so we can try and save the same
@@ -130,7 +160,7 @@ class OptimisticJoinedChild extends OptimisticJoinedParent
     /**
      * @Column(type="string", length=255)
      */
-    public $name;
+    public $whatever;
 }
 
 /**