Merge pull request #1192 from malarzm/collection-vs-lock

Mind versioning when updating collections

Author: malarzm

lib/Doctrine/ODM/MongoDB/Persisters/CollectionPersister.php

@@ -20,6 +20,7 @@
 namespace Doctrine\ODM\MongoDB\Persisters;
 
 use Doctrine\ODM\MongoDB\DocumentManager;
+use Doctrine\ODM\MongoDB\LockException;
 use Doctrine\ODM\MongoDB\PersistentCollection;
 use Doctrine\ODM\MongoDB\Persisters\PersistenceBuilder;
 use Doctrine\ODM\MongoDB\UnitOfWork;
@@ -260,15 +261,22 @@ private function getPathAndParent(PersistentCollection $coll)
      * Executes a query updating the given document.
      *
      * @param object $document
-     * @param array $query
+     * @param array $newObj
      * @param array $options
      */
-    private function executeQuery($document, array $query, array $options)
+    private function executeQuery($document, array $newObj, array $options)
     {
         $className = get_class($document);
         $class = $this->dm->getClassMetadata($className);
         $id = $class->getDatabaseIdentifierValue($this->uow->getDocumentIdentifier($document));
+        $query = array('_id' => $id);
+        if ($class->isVersioned) {
+            $query[$class->versionField] = $class->reflFields[$class->versionField]->getValue($document);
+        }
         $collection = $this->dm->getDocumentCollection($className);
-        $collection->update(array('_id' => $id), $query, $options);
+        $result = $collection->update($query, $newObj, $options);
+        if (($class->isVersioned) && ! $result['n']) {
+            throw LockException::lockFailed($document);
+        }
     }
 }

tests/Doctrine/ODM/MongoDB/Tests/Functional/LockTest.php

@@ -2,8 +2,11 @@
 
 namespace Doctrine\ODM\MongoDB\Tests\Functional;
 
+use Doctrine\Common\Collections\ArrayCollection;
+use Doctrine\ODM\MongoDB\LockException;
 use Doctrine\ODM\MongoDB\LockMode;
 use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM;
+use Documents\Issue;
 
 class LockTest extends \Doctrine\ODM\MongoDB\Tests\BaseTest
 {
@@ -365,6 +368,45 @@ public function testInvalidVersionDocument()
         $this->setExpectedException('Doctrine\ODM\MongoDB\MongoDBException', 'Invalid version field type string. Version field must be int or date.');
         $this->dm->getClassMetadata(__NAMESPACE__.'\InvalidVersionDocument');
     }
+
+    /**
+     * @expectedException Doctrine\ODM\MongoDB\LockException
+     */
+    public function testUpdatingCollectionRespectsVersionNumber()
+    {
+        $d = new LockInt('test');
+        $d->issues->add(new Issue('hi', 'ohai'));
+        $this->dm->persist($d);
+        $this->dm->flush();
+
+        // simulate another request updating document in the meantime
+        $this->dm->getDocumentCollection(__NAMESPACE__.'\LockInt')->update(
+            array('_id' => new \MongoId($d->id)),
+            array('$set' => array('version' => 2))
+        );
+
+        $d->issues->add(new Issue('oops', 'version mismatch'));
+        $this->uow->getCollectionPersister()->update($d->issues, array());
+    }
+
+    /**
+     * @expectedException Doctrine\ODM\MongoDB\LockException
+     */
+    public function testDeletingCollectionRespectsVersionNumber()
+    {
+        $d = new LockInt('test');
+        $d->issues->add(new Issue('hi', 'ohai'));
+        $this->dm->persist($d);
+        $this->dm->flush();
+
+        // simulate another request updating document in the meantime
+        $this->dm->getDocumentCollection(__NAMESPACE__.'\LockInt')->update(
+            array('_id' => new \MongoId($d->id)),
+            array('$set' => array('version' => 2))
+        );
+
+        $this->uow->getCollectionPersister()->delete($d->issues, array());
+    }
 }
 
 /** @ODM\MappedSuperclass */
@@ -379,8 +421,14 @@ abstract class AbstractVersionBase
     /** @ODM\Lock @ODM\Int */
     public $locked;
 
+    /**
+     * @ODM\EmbedMany(targetDocument="Documents\Issue")
+     */
+    public $issues;
+
     public function __construct($title = null)
     {
+        $this->issues = new ArrayCollection();
         $this->title = $title;
     }
 
@@ -391,7 +439,7 @@ public function getId()
 
     public function getTitle()
     {
-        return $title;
+        return $this->title;
     }
 
     public function getVersion()