Manybubbles has uploaded a new change for review. https://gerrit.wikimedia.org/r/176928
Change subject: Empty prefix search searches everything ...................................................................... Empty prefix search searches everything Before this empty prefix searches would search for only pages with empty titles which was totally weird and wrong wrong wrong. Now it searches for all pages in the namespace and sorts them based on weighting. That is what lsearchd used to do I believe. Fixes T76350. Change-Id: I36763d13c04ad2f2cd9e796aa699ac1a8ab4515f --- M includes/Hooks.php M includes/Searcher.php M tests/browser/features/prefix_search.feature M tests/browser/features/step_definitions/search_steps.rb 4 files changed, 49 insertions(+), 21 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch refs/changes/28/176928/1 diff --git a/includes/Hooks.php b/includes/Hooks.php index e6f0f68..d6dd49a 100644 --- a/includes/Hooks.php +++ b/includes/Hooks.php @@ -6,6 +6,7 @@ use \BetaFeatures; use \CirrusSearch; use \CirrusSearch\Search\FancyTitleResultsType; +use \CirrusSearch\Search\TitleResultsType; use \DeferredUpdates; use \JobQueueGroup; use \LinksUpdate; @@ -389,12 +390,21 @@ public static function prefixSearch( $namespaces, $search, $limit, &$results ) { $user = RequestContext::getMain()->getUser(); $searcher = new Searcher( 0, $limit, $namespaces, $user ); - $searcher->setResultsType( new FancyTitleResultsType( 'prefix' ) ); + if ( $search ) { + $searcher->setResultsType( new FancyTitleResultsType( 'prefix' ) ); + } else { + // Empty searches always find the title. + $searcher->setResultsType( new TitleResultsType() ); + } $status = $searcher->prefixSearch( $search ); // There is no way to send errors or warnings back to the caller here so we have to make do with // only sending results back if there are results and relying on the logging done at the status // constrution site to log errors. if ( $status->isOK() ) { + if ( !$search ) { + // No need to unpack the simple title matches from non-fancy TitleResultsType + return $status->getValue(); + } $results = array(); foreach ( $status->getValue() as $match ) { if ( isset( $match[ 'titleMatch' ] ) ) { diff --git a/includes/Searcher.php b/includes/Searcher.php index 59a690f..d5411d5 100644 --- a/includes/Searcher.php +++ b/includes/Searcher.php @@ -284,25 +284,29 @@ self::checkTitleSearchRequestLength( $search ); - if ( $wgCirrusSearchPrefixSearchStartsWithAnyWord ) { - $match = new \Elastica\Query\Match(); - $match->setField( 'title.word_prefix', array( - 'query' => $search, - 'analyzer' => 'plain', - 'operator' => 'and', - ) ); - $this->filters[] = new \Elastica\Filter\Query( $match ); + if ( $search ) { + if ( $wgCirrusSearchPrefixSearchStartsWithAnyWord ) { + $match = new \Elastica\Query\Match(); + $match->setField( 'title.word_prefix', array( + 'query' => $search, + 'analyzer' => 'plain', + 'operator' => 'and', + ) ); + $this->filters[] = new \Elastica\Filter\Query( $match ); + } else { + // Elasticsearch seems to have trouble extracting the proper terms to highlight + // from the default query we make so we feed it exactly the right query to highlight. + $this->query = new \Elastica\Query\MultiMatch(); + $this->query->setQuery( $search ); + $this->query->setFields( array( + 'title.prefix^' . $wgCirrusSearchPrefixWeights[ 'title' ], + 'redirect.title.prefix^' . $wgCirrusSearchPrefixWeights[ 'redirect' ], + 'title.prefix_asciifolding^' . $wgCirrusSearchPrefixWeights[ 'title_asciifolding' ], + 'redirect.title.prefix_asciifolding^' . $wgCirrusSearchPrefixWeights[ 'redirect_asciifolding' ], + ) ); + } } else { - // Elasticsearch seems to have trouble extracting the proper terms to highlight - // from the default query we make so we feed it exactly the right query to highlight. - $this->query = new \Elastica\Query\MultiMatch(); - $this->query->setQuery( $search ); - $this->query->setFields( array( - 'title.prefix^' . $wgCirrusSearchPrefixWeights[ 'title' ], - 'redirect.title.prefix^' . $wgCirrusSearchPrefixWeights[ 'redirect' ], - 'title.prefix_asciifolding^' . $wgCirrusSearchPrefixWeights[ 'title_asciifolding' ], - 'redirect.title.prefix_asciifolding^' . $wgCirrusSearchPrefixWeights[ 'redirect_asciifolding' ], - ) ); + $this->query = new \Elastica\Query\MatchAll(); } $this->boostTemplates = self::getDefaultBoostTemplates(); $this->boostLinks = true; diff --git a/tests/browser/features/prefix_search.feature b/tests/browser/features/prefix_search.feature index 0a6f2c2..5bfccd4 100644 --- a/tests/browser/features/prefix_search.feature +++ b/tests/browser/features/prefix_search.feature @@ -93,6 +93,14 @@ | Accent Sorting | Accent Sorting | Áccent Sorting | | accent Sorting | Accent Sorting | Áccent Sorting | + Scenario: Searching for a bare namespace finds everything in the namespace + Given a page named Template talk:Foo exists + And within 20 seconds searching for Template talk:Foo yields Template talk:Foo as the first result + And I am at a random page + When I type template talk: into the search box + Then suggestions should appear + And Template talk:Foo is in the suggestions + # Just take too long to run on a regular basis # @redirect @huge # Scenario: Prefix search on pages with tons of redirects is reasonably fast diff --git a/tests/browser/features/step_definitions/search_steps.rb b/tests/browser/features/step_definitions/search_steps.rb index 8d2cb25..8d85a1d 100644 --- a/tests/browser/features/step_definitions/search_steps.rb +++ b/tests/browser/features/step_definitions/search_steps.rb @@ -117,10 +117,16 @@ on(SearchPage).second_result.should == title end end -Then(/^(.+) is not in the suggestions$/) do |title| +Then(/^(.+) is( not)? in the suggestions$/) do |title, should_not| + found = false on(SearchPage).all_results_elements.each do |result| - result.text.should_not == title + if should_not then + result.text.should_not == title + else + found |= result.text == title + end end + found.should == true unless should_not end Then(/^I should be offered to search for (.+)$/) do |term| on(SearchPage).search_special.should == "containing...\n" + term -- To view, visit https://gerrit.wikimedia.org/r/176928 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I36763d13c04ad2f2cd9e796aa699ac1a8ab4515f Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/extensions/CirrusSearch Gerrit-Branch: master Gerrit-Owner: Manybubbles <never...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits