From af03397b3ba49518db7e5b23a3c37eea4a8fe078 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 12 Aug 2013 17:25:30 -0400 Subject: [PATCH 1/4] Remove array type-hint from Collection::createDBRef() param See: http://php.net/manual/en/mongocollection.createdbref.php --- lib/Doctrine/MongoDB/Collection.php | 8 ++++---- lib/Doctrine/MongoDB/LoggableCollection.php | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/Doctrine/MongoDB/Collection.php b/lib/Doctrine/MongoDB/Collection.php index 74833b85..9b5aa826 100644 --- a/lib/Doctrine/MongoDB/Collection.php +++ b/lib/Doctrine/MongoDB/Collection.php @@ -190,10 +190,10 @@ public function count(array $query = array(), $limit = 0, $skip = 0) * Wrapper method for MongoCollection::createDBRef(). * * @see http://php.net/manual/en/mongocollection.createdbref.php - * @param array $a + * @param mixed $a * @return array */ - public function createDBRef(array $a) + public function createDBRef($a) { if ($this->eventManager->hasListeners(Events::preCreateDBRef)) { $this->eventManager->dispatchEvent(Events::preCreateDBRef, new EventArgs($this, $a)); @@ -898,10 +898,10 @@ protected function doBatchInsert(array &$a, array $options = array()) * Creates a database reference. * * @see Collection::createDBRef() - * @param array $a + * @param mixed $a * @return array */ - protected function doCreateDBRef(array $a) + protected function doCreateDBRef($a) { return $this->getMongoCollection()->createDBRef($a); } diff --git a/lib/Doctrine/MongoDB/LoggableCollection.php b/lib/Doctrine/MongoDB/LoggableCollection.php index bdaa079e..bd305646 100644 --- a/lib/Doctrine/MongoDB/LoggableCollection.php +++ b/lib/Doctrine/MongoDB/LoggableCollection.php @@ -103,7 +103,7 @@ public function count(array $query = array(), $limit = 0, $skip = 0) /** * @see Collection::createDBRef() */ - public function createDBRef(array $a) + public function createDBRef($a) { $this->log(array( 'createDBRef' => true, From d907c5fb6aeddb57f7ab2cf0e5b72b81401cb27b Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 12 Aug 2013 17:29:14 -0400 Subject: [PATCH 2/4] Remove events/logging for Collection::createDBRef() This method is incredibly trivial, and there is little value in dispatching events or logging its calls. --- lib/Doctrine/MongoDB/Collection.php | 28 ++----------------- lib/Doctrine/MongoDB/Events.php | 3 -- lib/Doctrine/MongoDB/LoggableCollection.php | 13 --------- .../MongoDB/Tests/CollectionEventsTest.php | 16 ----------- 4 files changed, 3 insertions(+), 57 deletions(-) diff --git a/lib/Doctrine/MongoDB/Collection.php b/lib/Doctrine/MongoDB/Collection.php index 9b5aa826..e035497d 100644 --- a/lib/Doctrine/MongoDB/Collection.php +++ b/lib/Doctrine/MongoDB/Collection.php @@ -190,22 +190,12 @@ public function count(array $query = array(), $limit = 0, $skip = 0) * Wrapper method for MongoCollection::createDBRef(). * * @see http://php.net/manual/en/mongocollection.createdbref.php - * @param mixed $a + * @param mixed $documentOrId * @return array */ - public function createDBRef($a) + public function createDBRef($documentOrId) { - if ($this->eventManager->hasListeners(Events::preCreateDBRef)) { - $this->eventManager->dispatchEvent(Events::preCreateDBRef, new EventArgs($this, $a)); - } - - $result = $this->doCreateDBRef($a); - - if ($this->eventManager->hasListeners(Events::postCreateDBRef)) { - $this->eventManager->dispatchEvent(Events::postCreateDBRef, new EventArgs($this, $result)); - } - - return $result; + return $this->getMongoCollection()->createDBRef($documentOrId); } /** @@ -894,18 +884,6 @@ protected function doBatchInsert(array &$a, array $options = array()) return $this->getMongoCollection()->batchInsert($a, $options); } - /** - * Creates a database reference. - * - * @see Collection::createDBRef() - * @param mixed $a - * @return array - */ - protected function doCreateDBRef($a) - { - return $this->getMongoCollection()->createDBRef($a); - } - /** * Execute the distinct command. * diff --git a/lib/Doctrine/MongoDB/Events.php b/lib/Doctrine/MongoDB/Events.php index 7dc2a22a..c05265d1 100644 --- a/lib/Doctrine/MongoDB/Events.php +++ b/lib/Doctrine/MongoDB/Events.php @@ -40,9 +40,6 @@ private function __construct() {} const preCreateCollection = 'preCreateCollection'; const postCreateCollection = 'postCreateCollection'; - const preCreateDBRef = 'collectionPreCreateDBRef'; - const postCreateDBRef = 'collectionPostCreateDBRef'; - const preConnect = 'preConnect'; const postConnect = 'postConnect'; diff --git a/lib/Doctrine/MongoDB/LoggableCollection.php b/lib/Doctrine/MongoDB/LoggableCollection.php index bd305646..ce0e1a16 100644 --- a/lib/Doctrine/MongoDB/LoggableCollection.php +++ b/lib/Doctrine/MongoDB/LoggableCollection.php @@ -100,19 +100,6 @@ public function count(array $query = array(), $limit = 0, $skip = 0) return parent::count($query, $limit, $skip); } - /** - * @see Collection::createDBRef() - */ - public function createDBRef($a) - { - $this->log(array( - 'createDBRef' => true, - 'reference' => $a, - )); - - return parent::createDBRef($a); - } - /** * @see Collection::deleteIndex() */ diff --git a/tests/Doctrine/MongoDB/Tests/CollectionEventsTest.php b/tests/Doctrine/MongoDB/Tests/CollectionEventsTest.php index 47c36d19..cb9b0a21 100644 --- a/tests/Doctrine/MongoDB/Tests/CollectionEventsTest.php +++ b/tests/Doctrine/MongoDB/Tests/CollectionEventsTest.php @@ -51,22 +51,6 @@ public function testBatchInsert() $this->assertSame($result, $collection->batchInsert($documents, $options)); } - public function testCreateDBRef() - { - $document = array('_id' => 1); - $result = array('$ref' => self::collectionName, '$id' => 1); - - $eventManager = $this->getMockEventManager(); - $collection = $this->getMockCollection($eventManager, array('doCreateDBRef' => $result)); - - $this->expectEvents($eventManager, array( - array(Events::preCreateDBRef, new EventArgs($collection, $document)), - array(Events::postCreateDBRef, new EventArgs($collection, $result)), - )); - - $this->assertSame($result, $collection->createDBRef($document)); - } - public function testDistinct() { $field = 'x'; From 25ef3ed7414fc7754c072d873f2ee18ac500bdeb Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 12 Aug 2013 17:47:31 -0400 Subject: [PATCH 3/4] Remove logging for Database::createDBRef() This is being removed for the same reason as the Collection method (creating DBRefs is a very trivial operation). --- lib/Doctrine/MongoDB/LoggableDatabase.php | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/lib/Doctrine/MongoDB/LoggableDatabase.php b/lib/Doctrine/MongoDB/LoggableDatabase.php index 4575799b..dd68e756 100644 --- a/lib/Doctrine/MongoDB/LoggableDatabase.php +++ b/lib/Doctrine/MongoDB/LoggableDatabase.php @@ -111,20 +111,6 @@ public function createCollection($name, $capped = false, $size = 0, $max = 0) return parent::createCollection($name, $capped, $size, $max); } - /** - * @see Database::createDBRef() - */ - public function createDBRef($collection, $a) - { - $this->log(array( - 'createDBRef' => true, - 'collection' => $collection, - 'reference' => $a, - )); - - return parent::createDBRef($collection, $a); - } - /** * @see Database::drop() */ From c4f20e5054c4a80ecc77cc513fb51a3c9b755dfb Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Mon, 12 Aug 2013 17:45:08 -0400 Subject: [PATCH 4/4] Database::getDBRef() should dispatch events Also create unit test for Database's event dispatching. --- lib/Doctrine/MongoDB/Database.php | 64 +++++++++++++- .../MongoDB/Tests/DatabaseEventsTest.php | 83 +++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 tests/Doctrine/MongoDB/Tests/DatabaseEventsTest.php diff --git a/lib/Doctrine/MongoDB/Database.php b/lib/Doctrine/MongoDB/Database.php index ab07a0ca..70608569 100644 --- a/lib/Doctrine/MongoDB/Database.php +++ b/lib/Doctrine/MongoDB/Database.php @@ -22,6 +22,7 @@ use Doctrine\Common\EventManager; use Doctrine\MongoDB\Event\CreateCollectionEventArgs; use Doctrine\MongoDB\Event\EventArgs; +use Doctrine\MongoDB\Event\MutableEventArgs; use Doctrine\MongoDB\Util\ReadPreference; /** @@ -224,13 +225,27 @@ public function forceError() /** * Wrapper method for MongoDB::getDBRef(). * + * This method will dispatch preGetDBRef and postGetDBRef events. + * * @see http://php.net/manual/en/mongodb.getdbref.php * @param array $reference * @return array|null */ public function getDBRef(array $reference) { - return $this->getMongoDB()->getDBRef($reference); + if ($this->eventManager->hasListeners(Events::preGetDBRef)) { + $this->eventManager->dispatchEvent(Events::preGetDBRef, new EventArgs($this, $reference)); + } + + $result = $this->doGetDBRef($reference); + + if ($this->eventManager->hasListeners(Events::postGetDBRef)) { + $eventArgs = new MutableEventArgs($this, $result); + $this->eventManager->dispatchEvent(Events::postGetDBRef, $eventArgs); + $result = $eventArgs->getData(); + } + + return $result; } /** @@ -489,6 +504,21 @@ public function __toString() return $this->name; } + /** + * Resolves a database reference. + * + * @see Database::getDBRef() + * @param array $reference + * @return array|null + */ + protected function doGetDBRef(array $reference) + { + $database = $this; + return $this->retry(function() use ($database, $reference) { + return $database->getMongoDB()->getDBRef($reference); + }); + } + /** * Return a new GridFS instance. * @@ -512,4 +542,36 @@ protected function doSelectCollection($name) { return new Collection($this->connection, $name, $this, $this->eventManager, $this->cmd, $this->numRetries); } + + /** + * Conditionally retry a closure if it yields an exception. + * + * If the closure does not return successfully within the configured number + * of retries, its first exception will be thrown. + * + * This method should not be used for write operations. + * + * @param \Closure $retry + * @return mixed + */ + protected function retry(\Closure $retry) + { + if ($this->numRetries) { + $firstException = null; + for ($i = 0; $i <= $this->numRetries; $i++) { + try { + return $retry(); + } catch (\MongoException $e) { + if (!$firstException) { + $firstException = $e; + } + if ($i === $this->numRetries) { + throw $firstException; + } + } + } + } else { + return $retry(); + } + } } diff --git a/tests/Doctrine/MongoDB/Tests/DatabaseEventsTest.php b/tests/Doctrine/MongoDB/Tests/DatabaseEventsTest.php new file mode 100644 index 00000000..14c001f4 --- /dev/null +++ b/tests/Doctrine/MongoDB/Tests/DatabaseEventsTest.php @@ -0,0 +1,83 @@ + 'collection', '$id' => 1); + $result = array('_id' => 1); + + $eventManager = $this->getMockEventManager(); + $db = $this->getMockDatabase($eventManager, array('doGetDBRef' => $result)); + + $this->expectEvents($eventManager, array( + array(Events::preGetDBRef, new EventArgs($db, $reference)), + array(Events::postGetDBRef, new MutableEventArgs($db, $result)), + )); + + $this->assertSame($result, $db->getDBRef($reference)); + } + + /** + * Expect events to be dispatched by the event manager in the given order. + * + * @param EventManager $em EventManager mock + * @param array $events Tuple of event name and dispatch argument + */ + private function expectEvents(EventManager $em, array $events) + { + /* Each event should be a tuple consisting of the event name and the + * dispatched argument (e.g. EventArgs). + * + * For each event, expect a call to hasListeners() immediately followed + * by a call to dispatchEvent(). The dispatch argument is passed as-is + * to with(), so constraints may be used (e.g. callback). + */ + foreach ($events as $i => $event) { + $em->expects($this->at($i * 2)) + ->method('hasListeners') + ->with($event[0]) + ->will($this->returnValue(true)); + + $em->expects($this->at($i * 2 + 1)) + ->method('dispatchEvent') + ->with($event[0], $event[1]); + } + } + + private function getMockDatabase(EventManager $em, array $methods) + { + $c = $this->getMockBuilder('Doctrine\MongoDB\Connection') + ->disableOriginalConstructor() + ->getMock(); + + $db = $this->getMockBuilder('Doctrine\MongoDB\Database') + ->setConstructorArgs(array($c, self::databaseName, $em, '$')) + ->setMethods(array_keys($methods)) + ->getMock(); + + foreach ($methods as $method => $returnValue) { + $db->expects($this->once()) + ->method($method) + ->will($this->returnValue($returnValue)); + } + + return $db; + } + + private function getMockEventManager() + { + return $this->getMockBuilder('Doctrine\Common\EventManager') + ->disableOriginalConstructor() + ->getMock(); + } +}