jenkins-bot has submitted this change and it was merged.
Change subject: Only return Status objects from CirrusSearch::searchText
......................................................................
Only return Status objects from CirrusSearch::searchText
There are times when elasticsearch can return partial results, such as
timeouts. To be able to tell users about these we need to be able to
pass back a Status object that contains both the result set and some
warnings.
Depends on a core patch which changes the assumptions around using
SearchEngine::searchText() to allow for Status objects that are 'OK'
rather than only 'Good'. There are also patches in Flow and
ProofreadPages extensions that will need to be deployed in the same
train.
Change-Id: I34a522bee18d14137d4640b7d5cf2893985b6eea
Depends-On: Ic5e0db727790f4fd189caa54ea5f01672d6a8ea4
Bug: T134157
---
M i18n/en.json
M i18n/qqq.json
M includes/CirrusSearch.php
M includes/Searcher.php
M tests/unit/LanguageDetectTest.php
M tests/unit/SearcherTest.php
6 files changed, 38 insertions(+), 31 deletions(-)
Approvals:
Cindy-the-browser-test-bot: Looks good to me, but someone else must approve
Tjones: Looks good to me, but someone else must approve
DCausse: Looks good to me, approved
jenkins-bot: Verified
diff --git a/i18n/en.json b/i18n/en.json
index baa7479..8acbc2a 100644
--- a/i18n/en.json
+++ b/i18n/en.json
@@ -52,5 +52,7 @@
"cirrussearch-pref-completion-legacy-section-desc": "Prefix search",
"cirrussearch-pref-completion-legacy-section-legend": "The legacy
search-as-you-type suggestion algorithm.",
"cirrussearch-completion-profile-classic-pref-name": "Classic prefix
search",
- "cirrussearch-completion-profile-classic-pref-desc": "No typo
correction. Matches the beginning of titles."
+ "cirrussearch-completion-profile-classic-pref-desc": "No typo
correction. Matches the beginning of titles.",
+ "cirrussearch-timed-out": "The search timed out, only partial results
are available.",
+ "cirrussearch-regex-timed-out": "The regex search timed out, only
partial results are available. Try simplifying your regular expression to get
complete results."
}
diff --git a/i18n/qqq.json b/i18n/qqq.json
index 6eeabb2..fdb9102 100644
--- a/i18n/qqq.json
+++ b/i18n/qqq.json
@@ -61,5 +61,7 @@
"cirrussearch-pref-completion-legacy-section-desc": "Name of the
subsection Prefix search in the user preferences",
"cirrussearch-pref-completion-legacy-section-legend": "Legend of the
subsection Prefix search in the user preferences.",
"cirrussearch-completion-profile-classic-pref-name": "Name of the
completion profile classic.",
- "cirrussearch-completion-profile-classic-pref-desc": "Description of
the completion profile classic."
+ "cirrussearch-completion-profile-classic-pref-desc": "Description of
the completion profile classic.",
+ "cirrussearch-timed-out": "Message displayed to user when the search
query gave up due to timeout before completing.",
+ "cirrussearch-regex-timed-out": "Message displayed to user when the
regex search query gave up due to timeout before completing."
}
diff --git a/includes/CirrusSearch.php b/includes/CirrusSearch.php
index 4681375..f5afd7f 100644
--- a/includes/CirrusSearch.php
+++ b/includes/CirrusSearch.php
@@ -152,25 +152,26 @@
/**
* Overridden to delegate prefix searching to Searcher.
* @param string $term text to search
- * @return ResultSet|null|Status results, no results, or error
respectively
+ * @return Status Value is either SearchResultSet, or null on error.
*/
public function searchText( $term ) {
$config = $this->config;
if ( $this->request && $this->request->getVal( 'cirrusLang' ) )
{
$config = new SearchConfig( $this->request->getVal(
'cirrusLang' ) );
}
- $matches = $this->searchTextReal( $term, $config );
- if (!$matches instanceof ResultSet) {
- return $matches;
+ $status = $this->searchTextReal( $term, $config );
+ $matches = $status->getValue();
+ if ( !$status->isOK() || !$matches instanceof ResultSet ) {
+ return $status;
}
if ( $this->isFeatureEnabled( 'rewrite' ) &&
$matches->isQueryRewriteAllowed(
$GLOBALS['wgCirrusSearchInterwikiThreshold'] ) ) {
- $matches = $this->searchTextSecondTry( $term, $matches
);
+ $status = $this->searchTextSecondTry( $term, $status );
}
- ElasticsearchIntermediary::setResultPages( [ $matches ] );
+ ElasticsearchIntermediary::setResultPages( [
$status->getValue() ] );
- return $matches;
+ return $status;
}
/**
@@ -263,16 +264,18 @@
/**
* @param string $term
- * @param ResultSet $oldResult
- * @return ResultSet
+ * @param Status $oldStatus
+ * @return Status
*/
- private function searchTextSecondTry( $term, ResultSet $oldResult ) {
+ private function searchTextSecondTry( $term, Status $oldStatus ) {
// TODO: figure out who goes first - language or suggestion?
+ $oldResult = $oldStatus->getValue();
if ( $oldResult->numRows() == 0 && $oldResult->hasSuggestion()
) {
$rewritten = $oldResult->getSuggestionQuery();
$rewrittenSnippet = $oldResult->getSuggestionSnippet();
$this->showSuggestion = false;
- $rewrittenResult = $this->searchTextReal( $rewritten,
$this->config );
+ $rewrittenStatus = $this->searchTextReal( $rewritten,
$this->config );
+ $rewrittenResult = $rewrittenStatus->getValue();
if (
$rewrittenResult instanceof ResultSet
&& $rewrittenResult->numRows() > 0
@@ -282,7 +285,7 @@
// replace the result but still try the
alt language
$oldResult = $rewrittenResult;
} else {
- return $rewrittenResult;
+ return $rewrittenStatus;
}
}
}
@@ -303,7 +306,8 @@
}
if ( $config ) {
$this->indexBaseName = $config->get(
SearchConfig::INDEX_BASE_NAME );
- $matches = $this->searchTextReal( $term,
$config, true );
+ $status = $this->searchTextReal( $term,
$config, true );
+ $matches = $status->getValue();
if ( $matches instanceof ResultSet ) {
$numRows = $matches->numRows();
$this->extraSearchMetrics['wgCirrusSearchAltLanguageNumResults'] = $numRows;
@@ -319,7 +323,7 @@
}
// Don't have any other options yet.
- return $oldResult;
+ return $oldStatus;
}
/**
@@ -328,7 +332,7 @@
* @param SearchConfig $config
* @param boolean $forceLocal set to true to force searching on the
* local wiki (e.g. avoid searching on commons)
- * @return null|Status|ResultSet
+ * @return Status
*/
protected function searchTextReal( $term, SearchConfig $config,
$forceLocal = false ) {
$searcher = new Searcher( $this->connection, $this->offset,
$this->limit, $config, $this->namespaces, null, $this->indexBaseName );
@@ -401,9 +405,6 @@
return $status;
}
- // For historical reasons all callers of searchText interpret
any Status return as an error
- // so we must unwrap all OK statuses. Note that $status can be
"good" and still contain null
- // since that is interpreted as no results.
$result = $status->getValue();
// Add interwiki results, if we have a sane result
@@ -466,12 +467,11 @@
exit();
} else {
// This breaks the return types and is only
used in the fixtures unit tests
- return $result;
+ $status->setResult( true, $result );
}
}
-
- return $status->isOK() ? $status->getValue() : $status;
+ return $status;
}
/**
diff --git a/includes/Searcher.php b/includes/Searcher.php
index c1e7f1e..5dc515e 100644
--- a/includes/Searcher.php
+++ b/includes/Searcher.php
@@ -362,17 +362,17 @@
$qb = $this->buildFullTextSearch( $term, $showSuggestion );
- $result = $this->searchOne();
- if ( !$result->isOK() && ElasticaErrorHandler::isParseError(
$result ) ) {
+ $status = $this->searchOne();
+ if ( !$status->isOK() && ElasticaErrorHandler::isParseError(
$status ) ) {
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();
+ $status = $this->searchOne();
}
}
- return $result;
+ return $status;
}
/**
@@ -786,7 +786,10 @@
$log->getDescription() . " timed out and only
returned partial results!",
$log->getLogVariables()
);
- $status->warning( 'cirrussearch-timed-out' );
+ $status->warning( $this->searchContext->getSearchType()
=== 'regex'
+ ? 'cirrussearch-regex-timed-out'
+ : 'cirrussearch-timed-out'
+ );
}
$status->setResult( true, $retval );
diff --git a/tests/unit/LanguageDetectTest.php
b/tests/unit/LanguageDetectTest.php
index 3439bc0..38a81d6 100644
--- a/tests/unit/LanguageDetectTest.php
+++ b/tests/unit/LanguageDetectTest.php
@@ -91,10 +91,10 @@
$cirrus = new MyCirrusSearch();
$cirrus->setNamespaces( [NS_FILE] );
$cirrus->setDumpAndDie( false );
- $result = $cirrus->mySearchTextReal( 'hello',
$cirrus->getConfig(), true );
+ $result = $cirrus->mySearchTextReal( 'hello',
$cirrus->getConfig(), true )->getValue();
$result = json_decode( $result, true );
$this->assertEquals( 'mywiki_general/page/_search',
$result['path'] );
- $result = $cirrus->mySearchTextReal( 'hello',
$cirrus->getConfig() );
+ $result = $cirrus->mySearchTextReal( 'hello',
$cirrus->getConfig() )->getValue();
$result = json_decode( $result, true );
$this->assertEquals(
'mywiki_general,externalwiki_file/page/_search', $result['path'] );
}
diff --git a/tests/unit/SearcherTest.php b/tests/unit/SearcherTest.php
index 6cd4d30..ef61a98 100644
--- a/tests/unit/SearcherTest.php
+++ b/tests/unit/SearcherTest.php
@@ -98,7 +98,7 @@
$engine->setShowSuggestion( true );
$engine->setLimitOffset( 20, 0 );
$engine->setDumpAndDie( false );
- $encodedQuery = $engine->searchText( $queryString );
+ $encodedQuery = $engine->searchText( $queryString )->getValue();
$elasticQuery = json_decode( $encodedQuery, true );
// For extra fun, prefer-recent queries include a 'now'
timestamp. We need to normalize that so
// the output is actually the same.
--
To view, visit https://gerrit.wikimedia.org/r/320554
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I34a522bee18d14137d4640b7d5cf2893985b6eea
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Cindy-the-browser-test-bot <bernhardsone...@gmail.com>
Gerrit-Reviewer: DCausse <dcau...@wikimedia.org>
Gerrit-Reviewer: EBernhardson <ebernhard...@wikimedia.org>
Gerrit-Reviewer: Gehel <gleder...@wikimedia.org>
Gerrit-Reviewer: Manybubbles <never...@wikimedia.org>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: Smalyshev <smalys...@wikimedia.org>
Gerrit-Reviewer: Tjones <tjo...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
