DCausse has uploaded a new change for review.
https://gerrit.wikimedia.org/r/318548
Change subject: Fix errors when searching ideographic whitespaces and keep
track of the original query
......................................................................
Fix errors when searching ideographic whitespaces and keep track of the
original query
Cirrus should log original queries not the query that was
munged/trimmed by cirrus itself.
Moved the trim and empty query detection into the Searcher so that the
state is coherent when SpecialSearch calls our hooks.
Bug: T149416
Change-Id: I395bada2d74f805b7a62901b28890b3121ecde10
---
M includes/CirrusSearch.php
M includes/InterwikiSearcher.php
M includes/Search/SearchContext.php
M includes/Searcher.php
M tests/unit/SearcherTest.php
5 files changed, 42 insertions(+), 21 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/CirrusSearch
refs/changes/48/318548/1
diff --git a/includes/CirrusSearch.php b/includes/CirrusSearch.php
index 2ecc29e..c6302c7 100644
--- a/includes/CirrusSearch.php
+++ b/includes/CirrusSearch.php
@@ -330,15 +330,6 @@
* @return null|Status|ResultSet
*/
protected function searchTextReal( $term, SearchConfig $config = null,
$forceLocal = false ) {
- // Convert the unicode character 'ideographic whitespace' into
standard
- // whitespace. Cirrussearch treats them both as normal
whitespace, but
- // the preceding isn't appropriately trimmed.
- $term = trim( str_replace( "\xE3\x80\x80", " ", $term) );
- // No searching for nothing! That takes forever!
- if ( $term === '' ) {
- return null;
- }
-
if ( $config ) {
$this->indexBaseName = $config->get(
SearchConfig::INDEX_BASE_NAME );
}
diff --git a/includes/InterwikiSearcher.php b/includes/InterwikiSearcher.php
index ffa237d..30f5c8c 100644
--- a/includes/InterwikiSearcher.php
+++ b/includes/InterwikiSearcher.php
@@ -73,6 +73,7 @@
);
$this->searchContext->setLimitSearchToLocalWiki( true );
+ $this->searchContext->setOriginalSearchTerm( $term );
$this->buildFullTextSearch( $term, false );
$context = $this->searchContext;
@@ -92,7 +93,7 @@
}
}
- $results = $this->searchMulti( $searches, $term, $resultsTypes
);
+ $results = $this->searchMulti( $searches, $resultsTypes );
if ( !$results->isOK() ) {
return null;
}
diff --git a/includes/Search/SearchContext.php
b/includes/Search/SearchContext.php
index 8584a27..90912c7 100644
--- a/includes/Search/SearchContext.php
+++ b/includes/Search/SearchContext.php
@@ -170,6 +170,11 @@
private $cacheTtl = 0;
/**
+ * @param string The original search
+ */
+ private $originalSearchTerm;
+
+ /**
* @param SearchConfig $config
* @param int[]|null $namespaces
*/
@@ -647,4 +652,18 @@
public function setCacheTtl( $ttl ) {
$this->cacheTtl = $ttl;
}
+
+ /**
+ * @return string the original search term
+ */
+ public function getOriginalSearchTerm() {
+ return $this->originalSearchTerm;
+ }
+
+ /**
+ * @param string set the original search term
+ */
+ public function setOriginalSearchTerm( $term ) {
+ $this->originalSearchTerm = $term;
+ }
}
diff --git a/includes/Searcher.php b/includes/Searcher.php
index 56b376c..3413eee 100644
--- a/includes/Searcher.php
+++ b/includes/Searcher.php
@@ -210,6 +210,7 @@
public function nearMatchTitleSearch( $term ) {
$this->checkTitleSearchRequestLength( $term );
+ $this->searchContext->setOriginalSearchTerm( $term );
// 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.
$highlightQuery = new \Elastica\Query\MultiMatch();
@@ -230,7 +231,7 @@
$this->searchContext->setHighlightQuery( $highlightQuery );
$this->searchContext->setSearchType( 'near_match' );
- return $this->searchOne( $term );
+ return $this->searchOne();
}
/**
@@ -240,6 +241,7 @@
*/
public function prefixSearch( $term ) {
$this->checkTitleSearchRequestLength( $term );
+ $this->searchContext->setOriginalSearchTerm( $term );
$this->searchContext->setSearchType( 'prefix' );
if ( strlen( $term ) > 0 ) {
@@ -270,7 +272,7 @@
/** @suppress PhanDeprecatedFunction */
$this->searchContext->setBoostLinks( true );
- return $this->searchOne( $term );
+ return $this->searchOne();
}
/**
@@ -290,8 +292,14 @@
* @return FullTextQueryBuilder
*/
protected function buildFullTextSearch( $term, $showSuggestion ) {
- // save original term for logging
- $originalTerm = $term;
+ // Convert the unicode character 'ideographic whitespace' into
standard
+ // whitespace. Cirrussearch treats them both as normal
whitespace, but
+ // the preceding isn't appropriately trimmed.
+ // No searching for nothing! That takes forever!
+ $term = trim( str_replace( "\xE3\x80\x80", " ", $term ) );
+ if ( $term === '' ) {
+ $this->searchContext->setResultsPossible( false );
+ }
$term = Util::stripQuestionMarks( $term, $this->config->get(
'CirrusSearchStripQuestionMarks' ) );
// Transform Mediawiki specific syntax to filters and extra
(pre-escaped) query string
@@ -355,19 +363,20 @@
*/
public function searchText( $term, $showSuggestion ) {
$checkLengthStatus = $this->checkTextSearchRequestLength( $term
);
+ $this->searchContext->setOriginalSearchTerm( $term );
if ( !$checkLengthStatus->isOK() ) {
return $checkLengthStatus;
}
$qb = $this->buildFullTextSearch( $term, $showSuggestion );
- $result = $this->searchOne( $term );
+ $result = $this->searchOne();
if ( !$result->isOK() && ElasticaErrorHandler::isParseError(
$result ) ) {
if ( $qb->buildDegraded( $this->searchContext ) ) {
// If that doesn't work we're out of luck but
it should. There no guarantee it'll work properly
// with the syntax we've built above but it'll
do _something_ and we'll still work on fixing all
// the parse errors that come in.
- $result = $this->searchOne( $term );
+ $result = $this->searchOne();
}
}
@@ -574,14 +583,14 @@
return $search;
}
- protected function searchOne( $for ) {
+ protected function searchOne() {
$search = $this->buildSearch();
if ( !$this->searchContext->areResultsPossible() ) {
return Status::newGood( new SearchResultSet( true ) );
}
- $result = $this->searchMulti( [$search], $for );
+ $result = $this->searchMulti( [$search] );
if ( $result->isOK() ) {
// Convert array of responses to single value
$value = $result->getValue();
@@ -595,12 +604,11 @@
* Powers full-text-like searches including prefix search.
*
* @param \Elastica\Search[] $searches
- * @param string $for
* @param ResultsType[] $resultsTypes Specific ResultType instances to
use with $searches. Any
* search without a matching key in this array uses $this->resultsType.
* @return Status results from the query transformed by the resultsType
*/
- protected function searchMulti( $searches, $for, array $resultsTypes =
[] ) {
+ protected function searchMulti( $searches, array $resultsTypes = [] ) {
if ( $this->limit <= 0 && ! $this->returnQuery ) {
if ( $this->returnResult ) {
return Status::newGood( [
@@ -624,7 +632,7 @@
"{queryType} search for '{query}'",
$this->searchContext->getSearchType(),
[
- 'query' => $for,
+ 'query' =>
$this->searchContext->getOriginalSearchTerm(),
'limit' => $this->limit ?: null,
// null means not requested, '' means not
found. If found
// parent::buildLogContext will replace the ''
with an
diff --git a/tests/unit/SearcherTest.php b/tests/unit/SearcherTest.php
index 1995ae9..02d74a3 100644
--- a/tests/unit/SearcherTest.php
+++ b/tests/unit/SearcherTest.php
@@ -19,6 +19,8 @@
foreach ( glob( __DIR__ . '/fixtures/searchText/*.query' ) as
$queryFile ) {
$testName = substr( basename( $queryFile ), 0, -6 );
$query = file_get_contents( $queryFile );
+ // Remove trailing newline
+ $query = preg_replace( '/\n$/', '', $query );
foreach ( $configs as $configName => $config ) {
$expectedFile = substr( $queryFile, 0, -5 ) .
$configName . '.expected';
$expected = is_file( $expectedFile )
--
To view, visit https://gerrit.wikimedia.org/r/318548
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I395bada2d74f805b7a62901b28890b3121ecde10
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: DCausse <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
