From 4671118fb2fdbac77afa58a774cf11f1abd22256 Mon Sep 17 00:00:00 2001 From: Demian Katz <demian.katz@villanova.edu> Date: Wed, 9 Nov 2016 09:24:31 -0500 Subject: [PATCH] Use fluent interface for UrlQueryHelper. (#850) - Improves efficiency of query generation by reducing redundant processing - Allows multiple operations to be stacked together - Includes improved test coverage --- .../src/VuFind/Controller/AjaxController.php | 5 +- .../VuFind/Controller/SearchController.php | 18 +- .../src/VuFind/Recommend/MapSelection.php | 5 +- .../src/VuFind/Recommend/RemoveFilters.php | 2 +- .../VuFind/src/VuFind/Search/Base/Results.php | 19 +- .../Search/Factory/UrlQueryHelperFactory.php | 166 ++++++++ .../Search/Solr/HierarchicalFacetHelper.php | 29 +- .../src/VuFind/Search/SolrAuthor/Results.php | 11 +- .../src/VuFind/Search/UrlQueryHelper.php | 372 ++++++++++-------- .../src/VuFind/View/Helper/AbstractSearch.php | 4 +- .../VuFind/View/Helper/Root/ResultFeed.php | 10 +- .../VuFind/View/Helper/Root/SortFacetList.php | 4 +- .../VuFindTest/Search/UrlQueryHelperTest.php | 161 ++++++++ 13 files changed, 586 insertions(+), 220 deletions(-) create mode 100644 module/VuFind/src/VuFind/Search/Factory/UrlQueryHelperFactory.php create mode 100644 module/VuFind/tests/unit-tests/src/VuFindTest/Search/UrlQueryHelperTest.php diff --git a/module/VuFind/src/VuFind/Controller/AjaxController.php b/module/VuFind/src/VuFind/Controller/AjaxController.php index e7ef3cb711b..600b3784f5a 100644 --- a/module/VuFind/src/VuFind/Controller/AjaxController.php +++ b/module/VuFind/src/VuFind/Controller/AjaxController.php @@ -909,9 +909,8 @@ class AjaxController extends AbstractBase $facets[$field]['removalURL'] = $results->getUrlQuery()->removeFacet( $field, - isset($filters[$field][0]) ? $filters[$field][0] : null, - false - ); + isset($filters[$field][0]) ? $filters[$field][0] : null + )->getParams(false); } return $this->output($facets, self::STATUS_OK); } diff --git a/module/VuFind/src/VuFind/Controller/SearchController.php b/module/VuFind/src/VuFind/Controller/SearchController.php index e68c1fa05c2..b5acb49604d 100644 --- a/module/VuFind/src/VuFind/Controller/SearchController.php +++ b/module/VuFind/src/VuFind/Controller/SearchController.php @@ -365,10 +365,10 @@ class SearchController extends AbstractSearch // (check it's set first -- RSS feed will return a response model rather // than a view model): if (isset($view->results)) { - $url = $view->results->getUrlQuery(); - $url->setDefaultParameter('range', $range); - $url->setDefaultParameter('department', $dept); - $url->setSuppressQuery(true); + $view->results->getUrlQuery() + ->setDefaultParameter('range', $range) + ->setDefaultParameter('department', $dept) + ->setSuppressQuery(true); } // We don't want new items hidden filters to propagate to other searches: @@ -483,11 +483,11 @@ class SearchController extends AbstractSearch // (but only do this if we have access to a results object, which we // won't in RSS mode): if (isset($view->results)) { - $url = $view->results->getUrlQuery(); - $url->setDefaultParameter('course', $course); - $url->setDefaultParameter('inst', $inst); - $url->setDefaultParameter('dept', $dept); - $url->setSuppressQuery(true); + $view->results->getUrlQuery() + ->setDefaultParameter('course', $course) + ->setDefaultParameter('inst', $inst) + ->setDefaultParameter('dept', $dept) + ->setSuppressQuery(true); } return $view; } diff --git a/module/VuFind/src/VuFind/Recommend/MapSelection.php b/module/VuFind/src/VuFind/Recommend/MapSelection.php index 31a33758bac..d22e4e84224 100644 --- a/module/VuFind/src/VuFind/Recommend/MapSelection.php +++ b/module/VuFind/src/VuFind/Recommend/MapSelection.php @@ -234,9 +234,8 @@ class MapSelection implements \VuFind\Recommend\RecommendInterface ); $this->selectedCoordinates = $reorder_coords; } - $this->searchParams = $results->getUrlQuery()->removeFacet( - $this->geoField, $value[0], false - ); + $this->searchParams = $results->getUrlQuery() + ->removeFacet($this->geoField, $value[0])->getParams(false); } } if ($this->searchParams == null) { diff --git a/module/VuFind/src/VuFind/Recommend/RemoveFilters.php b/module/VuFind/src/VuFind/Recommend/RemoveFilters.php index 9253f8bd17d..8f921e9badf 100644 --- a/module/VuFind/src/VuFind/Recommend/RemoveFilters.php +++ b/module/VuFind/src/VuFind/Recommend/RemoveFilters.php @@ -122,7 +122,7 @@ class RemoveFilters implements RecommendInterface */ public function getFilterlessUrl() { - return $this->results->getUrlQuery()->removeAllFilters(); + return $this->results->getUrlQuery()->removeAllFilters()->getParams(); } /** diff --git a/module/VuFind/src/VuFind/Search/Base/Results.php b/module/VuFind/src/VuFind/Search/Base/Results.php index a2b917e5586..4a94f9a9a02 100644 --- a/module/VuFind/src/VuFind/Search/Base/Results.php +++ b/module/VuFind/src/VuFind/Search/Base/Results.php @@ -26,7 +26,7 @@ * @link https://vufind.org Main Page */ namespace VuFind\Search\Base; -use VuFind\Search\UrlQueryHelper, Zend\Paginator\Paginator, +use VuFind\Search\Factory\UrlQueryHelperFactory, Zend\Paginator\Paginator, Zend\ServiceManager\ServiceLocatorAwareInterface, Zend\ServiceManager\ServiceLocatorInterface; use VuFindSearch\Service as SearchService; @@ -196,16 +196,29 @@ abstract class Results implements ServiceLocatorAwareInterface return $this->getParams()->getOptions(); } + /** + * Options for UrlQueryHelper + * + * @return array + */ + protected function getUrlQueryHelperOptions() + { + return []; + } + /** * Get the URL helper for this object. * - * @return UrlHelper + * @return \VuFind\Search\UrlQueryHelper */ public function getUrlQuery() { // Set up URL helper: if (!isset($this->helpers['urlQuery'])) { - $this->helpers['urlQuery'] = new UrlQueryHelper($this->getParams()); + $factory = new UrlQueryHelperFactory(); + $this->helpers['urlQuery'] = $factory->fromParams( + $this->getParams(), $this->getUrlQueryHelperOptions() + ); } return $this->helpers['urlQuery']; } diff --git a/module/VuFind/src/VuFind/Search/Factory/UrlQueryHelperFactory.php b/module/VuFind/src/VuFind/Search/Factory/UrlQueryHelperFactory.php new file mode 100644 index 00000000000..a602fcd5408 --- /dev/null +++ b/module/VuFind/src/VuFind/Search/Factory/UrlQueryHelperFactory.php @@ -0,0 +1,166 @@ +<?php +/** + * Factory to build UrlQueryHelper. + * + * PHP version 5 + * + * Copyright (C) Villanova University 2016. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * @category VuFind + * @package Search + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org Main Site + */ +namespace VuFind\Search\Factory; +use VuFind\Search\UrlQueryHelper; +use VuFind\Search\Base\Params; + +/** + * Factory to build UrlQueryHelper. + * + * @category VuFind + * @package Search + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org Main Site + */ +class UrlQueryHelperFactory +{ + /** + * Extract default settings from the search parameters. + * + * @param Params $params VuFind search parameters + * + * @return array + */ + protected function getDefaults(Params $params) + { + $options = $params->getOptions(); + return [ + 'handler' => $options->getDefaultHandler(), + 'limit' => $options->getDefaultLimit(), + 'selectedShards' => $options->getDefaultSelectedShards(), + 'sort' => $params->getDefaultSort(), + 'view' => $options->getDefaultView(), + ]; + } + + /** + * Load default settings into the user-provided configuration. + * + * @param Params $params VuFind search parameters + * @param array $config Config options + * + * @return array + */ + protected function addDefaultsToConfig(Params $params, array $config) + { + // Load defaults unless they have been overridden in existing config + // array. + foreach ($this->getDefaults($params) as $key => $value) { + if (!isset($config['defaults'][$key])) { + $config['defaults'][$key] = $value; + } + } + + // Load useful callbacks if they have not been specifically overridden + if (!isset($config['parseFilterCallback'])) { + $config['parseFilterCallback'] = [$params, 'parseFilter']; + } + if (!isset($config['getAliasesForFacetFieldCallback'])) { + $config['getAliasesForFacetFieldCallback'] + = [$params, 'getAliasesForFacetField']; + } + return $config; + } + + /** + * Extract URL query parameters from VuFind search parameters. + * + * @param Params $params VuFind search parameters + * @param array $config Config options + * + * @return array + */ + protected function getUrlParams(Params $params, array $config) + { + $urlParams = []; + $sort = $params->getSort(); + if (null !== $sort && $sort != $config['defaults']['sort']) { + $urlParams['sort'] = $sort; + } + $limit = $params->getLimit(); + if (null !== $limit && $limit != $config['defaults']['limit']) { + $urlParams['limit'] = $limit; + } + $view = $params->getView(); + if (null !== $view && $view != $config['defaults']['view']) { + $urlParams['view'] = $view; + } + if ($params->getPage() != 1) { + $urlParams['page'] = $params->getPage(); + } + $filters = $params->getFilters(); + if (!empty($filters)) { + $urlParams['filter'] = []; + foreach ($filters as $field => $values) { + foreach ($values as $current) { + $urlParams['filter'][] = $field . ':"' . $current . '"'; + } + } + } + $hiddenFilters = $params->getHiddenFilters(); + if (!empty($hiddenFilters)) { + foreach ($hiddenFilters as $field => $values) { + foreach ($values as $current) { + $urlParams['hiddenFilters'][] = $field . ':"' . $current . '"'; + } + } + } + $shards = $params->getSelectedShards(); + if (!empty($shards)) { + sort($shards); + $defaultShards = $config['defaults']['selectedShards']; + sort($defaultShards); + if (implode(':::', $shards) != implode(':::', $defaultShards)) { + $urlParams['shard'] = $shards; + } + } + if ($params->hasDefaultsApplied()) { + $urlParams['dfApplied'] = 1; + } + return $urlParams; + } + + /** + * Construct the UrlQueryHelper + * + * @param Params $params VuFind search parameters + * @param array $config Config options + * + * @return UrlQueryHelper + */ + public function fromParams(Params $params, array $config = []) + { + $finalConfig = $this->addDefaultsToConfig($params, $config); + return new UrlQueryHelper( + $this->getUrlParams($params, $finalConfig), + $params->getQuery(), + $finalConfig + ); + } +} diff --git a/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php b/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php index 7f555923a3e..14db33a94d8 100644 --- a/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php +++ b/module/VuFind/src/VuFind/Search/Solr/HierarchicalFacetHelper.php @@ -28,6 +28,7 @@ namespace VuFind\Search\Solr; use VuFind\I18n\TranslatableString; +use VuFind\Search\UrlQueryHelper; /** * Functions for manipulating facets @@ -94,13 +95,11 @@ class HierarchicalFacetHelper */ public function buildFacetArray($facet, $facetList, $urlHelper = false) { - // getParamArray() is expensive, so call it just once and pass it on - $paramArray = $urlHelper !== false ? $urlHelper->getParamArray() : null; // Create a keyed (for conversion to hierarchical) array of facet data $keyedList = []; foreach ($facetList as $item) { $keyedList[$item['value']] = $this->createFacetItem( - $facet, $item, $urlHelper, $paramArray + $facet, $item, $urlHelper ); } @@ -175,16 +174,13 @@ class HierarchicalFacetHelper /** * Create an item for the hierarchical facet array * - * @param string $facet Facet name - * @param array $item Facet item received from Solr - * @param UrlQueryHelper $urlHelper UrlQueryHelper for creating facet - * url's - * @param array $paramArray URL parameters - * active children + * @param string $facet Facet name + * @param array $item Facet item received from Solr + * @param UrlQueryHelper $urlHelper UrlQueryHelper for creating facet URLs * * @return array Facet item */ - protected function createFacetItem($facet, $item, $urlHelper, $paramArray) + protected function createFacetItem($facet, $item, $urlHelper) { $href = ''; $exclude = ''; @@ -192,16 +188,15 @@ class HierarchicalFacetHelper if ($urlHelper !== false) { if ($item['isApplied']) { $href = $urlHelper->removeFacet( - $facet, $item['value'], true, $item['operator'], $paramArray - ); + $facet, $item['value'], true, $item['operator'] + )->getParams(); } else { $href = $urlHelper->addFacet( - $facet, $item['value'], $item['operator'], $paramArray - ); + $facet, $item['value'], $item['operator'] + )->getParams(); } - $exclude = $urlHelper->addFacet( - $facet, $item['value'], 'NOT', $paramArray - ); + $exclude = $urlHelper->addFacet($facet, $item['value'], 'NOT') + ->getParams(); } $displayText = $item['displayText']; diff --git a/module/VuFind/src/VuFind/Search/SolrAuthor/Results.php b/module/VuFind/src/VuFind/Search/SolrAuthor/Results.php index c75436b2e06..d12e633069c 100644 --- a/module/VuFind/src/VuFind/Search/SolrAuthor/Results.php +++ b/module/VuFind/src/VuFind/Search/SolrAuthor/Results.php @@ -49,9 +49,16 @@ class Results extends SolrResults { // Call parent constructor: parent::__construct($params); + } - // Set up URL helper to use appropriate search parameter: - $this->getUrlQuery()->setBasicSearchParam('author'); + /** + * Options for UrlQueryHelper + * + * @return array + */ + protected function getUrlQueryHelperOptions() + { + return ['basicSearchParam' => 'author']; } /** diff --git a/module/VuFind/src/VuFind/Search/UrlQueryHelper.php b/module/VuFind/src/VuFind/Search/UrlQueryHelper.php index a112d5c97ae..8003bb21642 100644 --- a/module/VuFind/src/VuFind/Search/UrlQueryHelper.php +++ b/module/VuFind/src/VuFind/Search/UrlQueryHelper.php @@ -26,6 +26,9 @@ * @link https://vufind.org Main Site */ namespace VuFind\Search; +use VuFind\Search\Base\Options; +use VuFindSearch\Query\AbstractQuery; +use VuFindSearch\Query\Query; use VuFindSearch\Query\QueryGroup; /** @@ -40,61 +43,134 @@ use VuFindSearch\Query\QueryGroup; class UrlQueryHelper { /** - * Options object + * Configuration for this helper. * - * @var \VuFind\Search\Base\Options + * @var array */ - protected $options; + protected $config; /** - * Params object + * URL query parameters * - * @var \VuFind\Search\Base\Params + * @var array */ - protected $params; + protected $urlParams = []; /** - * URL search param + * Current query object * - * @var string + * @var AbstractQuery */ - protected $basicSearchParam = 'lookfor'; + protected $queryObject; /** - * Base parameters for every search + * Constructor * - * @var array + * @param array $urlParams Array of URL query parameters. + * @param AbstractQuery $query Query object to use to update + * URL query. + * @param array $options Configuration options for the + * object. + * @param bool $regenerateQueryParams Should we add parameters based + * on the contents of $query to $urlParams (true) or are they already there + * (false)? */ - protected $defaultParams = []; + public function __construct(array $urlParams, AbstractQuery $query, + array $options = [], $regenerateQueryParams = true + ) { + $this->config = $options; + $this->urlParams = $urlParams; + $this->queryObject = $query; + if ($regenerateQueryParams) { + $this->regenerateSearchQueryParams(); + } + } /** - * Should we suppress the standard query parameter? + * Get the name of the basic search param. * - * @var bool + * @return string */ - protected $suppressQuery = false; + protected function getBasicSearchParam() + { + return isset($this->config['basicSearchParam']) + ? $this->config['basicSearchParam'] : 'lookfor'; + } /** - * Constructor + * Reset search-related parameters in the internal array. * - * @param \VuFind\Search\Base\Params $params VuFind search results object. + * @return void */ - public function __construct(\VuFind\Search\Base\Params $params) + protected function clearSearchQueryParams() { - $this->params = $params; - $this->options = $params->getOptions(); + unset($this->urlParams[$this->getBasicSearchParam()]); + unset($this->urlParams['join']); + unset($this->urlParams['type']); + $searchParams = ['bool', 'lookfor', 'type', 'op']; + foreach (array_keys($this->urlParams) as $key) { + if (preg_match('/(' . implode('|', $searchParams) . ')[0-9]+/', $key)) { + unset($this->urlParams[$key]); + } + } } /** - * Set the name of the parameter used for basic search terms. - * - * @param string $param Parameter name to set. + * Adjust the internal query array based on the query object. * * @return void */ - public function setBasicSearchParam($param) + protected function regenerateSearchQueryParams() { - $this->basicSearchParam = $param; + $this->clearSearchQueryParams(); + if ($this->isQuerySuppressed()) { + return; + } + if ($this->queryObject instanceof QueryGroup) { + $this->urlParams['join'] = $this->queryObject->getOperator(); + foreach ($this->queryObject->getQueries() as $i => $current) { + if ($current instanceof QueryGroup) { + $operator = $current->isNegated() + ? 'NOT' : $current->getOperator(); + $this->urlParams['bool' . $i] = [$operator]; + foreach ($current->getQueries() as $inner) { + if (!isset($this->urlParams['lookfor' . $i])) { + $this->urlParams['lookfor' . $i] = []; + } + if (!isset($this->urlParams['type' . $i])) { + $this->urlParams['type' . $i] = []; + } + $this->urlParams['lookfor' . $i][] = $inner->getString(); + $this->urlParams['type' . $i][] = $inner->getHandler(); + if (null !== ($op = $inner->getOperator())) { + $this->urlParams['op' . $i][] = $op; + } + } + } + } + } else if ($this->queryObject instanceof Query) { + $search = $this->queryObject->getString(); + if (!empty($search)) { + $this->urlParams[$this->getBasicSearchParam()] = $search; + } + $type = $this->queryObject->getHandler(); + if (!empty($type)) { + $this->urlParams['type'] = $type; + } + } + } + + /** + * Look up a default value in the internal configuration array. + * + * @param string $key Name of default to load + * + * @return mixed + */ + protected function getDefault($key) + { + return isset($this->config['defaults'][$key]) + ? $this->config['defaults'][$key] : null; } /** @@ -103,11 +179,12 @@ class UrlQueryHelper * @param string $name Name of parameter * @param string $value Value of parameter * - * @return void + * @return UrlQueryHelper */ public function setDefaultParameter($name, $value) { - $this->defaultParams[$name] = $value; + $this->urlParams[$name] = $value; + return $this; } /** @@ -115,11 +192,13 @@ class UrlQueryHelper * * @param bool $suppress Should we suppress queries? * - * @return void + * @return UrlQueryHelper */ public function setSuppressQuery($suppress) { - $this->suppressQuery = $suppress; + $this->config['suppressQuery'] = $suppress; + $this->regenerateSearchQueryParams(); + return $this; } /** @@ -129,7 +208,8 @@ class UrlQueryHelper */ public function isQuerySuppressed() { - return $this->suppressQuery; + return isset($this->config['suppressQuery']) + ? (bool)$this->config['suppressQuery'] : false; } /** @@ -139,102 +219,18 @@ class UrlQueryHelper */ public function getParamArray() { - $params = $this->defaultParams; - - // Build all the URL parameters based on search object settings: - if (!$this->suppressQuery) { - if ($this->params->getSearchType() == 'advanced') { - $query = $this->params->getQuery(); - if ($query instanceof QueryGroup) { - $params['join'] = $query->getOperator(); - foreach ($query->getQueries() as $i => $current) { - if ($current instanceof QueryGroup) { - $operator = $current->isNegated() - ? 'NOT' : $current->getOperator(); - $params['bool' . $i] = [$operator]; - foreach ($current->getQueries() as $inner) { - if (!isset($params['lookfor' . $i])) { - $params['lookfor' . $i] = []; - } - if (!isset($params['type' . $i])) { - $params['type' . $i] = []; - } - $params['lookfor' . $i][] = $inner->getString(); - $params['type' . $i][] = $inner->getHandler(); - if (null !== ($op = $inner->getOperator())) { - $params['op' . $i][] = $op; - } - } - } else { - throw new \Exception('Unexpected Query object.'); - } - } - } else { - throw new \Exception('Unexpected Query object.'); - } - } else { - $search = $this->params->getDisplayQuery(); - if (!empty($search)) { - $params[$this->basicSearchParam] = $search; - } - $type = $this->params->getSearchHandler(); - if (!empty($type)) { - $params['type'] = $type; - } - } - } - $sort = $this->params->getSort(); - if (!is_null($sort) - && $sort != $this->params->getDefaultSort() - ) { - $params['sort'] = $sort; - } - $limit = $this->params->getLimit(); - if (!is_null($limit) - && $limit != $this->options->getDefaultLimit() - ) { - $params['limit'] = $limit; - } - $view = $this->params->getView(); - if (!is_null($view) - && $view != $this->options->getDefaultView() - ) { - $params['view'] = $view; - } - if ($this->params->getPage() != 1) { - $params['page'] = $this->params->getPage(); - } - $filters = $this->params->getFilters(); - if (!empty($filters)) { - $params['filter'] = []; - foreach ($filters as $field => $values) { - foreach ($values as $current) { - $params['filter'][] = $field . ':"' . $current . '"'; - } - } - } - $hiddenFilters = $this->params->getHiddenFilters(); - if (!empty($hiddenFilters)) { - foreach ($hiddenFilters as $field => $values) { - foreach ($values as $current) { - $params['hiddenFilters'][] = $field . ':"' . $current . '"'; - } - } - } - $shards = $this->params->getSelectedShards(); - if (!empty($shards)) { - sort($shards); - $defaultShards = $this->options->getDefaultSelectedShards(); - sort($defaultShards); - if (implode(':::', $shards) != implode(':::', $defaultShards)) { - $params['shard'] = $shards; - } - } - if ($this->params->hasDefaultsApplied()) { - $params['dfApplied'] = 1; - } + return $this->urlParams; + } - return $params; + /** + * Magic method: behavior when this object is treated as a string. + * + * @return string + */ + public function __toString() + { + $escape = isset($this->config['escape']) ? $this->config['escape'] : true; + return $this->getParams($escape); } /** @@ -243,14 +239,13 @@ class UrlQueryHelper * @param string $from Search term to find * @param string $to Search term to insert * - * @return string + * @return UrlQueryHelper */ public function replaceTerm($from, $to) { - $newParams = clone($this->params); - $newParams->getQuery()->replaceTerm($from, $to); - $helper = new static($newParams); - return $helper->getParams(); + $query = clone($this->queryObject); + $query->replaceTerm($from, $to); + return new static($this->urlParams, $query, $this->config); } /** @@ -260,9 +255,9 @@ class UrlQueryHelper * @param string $value Facet value * @param string $operator Facet type to add (AND, OR, NOT) * @param array $paramArray Optional array of parameters to use instead of - * getParamArray() + * internally stored values. * - * @return string + * @return UrlQueryHelper */ public function addFacet($field, $value, $operator = 'AND', $paramArray = null) { @@ -276,13 +271,13 @@ class UrlQueryHelper * * @param string $filter Filter to add * @param array $paramArray Optional array of parameters to use instead of - * getParamArray() + * internally stored values. * - * @return string + * @return UrlQueryHelper */ public function addFilter($filter, $paramArray = null) { - $params = is_null($paramArray) ? $this->getParamArray() : $paramArray; + $params = (null === $paramArray) ? $this->urlParams : $paramArray; // Add the filter: if (!isset($params['filter'])) { @@ -293,7 +288,7 @@ class UrlQueryHelper // Clear page: unset($params['page']); - return '?' . $this->buildQueryString($params); + return new static($params, $this->queryObject, $this->config, false); } /** @@ -303,11 +298,11 @@ class UrlQueryHelper */ public function removeAllFilters() { - $params = $this->getParamArray(); + $params = $this->urlParams; // Clear page: unset($params['filter']); - return '?' . $this->buildQueryString($params); + return new static($params, $this->queryObject, $this->config, false); } /** @@ -319,7 +314,48 @@ class UrlQueryHelper */ public function getParams($escape = true) { - return '?' . $this->buildQueryString($this->getParamArray(), $escape); + return '?' . $this->buildQueryString($this->urlParams, $escape); + } + + /** + * Parse apart the field and value from a URL filter string. + * + * @param string $filter A filter string from url : "field:value" + * + * @return array Array with elements 0 = field, 1 = value. + */ + protected function parseFilter($filter) + { + // Simplistic explode/trim behavior if no callback is provided: + if (!isset($this->config['parseFilterCallback']) + || !is_callable($this->config['parseFilterCallback']) + ) { + $parts = explode(':', $filter, 2); + $parts[1] = trim($parts[1], '"'); + return $parts; + } + return call_user_func($this->config['parseFilterCallback'], $filter); + } + + /** + * Given a facet field, return an array containing all aliases of that + * field. + * + * @param string $field Field to look up + * + * @return array + */ + protected function getAliasesForFacetField($field) + { + // If no callback is provided, aliases are unsupported: + if (!isset($this->config['getAliasesForFacetFieldCallback']) + || !is_callable($this->config['getAliasesForFacetFieldCallback']) + ) { + return [$field]; + } + return call_user_func( + $this->config['getAliasesForFacetFieldCallback'], $field + ); } /** @@ -330,14 +366,14 @@ class UrlQueryHelper * @param bool $escape Should we escape the string for use in the view? * @param string $operator Facet type to add (AND, OR, NOT) * @param array $paramArray Optional array of parameters to use instead of - * getParamArray() + * internally stored values. * * @return string */ public function removeFacet($field, $value, $escape = true, $operator = 'AND', $paramArray = null ) { - $params = is_null($paramArray) ? $this->getParamArray() : $paramArray; + $params = (null === $paramArray) ? $this->urlParams : $paramArray; // Account for operators: if ($operator == 'NOT') { @@ -346,14 +382,14 @@ class UrlQueryHelper $field = '~' . $field; } - $fieldAliases = $this->params->getAliasesForFacetField($field); + $fieldAliases = $this->getAliasesForFacetField($field); // Remove the filter: $newFilter = []; if (isset($params['filter']) && is_array($params['filter'])) { foreach ($params['filter'] as $current) { list($currentField, $currentValue) - = $this->params->parseFilter($current); + = $this->parseFilter($current); if (!in_array($currentField, $fieldAliases) || $currentValue != $value ) { @@ -370,7 +406,9 @@ class UrlQueryHelper // Clear page: unset($params['page']); - return '?' . $this->buildQueryString($params, $escape); + $config = $this->config; + $config['escape'] = $escape; + return new static($params, $this->queryObject, $config, false); } /** @@ -384,7 +422,7 @@ class UrlQueryHelper public function removeFilter($filter, $escape = true) { // Treat this as a special case of removeFacet: - list($field, $value) = $this->params->parseFilter($filter); + list($field, $value) = $this->parseFilter($filter); return $this->removeFacet($field, $value, $escape); } @@ -413,7 +451,7 @@ class UrlQueryHelper public function setSort($s, $escape = true) { return $this->updateQueryString( - 'sort', $s, $this->params->getDefaultSort(), $escape, true + 'sort', $s, $this->getDefault('sort'), $escape, true ); } @@ -428,10 +466,12 @@ class UrlQueryHelper */ public function setHandler($handler, $escape = true) { - return $this->updateQueryString( - 'type', $handler, $this->options->getDefaultHandler(), - $escape - ); + $query = clone($this->queryObject); + // We can only set the handler on basic queries: + if ($query instanceof Query) { + $query->setHandler($handler); + } + return new static($this->urlParams, $query, $this->config); } /** @@ -466,7 +506,7 @@ class UrlQueryHelper public function setLimit($l, $escape = true) { return $this->updateQueryString( - 'limit', $l, $this->options->getDefaultLimit(), $escape, true + 'limit', $l, $this->getDefault('limit'), $escape, true ); } @@ -481,24 +521,8 @@ class UrlQueryHelper */ public function setSearchTerms($lookfor, $escape = true) { - // If we're currently dealing with an advanced query, turn it off so - // that it can be overridden: - if ($this->params->getSearchType() == 'advanced') { - $savedSuppressQuery = $this->suppressQuery; - $this->suppressQuery = true; - } - - // Generate the URL: - $new = $this->updateQueryString( - $this->basicSearchParam, $lookfor, null, $escape, true - ); - - // Restore settings to their previous state: - if (isset($savedSuppressQuery)) { - $this->suppressQuery = $savedSuppressQuery; - } - - return $new; + $query = new Query($lookfor); + return new static($this->urlParams, $query, $this->config); } /** @@ -512,7 +536,7 @@ class UrlQueryHelper public function asHiddenFields($filter = []) { $retVal = ''; - foreach ($this->getParamArray() as $paramName => $paramValue) { + foreach ($this->urlParams as $paramName => $paramValue) { if (is_array($paramValue)) { foreach ($paramValue as $paramValue2) { if (!$this->filtered($paramName, $paramValue2, $filter)) { @@ -562,8 +586,8 @@ class UrlQueryHelper protected function updateQueryString($field, $value, $default = null, $escape = true, $clearPage = false ) { - $params = $this->getParamArray(); - if (is_null($value) || $value == $default) { + $params = $this->urlParams; + if (null === $value || $value == $default) { unset($params[$field]); } else { $params[$field] = $value; @@ -571,7 +595,9 @@ class UrlQueryHelper if ($clearPage && isset($params['page'])) { unset($params['page']); } - return '?' . $this->buildQueryString($params, $escape); + $config = $this->config; + $config['escape'] = $escape; + return new static($params, $this->queryObject, $config, false); } /** diff --git a/module/VuFind/src/VuFind/View/Helper/AbstractSearch.php b/module/VuFind/src/VuFind/View/Helper/AbstractSearch.php index 77df6b724e7..04a70abd3cb 100644 --- a/module/VuFind/src/VuFind/View/Helper/AbstractSearch.php +++ b/module/VuFind/src/VuFind/View/Helper/AbstractSearch.php @@ -84,11 +84,11 @@ abstract class AbstractSearch extends AbstractHelper } $html .= '<a href="' . $results->getUrlQuery() - ->replaceTerm($term, $data['new_term']) + ->replaceTerm($term, $data['new_term'])->getParams() . '">' . $view->escapeHtml($word) . '</a>'; if (isset($data['expand_term']) && !empty($data['expand_term'])) { $url = $results->getUrlQuery() - ->replaceTerm($term, $data['expand_term']); + ->replaceTerm($term, $data['expand_term'])->getParams(); $html .= $this->renderExpandLink($url, $view); } } diff --git a/module/VuFind/src/VuFind/View/Helper/Root/ResultFeed.php b/module/VuFind/src/VuFind/View/Helper/Root/ResultFeed.php index eecc7864725..e93c66bdd0c 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/ResultFeed.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/ResultFeed.php @@ -121,7 +121,7 @@ class ResultFeed extends AbstractHelper implements TranslatorAwareInterface ); } $feed->setLink( - $baseUrl . $results->getUrlQuery()->setViewParam(null, false) + $baseUrl . $results->getUrlQuery()->setViewParam(null)->getParams(false) ); $feed->setFeedLink( $baseUrl . $results->getUrlQuery()->getParams(false), @@ -137,14 +137,14 @@ class ResultFeed extends AbstractHelper implements TranslatorAwareInterface // add atom links for easier paging $feed->addOpensearchLink( - $baseUrl . $results->getUrlQuery()->setPage(1, false), + $baseUrl . $results->getUrlQuery()->setPage(1)->getParams(false), 'first', $params->getView() ); if ($params->getPage() > 1) { $feed->addOpensearchLink( $baseUrl . $results->getUrlQuery() - ->setPage($params->getPage() - 1, false), + ->setPage($params->getPage() - 1)->getParams(false), 'previous', $params->getView() ); @@ -153,13 +153,13 @@ class ResultFeed extends AbstractHelper implements TranslatorAwareInterface if ($params->getPage() < $lastPage) { $feed->addOpensearchLink( $baseUrl . $results->getUrlQuery() - ->setPage($params->getPage() + 1, false), + ->setPage($params->getPage() + 1)->getParams(false), 'next', $params->getView() ); } $feed->addOpensearchLink( - $baseUrl . $results->getUrlQuery()->setPage($lastPage, false), + $baseUrl . $results->getUrlQuery()->setPage($lastPage)->getParams(false), 'last', $params->getView() ); diff --git a/module/VuFind/src/VuFind/View/Helper/Root/SortFacetList.php b/module/VuFind/src/VuFind/View/Helper/Root/SortFacetList.php index cf17f6b55e7..3bfcf4a1783 100644 --- a/module/VuFind/src/VuFind/View/Helper/Root/SortFacetList.php +++ b/module/VuFind/src/VuFind/View/Helper/Root/SortFacetList.php @@ -59,8 +59,8 @@ class SortFacetList extends AbstractHelper $results->getParams()->setLimit($results->getOptions()->getDefaultLimit()); $urlHelper = $this->getView()->plugin('url'); foreach ($list as $value) { - $url = $urlHelper($searchRoute) - . $results->getUrlQuery()->addFacet($field, $value['value']); + $url = $urlHelper($searchRoute) . $results->getUrlQuery() + ->addFacet($field, $value['value'])->getParams(); $facets[$url] = $value['displayText']; } natcasesort($facets); diff --git a/module/VuFind/tests/unit-tests/src/VuFindTest/Search/UrlQueryHelperTest.php b/module/VuFind/tests/unit-tests/src/VuFindTest/Search/UrlQueryHelperTest.php new file mode 100644 index 00000000000..3543fedbf19 --- /dev/null +++ b/module/VuFind/tests/unit-tests/src/VuFindTest/Search/UrlQueryHelperTest.php @@ -0,0 +1,161 @@ +<?php + +/** + * UrlQueryHelper unit tests. + * + * PHP version 5 + * + * Copyright (C) Villanova University 2010. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + * + * @category VuFind + * @package Tests + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org/wiki/development:testing:unit_tests Wiki + */ +namespace VuFindTest\Search; + +use VuFind\Search\UrlQueryHelper; +use VuFind\Search\Factory\UrlQueryHelperFactory; +use VuFindTest\Unit\TestCase as TestCase; +use VuFindSearch\Query\Query; + +/** + * UrlQueryHelper unit tests. + * + * @category VuFind + * @package Tests + * @author Demian Katz <demian.katz@villanova.edu> + * @license http://opensource.org/licenses/gpl-2.0.php GNU General Public License + * @link https://vufind.org/wiki/development:testing:unit_tests Wiki + */ +class UrlQueryHelperTest extends TestCase +{ + /** + * Test the basic functionality of the helper. + * + * @return void + */ + public function testBasicFunctionality() + { + // Test basic getters + $query = new Query('search'); + $helper = new UrlQueryHelper(['foo' => 'bar'], $query); + $this->assertEquals('?foo=bar&lookfor=search', $helper->getParams()); + $this->assertEquals('?foo=bar&lookfor=search', (string)$helper); + $this->assertEquals( + ['foo' => 'bar', 'lookfor' => 'search'], $helper->getParamArray() + ); + $this->assertEquals( + '<input type="hidden" name="foo" value="bar" />', + $helper->asHiddenFields(['lookfor' => '/.*/']) + ); + + // Test setDefaultParameters and disabling escaping + $this->assertEquals( + '?foo=baz&lookfor=search', + $helper->setDefaultParameter('foo', 'baz')->getParams(false) + ); + + // Test query suppression + $this->assertEquals(false, $helper->isQuerySuppressed()); + $helper->setSuppressQuery(true); + $this->assertEquals(true, $helper->isQuerySuppressed()); + $this->assertEquals('?foo=baz', $helper->getParams()); + $helper->setSuppressQuery(false); + $this->assertEquals(false, $helper->isQuerySuppressed()); + $this->assertEquals('?foo=baz&lookfor=search', $helper->getParams(false)); + + // Test replacing query terms + $this->assertEquals( + '?foo=baz&lookfor=srch', + $helper->replaceTerm('search', 'srch')->getParams() + ); + $this->assertEquals( + '?foo=baz&lookfor=srch', + $helper->setSearchTerms('srch')->getParams() + ); + + // Test adding/removing facets and filters + $faceted = $helper->addFacet('f', '1')->addFilter('f:2'); + $this->assertEquals( + '?foo=baz&lookfor=search&filter%5B%5D=f%3A%221%22&filter%5B%5D=f%3A2', + $faceted->getParams(false) + ); + $this->assertEquals( + '?foo=baz&lookfor=search&filter%5B%5D=f%3A%221%22', + $faceted->removeFacet('f', '2')->getParams(false) + ); + $this->assertEquals( + '?foo=baz&lookfor=search&filter%5B%5D=f%3A2', + $faceted->removeFilter('f:1')->getParams(false) + ); + $this->assertEquals( + '?foo=baz&lookfor=search', + $faceted->removeAllFilters()->getParams(false) + ); + + // Test stacking setters + $this->assertEquals( + '?foo=baz&sort=title&view=grid&lookfor=search&type=x&limit=50&page=3', + $helper->setSort('title')->setViewParam('grid')->setHandler('x') + ->setLimit(50)->setPage(3)->getParams(false) + ); + } + + /** + * Test advanced search param building. + * + * @return void + */ + public function testAdvancedSearch() + { + $fixturePath = realpath(__DIR__ . '/../../../../fixtures/searches') . '/advanced/'; + $q = unserialize(file_get_contents($fixturePath . 'query')); + $helper = new UrlQueryHelper([], $q); + $this->assertEquals( + '?join=OR&bool0%5B%5D=AND&lookfor0%5B%5D=oranges&lookfor0%5B%5D=bananas' + . '&lookfor0%5B%5D=pears&type0%5B%5D=CallNumber&type0%5B%5D=toc' + . '&type0%5B%5D=ISN&bool1%5B%5D=OR&lookfor1%5B%5D=cars' + . '&lookfor1%5B%5D=trucks&type1%5B%5D=Title&type1%5B%5D=Subject' + . '&bool2%5B%5D=NOT&lookfor2%5B%5D=squid&type2%5B%5D=AllFields', + $helper->getParams(false) + ); + } + + /** + * Test that the factory does its job properly. + * + * @return void + */ + public function testFactory() + { + $factory = new UrlQueryHelperFactory(); + $config = $this->getMock('VuFind\Config\PluginManager'); + $params = new \VuFindTest\Search\TestHarness\Params( + new \VuFindTest\Search\TestHarness\Options($config), $config + ); + $params->setBasicSearch('foo', 'bar'); + $params->setLimit(100); + $params->setPage(5); + $params->setView('grid'); + $helper = $factory->fromParams($params); + $this->assertEquals( + '?limit=100&view=grid&page=5&lookfor=foo&type=bar', + $helper->getParams(false) + ); + } +} -- GitLab