Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Use replacement document for upserts with no modifiers (#741) #750

Merged
merged 2 commits into from

1 participant

@jmikola
Owner

See #741 and SERVER-12266.

...Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
((10 lines not shown))
}
+
+ /* If there are no modifiers remaining, we're inserting a document with
+ * an identifier as its only field. Use the criteria as the replacement
+ * document.
+ */
+ if (empty($data)) {
+ $data = $criteria;
@jmikola Owner
jmikola added a note

Fixing the attached test case so that we never inadvertently replace an existing document could be done by adding {$where: 'false'} to the criteria (forcing an insert) or more simply falling back to an insert operation. In either case, users with perfectly legitimate use cases might see a uniqueness exception from MongoDB.

An example of "perfectly legitimate" is in the test case. The collection happens to contain a document with an _id XYZ and some additional fields. The user creates a new document with nothing more than the _id XYZ and flushes it. Maybe they have @NotSaved and other migration mappings for those original, additional fields. They shouldn't expect the persist operation to clobber the existing document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@jmikola
Owner

Tested on 1.8.5, 2.0.6, 2.2.6, 2.4.9, and master branch (2.5.6-pre-). For earlier versions of MongoDB (before 2.6), we rely on an assertion in update_internal.cpp which yields error code 10148 and message "Mod on _id not allowed". I opted to check for the message string instead of the error code on the off chance that a future version of the driver decides to no longer use the original server error code as the exception's code. I imagine the original message has a higher chance of always sticking around in the exception message somewhere.

@jmikola jmikola merged commit 6397899 into master
@jmikola jmikola deleted the mongodb26-upsert branch
@jmikola jmikola referenced this pull request
Closed

MongoDB 2.6 compatibility and test failures #741

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
35 lib/Doctrine/ODM/MongoDB/Persisters/DocumentPersister.php
@@ -289,11 +289,40 @@ private function executeUpsert(array $data, array $options)
$options['upsert'] = true;
$criteria = array('_id' => $data['$set']['_id']);
unset($data['$set']['_id']);
- // stupid php
+
+ // Do not send an empty $set modifier
if (empty($data['$set'])) {
- $data['$set'] = new \stdClass;
+ unset($data['$set']);
+ }
+
+ /* If there are no modifiers remaining, we're upserting a document with
+ * an identifier as its only field. Since a document with the identifier
+ * may already exist, the desired behavior is "insert if not exists" and
+ * NOOP otherwise. MongoDB 2.6+ does not allow empty modifiers, so $set
+ * the identifier to the same value in our criteria.
+ *
+ * This will fail for versions before MongoDB 2.6, which require an
+ * empty $set modifier. The best we can do (without attempting to check
+ * server versions in advance) is attempt the 2.6+ behavior and retry
+ * after the relevant exception.
+ *
+ * See: https://jira.mongodb.org/browse/SERVER-12266
+ */
+ if (empty($data)) {
+ $retry = true;
+ $data = array('$set' => array('_id' => $criteria['_id']));
}
- $this->collection->update($criteria, $data, $options);
+
+ try {
+ $this->collection->update($criteria, $data, $options);
+ return;
+ } catch (\MongoCursorException $e) {
+ if (empty($retry) || strpos($e->getMessage(), 'Mod on _id not allowed') === false) {
+ throw $e;
+ }
+ }
+
+ $this->collection->update($criteria, array('$set' => new \stdClass), $options);
}
/**
View
15 tests/Doctrine/ODM/MongoDB/Tests/Functional/DocumentPersisterTest.php
@@ -26,6 +26,21 @@ public function setUp()
$this->documentPersister = $this->uow->getDocumentPersister($this->class);
}
+ public function testExecuteUpsertShouldNeverReplaceDocuments()
+ {
+ $originalData = $this->dm->getDocumentCollection($this->class)->findOne();
+
+ $document = new DocumentPersisterTestDocument();
+ $document->id = $originalData['_id'];
+
+ $this->dm->persist($document);
+ $this->dm->flush();
+
+ $updatedData = $this->dm->getDocumentCollection($this->class)->findOne(array('_id' => $originalData['_id']));
+
+ $this->assertEquals($originalData, $updatedData);
+ }
+
public function testLoadPreparesCriteriaAndSort()
{
$criteria = array('name' => array('$in' => array('a', 'b')));
Something went wrong with that request. Please try again.