From 23acf6e70f013b84110b8e6abdde40273e29018a Mon Sep 17 00:00:00 2001 From: Erik Bernhardson Date: Tue, 28 Jul 2015 11:26:21 -0700 Subject: [PATCH] Move query rewriting into search backend Special:Search recently gained query rewriting behavior when the original query returned no results. We want to expose this query rewriting behavior to both api and web requests. Additionally we want to be able to test different configurations of the query suggestions. This patch allows for both by moving the rewriting from core into the search backend. This defaults to enabled for Special:Search, and disabled for ApiQuerySearch. Internal code that talks to the search backend needs to specifically enable this feature. Bug: T106888 Change-Id: I0a8f75759f9148f53358707369b8a7128215de86 --- includes/api/ApiQuerySearch.php | 11 +++- includes/api/i18n/en.json | 1 + includes/api/i18n/qqq.json | 1 + includes/search/SearchResultSet.php | 27 +++++++++ includes/specials/SpecialSearch.php | 13 ++--- .../includes/specials/SpecialSearchTest.php | 57 ++++++++++--------- 6 files changed, 73 insertions(+), 37 deletions(-) diff --git a/includes/api/ApiQuerySearch.php b/includes/api/ApiQuerySearch.php index 65fd727773..b866f43e09 100644 --- a/includes/api/ApiQuerySearch.php +++ b/includes/api/ApiQuerySearch.php @@ -82,6 +82,7 @@ class ApiQuerySearch extends ApiQueryGeneratorBase { SearchEngine::create( $params['backend'] ) : SearchEngine::create(); $search->setLimitOffset( $limit + 1, $params['offset'] ); $search->setNamespaces( $params['namespace'] ); + $search->setFeatureData( 'rewrite', (bool)$params['enablerewrites'] ); $query = $search->transformSearchTerm( $query ); $query = $search->replacePrefixes( $query ); @@ -134,6 +135,12 @@ class ApiQuerySearch extends ApiQueryGeneratorBase { $apiResult->addValue( array( 'query', 'searchinfo' ), 'suggestionsnippet', $matches->getSuggestionSnippet() ); } + if ( isset( $searchInfo['rewrittenquery'] ) && $matches->hasRewrittenQuery() ) { + $apiResult->addValue( array( 'query', 'searchinfo' ), + 'rewrittenquery', $matches->getQueryAfterRewrite() ); + $apiResult->addValue( array( 'query', 'searchinfo' ), + 'rewrittenquerysnippet', $matches->getQueryAfterRewriteSnippet() ); + } } // Add the search results to the result @@ -303,10 +310,11 @@ class ApiQuerySearch extends ApiQueryGeneratorBase { ) ), 'info' => array( - ApiBase::PARAM_DFLT => 'totalhits|suggestion', + ApiBase::PARAM_DFLT => 'totalhits|suggestion|rewrittenquery', ApiBase::PARAM_TYPE => array( 'totalhits', 'suggestion', + 'rewrittenquery', ), ApiBase::PARAM_ISMULTI => true, ), @@ -342,6 +350,7 @@ class ApiQuerySearch extends ApiQueryGeneratorBase { ApiBase::PARAM_MAX2 => ApiBase::LIMIT_SML2 ), 'interwiki' => false, + 'enablerewrites' => false, ); $alternatives = SearchEngine::getSearchTypes(); diff --git a/includes/api/i18n/en.json b/includes/api/i18n/en.json index d3189c6194..212dc51210 100644 --- a/includes/api/i18n/en.json +++ b/includes/api/i18n/en.json @@ -1036,6 +1036,7 @@ "apihelp-query+search-param-limit": "How many total pages to return.", "apihelp-query+search-param-interwiki": "Include interwiki results in the search, if available.", "apihelp-query+search-param-backend": "Which search backend to use, if not the default.", + "apihelp-query+search-param-enablerewrites": "Enable internal query rewriting. Some search backends can rewrite the query into one its thinks gives better results, such as correcting spelling errors.", "apihelp-query+search-example-simple": "Search for meaning.", "apihelp-query+search-example-text": "Search texts for meaning.", "apihelp-query+search-example-generator": "Get page info about the pages returned for a search for meaning.", diff --git a/includes/api/i18n/qqq.json b/includes/api/i18n/qqq.json index 5ac1e0dd77..58872a03af 100644 --- a/includes/api/i18n/qqq.json +++ b/includes/api/i18n/qqq.json @@ -965,6 +965,7 @@ "apihelp-query+search-param-limit": "{{doc-apihelp-param|query+search|limit}}", "apihelp-query+search-param-interwiki": "{{doc-apihelp-param|query+search|interwiki}}", "apihelp-query+search-param-backend": "{{doc-apihelp-param|query+search|backend}}", + "apihelp-query+search-param-enablerewrites": "{{doc-apihelp-param|query+search|enablerewrites}}", "apihelp-query+search-example-simple": "{{doc-apihelp-example|query+search}}", "apihelp-query+search-example-text": "{{doc-apihelp-example|query+search}}", "apihelp-query+search-example-generator": "{{doc-apihelp-example|query+search}}", diff --git a/includes/search/SearchResultSet.php b/includes/search/SearchResultSet.php index 0a05eef159..8d18b0e6ed 100644 --- a/includes/search/SearchResultSet.php +++ b/includes/search/SearchResultSet.php @@ -60,6 +60,33 @@ class SearchResultSet { return null; } + /** + * Some search modes will run an alternative query that it thinks gives + * a better result than the provided search. Returns true if this has + * occured. + * + * @return bool + */ + function hasRewrittenQuery() { + return false; + } + + /** + * @return string|null The search the query was internally rewritten to, + * or null when the result of the original query was returned. + */ + function getQueryAfterRewrite() { + return null; + } + + /** + * @return string|null Same as self::getQueryAfterRewrite(), but in HTML + * and with changes highlighted. Null when the query was not rewritten. + */ + function getQueryAfterRewriteSnippet() { + return null; + } + /** * Some search modes return a suggested alternate term if there are * no exact hits. Returns true if there is one on this set. diff --git a/includes/specials/SpecialSearch.php b/includes/specials/SpecialSearch.php index af2dc94b32..6606c7f883 100644 --- a/includes/specials/SpecialSearch.php +++ b/includes/specials/SpecialSearch.php @@ -216,6 +216,7 @@ class SpecialSearch extends SpecialPage { global $wgContLang; $search = $this->getSearchEngine(); + $search->setFeatureData( 'rewrite', $this->runSuggestion ); $search->setLimitOffset( $this->limit, $this->offset ); $search->setNamespaces( $this->namespaces ); $search->prefix = $this->mPrefix; @@ -272,12 +273,8 @@ class SpecialSearch extends SpecialPage { // did you mean... suggestions $didYouMeanHtml = ''; if ( $showSuggestion && $textMatches && !$textStatus ) { - if ( $this->shouldRunSuggestedQuery( $textMatches ) ) { - $newMatches = $search->searchText( $textMatches->getSuggestionQuery() ); - if ( $newMatches instanceof SearchResultSet && $newMatches->numRows() > 0 ) { - $didYouMeanHtml = $this->getDidYouMeanRewrittenHtml( $term, $textMatches ); - $textMatches = $newMatches; - } + if ( $textMatches->hasRewrittenQuery() ) { + $didYouMeanHtml = $this->getDidYouMeanRewrittenHtml( $term, $textMatches ); } elseif ( $textMatches->hasSuggestion() ) { $didYouMeanHtml = $this->getDidYouMeanHtml( $textMatches ); } @@ -463,7 +460,7 @@ class SpecialSearch extends SpecialPage { // Showing results for '$rewritten' // Search instead for '$orig' - $params = array( 'search' => $textMatches->getSuggestionQuery() ); + $params = array( 'search' => $textMatches->getQueryAfterRewrite() ); if ( $this->fulltext != null ) { $params['fulltext'] = $this->fulltext; } @@ -471,7 +468,7 @@ class SpecialSearch extends SpecialPage { $rewritten = Linker::linkKnown( $this->getPageTitle(), - $textMatches->getSuggestionSnippet() ?: null, + $textMatches->getQueryAfterRewriteSnippet() ?: null, array(), $stParams ); diff --git a/tests/phpunit/includes/specials/SpecialSearchTest.php b/tests/phpunit/includes/specials/SpecialSearchTest.php index 7e60fddec2..13c283816d 100644 --- a/tests/phpunit/includes/specials/SpecialSearchTest.php +++ b/tests/phpunit/includes/specials/SpecialSearchTest.php @@ -145,34 +145,25 @@ class SpecialSearchTest extends MediaWikiTestCase { public function provideRewriteQueryWithSuggestion() { return array( array( - 'With results and a suggestion does not run suggested query', + 'With suggestion and no rewritten query shows did you mean', '/Did you mean: ]+>first suggestion/', - array( - new SpecialSearchTestMockResultSet( 'first suggestion', array( - SearchResult::newFromTitle( Title::newMainPage() ), - ) ), - new SpecialSearchTestMockResultSet( 'was never run', array() ), - ), + new SpecialSearchTestMockResultSet( 'first suggestion', null, array( + SearchResult::newFromTitle( Title::newMainPage() ), + ) ), ), array( - 'With no results and a suggestion responds with suggested query results', + 'With rewritten query informs user of change', '/Showing results for ]+>first suggestion/', - array( - new SpecialSearchTestMockResultSet( 'first suggestion', array() ), - new SpecialSearchTestMockResultSet( 'second suggestion', array( - SearchResult::newFromTitle( Title::newMainPage() ), - ) ), - ), + new SpecialSearchTestMockResultSet( 'asdf', 'first suggestion', array( + SearchResult::newFromTitle( Title::newMainPage() ), + ) ), ), array( 'When both queries have no results user gets no results', '/There were no results matching the query/', - array( - new SpecialSearchTestMockResultSet( 'first suggestion', array() ), - new SpecialSearchTestMockResultSet( 'second suggestion', array() ), - ), + new SpecialSearchTestMockResultSet( 'first suggestion', 'first suggestion', array() ), ), ); } @@ -180,8 +171,8 @@ class SpecialSearchTest extends MediaWikiTestCase { /** * @dataProvider provideRewriteQueryWithSuggestion */ - public function testRewriteQueryWithSuggestion( $message, $expectRegex, $fromResults ) { - $mockSearchEngine = $this->mockSearchEngine( $fromResults ); + public function testRewriteQueryWithSuggestion( $message, $expectRegex, $results ) { + $mockSearchEngine = $this->mockSearchEngine( $results ); $search = $this->getMockBuilder( 'SpecialSearch' ) ->setMethods( array( 'getSearchEngine' ) ) ->getMock(); @@ -199,17 +190,14 @@ class SpecialSearchTest extends MediaWikiTestCase { } } - protected function mockSearchEngine( array $returnValues ) { + protected function mockSearchEngine( $results ) { $mock = $this->getMockBuilder( 'SearchEngine' ) - ->setMethods( array( 'searchText' ) ) + ->setMethods( array( 'searchText', 'searchTitle' ) ) ->getMock(); $mock->expects( $this->any() ) ->method( 'searchText' ) - ->will( call_user_func_array( - array( $this, 'onConsecutiveCalls' ), - array_map( array( $this, 'returnValue' ), $returnValues ) - ) ); + ->will( $this->returnValue( $results ) ); return $mock; } @@ -219,9 +207,10 @@ class SpecialSearchTestMockResultSet extends SearchResultSet { protected $results; protected $suggestion; - public function __construct( $suggestion = null, array $results = array(), $containedSyntax = false) { - $this->results = $results; + public function __construct( $suggestion = null, $rewrittenQuery = null, array $results = array(), $containedSyntax = false) { $this->suggestion = $suggestion; + $this->rewrittenQuery = $rewrittenQuery; + $this->results = $results; $this->containedSyntax = $containedSyntax; } @@ -244,4 +233,16 @@ class SpecialSearchTestMockResultSet extends SearchResultSet { public function getSuggestionSnippet() { return $this->suggestion; } + + public function hasRewrittenQuery() { + return $this->rewrittenQuery !== null; + } + + public function getQueryAfterRewrite() { + return $this->rewrittenQuery; + } + + public function getQueryAfterRewriteSnippet() { + return htmlspecialchars( $this->rewrittenQuery ); + } } -- 2.20.1