Pass parameters to write methods by reference

README.md

@@ -74,6 +74,12 @@ root:
  - The [createIndex](https://secure.php.net/manual/en/mongocollection.createindex.php)
  method does not yet return the same result as the original method. Instead, it
  always returns the name of the index created.
+ - The [insert](https://php.net/manual/en/mongocollection.insert.php),
+ [batchInsert](https://php.net/manual/en/mongocollection.batchinsert.php),
+ and [save](https://php.net/manual/en/mongocollection.save.php)
+ methods take the first argument by reference. While the original API does not
+ explicitely specify by-reference arguments it does add an ID field to the
+ objects and documents given.
  - The [parallelCollectionScan](https://php.net/manual/en/mongocollection.parallelcollectionscan.php)
  method is not yet implemented.
 

lib/Mongo/MongoCollection.php

@@ -254,8 +254,13 @@ class MongoCollection
      * @throws MongoCursorTimeoutException if the "w" option is set to a value greater than one and the operation takes longer than MongoCursor::$timeout milliseconds to complete. This does not kill the operation on the server, it is a client-side timeout. The operation in MongoCollection::$wtimeout is milliseconds.
      * @return bool|array Returns an array containing the status of the insertion if the "w" option is set.
      */
-    public function insert($a, array $options = [])
+    public function insert(&$a, array $options = [])
     {
+        if (! $this->ensureDocumentHasMongoId($a)) {
+            trigger_error(sprintf('%s expects parameter %d to be an array or object, %s given', __METHOD__, 1, gettype($a)), E_WARNING);
+            return;
+        }
+
         $result = $this->collection->insertOne(
             TypeConverter::fromLegacy($a),
             $this->convertWriteConcernOptions($options)
@@ -282,8 +287,12 @@ class MongoCollection
      * @throws MongoCursorException
      * @return mixed If "safe" is set, returns an associative array with the status of the inserts ("ok") and any error that may have occured ("err"). Otherwise, returns TRUE if the batch insert was successfully sent, FALSE otherwise.
      */
-    public function batchInsert(array $a, array $options = [])
+    public function batchInsert(array &$a, array $options = [])
     {
+        foreach ($a as $key => $item) {
+            $this->ensureDocumentHasMongoId($a[$key]);
+        }
+
         $result = $this->collection->insertMany(
             TypeConverter::fromLegacy(array_values($a)),
             $this->convertWriteConcernOptions($options)
@@ -576,21 +585,16 @@ class MongoCollection
      * @return array|boolean If w was set, returns an array containing the status of the save.
      * Otherwise, returns a boolean representing if the array was not empty (an empty array will not be inserted).
      */
-    public function save($a, array $options = [])
+    public function save(&$a, array $options = [])
     {
-        if (is_object($a)) {
-            $a = (array) $a;
-        }
+        $id = $this->ensureDocumentHasMongoId($a);
+
+        $document = (array) $a;
+        unset($document['_id']);
 
-        if ( ! array_key_exists('_id', $a)) {
-            $id = new \MongoId();
-        } else {
-            $id = $a['_id'];
-            unset($a['_id']);
-        }
         $options['upsert'] = true;
 
-        return $this->update(['_id' => $id], ['$set' => $a], $options);
+        return $this->update(['_id' => $id], ['$set' => $document], $options);
     }
 
     /**
@@ -745,5 +749,28 @@ class MongoCollection
 
         return $options;
     }
+
+    /**
+     * @param array|object $document
+     * @return MongoId
+     */
+    private function ensureDocumentHasMongoId(&$document)
+    {
+        if (is_array($document)) {
+            if (! isset($document['_id'])) {
+                $document['_id'] = new \MongoId();
+            }
+
+            return $document['_id'];
+        } elseif (is_object($document)) {
+            if (! isset($document->_id)) {
+                $document->_id = new \MongoId();
+            }
+
+            return $document->_id;
+        }
+
+        return null;
+    }
 }
 

tests/Alcaeus/MongoDbAdapter/MongoClientTest.php

@@ -82,7 +82,8 @@ class MongoClientTest extends TestCase
 
     public function testListDBs()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         $databases = $this->getClient()->listDBs();
 
         $this->assertSame(1.0, $databases['ok']);

tests/Alcaeus/MongoDbAdapter/MongoCollectionTest.php

@@ -16,7 +16,6 @@ class MongoCollectionTest extends TestCase
 
     public function testCreateRecord()
     {
-        $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
         $expected = [
@@ -25,7 +24,11 @@ class MongoCollectionTest extends TestCase
             'err' => null,
             'errmsg' => null,
         ];
-        $this->assertSame($expected, $collection->insert(['_id' => new \MongoId($id), 'foo' => 'bar']));
+        $document = ['foo' => 'bar'];
+        $this->assertSame($expected, $collection->insert($document));
+
+        $this->assertInstanceOf('MongoId', $document['_id']);
+        $id = (string) $document['_id'];
 
         $newCollection = $this->getCheckDatabase()->selectCollection('test');
         $this->assertSame(1, $newCollection->count());
@@ -40,7 +43,8 @@ class MongoCollectionTest extends TestCase
 
     public function testUnacknowledgedWrite()
     {
-        $this->assertTrue($this->getCollection()->insert(['foo' => 'bar'], ['w' => 0]));
+        $document = ['foo' => 'bar'];
+        $this->assertTrue($this->getCollection()->insert($document, ['w' => 0]));
     }
 
     public function testInsertMany()
@@ -59,6 +63,10 @@ class MongoCollectionTest extends TestCase
             ['bar' => 'foo']
         ];
         $this->assertSame($expected, $this->getCollection()->batchInsert($documents));
+
+        foreach ($documents as $document) {
+            $this->assertInstanceOf('MongoId', $document['_id']);
+        }
     }
 
     public function testInsertManyWithNonNumericKeys()
@@ -81,8 +89,13 @@ class MongoCollectionTest extends TestCase
 
     public function testUpdateOne()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
+
+        // Unset ID to re-insert
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
+
         $expected = [
             'ok' => 1.0,
             'nModified' => 1,
@@ -100,9 +113,14 @@ class MongoCollectionTest extends TestCase
 
     public function testUpdateMany()
     {
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'foo']);
+        $document = ['change' => true, 'foo' => 'bar'];
+        $this->getCollection()->insert($document);
+
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
+
+        $document = ['change' => true, 'foo' => 'foo'];
+        $this->getCollection()->insert($document);
         $expected = [
             'ok' => 1.0,
             'nModified' => 2,
@@ -120,17 +138,24 @@ class MongoCollectionTest extends TestCase
 
     public function testUnacknowledgedUpdate()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
 
-        $this->assertTrue($this->getCollection()->update(['foo' => 'bar'], ['$set' => ['foo' => 'foo']], ['w' => 0]));
+        $this->assertTrue($this->getCollection()->update($document, ['$set' => ['foo' => 'foo']], ['w' => 0]));
     }
 
     public function testRemoveMultiple()
     {
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'foo']);
+        $document = ['change' => true, 'foo' => 'bar'];
+        $this->getCollection()->insert($document);
+
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
+
+        $document = ['change' => true, 'foo' => 'foo'];
+        $this->getCollection()->insert($document);
         $expected = [
             'ok' => 1.0,
             'n' => 2,
@@ -146,9 +171,12 @@ class MongoCollectionTest extends TestCase
 
     public function testRemoveSingle()
     {
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'foo']);
+        $document = ['change' => true, 'foo' => 'bar'];
+        $this->getCollection()->insert($document);
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
         $expected = [
             'ok' => 1.0,
             'n' => 1,
@@ -164,9 +192,12 @@ class MongoCollectionTest extends TestCase
 
     public function testRemoveUnacknowledged()
     {
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'bar']);
-        $this->getCollection()->insert(['change' => true, 'foo' => 'foo']);
+        $document = ['change' => true, 'foo' => 'bar'];
+        $this->getCollection()->insert($document);
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
+        unset($document['_id']);
+        $this->getCollection()->insert($document);
 
         $this->assertTrue($this->getCollection()->remove(['foo' => 'bar'], ['w' => 0]));
     }
@@ -221,9 +252,7 @@ class MongoCollectionTest extends TestCase
     {
         $collection = $this->getCollection();
 
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'foo']);
+        $this->prepareData();
 
         $pipeline = [
             [
@@ -251,9 +280,7 @@ class MongoCollectionTest extends TestCase
     {
         $collection = $this->getCollection();
 
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'foo']);
+        $this->prepareData();
 
         $pipeline = [
             [
@@ -342,10 +369,13 @@ class MongoCollectionTest extends TestCase
 
     public function testSaveInsert()
     {
-        $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->save(['_id' => new \MongoId($id), 'foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->save($document);
+        $this->assertInstanceOf('MongoId', $document['_id']);
+        $id = (string) $document['_id'];
+
         $newCollection = $this->getCheckDatabase()->selectCollection('test');
         $this->assertSame(1, $newCollection->count());
         $object = $newCollection->findOne();
@@ -362,7 +392,8 @@ class MongoCollectionTest extends TestCase
         $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->insert(['_id' => new \MongoId($id), 'foo' => 'bar']);
+        $document = ['_id' => new \MongoId($id), 'foo' => 'bar'];
+        $collection->insert($document);
         $collection->remove(['_id' => new \MongoId($id)]);
 
         $newCollection = $this->getCheckDatabase()->selectCollection('test');
@@ -374,8 +405,10 @@ class MongoCollectionTest extends TestCase
         $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->insert(['_id' => new \MongoId($id), 'foo' => 'bar']);
-        $collection->save(['_id' => new \MongoId($id), 'foo' => 'foo']);
+        $insertDocument = ['_id' => new \MongoId($id), 'foo' => 'bar'];
+        $saveDocument = ['_id' => new \MongoId($id), 'foo' => 'foo'];
+        $collection->insert($insertDocument);
+        $collection->save($saveDocument);
 
         $newCollection = $this->getCheckDatabase()->selectCollection('test');
         $this->assertSame(1, $newCollection->count());
@@ -392,13 +425,14 @@ class MongoCollectionTest extends TestCase
     {
         $collection = $this->getCollection();
 
-        $collection->insert(['_id' => 1, 'foo' => 'bar']);
+        $insertDocument = ['_id' => 1, 'foo' => 'bar'];
+        $collection->insert($insertDocument);
 
         $document = $collection->getDBRef([
             '$ref' => 'test',
             '$id' => 1,
         ]);
-        $this->assertEquals(['_id' => 1, 'foo' => 'bar'], $document);
+        $this->assertEquals($insertDocument, $document);
     }
 
     public function testCreateDBRef()
@@ -516,7 +550,8 @@ class MongoCollectionTest extends TestCase
         $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->insert(['_id' => new \MongoId($id), 'foo' => 'bar']);
+        $document = ['_id' => new \MongoId($id), 'foo' => 'bar'];
+        $collection->insert($document);
         $document = $collection->findAndModify(
             ['_id' => new \MongoId($id)],
             ['$set' => ['foo' => 'foo']]
@@ -536,7 +571,8 @@ class MongoCollectionTest extends TestCase
         $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->insert(['_id' => new \MongoId($id), 'foo' => 'bar']);
+        $document = ['_id' => new \MongoId($id), 'foo' => 'bar'];
+        $collection->insert($document);
         $document = $collection->findAndModify(
             ['_id' => new \MongoId($id)],
             ['$set' => ['foo' => 'foo']],
@@ -551,11 +587,12 @@ class MongoCollectionTest extends TestCase
         $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->insert([
+        $document = [
             '_id' => new \MongoId($id),
             'foo' => 'bar',
             'bar' => 'foo',
-        ]);
+        ];
+        $collection->insert($document);
         $document = $collection->findAndModify(
             ['_id' => new \MongoId($id)],
             ['$set' => ['foo' => 'foo']],
@@ -569,9 +606,12 @@ class MongoCollectionTest extends TestCase
     {
         $collection = $this->getCollection();
 
-        $collection->insert(['a' => 2]);
-        $collection->insert(['b' => 5]);
-        $collection->insert(['a' => 1]);
+        $document1 = ['a' => 2];
+        $collection->insert($document1);
+        $document2 = ['b' => 5];
+        $collection->insert($document2);
+        $document3 = ['a' => 1];
+        $collection->insert($document3);
         $keys = [];
         $initial = ["count" => 0];
         $reduce = "function (obj, prev) { prev.count++; }";
@@ -595,7 +635,8 @@ class MongoCollectionTest extends TestCase
         $id = '54203e08d51d4a1f868b456e';
         $collection = $this->getCollection();
 
-        $collection->insert(['_id' => new \MongoId($id), 'foo' => 'bar']);
+        $document = ['_id' => new \MongoId($id), 'foo' => 'bar'];
+        $collection->insert($document);
         $document = $collection->findAndModify(
             ['_id' => new \MongoId($id)],
             null,
@@ -612,7 +653,8 @@ class MongoCollectionTest extends TestCase
     public function testValidate()
     {
         $collection = $this->getCollection();
-        $collection->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
         $result = $collection->validate();
 
         $this->assertArraySubset(
@@ -632,7 +674,8 @@ class MongoCollectionTest extends TestCase
 
     public function testDrop()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         $expected = [
             'ns' => (string) $this->getCollection(),
             'nIndexesWas' => 1,
@@ -640,17 +683,4 @@ class MongoCollectionTest extends TestCase
         ];
         $this->assertSame($expected, $this->getCollection()->drop());
     }
-
-    /**
-     * @return \MongoCollection
-     */
-    protected function prepareData()
-    {
-        $collection = $this->getCollection();
-
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'foo']);
-        return $collection;
-    }
 }

tests/Alcaeus/MongoDbAdapter/MongoCommandCursorTest.php

@@ -50,17 +50,4 @@ class MongoCommandCursorTest extends TestCase
 
         $this->assertEquals($expected, $cursor->info());
     }
-
-    /**
-     * @return \MongoCollection
-     */
-    protected function prepareData()
-    {
-        $collection = $this->getCollection();
-
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'foo']);
-        return $collection;
-    }
 }

tests/Alcaeus/MongoDbAdapter/MongoCursorTest.php

@@ -255,17 +255,4 @@ class MongoCursorTest extends TestCase
     {
         return $this->getMock('MongoDB\Collection', [], [], '', false);
     }
-
-    /**
-     * @return \MongoCollection
-     */
-    protected function prepareData()
-    {
-        $collection = $this->getCollection();
-
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'foo']);
-        return $collection;
-    }
 }

tests/Alcaeus/MongoDbAdapter/MongoDBTest.php

@@ -125,19 +125,22 @@ class MongoDBTest extends TestCase
     public function testExecute()
     {
         $db = $this->getDatabase();
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         $this->assertEquals(['ok' => 1, 'retval' => 1], $db->execute("return db.test.count();"));
     }
 
     public function testGetCollectionNames()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         $this->assertContains('test', $this->getDatabase()->getCollectionNames());
     }
 
     public function testGetCollectionInfo()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
 
         foreach ($this->getDatabase()->getCollectionInfo() as $collectionInfo) {
             if ($collectionInfo['name'] === 'test') {
@@ -151,7 +154,8 @@ class MongoDBTest extends TestCase
 
     public function testListCollections()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         foreach ($this->getDatabase()->listCollections() as $collection) {
             $this->assertInstanceOf('MongoCollection', $collection);
 
@@ -165,13 +169,15 @@ class MongoDBTest extends TestCase
 
     public function testDrop()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         $this->assertSame(['dropped' => 'mongo-php-adapter', 'ok' => 1.0], $this->getDatabase()->drop());
     }
 
     public function testDropCollection()
     {
-        $this->getCollection()->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $this->getCollection()->insert($document);
         $expected = [
             'ns' => (string) $this->getCollection(),
             'nIndexesWas' => 1,

tests/Alcaeus/MongoDbAdapter/MongoDeleteBatchTest.php

@@ -9,8 +9,10 @@ class MongoDeleteBatchTest extends TestCase
         $collection = $this->getCollection();
         $batch = new \MongoDeleteBatch($collection);
 
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
+        unset($document['_id']);
+        $collection->insert($document);
 
         $this->assertTrue($batch->add(['q' => ['foo' => 'bar'], 'limit' => 1]));
 
@@ -34,8 +36,10 @@ class MongoDeleteBatchTest extends TestCase
         $collection = $this->getCollection();
         $batch = new \MongoDeleteBatch($collection);
 
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
+        unset($document['_id']);
+        $collection->insert($document);
 
         $this->assertTrue($batch->add(['q' => ['foo' => 'bar'], 'limit' => 0]));
 

tests/Alcaeus/MongoDbAdapter/MongoGridFSTest.php

@@ -23,8 +23,10 @@ class MongoGridFSTest extends TestCase
     {
         $collection = $this->getGridFS();
 
-        $collection->insert(['foo' => 'bar']);
-        $collection->chunks->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
+        unset($document['_id']);
+        $collection->chunks->insert($document);
 
         $collection->drop();
 
@@ -304,17 +306,4 @@ class MongoGridFSTest extends TestCase
 
         return $collection->storeBytes($data, $extra);
     }
-
-    /**
-     * @return \MongoCollection
-     */
-    protected function prepareData()
-    {
-        $collection = $this->getGridFS();
-
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'foo']);
-        return $collection;
-    }
 }

tests/Alcaeus/MongoDbAdapter/MongoUpdateBatchTest.php

@@ -11,7 +11,8 @@ class MongoUpdateBatchTest extends TestCase
         $collection = $this->getCollection();
         $batch = new \MongoUpdateBatch($collection);
 
-        $collection->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
 
         $this->assertTrue($batch->add(['q' => ['foo' => 'bar'], 'u' => ['$set' => ['foo' => 'foo']]]));
 
@@ -39,8 +40,10 @@ class MongoUpdateBatchTest extends TestCase
         $collection = $this->getCollection();
         $batch = new \MongoUpdateBatch($collection);
 
-        $collection->insert(['foo' => 'bar']);
-        $collection->insert(['foo' => 'bar']);
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
+        unset($document['_id']);
+        $collection->insert($document);
 
 
         $this->assertTrue($batch->add(['q' => ['foo' => 'bar'], 'u' => ['$set' => ['foo' => 'foo']], 'multi' => true]));

tests/Alcaeus/MongoDbAdapter/TestCase.php

@@ -76,4 +76,23 @@ abstract class TestCase extends \PHPUnit_Framework_TestCase
 
         return $database->getGridFS($prefix);
     }
+
+    /**
+     * @return \MongoCollection
+     */
+    protected function prepareData()
+    {
+        $collection = $this->getCollection();
+
+        $document = ['foo' => 'bar'];
+        $collection->insert($document);
+
+        unset($document['_id']);
+        $collection->insert($document);
+
+        $document = ['foo' => 'foo'];
+        $collection->insert($document);
+
+        return $collection;
+    }
 }