From 0458227e65dc8235ae2d6dc43480396a92e853af Mon Sep 17 00:00:00 2001
From: Mantas Varatiejus <mantas.varatiejus@nfq.com>
Date: Mon, 21 Dec 2015 17:00:15 +0200
Subject: [PATCH] Fix nested query in case "bool" with single "must" given
---
src/Aggregation/FilterAggregation.php | 8 +----
src/Query/BoolQuery.php | 36 +++++++------------
src/SearchEndpoint/FilterEndpoint.php | 8 +----
src/SearchEndpoint/PostFilterEndpoint.php | 7 +---
src/SearchEndpoint/QueryEndpoint.php | 5 +--
tests/Query/BoolQueryTest.php | 31 +---------------
tests/Query/NestedQueryTest.php | 28 +++++++++++++++
tests/SearchEndpoint/FilterEndpointTest.php | 10 +++++-
.../SearchEndpoint/PostFilterEndpointTest.php | 2 +-
tests/SearchEndpoint/QueryEndpointTest.php | 2 +-
10 files changed, 57 insertions(+), 80 deletions(-)
diff --git a/src/Aggregation/FilterAggregation.php b/src/Aggregation/FilterAggregation.php
index 58e3deb..3e49b7d 100644
--- a/src/Aggregation/FilterAggregation.php
+++ b/src/Aggregation/FilterAggregation.php
@@ -69,13 +69,7 @@ class FilterAggregation extends AbstractAggregation
throw new \LogicException("Filter aggregation `{$this->getName()}` has no filter added");
}
- if ($this->filter instanceof BoolFilter && $this->filter->isRelevant()
- || !$this->filter instanceof BoolFilter
- ) {
- $filterData = [$this->filter->getType() => $this->filter->toArray()];
- } else {
- $filterData = $this->filter->toArray();
- }
+ $filterData = [$this->filter->getType() => $this->filter->toArray()];
return $filterData;
}
diff --git a/src/Query/BoolQuery.php b/src/Query/BoolQuery.php
index 924ee6c..0c70493 100644
--- a/src/Query/BoolQuery.php
+++ b/src/Query/BoolQuery.php
@@ -30,30 +30,26 @@ class BoolQuery implements BuilderInterface
/**
* @var array
*/
- private $container = [];
+ private $container;
/**
* Constructor to prepare container.
*/
public function __construct()
{
- $this->container = [
- self::MUST => [],
- self::MUST_NOT => [],
- self::SHOULD => [],
- ];
+ $this->container = [];
}
/**
* Checks if bool expression is relevant.
*
* @return bool
+ *
+ * @deprecated Will be removed in 2.0. No replacement. Always use full structure.
*/
public function isRelevant()
{
- return
- (count($this->container[self::MUST_NOT]) + count($this->container[self::SHOULD])) > 0
- || count($this->container[self::MUST]) > 1;
+ return true;
}
/**
@@ -63,11 +59,13 @@ class BoolQuery implements BuilderInterface
public function getQueries($boolType = null)
{
if ($boolType === null) {
- return array_merge(
- $this->container[self::MUST],
- $this->container[self::MUST_NOT],
- $this->container[self::SHOULD]
- );
+ $queries = [];
+
+ foreach ($this->container as $item) {
+ $queries = array_merge($queries, $item);
+ }
+
+ return $queries;
}
return $this->container[$boolType];
@@ -104,15 +102,7 @@ class BoolQuery implements BuilderInterface
*/
public function toArray()
{
- $output = $this->processArray();
-
- if (!$this->isRelevant()) {
- /** @var BuilderInterface $query */
- $mustContainer = $this->container[self::MUST];
- $query = array_shift($mustContainer);
-
- return [$query->getType() => $query->toArray()];
- }
+ $output = [];
foreach ($this->container as $boolType => $builders) {
/** @var BuilderInterface $builder */
diff --git a/src/SearchEndpoint/FilterEndpoint.php b/src/SearchEndpoint/FilterEndpoint.php
index 2340798..6b9ba4c 100644
--- a/src/SearchEndpoint/FilterEndpoint.php
+++ b/src/SearchEndpoint/FilterEndpoint.php
@@ -35,14 +35,8 @@ class FilterEndpoint extends QueryEndpoint
}
$query = new FilteredQuery();
- if (!$this->getBool()->isRelevant()) {
- $filters = $this->getBool()->getQueries(BoolFilter::MUST);
- $filter = array_shift($filters);
- } else {
- $filter = $this->getBool();
- }
+ $query->setFilter($this->getBool());
- $query->setFilter($filter);
$this->addReference('filtered_query', $query);
}
diff --git a/src/SearchEndpoint/PostFilterEndpoint.php b/src/SearchEndpoint/PostFilterEndpoint.php
index 5a8d719..3fadc68 100644
--- a/src/SearchEndpoint/PostFilterEndpoint.php
+++ b/src/SearchEndpoint/PostFilterEndpoint.php
@@ -33,12 +33,7 @@ class PostFilterEndpoint extends FilterEndpoint
return null;
}
- if (!$this->getBool()->isRelevant()) {
- $filters = $this->getBool()->getQueries(BoolFilter::MUST);
- $filter = array_shift($filters);
- } else {
- $filter = $this->getBool();
- }
+ $filter = $this->getBool();
return [$filter->getType() => $filter->toArray()];
}
diff --git a/src/SearchEndpoint/QueryEndpoint.php b/src/SearchEndpoint/QueryEndpoint.php
index e51414f..9b610b5 100644
--- a/src/SearchEndpoint/QueryEndpoint.php
+++ b/src/SearchEndpoint/QueryEndpoint.php
@@ -51,10 +51,7 @@ class QueryEndpoint extends AbstractSearchEndpoint implements OrderedNormalizerI
}
$queryArray = $this->bool->toArray();
-
- if ($this->bool->isRelevant()) {
- $queryArray = [$this->bool->getType() => $queryArray];
- }
+ $queryArray = [$this->bool->getType() => $queryArray];
return $queryArray;
}
diff --git a/tests/Query/BoolQueryTest.php b/tests/Query/BoolQueryTest.php
index 4505857..d6fa124 100644
--- a/tests/Query/BoolQueryTest.php
+++ b/tests/Query/BoolQueryTest.php
@@ -23,40 +23,11 @@ class BoolQueryTest extends \PHPUnit_Framework_TestCase
/**
* Tests isRelevant method.
*/
- public function testBoolIsRelevantWithOneQuery()
+ public function testIsRelevant()
{
$bool = new BoolQuery();
- $this->assertFalse($bool->isRelevant());
- $bool->add(new TermQuery('acme', 'foo'));
-
- $this->assertFalse($bool->isRelevant());
- }
-
- /**
- * Tests isRelevant method when there is query added to should case.
- */
- public function testBoolIsRelevantWithOneShouldQuery()
- {
- $bool = new BoolQuery();
- $this->assertFalse($bool->isRelevant());
- $bool->add(new TermQuery('acme', 'foo'), BoolQuery::SHOULD);
-
$this->assertTrue($bool->isRelevant());
}
-
- /**
- * Tests isRelevant method with 2 queries.
- */
- public function testBoolIsRelevantWithTwoQuery()
- {
- $bool = new BoolQuery();
- $this->assertFalse($bool->isRelevant());
- $bool->add(new TermQuery('acme', 'foo'));
- $bool->add(new TermQuery('bar', 'go'));
-
- $this->assertTrue($bool->isRelevant());
- }
-
/**
* Test for addToBool() without setting a correct bool operator.
*
diff --git a/tests/Query/NestedQueryTest.php b/tests/Query/NestedQueryTest.php
index 6958369..abf9efa 100644
--- a/tests/Query/NestedQueryTest.php
+++ b/tests/Query/NestedQueryTest.php
@@ -11,7 +11,9 @@
namespace ONGR\ElasticsearchDSL\Tests\Unit\DSL\Query;
+use ONGR\ElasticsearchDSL\Query\BoolQuery;
use ONGR\ElasticsearchDSL\Query\NestedQuery;
+use ONGR\ElasticsearchDSL\Query\TermQuery;
class NestedQueryTest extends \PHPUnit_Framework_TestCase
{
@@ -57,4 +59,30 @@ class NestedQueryTest extends \PHPUnit_Framework_TestCase
$this->assertTrue(method_exists($nestedQuery, 'hasParameter'), 'Nested query must have hasParameter method');
$this->assertTrue(method_exists($nestedQuery, 'getParameter'), 'Nested query must have getParameter method');
}
+
+ /**
+ * Test for toArray() in case bool with single clause given.
+ */
+ public function testSingleBoolMust()
+ {
+ $bool = new BoolQuery();
+ $bool->add(new TermQuery('field1', 'value1'));
+
+ $query = new NestedQuery('obj1', $bool);
+
+ $expected = [
+ 'path' => 'obj1',
+ 'query' => [
+ 'bool' => [
+ 'must' => [
+ [
+ 'term' => ['field1' => 'value1'],
+ ],
+ ],
+ ],
+ ],
+ ];
+
+ $this->assertEquals($expected, $query->toArray());
+ }
}
diff --git a/tests/SearchEndpoint/FilterEndpointTest.php b/tests/SearchEndpoint/FilterEndpointTest.php
index 814fbd1..5e3b4ad 100644
--- a/tests/SearchEndpoint/FilterEndpointTest.php
+++ b/tests/SearchEndpoint/FilterEndpointTest.php
@@ -64,7 +64,15 @@ class FilterEndpointTest extends \PHPUnit_Framework_TestCase
/** @var FilteredQuery $reference */
$reference = $instance->getReference('filtered_query');
$this->assertInstanceOf('ONGR\ElasticsearchDSL\Query\FilteredQuery', $reference);
- $this->assertSame($matchAllFilter, $reference->getFilter());
+
+ /** @var \ONGR\ElasticsearchDSL\Query\BoolQuery $bool */
+ $bool = $reference->getFilter();
+ $this->assertInstanceOf('ONGR\ElasticsearchDSL\Query\BoolQuery', $bool);
+
+ $must = $bool->getQueries('must');
+ $realReference = reset($must);
+
+ $this->assertSame($matchAllFilter, $realReference);
}
/**
diff --git a/tests/SearchEndpoint/PostFilterEndpointTest.php b/tests/SearchEndpoint/PostFilterEndpointTest.php
index 75236a1..7c22d4a 100644
--- a/tests/SearchEndpoint/PostFilterEndpointTest.php
+++ b/tests/SearchEndpoint/PostFilterEndpointTest.php
@@ -54,7 +54,7 @@ class PostFilterEndpointTest extends \PHPUnit_Framework_TestCase
$instance->add($matchAll);
$this->assertEquals(
- json_encode([$matchAll->getType() => $matchAll->toArray()]),
+ json_encode(['bool' => ['must' => [[$matchAll->getType() => $matchAll->toArray()]]]]),
json_encode($instance->normalize($normalizerInterface))
);
}
diff --git a/tests/SearchEndpoint/QueryEndpointTest.php b/tests/SearchEndpoint/QueryEndpointTest.php
index 71dfd09..997f82f 100644
--- a/tests/SearchEndpoint/QueryEndpointTest.php
+++ b/tests/SearchEndpoint/QueryEndpointTest.php
@@ -55,7 +55,7 @@ class QueryEndpointTest extends \PHPUnit_Framework_TestCase
$instance->add($matchAll);
$this->assertEquals(
- [$matchAll->getType() => $matchAll->toArray()],
+ ['bool' => ['must' => [[$matchAll->getType() => $matchAll->toArray()]]]],
$instance->normalize($normalizerInterface)
);
}
--
GitLab