From 9315437da7a79a8edbb6076817806dfd91ac3fd5 Mon Sep 17 00:00:00 2001 From: Martin Holzhauer Date: Mon, 3 Feb 2014 22:56:21 +0100 Subject: [PATCH 1/6] #151 - initial work for QueryLogger integration --- lib/Doctrine/MongoDB/Configuration.php | 20 +++ lib/Doctrine/MongoDB/Connection.php | 5 +- lib/Doctrine/MongoDB/LoggableCollection.php | 149 +++++++++++++------ lib/Doctrine/MongoDB/LoggableCursor.php | 71 ++++++--- lib/Doctrine/MongoDB/LoggableDatabase.php | 80 +++++++--- lib/Doctrine/MongoDB/Logging/QueryLogger.php | 31 ++++ 6 files changed, 271 insertions(+), 85 deletions(-) create mode 100644 lib/Doctrine/MongoDB/Logging/QueryLogger.php diff --git a/lib/Doctrine/MongoDB/Configuration.php b/lib/Doctrine/MongoDB/Configuration.php index 4f167a82..f5ef783a 100644 --- a/lib/Doctrine/MongoDB/Configuration.php +++ b/lib/Doctrine/MongoDB/Configuration.php @@ -58,6 +58,26 @@ public function setLoggerCallable($loggerCallable) $this->attributes['loggerCallable'] = $loggerCallable; } + /** + * Set QueryLogger + * + * @param Logging\QueryLogger $queryLogger + */ + public function setQueryLogger(Logging\QueryLogger $queryLogger) + { + $this->attributes['queryLogger'] = $queryLogger; + } + + /** + * Get QueryLogger + * + * @return null|Logging\QueryLogger + */ + public function getQueryLogger() + { + return isset($this->attributes['queryLogger']) ? $this->attributes['queryLogger'] : null; + } + /** * Get the MongoDB command prefix. * diff --git a/lib/Doctrine/MongoDB/Connection.php b/lib/Doctrine/MongoDB/Connection.php index 662ccf31..63efe71b 100644 --- a/lib/Doctrine/MongoDB/Connection.php +++ b/lib/Doctrine/MongoDB/Connection.php @@ -404,9 +404,10 @@ protected function doSelectDatabase($name) $mongoDB = $this->mongoClient->selectDB($name); $numRetries = $this->config->getRetryQuery(); $loggerCallable = $this->config->getLoggerCallable(); + $queryLogger = $this->config->getQueryLogger(); - return $loggerCallable !== null - ? new LoggableDatabase($this, $mongoDB, $this->eventManager, $numRetries, $loggerCallable) + return ($loggerCallable !== null || $queryLogger!== null) + ? new LoggableDatabase($this, $mongoDB, $this->eventManager, $numRetries, $loggerCallable, $queryLogger) : new Database($this, $mongoDB, $this->eventManager, $numRetries); } diff --git a/lib/Doctrine/MongoDB/LoggableCollection.php b/lib/Doctrine/MongoDB/LoggableCollection.php index 6228810d..ca91ed8a 100644 --- a/lib/Doctrine/MongoDB/LoggableCollection.php +++ b/lib/Doctrine/MongoDB/LoggableCollection.php @@ -37,6 +37,11 @@ class LoggableCollection extends Collection implements Loggable */ protected $loggerCallable; + /** + * @var Logging\QueryLogger + */ + protected $queryLogger; + /** * Constructor. * @@ -45,13 +50,15 @@ class LoggableCollection extends Collection implements Loggable * @param EventManager $evm EventManager instance * @param integer $numRetries Number of times to retry queries * @param callable $loggerCallable The logger callable + * @param Logging\QueryLogger $queryLogger The QueryLogger object */ - public function __construct(Database $database, \MongoCollection $mongoCollection, EventManager $evm, $numRetries, $loggerCallable) + public function __construct(Database $database, \MongoCollection $mongoCollection, EventManager $evm, $numRetries, $loggerCallable, Logging\QueryLogger $queryLogger = null) { if ( ! is_callable($loggerCallable)) { throw new \InvalidArgumentException('$loggerCallable must be a valid callback'); } $this->loggerCallable = $loggerCallable; + $this->queryLogger = $queryLogger; parent::__construct($database, $mongoCollection, $evm, $numRetries); } @@ -65,7 +72,27 @@ public function log(array $log) { $log['db'] = $this->database->getName(); $log['collection'] = $this->getName(); - call_user_func_array($this->loggerCallable, array($log)); + if($this->loggerCallable){ + call_user_func_array($this->loggerCallable, array($log)); + } + + if($this->queryLogger instanceof Logging\QueryLogger){ + $this->queryLogger->startQuery($log); + } + } + + /** + * @param array $log + * @param callable $callback + * @return mixed + */ + protected function logMethod($log, $callback) { + $this->log($log); + $data = call_user_func($callback); + if ($this->queryLogger instanceof Logging\QueryLogger) { + $this->queryLogger->stopQuery(); + } + return $data; } /** @@ -73,14 +100,16 @@ public function log(array $log) */ public function batchInsert(array &$a, array $options = array()) { - $this->log(array( + $log = array( 'batchInsert' => true, 'num' => count($a), 'data' => $a, 'options' => $options, - )); + ); - return parent::batchInsert($a, $options); + return $this->logMethod($log, function () use (&$a, $options) { + return parent::batchInsert($a, $options); + }); } /** @@ -88,14 +117,16 @@ public function batchInsert(array &$a, array $options = array()) */ public function count(array $query = array(), $limit = 0, $skip = 0) { - $this->log(array( + $log = array( 'count' => true, 'query' => $query, 'limit' => $limit, 'skip' => $skip, - )); + ); - return parent::count($query, $limit, $skip); + return $this->logMethod($log, function () use ($query, $limit, $skip) { + return parent::count($query, $limit, $skip); + }); } /** @@ -103,12 +134,14 @@ public function count(array $query = array(), $limit = 0, $skip = 0) */ public function deleteIndex($keys) { - $this->log(array( + $log = array( 'deleteIndex' => true, 'keys' => $keys, - )); + ); - return parent::deleteIndex($keys); + return $this->logMethod($log, function () use ($keys) { + return parent::deleteIndex($keys); + }); } /** @@ -116,9 +149,11 @@ public function deleteIndex($keys) */ public function deleteIndexes() { - $this->log(array('deleteIndexes' => true)); + $log = array('deleteIndexes' => true); - return parent::deleteIndexes(); + return $this->logMethod($log, function () { + return parent::deleteIndexes(); + }); } /** @@ -126,9 +161,11 @@ public function deleteIndexes() */ public function drop() { - $this->log(array('drop' => true)); + $log = array('drop' => true); - return parent::drop(); + return $this->logMethod($log, function () { + return parent::drop(); + }); } /** @@ -136,13 +173,15 @@ public function drop() */ public function ensureIndex(array $keys, array $options = array()) { - $this->log(array( + $log = array( 'ensureIndex' => true, 'keys' => $keys, 'options' => $options, - )); + ); - return parent::ensureIndex($keys, $options); + return $this->logMethod($log, function () use ($keys, $options) { + return parent::ensureIndex($keys, $options); + }); } /** @@ -150,13 +189,15 @@ public function ensureIndex(array $keys, array $options = array()) */ public function find(array $query = array(), array $fields = array()) { - $this->log(array( + $log = array( 'find' => true, 'query' => $query, 'fields' => $fields, - )); + ); - return parent::find($query, $fields); + return $this->logMethod($log, function () use ($query, $fields) { + return parent::find($query, $fields); + }); } /** @@ -164,13 +205,15 @@ public function find(array $query = array(), array $fields = array()) */ public function findOne(array $query = array(), array $fields = array()) { - $this->log(array( + $log = array( 'findOne' => true, 'query' => $query, 'fields' => $fields, - )); + ); - return parent::findOne($query, $fields); + return $this->logMethod($log, function () use ($query, $fields) { + return parent::findOne($query, $fields); + }); } /** @@ -178,12 +221,14 @@ public function findOne(array $query = array(), array $fields = array()) */ public function getDBRef(array $reference) { - $this->log(array( + $log = array( 'getDBRef' => true, 'reference' => $reference, - )); + ); - return parent::getDBRef($reference); + return $this->logMethod($log, function () use ($reference) { + return parent::getDBRef($reference); + }); } /** @@ -191,15 +236,17 @@ public function getDBRef(array $reference) */ public function group($keys, array $initial, $reduce, array $options = array()) { - $this->log(array( + $log = array( 'group' => true, 'keys' => $keys, 'initial' => $initial, 'reduce' => $reduce, 'options' => $options, - )); + ); - return parent::group($keys, $initial, $reduce, $options); + return $this->logMethod($log, function () use ($keys, $initial, $reduce, $options) { + return parent::group($keys, $initial, $reduce, $options); + }); } /** @@ -207,13 +254,15 @@ public function group($keys, array $initial, $reduce, array $options = array()) */ public function insert(array &$a, array $options = array()) { - $this->log(array( + $log = array( 'insert' => true, 'document' => $a, 'options' => $options, - )); + ); - return parent::insert($a, $options); + return $this->logMethod($log, function () use (&$a, $options) { + return parent::insert($a, $options); + }); } /** @@ -221,13 +270,15 @@ public function insert(array &$a, array $options = array()) */ public function remove(array $query, array $options = array()) { - $this->log(array( + $log = array( 'remove' => true, 'query' => $query, 'options' => $options, - )); + ); - return parent::remove($query, $options); + return $this->logMethod($log, function () use ($query, $options) { + return parent::remove($query, $options); + }); } /** @@ -235,13 +286,15 @@ public function remove(array $query, array $options = array()) */ public function save(array &$a, array $options = array()) { - $this->log(array( + $log = array( 'save' => true, 'document' => $a, 'options' => $options, - )); + ); - return parent::save($a, $options); + return $this->logMethod($log, function () use (&$a, $options) { + return parent::save($a, $options); + }); } /** @@ -249,14 +302,16 @@ public function save(array &$a, array $options = array()) */ public function update($query, array $newObj, array $options = array()) { - $this->log(array( + $log = array( 'update' => true, 'query' => $query, 'newObj' => $newObj, 'options' => $options, - )); + ); - return parent::update($query, $newObj, $options); + return $this->logMethod($log, function () use ($query, $newObj, $options) { + return parent::update($query, $newObj, $options); + }); } /** @@ -264,12 +319,14 @@ public function update($query, array $newObj, array $options = array()) */ public function validate($scanData = false) { - $this->log(array( + $log = array( 'validate' => true, 'scanData' => $scanData, - )); + ); - return parent::validate($scanData); + return $this->logMethod($log, function () use ($scanData) { + return parent::validate($scanData); + }); } /** @@ -283,6 +340,6 @@ public function validate($scanData = false) */ protected function wrapCursor(\MongoCursor $cursor, $query, $fields) { - return new LoggableCursor($this, $cursor, $query, $fields, $this->numRetries, $this->loggerCallable); + return new LoggableCursor($this, $cursor, $query, $fields, $this->numRetries, $this->loggerCallable, $this->queryLogger); } } diff --git a/lib/Doctrine/MongoDB/LoggableCursor.php b/lib/Doctrine/MongoDB/LoggableCursor.php index e76e86f6..41a42249 100644 --- a/lib/Doctrine/MongoDB/LoggableCursor.php +++ b/lib/Doctrine/MongoDB/LoggableCursor.php @@ -34,6 +34,11 @@ class LoggableCursor extends Cursor implements Loggable */ protected $loggerCallable; + /** + * @var Logging\QueryLogger + */ + protected $queryLogger; + /** * Constructor. * @@ -43,14 +48,16 @@ class LoggableCursor extends Cursor implements Loggable * @param array $fields Selected fields (projection) * @param integer $numRetries Number of times to retry queries * @param callable $loggerCallable Logger callable + * @param Logging\QueryLogger $queryLogger QueryLogger object */ - public function __construct(Collection $collection, \MongoCursor $mongoCursor, array $query, array $fields, $numRetries, $loggerCallable) + public function __construct(Collection $collection, \MongoCursor $mongoCursor, array $query, array $fields, $numRetries, $loggerCallable, Logging\QueryLogger $queryLogger = null) { if ( ! is_callable($loggerCallable)) { throw new \InvalidArgumentException('$loggerCallable must be a valid callback'); } $this->loggerCallable = $loggerCallable; parent::__construct($collection, $mongoCursor, $query, $fields, $numRetries); + $this->queryLogger = $queryLogger; } /** @@ -63,7 +70,27 @@ public function log(array $data) { $data['query'] = $this->query; $data['fields'] = $this->fields; - call_user_func($this->loggerCallable, $data); + if ($this->loggerCallable) { + call_user_func($this->loggerCallable, $data); + } + + if($this->queryLogger instanceof Logging\QueryLogger){ + $this->queryLogger->startQuery($data); + } + } + + /** + * @param array $log + * @param callable $callback + * @return mixed + */ + protected function logMethod($log, $callback) { + $this->log($log); + $data = call_user_func($callback); + if ($this->queryLogger instanceof Logging\QueryLogger) { + $this->queryLogger->stopQuery(); + } + return $data; } /** @@ -81,12 +108,14 @@ public function getLoggerCallable() */ public function hint(array $keyPattern) { - $this->log(array( + $log = array( 'hint' => true, 'keyPattern' => $keyPattern, - )); + ); - return parent::hint($keyPattern); + return $this->logMethod($log, function () use ($keyPattern) { + return parent::hint($keyPattern); + }); } /** @@ -94,12 +123,14 @@ public function hint(array $keyPattern) */ public function limit($num) { - $this->log(array( + $log = array( 'limit' => true, 'limitNum' => $num, - )); + ); - return parent::limit($num); + return $this->logMethod($log, function () use ($num) { + return parent::limit($num); + }); } /** @@ -107,12 +138,14 @@ public function limit($num) */ public function skip($num) { - $this->log(array( + $log = array( 'skip' => true, 'skipNum' => $num, - )); + ); - return parent::skip($num); + return $this->logMethod($log, function () use ($num) { + return parent::skip($num); + }); } /** @@ -120,11 +153,13 @@ public function skip($num) */ public function snapshot() { - $this->log(array( + $log = array( 'snapshot' => true, - )); + ); - return parent::snapshot(); + return $this->logMethod($log, function () { + return parent::snapshot(); + }); } /** @@ -132,11 +167,13 @@ public function snapshot() */ public function sort($fields) { - $this->log(array( + $log = array( 'sort' => true, 'sortFields' => $fields, - )); + ); - return parent::sort($fields); + return $this->logMethod($log, function () use ($fields) { + return parent::sort($fields); + }); } } diff --git a/lib/Doctrine/MongoDB/LoggableDatabase.php b/lib/Doctrine/MongoDB/LoggableDatabase.php index 72a976f8..dac7d487 100644 --- a/lib/Doctrine/MongoDB/LoggableDatabase.php +++ b/lib/Doctrine/MongoDB/LoggableDatabase.php @@ -37,6 +37,11 @@ class LoggableDatabase extends Database implements Loggable */ protected $loggerCallable; + /** + * @var Logging\QueryLogger + */ + protected $queryLogger; + /** * Constructor. * @@ -45,14 +50,16 @@ class LoggableDatabase extends Database implements Loggable * @param EventManager $evm EventManager instance * @param integer $numRetries Number of times to retry queries * @param callable $loggerCallable The logger callable + * @param Logging\QueryLogger $queryLogger The QueryLogger object */ - public function __construct(Connection $connection, \MongoDB $mongoDB, EventManager $evm, $numRetries, $loggerCallable) + public function __construct(Connection $connection, \MongoDB $mongoDB, EventManager $evm, $numRetries, $loggerCallable, Logging\QueryLogger $queryLogger = null) { if ( ! is_callable($loggerCallable)) { throw new \InvalidArgumentException('$loggerCallable must be a valid callback'); } parent::__construct($connection, $mongoDB, $evm, $numRetries); $this->loggerCallable = $loggerCallable; + $this->queryLogger = $queryLogger; } /** @@ -64,7 +71,27 @@ public function __construct(Connection $connection, \MongoDB $mongoDB, EventMana public function log(array $log) { $log['db'] = $this->getName(); - call_user_func_array($this->loggerCallable, array($log)); + if($this->loggerCallable){ + call_user_func_array($this->loggerCallable, array($log)); + } + + if($this->queryLogger instanceof Logging\QueryLogger){ + $this->queryLogger->startQuery($log); + } + } + + /** + * @param array $log + * @param callable $callback + * @return mixed + */ + protected function logMethod($log, $callback) { + $this->log($log); + $data = call_user_func($callback); + if($this->queryLogger instanceof Logging\QueryLogger){ + $this->queryLogger->stopQuery(); + } + return $data; } /** @@ -72,13 +99,15 @@ public function log(array $log) */ public function authenticate($username, $password) { - $this->log(array( + $log = array( 'authenticate' => true, 'username' => $username, 'password' => $password, - )); + ); - return parent::authenticate($username, $password); + return $this->logMethod($log, function () use ($username, $password) { + return parent::authenticate($username, $password); + }); } /** @@ -86,13 +115,15 @@ public function authenticate($username, $password) */ public function command(array $data, array $options = array()) { - $this->log(array( + $log = array( 'command' => true, 'data' => $data, 'options' => $options, - )); + ); - return parent::command($data, $options); + return $this->logMethod($log, function () use ($data, $options) { + return parent::command($data, $options); + }); } /** @@ -104,7 +135,7 @@ public function createCollection($name, $cappedOrOptions = false, $size = 0, $ma ? array_merge(array('capped' => false, 'size' => 0, 'max' => 0), $cappedOrOptions) : array('capped' => $cappedOrOptions, 'size' => $size, 'max' => $max); - $this->log(array( + $log = array( 'createCollection' => true, 'name' => $name, 'options' => $options, @@ -112,9 +143,11 @@ public function createCollection($name, $cappedOrOptions = false, $size = 0, $ma 'capped' => $options['capped'], 'size' => $options['size'], 'max' => $options['max'], - )); + ); - return parent::createCollection($name, $options); + return $this->logMethod($log, function () use ($name, $options) { + return parent::createCollection($name, $options); + }); } /** @@ -122,9 +155,12 @@ public function createCollection($name, $cappedOrOptions = false, $size = 0, $ma */ public function drop() { - $this->log(array('dropDatabase' => true)); + $log = array('dropDatabase' => true); - return parent::drop(); + return $this->logMethod($log, + function () { + return parent::drop(); + }); } /** @@ -132,13 +168,15 @@ public function drop() */ public function execute($code, array $args = array()) { - $this->log(array( + $log = array( 'execute' => true, 'code' => $code, 'args' => $args, - )); + ); - return parent::execute($code, $args); + return $this->logMethod($log, function () use ($code, $args) { + return parent::execute($code, $args); + }); } /** @@ -146,12 +184,14 @@ public function execute($code, array $args = array()) */ public function getDBRef(array $ref) { - $this->log(array( + $log = array( 'getDBRef' => true, 'reference' => $ref, - )); + ); - return parent::getDBRef($ref); + return $this->logMethod($log, function () use ($ref) { + return parent::getDBRef($ref); + }); } /** @@ -165,6 +205,6 @@ protected function doSelectCollection($name) { $mongoCollection = $this->mongoDB->selectCollection($name); - return new LoggableCollection($this, $mongoCollection, $this->eventManager, $this->numRetries, $this->loggerCallable); + return new LoggableCollection($this, $mongoCollection, $this->eventManager, $this->numRetries, $this->loggerCallable, $this->queryLogger); } } diff --git a/lib/Doctrine/MongoDB/Logging/QueryLogger.php b/lib/Doctrine/MongoDB/Logging/QueryLogger.php new file mode 100644 index 00000000..44b9d20f --- /dev/null +++ b/lib/Doctrine/MongoDB/Logging/QueryLogger.php @@ -0,0 +1,31 @@ +. + */ + +namespace Doctrine\MongoDB\Logging; + +/** + * QueryLogger interface. + * + * @author Martin Holzhauer + */ +interface QueryLogger { + public function startQuery($parameter); + + public function stopQuery(); +} \ No newline at end of file From 4ae21a140ad8b3beb32f15f0d1ab0b51b153645e Mon Sep 17 00:00:00 2001 From: Martin Holzhauer Date: Mon, 3 Feb 2014 23:02:08 +0100 Subject: [PATCH 2/6] #151 - added sample implementations --- .../MongoDB/Logging/EchoQueryLogger.php | 39 +++++++++++++++++ lib/Doctrine/MongoDB/Logging/LoggerChain.php | 43 +++++++++++++++++++ 2 files changed, 82 insertions(+) create mode 100644 lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php create mode 100644 lib/Doctrine/MongoDB/Logging/LoggerChain.php diff --git a/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php b/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php new file mode 100644 index 00000000..08441bba --- /dev/null +++ b/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php @@ -0,0 +1,39 @@ +. + */ + +namespace Doctrine\MongoDB\Logging; + +/** + * EchoLogger implementation. + * + * @author Martin Holzhauer + */ +class EchoQueryLogger implements QueryLogger { + + public function startQuery($parameter) + { + var_dump($parameter); + echo PHP_EOL; + } + + public function stopQuery() + { + // TODO: Implement stopQuery() method. + } +} \ No newline at end of file diff --git a/lib/Doctrine/MongoDB/Logging/LoggerChain.php b/lib/Doctrine/MongoDB/Logging/LoggerChain.php new file mode 100644 index 00000000..99a02a37 --- /dev/null +++ b/lib/Doctrine/MongoDB/Logging/LoggerChain.php @@ -0,0 +1,43 @@ +logger[] = $queryLogger; + } + + public function startQuery($parameter) + { + foreach($this->logger as $logger) + { + $logger->startQuery($parameter); + } + } + + public function stopQuery() + { + foreach($this->logger as $logger) + { + $logger->stopQuery(); + } + } +} \ No newline at end of file From c9f8c970f7163079b2e393de9266afd9b838389d Mon Sep 17 00:00:00 2001 From: Martin Holzhauer Date: Mon, 3 Feb 2014 23:09:37 +0100 Subject: [PATCH 3/6] #151 - replaced phpstorm comment with doctrine copyright block --- lib/Doctrine/MongoDB/Logging/LoggerChain.php | 27 +++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/lib/Doctrine/MongoDB/Logging/LoggerChain.php b/lib/Doctrine/MongoDB/Logging/LoggerChain.php index 99a02a37..5a93fa91 100644 --- a/lib/Doctrine/MongoDB/Logging/LoggerChain.php +++ b/lib/Doctrine/MongoDB/Logging/LoggerChain.php @@ -1,14 +1,29 @@ . */ namespace Doctrine\MongoDB\Logging; - +/** + * QueryLogger chain implementation. + * + * @author Martin Holzhauer + */ class LoggerChain implements QueryLogger { /** From 850f82dbcdc2d488807b19d250e881fe7ac145bf Mon Sep 17 00:00:00 2001 From: Martin Holzhauer Date: Wed, 5 Feb 2014 17:40:04 +0100 Subject: [PATCH 4/6] replaced closure solution with simple variable --- lib/Doctrine/MongoDB/LoggableCollection.php | 117 +++++++++++--------- lib/Doctrine/MongoDB/LoggableCursor.php | 47 ++++---- lib/Doctrine/MongoDB/LoggableDatabase.php | 53 +++++---- 3 files changed, 109 insertions(+), 108 deletions(-) diff --git a/lib/Doctrine/MongoDB/LoggableCollection.php b/lib/Doctrine/MongoDB/LoggableCollection.php index ca91ed8a..38aced6a 100644 --- a/lib/Doctrine/MongoDB/LoggableCollection.php +++ b/lib/Doctrine/MongoDB/LoggableCollection.php @@ -81,18 +81,10 @@ public function log(array $log) } } - /** - * @param array $log - * @param callable $callback - * @return mixed - */ - protected function logMethod($log, $callback) { - $this->log($log); - $data = call_user_func($callback); - if ($this->queryLogger instanceof Logging\QueryLogger) { + private function logAfter() { + if($this->queryLogger instanceof Logging\QueryLogger){ $this->queryLogger->stopQuery(); } - return $data; } /** @@ -107,9 +99,10 @@ public function batchInsert(array &$a, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use (&$a, $options) { - return parent::batchInsert($a, $options); - }); + $this->log($log); + $data = parent::batchInsert($a, $options); + $this->logAfter(); + return $data; } /** @@ -124,9 +117,10 @@ public function count(array $query = array(), $limit = 0, $skip = 0) 'skip' => $skip, ); - return $this->logMethod($log, function () use ($query, $limit, $skip) { - return parent::count($query, $limit, $skip); - }); + $this->log($log); + $data = parent::count($query, $limit, $skip); + $this->logAfter(); + return $data; } /** @@ -139,9 +133,10 @@ public function deleteIndex($keys) 'keys' => $keys, ); - return $this->logMethod($log, function () use ($keys) { - return parent::deleteIndex($keys); - }); + $this->log($log); + $data = parent::deleteIndex($keys); + $this->logAfter(); + return $data; } /** @@ -151,9 +146,10 @@ public function deleteIndexes() { $log = array('deleteIndexes' => true); - return $this->logMethod($log, function () { - return parent::deleteIndexes(); - }); + $this->log($log); + $data = parent::deleteIndexes(); + $this->logAfter(); + return $data; } /** @@ -163,9 +159,10 @@ public function drop() { $log = array('drop' => true); - return $this->logMethod($log, function () { - return parent::drop(); - }); + $this->log($log); + $data = parent::drop(); + $this->logAfter(); + return $data; } /** @@ -179,9 +176,10 @@ public function ensureIndex(array $keys, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use ($keys, $options) { - return parent::ensureIndex($keys, $options); - }); + $this->log($log); + $data = parent::ensureIndex($keys, $options); + $this->logAfter(); + return $data; } /** @@ -195,9 +193,10 @@ public function find(array $query = array(), array $fields = array()) 'fields' => $fields, ); - return $this->logMethod($log, function () use ($query, $fields) { - return parent::find($query, $fields); - }); + $this->log($log); + $data = parent::find($query, $fields); + $this->logAfter(); + return $data; } /** @@ -211,9 +210,10 @@ public function findOne(array $query = array(), array $fields = array()) 'fields' => $fields, ); - return $this->logMethod($log, function () use ($query, $fields) { - return parent::findOne($query, $fields); - }); + $this->log($log); + $data = parent::findOne($query, $fields); + $this->logAfter(); + return $data; } /** @@ -226,9 +226,10 @@ public function getDBRef(array $reference) 'reference' => $reference, ); - return $this->logMethod($log, function () use ($reference) { - return parent::getDBRef($reference); - }); + $this->log($log); + $data = parent::getDBRef($reference); + $this->logAfter(); + return $data; } /** @@ -244,9 +245,10 @@ public function group($keys, array $initial, $reduce, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use ($keys, $initial, $reduce, $options) { - return parent::group($keys, $initial, $reduce, $options); - }); + $this->log($log); + $data = parent::group($keys, $initial, $reduce, $options); + $this->logAfter(); + return $data; } /** @@ -260,9 +262,10 @@ public function insert(array &$a, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use (&$a, $options) { - return parent::insert($a, $options); - }); + $this->log($log); + $data = parent::insert($a, $options); + $this->logAfter(); + return $data; } /** @@ -276,9 +279,10 @@ public function remove(array $query, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use ($query, $options) { - return parent::remove($query, $options); - }); + $this->log($log); + $data = parent::remove($query, $options); + $this->logAfter(); + return $data; } /** @@ -292,9 +296,10 @@ public function save(array &$a, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use (&$a, $options) { - return parent::save($a, $options); - }); + $this->log($log); + $data = parent::save($a, $options); + $this->logAfter(); + return $data; } /** @@ -309,9 +314,10 @@ public function update($query, array $newObj, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use ($query, $newObj, $options) { - return parent::update($query, $newObj, $options); - }); + $this->log($log); + $data = parent::update($query, $newObj, $options); + $this->logAfter(); + return $data; } /** @@ -324,9 +330,10 @@ public function validate($scanData = false) 'scanData' => $scanData, ); - return $this->logMethod($log, function () use ($scanData) { - return parent::validate($scanData); - }); + $this->log($log); + $data = parent::validate($scanData); + $this->logAfter(); + return $data; } /** diff --git a/lib/Doctrine/MongoDB/LoggableCursor.php b/lib/Doctrine/MongoDB/LoggableCursor.php index 41a42249..1a6def3f 100644 --- a/lib/Doctrine/MongoDB/LoggableCursor.php +++ b/lib/Doctrine/MongoDB/LoggableCursor.php @@ -79,18 +79,10 @@ public function log(array $data) } } - /** - * @param array $log - * @param callable $callback - * @return mixed - */ - protected function logMethod($log, $callback) { - $this->log($log); - $data = call_user_func($callback); - if ($this->queryLogger instanceof Logging\QueryLogger) { + private function logAfter() { + if($this->queryLogger instanceof Logging\QueryLogger){ $this->queryLogger->stopQuery(); } - return $data; } /** @@ -113,9 +105,10 @@ public function hint(array $keyPattern) 'keyPattern' => $keyPattern, ); - return $this->logMethod($log, function () use ($keyPattern) { - return parent::hint($keyPattern); - }); + $this->log($log); + $data = parent::hint($keyPattern); + $this->logAfter(); + return $data; } /** @@ -128,9 +121,10 @@ public function limit($num) 'limitNum' => $num, ); - return $this->logMethod($log, function () use ($num) { - return parent::limit($num); - }); + $this->log($log); + $data = parent::limit($num); + $this->logAfter(); + return $data; } /** @@ -143,9 +137,10 @@ public function skip($num) 'skipNum' => $num, ); - return $this->logMethod($log, function () use ($num) { - return parent::skip($num); - }); + $this->log($log); + $data = parent::skip($num); + $this->logAfter(); + return $data; } /** @@ -157,9 +152,10 @@ public function snapshot() 'snapshot' => true, ); - return $this->logMethod($log, function () { - return parent::snapshot(); - }); + $this->log($log); + $data = parent::snapshot(); + $this->logAfter(); + return $data; } /** @@ -172,8 +168,9 @@ public function sort($fields) 'sortFields' => $fields, ); - return $this->logMethod($log, function () use ($fields) { - return parent::sort($fields); - }); + $this->log($log); + $data = parent::sort($fields); + $this->logAfter(); + return $data; } } diff --git a/lib/Doctrine/MongoDB/LoggableDatabase.php b/lib/Doctrine/MongoDB/LoggableDatabase.php index dac7d487..f5537dba 100644 --- a/lib/Doctrine/MongoDB/LoggableDatabase.php +++ b/lib/Doctrine/MongoDB/LoggableDatabase.php @@ -80,18 +80,10 @@ public function log(array $log) } } - /** - * @param array $log - * @param callable $callback - * @return mixed - */ - protected function logMethod($log, $callback) { - $this->log($log); - $data = call_user_func($callback); + private function logAfter() { if($this->queryLogger instanceof Logging\QueryLogger){ $this->queryLogger->stopQuery(); } - return $data; } /** @@ -105,9 +97,10 @@ public function authenticate($username, $password) 'password' => $password, ); - return $this->logMethod($log, function () use ($username, $password) { - return parent::authenticate($username, $password); - }); + $this->log($log); + $data = parent::authenticate($username, $password); + $this->logAfter(); + return $data; } /** @@ -121,9 +114,10 @@ public function command(array $data, array $options = array()) 'options' => $options, ); - return $this->logMethod($log, function () use ($data, $options) { - return parent::command($data, $options); - }); + $this->log($log); + $data = parent::command($data, $options); + $this->logAfter(); + return $data; } /** @@ -145,9 +139,10 @@ public function createCollection($name, $cappedOrOptions = false, $size = 0, $ma 'max' => $options['max'], ); - return $this->logMethod($log, function () use ($name, $options) { - return parent::createCollection($name, $options); - }); + $this->log($log); + $data = parent::createCollection($name, $options); + $this->logAfter(); + return $data; } /** @@ -157,10 +152,10 @@ public function drop() { $log = array('dropDatabase' => true); - return $this->logMethod($log, - function () { - return parent::drop(); - }); + $this->log($log); + $data = parent::drop(); + $this->logAfter(); + return $data; } /** @@ -174,9 +169,10 @@ public function execute($code, array $args = array()) 'args' => $args, ); - return $this->logMethod($log, function () use ($code, $args) { - return parent::execute($code, $args); - }); + $this->log($log); + $data = parent::execute($code, $args); + $this->logAfter(); + return $data; } /** @@ -189,9 +185,10 @@ public function getDBRef(array $ref) 'reference' => $ref, ); - return $this->logMethod($log, function () use ($ref) { - return parent::getDBRef($ref); - }); + $this->log($log); + $data = parent::getDBRef($ref); + $this->logAfter(); + return $data; } /** From 35faf6edeb3405684cc0b7bc04b6b6b2817b878d Mon Sep 17 00:00:00 2001 From: Martin Holzhauer Date: Thu, 6 Feb 2014 11:46:48 +0100 Subject: [PATCH 5/6] fixed small bug for logging and added unit tests for them --- lib/Doctrine/MongoDB/LoggableCollection.php | 6 +- lib/Doctrine/MongoDB/LoggableCursor.php | 6 +- lib/Doctrine/MongoDB/LoggableDatabase.php | 6 +- .../MongoDB/Tests/LoggableDatabaseTest.php | 83 ++++++++++++++++++- 4 files changed, 91 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/MongoDB/LoggableCollection.php b/lib/Doctrine/MongoDB/LoggableCollection.php index 38aced6a..ccdeac4c 100644 --- a/lib/Doctrine/MongoDB/LoggableCollection.php +++ b/lib/Doctrine/MongoDB/LoggableCollection.php @@ -52,10 +52,10 @@ class LoggableCollection extends Collection implements Loggable * @param callable $loggerCallable The logger callable * @param Logging\QueryLogger $queryLogger The QueryLogger object */ - public function __construct(Database $database, \MongoCollection $mongoCollection, EventManager $evm, $numRetries, $loggerCallable, Logging\QueryLogger $queryLogger = null) + public function __construct(Database $database, \MongoCollection $mongoCollection, EventManager $evm, $numRetries, $loggerCallable = null, Logging\QueryLogger $queryLogger = null) { - if ( ! is_callable($loggerCallable)) { - throw new \InvalidArgumentException('$loggerCallable must be a valid callback'); + if ( ! is_callable($loggerCallable) && !($queryLogger instanceof Logging\QueryLogger)) { + throw new \InvalidArgumentException('$loggerCallable must be a valid callback or $queryLogger must be an instance of Doctrine\MongoDB\Logging\QueryLogger'); } $this->loggerCallable = $loggerCallable; $this->queryLogger = $queryLogger; diff --git a/lib/Doctrine/MongoDB/LoggableCursor.php b/lib/Doctrine/MongoDB/LoggableCursor.php index 1a6def3f..80ac7d9f 100644 --- a/lib/Doctrine/MongoDB/LoggableCursor.php +++ b/lib/Doctrine/MongoDB/LoggableCursor.php @@ -50,10 +50,10 @@ class LoggableCursor extends Cursor implements Loggable * @param callable $loggerCallable Logger callable * @param Logging\QueryLogger $queryLogger QueryLogger object */ - public function __construct(Collection $collection, \MongoCursor $mongoCursor, array $query, array $fields, $numRetries, $loggerCallable, Logging\QueryLogger $queryLogger = null) + public function __construct(Collection $collection, \MongoCursor $mongoCursor, array $query, array $fields, $numRetries, $loggerCallable = null, Logging\QueryLogger $queryLogger = null) { - if ( ! is_callable($loggerCallable)) { - throw new \InvalidArgumentException('$loggerCallable must be a valid callback'); + if ( ! is_callable($loggerCallable) && !($queryLogger instanceof Logging\QueryLogger)) { + throw new \InvalidArgumentException('$loggerCallable must be a valid callback or $queryLogger must be an instance of Doctrine\MongoDB\Logging\QueryLogger'); } $this->loggerCallable = $loggerCallable; parent::__construct($collection, $mongoCursor, $query, $fields, $numRetries); diff --git a/lib/Doctrine/MongoDB/LoggableDatabase.php b/lib/Doctrine/MongoDB/LoggableDatabase.php index f5537dba..a2e8c9b3 100644 --- a/lib/Doctrine/MongoDB/LoggableDatabase.php +++ b/lib/Doctrine/MongoDB/LoggableDatabase.php @@ -52,10 +52,10 @@ class LoggableDatabase extends Database implements Loggable * @param callable $loggerCallable The logger callable * @param Logging\QueryLogger $queryLogger The QueryLogger object */ - public function __construct(Connection $connection, \MongoDB $mongoDB, EventManager $evm, $numRetries, $loggerCallable, Logging\QueryLogger $queryLogger = null) + public function __construct(Connection $connection, \MongoDB $mongoDB, EventManager $evm, $numRetries, $loggerCallable = null, Logging\QueryLogger $queryLogger = null) { - if ( ! is_callable($loggerCallable)) { - throw new \InvalidArgumentException('$loggerCallable must be a valid callback'); + if ( ! is_callable($loggerCallable) && !($queryLogger instanceof Logging\QueryLogger)) { + throw new \InvalidArgumentException('$loggerCallable must be a valid callback or $queryLogger must be an instance of Doctrine\MongoDB\Logging\QueryLogger'); } parent::__construct($connection, $mongoDB, $evm, $numRetries); $this->loggerCallable = $loggerCallable; diff --git a/tests/Doctrine/MongoDB/Tests/LoggableDatabaseTest.php b/tests/Doctrine/MongoDB/Tests/LoggableDatabaseTest.php index eaf08c61..86fb829c 100644 --- a/tests/Doctrine/MongoDB/Tests/LoggableDatabaseTest.php +++ b/tests/Doctrine/MongoDB/Tests/LoggableDatabaseTest.php @@ -22,6 +22,66 @@ public function testLog() $this->assertEquals(array('db' => self::databaseName, 'test' => 'test'), $called); } + public function testClosureLogging() + { + $called = false; + + $loggerCallable = function($msg) use (&$called) { + $called = $msg; + }; + + $db = $this->getTestLoggableDatabase($loggerCallable); + $db->execute('{}'); + + $this->assertInternalType('array', $called); + $this->assertArrayHasKey('execute', $called); + $this->assertArrayHasKey('code', $called); + $this->assertArrayHasKey('args', $called); + $this->assertArrayHasKey('db', $called); + + $this->assertEquals('{}', $called['code']); + $this->assertEquals(self::databaseName, $called['db']); + } + + public function testQueryLogger() + { + $logger = $this->getMockBuilder('Doctrine\MongoDB\Logging\QueryLogger') + ->disableOriginalConstructor() + ->getMock(); + + $db = $this->getTestQueryLoggerEnabledLoggableDatabase($logger); + + $called = null; + + $logger->expects($this->once()) + ->method('startQuery') + ->will($this->returnCallback(function($log)use(&$called){ + $called = $log; + })); + + $logger->expects($this->once()) + ->method('stopQuery'); + + $db->execute('{}'); + + $this->assertInternalType('array', $called); + $this->assertArrayHasKey('execute', $called); + $this->assertArrayHasKey('code', $called); + $this->assertArrayHasKey('args', $called); + $this->assertArrayHasKey('db', $called); + + $this->assertEquals('{}', $called['code']); + $this->assertEquals(self::databaseName, $called['db']); + } + + /** + * @expectedException \InvalidArgumentException + */ + public function testNoLoggerGivenThrowsException() + { + $db = $this->getTestLoggableDatabase(null); + } + private function getTestLoggableDatabase($loggerCallable) { $connection = $this->getMockBuilder('Doctrine\MongoDB\Connection') @@ -42,4 +102,25 @@ private function getTestLoggableDatabase($loggerCallable) return new LoggableDatabase($connection, $mongoDB, $eventManager, 0, $loggerCallable); } -} + + private function getTestQueryLoggerEnabledLoggableDatabase($logger) + { + $connection = $this->getMockBuilder('Doctrine\MongoDB\Connection') + ->disableOriginalConstructor() + ->getMock(); + + $mongoDB = $this->getMockBuilder('MongoDB') + ->disableOriginalConstructor() + ->getMock(); + + $mongoDB->expects($this->any()) + ->method('__toString') + ->will($this->returnValue(self::databaseName)); + + $eventManager = $this->getMockBuilder('Doctrine\Common\EventManager') + ->disableOriginalConstructor() + ->getMock(); + + return new LoggableDatabase($connection, $mongoDB, $eventManager, 0, null, $logger); + } +} \ No newline at end of file From 715a1f7d73c00ef9f12954c243d9edfdece7b716 Mon Sep 17 00:00:00 2001 From: Martin Holzhauer Date: Sat, 8 Feb 2014 19:44:14 +0100 Subject: [PATCH 6/6] code style fixes --- lib/Doctrine/MongoDB/LoggableCollection.php | 9 +++++---- lib/Doctrine/MongoDB/LoggableCursor.php | 7 ++++--- lib/Doctrine/MongoDB/LoggableDatabase.php | 9 +++++---- lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php | 5 +++-- lib/Doctrine/MongoDB/Logging/LoggerChain.php | 12 ++++++++++-- lib/Doctrine/MongoDB/Logging/QueryLogger.php | 3 ++- 6 files changed, 29 insertions(+), 16 deletions(-) diff --git a/lib/Doctrine/MongoDB/LoggableCollection.php b/lib/Doctrine/MongoDB/LoggableCollection.php index ccdeac4c..63b00031 100644 --- a/lib/Doctrine/MongoDB/LoggableCollection.php +++ b/lib/Doctrine/MongoDB/LoggableCollection.php @@ -72,17 +72,18 @@ public function log(array $log) { $log['db'] = $this->database->getName(); $log['collection'] = $this->getName(); - if($this->loggerCallable){ + if ($this->loggerCallable) { call_user_func_array($this->loggerCallable, array($log)); } - if($this->queryLogger instanceof Logging\QueryLogger){ + if ($this->queryLogger instanceof Logging\QueryLogger) { $this->queryLogger->startQuery($log); } } - private function logAfter() { - if($this->queryLogger instanceof Logging\QueryLogger){ + private function logAfter() + { + if ($this->queryLogger instanceof Logging\QueryLogger) { $this->queryLogger->stopQuery(); } } diff --git a/lib/Doctrine/MongoDB/LoggableCursor.php b/lib/Doctrine/MongoDB/LoggableCursor.php index 80ac7d9f..4e1d2847 100644 --- a/lib/Doctrine/MongoDB/LoggableCursor.php +++ b/lib/Doctrine/MongoDB/LoggableCursor.php @@ -74,13 +74,14 @@ public function log(array $data) call_user_func($this->loggerCallable, $data); } - if($this->queryLogger instanceof Logging\QueryLogger){ + if ($this->queryLogger instanceof Logging\QueryLogger) { $this->queryLogger->startQuery($data); } } - private function logAfter() { - if($this->queryLogger instanceof Logging\QueryLogger){ + private function logAfter() + { + if ($this->queryLogger instanceof Logging\QueryLogger) { $this->queryLogger->stopQuery(); } } diff --git a/lib/Doctrine/MongoDB/LoggableDatabase.php b/lib/Doctrine/MongoDB/LoggableDatabase.php index a2e8c9b3..f62fb4dd 100644 --- a/lib/Doctrine/MongoDB/LoggableDatabase.php +++ b/lib/Doctrine/MongoDB/LoggableDatabase.php @@ -71,17 +71,18 @@ public function __construct(Connection $connection, \MongoDB $mongoDB, EventMana public function log(array $log) { $log['db'] = $this->getName(); - if($this->loggerCallable){ + if ($this->loggerCallable) { call_user_func_array($this->loggerCallable, array($log)); } - if($this->queryLogger instanceof Logging\QueryLogger){ + if ($this->queryLogger instanceof Logging\QueryLogger) { $this->queryLogger->startQuery($log); } } - private function logAfter() { - if($this->queryLogger instanceof Logging\QueryLogger){ + private function logAfter() + { + if ($this->queryLogger instanceof Logging\QueryLogger) { $this->queryLogger->stopQuery(); } } diff --git a/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php b/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php index 08441bba..10f2df74 100644 --- a/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php +++ b/lib/Doctrine/MongoDB/Logging/EchoQueryLogger.php @@ -24,7 +24,8 @@ * * @author Martin Holzhauer */ -class EchoQueryLogger implements QueryLogger { +class EchoQueryLogger implements QueryLogger +{ public function startQuery($parameter) { @@ -34,6 +35,6 @@ public function startQuery($parameter) public function stopQuery() { - // TODO: Implement stopQuery() method. + // not needed in this case } } \ No newline at end of file diff --git a/lib/Doctrine/MongoDB/Logging/LoggerChain.php b/lib/Doctrine/MongoDB/Logging/LoggerChain.php index 5a93fa91..d2f1bc5c 100644 --- a/lib/Doctrine/MongoDB/Logging/LoggerChain.php +++ b/lib/Doctrine/MongoDB/Logging/LoggerChain.php @@ -24,7 +24,8 @@ * * @author Martin Holzhauer */ -class LoggerChain implements QueryLogger { +class LoggerChain implements QueryLogger +{ /** * @var array|QueryLogger[] @@ -36,10 +37,14 @@ class LoggerChain implements QueryLogger { * * @param QueryLogger $queryLogger */ - public function addLogger(QueryLogger $queryLogger){ + public function addLogger(QueryLogger $queryLogger) + { $this->logger[] = $queryLogger; } + /** + * @param $parameter + */ public function startQuery($parameter) { foreach($this->logger as $logger) @@ -48,6 +53,9 @@ public function startQuery($parameter) } } + /** + * + */ public function stopQuery() { foreach($this->logger as $logger) diff --git a/lib/Doctrine/MongoDB/Logging/QueryLogger.php b/lib/Doctrine/MongoDB/Logging/QueryLogger.php index 44b9d20f..653beeba 100644 --- a/lib/Doctrine/MongoDB/Logging/QueryLogger.php +++ b/lib/Doctrine/MongoDB/Logging/QueryLogger.php @@ -24,7 +24,8 @@ * * @author Martin Holzhauer */ -interface QueryLogger { +interface QueryLogger +{ public function startQuery($parameter); public function stopQuery();