Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removes requirement of the arguments in the constructor method (Filtered) in favor #615 #616

Merged
merged 8 commits into from
Jun 14, 2014
4 changes: 4 additions & 0 deletions changes.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
CHANGES

2014-05-15
- Removed the requirement to set arguments filter and/or query in Filtered, according to the documentation:
http://www.elasticsearch.org/guide/en/elasticsearch/reference/current/query-dsl-filtered-query.html

2014-05-13
- Add JSON compat library; Elasticsearch JSON flags and nicer error handling

Expand Down
31 changes: 23 additions & 8 deletions lib/Elastica/Query/Filtered.php
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
<?php

namespace Elastica\Query;

use Elastica\Filter\AbstractFilter;
use Elastica\Exception\InvalidException;

/**
* Filtered query. Needs a query and a filter
Expand Down Expand Up @@ -33,10 +35,12 @@ class Filtered extends AbstractQuery
* @param \Elastica\Query\AbstractQuery $query Query object
* @param \Elastica\Filter\AbstractFilter $filter Filter object
*/
public function __construct(AbstractQuery $query, AbstractFilter $filter)
{
$this->setQuery($query);
$this->setFilter($filter);
public function __construct(
AbstractQuery $query = null,
AbstractFilter $filter = null
) {
$this->_query = $query;
$this->_filter = $filter;
}

/**
Expand Down Expand Up @@ -93,9 +97,20 @@ public function getQuery()
*/
public function toArray()
{
return array('filtered' => array(
'query' => $this->_query->toArray(),
'filter' => $this->_filter->toArray()
));
$filtered = array();

if ($this->_query !== null) {
$filtered['query'] = $this->_query->toArray();
}

if ($this->_filter !== null) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking for not null, I would suggest to check if it is actually a Filter) (or above a query) as you did before in the constructor.

$filtered['filter'] = $this->_filter->toArray();
}

if (empty($filtered)) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use the InvalidException here, as not implemented is used for features which were not implemented yet. But here the arguments are invalid.

throw new InvalidException('A query and/or filter is required');
}

return array('filtered' => $filtered);
}
}
9 changes: 9 additions & 0 deletions test/lib/Elastica/Test/Query/FilteredTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,13 @@ public function testFilteredGetter()
$this->assertEquals($query1->getFilter(), $filter1);
$this->assertEquals($query2->getFilter(), $filter2);
}

/**
* @expectedException \Elastica\Exception\InvalidException
*/
public function testFilteredWithoutArgumentsShouldRaiseException()
{
$query = new Filtered();
$query->toArray();
}
}