Lock exception leaves document with bad revision #1592
Conversation
|
Changes look good, can you create a test for this use-case? Thanks! |
| @@ -426,6 +425,8 @@ public function update($document, array $options = array()) | |||
|
|
|||
| if (($this->class->isVersioned || $this->class->isLockable) && ! $result['n']) { | |||
| throw LockException::lockFailed($document); | |||
| } else if ($this->class->isVersioned && $nextVersion) { | |||
There was a problem hiding this comment.
We tend to keep elseif written together. Also If the class is versioned the $nextVersion must be present so there's no need for checking it here (the latter may not be true by looking only at if statements you've modifed above but that's validated on class metadata level)
|
@mlodick any chance you could write a test for the change you've implemented? |
|
@malarzm sorry, I've been out of town. I'll get a test written today or tomorrow |
|
@mlodick could you add that test? I'd like to get this merged next week so we can release it in 1.1.6, otherwise I'd have to push it back to 1.1.7. |
|
@alcaeus are these tests ok? |
|
@mlodick I'll take a look tonight and update them myself if necessary. Thank you very much! ❤️ |
| $class = DocumentPersisterTestDocumentWithVersion::class; | ||
| $documentPersister = $this->uow->getDocumentPersister($class); | ||
|
|
||
| $collection = $this->createMock('\MongoCollection'); |
There was a problem hiding this comment.
You can use MongoCollection::class
| $testDocument = new $class(); | ||
| $testDocument->id = 12345; | ||
| $this->uow->registerManaged($testDocument, 12345, array('id' => 12345)); | ||
| $this->setExpectedException('\Doctrine\ODM\MongoDB\LockException'); |
| $class = DocumentPersisterTestDocumentWithVersion::class; | ||
| $documentPersister = $this->uow->getDocumentPersister($class); | ||
|
|
||
| $collection = $this->createMock('\MongoCollection'); |
mlodick
force-pushed
the
lock-exception-leaves-document-with-bad-revision
branch
from
8749043
to
588fc3d
Compare
|
@alcaeus I think this is all set then, though I'm not sure how to retrigger the test that failed - seems to have failed getting a pecl package. |
|
@mlodick just restarted the job, let's see how it ends :) |
|
Green it is, thanks @mlodick! |
|
👍 thank you |
A document's local version should only be incremented when the database version is, otherwise, if the lock exception is caught, a second persist & can succeed and overwrite data.