From 0964c9084296d6f23ce2770b51387627aea5d765 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 13 Aug 2013 16:30:53 -0400 Subject: [PATCH 01/34] Query::execute() should use a copy of the $options property The Query object is immutable, so this shouldn't make a difference in practice. --- lib/Doctrine/MongoDB/Query/Query.php | 38 +++++++++++++++------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 31c2a3f9..62a7ae86 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -149,6 +149,8 @@ public function debug($name = null) */ public function execute() { + $options = $this->options; + switch ($this->query['type']) { case self::TYPE_FIND: $cursor = $this->collection->find($this->query['query'], $this->query['select']); @@ -156,49 +158,49 @@ public function execute() case self::TYPE_FIND_AND_UPDATE: if ($this->query['sort']) { - $this->options['sort'] = $this->query['sort']; + $options['sort'] = $this->query['sort']; } if ($this->query['select']) { - $this->options['fields'] = $this->query['select']; + $options['fields'] = $this->query['select']; } if ($this->query['upsert']) { - $this->options['upsert'] = true; + $options['upsert'] = true; } if ($this->query['new']) { - $this->options['new'] = true; + $options['new'] = true; } - return $this->collection->findAndUpdate($this->query['query'], $this->query['newObj'], $this->options); + return $this->collection->findAndUpdate($this->query['query'], $this->query['newObj'], $options); case self::TYPE_FIND_AND_REMOVE: if ($this->query['sort']) { - $this->options['sort'] = $this->query['sort']; + $options['sort'] = $this->query['sort']; } if ($this->query['select']) { - $this->options['fields'] = $this->query['select']; + $options['fields'] = $this->query['select']; } - return $this->collection->findAndRemove($this->query['query'], $this->options); + return $this->collection->findAndRemove($this->query['query'], $options); case self::TYPE_INSERT: return $this->collection->insert($this->query['newObj']); case self::TYPE_UPDATE: if ($this->query['upsert']) { - $this->options['upsert'] = $this->query['upsert']; + $options['upsert'] = $this->query['upsert']; } if ($this->query['multiple']) { - $this->options['multiple'] = $this->query['multiple']; + $options['multiple'] = $this->query['multiple']; } - return $this->collection->update($this->query['query'], $this->query['newObj'], $this->options); + return $this->collection->update($this->query['query'], $this->query['newObj'], $options); case self::TYPE_REMOVE: - return $this->collection->remove($this->query['query'], $this->options); + return $this->collection->remove($this->query['query'], $options); case self::TYPE_GROUP: if (!empty($this->query['query'])) { $this->query['group']['options']['condition'] = $this->query['query']; } - $options = array_merge($this->options, $this->query['group']['options']); + $options = array_merge($options, $this->query['group']['options']); return $this->collection->group($this->query['group']['keys'], $this->query['group']['initial'], $this->query['group']['reduce'], $options); @@ -207,7 +209,7 @@ public function execute() $this->query['mapReduce']['out'] = array('inline' => true); } - $options = array_merge($this->options, $this->query['mapReduce']['options']); + $options = array_merge($options, $this->query['mapReduce']['options']); $cursor = $this->collection->mapReduce($this->query['mapReduce']['map'], $this->query['mapReduce']['reduce'], $this->query['mapReduce']['out'], $this->query['query'], $options); if ($cursor instanceof Cursor) { @@ -217,20 +219,20 @@ public function execute() return $cursor; case self::TYPE_DISTINCT_FIELD: - return $this->collection->distinct($this->query['distinctField'], $this->query['query'], $this->options); + return $this->collection->distinct($this->query['distinctField'], $this->query['query'], $options); case self::TYPE_GEO_NEAR: if (isset($this->query['limit']) && $this->query['limit']) { - $this->options['num'] = $this->query['limit']; + $options['num'] = $this->query['limit']; } foreach (array('distanceMultiplier', 'maxDistance', 'spherical') as $key) { if (isset($this->query['geoNear'][$key]) && $this->query['geoNear'][$key]) { - $this->options[$key] = $this->query['geoNear'][$key]; + $options[$key] = $this->query['geoNear'][$key]; } } - return $this->collection->near($this->query['geoNear']['near'], $this->query['query'], $this->options); + return $this->collection->near($this->query['geoNear']['near'], $this->query['query'], $options); case self::TYPE_COUNT: return $this->collection->count($this->query['query']); From 2e266d8ac19495646c61388953bdbc57508f0f2a Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 13 Aug 2013 17:02:38 -0400 Subject: [PATCH 02/34] Refactor Builder::sort() to not use recursion --- lib/Doctrine/MongoDB/Query/Builder.php | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 89f024c9..be883553 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -1308,22 +1308,20 @@ public function snapshot($bool = true) * field name (key) and order (value) pairs. * * @param array|string $fieldName Field name or array of field/order pairs - * @param string $order Field order (if one field is specified) + * @param int|string $order Field order (if one field is specified) * @return self */ public function sort($fieldName, $order = null) { - if (is_array($fieldName)) { - foreach ($fieldName as $fieldName => $order) { - $this->sort($fieldName, $order); - } - } else { + $fields = is_array($fieldName) ? $fieldName : array($fieldName => $order); + + foreach ($fields as $fieldName => $order) { if (is_string($order)) { $order = strtolower($order) === 'asc' ? 1 : -1; } - $order = (int) $order; - $this->query['sort'][$fieldName] = $order; + $this->query['sort'][$fieldName] = (int) $order; } + return $this; } From d6ec8e939b0fe4af1a30d582a3da4bffce1abf4a Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 13 Aug 2013 17:08:57 -0400 Subject: [PATCH 03/34] Do not filter out falsey values in Query/Builder debug() methods --- lib/Doctrine/MongoDB/Query/Builder.php | 11 +---------- lib/Doctrine/MongoDB/Query/Query.php | 11 +---------- 2 files changed, 2 insertions(+), 20 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index be883553..ee566480 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -235,16 +235,7 @@ public function count() */ public function debug($name = null) { - $debug = $this->query; - if ($name !== null) { - return $debug[$name]; - } - foreach ($debug as $key => $value) { - if ( ! $value) { - unset($debug[$key]); - } - } - return $debug; + return $name !== null ? $this->query[$name] : $this->query; } /** diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 62a7ae86..e6fffd3c 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -124,16 +124,7 @@ public function count($foundOnly = false) */ public function debug($name = null) { - $debug = $this->query['query']; - if ($name !== null) { - return $debug[$name]; - } - foreach ($debug as $key => $value) { - if ( ! $value) { - unset($debug[$key]); - } - } - return $debug; + return $name !== null ? $this->query['query'][$name] : $this->query['query']; } /** From 1ea8ddc066307f30b0a73cf49d19612285547081 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 15:02:08 -0400 Subject: [PATCH 04/34] Query::debug() should access the entire $query property This will require changes to ODM, as several of its tests expect this to access only $query['query'] contents. --- lib/Doctrine/MongoDB/Query/Query.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index e6fffd3c..9f29bf50 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -124,7 +124,7 @@ public function count($foundOnly = false) */ public function debug($name = null) { - return $name !== null ? $this->query['query'][$name] : $this->query['query']; + return $name !== null ? $this->query[$name] : $this->query; } /** From 16857904b2a04b898aebf578197beed28feeb5ec Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Tue, 13 Aug 2013 17:26:24 -0400 Subject: [PATCH 05/34] Support values and expressions for $pull in query builder Note: $pullAll only supports an array of values (no expressions), so it remains as-is. --- lib/Doctrine/MongoDB/Query/Builder.php | 10 +++++----- lib/Doctrine/MongoDB/Query/Expr.php | 8 ++++++-- .../Doctrine/MongoDB/Tests/Query/ExprTest.php | 18 ++++++++++++++++++ 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index ee566480..dbc1f24f 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -1025,17 +1025,17 @@ public function popLast() } /** - * Remove all elements matching the given value from the current array - * field. + * Remove all elements matching the given value or expression from the + * current array field. * * @see Expr::pull() * @see http://docs.mongodb.org/manual/reference/operator/pull/ - * @param mixed $value + * @param mixed|Expr $valueOrExpression * @return self */ - public function pull($value) + public function pull($valueOrExpression) { - $this->expr->pull($value); + $this->expr->pull($valueOrExpression); return $this; } diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index a1e73b17..8e9dcb15 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -466,10 +466,14 @@ public function popLast() return $this; } - public function pull($value) + public function pull($valueOrExpression) { + if ($valueOrExpression instanceof Expr) { + $valueOrExpression = $valueOrExpression->getQuery(); + } + $this->requiresCurrentField(); - $this->newObj[$this->cmd . 'pull'][$this->currentField] = $value; + $this->newObj[$this->cmd . 'pull'][$this->currentField] = $valueOrExpression; return $this; } diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index e772043f..1a9f768a 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -122,6 +122,24 @@ public function testNearSphereWithLegacyCoordinates() $this->assertEquals(array('$nearSphere' => array(1, 2)), $expr->getQuery()); } + public function testPullWithValue() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a')->pull(1)); + $this->assertEquals(array('$pull' => array('a' => 1)), $expr->getNewObj()); + } + + public function testPullWithExpression() + { + $expr = new Expr('$'); + $nestedExpr = new Expr('$'); + $nestedExpr->gt(3); + + $this->assertSame($expr, $expr->field('a')->pull($nestedExpr)); + $this->assertEquals(array('$pull' => array('a' => array('$gt' => 3))), $expr->getNewObj()); + } + /** * @dataProvider provideGeoJsonPolygon */ From 0d10218824847f5300790962233dacab9c5e8db4 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 11:40:38 -0400 Subject: [PATCH 06/34] Rename argument for Expr pullAll/pushAll methods --- lib/Doctrine/MongoDB/Query/Expr.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 8e9dcb15..3d65ab53 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -477,10 +477,10 @@ public function pull($valueOrExpression) return $this; } - public function pullAll(array $valueArray) + public function pullAll(array $values) { $this->requiresCurrentField(); - $this->newObj[$this->cmd . 'pullAll'][$this->currentField] = $valueArray; + $this->newObj[$this->cmd . 'pullAll'][$this->currentField] = $values; return $this; } @@ -491,10 +491,10 @@ public function push($value) return $this; } - public function pushAll(array $valueArray) + public function pushAll(array $values) { $this->requiresCurrentField(); - $this->newObj[$this->cmd . 'pushAll'][$this->currentField] = $valueArray; + $this->newObj[$this->cmd . 'pushAll'][$this->currentField] = $values; return $this; } From f32e433318f41b4cfc927667ef5e25e2bd2481c1 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 13:45:41 -0400 Subject: [PATCH 07/34] Remove recursive merging in Expr::addManyToSet() The recursive merging in addManyToSet() dates back to: doctrine/mongodb-odm@59a2da0bf7a53075f9d93b9b1327abccf561c1d1. This could be problematic if addManyToSet() was called multiple times with deep array values, as they could be merged together in PHP. The new behavior will just overwrite the existing value in $newObj, like other update operators. --- lib/Doctrine/MongoDB/Query/Builder.php | 5 +++-- lib/Doctrine/MongoDB/Query/Expr.php | 9 ++------- tests/Doctrine/MongoDB/Tests/Query/ExprTest.php | 8 ++++++++ 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index dbc1f24f..2766b2c9 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -133,8 +133,9 @@ public function addAnd($expression) * Append multiple values to the current array field only if they do not * already exist in the array. * - * If the field does not exist, it will be set to an array containing this - * value. If the field is not an array, the query will yield an error. + * If the field does not exist, it will be set to an array containing the + * unique values in the argument. If the field is not an array, the query + * will yield an error. * * @see Expr::addManyToSet() * @see http://docs.mongodb.org/manual/reference/operator/addToSet/ diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 3d65ab53..7b7b8de1 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -79,13 +79,8 @@ public function addAnd($expression) public function addManyToSet(array $values) { $this->requiresCurrentField(); - if ( ! isset($this->newObj[$this->cmd . 'addToSet'][$this->currentField])) { - $this->newObj[$this->cmd . 'addToSet'][$this->currentField][$this->cmd . 'each'] = array(); - } - if ( ! is_array($this->newObj[$this->cmd . 'addToSet'][$this->currentField])) { - $this->newObj[$this->cmd . 'addToSet'][$this->currentField] = array($this->cmd . 'each' => array($this->newObj[$this->cmd . 'addToSet'][$this->currentField])); - } - $this->newObj[$this->cmd . 'addToSet'][$this->currentField][$this->cmd . 'each'] = array_merge_recursive($this->newObj[$this->cmd . 'addToSet'][$this->currentField][$this->cmd . 'each'], $values); + $this->newObj[$this->cmd . 'addToSet'][$this->currentField] = array($this->cmd . 'each' => $values); + return $this; } public function addNor($expression) diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index 1a9f768a..00761186 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -6,6 +6,14 @@ class ExprTest extends \PHPUnit_Framework_TestCase { + public function testAddManyToSet() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a')->addManyToSet(array(1, 2))); + $this->assertEquals(array('$addToSet' => array('a' => array('$each' => array(1, 2)))), $expr->getNewObj()); + } + public function testOperatorWithCurrentField() { $expr = new Expr('$'); From c159d274f1da9ea792130d4ee2cd61e7b07feccd Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 13:55:27 -0400 Subject: [PATCH 08/34] Implement Expr::each() and allow it to be used with addToSet() --- lib/Doctrine/MongoDB/Query/Builder.php | 19 ++++++++++++------- lib/Doctrine/MongoDB/Query/Expr.php | 13 +++++++++++-- .../MongoDB/Tests/Query/BuilderTest.php | 3 ++- .../Doctrine/MongoDB/Tests/Query/ExprTest.php | 18 ++++++++++++++++++ 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 2766b2c9..71321502 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -182,20 +182,25 @@ public function addOr($expression) } /** - * Append a value to the current array field only if it does not already - * exist in the array. + * Append one or more values to the current array field only if they do not + * already exist in the array. * - * If the field does not exist, it will be set to an array containing this - * value. If the field is not an array, the query will yield an error. + * If the field does not exist, it will be set to an array containing the + * unique value(s) in the argument. If the field is not an array, the query + * will yield an error. + * + * Multiple values may be specified by provided an Expr object and using + * {@link Expr::each()}. * * @see Expr::addToSet() * @see http://docs.mongodb.org/manual/reference/operator/addToSet/ - * @param mixed $value + * @see http://docs.mongodb.org/manual/reference/operator/each/ + * @param mixed|Expr $valueOrExpression * @return self */ - public function addToSet($value) + public function addToSet($valueOrExpression) { - $this->expr->addToSet($value); + $this->expr->addToSet($valueOrExpression); return $this; } diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 7b7b8de1..ee4c69ab 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -101,10 +101,14 @@ public function addOr($expression) return $this; } - public function addToSet($value) + public function addToSet($valueOrExpression) { + if ($valueOrExpression instanceof Expr) { + $valueOrExpression = $valueOrExpression->getQuery(); + } + $this->requiresCurrentField(); - $this->newObj[$this->cmd . 'addToSet'][$this->currentField] = $value; + $this->newObj[$this->cmd . 'addToSet'][$this->currentField] = $valueOrExpression; return $this; } @@ -113,6 +117,11 @@ public function all($values) return $this->operator($this->cmd . 'all', (array) $values); } + public function each(array $values) + { + return $this->operator($this->cmd . 'each', $values); + } + public function elemMatch($expression) { if ($expression instanceof Expr) { diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index 426b51d6..d7b3e9a1 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -348,7 +348,8 @@ public function provideProxiedExprMethods() 'unsetField()' => array('unsetField'), 'push()' => array('push', array('value')), 'pushAll()' => array('pushAll', array(array('value1', 'value2'))), - 'addToSet()' => array('addToSet', array('value')), + 'addToSet() with value' => array('addToSet', array('value')), + 'addToSet() with Expr' => array('addToSet', array($this->getMockExpr())), 'addManyToSet()' => array('addManyToSet', array(array('value1', 'value2'))), 'popFirst()' => array('popFirst'), 'popLast()' => array('popLast'), diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index 00761186..b4166316 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -14,6 +14,24 @@ public function testAddManyToSet() $this->assertEquals(array('$addToSet' => array('a' => array('$each' => array(1, 2)))), $expr->getNewObj()); } + public function testAddToSetWithValue() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a')->addToSet(1)); + $this->assertEquals(array('$addToSet' => array('a' => 1)), $expr->getNewObj()); + } + + public function testAddToSetWithExpression() + { + $expr = new Expr('$'); + $eachExpr = new Expr('$'); + $eachExpr->each(array(1, 2)); + + $this->assertSame($expr, $expr->field('a')->addToSet($eachExpr)); + $this->assertEquals(array('$addToSet' => array('a' => array('$each' => array(1, 2)))), $expr->getNewObj()); + } + public function testOperatorWithCurrentField() { $expr = new Expr('$'); From 5999f1505a0053b9c890e1eb8c6e87554e8d3e36 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 13:56:02 -0400 Subject: [PATCH 09/34] Deprecate Expr::addManyToSet() in favor of addToSet() and each() --- lib/Doctrine/MongoDB/Query/Builder.php | 1 + lib/Doctrine/MongoDB/Query/Expr.php | 3 +++ 2 files changed, 4 insertions(+) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 71321502..8d845cf9 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -137,6 +137,7 @@ public function addAnd($expression) * unique values in the argument. If the field is not an array, the query * will yield an error. * + * @deprecated 1.1 Use {@link Builder::addToSet()} with {@link Expr::each()}; Will be removed in 2.0 * @see Expr::addManyToSet() * @see http://docs.mongodb.org/manual/reference/operator/addToSet/ * @see http://docs.mongodb.org/manual/reference/operator/each/ diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index ee4c69ab..3dc04ebf 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -76,6 +76,9 @@ public function addAnd($expression) return $this; } + /** + * @deprecated 1.1 Use {@link Expr::addToSet()} with {@link Expr::each()}; Will be removed in 2.0 + */ public function addManyToSet(array $values) { $this->requiresCurrentField(); From 0d88e84ff86897549d3b9735907d0eed70cc9b8e Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 14:19:33 -0400 Subject: [PATCH 10/34] Support $each/$slice/$sort operators with push() MongoDB 2.4 deprecates the $pushAll operator in favor of $push with $each. Additionally, it adds support for $slice and $sort (only in combination with $each). --- lib/Doctrine/MongoDB/Query/Builder.php | 28 ++++++++++---- lib/Doctrine/MongoDB/Query/Expr.php | 37 ++++++++++++++++++- .../MongoDB/Tests/Query/BuilderTest.php | 3 +- .../Doctrine/MongoDB/Tests/Query/ExprTest.php | 29 +++++++++++++++ 4 files changed, 86 insertions(+), 11 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 8d845cf9..8245da1e 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -1062,27 +1062,39 @@ public function pullAll(array $values) } /** - * Append a value to the current array field. + * Append one or more values to the current array field. * - * If the field does not exist, it will be set to an array containing this - * value. If the field is not an array, the query will yield an error. + * If the field does not exist, it will be set to an array containing the + * value(s) in the argument. If the field is not an array, the query + * will yield an error. + * + * Multiple values may be specified by providing an Expr object and using + * {@link Expr::each()}. {@link Expr::slice()} and {@link Expr::sort()} may + * also be used to limit and order array elements, respectively. * * @see Expr::push() * @see http://docs.mongodb.org/manual/reference/operator/push/ - * @param mixed $value + * @see http://docs.mongodb.org/manual/reference/operator/each/ + * @see http://docs.mongodb.org/manual/reference/operator/slice/ + * @see http://docs.mongodb.org/manual/reference/operator/sort/ + * @param mixed|Expr $valueOrExpression * @return self */ - public function push($value) + public function push($valueOrExpression) { - $this->expr->push($value); + $this->expr->push($valueOrExpression); return $this; } /** * Append multiple values to the current array field. * - * If the field does not exist, it will be set to an array containing these - * values. If the field is not an array, the query will yield an error. + * If the field does not exist, it will be set to an array containing the + * values in the argument. If the field is not an array, the query will + * yield an error. + * + * This operator is deprecated in MongoDB 2.4. {@link Builder::push()} and + * {@link Expr::each()} should be used in its place. * * @see Expr::pushAll() * @see http://docs.mongodb.org/manual/reference/operator/pushAll/ diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 3dc04ebf..dbcbafca 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -491,10 +491,14 @@ public function pullAll(array $values) return $this; } - public function push($value) + public function push($valueOrExpression) { + if ($valueOrExpression instanceof Expr) { + $valueOrExpression = $valueOrExpression->getQuery(); + } + $this->requiresCurrentField(); - $this->newObj[$this->cmd . 'push'][$this->currentField] = $value; + $this->newObj[$this->cmd . 'push'][$this->currentField] = $valueOrExpression; return $this; } @@ -535,6 +539,35 @@ public function size($size) return $this->operator($this->cmd . 'size', $size); } + public function slice($slice) + { + return $this->operator($this->cmd . 'slice', $slice); + } + + /** + * Set one or more field/order pairs for the sort operator. + * + * If sorting by multiple fields, the first argument should be an array of + * field name (key) and order (value) pairs. + * + * @param array|string $fieldName Field name or array of field/order pairs + * @param int|string $order Field order (if one field is specified) + * @return self + */ + public function sort($fieldName, $order = null) + { + $fields = is_array($fieldName) ? $fieldName : array($fieldName => $order); + + foreach ($fields as $fieldName => $order) { + if (is_string($order)) { + $order = strtolower($order) === 'asc' ? 1 : -1; + } + $sort[$fieldName] = (int) $order; + } + + return $this->operator($this->cmd . 'sort', $sort); + } + public function type($type) { $map = array( diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index d7b3e9a1..3828fab8 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -346,7 +346,8 @@ public function provideProxiedExprMethods() 'geoWithinPolygon()' => array('geoWithinPolygon', array(array(0, 0), array(1, 1), array(1, 0))), 'inc()' => array('inc', array(1)), 'unsetField()' => array('unsetField'), - 'push()' => array('push', array('value')), + 'push() with value' => array('push', array('value')), + 'push() with Expr' => array('push', array($this->getMockExpr())), 'pushAll()' => array('pushAll', array(array('value1', 'value2'))), 'addToSet() with value' => array('addToSet', array('value')), 'addToSet() with Expr' => array('addToSet', array($this->getMockExpr())), diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index b4166316..6503ceed 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -166,6 +166,35 @@ public function testPullWithExpression() $this->assertEquals(array('$pull' => array('a' => array('$gt' => 3))), $expr->getNewObj()); } + public function testPushWithValue() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a')->push(1)); + $this->assertEquals(array('$push' => array('a' => 1)), $expr->getNewObj()); + } + + public function testPushWithExpression() + { + $expr = new Expr('$'); + $innerExpr = new Expr('$'); + $innerExpr + ->each(array(array('x' => 1), array('x' => 2))) + ->slice(-2) + ->sort('x', 1); + + $expectedNewObj = array( + '$push' => array('a' => array( + '$each' => array(array('x' => 1), array('x' => 2)), + '$slice' => -2, + '$sort' => array('x' => 1), + )), + ); + + $this->assertSame($expr, $expr->field('a')->push($innerExpr)); + $this->assertEquals($expectedNewObj, $expr->getNewObj()); + } + /** * @dataProvider provideGeoJsonPolygon */ From ac04fff4344bd3151488db1eff171c11bdc8cf7b Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 14:31:15 -0400 Subject: [PATCH 11/34] Expr::push() should ensure $each operator appears first If the $each operator does not appear first, the $push argument will be considered a single value to append to the array field. --- lib/Doctrine/MongoDB/Query/Expr.php | 5 ++++- .../Doctrine/MongoDB/Tests/Query/ExprTest.php | 21 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index dbcbafca..9b294ef2 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -494,7 +494,10 @@ public function pullAll(array $values) public function push($valueOrExpression) { if ($valueOrExpression instanceof Expr) { - $valueOrExpression = $valueOrExpression->getQuery(); + $valueOrExpression = array_merge( + array($this->cmd . 'each' => array()), + $valueOrExpression->getQuery() + ); } $this->requiresCurrentField(); diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index 6503ceed..0e0147b5 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -195,6 +195,27 @@ public function testPushWithExpression() $this->assertEquals($expectedNewObj, $expr->getNewObj()); } + public function testPushWithExpressionShouldEnsureEachOperatorAppearsFirst() + { + $expr = new Expr('$'); + $innerExpr = new Expr('$'); + $innerExpr + ->sort('x', 1) + ->slice(-2) + ->each(array(array('x' => 1), array('x' => 2))); + + $expectedNewObj = array( + '$push' => array('a' => array( + '$each' => array(array('x' => 1), array('x' => 2)), + '$sort' => array('x' => 1), + '$slice' => -2, + )), + ); + + $this->assertSame($expr, $expr->field('a')->push($innerExpr)); + $this->assertSame($expectedNewObj, $expr->getNewObj()); + } + /** * @dataProvider provideGeoJsonPolygon */ From 975b2470dfc43607626f99673e3e4be6c60cea6d Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 14:37:36 -0400 Subject: [PATCH 12/34] Test array/Expr arguments for logical operators and elemMatch --- lib/Doctrine/MongoDB/Query/Expr.php | 25 ++++--------------- .../MongoDB/Tests/Query/BuilderTest.php | 12 ++++++--- 2 files changed, 13 insertions(+), 24 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 9b294ef2..d7e8d492 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -69,10 +69,7 @@ public function __construct($cmd) public function addAnd($expression) { - if ($expression instanceof Expr) { - $expression = $expression->getQuery(); - } - $this->query[$this->cmd . 'and'][] = $expression; + $this->query[$this->cmd . 'and'][] = $expression instanceof Expr ? $expression->getQuery() : $expression; return $this; } @@ -88,19 +85,13 @@ public function addManyToSet(array $values) public function addNor($expression) { - if ($expression instanceof Expr) { - $expression = $expression->getQuery(); - } - $this->query[$this->cmd . 'nor'][] = $expression; + $this->query[$this->cmd . 'nor'][] = $expression instanceof Expr ? $expression->getQuery() : $expression; return $this; } public function addOr($expression) { - if ($expression instanceof Expr) { - $expression = $expression->getQuery(); - } - $this->query[$this->cmd . 'or'][] = $expression; + $this->query[$this->cmd . 'or'][] = $expression instanceof Expr ? $expression->getQuery() : $expression; return $this; } @@ -127,10 +118,7 @@ public function each(array $values) public function elemMatch($expression) { - if ($expression instanceof Expr) { - $expression = $expression->getQuery(); - } - return $this->operator($this->cmd . 'elemMatch', $expression); + return $this->operator($this->cmd . 'elemMatch', $expression instanceof Expr ? $expression->getQuery() : $expression); } public function equals($value) @@ -433,10 +421,7 @@ public function nearSphere($x, $y = null) public function not($expression) { - if ($expression instanceof Expr) { - $expression = $expression->getQuery(); - } - return $this->operator($this->cmd . 'not', $expression); + return $this->operator($this->cmd . 'not', $expression instanceof Expr ? $expression->getQuery() : $expression); } public function notEqual($value) diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index 3828fab8..08b4c7e6 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -356,10 +356,14 @@ public function provideProxiedExprMethods() 'popLast()' => array('popLast'), 'pull()' => array('pull', array('value')), 'pullAll()' => array('pullAll', array(array('value1', 'value2'))), - 'addAnd()' => array('addAnd', array($this->getMockExpr())), - 'addOr()' => array('addOr', array($this->getMockExpr())), - 'addNor()' => array('addNor', array($this->getMockExpr())), - 'elemMatch()' => array('elemMatch', array($this->getMockExpr())), + 'addAnd() array' => array('addAnd', array(array())), + 'addAnd() Expr' => array('addAnd', array($this->getMockExpr())), + 'addOr() array' => array('addOr', array(array())), + 'addOr() Expr' => array('addOr', array($this->getMockExpr())), + 'addNor() array' => array('addNor', array(array())), + 'addNor() Expr' => array('addNor', array($this->getMockExpr())), + 'elemMatch() array' => array('elemMatch', array(array())), + 'elemMatch() Expr' => array('elemMatch', array($this->getMockExpr())), 'not()' => array('not', array($this->getMockExpr())), ); } From 8207985d3532b08c50c3102bd66dec23543e2710 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 16:53:02 -0400 Subject: [PATCH 13/34] Use casting and array type-hinting for method args This changes the public API for in/notIn and removes the array cast in notIn (behavior change); however, the methods are now consistent. Other methods that take a single scalar type can use casting to sanitize their input. --- lib/Doctrine/MongoDB/Query/Builder.php | 34 +++++++++++++------------- lib/Doctrine/MongoDB/Query/Expr.php | 12 ++++----- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 8245da1e..348ea8a3 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -284,7 +284,7 @@ public function distinct($field) */ public function eagerCursor($bool = true) { - $this->query['eagerCursor'] = $bool; + $this->query['eagerCursor'] = (boolean) $bool; return $this; } @@ -347,7 +347,7 @@ public function exclude($fieldName = null) */ public function exists($bool) { - $this->expr->exists($bool); + $this->expr->exists((boolean) $bool); return $this; } @@ -371,7 +371,7 @@ public function expr() */ public function field($field) { - $this->expr->field($field); + $this->expr->field((string) $field); return $this; } @@ -715,7 +715,7 @@ public function hint($keyPattern) */ public function immortal($bool = true) { - $this->query['immortal'] = $bool; + $this->query['immortal'] = (boolean) $bool; return $this; } @@ -727,7 +727,7 @@ public function immortal($bool = true) * @param array|mixed $values * @return self */ - public function in($values) + public function in(array $values) { $this->expr->in($values); return $this; @@ -772,7 +772,7 @@ public function insert() */ public function limit($limit) { - $this->query['limit'] = $limit; + $this->query['limit'] = (integer) $limit; return $this; } @@ -907,7 +907,7 @@ public function mod($mod) */ public function multiple($bool = true) { - $this->query['multiple'] = $bool; + $this->query['multiple'] = (boolean) $bool; return $this; } @@ -987,7 +987,7 @@ public function notEqual($value) * @param array|mixed $values * @return self */ - public function notIn($values) + public function notIn(array $values) { $this->expr->notIn($values); return $this; @@ -1168,7 +1168,7 @@ public function remove() */ public function returnNew($bool = true) { - $this->query['new'] = $bool; + $this->query['new'] = (boolean) $bool; return $this; } @@ -1248,7 +1248,7 @@ public function set($value, $atomic = true) if ($this->query['type'] == Query::TYPE_INSERT) { $atomic = false; } - $this->expr->set($value, $atomic); + $this->expr->set($value, (boolean) $atomic); return $this; } @@ -1262,7 +1262,7 @@ public function set($value, $atomic = true) */ public function size($size) { - $this->expr->size($size); + $this->expr->size((integer) $size); return $this; } @@ -1278,7 +1278,7 @@ public function size($size) */ public function skip($skip) { - $this->query['skip'] = $skip; + $this->query['skip'] = (integer) $skip; return $this; } @@ -1295,7 +1295,7 @@ public function skip($skip) */ public function slaveOkay($bool = true) { - $this->query['slaveOkay'] = $bool; + $this->query['slaveOkay'] = (boolean) $bool; return $this; } @@ -1307,7 +1307,7 @@ public function slaveOkay($bool = true) */ public function snapshot($bool = true) { - $this->query['snapshot'] = $bool; + $this->query['snapshot'] = (boolean) $bool; return $this; } @@ -1357,7 +1357,7 @@ public function spherical($spherical = true) * * @see Expr::type() * @see http://docs.mongodb.org/manual/reference/operator/type/ - * @param integer $type + * @param integer|string $type * @return self */ public function type($type) @@ -1400,7 +1400,7 @@ public function update() */ public function upsert($bool = true) { - $this->query['upsert'] = $bool; + $this->query['upsert'] = (boolean) $bool; return $this; } @@ -1409,7 +1409,7 @@ public function upsert($bool = true) * * @see Expr::where() * @see http://docs.mongodb.org/manual/reference/operator/where/ - * @param string $javascript + * @param string|\MongoCode $javascript * @return self */ public function where($javascript) diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index d7e8d492..ff6d7c9d 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -133,12 +133,12 @@ public function equals($value) public function exists($bool) { - return $this->operator($this->cmd . 'exists', $bool); + return $this->operator($this->cmd . 'exists', (boolean) $bool); } public function field($field) { - $this->currentField = $field; + $this->currentField = (string) $field; return $this; } @@ -303,7 +303,7 @@ public function gte($value) return $this->operator($this->cmd . 'gte', $value); } - public function in($values) + public function in(array $values) { return $this->operator($this->cmd . 'in', $values); } @@ -429,9 +429,9 @@ public function notEqual($value) return $this->operator($this->cmd . 'ne', $value); } - public function notIn($values) + public function notIn(array $values) { - return $this->operator($this->cmd . 'nin', (array) $values); + return $this->operator($this->cmd . 'nin', $values); } public function operator($operator, $value) @@ -524,7 +524,7 @@ public function set($value, $atomic = true) public function size($size) { - return $this->operator($this->cmd . 'size', $size); + return $this->operator($this->cmd . 'size', (integer) $size); } public function slice($slice) From 4ee53d6f5c5a84a15ad7cbece0a56b0bea2d88aa Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 17:34:55 -0400 Subject: [PATCH 14/34] Formatting Builder method docs --- lib/Doctrine/MongoDB/Query/Builder.php | 46 +++++++++++++------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 348ea8a3..aa7fb924 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -250,12 +250,12 @@ public function debug($name = null) * * @param float $distanceMultiplier * @return self - * @throws BadMethodCallException if the query is not a $geoNear command + * @throws BadMethodCallException if the query is not a geoNear command */ public function distanceMultiplier($distanceMultiplier) { if ($this->query['type'] !== Query::TYPE_GEO_NEAR) { - throw new BadMethodCallException('This method requires a $geoNear command (call geoNear() first)'); + throw new BadMethodCallException('This method requires a geoNear command (call geoNear() first)'); } $this->query['geoNear']['distanceMultiplier'] = $distanceMultiplier; @@ -376,11 +376,11 @@ public function field($field) } /** - * Set the finalize option for a mapReduce or group query. + * Set the "finalize" option for a mapReduce or group command. * * @param string|\MongoCode $finalize * @return self - * @throws \BadMethodCallException if the query type is unsupported + * @throws BadMethodCallException if the query is not a mapReduce or group command */ public function finalize($finalize) { @@ -394,7 +394,7 @@ public function finalize($finalize) break; default: - throw new \BadMethodCallException('mapReduce(), map() or group() must be called before finalize()'); + throw new BadMethodCallException('mapReduce(), map() or group() must be called before finalize()'); } return $this; @@ -635,7 +635,7 @@ public function setQueryArray(array $query) /** * Get the type of this query. * - * @return string $type + * @return integer $type */ public function getType() { @@ -645,7 +645,7 @@ public function getType() /** * Change the query type to a group command. * - * If the reduce option is not specified when calling this method, it must + * If the "reduce" option is not specified when calling this method, it must * be set with the {@link Builder::reduce()} method. * * @see http://docs.mongodb.org/manual/reference/command/group/ @@ -763,8 +763,8 @@ public function insert() /** * Set the limit for the query. * - * This is only relevant for find and geoNear queries, or mapReduce queries - * that store results in an output collecton and return a cursor. + * This is only relevant for find queries and geoNear and mapReduce + * commands. * * @see Query::prepareCursor() * @param integer $limit @@ -807,7 +807,7 @@ public function lte($value) /** * Change the query type to a mapReduce command. * - * The reduce option is not specified when calling this method; it must + * The "reduce" option is not specified when calling this method; it must * be set with the {@link Builder::reduce()} method. * * @see http://docs.mongodb.org/manual/reference/command/mapReduce/ @@ -844,7 +844,7 @@ public function mapReduce($map, $reduce, $out = array('inline' => true), array $ } /** - * Set additional options for a mapReduce query. + * Set additional options for a mapReduce command. * * @param array $options * @return self @@ -859,9 +859,9 @@ public function mapReduceOptions(array $options) * Set the "maxDistance" option for a geoNear command query or add * $maxDistance criteria to the query. * - * If the query type is geospatial (i.e. geoNear() was called), the - * "maxDistance" command option will be set; otherwise, $maxDistance will be - * added to the current expression. + * If the query is a geoNear command ({@link Expr::geoNear()} was called), + * the "maxDistance" command option will be set; otherwise, $maxDistance + * will be added to the current expression. * * If the query uses GeoJSON points, $maxDistance will be interpreted in * meters. If legacy point coordinates are used, $maxDistance will be @@ -900,7 +900,7 @@ public function mod($mod) } /** - * Set the multiple option for an update query. + * Set the "multiple" option for an update query. * * @param boolean $bool * @return self @@ -994,7 +994,7 @@ public function notIn(array $values) } /** - * Set the out option for a mapReduce query. + * Set the "out" option for a mapReduce command. * * @param array|string $out * @return self @@ -1125,11 +1125,11 @@ public function range($start, $end) } /** - * Set the reduce option for a mapReduce or group query. + * Set the "reduce" option for a mapReduce or group command. * * @param string|\MongoCode $reduce * @return self - * @throws \BadMethodCallException if the query type is unsupported + * @throws BadMethodCallException if the query is not a mapReduce or group command */ public function reduce($reduce) { @@ -1143,7 +1143,7 @@ public function reduce($reduce) break; default: - throw new \BadMethodCallException('mapReduce(), map() or group() must be called before reduce()'); + throw new BadMethodCallException('mapReduce(), map() or group() must be called before reduce()'); } return $this; @@ -1161,7 +1161,7 @@ public function remove() } /** - * Set the new option for a findAndUpdate query. + * Set the "new" option for a findAndUpdate command. * * @param boolean $bool * @return self @@ -1340,12 +1340,12 @@ public function sort($fieldName, $order = null) * * @param bool $spherical * @return self - * @throws BadMethodCallException if the query is not a $geoNear command + * @throws BadMethodCallException if the query is not a geoNear command */ public function spherical($spherical = true) { if ($this->query['type'] !== Query::TYPE_GEO_NEAR) { - throw new BadMethodCallException('This method requires a $geoNear command (call geoNear() first)'); + throw new BadMethodCallException('This method requires a geoNear command (call geoNear() first)'); } $this->query['geoNear']['spherical'] = $spherical; @@ -1393,7 +1393,7 @@ public function update() } /** - * Set the upsert option for an update or findAndUpdate query. + * Set the "upsert" option for an update or findAndUpdate query. * * @param boolean $bool * @return self From a512d7f55a3e1e1a28d147ca364b26898ffb3ce9 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 17:38:43 -0400 Subject: [PATCH 15/34] Expr::where() need not alter the current field --- lib/Doctrine/MongoDB/Query/Expr.php | 3 ++- tests/Doctrine/MongoDB/Tests/Query/ExprTest.php | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index ff6d7c9d..fbe88829 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -594,7 +594,8 @@ public function unsetField() public function where($javascript) { - return $this->field($this->cmd . 'where')->equals($javascript); + $this->query[$this->cmd . 'where'] = $javascript; + return $this; } /** diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index 0e0147b5..523d56f8 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -295,6 +295,15 @@ public function testGeoWithinPolygonRequiresAtLeastThreePoints() $expr->geoWithinPolygon(array(0, 0), array(1, 1)); } + public function testWhere() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->where('javascript')); + $this->assertEquals(array('$where' => 'javascript'), $expr->getQuery()); + $this->assertNull($expr->getCurrentField()); + } + public function testWithinBox() { $expr = new Expr('$'); From de44c4258516a2f3cd3770cdbfaeb133ba0fbde3 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 17:42:06 -0400 Subject: [PATCH 16/34] Builder mapReduceOptions/out methods should require mapReduce command --- lib/Doctrine/MongoDB/Query/Builder.php | 10 ++++++++++ .../MongoDB/Tests/Query/BuilderTest.php | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index aa7fb924..5e69de65 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -848,9 +848,14 @@ public function mapReduce($map, $reduce, $out = array('inline' => true), array $ * * @param array $options * @return self + * @throws BadMethodCallException if the query is not a mapReduce command */ public function mapReduceOptions(array $options) { + if ($this->query['type'] !== Query::TYPE_MAP_REDUCE) { + throw new BadMethodCallException('This method requires a mapReduce command (call map() or mapReduce() first)'); + } + $this->query['mapReduce']['options'] = $options; return $this; } @@ -998,9 +1003,14 @@ public function notIn(array $values) * * @param array|string $out * @return self + * @throws BadMethodCallException if the query is not a mapReduce command */ public function out($out) { + if ($this->query['type'] !== Query::TYPE_MAP_REDUCE) { + throw new BadMethodCallException('This method requires a mapReduce command (call map() or mapReduce() first)'); + } + $this->query['mapReduce']['out'] = $out; return $this; } diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index 08b4c7e6..a287d76c 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -401,6 +401,24 @@ public function testDistanceMultiplerRequiresGeoNearCommand() $qb->distanceMultiplier(1); } + /** + * @expectedException BadMethodCallException + */ + public function testMapReduceOptionsRequiresMapReduceCommand() + { + $qb = $this->getTestQueryBuilder(); + $qb->mapReduceOptions(array()); + } + + /** + * @expectedException BadMethodCallException + */ + public function testOutRequiresMapReduceCommand() + { + $qb = $this->getTestQueryBuilder(); + $qb->out('collection'); + } + /** * @expectedException BadMethodCallException */ From 08ade88a31784a1484d810e1e12776a84fe86d54 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 18:41:09 -0400 Subject: [PATCH 17/34] Support GeoJSON in Builder::geoNear() and set spherical default geoNear commands against 2dsphere indexes require the "spherical" option to be true, so we can infer its default based on whether or not the "near" option is GeoJSON (in both Builder and Collection). --- lib/Doctrine/MongoDB/Collection.php | 1 + lib/Doctrine/MongoDB/Query/Builder.php | 28 ++++++++--- .../Doctrine/MongoDB/Tests/CollectionTest.php | 11 +++-- .../MongoDB/Tests/Query/BuilderTest.php | 46 +++++++++++++++++++ 4 files changed, 74 insertions(+), 12 deletions(-) diff --git a/lib/Doctrine/MongoDB/Collection.php b/lib/Doctrine/MongoDB/Collection.php index e035497d..a3485f1c 100644 --- a/lib/Doctrine/MongoDB/Collection.php +++ b/lib/Doctrine/MongoDB/Collection.php @@ -1159,6 +1159,7 @@ protected function doNear($near, array $query, array $options) $command = array(); $command['geoNear'] = $this->getMongoCollection()->getName(); $command['near'] = $near; + $command['spherical'] = isset($near['type']); $command['query'] = (object) $query; $command = array_merge($command, $options); diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 5e69de65..c2a71d2f 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -453,21 +453,35 @@ public function geoIntersects($geometry) } /** - * Specify a geoNear command for this query. + * Change the query type to a geoNear command. + * + * A GeoJSON point may be provided as the first and only argument for + * 2dsphere queries. This single parameter may be a GeoJSON point object or + * an array corresponding to the point's JSON representation. If GeoJSON is + * used, the "spherical" option will default to true. * * This method sets the "near" option for the geoNear command. The "num" - * option may be set using limit(). The "distanceMultiplier" and - * "maxDistance" options may be set using their respective builder methods. - * Additional query criteria will be assigned to the "query" option. + * option may be set using {@link Expr::limit()}. The "distanceMultiplier", + * "maxDistance", and "spherical" options may be set using their respective + * builder methods. Additional query criteria will be assigned to the + * "query" option. * - * @param float $x + * @see http://docs.mongodb.org/manual/reference/command/geoNear/ + * @param float|array|Point $x * @param float $y * @return self */ - public function geoNear($x, $y) + public function geoNear($x, $y = null) { + if ($x instanceof Point) { + $x = $x->jsonSerialize(); + } + $this->query['type'] = Query::TYPE_GEO_NEAR; - $this->query['geoNear'] = array('near' => array($x, $y)); + $this->query['geoNear'] = array( + 'near' => is_array($x) ? $x : array($x, $y), + 'spherical' => is_array($x) && isset($x['type']), + ); return $this; } diff --git a/tests/Doctrine/MongoDB/Tests/CollectionTest.php b/tests/Doctrine/MongoDB/Tests/CollectionTest.php index 00b2377d..a3be7df0 100644 --- a/tests/Doctrine/MongoDB/Tests/CollectionTest.php +++ b/tests/Doctrine/MongoDB/Tests/CollectionTest.php @@ -653,7 +653,7 @@ public function testNearShouldThrowExceptionOnError() /** * @dataProvider providePoint */ - public function testNearWithGeoJsonPoint($point, array $expected) + public function testNear($point, array $near, $spherical) { $results = array( array('dis' => 1, 'obj' => array('_id' => 1, 'loc' => array(1, 0))), @@ -662,7 +662,8 @@ public function testNearWithGeoJsonPoint($point, array $expected) $command = array( 'geoNear' => self::collectionName, - 'near' => $expected, + 'near' => $near, + 'spherical' => $spherical, 'query' => new \stdClass(), ); @@ -684,9 +685,9 @@ public function providePoint() $json = array('type' => 'Point', 'coordinates' => $coordinates); return array( - 'legacy array' => array($coordinates, $coordinates), - 'GeoJSON array' => array($json, $json), - 'GeoJSON object' => array($this->getMockPoint($json), $json), + 'legacy array' => array($coordinates, $coordinates, false), + 'GeoJSON array' => array($json, $json, true), + 'GeoJSON object' => array($this->getMockPoint($json), $json, true), ); } diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index a287d76c..772e2380 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -392,6 +392,39 @@ public function testGeoNear() $this->assertEquals(10, $qb->debug('limit')); } + /** + * @dataProvider providePoint + */ + public function testGeoNearWithSingleArgument($point, array $near, $spherical) + { + $qb = $this->getTestQueryBuilder(); + + $this->assertSame($qb, $qb->geoNear($point)); + $this->assertEquals(Query::TYPE_GEO_NEAR, $qb->getType()); + $this->assertEquals(array('near' => $near, 'spherical' => $spherical), $qb->debug('geoNear')); + } + + public function providePoint() + { + $coordinates = array(0, 0); + $json = array('type' => 'Point', 'coordinates' => $coordinates); + + return array( + 'legacy array' => array($coordinates, $coordinates, false), + 'GeoJSON array' => array($json, $json, true), + 'GeoJSON object' => array($this->getMockPoint($json), $json, true), + ); + } + + public function testGeoNearWithBothArguments() + { + $qb = $this->getTestQueryBuilder(); + + $this->assertSame($qb, $qb->geoNear(array(0, 0))); + $this->assertEquals(Query::TYPE_GEO_NEAR, $qb->getType()); + $this->assertEquals(array('near' => array(0, 0), 'spherical' => false), $qb->debug('geoNear')); + } + /** * @expectedException BadMethodCallException */ @@ -568,6 +601,19 @@ private function getMockGeometry() ->getMock(); } + private function getMockPoint($json) + { + $point = $this->getMockBuilder('GeoJson\Geometry\Point') + ->disableOriginalConstructor() + ->getMock(); + + $point->expects($this->once()) + ->method('jsonSerialize') + ->will($this->returnValue($json)); + + return $point; + } + private function assertArrayHasKeyValue($expected, $array, $message = '') { foreach ((array) $expected as $key => $value) { From 5a31a2be9deeee224a008f52b1ecbe22ff517ad3 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 20:40:11 -0400 Subject: [PATCH 18/34] Make query type equality checks consistent --- lib/Doctrine/MongoDB/Query/Builder.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index c2a71d2f..b90197a8 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -896,7 +896,7 @@ public function mapReduceOptions(array $options) */ public function maxDistance($maxDistance) { - if (Query::TYPE_GEO_NEAR === $this->query['type']) { + if ($this->query['type'] === Query::TYPE_GEO_NEAR) { $this->query['geoNear']['maxDistance'] = $maxDistance; } else { $this->expr->maxDistance($maxDistance); @@ -1269,7 +1269,7 @@ public function selectSlice($fieldName, $countOrSkip, $limit = null) */ public function set($value, $atomic = true) { - if ($this->query['type'] == Query::TYPE_INSERT) { + if ($this->query['type'] === Query::TYPE_INSERT) { $atomic = false; } $this->expr->set($value, (boolean) $atomic); From 16a92eb1626da1b424908e757ac2cfbe39cc02bb Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 20:40:54 -0400 Subject: [PATCH 19/34] Builder::map() should init full query array, default to inline output This makes map() as functional as mapReduce(), apart from setting the "reduce" option. --- lib/Doctrine/MongoDB/Query/Builder.php | 9 ++++++++- tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php | 1 + 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index b90197a8..fe54502c 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -824,6 +824,8 @@ public function lte($value) * The "reduce" option is not specified when calling this method; it must * be set with the {@link Builder::reduce()} method. * + * The "out" option defaults to inline, like {@link Builder::mapReduce()}. + * * @see http://docs.mongodb.org/manual/reference/command/mapReduce/ * @param string|\MongoCode $map * @return self @@ -831,7 +833,12 @@ public function lte($value) public function map($map) { $this->query['type'] = Query::TYPE_MAP_REDUCE; - $this->query['mapReduce']['map'] = $map; + $this->query['mapReduce'] = array( + 'map' => $map, + 'reduce' => null, + 'out' => array('inline' => true), + 'options' => array(), + ); return $this; } diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index 772e2380..da51f080 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -78,6 +78,7 @@ public function testMapReduceQueryWithMultipleMethodsAndQueryArray() 'map' => $map, 'reduce' => $reduce, 'options' => array('finalize' => $finalize), + 'out' => array('inline' => true), ); $this->assertEquals(Query::TYPE_MAP_REDUCE, $qb->getType()); From a7ba5c2dfb7b5125f8c61e91c592218b88f2f685 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Wed, 14 Aug 2013 20:56:23 -0400 Subject: [PATCH 20/34] Refactor Builder/Expr set() and add tests --- lib/Doctrine/MongoDB/Query/Builder.php | 5 +--- lib/Doctrine/MongoDB/Query/Expr.php | 28 +++++++++++-------- .../Doctrine/MongoDB/Tests/Query/ExprTest.php | 24 ++++++++++++++++ 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index fe54502c..b9a8676e 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -1276,10 +1276,7 @@ public function selectSlice($fieldName, $countOrSkip, $limit = null) */ public function set($value, $atomic = true) { - if ($this->query['type'] === Query::TYPE_INSERT) { - $atomic = false; - } - $this->expr->set($value, (boolean) $atomic); + $this->expr->set($value, $atomic && $this->query['type'] !== Query::TYPE_INSERT); return $this; } diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index fbe88829..83c09ed6 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -505,20 +505,24 @@ public function range($start, $end) public function set($value, $atomic = true) { $this->requiresCurrentField(); - if ($atomic === true) { + + if ($atomic) { $this->newObj[$this->cmd . 'set'][$this->currentField] = $value; - } else { - if (strpos($this->currentField, '.') !== false) { - $e = explode('.', $this->currentField); - $current = &$this->newObj; - foreach ($e as $v) { - $current = &$current[$v]; - } - $current = $value; - } else { - $this->newObj[$this->currentField] = $value; - } + return $this; + } + + if (strpos($this->currentField, '.') === false) { + $this->newObj[$this->currentField] = $value; + return $this; + } + + $keys = explode('.', $this->currentField); + $current = &$this->newObj; + foreach ($keys as $key) { + $current = &$current[$key]; } + $current = $value; + return $this; } diff --git a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php index 523d56f8..4145d143 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/ExprTest.php @@ -295,6 +295,30 @@ public function testGeoWithinPolygonRequiresAtLeastThreePoints() $expr->geoWithinPolygon(array(0, 0), array(1, 1)); } + public function testSetWithAtomic() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a')->set(1, true)); + $this->assertEquals(array('$set' => array('a' => 1)), $expr->getNewObj()); + } + + public function testSetWithoutAtomicWithTopLevelField() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a')->set(1, false)); + $this->assertEquals(array('a' => 1), $expr->getNewObj()); + } + + public function testSetWithoutAtomicWithNestedField() + { + $expr = new Expr('$'); + + $this->assertSame($expr, $expr->field('a.b.c')->set(1, false)); + $this->assertEquals(array('a' => array('b' => array('c' => 1))), $expr->getNewObj()); + } + public function testWhere() { $expr = new Expr('$'); From e6dfbc36171861d58b08c5d2c35144f7c02f14f8 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 14:09:14 -0400 Subject: [PATCH 21/34] Remove optional fields from Builder::$query array These are not accessed in Query unless the query type is appropriate, and the initializer methods for those query types already create the expected fields. --- lib/Doctrine/MongoDB/Query/Builder.php | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index b9a8676e..30d8ad99 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -54,28 +54,15 @@ class Builder */ protected $query = array( 'type' => Query::TYPE_FIND, - 'distinctField' => null, 'select' => array(), 'sort' => array(), 'limit' => null, 'skip' => null, - 'group' => array( - 'keys' => null, - 'initial' => null, - 'reduce' => null, - 'options' => array(), - ), 'hints' => array(), 'immortal' => false, 'snapshot' => false, 'slaveOkay' => null, 'eagerCursor' => false, - 'mapReduce' => array( - 'map' => null, - 'reduce' => null, - 'options' => array(), - ), - 'near' => array(), 'new' => false, 'upsert' => false, 'multiple' => false, From 66e03550ea15c2a39f69c635d58db460416f0e3e Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 15:37:35 -0400 Subject: [PATCH 22/34] Fix preparation of group "cond" option in Query::execute() --- lib/Doctrine/MongoDB/Query/Query.php | 2 +- .../MongoDB/Tests/Query/QueryTest.php | 27 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 9f29bf50..4b4df621 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -188,7 +188,7 @@ public function execute() case self::TYPE_GROUP: if (!empty($this->query['query'])) { - $this->query['group']['options']['condition'] = $this->query['query']; + $this->query['group']['options']['cond'] = $this->query['query']; } $options = array_merge($options, $this->query['group']['options']); diff --git a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php index 3ca8d8b4..fcfb3577 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php @@ -6,6 +6,33 @@ class QueryTest extends \PHPUnit_Framework_TestCase { + public function testGroup() + { + $keys = array('a' => 1); + $initial = array('count' => 0, 'sum' => 0); + $reduce = 'function(obj, prev) { prev.count++; prev.sum += obj.a; }'; + $finalize = 'function(obj) { if (obj.count) { obj.avg = obj.sum / obj.count; } else { obj.avg = 0; } }'; + + $query = array( + 'type' => Query::TYPE_GROUP, + 'group' => array( + 'keys' => $keys, + 'initial' => $initial, + 'reduce' => $reduce, + 'options' => array('finalize' => $finalize), + ), + 'query' => array('type' => 1), + ); + + $collection = $this->getMockCollection(); + $collection->expects($this->once()) + ->method('group') + ->with($keys, $initial, $reduce, array('finalize' => $finalize, 'cond' => array('type' => 1))); + + $query = new Query($this->getMockDatabase(), $collection, $query, array(), '$'); + $query->execute(); + } + public function testMapReduceOptionsArePassed() { $queryArray = array( From fe4b3ca2898a7faf7525c777c5fa07d5fcef34b1 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 16:19:12 -0400 Subject: [PATCH 23/34] Rename Query::DISTINCT_FIELD to DISTINCT Also renamed the option, which is set in the query array. --- lib/Doctrine/MongoDB/Query/Builder.php | 4 ++-- lib/Doctrine/MongoDB/Query/Query.php | 6 +++--- tests/Doctrine/MongoDB/Tests/Query/FunctionalTest.php | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 30d8ad99..a1db3c8a 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -258,8 +258,8 @@ public function distanceMultiplier($distanceMultiplier) */ public function distinct($field) { - $this->query['type'] = Query::TYPE_DISTINCT_FIELD; - $this->query['distinctField'] = $field; + $this->query['type'] = Query::TYPE_DISTINCT; + $this->query['distinct'] = $field; return $this; } diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 4b4df621..1f242ec3 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -44,7 +44,7 @@ class Query implements IteratorAggregate const TYPE_REMOVE = 6; const TYPE_GROUP = 7; const TYPE_MAP_REDUCE = 8; - const TYPE_DISTINCT_FIELD = 9; + const TYPE_DISTINCT = 9; const TYPE_GEO_NEAR = 10; const TYPE_COUNT = 11; @@ -209,8 +209,8 @@ public function execute() return $cursor; - case self::TYPE_DISTINCT_FIELD: - return $this->collection->distinct($this->query['distinctField'], $this->query['query'], $options); + case self::TYPE_DISTINCT: + return $this->collection->distinct($this->query['distinct'], $this->query['query'], $options); case self::TYPE_GEO_NEAR: if (isset($this->query['limit']) && $this->query['limit']) { diff --git a/tests/Doctrine/MongoDB/Tests/Query/FunctionalTest.php b/tests/Doctrine/MongoDB/Tests/Query/FunctionalTest.php index 9f8009bf..cf1e1f3a 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/FunctionalTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/FunctionalTest.php @@ -10,7 +10,7 @@ */ class FunctionalTest extends BaseTest { - public function testDistinctFieldQuery() + public function testDistinctQuery() { $qb = $this->getTestQueryBuilder() ->distinct('count') @@ -21,7 +21,7 @@ public function testDistinctFieldQuery() ); $query = $qb->getQuery(); $this->assertInstanceOf('Doctrine\MongoDB\Query\Query', $query); - $this->assertEquals(Query::TYPE_DISTINCT_FIELD, $query->getType()); + $this->assertEquals(Query::TYPE_DISTINCT, $query->getType()); $this->assertEquals($expected, $qb->getQueryArray()); $this->assertInstanceof('Doctrine\MongoDB\ArrayIterator', $query->execute()); } From 93d9a990824f44f1a30cc8cb48334ab1a01919bd Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 16:21:31 -0400 Subject: [PATCH 24/34] Throw exception for invalid query types in Query constructor --- lib/Doctrine/MongoDB/Query/Query.php | 34 +++++++++++++++++-- .../MongoDB/Tests/Query/QueryTest.php | 8 +++++ 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 1f242ec3..6d639ecf 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -25,6 +25,8 @@ use Doctrine\MongoDB\EagerCursor; use Doctrine\MongoDB\Iterator; use Doctrine\MongoDB\IteratorAggregate; +use BadMethodCallException; +use InvalidArgumentException; /** * Query class used in conjunction with the Builder class for executing queries @@ -88,8 +90,36 @@ class Query implements IteratorAggregate */ protected $options; + /** + * Constructor. + * + * @param Database $database + * @param Collection $collection + * @param array $query + * @param array $options + * @param string $cmd + * @throws InvalidArgumentException if query type is invalid + */ public function __construct(Database $database, Collection $collection, array $query, array $options, $cmd) { + switch ($query['type']) { + case self::TYPE_FIND: + case self::TYPE_FIND_AND_UPDATE: + case self::TYPE_FIND_AND_REMOVE: + case self::TYPE_INSERT: + case self::TYPE_UPDATE: + case self::TYPE_REMOVE: + case self::TYPE_GROUP: + case self::TYPE_MAP_REDUCE: + case self::TYPE_DISTINCT: + case self::TYPE_GEO_NEAR: + case self::TYPE_COUNT: + break; + + default: + throw new InvalidArgumentException('Invalid query type: ' . $query['type']); + } + $this->database = $database; $this->collection = $collection; $this->query = $query; @@ -235,14 +265,14 @@ public function execute() * * @see http://php.net/manual/en/iteratoraggregate.getiterator.php * @return Iterator - * @throws \BadMethodCallException if the query did not return an Iterator + * @throws BadMethodCallException if the query did not return an Iterator */ public function getIterator() { if ($this->iterator === null) { $iterator = $this->execute(); if ($iterator !== null && !$iterator instanceof Iterator) { - throw new \BadMethodCallException('Query execution did not return an iterator. This query may not support returning iterators.'); + throw new BadMethodCallException('Query execution did not return an iterator. This query may not support returning iterators.'); } $this->iterator = $iterator; } diff --git a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php index fcfb3577..ae77c0d3 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php @@ -6,6 +6,14 @@ class QueryTest extends \PHPUnit_Framework_TestCase { + /** + * @expectedException InvalidArgumentException + */ + public function testConstructorShouldThrowExceptionForInvalidType() + { + new Query($this->getMockDatabase(), $this->getMockCollection(), array('type' => -1), array(), '$'); + } + public function testGroup() { $keys = array('a' => 1); From f19c70bbd5359c0aba2f320c800a6c24627aeb78 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 16:46:56 -0400 Subject: [PATCH 25/34] Refactor Query execution and Builder array structure Builder now initializes a much smaller array, which only contains the default type. The array structure will grow as methods are called. In turn, Query only expects the "type", "query", and possibly "newObj" fields to exist. All others are checked for existence before access. geoNear options were moved into an "options" array, which is now consistent with mapReduce and group. A subtle behavioral change is that defaults for update operations and cursor options will no longer be applied unless explicitly set by the user. This should not be a problem, as the Builder defaults were consistent with those in the driver itself. --- lib/Doctrine/MongoDB/Query/Builder.php | 50 ++++--- lib/Doctrine/MongoDB/Query/Query.php | 129 +++++++++--------- .../MongoDB/Tests/Query/BuilderTest.php | 78 +++++++---- .../MongoDB/Tests/Query/QueryTest.php | 49 +++---- 4 files changed, 164 insertions(+), 142 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index a1db3c8a..c3849a5c 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -52,21 +52,7 @@ class Builder * * @var array */ - protected $query = array( - 'type' => Query::TYPE_FIND, - 'select' => array(), - 'sort' => array(), - 'limit' => null, - 'skip' => null, - 'hints' => array(), - 'immortal' => false, - 'snapshot' => false, - 'slaveOkay' => null, - 'eagerCursor' => false, - 'new' => false, - 'upsert' => false, - 'multiple' => false, - ); + protected $query = array('type' => Query::TYPE_FIND); /** * Mongo command prefix @@ -245,7 +231,7 @@ public function distanceMultiplier($distanceMultiplier) throw new BadMethodCallException('This method requires a geoNear command (call geoNear() first)'); } - $this->query['geoNear']['distanceMultiplier'] = $distanceMultiplier; + $this->query['geoNear']['options']['distanceMultiplier'] = $distanceMultiplier; return $this; } @@ -315,6 +301,10 @@ public function equals($value) */ public function exclude($fieldName = null) { + if ( ! isset($this->query['select'])) { + $this->query['select'] = array(); + } + $fieldNames = is_array($fieldName) ? $fieldName : func_get_args(); foreach ($fieldNames as $fieldName) { @@ -467,7 +457,9 @@ public function geoNear($x, $y = null) $this->query['type'] = Query::TYPE_GEO_NEAR; $this->query['geoNear'] = array( 'near' => is_array($x) ? $x : array($x, $y), - 'spherical' => is_array($x) && isset($x['type']), + 'options' => array( + 'spherical' => is_array($x) && isset($x['type']), + ), ); return $this; } @@ -704,6 +696,10 @@ public function gte($value) */ public function hint($keyPattern) { + if ( ! isset($this->query['hint'])) { + $this->query['hint'] = array(); + } + $this->query['hints'][] = $keyPattern; return $this; } @@ -891,7 +887,7 @@ public function mapReduceOptions(array $options) public function maxDistance($maxDistance) { if ($this->query['type'] === Query::TYPE_GEO_NEAR) { - $this->query['geoNear']['maxDistance'] = $maxDistance; + $this->query['geoNear']['options']['maxDistance'] = $maxDistance; } else { $this->expr->maxDistance($maxDistance); } @@ -1198,6 +1194,10 @@ public function returnNew($bool = true) */ public function select($fieldName = null) { + if ( ! isset($this->query['select'])) { + $this->query['select'] = array(); + } + $fieldNames = is_array($fieldName) ? $fieldName : func_get_args(); foreach ($fieldNames as $fieldName) { @@ -1218,6 +1218,10 @@ public function select($fieldName = null) */ public function selectElemMatch($fieldName, $expression) { + if ( ! isset($this->query['select'])) { + $this->query['select'] = array(); + } + if ($expression instanceof Expr) { $expression = $expression->getQuery(); } @@ -1240,6 +1244,10 @@ public function selectElemMatch($fieldName, $expression) */ public function selectSlice($fieldName, $countOrSkip, $limit = null) { + if ( ! isset($this->query['select'])) { + $this->query['select'] = array(); + } + $slice = $countOrSkip; if ($limit !== null) { $slice = array($slice, $limit); @@ -1338,6 +1346,10 @@ public function snapshot($bool = true) */ public function sort($fieldName, $order = null) { + if ( ! isset($this->query['sort'])) { + $this->query['sort'] = array(); + } + $fields = is_array($fieldName) ? $fieldName : array($fieldName => $order); foreach ($fields as $fieldName => $order) { @@ -1363,7 +1375,7 @@ public function spherical($spherical = true) throw new BadMethodCallException('This method requires a geoNear command (call geoNear() first)'); } - $this->query['geoNear']['spherical'] = $spherical; + $this->query['geoNear']['options']['spherical'] = $spherical; return $this; } diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 6d639ecf..344b8d07 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -174,86 +174,75 @@ public function execute() switch ($this->query['type']) { case self::TYPE_FIND: - $cursor = $this->collection->find($this->query['query'], $this->query['select']); + $cursor = $this->collection->find( + $this->query['query'], + isset($this->query['select']) ? $this->query['select'] : array() + ); + return $this->prepareCursor($cursor); case self::TYPE_FIND_AND_UPDATE: - if ($this->query['sort']) { - $options['sort'] = $this->query['sort']; - } - if ($this->query['select']) { - $options['fields'] = $this->query['select']; - } - if ($this->query['upsert']) { - $options['upsert'] = true; - } - if ($this->query['new']) { - $options['new'] = true; - } - return $this->collection->findAndUpdate($this->query['query'], $this->query['newObj'], $options); + return $this->collection->findAndUpdate( + $this->query['query'], + $this->query['newObj'], + array_merge($options, $this->getQueryOptions('new', 'select', 'sort', 'upsert')) + ); case self::TYPE_FIND_AND_REMOVE: - if ($this->query['sort']) { - $options['sort'] = $this->query['sort']; - } - if ($this->query['select']) { - $options['fields'] = $this->query['select']; - } - return $this->collection->findAndRemove($this->query['query'], $options); + return $this->collection->findAndRemove( + $this->query['query'], + array_merge($options, $this->getQueryOptions('select', 'sort')) + ); case self::TYPE_INSERT: - return $this->collection->insert($this->query['newObj']); + return $this->collection->insert($this->query['newObj'], $options); case self::TYPE_UPDATE: - if ($this->query['upsert']) { - $options['upsert'] = $this->query['upsert']; - } - if ($this->query['multiple']) { - $options['multiple'] = $this->query['multiple']; - } - return $this->collection->update($this->query['query'], $this->query['newObj'], $options); + return $this->collection->update( + $this->query['query'], + $this->query['newObj'], + array_merge($options, $this->getQueryOptions('multiple', 'upsert')) + ); case self::TYPE_REMOVE: return $this->collection->remove($this->query['query'], $options); case self::TYPE_GROUP: - if (!empty($this->query['query'])) { - $this->query['group']['options']['cond'] = $this->query['query']; + if ( ! empty($this->query['query'])) { + $options['cond'] = $this->query['query']; } - $options = array_merge($options, $this->query['group']['options']); - - return $this->collection->group($this->query['group']['keys'], $this->query['group']['initial'], $this->query['group']['reduce'], $options); + return $this->collection->group( + $this->query['group']['keys'], + $this->query['group']['initial'], + $this->query['group']['reduce'], + array_merge($options, $this->query['group']['options']) + ); case self::TYPE_MAP_REDUCE: - if (!isset($this->query['mapReduce']['out'])) { - $this->query['mapReduce']['out'] = array('inline' => true); - } - - $options = array_merge($options, $this->query['mapReduce']['options']); - $cursor = $this->collection->mapReduce($this->query['mapReduce']['map'], $this->query['mapReduce']['reduce'], $this->query['mapReduce']['out'], $this->query['query'], $options); + $results = $this->collection->mapReduce( + $this->query['mapReduce']['map'], + $this->query['mapReduce']['reduce'], + $this->query['mapReduce']['out'], + $this->query['query'], + array_merge($options, $this->query['mapReduce']['options']) + ); - if ($cursor instanceof Cursor) { - $cursor = $this->prepareCursor($cursor); - } - - return $cursor; + return ($results instanceof Cursor) ? $this->prepareCursor($results) : $results; case self::TYPE_DISTINCT: return $this->collection->distinct($this->query['distinct'], $this->query['query'], $options); case self::TYPE_GEO_NEAR: - if (isset($this->query['limit']) && $this->query['limit']) { + if (isset($this->query['limit'])) { $options['num'] = $this->query['limit']; } - foreach (array('distanceMultiplier', 'maxDistance', 'spherical') as $key) { - if (isset($this->query['geoNear'][$key]) && $this->query['geoNear'][$key]) { - $options[$key] = $this->query['geoNear'][$key]; - } - } - - return $this->collection->near($this->query['geoNear']['near'], $this->query['query'], $options); + return $this->collection->near( + $this->query['geoNear']['near'], + $this->query['query'], + array_merge($options, $this->query['geoNear']['options']) + ); case self::TYPE_COUNT: return $this->collection->count($this->query['query']); @@ -342,27 +331,39 @@ public function toArray() */ protected function prepareCursor(Cursor $cursor) { - $cursor->limit($this->query['limit']); - $cursor->skip($this->query['skip']); - $cursor->sort($this->query['sort']); - $cursor->immortal($this->query['immortal']); - - if (null !== $this->query['slaveOkay']) { - $cursor->slaveOkay($this->query['slaveOkay']); + foreach ($this->getQueryOptions('immortal', 'limit', 'skip', 'slaveOkay', 'sort') as $key => $value) { + $cursor->$key($value); } - if ($this->query['snapshot']) { + if ( ! empty($this->query['snapshot'])) { $cursor->snapshot(); } - foreach ($this->query['hints'] as $keyPattern) { - $cursor->hint($keyPattern); + if ( ! empty($this->query['hints'])) { + foreach ($this->query['hints'] as $keyPattern) { + $cursor->hint($keyPattern); + } } - if ($this->query['eagerCursor'] === true) { + if ( ! empty($this->query['eagerCursor'])) { $cursor = new EagerCursor($cursor); } return $cursor; } + + /** + * Returns an array containing the specified keys and their values from the + * query array, provided they exist and are not null. + * + * @param string $key,... One or more option keys to be read + * @return array + */ + private function getQueryOptions(/* $key, ... */) + { + return array_filter( + array_intersect_key($this->query, array_flip(func_get_args())), + function($value) { return $value !== null; } + ); + } } diff --git a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php index da51f080..6b872102 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/BuilderTest.php @@ -332,6 +332,7 @@ public function provideProxiedExprMethods() 'exists()' => array('exists', array(true)), 'type()' => array('type', array(7)), 'all()' => array('all', array(array('value1', 'value2'))), + 'maxDistance' => array('maxDistance', array(5)), 'mod()' => array('mod', array(2)), 'near()' => array('near', array(1, 2)), 'nearSphere()' => array('nearSphere', array(1, 2)), @@ -369,40 +370,21 @@ public function provideProxiedExprMethods() ); } - public function testGeoNear() - { - $qb = $this->getTestQueryBuilder(); - - $this->assertSame($qb, $qb->geoNear(50, 50)); - $this->assertSame($qb, $qb->distanceMultiplier(2.5)); - $this->assertSame($qb, $qb->maxDistance(5)); - $this->assertSame($qb, $qb->spherical(true)); - $this->assertSame($qb, $qb->field('type')->equals('restaurant')); - $this->assertSame($qb, $qb->limit(10)); - - $this->assertEquals(Query::TYPE_GEO_NEAR, $qb->getType()); - - $expectedQuery = array('type' => 'restaurant'); - $this->assertEquals($expectedQuery, $qb->getQueryArray()); - - $geoNear = $qb->debug('geoNear'); - $this->assertEquals(array(50, 50), $geoNear['near']); - $this->assertEquals(2.5, $geoNear['distanceMultiplier']); - $this->assertEquals(5, $geoNear['maxDistance']); - $this->assertEquals(true, $geoNear['spherical']); - $this->assertEquals(10, $qb->debug('limit')); - } - /** * @dataProvider providePoint */ public function testGeoNearWithSingleArgument($point, array $near, $spherical) { + $expected = array( + 'near' => $near, + 'options' => array('spherical' => $spherical), + ); + $qb = $this->getTestQueryBuilder(); $this->assertSame($qb, $qb->geoNear($point)); $this->assertEquals(Query::TYPE_GEO_NEAR, $qb->getType()); - $this->assertEquals(array('near' => $near, 'spherical' => $spherical), $qb->debug('geoNear')); + $this->assertEquals($expected, $qb->debug('geoNear')); } public function providePoint() @@ -419,11 +401,29 @@ public function providePoint() public function testGeoNearWithBothArguments() { + $expected = array( + 'near' => array(0, 0), + 'options' => array('spherical' => false), + ); + $qb = $this->getTestQueryBuilder(); $this->assertSame($qb, $qb->geoNear(array(0, 0))); $this->assertEquals(Query::TYPE_GEO_NEAR, $qb->getType()); - $this->assertEquals(array('near' => array(0, 0), 'spherical' => false), $qb->debug('geoNear')); + $this->assertEquals($expected, $qb->debug('geoNear')); + } + + public function testDistanceMultipler() + { + $expected = array( + 'near' => array(0, 0), + 'options' => array('spherical' => false, 'distanceMultiplier' => 1), + ); + + $qb = $this->getTestQueryBuilder(); + + $this->assertSame($qb, $qb->geoNear(0, 0)->distanceMultiplier(1)); + $this->assertEquals($expected, $qb->debug('geoNear')); } /** @@ -444,6 +444,19 @@ public function testMapReduceOptionsRequiresMapReduceCommand() $qb->mapReduceOptions(array()); } + public function testMaxDistanceWithGeoNearCommand() + { + $expected = array( + 'near' => array(0, 0), + 'options' => array('spherical' => false, 'maxDistance' => 5), + ); + + $qb = $this->getTestQueryBuilder(); + + $this->assertSame($qb, $qb->geoNear(0, 0)->maxDistance(5)); + $this->assertEquals($expected, $qb->debug('geoNear')); + } + /** * @expectedException BadMethodCallException */ @@ -453,6 +466,19 @@ public function testOutRequiresMapReduceCommand() $qb->out('collection'); } + public function testSpherical() + { + $expected = array( + 'near' => array(0, 0), + 'options' => array('spherical' => true), + ); + + $qb = $this->getTestQueryBuilder(); + + $this->assertSame($qb, $qb->geoNear(0, 0)->spherical(true)); + $this->assertEquals($expected, $qb->debug('geoNear')); + } + /** * @expectedException BadMethodCallException */ diff --git a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php index ae77c0d3..5cb34fbb 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php @@ -21,7 +21,7 @@ public function testGroup() $reduce = 'function(obj, prev) { prev.count++; prev.sum += obj.a; }'; $finalize = 'function(obj) { if (obj.count) { obj.avg = obj.sum / obj.count; } else { obj.avg = 0; } }'; - $query = array( + $queryArray = array( 'type' => Query::TYPE_GROUP, 'group' => array( 'keys' => $keys, @@ -37,36 +37,30 @@ public function testGroup() ->method('group') ->with($keys, $initial, $reduce, array('finalize' => $finalize, 'cond' => array('type' => 1))); - $query = new Query($this->getMockDatabase(), $collection, $query, array(), '$'); + $query = new Query($this->getMockDatabase(), $collection, $queryArray, array(), '$'); $query->execute(); } public function testMapReduceOptionsArePassed() { + $map = 'function() { emit(this.a, 1); }'; + $reduce = 'function(key, values) { return Array.sum(values); }'; + $queryArray = array( 'type' => Query::TYPE_MAP_REDUCE, 'mapReduce' => array( - 'map' => 'map', - 'reduce' => 'reduce', + 'map' => $map, + 'reduce' => $reduce, 'out' => 'collection', - 'options' => array('limit' => 10, 'jsMode' => true), + 'options' => array('jsMode' => true), ), 'query' => array('type' => 1), ); $collection = $this->getMockCollection(); - $collection->expects($this->any()) + $collection->expects($this->once()) ->method('mapReduce') - ->with( - 'map', - 'reduce', - 'collection', - array('type' => 1), - $this->logicalAnd( - new ArrayHasKeyAndValue('limit', 10), - new ArrayHasKeyAndValue('jsMode', true) - ) - ); + ->with($map, $reduce, 'collection', array('type' => 1), array('jsMode' => true)); $query = new Query($this->getMockDatabase(), $collection, $queryArray, array(), '$'); $query->execute(); @@ -77,28 +71,17 @@ public function testGeoNearOptionsArePassed() $queryArray = array( 'type' => Query::TYPE_GEO_NEAR, 'geoNear' => array( - 'near' => array(50, 50), - 'distanceMultiplier' => 2.5, - 'maxDistance' => 5, - 'spherical' => true, + 'near' => array(1, 1), + 'options' => array('spherical' => true), ), 'limit' => 10, - 'query' => array('altitude' => array('$gt' => 1)), + 'query' => array('type' => 1), ); $collection = $this->getMockCollection(); - $collection->expects($this->any()) - ->method('geoNear') - ->with( - array(50, 50), - array('altitude' => array('$gt' => 1)), - $this->logicalAnd( - new ArrayHasKeyAndValue('distanceMultiplier', 2.5), - new ArrayHasKeyAndValue('maxDistance', 5), - new ArrayHasKeyAndValue('spherical', true), - new ArrayHasKeyAndValue('num', 10) - ) - ); + $collection->expects($this->once()) + ->method('near') + ->with(array(1, 1), array('type' => 1), array('num' => 10, 'spherical' => true)); $query = new Query($this->getMockDatabase(), $collection, $queryArray, array(), '$'); $query->execute(); From 7735482735cb2f900443b8ca009b6c4262689a3b Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 16:52:05 -0400 Subject: [PATCH 26/34] Query should allow a single cursor hint to be specified Builder::hint() is only useful for setting a cursor hint (not related to ODM's UnitOfWork hints), so there is no reason to track multiple calls and apply them all to the cursor in Query::prepareCursor(). --- lib/Doctrine/MongoDB/Query/Builder.php | 10 +++------- lib/Doctrine/MongoDB/Query/Query.php | 8 +------- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index c3849a5c..0192d100 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -691,16 +691,12 @@ public function gte($value) /** * Set the index hint for the query. * - * @param array|string $keyPattern + * @param array|string $index * @return self */ - public function hint($keyPattern) + public function hint($index) { - if ( ! isset($this->query['hint'])) { - $this->query['hint'] = array(); - } - - $this->query['hints'][] = $keyPattern; + $this->query['hint'] = $index; return $this; } diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 344b8d07..90cbca21 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -331,7 +331,7 @@ public function toArray() */ protected function prepareCursor(Cursor $cursor) { - foreach ($this->getQueryOptions('immortal', 'limit', 'skip', 'slaveOkay', 'sort') as $key => $value) { + foreach ($this->getQueryOptions('hint', 'immortal', 'limit', 'skip', 'slaveOkay', 'sort') as $key => $value) { $cursor->$key($value); } @@ -339,12 +339,6 @@ protected function prepareCursor(Cursor $cursor) $cursor->snapshot(); } - if ( ! empty($this->query['hints'])) { - foreach ($this->query['hints'] as $keyPattern) { - $cursor->hint($keyPattern); - } - } - if ( ! empty($this->query['eagerCursor'])) { $cursor = new EagerCursor($cursor); } From a040ddf84f0b99d8470bfdfeba5e295317d148dc Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 15 Aug 2013 16:56:49 -0400 Subject: [PATCH 27/34] Query should apply "limit" option for mapReduce commands This is consistent with what is already done for geoNear commands. --- lib/Doctrine/MongoDB/Query/Query.php | 4 ++++ tests/Doctrine/MongoDB/Tests/Query/QueryTest.php | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 90cbca21..06afcf0f 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -220,6 +220,10 @@ public function execute() ); case self::TYPE_MAP_REDUCE: + if (isset($this->query['limit'])) { + $options['limit'] = $this->query['limit']; + } + $results = $this->collection->mapReduce( $this->query['mapReduce']['map'], $this->query['mapReduce']['reduce'], diff --git a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php index 5cb34fbb..d5ccfa4b 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php @@ -54,13 +54,14 @@ public function testMapReduceOptionsArePassed() 'out' => 'collection', 'options' => array('jsMode' => true), ), + 'limit' => 10, 'query' => array('type' => 1), ); $collection = $this->getMockCollection(); $collection->expects($this->once()) ->method('mapReduce') - ->with($map, $reduce, 'collection', array('type' => 1), array('jsMode' => true)); + ->with($map, $reduce, 'collection', array('type' => 1), array('limit' => 10, 'jsMode' => true)); $query = new Query($this->getMockDatabase(), $collection, $queryArray, array(), '$'); $query->execute(); From ee0f3c3f441a48547066ee7b6487ebbea6286990 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:17:15 -0400 Subject: [PATCH 28/34] Fix Collection::count() return type documentation --- lib/Doctrine/MongoDB/Collection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Doctrine/MongoDB/Collection.php b/lib/Doctrine/MongoDB/Collection.php index a3485f1c..45a67466 100644 --- a/lib/Doctrine/MongoDB/Collection.php +++ b/lib/Doctrine/MongoDB/Collection.php @@ -176,7 +176,7 @@ public function batchInsert(array &$a, array $options = array()) * @param array $query * @param integer $limit * @param integer $skip - * @return ArrayIterator + * @return integer */ public function count(array $query = array(), $limit = 0, $skip = 0) { From 7cc7c5e35566c9d0ed74c18d1149632224046958 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:22:54 -0400 Subject: [PATCH 29/34] Add docblocks to Expr methods (mostly copied from Builder) --- lib/Doctrine/MongoDB/Query/Builder.php | 6 +- lib/Doctrine/MongoDB/Query/Expr.php | 389 ++++++++++++++++++++++++- 2 files changed, 386 insertions(+), 9 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 0192d100..e7f7d663 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -717,7 +717,7 @@ public function immortal($bool = true) * * @see Expr::in() * @see http://docs.mongodb.org/manual/reference/operator/in/ - * @param array|mixed $values + * @param array $values * @return self */ public function in(array $values) @@ -989,7 +989,7 @@ public function notEqual($value) * * @see Expr::notIn() * @see http://docs.mongodb.org/manual/reference/operator/nin/ - * @param array|mixed $values + * @param array $values * @return self */ public function notIn(array $values) @@ -1352,7 +1352,7 @@ public function sort($fieldName, $order = null) if (is_string($order)) { $order = strtolower($order) === 'asc' ? 1 : -1; } - $this->query['sort'][$fieldName] = (int) $order; + $this->query['sort'][$fieldName] = (integer) $order; } return $this; diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 83c09ed6..d241b151 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -23,6 +23,7 @@ use GeoJson\Geometry\Point; use BadMethodCallException; use InvalidArgumentException; +use LogicException; /** * Fluent interface for building query and update expressions. @@ -62,11 +63,24 @@ class Expr */ protected $currentField; + /** + * Constructor. + * + * @param string $cmd + */ public function __construct($cmd) { $this->cmd = $cmd; } + /** + * Add an $and clause to the current query. + * + * @see Builder::addAnd() + * @see http://docs.mongodb.org/manual/reference/operator/and/ + * @param array|Expr $expression + * @return self + */ public function addAnd($expression) { $this->query[$this->cmd . 'and'][] = $expression instanceof Expr ? $expression->getQuery() : $expression; @@ -74,7 +88,19 @@ public function addAnd($expression) } /** + * Append multiple values to the current array field only if they do not + * already exist in the array. + * + * If the field does not exist, it will be set to an array containing the + * unique values in the argument. If the field is not an array, the query + * will yield an error. + * * @deprecated 1.1 Use {@link Expr::addToSet()} with {@link Expr::each()}; Will be removed in 2.0 + * @see Builder::addManyToSet() + * @see http://docs.mongodb.org/manual/reference/operator/addToSet/ + * @see http://docs.mongodb.org/manual/reference/operator/each/ + * @param array $values + * @return self */ public function addManyToSet(array $values) { @@ -83,18 +109,51 @@ public function addManyToSet(array $values) return $this; } + /** + * Add a $nor clause to the current query. + * + * @see Builder::addNor() + * @see http://docs.mongodb.org/manual/reference/operator/nor/ + * @param array|Expr $expression + * @return self + */ public function addNor($expression) { $this->query[$this->cmd . 'nor'][] = $expression instanceof Expr ? $expression->getQuery() : $expression; return $this; } + /** + * Add an $or clause to the current query. + * + * @see Builder::addOr() + * @see http://docs.mongodb.org/manual/reference/operator/or/ + * @param array|Expr $expression + * @return self + */ public function addOr($expression) { $this->query[$this->cmd . 'or'][] = $expression instanceof Expr ? $expression->getQuery() : $expression; return $this; } + /** + * Append one or more values to the current array field only if they do not + * already exist in the array. + * + * If the field does not exist, it will be set to an array containing the + * unique value(s) in the argument. If the field is not an array, the query + * will yield an error. + * + * Multiple values may be specified by provided an Expr object and using + * {@link Expr::each()}. + * + * @see Builder::addToSet() + * @see http://docs.mongodb.org/manual/reference/operator/addToSet/ + * @see http://docs.mongodb.org/manual/reference/operator/each/ + * @param mixed|Expr $valueOrExpression + * @return self + */ public function addToSet($valueOrExpression) { if ($valueOrExpression instanceof Expr) { @@ -106,21 +165,52 @@ public function addToSet($valueOrExpression) return $this; } + /** + * Specify $all criteria for the current field. + * + * @see Builder::all() + * @see http://docs.mongodb.org/manual/reference/operator/all/ + * @param array|mixed $values + * @return self + */ public function all($values) { return $this->operator($this->cmd . 'all', (array) $values); } + /** + * Add $each criteria to the expression for a $push operation. + * + * @see Expr::push() + * @see http://docs.mongodb.org/manual/reference/operator/each/ + * @param array $values + * @return self + */ public function each(array $values) { return $this->operator($this->cmd . 'each', $values); } + /** + * Specify $elemMatch criteria for the current field. + * + * @see Builder::elemMatch() + * @see http://docs.mongodb.org/manual/reference/operator/elemMatch/ + * @param array|Expr $expression + * @return self + */ public function elemMatch($expression) { return $this->operator($this->cmd . 'elemMatch', $expression instanceof Expr ? $expression->getQuery() : $expression); } + /** + * Specify an equality match for the current field. + * + * @see Builder::equals() + * @param mixed $value + * @return self + */ public function equals($value) { if ($this->currentField) { @@ -131,11 +221,26 @@ public function equals($value) return $this; } + /** + * Specify $exists criteria for the current field. + * + * @see Builder::exists() + * @see http://docs.mongodb.org/manual/reference/operator/exists/ + * @param boolean $bool + * @return self + */ public function exists($bool) { return $this->operator($this->cmd . 'exists', (boolean) $bool); } + /** + * Set the current field for building the expression. + * + * @see Builder::field() + * @param string $field + * @return self + */ public function field($field) { $this->currentField = (string) $field; @@ -148,6 +253,7 @@ public function field($field) * The geometry parameter GeoJSON object or an array corresponding to the * geometry's JSON representation. * + * @see Builder::geoIntersects() * @see http://docs.mongodb.org/manual/reference/operator/geoIntersects/ * @param array|Geometry $geometry * @return self @@ -167,6 +273,7 @@ public function geoIntersects($geometry) * The geometry parameter GeoJSON object or an array corresponding to the * geometry's JSON representation. * + * @see Builder::geoWithin() * @see http://docs.mongodb.org/manual/reference/operator/geoIntersects/ * @param array|Geometry $geometry * @return self @@ -189,6 +296,7 @@ public function geoWithin($geometry) * Note: the $box operator only supports legacy coordinate pairs and 2d * indexes. This cannot be used with 2dsphere indexes and GeoJSON shapes. * + * @see Builder::geoWithinBox() * @see http://docs.mongodb.org/manual/reference/operator/box/ * @param float $x1 * @param float $y1 @@ -209,7 +317,7 @@ public function geoWithinBox($x1, $y1, $x2, $y2) * Note: the $center operator only supports legacy coordinate pairs and 2d * indexes. This cannot be used with 2dsphere indexes and GeoJSON shapes. * - * @see Expr::geoWithinCenter() + * @see Builider::geoWithinCenter() * @see http://docs.mongodb.org/manual/reference/operator/center/ * @param float $x * @param float $y @@ -228,6 +336,7 @@ public function geoWithinCenter($x, $y, $radius) * * Note: the $centerSphere operator supports both 2d and 2dsphere indexes. * + * @see Builder::geoWithinCenterSphere() * @see http://docs.mongodb.org/manual/reference/operator/centerSphere/ * @param float $x * @param float $y @@ -252,6 +361,7 @@ public function geoWithinCenterSphere($x, $y, $radius) * Note: the $polygon operator only supports legacy coordinate pairs and 2d * indexes. This cannot be used with 2dsphere indexes and GeoJSON shapes. * + * @see Builder::geoWithinPolygon() * @see http://docs.mongodb.org/manual/reference/operator/polygon/ * @param array $point,... Three or more point coordinate tuples * @return self @@ -268,46 +378,111 @@ public function geoWithinPolygon(/* array($x1, $y1), ... */) return $this->operator($this->cmd . 'geoWithin', $shape); } + /** + * Return the current field. + * + * @return string + */ public function getCurrentField() { return $this->currentField; } + /** + * Return the "new object". + * + * @see Builder::getNewObj() + * @return array + */ public function getNewObj() { return $this->newObj; } + /** + * Set the "new object". + * + * @see Builder::setNewObj() + * @param array $newObj + * @return self + */ public function setNewObj(array $newObj) { $this->newObj = $newObj; } + /** + * Return the query criteria. + * + * @see Builder::getQueryArray() + * @return array + */ public function getQuery() { return $this->query; } + /** + * Set the query criteria. + * + * @see Builder::setQueryArray() + * @param array $query + * @return self + */ public function setQuery(array $query) { $this->query = $query; } + /** + * Specify $gt criteria for the current field. + * + * @see Builder::gt() + * @see http://docs.mongodb.org/manual/reference/operator/gt/ + * @param mixed $value + * @return self + */ public function gt($value) { return $this->operator($this->cmd . 'gt', $value); } + /** + * Specify $gte criteria for the current field. + * + * @see Builder::gte() + * @see http://docs.mongodb.org/manual/reference/operator/gte/ + * @param mixed $value + * @return self + */ public function gte($value) { return $this->operator($this->cmd . 'gte', $value); } + /** + * Specify $in criteria for the current field. + * + * @see Builder::in() + * @see http://docs.mongodb.org/manual/reference/operator/in/ + * @param array $values + * @return self + */ public function in(array $values) { return $this->operator($this->cmd . 'in', $values); } + /** + * Increment the current field. + * + * If the field does not exist, it will be set to this value. + * + * @see Builder::inc() + * @see http://docs.mongodb.org/manual/reference/operator/inc/ + * @param float|integer $value + * @return self + */ public function inc($value) { $this->requiresCurrentField(); @@ -315,11 +490,27 @@ public function inc($value) return $this; } + /** + * Specify $lt criteria for the current field. + * + * @see Builder::lte() + * @see http://docs.mongodb.org/manual/reference/operator/lte/ + * @param mixed $value + * @return self + */ public function lt($value) { return $this->operator($this->cmd . 'lt', $value); } + /** + * Specify $lte criteria for the current field. + * + * @see Builder::lte() + * @see http://docs.mongodb.org/manual/reference/operator/lte/ + * @param mixed $value + * @return self + */ public function lte($value) { return $this->operator($this->cmd . 'lte', $value); @@ -362,6 +553,14 @@ public function maxDistance($maxDistance) return $this; } + /** + * Specify $mod criteria for the current field. + * + * @see Builder::mod() + * @see http://docs.mongodb.org/manual/reference/operator/mod/ + * @param float|integer $mod + * @return self + */ public function mod($mod) { return $this->operator($this->cmd . 'mod', $mod); @@ -374,7 +573,7 @@ public function mod($mod) * 2dsphere queries. This single parameter may be a GeoJSON point object or * an array corresponding to the point's JSON representation. * - * @see Expr::near() + * @see Builder::near() * @see http://docs.mongodb.org/manual/reference/operator/near/ * @param float|array|Point $x * @param float $y @@ -400,7 +599,7 @@ public function near($x, $y = null) * 2dsphere queries. This single parameter may be a GeoJSON point object or * an array corresponding to the point's JSON representation. * - * @see Expr::nearSphere() + * @see Builder::nearSphere() * @see http://docs.mongodb.org/manual/reference/operator/nearSphere/ * @param float|array|Point $x * @param float $y @@ -419,21 +618,55 @@ public function nearSphere($x, $y = null) return $this->operator($this->cmd . 'nearSphere', array($x, $y)); } + /** + * Negates an expression for the current field. + * + * @see Builder::not() + * @see http://docs.mongodb.org/manual/reference/operator/not/ + * @param array|Expr $expression + * @return self + */ public function not($expression) { return $this->operator($this->cmd . 'not', $expression instanceof Expr ? $expression->getQuery() : $expression); } + /** + * Specify $ne criteria for the current field. + * + * @see Builder::notEqual() + * @see http://docs.mongodb.org/manual/reference/operator/ne/ + * @param mixed $value + * @return self + */ public function notEqual($value) { return $this->operator($this->cmd . 'ne', $value); } + /** + * Specify $nin criteria for the current field. + * + * @see Builder::notIn() + * @see http://docs.mongodb.org/manual/reference/operator/nin/ + * @param array $values + * @return self + */ public function notIn(array $values) { return $this->operator($this->cmd . 'nin', $values); } + /** + * Defines an operator and value on the expression. + * + * If there is a current field, the operator will be set on it; otherwise, + * the operator is set at the top level of the query. + * + * @param string $operator + * @param mixed $value + * @return self + */ public function operator($operator, $value) { if ($this->currentField) { @@ -444,6 +677,13 @@ public function operator($operator, $value) return $this; } + /** + * Remove the first element from the current array field. + * + * @see Builder::popFirst() + * @see http://docs.mongodb.org/manual/reference/operator/pop/ + * @return self + */ public function popFirst() { $this->requiresCurrentField(); @@ -451,6 +691,13 @@ public function popFirst() return $this; } + /** + * Remove the last element from the current array field. + * + * @see Builder::popLast() + * @see http://docs.mongodb.org/manual/reference/operator/pop/ + * @return self + */ public function popLast() { $this->requiresCurrentField(); @@ -458,6 +705,15 @@ public function popLast() return $this; } + /** + * Remove all elements matching the given value or expression from the + * current array field. + * + * @see Builder::pull() + * @see http://docs.mongodb.org/manual/reference/operator/pull/ + * @param mixed|Expr $valueOrExpression + * @return self + */ public function pull($valueOrExpression) { if ($valueOrExpression instanceof Expr) { @@ -469,6 +725,15 @@ public function pull($valueOrExpression) return $this; } + /** + * Remove all elements matching any of the given values from the current + * array field. + * + * @see Builder::pullAll() + * @see http://docs.mongodb.org/manual/reference/operator/pullAll/ + * @param array $values + * @return self + */ public function pullAll(array $values) { $this->requiresCurrentField(); @@ -476,6 +741,25 @@ public function pullAll(array $values) return $this; } + /** + * Append one or more values to the current array field. + * + * If the field does not exist, it will be set to an array containing the + * value(s) in the argument. If the field is not an array, the query + * will yield an error. + * + * Multiple values may be specified by providing an Expr object and using + * {@link Expr::each()}. {@link Expr::slice()} and {@link Expr::sort()} may + * also be used to limit and order array elements, respectively. + * + * @see Builder::push() + * @see http://docs.mongodb.org/manual/reference/operator/push/ + * @see http://docs.mongodb.org/manual/reference/operator/each/ + * @see http://docs.mongodb.org/manual/reference/operator/slice/ + * @see http://docs.mongodb.org/manual/reference/operator/sort/ + * @param mixed|Expr $valueOrExpression + * @return self + */ public function push($valueOrExpression) { if ($valueOrExpression instanceof Expr) { @@ -490,6 +774,21 @@ public function push($valueOrExpression) return $this; } + /** + * Append multiple values to the current array field. + * + * If the field does not exist, it will be set to an array containing the + * values in the argument. If the field is not an array, the query will + * yield an error. + * + * This operator is deprecated in MongoDB 2.4. {@link Expr::push()} and + * {@link Expr::each()} should be used in its place. + * + * @see Builder::pushAll() + * @see http://docs.mongodb.org/manual/reference/operator/pushAll/ + * @param array $values + * @return self + */ public function pushAll(array $values) { $this->requiresCurrentField(); @@ -497,11 +796,35 @@ public function pushAll(array $values) return $this; } + /** + * Specify $gte and $lt criteria for the current field. + * + * This method is shorthand for specifying $gte criteria on the lower bound + * and $lt criteria on the upper bound. The upper bound is not inclusive. + * + * @see Builder::range() + * @param mixed $start + * @param mixed $end + * @return self + */ public function range($start, $end) { return $this->operator($this->cmd . 'gte', $start)->operator($this->cmd . 'lt', $end); } + /** + * Set the current field to a value. + * + * This is only relevant for insert, update, or findAndUpdate queries. For + * update and findAndUpdate queries, the $atomic parameter will determine + * whether or not a $set operator is used. + * + * @see Builder::set() + * @see http://docs.mongodb.org/manual/reference/operator/set/ + * @param mixed $value + * @param boolean $atomic + * @return self + */ public function set($value, $atomic = true) { $this->requiresCurrentField(); @@ -526,22 +849,46 @@ public function set($value, $atomic = true) return $this; } + /** + * Specify $size criteria for the current field. + * + * @see Builder::size() + * @see http://docs.mongodb.org/manual/reference/operator/size/ + * @param integer $size + * @return self + */ public function size($size) { return $this->operator($this->cmd . 'size', (integer) $size); } + /** + * Add $slice criteria to the expression for a $push operation. + * + * This is useful in conjunction with {@link Expr::each()} for a + * {@link Expr::push()} operation. {@link Builder::selectSlice()} should be + * used for specifying $slice for a query projection. + * + * @see http://docs.mongodb.org/manual/reference/operator/slice/ + * @param integer $slice + * @return self + */ public function slice($slice) { return $this->operator($this->cmd . 'slice', $slice); } /** - * Set one or more field/order pairs for the sort operator. + * Add $sort criteria to the expression for a $push operation. * * If sorting by multiple fields, the first argument should be an array of * field name (key) and order (value) pairs. * + * This is useful in conjunction with {@link Expr::each()} for a + * {@link Expr::push()} operation. {@link Builder::sort()} should be used to + * sort the results of a query. + * + * @see http://docs.mongodb.org/manual/reference/operator/sort/ * @param array|string $fieldName Field name or array of field/order pairs * @param int|string $order Field order (if one field is specified) * @return self @@ -554,12 +901,20 @@ public function sort($fieldName, $order = null) if (is_string($order)) { $order = strtolower($order) === 'asc' ? 1 : -1; } - $sort[$fieldName] = (int) $order; + $sort[$fieldName] = (integer) $order; } return $this->operator($this->cmd . 'sort', $sort); } + /** + * Specify $type criteria for the current field. + * + * @see Builder::type() + * @see http://docs.mongodb.org/manual/reference/operator/type/ + * @param integer|string $type + * @return self + */ public function type($type) { $map = array( @@ -589,6 +944,15 @@ public function type($type) return $this->operator($this->cmd . 'type', $type); } + /** + * Unset the current field. + * + * The field will be removed from the document (not set to null). + * + * @see Builder::unsetField() + * @see http://docs.mongodb.org/manual/reference/operator/unset/ + * @return self + */ public function unsetField() { $this->requiresCurrentField(); @@ -596,6 +960,14 @@ public function unsetField() return $this; } + /** + * Specify a JavaScript expression to use for matching documents. + * + * @see Builder::where() + * @see http://docs.mongodb.org/manual/reference/operator/where/ + * @param string|\MongoCode $javascript + * @return self + */ public function where($javascript) { $this->query[$this->cmd . 'where'] = $javascript; @@ -683,10 +1055,15 @@ public function withinPolygon(/* array($x1, $y1), array($x2, $y2), ... */) return $this->operator($this->cmd . 'within', $shape); } + /** + * Ensure that a current field has been set. + * + * @throws LogicException if a current field has not been set + */ private function requiresCurrentField() { if ( ! $this->currentField) { - throw new \LogicException('This method requires you set a current field using field().'); + throw new LogicException('This method requires you set a current field using field().'); } } } From 9e117fea6cd6a6d01078b4c36760440618c51e8f Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:23:48 -0400 Subject: [PATCH 30/34] Add array type-hint to Builder and Expr::all() --- lib/Doctrine/MongoDB/Query/Builder.php | 4 ++-- lib/Doctrine/MongoDB/Query/Expr.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index e7f7d663..069ea5f0 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -183,10 +183,10 @@ public function addToSet($valueOrExpression) * * @see Expr::all() * @see http://docs.mongodb.org/manual/reference/operator/all/ - * @param array|mixed $values + * @param array $values * @return self */ - public function all($values) + public function all(array $values) { $this->expr->all($values); return $this; diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index d241b151..394eeadb 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -170,10 +170,10 @@ public function addToSet($valueOrExpression) * * @see Builder::all() * @see http://docs.mongodb.org/manual/reference/operator/all/ - * @param array|mixed $values + * @param array $values * @return self */ - public function all($values) + public function all(array $values) { return $this->operator($this->cmd . 'all', (array) $values); } From 7f675058b498cb7def2b59bb22c3f9669c6c99c6 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:27:40 -0400 Subject: [PATCH 31/34] Don't document string argument for Builder and Expr::type() This can be removed in 2.0. It's currently a bit clumsy as the names for some of these types are arbitrary. It would be preferable to use constants. --- lib/Doctrine/MongoDB/Query/Builder.php | 2 +- lib/Doctrine/MongoDB/Query/Expr.php | 51 ++++++++++++++------------ 2 files changed, 28 insertions(+), 25 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Builder.php b/lib/Doctrine/MongoDB/Query/Builder.php index 069ea5f0..4cc9ceda 100644 --- a/lib/Doctrine/MongoDB/Query/Builder.php +++ b/lib/Doctrine/MongoDB/Query/Builder.php @@ -1380,7 +1380,7 @@ public function spherical($spherical = true) * * @see Expr::type() * @see http://docs.mongodb.org/manual/reference/operator/type/ - * @param integer|string $type + * @param integer $type * @return self */ public function type($type) diff --git a/lib/Doctrine/MongoDB/Query/Expr.php b/lib/Doctrine/MongoDB/Query/Expr.php index 394eeadb..3caada3b 100644 --- a/lib/Doctrine/MongoDB/Query/Expr.php +++ b/lib/Doctrine/MongoDB/Query/Expr.php @@ -910,37 +910,40 @@ public function sort($fieldName, $order = null) /** * Specify $type criteria for the current field. * + * @todo Remove support for string $type argument in 2.0 * @see Builder::type() * @see http://docs.mongodb.org/manual/reference/operator/type/ - * @param integer|string $type + * @param integer $type * @return self */ public function type($type) { - $map = array( - 'double' => 1, - 'string' => 2, - 'object' => 3, - 'array' => 4, - 'binary' => 5, - 'undefined' => 6, - 'objectid' => 7, - 'boolean' => 8, - 'date' => 9, - 'null' => 10, - 'regex' => 11, - 'jscode' => 13, - 'symbol' => 14, - 'jscodewithscope' => 15, - 'integer32' => 16, - 'timestamp' => 17, - 'integer64' => 18, - 'minkey' => 255, - 'maxkey' => 127 - ); - if (is_string($type) && isset($map[$type])) { - $type = $map[$type]; + if (is_string($type)) { + $map = array( + 'double' => 1, + 'string' => 2, + 'object' => 3, + 'array' => 4, + 'binary' => 5, + 'undefined' => 6, + 'objectid' => 7, + 'boolean' => 8, + 'date' => 9, + 'null' => 10, + 'regex' => 11, + 'jscode' => 13, + 'symbol' => 14, + 'jscodewithscope' => 15, + 'integer32' => 16, + 'timestamp' => 17, + 'integer64' => 18, + 'maxkey' => 127, + 'minkey' => 255, + ); + + $type = isset($map[$type]) ? $map[$type] : $type; } + return $this->operator($this->cmd . 'type', $type); } From b5f3e0cd35663f3b5903042ed477497bc89826e3 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:30:31 -0400 Subject: [PATCH 32/34] Query::getIterator() should not execute if an exception is guaranteed Changed to avoid calling execute() and throw BadMethodCallException immediately if the query type is not documented as returning an Iterator. If the query type should return an Iterator, we execute it and then throw UnexpectedValueException if it does not. --- lib/Doctrine/MongoDB/Query/Query.php | 28 ++++++++- .../MongoDB/Tests/Query/QueryTest.php | 63 +++++++++++++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 06afcf0f..9d7cb460 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -27,6 +27,7 @@ use Doctrine\MongoDB\IteratorAggregate; use BadMethodCallException; use InvalidArgumentException; +use UnexpectedValueException; /** * Query class used in conjunction with the Builder class for executing queries @@ -256,19 +257,40 @@ public function execute() /** * Execute the query and return its result, which must be an Iterator. * + * If the query type is not expected to return an Iterator, + * BadMethodCallException will be thrown before executing the query. + * Otherwise, the query will be executed and UnexpectedValueException will + * be thrown if {@link Query::execute()} does not return an Iterator. + * * @see http://php.net/manual/en/iteratoraggregate.getiterator.php * @return Iterator - * @throws BadMethodCallException if the query did not return an Iterator + * @throws BadMethodCallException if the query type would not return an Iterator + * @throws UnexpectedValueException if the query did not return an Iterator */ public function getIterator() { + switch ($this->query['type']) { + case self::TYPE_FIND: + case self::TYPE_GROUP: + case self::TYPE_MAP_REDUCE: + case self::TYPE_DISTINCT: + case self::TYPE_GEO_NEAR: + break; + + default: + throw new BadMethodCallException('Iterator would not be returned for query type: ' . $this->query['type']); + } + if ($this->iterator === null) { $iterator = $this->execute(); - if ($iterator !== null && !$iterator instanceof Iterator) { - throw new BadMethodCallException('Query execution did not return an iterator. This query may not support returning iterators.'); + + if ( ! $iterator instanceof Iterator) { + throw new UnexpectedValueException('Iterator was not returned from executed query'); } + $this->iterator = $iterator; } + return $this->iterator; } diff --git a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php index d5ccfa4b..81ddb8be 100644 --- a/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php +++ b/tests/Doctrine/MongoDB/Tests/Query/QueryTest.php @@ -14,6 +14,69 @@ public function testConstructorShouldThrowExceptionForInvalidType() new Query($this->getMockDatabase(), $this->getMockCollection(), array('type' => -1), array(), '$'); } + /** + * @dataProvider provideQueryTypesThatDoNotReturnAnIterator + * @expectedException BadMethodCallException + */ + public function testGetIteratorShouldThrowExceptionWithoutExecutingForTypesThatDoNotReturnAnIterator($type, $method) + { + $collection = $this->getMockCollection(); + $collection->expects($this->never())->method($method); + + $query = new Query($this->getMockDatabase(), $collection, array('type' => $type), array(), '$'); + + $query->getIterator(); + } + + public function provideQueryTypesThatDoNotReturnAnIterator() + { + return array( + array(Query::TYPE_FIND_AND_UPDATE, 'findAndUpdate'), + array(Query::TYPE_FIND_AND_REMOVE, 'findAndRemove'), + array(Query::TYPE_INSERT, 'insert'), + array(Query::TYPE_UPDATE, 'update'), + array(Query::TYPE_REMOVE, 'remove'), + array(Query::TYPE_COUNT, 'count'), + ); + } + + /** + * @dataProvider provideQueryTypesThatDoReturnAnIterator + * @expectedException UnexpectedValueException + */ + public function testGetIteratorShouldThrowExceptionAfterExecutingForTypesThatShouldReturnAnIteratorButDoNot($type, $method) + { + $collection = $this->getMockCollection(); + $collection->expects($this->once()) + ->method($method) + ->will($this->returnValue(null)); + + // Create a query array with any fields that may be expected to exist + $queryArray = array( + 'type' => $type, + 'query' => array(), + 'group' => array('keys' => array(), 'initial' => array(), 'reduce' => '', 'options' => array()), + 'mapReduce' => array('map' => '', 'reduce' => '', 'out' => '', 'options' => array()), + 'geoNear' => array('near' => array(), 'options' => array()), + 'distinct' => 0, + ); + + $query = new Query($this->getMockDatabase(), $collection, $queryArray, array(), '$'); + + $query->getIterator(); + } + + public function provideQueryTypesThatDoReturnAnIterator() + { + return array( + // Skip Query::TYPE_FIND, since prepareCursor() would error first + array(Query::TYPE_GROUP, 'group'), + array(Query::TYPE_MAP_REDUCE, 'mapReduce'), + array(Query::TYPE_DISTINCT, 'distinct'), + array(Query::TYPE_GEO_NEAR, 'near'), + ); + } + public function testGroup() { $keys = array('a' => 1); From b92692659abf5a98aeb83fcb32a9038420542e3e Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:55:11 -0400 Subject: [PATCH 33/34] Deprecate Query::iterate() alias in favor of getIterator() --- lib/Doctrine/MongoDB/Query/Query.php | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index 9d7cb460..a1126ad8 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -328,6 +328,7 @@ public function getType() /** * Alias of {@link Query::getIterator()}. * + * @deprecated 1.1 Use {@link Query::getIterator()}; will be removed for 2.0 * @return Iterator */ public function iterate() From d13aab0a3cef447aa4df3def28efad3965330540 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Fri, 30 Aug 2013 16:55:45 -0400 Subject: [PATCH 34/34] Update references to IteratorAggregate interface methods in Query --- lib/Doctrine/MongoDB/Query/Query.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Doctrine/MongoDB/Query/Query.php b/lib/Doctrine/MongoDB/Query/Query.php index a1126ad8..ee95bc57 100644 --- a/lib/Doctrine/MongoDB/Query/Query.php +++ b/lib/Doctrine/MongoDB/Query/Query.php @@ -307,7 +307,7 @@ public function getQuery() /** * Execute the query and return the first result. * - * @see Iterator::getSingleResult() + * @see IteratorAggregate::getSingleResult() * @return array|object|null */ public function getSingleResult() @@ -339,7 +339,7 @@ public function iterate() /** * Execute the query and return its results as an array. * - * @see Iterator::toArray() + * @see IteratorAggregate::toArray() * @return array */ public function toArray()