From 2a7cbb6d98675c1ea51b32c3909a31d0aef05ba9 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 30 Apr 2015 16:44:34 -0400 Subject: [PATCH 01/11] Fix documentation for getInsertedId(s) methods on insert results Per PHPLIB-93, these classes should always have an ID, regardless of whether the driver needed to generate one. --- src/InsertManyResult.php | 2 +- src/InsertOneResult.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/InsertManyResult.php b/src/InsertManyResult.php index e7401b541..945da2bf4 100644 --- a/src/InsertManyResult.php +++ b/src/InsertManyResult.php @@ -38,7 +38,7 @@ public function getInsertedCount() } /** - * Return the map of inserted IDs that were generated by the driver. + * Return a map of the inserted documents' IDs. * * The index of each ID in the map corresponds to the document's position * in bulk operation. If the document already an ID prior to insertion (i.e. diff --git a/src/InsertOneResult.php b/src/InsertOneResult.php index fc377e999..416f77401 100644 --- a/src/InsertOneResult.php +++ b/src/InsertOneResult.php @@ -38,7 +38,7 @@ public function getInsertedCount() } /** - * Return the inserted ID that was generated by the driver. + * Return the inserted document's ID. * * If the document already an ID prior to insertion (i.e. the driver did not * need to generate an ID), this will contain its "_id". Any From 5d1686fee072d51dd9a6315504997d34e2cf66e1 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 30 Apr 2015 16:46:13 -0400 Subject: [PATCH 02/11] Note that UpdateResult::getModifiedCount() is undefined for legacy ops --- src/UpdateResult.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/UpdateResult.php b/src/UpdateResult.php index 8df698478..96c4b57b3 100644 --- a/src/UpdateResult.php +++ b/src/UpdateResult.php @@ -38,7 +38,8 @@ public function getMatchedCount() /** * Return the number of documents that were modified. * - * This value is undefined if the write was not acknowledged. + * This value is undefined if the write was not acknowledged or if the write + * executed as a legacy operation instead of write command. * * @see UpdateResult::isAcknowledged() * @return integer From adff7f0ff2cddf85b05005120b5389633c799aee Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:39:14 -0400 Subject: [PATCH 03/11] PHPLIB-92: bulkWrite() updates should use "multi" option --- src/Collection.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index a3cddee23..b810b2255 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -223,7 +223,7 @@ public function bulkWrite(array $bulk, array $options = array()) if (!isset($args[1])) { throw new InvalidArgumentException(sprintf("Missing argument#2 for '%s' (operation#%d)", $opname, $n)); } - $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("limit" => 0)); + $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("multi" => true)); $bulk->update($args[0], $args[1], $options); break; @@ -232,7 +232,7 @@ public function bulkWrite(array $bulk, array $options = array()) if (!isset($args[1])) { throw new InvalidArgumentException(sprintf("Missing argument#2 for '%s' (operation#%d)", $opname, $n)); } - $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("limit" => 1)); + $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("multi" => false)); $firstKey = key($args[1]); if (!isset($firstKey[0]) || $firstKey[0] != '$') { throw new InvalidArgumentException("First key in \$update must be a \$operator"); @@ -245,7 +245,7 @@ public function bulkWrite(array $bulk, array $options = array()) if (!isset($args[1])) { throw new InvalidArgumentException(sprintf("Missing argument#2 for '%s' (operation#%d)", $opname, $n)); } - $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("limit" => 1)); + $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("multi" => false)); $firstKey = key($args[1]); if (isset($firstKey[0]) && $firstKey[0] == '$') { throw new InvalidArgumentException("First key in \$update must NOT be a \$operator"); From d340f1fa8ece383162cced696d01a23cb7d2064a Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:40:40 -0400 Subject: [PATCH 04/11] Fix iteration on operations in bulkWrite() --- src/Collection.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Collection.php b/src/Collection.php index b810b2255..b054187d9 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -198,17 +198,17 @@ function (\stdClass $document) { return (array) $document; }, * * @see Collection::getBulkOptions() for supported $options * - * @param array $bulk Array of operations + * @param array $ops Array of operations * @param array $options Additional options * @return WriteResult */ - public function bulkWrite(array $bulk, array $options = array()) + public function bulkWrite(array $ops, array $options = array()) { $options = array_merge($this->getBulkOptions(), $options); $bulk = new BulkWrite($options["ordered"]); - foreach ($bulk as $n => $op) { + foreach ($ops as $n => $op) { foreach ($op as $opname => $args) { if (!isset($args[0])) { throw new InvalidArgumentException(sprintf("Missing argument#1 for '%s' (operation#%d)", $opname, $n)); From db628d932f17a5441d3c3955aaa73f9c262505c7 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:42:19 -0400 Subject: [PATCH 05/11] Revise InsertManyResult::getInsertedIds() documentation --- src/InsertManyResult.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/InsertManyResult.php b/src/InsertManyResult.php index 945da2bf4..e6c06da8d 100644 --- a/src/InsertManyResult.php +++ b/src/InsertManyResult.php @@ -41,8 +41,8 @@ public function getInsertedCount() * Return a map of the inserted documents' IDs. * * The index of each ID in the map corresponds to the document's position - * in bulk operation. If the document already an ID prior to insertion (i.e. - * the driver did not need to generate an ID), this will contain its "_id". + * in bulk operation. If the document had an ID prior to insertion (i.e. the + * driver did not generate an ID), this will contain its "_id" field value. * Any driver-generated ID will be an MongoDB\Driver\ObjectID instance. * * @return mixed[] From a6b2d856e61fe364f5ce9c60e2968a54e743d330 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:44:24 -0400 Subject: [PATCH 06/11] PHPLIB-102: Implement UpdateResult::getUpsertedCount() --- src/UpdateResult.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/src/UpdateResult.php b/src/UpdateResult.php index 96c4b57b3..179699870 100644 --- a/src/UpdateResult.php +++ b/src/UpdateResult.php @@ -49,6 +49,19 @@ public function getModifiedCount() return $this->writeResult->getModifiedCount(); } + /** + * Return the number of documents that were upserted. + * + * This value is undefined if the write was not acknowledged. + * + * @see UpdateResult::isAcknowledged() + * @return integer + */ + public function getUpsertedCount() + { + return $this->writeResult->getUpsertedCount(); + } + /** * Return the ID of the document inserted by an upsert operation. * From 053fa97ebede4a3c958cce7855320ebea5027aff Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:46:19 -0400 Subject: [PATCH 07/11] UpdateResult::getUpsertedId() may return any value --- src/UpdateResult.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/UpdateResult.php b/src/UpdateResult.php index 179699870..78dd8f014 100644 --- a/src/UpdateResult.php +++ b/src/UpdateResult.php @@ -2,7 +2,6 @@ namespace MongoDB; -use BSON\ObjectId; use MongoDB\Driver\WriteResult; /** @@ -67,7 +66,7 @@ public function getUpsertedCount() * * This value is undefined if an upsert did not take place. * - * @return ObjectId|null + * @return mixed|null */ public function getUpsertedId() { From bfd68613fba93affd7bc6cd15f71b1b84ec0f914 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:48:33 -0400 Subject: [PATCH 08/11] InsertManyResult should require $insertedIds array --- src/InsertManyResult.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/InsertManyResult.php b/src/InsertManyResult.php index e6c06da8d..14c9f41c1 100644 --- a/src/InsertManyResult.php +++ b/src/InsertManyResult.php @@ -18,7 +18,7 @@ class InsertManyResult * @param WriteResult $writeResult * @param mixed[] $insertedIds */ - public function __construct(WriteResult $writeResult, array $insertedIds = array()) + public function __construct(WriteResult $writeResult, array $insertedIds) { $this->writeResult = $writeResult; $this->insertedIds = $insertedIds; From 14971821af6f65fed06a85fe1c15154c867dfe45 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 16:49:08 -0400 Subject: [PATCH 09/11] PHPLIB-94: BulkWriteResult and collect inserted IDs in bulkWrite() --- src/BulkWriteResult.php | 136 ++++++++++++++++++++++++++++++++++++++++ src/Collection.php | 15 ++++- 2 files changed, 149 insertions(+), 2 deletions(-) create mode 100644 src/BulkWriteResult.php diff --git a/src/BulkWriteResult.php b/src/BulkWriteResult.php new file mode 100644 index 000000000..2862a59cd --- /dev/null +++ b/src/BulkWriteResult.php @@ -0,0 +1,136 @@ +writeResult = $writeResult; + $this->insertedIds = $insertedIds; + } + + /** + * Return the number of documents that were deleted. + * + * This value is undefined if the write was not acknowledged. + * + * @see BulkWriteResult::isAcknowledged() + * @return integer + */ + public function getDeletedCount() + { + return $this->writeResult->getDeletedCount(); + } + + /** + * Return the number of documents that were inserted. + * + * This value is undefined if the write was not acknowledged. + * + * @see BulkWriteResult::isAcknowledged() + * @return integer + */ + public function getInsertedCount() + { + return $this->writeResult->getInsertedCount(); + } + + /** + * Return a map of the inserted documents' IDs. + * + * The index of each ID in the map corresponds to the document's position + * in bulk operation. If the document had an ID prior to insertion (i.e. the + * driver did not generate an ID), this will contain its "_id" field value. + * Any driver-generated ID will be an MongoDB\Driver\ObjectID instance. + * + * @return mixed[] + */ + public function getInsertedIds() + { + return $this->insertedIds; + } + + /** + * Return the number of documents that were matched by the filter. + * + * This value is undefined if the write was not acknowledged. + * + * @see BulkWriteResult::isAcknowledged() + * @return integer + */ + public function getMatchedCount() + { + return $this->writeResult->getMatchedCount(); + } + + /** + * Return the number of documents that were modified. + * + * This value is undefined if the write was not acknowledged or if the write + * executed as a legacy operation instead of write command. + * + * @see BulkWriteResult::isAcknowledged() + * @return integer|null + */ + public function getModifiedCount() + { + return $this->writeResult->getModifiedCount(); + } + + /** + * Return the number of documents that were upserted. + * + * This value is undefined if the write was not acknowledged. + * + * @see BulkWriteResult::isAcknowledged() + * @return integer + */ + public function getUpsertedCount() + { + return $this->writeResult->getUpsertedCount(); + } + + /** + * Return a map of the upserted documents' IDs. + * + * The index of each ID in the map corresponds to the document's position + * in bulk operation. If the document had an ID prior to upserting (i.e. the + * server did not need to generate an ID), this will contain its "_id". Any + * server-generated ID will be an MongoDB\Driver\ObjectID instance. + * + * @return mixed[] + */ + public function getUpsertedIds() + { + return $this->writeResult->getUpsertedIds(); + } + + /** + * Return whether this update was acknowledged by the server. + * + * If the update was not acknowledged, other fields from the WriteResult + * (e.g. matchedCount) will be undefined. + * + * @return boolean + */ + public function isAcknowledged() + { + return $this->writeResult->isAcknowledged(); + } +} diff --git a/src/Collection.php b/src/Collection.php index b054187d9..c7d026da5 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -207,6 +207,7 @@ public function bulkWrite(array $ops, array $options = array()) $options = array_merge($this->getBulkOptions(), $options); $bulk = new BulkWrite($options["ordered"]); + $insertedIds = array(); foreach ($ops as $n => $op) { foreach ($op as $opname => $args) { @@ -216,7 +217,14 @@ public function bulkWrite(array $ops, array $options = array()) switch ($opname) { case "insertOne": - $bulk->insert($args[0]); + $insertedId = $bulk->insert($args[0]); + + if ($insertedId !== null) { + $insertedIds[$n] = $insertedId; + } else { + $insertedIds[$n] = is_array($args[0]) ? $args[0]['_id'] : $args[0]->_id; + } + break; case "updateMany": @@ -269,7 +277,10 @@ public function bulkWrite(array $ops, array $options = array()) } } } - return $this->manager->executeBulkWrite($this->ns, $bulk, $this->wc); + + $writeResult = $this->manager->executeBulkWrite($this->ns, $bulk, $this->wc); + + return new BulkWriteResult($writeResult, $insertedIds); } /** From aef7adb33f91bd8f5daebea3854603b1c401d08d Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 17:45:16 -0400 Subject: [PATCH 10/11] PHPLIB-103: bulkWrite() updateMany should require update operators --- src/Collection.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Collection.php b/src/Collection.php index c7d026da5..8e079bae1 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -232,6 +232,10 @@ public function bulkWrite(array $ops, array $options = array()) throw new InvalidArgumentException(sprintf("Missing argument#2 for '%s' (operation#%d)", $opname, $n)); } $options = array_merge($this->getWriteOptions(), isset($args[2]) ? $args[2] : array(), array("multi" => true)); + $firstKey = key($args[1]); + if (!isset($firstKey[0]) || $firstKey[0] != '$') { + throw new InvalidArgumentException("First key in \$update must be a \$operator"); + } $bulk->update($args[0], $args[1], $options); break; From 39376f3514190de7397693ce6896ceb684b0f8b9 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Sun, 3 May 2015 17:46:20 -0400 Subject: [PATCH 11/11] PHPLIB-94: Functional tests for bulkWrite() and BulkWriteResult --- tests/Collection/BulkWriteFunctionalTest.php | 218 +++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 tests/Collection/BulkWriteFunctionalTest.php diff --git a/tests/Collection/BulkWriteFunctionalTest.php b/tests/Collection/BulkWriteFunctionalTest.php new file mode 100644 index 000000000..73a913732 --- /dev/null +++ b/tests/Collection/BulkWriteFunctionalTest.php @@ -0,0 +1,218 @@ +omitModifiedCount = version_compare($this->getServerVersion(), '2.6.0', '<'); + } + + public function testInserts() + { + $ops = array( + array('insertOne' => array(array('_id' => 1, 'x' => 11))), + array('insertOne' => array(array('x' => 22))), + ); + + $result = $this->collection->bulkWrite($ops); + $this->assertInstanceOf('MongoDB\BulkWriteResult', $result); + $this->assertSame(2, $result->getInsertedCount()); + + $insertedIds = $result->getInsertedIds(); + $this->assertSame(1, $insertedIds[0]); + $this->assertInstanceOf('BSON\ObjectId', $insertedIds[1]); + + $expected = array( + array('_id' => $insertedIds[0], 'x' => 11), + array('_id' => $insertedIds[1], 'x' => 22), + ); + + $this->assertEquals($expected, $this->collection->find()->toArray()); + } + + public function testUpdates() + { + $this->createFixtures(4); + + $ops = array( + array('updateOne' => array(array('_id' => 2), array('$inc' => array('x' => 1)))), + array('updateMany' => array(array('_id' => array('$gt' => 2)), array('$inc' => array('x' => -1)))), + array('updateOne' => array(array('_id' => 5), array('$set' => array('x' => 55)), array('upsert' => true))), + array('updateOne' => array(array('x' => 66), array('$set' => array('x' => 66)), array('upsert' => true))), + array('updateMany' => array(array('x' => array('$gt' => 50)), array('$inc' => array('x' => 1)))), + ); + + $result = $this->collection->bulkWrite($ops); + $this->assertInstanceOf('MongoDB\BulkWriteResult', $result); + $this->assertSame(5, $result->getMatchedCount()); + $this->omitModifiedCount or $this->assertSame(5, $result->getModifiedCount()); + $this->assertSame(2, $result->getUpsertedCount()); + + $upsertedIds = $result->getUpsertedIds(); + $this->assertSame(5, $upsertedIds[2]); + $this->assertInstanceOf('BSON\ObjectId', $upsertedIds[3]); + + $expected = array( + array('_id' => 1, 'x' => 11), + array('_id' => 2, 'x' => 23), + array('_id' => 3, 'x' => 32), + array('_id' => 4, 'x' => 43), + array('_id' => 5, 'x' => 56), + array('_id' => $upsertedIds[3], 'x' => 67), + ); + + $this->assertEquals($expected, $this->collection->find()->toArray()); + } + + public function testDeletes() + { + $this->createFixtures(4); + + $ops = array( + array('deleteOne' => array(array('_id' => 1))), + array('deleteMany' => array(array('_id' => array('$gt' => 2)))), + ); + + $result = $this->collection->bulkWrite($ops); + $this->assertInstanceOf('MongoDB\BulkWriteResult', $result); + $this->assertSame(3, $result->getDeletedCount()); + + $expected = array( + array('_id' => 2, 'x' => 22), + ); + + $this->assertEquals($expected, $this->collection->find()->toArray()); + } + + public function testMixedOrderedOperations() + { + $this->createFixtures(3); + + $ops = array( + array('updateOne' => array(array('_id' => array('$gt' => 1)), array('$inc' => array('x' => 1)))), + array('updateMany' => array(array('_id' => array('$gt' => 1)), array('$inc' => array('x' => 1)))), + array('insertOne' => array(array('_id' => 4, 'x' => 44))), + array('deleteMany' => array(array('x' => array('$nin' => array(24, 34))))), + array('replaceOne' => array(array('_id' => 4), array('_id' => 4, 'x' => 44), array('upsert' => true))), + ); + + $result = $this->collection->bulkWrite($ops); + $this->assertInstanceOf('MongoDB\BulkWriteResult', $result); + + $this->assertSame(1, $result->getInsertedCount()); + $this->assertSame(array(2 => 4), $result->getInsertedIds()); + + $this->assertSame(3, $result->getMatchedCount()); + $this->omitModifiedCount or $this->assertSame(3, $result->getModifiedCount()); + $this->assertSame(1, $result->getUpsertedCount()); + $this->assertSame(array(4 => 4), $result->getUpsertedIds()); + + $this->assertSame(2, $result->getDeletedCount()); + + $expected = array( + array('_id' => 2, 'x' => 24), + array('_id' => 3, 'x' => 34), + array('_id' => 4, 'x' => 44), + ); + + $this->assertEquals($expected, $this->collection->find()->toArray()); + } + + /** + * @expectedException MongoDB\Exception\InvalidArgumentException + * @expectedExceptionMessage Unknown operation type called 'foo' (operation#0) + */ + public function testUnknownOperation() + { + $this->collection->bulkWrite(array( + array('foo' => array(array('_id' => 1))), + )); + } + + /** + * @expectedException MongoDB\Exception\InvalidArgumentException + * @expectedExceptionMessageRegExp /Missing argument#\d+ for '\w+' \(operation#\d+\)/ + * @dataProvider provideOpsWithMissingArguments + */ + public function testMissingArguments(array $ops) + { + $this->collection->bulkWrite($ops); + } + + public function provideOpsWithMissingArguments() + { + return array( + array(array(array('insertOne' => array()))), + array(array(array('updateOne' => array()))), + array(array(array('updateOne' => array(array('_id' => 1))))), + array(array(array('updateMany' => array()))), + array(array(array('updateMany' => array(array('_id' => 1))))), + array(array(array('replaceOne' => array()))), + array(array(array('replaceOne' => array(array('_id' => 1))))), + array(array(array('deleteOne' => array()))), + array(array(array('deleteMany' => array()))), + ); + } + + /** + * @expectedException MongoDB\Exception\InvalidArgumentException + * @expectedExceptionMessage First key in $update must be a $operator + */ + public function testUpdateOneRequiresUpdateOperators() + { + $this->collection->bulkWrite(array( + array('updateOne' => array(array('_id' => 1), array('x' => 1))), + )); + } + + /** + * @expectedException MongoDB\Exception\InvalidArgumentException + * @expectedExceptionMessage First key in $update must be a $operator + */ + public function testUpdateManyRequiresUpdateOperators() + { + $this->collection->bulkWrite(array( + array('updateMany' => array(array('_id' => array('$gt' => 1)), array('x' => 1))), + )); + } + + /** + * @expectedException MongoDB\Exception\InvalidArgumentException + * @expectedExceptionMessage First key in $update must NOT be a $operator + */ + public function testReplaceOneRequiresReplacementDocument() + { + $this->collection->bulkWrite(array( + array('replaceOne' => array(array('_id' => 1), array('$inc' => array('x' => 1)))), + )); + } + + /** + * Create data fixtures. + * + * @param integer $n + */ + private function createFixtures($n) + { + $bulkWrite = new BulkWrite(true); + + for ($i = 1; $i <= $n; $i++) { + $bulkWrite->insert(array( + '_id' => $i, + 'x' => (integer) ($i . $i), + )); + } + + $result = $this->manager->executeBulkWrite($this->getNamespace(), $bulkWrite); + + $this->assertEquals($n, $result->getInsertedCount()); + } +} \ No newline at end of file