From f49dc7e15aa75a818d8a25a71ea4fd8950fe9677 Mon Sep 17 00:00:00 2001 From: Ingo Schommer Date: Wed, 6 May 2015 00:14:45 +1200 Subject: [PATCH] Clean up SearchQuery API (less misleading names, fluent interface) SearchQuery->search() is highly misleading - it implies that a search is executed, while in fact it only adds a search term to an internal property. This gets even more confusing since the actual method executing passing on the search request to the backend is also called SearchIndex->search(). Renamed other methods along the same lines, for example start() is a confusing name for adding a start value for pagination, and inClass() implies a boolean return while in fact its adding a filter. Also added getters to get naming a bit more consistent (e.g. filter() sets $require). Set the deprecation notices to 2.0, so a future release. --- _config.php | 2 + code/search/SearchQuery.php | 163 +++++++++++++++++++++++++++++++----- tests/SolrIndexTest.php | 2 +- 3 files changed, 147 insertions(+), 20 deletions(-) diff --git a/_config.php b/_config.php index 3a5121a8..4f0a96b0 100644 --- a/_config.php +++ b/_config.php @@ -2,3 +2,5 @@ global $databaseConfig; if (isset($databaseConfig['type'])) SearchUpdater::bind_manipulation_capture(); + +Deprecation::notification_version('1.0', 'fulltextsearch'); \ No newline at end of file diff --git a/code/search/SearchQuery.php b/code/search/SearchQuery.php index 52e4bb2e..e2ce54f8 100644 --- a/code/search/SearchQuery.php +++ b/code/search/SearchQuery.php @@ -8,6 +8,7 @@ class SearchQuery extends ViewableData { static $missing = null; + static $present = null; static $default_page_size = 10; @@ -19,12 +20,12 @@ class SearchQuery extends ViewableData { public $classes = array(); public $require = array(); + public $exclude = array(); protected $start = 0; - protected $limit = -1; - /** These are the API functions */ + protected $limit = -1; function __construct() { if (self::$missing === null) self::$missing = new stdClass(); @@ -37,38 +38,60 @@ function __construct() { * @param array $boost Map of composite field names to float values. The higher the value, * the more important the field gets for relevancy. */ - function search($text, $fields = null, $boost = array()) { + function addSearchTerm($text, $fields = null, $boost = array()) { $this->search[] = array('text' => $text, 'fields' => $fields ? (array)$fields : null, 'boost' => $boost, 'fuzzy' => false); + return $this; } /** - * Similar to {@link search()}, but uses stemming and other similarity algorithms + * Similar to {@link addSearchTerm()}, but uses stemming and other similarity algorithms * to find the searched terms. For example, a term "fishing" would also likely find results * containing "fish" or "fisher". Depends on search implementation. * - * @param String $text See {@link search()} - * @param array $fields See {@link search()} - * @param array $boost See {@link search()} + * @param String $text See {@link addSearchTerm()} + * @param array $fields See {@link addSearchTerm()} + * @param array $boost See {@link addSearchTerm()} */ - function fuzzysearch($text, $fields = null, $boost = array()) { + function addFuzzySearchTerm($text, $fields = null, $boost = array()) { $this->search[] = array('text' => $text, 'fields' => $fields ? (array)$fields : null, 'boost' => $boost, 'fuzzy' => true); + return $this; } - function inClass($class, $includeSubclasses = true) { + function getSearchTerms() { + return $this->search; + } + + /** + * Limit search to a specific class. Includes subclasses by default. + * + * @param String $class + * @param boolean $includeSubclasses + */ + function addClassFilter($class, $includeSubclasses = true) { $this->classes[] = array('class' => $class, 'includeSubclasses' => $includeSubclasses); + return $this; + } + + function getClassFilters() { + return $this->classes; } /** - * Similar to {@link search()}, but typically used to further narrow down + * Similar to {@link addSearchTerm()}, but typically used to further narrow down * based on other facets which don't influence the field relevancy. * * @param String $field Composite name of the field * @param Mixed $values Scalar value, array of values, or an instance of SearchQuery_Range */ - function filter($field, $values) { + function addFilter($field, $values) { $requires = isset($this->require[$field]) ? $this->require[$field] : array(); $values = is_array($values) ? $values : array($values); $this->require[$field] = array_merge($requires, $values); + return $this; + } + + function getFilters() { + return $this->require; } /** @@ -77,32 +100,116 @@ function filter($field, $values) { * @param String $field * @param mixed $values */ - function exclude($field, $values) { + function addExclude($field, $values) { $excludes = isset($this->exclude[$field]) ? $this->exclude[$field] : array(); $values = is_array($values) ? $values : array($values); $this->exclude[$field] = array_merge($excludes, $values); + return $this; } - function start($start) { + function getExcludes() { + return $this->exclude; + } + + function setStart($start) { $this->start = $start; + return $this; } - function limit($limit) { + function getStart() { + return $this->start; + } + + function setLimit($limit) { $this->limit = $limit; + return $this; } - function page($page) { + function getLimit() { + return $this->limit; + } + + function setPageSize($page) { $this->start = $page * self::$default_page_size; $this->limit = self::$default_page_size; + return $this; + } + + public function getPageSize() { + return $this->limit; } - function isfiltered() { + function isFiltered() { return $this->search || $this->classes || $this->require || $this->exclude; } function __toString() { return "Search Query\n"; } + + /** + * @deprecated + */ + function search($text, $fields = null, $boost = array()) { + Deprecation::notice('2.0', 'Use addSearchTerm()'); + return $this->addSearchTerm($text, $fields, $boost); + } + + /** + * @deprecated + */ + function fuzzysearch($text, $fields = null, $boost = array()) { + Deprecation::notice('2.0', 'Use addFuzzySearchTerm()'); + return $this->addFuzzySearchTerm($text, $fields, $boost); + } + + /** + * @deprecated + */ + function filter($field, $values) { + Deprecation::notice('2.0', 'Use addFilter()'); + return $this->addFilter($field, $values); + } + + /** + * @deprecated + */ + function inClass($class, $includeSubclasses = true) { + Deprecation::notice('2.0', 'Use addClassFilter()'); + return $this->addClassFilter($class, $includeSubclasses); + } + + /** + * @deprecated + */ + function exclude($field, $values) { + Deprecation::notice('2.0', 'Use addExclude()'); + return $this->addExclude($field, $values); + } + + /** + * @deprecated + */ + function start($start) { + Deprecation::notice('2.0', 'Use setStart()'); + return $this->setStart($start); + } + + /** + * @deprecated + */ + function limit($limit) { + Deprecation::notice('2.0', 'Use setLimit()'); + return $this->setLimit($limit); + } + + /** + * @deprecated + */ + function page($page) { + Deprecation::notice('2.0', 'Use setPageSize()'); + return $this->setPageSize($page); + } } /** @@ -119,15 +226,33 @@ function __construct($start = null, $end = null) { $this->end = $end; } - function start($start) { + function setStart($start) { $this->start = $start; + return $this; } - function end($end) { + function setEnd($end) { $this->end = $end; + return $this; } - function isfiltered() { + function isFiltered() { return $this->start !== null || $this->end !== null; } + + /** + * @deprecated + */ + function start($start) { + Deprecation::notice('2.0', 'Use setStart()'); + return $this->setStart($start); + } + + /** + * @deprecated + */ + function end($end) { + Deprecation::notice('2.0', 'Use setEnd()'); + return $this->setEnd($end); + } } \ No newline at end of file diff --git a/tests/SolrIndexTest.php b/tests/SolrIndexTest.php index b3aee60e..6a12f7f9 100644 --- a/tests/SolrIndexTest.php +++ b/tests/SolrIndexTest.php @@ -54,7 +54,7 @@ function testBoost() { $index->setService($serviceMock); $query = new SearchQuery(); - $query->search( + $query->addSearchTerm( 'term', null, array('Field1' => 1.5, 'HasOneObject_Field1' => 3)