jenkins-bot has submitted this change and it was merged.
Change subject: Refactor interwiki support
......................................................................
Refactor interwiki support
Use the new InterwikiResolver service to handle Interwiki searches.
The goal is to centralize the Elastic Result to Title conversion
into a single place with proper support for interwiki results.
For interwiki results the plan is to rely on the wiki field returned
by elastic to decide if the title is internal or external.
This is still transitional as we need some BC code to support wikis
where the wiki field is not fully populated.
Once done (reindex all wikis) we can remove the BC code marked with a
TODO comment and rely only on the wiki field to determine if the
result refers to an external title.
Added 2 new feature flags to activate either crossproject and
crosslanguage search. These flags are not fully effective, they only
affect behaviors if the config used by CirrusConfigInterwikiResolver
is not present and SiteMatrix is available. This is because it's the
way it works today: if some data are present in some of the map then
the interwiki feature is activated.
The plan is to the SiteMatrix approach for wmf sites and use a
feature flag to activate either crosslanguage fallback and/or
crossproject search.
Bug: T141033
Change-Id: I6aebfa22c05a82f6d887f94ef6c18ce5b3a1b0bc
---
M CirrusSearch.php
M autoload.php
M includes/BaseInterwikiResolver.php
M includes/CirrusSearch.php
M includes/Hooks.php
M includes/InterwikiResolver.php
M includes/InterwikiSearcher.php
M includes/Maintenance/Validators/CacheWarmersValidator.php
M includes/Search/Result.php
M includes/Search/ResultSet.php
M includes/Search/ResultsType.php
A includes/Search/TitleHelper.php
M includes/SearchConfig.php
M includes/Searcher.php
M tests/unit/InterwikiResolverTest.php
M tests/unit/LanguageDetectTest.php
M tests/unit/Search/ResultTest.php
M tests/unit/Search/ResultsTypeTest.php
18 files changed, 407 insertions(+), 233 deletions(-)
Approvals:
Cindy-the-browser-test-bot: Looks good to me, but someone else must approve
EBernhardson: Looks good to me, approved
jenkins-bot: Verified
diff --git a/CirrusSearch.php b/CirrusSearch.php
index bf19fc0..ddb757d 100644
--- a/CirrusSearch.php
+++ b/CirrusSearch.php
@@ -889,6 +889,22 @@
$wgCirrusSearchWikiToNameMap = [];
/**
+ * Enable crossproject search.
+ * Crossproject works by seaching on so-called sister wikis:
+ * Same language, sister project.
+ * NOTE: Experimental
+ */
+$wgCirrusSearchEnableCrossProjectSearch = false;
+
+/**
+ * Enable cross leanguage search.
+ * Usually implemented as fallback for queries
+ * that returns fewer than $wgCirrusSearchInterwikiThreshold results
+ * NOTE: Experimental
+ */
+$wgCirrusSearchEnableCrossLanguageSearch = false;
+
+/**
* If set to non-empty string, interwiki results will have ?wprov=XYZ
parameter added.
*/
$wgCirrusSearchInterwikiProv = false;
diff --git a/autoload.php b/autoload.php
index bef7cd0..92f7d56 100644
--- a/autoload.php
+++ b/autoload.php
@@ -138,6 +138,7 @@
'CirrusSearch\\Sanity\\Remediator' => __DIR__ .
'/includes/Sanity/Remediator.php',
'CirrusSearch\\SearchConfig' => __DIR__ . '/includes/SearchConfig.php',
'CirrusSearch\\SearchRequestLog' => __DIR__ .
'/includes/SearchRequestLog.php',
+ 'CirrusSearch\\Search\\BaseResultsType' => __DIR__ .
'/includes/Search/ResultsType.php',
'CirrusSearch\\Search\\BooleanIndexField' => __DIR__ .
'/includes/Search/BooleanIndexField.php',
'CirrusSearch\\Search\\BoostTemplatesFunctionScoreBuilder' => __DIR__ .
'/includes/Search/RescoreBuilders.php',
'CirrusSearch\\Search\\CirrusIndexField' => __DIR__ .
'/includes/Search/CirrusIndexField.php',
@@ -177,6 +178,7 @@
'CirrusSearch\\Search\\ShortTextIndexField' => __DIR__ .
'/includes/Search/ShortTextIndexField.php',
'CirrusSearch\\Search\\SourceTextIndexField' => __DIR__ .
'/includes/Search/SourceTextIndexField.php',
'CirrusSearch\\Search\\TextIndexField' => __DIR__ .
'/includes/Search/TextIndexField.php',
+ 'CirrusSearch\\Search\\TitleHelper' => __DIR__ .
'/includes/Search/TitleHelper.php',
'CirrusSearch\\Search\\TitleResultsType' => __DIR__ .
'/includes/Search/ResultsType.php',
'CirrusSearch\\Searcher' => __DIR__ . '/includes/Searcher.php',
'CirrusSearch\\SiteMatrixInterwikiResolver' => __DIR__ .
'/includes/SiteMatrixInterwikiResolver.php',
diff --git a/includes/BaseInterwikiResolver.php
b/includes/BaseInterwikiResolver.php
index 2835b8c..d70ca91 100644
--- a/includes/BaseInterwikiResolver.php
+++ b/includes/BaseInterwikiResolver.php
@@ -38,7 +38,7 @@
// Most of the time the language is equal to the interwiki
prefix.
// But it's not always the case, use the language_map to
identify the interwiki prefix first.
$lang = isset( $matrix['language_map'][$lang] ) ?
$matrix['language_map'][$lang] : $lang;
- return isset( $matrix['cross_language'][$lang] ) ? [ $lang =>
$matrix['cross_language'][$lang] ] : [];
+ return isset( $matrix['cross_language'][$lang] ) ? [
$matrix['cross_language'][$lang], $lang ] : [];
}
/** @return array[] */
diff --git a/includes/CirrusSearch.php b/includes/CirrusSearch.php
index 1a2399c..7c3cfba 100644
--- a/includes/CirrusSearch.php
+++ b/includes/CirrusSearch.php
@@ -154,11 +154,7 @@
* @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' ) );
- }
- $status = $this->searchTextReal( $term, $config );
+ $status = $this->searchTextReal( $term, $this->config );
$matches = $status->getValue();
if ( !$status->isOK() || !$matches instanceof ResultSet ) {
return $status;
@@ -176,12 +172,10 @@
/**
* Check whether we want to try another language.
* @param string $term Search term
- * @return string[]|null Array of (interwiki, dbname) for another wiki
to try, or null
+ * @return string|null dbname for another wiki to try, or null
*/
- private function hasSecondaryLanguage( $term ) {
- if ( empty( $GLOBALS['wgCirrusSearchLanguageToWikiMap'] ) ||
- empty( $GLOBALS['wgCirrusSearchWikiToNameMap']
) ) {
- // map's empty - no need to bother with detection
+ private function detectSecondaryLanguage( $term ) {
+ if ( !$this->config->isCrossLanguageSearchEnabled() ) {
return null;
}
@@ -210,8 +204,10 @@
continue;
}
$lang = $detector->detect( $this, $term );
- $wiki = self::wikiForLanguage( $lang );
- if ( $wiki !== null ) {
+ $wiki = MediaWikiServices::getInstance()
+ ->getService( InterwikiResolver::SERVICE )
+ ->getSameProjectWikiByLang( $lang );
+ if ( !empty( $wiki ) ) {
// it might be more accurate to attach these to
the 'next'
// log context? It would be inconsistent with
the
// langdetect => false condition which does not
have a next
@@ -223,34 +219,13 @@
}
if ( $detected === null ) {
Searcher::appendLastLogPayload( 'langdetect', 'failed'
);
+ return null;
} else {
// Report language detection with search metrics
+ // TODO: do we still need this metric? (see T151796)
$this->extraSearchMetrics['wgCirrusSearchAltLanguage']
= $detected;
+ return reset( $detected );
}
-
- return $detected;
- }
-
- /**
- * @param string $lang Language code to find wiki for
- * @return string[]|null Array of (interwiki, dbname) for wiki related
to specified language code
- */
- private static function wikiForLanguage( $lang ) {
- if ( empty( $GLOBALS['wgCirrusSearchLanguageToWikiMap'][$lang]
) ) {
- return null;
- }
- $interwiki = $GLOBALS['wgCirrusSearchLanguageToWikiMap'][$lang];
-
- if ( empty( $GLOBALS['wgCirrusSearchWikiToNameMap'][$interwiki]
) ) {
- return null;
- }
- $interWikiId =
$GLOBALS['wgCirrusSearchWikiToNameMap'][$interwiki];
- if ( $interWikiId == wfWikiID() ) {
- // we're back to the same wiki, no use to try again
- return null;
- }
-
- return [ $interwiki, $interWikiId ];
}
/**
@@ -288,16 +263,15 @@
}
}
}
- $altWiki = $this->hasSecondaryLanguage( $term );
- if ( $altWiki ) {
+ $altWikiId = $this->detectSecondaryLanguage( $term );
+ if ( $altWikiId ) {
try {
- $config = new SearchConfig( $altWiki[0],
$altWiki[1] );
+ $config = $this->config->newInterwikiConfig(
$altWikiId );
} catch ( MWException $e ) {
LoggerFactory::getInstance( 'CirrusSearch'
)->info(
- "Failed to get config for
{interwiki}:{dbwiki}",
+ "Failed to get config for {dbwiki}",
[
- "interwiki" => $altWiki[0],
- "dbwiki" => $altWiki[1],
+ "dbwiki" => $altWikiId,
"exception" => $e
]
);
@@ -315,7 +289,7 @@
// users in the control buckets, and
provide them the same latency as users
// in the test bucket.
if (
$GLOBALS['wgCirrusSearchEnableAltLanguage'] && $numRows > 0) {
-
$oldResult->addInterwikiResults( $matches, SearchResultSet::INLINE_RESULTS,
$altWiki[1] );
+
$oldResult->addInterwikiResults( $matches, SearchResultSet::INLINE_RESULTS,
$altWikiId );
}
}
}
@@ -394,7 +368,7 @@
$highlightingConfig ^=
FullTextResultsType::HIGHLIGHT_FILE_TEXT;
}
- $resultsType = new FullTextResultsType( $highlightingConfig,
$config ? $config->getWikiCode() : '');
+ $resultsType = new FullTextResultsType( $config,
$highlightingConfig );
$searcher->setResultsType( $resultsType );
$status = $searcher->searchText( $term, $this->showSuggestion );
@@ -648,10 +622,10 @@
$searcher = new Searcher( $this->connection, $this->offset,
$this->limit, $this->config, $this->namespaces );
if ( $search ) {
- $searcher->setResultsType( new FancyTitleResultsType(
'prefix' ) );
+ $searcher->setResultsType( new FancyTitleResultsType(
$this->config, 'prefix' ) );
} else {
// Empty searches always find the title.
- $searcher->setResultsType( new TitleResultsType() );
+ $searcher->setResultsType( new TitleResultsType(
$this->config ) );
}
try {
diff --git a/includes/Hooks.php b/includes/Hooks.php
index 9b4db24..aea940d 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -560,7 +560,7 @@
} else {
$term = $title->getText();
}
- $searcher->setResultsType( new FancyTitleResultsType(
'near_match' ) );
+ $searcher->setResultsType( new FancyTitleResultsType(
self::getConfig(), 'near_match' ) );
try {
$status = $searcher->nearMatchTitleSearch( $term );
} catch ( ApiUsageException $e ) {
diff --git a/includes/InterwikiResolver.php b/includes/InterwikiResolver.php
index 54f0502..331278c 100644
--- a/includes/InterwikiResolver.php
+++ b/includes/InterwikiResolver.php
@@ -31,7 +31,7 @@
* to generate crosslanguage interwiki links.
*
* @param string $lang
- * @return string[] a single elt array [ 'iw_prefix' => 'wikiId' ] or
[] if none found
+ * @return string[] a two elt array ['wikiId', 'iwPrefix'] or [] if
none found
*/
public function getSameProjectWikiByLang( $lang );
}
diff --git a/includes/InterwikiSearcher.php b/includes/InterwikiSearcher.php
index 2f3ea45..4ced44f 100644
--- a/includes/InterwikiSearcher.php
+++ b/includes/InterwikiSearcher.php
@@ -4,6 +4,7 @@
use CirrusSearch\Search\InterwikiResultsType;
use CirrusSearch\Search\ResultSet;
+use MediaWiki\MediaWikiServices;
use SpecialPageFactory;
use User;
@@ -90,7 +91,16 @@
return null;
}
- $sources = $this->config->get( 'CirrusSearchInterwikiSources' );
+ if ( !$this->config->isCrossProjectSearchEnabled() ) {
+ // TODO: we should probably call this before (in the
+ // CirrusSearch class) to avoid creating an object for
+ // nothing.
+ return null;
+ }
+
+ $sources = MediaWikiServices::getInstance()
+ ->getService( InterwikiResolver::SERVICE )
+ ->getSisterProjectPrefixes();
if ( !$sources ) {
return null;
}
@@ -122,7 +132,10 @@
// specialized to the interwiki use case, but because
we are not
// returning load test results to the users that is
acceptable.
if (!$this->isLoadTestEnabled ) {
- $resultsTypes[$interwiki] = new
InterwikiResultsType( $interwiki );
+ // TODO: remove when getWikiCode is removed.
+ // In theory we should be able to reuse the same
+ // Results type for all searches
+ $resultsTypes[$interwiki] = new
InterwikiResultsType( $this->config->newInterwikiConfig( $index, false ) );
$this->setResultsType(
$resultsTypes[$interwiki] );
}
$this->indexBaseName = $index;
@@ -147,27 +160,6 @@
} else {
return array_merge( $retval, $results->getValue() );
}
- }
-
- /**
- * Get the index basename for a given interwiki prefix, if one is
defined.
- * @param string $interwiki
- * @return string|null
- */
- public static function getIndexForInterwiki( $interwiki ) {
- // These settings should be common for all wikis, so globals
- // are _probably_ OK here.
- global $wgCirrusSearchInterwikiSources,
$wgCirrusSearchWikiToNameMap;
-
- if ( isset( $wgCirrusSearchInterwikiSources[$interwiki] ) ) {
- return $wgCirrusSearchInterwikiSources[$interwiki];
- }
-
- if ( isset( $wgCirrusSearchWikiToNameMap[$interwiki] ) ) {
- return $wgCirrusSearchWikiToNameMap[$interwiki];
- }
-
- return null;
}
/**
diff --git a/includes/Maintenance/Validators/CacheWarmersValidator.php
b/includes/Maintenance/Validators/CacheWarmersValidator.php
index 75a0546..ae8a5c3 100644
--- a/includes/Maintenance/Validators/CacheWarmersValidator.php
+++ b/includes/Maintenance/Validators/CacheWarmersValidator.php
@@ -33,7 +33,7 @@
* @param array $cacheWarmers
* @param Maintenance $out
*/
- public function __construct( $indexType, Type $pageType, array
$cacheWarmers = [], Maintenance $out = null ) {
+ public function __construct( $indexType, Type $pageType, array
$cacheWarmers, Maintenance $out ) {
parent::__construct( $out );
$this->indexType = $indexType;
@@ -93,7 +93,7 @@
// Null user because we won't be logging anything about
the user.
);
$searcher->setReturnQuery( true );
- $searcher->setResultsType( new FullTextResultsType(
FullTextResultsType::HIGHLIGHT_ALL ) );
+ $searcher->setResultsType( new FullTextResultsType(
$this->maint->getSearchConfig(), FullTextResultsType::HIGHLIGHT_ALL ) );
$searcher->limitSearchToLocalWiki( true );
$query = $searcher->searchText( $search, true )->getValue();
return $query[ 'query' ];
diff --git a/includes/Search/Result.php b/includes/Search/Result.php
index ce0d192..a1b1c73 100644
--- a/includes/Search/Result.php
+++ b/includes/Search/Result.php
@@ -5,6 +5,7 @@
use CirrusSearch\InterwikiSearcher;
use CirrusSearch\Util;
use CirrusSearch\Searcher;
+use CirrusSearch\SearchConfig;
use MediaWiki\Logger\LoggerFactory;
use MWTimestamp;
use SearchResult;
@@ -29,6 +30,8 @@
* http://www.gnu.org/copyleft/gpl.html
*/
class Result extends SearchResult {
+ use TitleHelper;
+
/** @var int */
private $namespace;
/** @var string */
@@ -47,10 +50,12 @@
private $textSnippet;
/** @var bool */
private $isFileMatch = false;
- /* @var string */
- private $interwiki = '';
+ /* @var SearchConfig */
+ private $config;
+ /* @var string result wiki */
+ private $wiki;
/** @var string */
- private $interwikiNamespace = '';
+ private $namespaceText = '';
/** @var int */
private $wordCount;
/** @var int */
@@ -71,18 +76,17 @@
*
* @param \Elastica\ResultSet $results containing all search results
* @param \Elastica\Result $result containing the given search result
- * @param string $interwiki Interwiki prefix, if any
- * @param \Elastica\Result $result containing information about the
result this class should represent
+ * @param SearchConfig $config
*/
- public function __construct( $results, $result, $interwiki = '' ) {
+ public function __construct( $results, $result, SearchConfig $config ) {
global $wgCirrusSearchDevelOptions;
$this->ignoreMissingRev = isset(
$wgCirrusSearchDevelOptions['ignore_missing_rev'] );
- if ( $interwiki ) {
- $this->setInterwiki( $result, $interwiki );
- }
+ $this->config = $config;
+ $this->namespaceText = $result->namespace_text;
+ $this->wiki = $result->wiki;
$this->docId = $result->getId();
$this->namespace = $result->namespace;
- $this->mTitle = $this->makeTitle( $result->namespace,
$result->title );
+ $this->mTitle = $this->makeTitle( $result );
if ( $this->getTitle()->getNamespace() == NS_FILE ) {
$this->mImage = wfFindFile( $this->mTitle );
}
@@ -105,8 +109,7 @@
if ( !isset( $highlights[ 'title' ] ) && isset( $highlights[
'redirect.title' ] ) ) {
// Make sure to find the redirect title before escaping
because escaping breaks it....
- $redirects = $result->redirect;
- $this->redirectTitle = $this->findRedirectTitle(
$highlights[ 'redirect.title' ][ 0 ], $redirects );
+ $this->redirectTitle = $this->findRedirectTitle(
$result, $highlights[ 'redirect.title' ][ 0 ] );
$this->redirectSnippet = $this->escapeHighlightedText(
$highlights[ 'redirect.title' ][ 0 ] );
}
@@ -204,18 +207,19 @@
/**
* Build the redirect title from the highlighted redirect snippet.
*
+ * @param \Elastica\Result $result
* @param string $snippet Highlighted redirect snippet
- * @param array[]|null $redirects Array of redirects stored as arrays
with 'title' and 'namespace' keys
* @return Title|null object representing the redirect
*/
- private function findRedirectTitle( $snippet, $redirects ) {
+ private function findRedirectTitle( \Elastica\Result $result, $snippet
) {
$title = $this->stripHighlighting( $snippet );
// Grab the redirect that matches the highlighted title with
the lowest namespace.
+ $redirects = $result->redirect;
// That is pretty arbitrary but it prioritizes 0 over others.
$best = null;
if ( $redirects !== null ) {
foreach ( $redirects as $redirect ) {
- if ( $redirect[ 'title' ] === $title && ( $best
=== null || $best[ 'namespace' ] > $redirect ) ) {
+ if ( $redirect[ 'title' ] === $title && ( $best
=== null || $best[ 'namespace' ] > $redirect['namespace'] ) ) {
$best = $redirect;
}
}
@@ -227,7 +231,7 @@
);
return null;
}
- return $this->makeTitle( $best[ 'namespace' ], $best[ 'title' ]
);
+ return $this->makeRedirectTitle( $result, $best['title'],
$best['namespace'] );
}
/**
@@ -246,22 +250,6 @@
private function stripHighlighting( $highlighted ) {
$markers = [ Searcher::HIGHLIGHT_PRE, Searcher::HIGHLIGHT_POST
];
return str_replace( $markers, '', $highlighted );
- }
-
- /**
- * Set interwiki and interwikiNamespace properties
- *
- * @param \Elastica\Result $result containing the given search result
- * @param string $interwiki Interwiki prefix, if any
- */
- private function setInterwiki( $result, $interwiki ) {
- $resultIndex = $result->getIndex();
- $indexBase = InterwikiSearcher::getIndexForInterwiki(
$interwiki );
- $pos = strpos( $resultIndex, $indexBase );
- if ( $pos === 0 && $resultIndex[strlen( $indexBase )] == '_' ) {
- $this->interwiki = $interwiki;
- $this->interwikiNamespace = $result->namespace_text ?
$result->namespace_text : '';
- }
}
/**
@@ -346,14 +334,15 @@
* @return string
*/
public function getInterwikiPrefix() {
- return $this->interwiki;
+ return $this->mTitle->getInterwiki();
}
/**
* @return string
*/
public function getInterwikiNamespaceText() {
- return $this->interwikiNamespace;
+ // Seems to be only useful for API
+ return $this->namespaceText;
}
/**
@@ -378,25 +367,9 @@
}
/**
- * Create a title. When making interwiki titles we should be providing
the
- * namespace text as a portion of the text, rather than a namespace id,
- * because namespace id's are not consistent across wiki's. This
- * additionally prevents the local wiki from localizing the namespace
text
- * when it should be using the localized name of the remote wiki.
- *
- * Unfortunately we don't always have the remote namespace text, such as
- * when handling redirects. Do the best we can in this case and take the
- * less-than ideal results when we don't.
- *
- * @param int $namespace
- * @param string $text
- * @return Title
+ * @return SearchConfig $config
*/
- private function makeTitle( $namespace, $text ) {
- if ( $this->interwikiNamespace && $namespace ===
$this->namespace ) {
- return Title::makeTitle( 0, $this->interwikiNamespace .
':' . $text, '', $this->interwiki );
- } else {
- return Title::makeTitle( $namespace, $text, '',
$this->interwiki );
- }
+ public function getConfig() {
+ return $this->config;
}
}
diff --git a/includes/Search/ResultSet.php b/includes/Search/ResultSet.php
index 92be8c3..aaec649 100644
--- a/includes/Search/ResultSet.php
+++ b/includes/Search/ResultSet.php
@@ -3,6 +3,7 @@
namespace CirrusSearch\Search;
use CirrusSearch\Searcher;
+use CirrusSearch\SearchConfig;
use LinkBatch;
use SearchResultSet;
@@ -25,6 +26,8 @@
* http://www.gnu.org/copyleft/gpl.html
*/
class ResultSet extends SearchResultSet {
+ use TitleHelper;
+
/**
* @var \Elastica\ResultSet
*/
@@ -56,9 +59,9 @@
private $searchContainedSyntax;
/**
- * @var string
+ * @var SearchConfig
*/
- private $interwikiPrefix;
+ private $config;
/**
* @var array
@@ -80,14 +83,14 @@
* @param string[] $suggestSuffixes
* @param \Elastica\ResultSet $res
* @param bool $searchContainedSyntax
- * @param string $interwiki
+ * @param SearchConfig $config
*/
- public function __construct( array $suggestPrefixes, array
$suggestSuffixes, \Elastica\ResultSet $res, $searchContainedSyntax, $interwiki
= '' ) {
+ public function __construct( array $suggestPrefixes, array
$suggestSuffixes, \Elastica\ResultSet $res, $searchContainedSyntax,
SearchConfig $config ) {
$this->result = $res;
$this->searchContainedSyntax = $searchContainedSyntax;
$this->hits = $res->count();
$this->totalHits = $res->getTotalHits();
- $this->interwikiPrefix = $interwiki;
+ $this->config = $config;
$this->preCacheContainedTitles( $this->result );
$suggestion = $this->findSuggestion();
if ( $suggestion && !
$this->resultContainsFullyHighlightedMatch() ) {
@@ -190,14 +193,14 @@
*
* @param \Elastica\ResultSet $resultSet Result set from which the
titles come
*/
- private function preCacheContainedTitles( \Elastica\ResultSet
$resultSet ) {
+ protected function preCacheContainedTitles( \Elastica\ResultSet
$resultSet ) {
// We can only pull in information about the local wiki
- if ( $this->interwikiPrefix !== '' ) {
- return;
- }
$lb = new LinkBatch;
foreach ( $resultSet->getResults() as $result ) {
- $lb->add( $result->namespace, $result->title );
+ if ( !$this->isExternal( $result ) ) {
+
+ $lb->add( $result->namespace, $result->title );
+ }
}
if ( !$lb->isEmpty() ) {
$lb->setCaller( __METHOD__ );
@@ -247,7 +250,7 @@
$current = $this->result->current();
if ( $current ) {
$this->result->next();
- $result = new Result( $this->result, $current,
$this->interwikiPrefix );
+ $result = new Result( $this->result, $current,
$this->config );
$this->augmentResult( $result );
return $result;
}
@@ -319,4 +322,11 @@
public function getQueryAfterRewriteSnippet() {
return $this->rewrittenQuerySnippet;
}
+
+ /**
+ * @return SearchConfig
+ */
+ public function getConfig() {
+ return $this->config;
+ }
}
diff --git a/includes/Search/ResultsType.php b/includes/Search/ResultsType.php
index df1ab73..446ea05 100644
--- a/includes/Search/ResultsType.php
+++ b/includes/Search/ResultsType.php
@@ -3,6 +3,7 @@
namespace CirrusSearch\Search;
use CirrusSearch\Searcher;
+use CirrusSearch\SearchConfig;
use MediaWiki\Logger\LoggerFactory;
use Title;
@@ -62,17 +63,39 @@
function createEmptyResult();
}
-/**
- * Returns titles and makes no effort to figure out how the titles matched.
- */
-class TitleResultsType implements ResultsType {
+abstract class BaseResultsType implements ResultsType {
+ use TitleHelper;
+
+ /** @var SearchConfig */
+ private $config;
+
+ /**
+ * @param SearchConfig $config
+ */
+ public function __construct( SearchConfig $config ) {
+ $this->config = $config;
+ }
+
+ /**
+ * TODO: remove when getWikiCode is removed
+ * @return SearchConfig
+ */
+ public function getConfig() {
+ return $this->config;
+ }
+
/**
* @return false|string|array corresponding to Elasticsearch source
filtering syntax
*/
public function getSourceFiltering() {
return [ 'namespace', 'title', 'namespace_text', 'wiki' ];
}
+}
+/**
+ * Returns titles and makes no effort to figure out how the titles matched.
+ */
+class TitleResultsType extends BaseResultsType {
/**
* @return false|string|array corresponding to Elasticsearch fields
syntax
*/
@@ -96,7 +119,7 @@
public function transformElasticsearchResult( SearchContext $context,
\Elastica\ResultSet $resultSet ) {
$results = [];
foreach( $resultSet->getResults() as $r ) {
- $results[] = Title::makeTitle( $r->namespace, $r->title
);
+ $results[] = $this->makeTitle( $r );
}
return $results;
}
@@ -120,9 +143,11 @@
* Build result type. The matchedAnalyzer is required to detect if
the match
* was from the title or a redirect (and is kind of a leaky
abstraction.)
*
+ * @param SearchConfig $config
* @param string $matchedAnalyzer the analyzer used to match the title
*/
- public function __construct( $matchedAnalyzer ) {
+ public function __construct( SearchConfig $config, $matchedAnalyzer ) {
+ parent::__construct( $config );
$this->matchedAnalyzer = $matchedAnalyzer;
}
@@ -187,7 +212,7 @@
public function transformElasticsearchResult( SearchContext $context,
\Elastica\ResultSet $resultSet ) {
$results = [];
foreach( $resultSet->getResults() as $r ) {
- $title = Title::makeTitle( $r->namespace, $r->title );
+ $title = $this->makeTitle( $r );
$highlights = $r->getHighlights();
$resultForTitle = [];
@@ -219,7 +244,7 @@
// Instead of getting the redirect's
real namespace we're going to just use the namespace
// of the title. This is not great but
OK given that we can't find cross namespace
// redirects properly any way.
- $redirectTitle = Title::makeTitle(
$r->namespace, $redirectTitle );
+ $redirectTitle =
$this->makeRedirectTitle( $r, $redirectTitle, $this->namespace );
$resultForTitle[ 'redirectMatches' ][]
= $redirectTitle;
}
}
@@ -247,7 +272,7 @@
/**
* Result type for a full text search.
*/
-class FullTextResultsType implements ResultsType {
+class FullTextResultsType extends BaseResultsType {
const HIGHLIGHT_NONE = 0;
const HIGHLIGHT_TITLE = 1;
const HIGHLIGHT_ALT_TITLE = 2;
@@ -269,29 +294,22 @@
private $highlightingConfig;
/**
- * @var string interwiki prefix mappings
- */
- private $prefix;
-
- /**
+ * @param SearchConfig $config
* @param int $highlightingConfig Bitmask, see HIGHLIGHT_* consts
- * @param string $interwiki
*/
- public function __construct( $highlightingConfig, $interwiki = '' ) {
+ public function __construct( SearchConfig $config, $highlightingConfig
) {
+ parent::__construct( $config );
$this->highlightingConfig = $highlightingConfig;
- $this->prefix = $interwiki;
}
/**
* @return false|string|array corresponding to Elasticsearch source
filtering syntax
*/
public function getSourceFiltering() {
- $fields = [ 'namespace', 'title', 'namespace_text', 'wiki',
'redirect.*', 'timestamp', 'text_bytes' ];
- if ( $this->prefix ) {
- $fields[] = 'namespace_text';
- }
-
- return $fields;
+ return array_merge(
+ parent::getSourceFiltering(),
+ [ 'redirect.*', 'timestamp', 'text_bytes' ]
+ );
}
/**
@@ -460,7 +478,7 @@
$context->getSuggestSuffixes(),
$result,
$context->isSyntaxUsed(),
- $this->prefix
+ $this->getConfig()
);
}
@@ -543,12 +561,20 @@
* Returns page ids. Less CPU load on Elasticsearch since all we're returning
* is an id.
*/
-class IdResultsType extends TitleResultsType {
+class IdResultsType implements ResultsType {
/**
* @return false|string|array corresponding to Elasticsearch source
filtering syntax
*/
public function getSourceFiltering() {
return false;
+ }
+
+ public function getFields() {
+ return false;
+ }
+
+ public function getHighlightingConfiguration( array $highlightSource ) {
+ return null;
}
/**
@@ -572,41 +598,24 @@
}
}
-class InterwikiResultsType implements ResultsType {
+/**
+ * This class does exactly the same as TitleResultsType
+ * but returns a ResultSet instead of an array of titles
+ */
+class InterwikiResultsType extends BaseResultsType {
/**
- * @var string interwiki prefix mappings
- */
- private $prefix;
-
- /**
- * Constructor
- *
- * @param string $interwiki
- */
- public function __construct( $interwiki ) {
- $this->prefix = $interwiki;
- }
-
- /**
- * @param SearchContext $context
- * @param \Elastica\ResultSet $result
- * @return ResultSet
- */
+ * @param SearchContext $context
+ * @param \Elastica\ResultSet $result
+ * @return ResultSet
+ */
public function transformElasticsearchResult( SearchContext $context,
\Elastica\ResultSet $result ) {
return new ResultSet(
$context->getSuggestPrefixes(),
$context->getSuggestSuffixes(),
$result,
$context->isSyntaxUsed(),
- $this->prefix
+ $this->getConfig()
);
- }
-
- /**
- * @return EmptyResultSet
- */
- public function createEmptyResult() {
- return new EmptyResultSet();
}
/**
@@ -614,20 +623,20 @@
* @return null
*/
public function getHighlightingConfiguration( array $highlightSource ) {
- return null;
- }
-
- /**
- * @return false|string|array corresponding to Elasticsearch source
filtering syntax
- */
- public function getSourceFiltering() {
- return [ 'namespace', 'title', 'namespace_text', 'wiki' ];
+ return null;
}
/**
* @return false
*/
public function getFields() {
- return false;
+ return false;
+ }
+
+ /**
+ * @return EmptyResultSet
+ */
+ public function createEmptyResult() {
+ return new EmptyResultSet();
}
}
diff --git a/includes/Search/TitleHelper.php b/includes/Search/TitleHelper.php
new file mode 100644
index 0000000..16d50e2
--- /dev/null
+++ b/includes/Search/TitleHelper.php
@@ -0,0 +1,100 @@
+<?php
+
+namespace CirrusSearch\Search;
+
+use Elastica\Result;
+use Title;
+use CirrusSearch\InterwikiResolver;
+use CirrusSearch\SearchConfig;
+use MediaWiki\MediaWikiServices;
+
+/**
+ * Trait to build MW Title from elastica Result/ResultSet classes
+ * This trait can be used in all classes that need to build a Title
+ * by reading the elasticsearch output.
+ */
+trait TitleHelper {
+ /**
+ * Create a title. When making interwiki titles we should be providing
the
+ * namespace text as a portion of the text, rather than a namespace id,
+ * because namespace id's are not consistent across wiki's. This
+ * additionally prevents the local wiki from localizing the namespace
text
+ * when it should be using the localized name of the remote wiki.
+ *
+ * @param Result $r int $namespace
+ * @param string $text
+ * @return Title
+ */
+ public function makeTitle( Result $r ) {
+ $iwPrefix = $this->identifyInterwikiPrefix( $r );
+ if ( empty( $iwPrefix ) ) {
+ return Title::makeTitle( $r->namespace, $r->title );
+ } else {
+ $nsPrefix = $r->namespace_text ? $r->namespace_text .
':' : '';
+ return Title::makeTitle( 0, $nsPrefix . $r->title, '',
$iwPrefix );
+ }
+ }
+
+ /**
+ * Build a Title to a redirect, this always works for internal titles.
+ * For external titles we need to use the namespace_text which is only
+ * valid if the redirect namespace is equals to the target title
namespace.
+ * If the namespaces do not match we return null.
+ *
+ * @param Result $r
+ * @param string $redirectText
+ * @param int $redirNamespace
+ * @return Title|null the Title to the Redirect or null if we can't
build it
+ */
+ public function makeRedirectTitle( Result $r, $redirectText,
$redirNamespace ) {
+ $iwPrefix = $this->identifyInterwikiPrefix( $r );
+ if ( empty( $iwPrefix ) ) {
+ return Title::makeTitle( $redirNamespace, $redirectText
);
+ }
+ if ( $redirNamespace === $r->namespace ) {
+ $nsPrefix = $r->namespace_text ? $r->namespace_text .
':' : '';
+ return Title::makeTitle(
+ 0,
+ $nsPrefix . $redirectText,
+ '',
+ $iwPrefix
+ );
+ } else {
+ // redir namespace does not match, we can't
+ // build this title.
+ // The caller should fallback to the target title.
+ return null;
+ }
+ }
+
+ /**
+ * @return bool true if this result refers to an external Title
+ */
+ public function isExternal( Result $r ) {
+ if ( isset ( $r->wiki ) && $r->wiki !== wfWikiID() ) {
+ return true;
+ }
+ // TODO: replace by return false when wiki is populated
+ return !empty( $this->getConfig()->getWikiCode() );
+ }
+
+ /**
+ * @return string|null the interwiki prefix for this result or null or
+ * empty if local.
+ */
+ public function identifyInterwikiPrefix( $r ) {
+ if ( isset ( $r->wiki ) && $r->wiki !== wfWikiID() ) {
+ return MediaWikiServices::getInstance()
+ ->getService( InterwikiResolver::SERVICE )
+ ->getInterwikiPrefix( $r->wiki );
+ }
+ // TODO: replace by return false when wiki is populated
+ return $this->getConfig()->getWikiCode();
+ }
+
+ /**
+ * TODO: remove when getWikiCode is removed
+ * @return SearchConfig
+ */
+ public abstract function getConfig();
+}
diff --git a/includes/SearchConfig.php b/includes/SearchConfig.php
index 89d73cf..41c4339 100644
--- a/includes/SearchConfig.php
+++ b/includes/SearchConfig.php
@@ -35,12 +35,6 @@
private $prefix = '';
/**
- * Interwiki name for this wiki
- * @var string
- */
- private $interwiki;
-
- /**
* Wiki id or null for current wiki
* @var string|null
*/
@@ -68,20 +62,23 @@
* issues when running queries on external wiki such as TextCat lang
detection
* see CirrusSearch::searchTextSecondTry().
*
- * @param string|null $overrideWiki Interwiki link name for wiki
* @param string|null $overrideName DB name for the wiki
+ * @param bool $fullLoad set to true to fully load the target wiki
config
+ * setting to false will only set the wikiId to $overrideName but will
+ * keep the current wiki config. This should be removed and no longer
+ * when all the wikis have the wiki field populated.
+ * TODO: remove $fullLoad
*/
- public function __construct( $overrideWiki = null, $overrideName = null
) {
- $this->interwiki = $overrideWiki;
- if ( $overrideWiki && $overrideName ) {
+ public function __construct( $overrideName = null, $fullLoad = true ) {
+ if ( $overrideName && $overrideName != wfWikiID() ) {
$this->wikiId = $overrideName;
- if ( $this->wikiId != wfWikiID() ) {
+ if ( $fullLoad ) {
$this->source = new \HashConfig(
$this->getConfigVars( $overrideName, self::CIRRUS_VAR_PREFIX ) );
$this->prefix = 'wg';
// Re-create language object
$this->source->set( 'wgContLang',
\Language::factory( $this->source->get( 'wgLanguageCode' ) ) );
- return;
}
+ return;
}
$this->source = new \GlobalVarConfig();
$this->wikiId = wfWikiID();
@@ -225,14 +222,6 @@
}
/**
- * Get wiki's interwiki code
- * @return string
- */
- public function getWikiCode() {
- return $this->interwiki;
- }
-
- /**
* Get chain of elements from config array
* @param string $configName
* @param string ... list of path elements
@@ -330,4 +319,66 @@
public static function getNonCirrusConfigVarNames() {
return self::$nonCirrusVars;
}
+
+ /**
+ * @return true if cross project (same language) is enabled
+ */
+ public function isCrossProjectSearchEnabled() {
+ // FIXME: temporary hack to support existing config
+ if ( CirrusConfigInterwikiResolver::accepts( $this ) &&
+ !empty( $this->get( 'CirrusSearchInterwikiSources' ) )
+ ) {
+ return true;
+ }
+
+ if ( $this->get( 'CirrusSearchEnableCrossProjectSearch' ) ) {
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * @return true if cross language (same project) is enabled
+ */
+ public function isCrossLanguageSearchEnabled() {
+ // FIXME: temporary hack to support existing config
+ if ( CirrusConfigInterwikiResolver::accepts( $this ) ) {
+ return true;
+ }
+ if ( $this->get( 'CirrusSearchEnableCrossLanguageSearch' ) ) {
+ return true;
+ }
+ return false;
+ }
+
+ /**
+ * Build a new SearchConfig based on $wiki
+ * TODO: remove $fullLoad
+ * @param $wiki dbname of the target wiki
+ * @return SearchConfig
+ */
+ public function newInterwikiConfig( $wiki, $fullLoad = true ) {
+ if ( $wiki === $this->wikiId ) {
+ return $this;
+ }
+
+ return new self( $wiki, $fullLoad );
+ }
+
+ /**
+ * Should not be needed when all wikis have the wiki field
+ * populated
+ * TODO: remove the interwiki prefix should not be stored here
+ * but infered from the wiki field.
+ * @deprecated
+ * @return string interwiki prefix
+ */
+ public function getWikiCode() {
+ if ( $this->wikiId != wfWikiID() ) {
+ return \MediaWiki\MediaWikiServices::getInstance()
+ ->getService( InterwikiResolver::SERVICE )
+ ->getInterwikiPrefix( $this->wikiId );
+ }
+ return '';
+ }
}
diff --git a/includes/Searcher.php b/includes/Searcher.php
index 8786f60..9a9ada1 100644
--- a/includes/Searcher.php
+++ b/includes/Searcher.php
@@ -464,7 +464,7 @@
protected function buildSearch() {
if ( $this->resultsType === null ) {
- $this->resultsType = new FullTextResultsType(
FullTextResultsType::HIGHLIGHT_ALL );
+ $this->resultsType = new FullTextResultsType(
$this->config, FullTextResultsType::HIGHLIGHT_ALL );
}
$query = new \Elastica\Query();
diff --git a/tests/unit/InterwikiResolverTest.php
b/tests/unit/InterwikiResolverTest.php
index 2d7393c..ede0aef 100644
--- a/tests/unit/InterwikiResolverTest.php
+++ b/tests/unit/InterwikiResolverTest.php
@@ -30,15 +30,15 @@
// Test by-language lookup
$this->assertEquals(
- [ 'fr' => 'frwiki' ],
+ [ 'frwiki', 'fr' ],
$resolver->getSameProjectWikiByLang( 'fr' )
);
$this->assertEquals(
- [ 'no' => 'nowiki' ],
+ [ 'nowiki', 'no' ],
$resolver->getSameProjectWikiByLang( 'no' )
);
$this->assertEquals(
- [ 'no' => 'nowiki' ],
+ [ 'nowiki', 'no' ],
$resolver->getSameProjectWikiByLang( 'nb' )
);
$this->assertEquals(
@@ -165,17 +165,17 @@
'enwiki cross lang lookup finds frwiki' => [
'enwiki',
'crosslang', 'fr',
- ['fr' => 'frwiki'],
+ ['frwiki', 'fr'],
],
'enwiki cross lang lookup finds nowiki' => [
'enwiki',
'crosslang', 'nb',
- ['no' => 'nowiki'],
+ ['nowiki', 'no'],
],
'enwikinews cross lang lookup finds frwikinews' => [
'enwikinews',
'crosslang', 'fr',
- ['fr' => 'frwikinews'],
+ ['frwikinews', 'fr'],
],
'enwikinews cross lang lookup cannot find inexistent
hawwikinews' => [
'enwikinews',
diff --git a/tests/unit/LanguageDetectTest.php
b/tests/unit/LanguageDetectTest.php
index c4f184b..3daa235 100644
--- a/tests/unit/LanguageDetectTest.php
+++ b/tests/unit/LanguageDetectTest.php
@@ -87,6 +87,7 @@
'cirrusDumpQuery' => 1,
] ) );
$this->setMwGlobals( [
+ 'wgCirrusSearchInterwikiSources' => null,
'wgCirrusSearchIndexBaseName' => 'mywiki',
'wgCirrusSearchExtraIndexes' => [NS_FILE =>
['externalwiki_file']],
] );
diff --git a/tests/unit/Search/ResultTest.php b/tests/unit/Search/ResultTest.php
index c792288..652c2eb 100644
--- a/tests/unit/Search/ResultTest.php
+++ b/tests/unit/Search/ResultTest.php
@@ -3,6 +3,7 @@
namespace CirrusSearch\Search;
use CirrusSearch\CirrusTestCase;
+use MediaWiki\Mediawikiservices;
/**
* @group CirrusSearch
@@ -14,15 +15,19 @@
'es' => 'eswiki',
],
] );
+ $config = MediaWikiServices::getInstance()
+ ->getConfigFactory()
+ ->makeConfig( 'CirrusSearch' );
$elasticaResultSet = $this->getMockBuilder(
\Elastica\ResultSet::class )
->disableOriginalConstructor()
->getMock();
- $elasticaResult = new \Elastica\Result( [
+ $data = [
'_index' => 'eswiki_content_123456',
'_source' => [
'namespace' => NS_MAIN,
+ 'namespace_text' => '',
'title' => 'Main Page',
'redirect' => [
[
@@ -35,11 +40,49 @@
'redirect.title' => [ 'Main' ],
'heading' => [ '...' ],
],
- ] );
- $result = new Result( $elasticaResultSet, $elasticaResult, 'es'
);
+ ];
+ $elasticaResult = new \Elastica\Result( $data );
+ // Test BC Code, interwiki info is obtained
+ // by SearchConfig::getWikiCode()
+ $result = new Result(
+ $elasticaResultSet,
+ $elasticaResult,
+ $config->newInterwikiConfig( 'eswiki', false )
+ );
- $this->assertTrue( $result->getTitle()->isExternal() );
- $this->assertTrue( $result->getRedirectTitle()->isExternal() );
- $this->assertTrue( $result->getSectionTitle()->isExternal() );
+ $this->assertTrue( $result->getTitle()->isExternal(),
'isExternal BC mode' );
+ $this->assertTrue( $result->getRedirectTitle()->isExternal(),
'redirect isExternal BC mode' );
+ $this->assertTrue( $result->getSectionTitle()->isExternal(),
'section title isExternal BC mode' );
+
+ // Should be the default mode soon interwiki is detected by
+ // reading the wiki source field
+ $data['_source']['wiki'] = 'eswiki';
+ $elasticaResult = new \Elastica\Result( $data );
+ $result = new Result(
+ $elasticaResultSet,
+ $elasticaResult,
+ $config
+ );
+
+ $this->assertTrue( $result->getTitle()->isExternal(),
'isExternal' );
+ $this->assertTrue( $result->getRedirectTitle()->isExternal(),
'redirect isExternal' );
+ $this->assertTrue( $result->getSectionTitle()->isExternal(),
'section title isExternal' );
+
+ // Test that we can't build the redirect title if the namespaces
+ // do not match
+ $data['_source']['namespace'] = NS_HELP;
+ $data['_source']['namespace_text'] = 'Help';
+ $elasticaResult = new \Elastica\Result( $data );
+
+ $result = new Result(
+ $elasticaResultSet,
+ $elasticaResult,
+ $config
+ );
+
+ $this->assertTrue( $result->getTitle()->isExternal(),
'isExternal namespace mismatch' );
+ $this->assertEquals( $result->getTitle()->getPrefixedText(),
'es:Help:Main Page' );
+ $this->assertTrue( $result->getRedirectTitle() === null,
'redirect is not built with ns mismatch' );
+ $this->assertTrue( $result->getSectionTitle()->isExternal(),
'section title isExternal' );
}
}
diff --git a/tests/unit/Search/ResultsTypeTest.php
b/tests/unit/Search/ResultsTypeTest.php
index 30c8354..1c1ea5c 100644
--- a/tests/unit/Search/ResultsTypeTest.php
+++ b/tests/unit/Search/ResultsTypeTest.php
@@ -35,7 +35,10 @@
array $expected
) {
$this->setMwGlobals(
'wgCirrusSearchUseExperimentalHighlighter', $useExperimentalHighlighter );
- $type = new FullTextResultsType( $highlightingConfig, '' );
+ $config = \MediaWiki\MediaWikiServices::getInstance()
+ ->getConfigFactory()
+ ->makeConfig( 'CirrusSearch' );
+ $type = new FullTextResultsType( $config, $highlightingConfig );
$this->assertEquals( $expected,
$type->getHighlightingConfiguration( $highlightSource ) );
}
--
To view, visit https://gerrit.wikimedia.org/r/320625
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6aebfa22c05a82f6d887f94ef6c18ce5b3a1b0bc
Gerrit-PatchSet: 22
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: DCausse <dcau...@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: Smalyshev <smalys...@wikimedia.org>
Gerrit-Reviewer: TTO <at.li...@live.com.au>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
