jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/320788 )
Change subject: Remove BC code after interwiki refactoring
......................................................................
Remove BC code after interwiki refactoring
This removes all the BC codes needed to support
interwiki searches on wikis where the wiki field
is not fully populated.
Bug: T141033
Change-Id: I9070e311ab1906038196dfda45ca1750cdceb9d0
---
M includes/CirrusSearch.php
M includes/Hooks.php
M includes/InterwikiSearcher.php
M includes/Search/Result.php
M includes/Search/ResultSet.php
M includes/Search/ResultsType.php
M includes/Search/TitleHelper.php
M includes/SearchConfig.php
M includes/Searcher.php
M tests/unit/Search/ResultTest.php
M tests/unit/Search/ResultsTypeTest.php
11 files changed, 52 insertions(+), 167 deletions(-)
Approvals:
Smalyshev: Looks good to me, approved
jenkins-bot: Verified
Objections:
Cindy-the-browser-test-bot: There's a problem with this change, please improve
diff --git a/includes/CirrusSearch.php b/includes/CirrusSearch.php
index ecfd76f..9b8e828 100644
--- a/includes/CirrusSearch.php
+++ b/includes/CirrusSearch.php
@@ -284,7 +284,7 @@
$altWikiId = $this->detectSecondaryLanguage( $term );
if ( $altWikiId ) {
try {
- $config = $this->config->newInterwikiConfig(
$altWikiId );
+ $config = new SearchConfig( $altWikiId );
} catch ( MWException $e ) {
LoggerFactory::getInstance( 'CirrusSearch'
)->info(
"Failed to get config for {dbwiki}",
@@ -381,7 +381,7 @@
$highlightingConfig ^=
FullTextResultsType::HIGHLIGHT_FILE_TEXT;
}
- $resultsType = new FullTextResultsType( $config,
$highlightingConfig );
+ $resultsType = new FullTextResultsType( $highlightingConfig );
$searcher->setResultsType( $resultsType );
$status = $searcher->searchText( $term, $this->showSuggestion );
@@ -617,10 +617,10 @@
$searcher = new Searcher( $this->connection, $this->offset,
$this->limit, $this->config, $this->namespaces );
if ( $search ) {
- $searcher->setResultsType( new FancyTitleResultsType(
$this->config, 'prefix' ) );
+ $searcher->setResultsType( new FancyTitleResultsType(
'prefix' ) );
} else {
// Empty searches always find the title.
- $searcher->setResultsType( new TitleResultsType(
$this->config ) );
+ $searcher->setResultsType( new TitleResultsType() );
}
try {
diff --git a/includes/Hooks.php b/includes/Hooks.php
index d3d1a11..99bdee7 100644
--- a/includes/Hooks.php
+++ b/includes/Hooks.php
@@ -550,7 +550,7 @@
} else {
$term = $title->getText();
}
- $searcher->setResultsType( new FancyTitleResultsType(
self::getConfig(), 'near_match' ) );
+ $searcher->setResultsType( new FancyTitleResultsType(
'near_match' ) );
try {
$status = $searcher->nearMatchTitleSearch( $term );
} catch ( ApiUsageException $e ) {
diff --git a/includes/InterwikiSearcher.php b/includes/InterwikiSearcher.php
index c3cad6b..20e0ec1 100644
--- a/includes/InterwikiSearcher.php
+++ b/includes/InterwikiSearcher.php
@@ -104,13 +104,8 @@
$retval = [];
$searches = [];
- $resultsTypes = [];
+ $this->setResultsType( new FullTextResultsType(
$this->highlightingConfig ) );
foreach ( $sources as $interwiki => $index ) {
- // TODO: remove when getWikiCode is removed.
- // In theory we should be able to reuse the same
- // Results type for all searches
- $resultsTypes[$interwiki] = new FullTextResultsType(
$this->config->newInterwikiConfig( $index, false ), $this->highlightingConfig );
- $this->setResultsType( $resultsTypes[$interwiki] );
$this->indexBaseName = $index;
$this->searchContext = clone $context;
$search = $this->buildSearch();
@@ -121,7 +116,7 @@
}
}
- $results = $this->searchMulti( $searches, $resultsTypes );
+ $results = $this->searchMulti( $searches );
if ( !$results->isOK() ) {
return null;
}
diff --git a/includes/Search/Result.php b/includes/Search/Result.php
index 071cfb5..2c08571 100644
--- a/includes/Search/Result.php
+++ b/includes/Search/Result.php
@@ -5,7 +5,6 @@
use CirrusSearch\InterwikiSearcher;
use CirrusSearch\Util;
use CirrusSearch\Searcher;
-use CirrusSearch\SearchConfig;
use MediaWiki\Logger\LoggerFactory;
use MWTimestamp;
use SearchResult;
@@ -30,7 +29,6 @@
* http://www.gnu.org/copyleft/gpl.html
*/
class Result extends SearchResult {
- use TitleHelper;
/** @var int */
private $namespace;
@@ -50,8 +48,6 @@
private $textSnippet;
/** @var bool */
private $isFileMatch = false;
- /* @var SearchConfig */
- private $config;
/* @var string result wiki */
private $wiki;
/** @var string */
@@ -76,17 +72,15 @@
*
* @param \Elastica\ResultSet $results containing all search results
* @param \Elastica\Result $result containing the given search result
- * @param SearchConfig $config
*/
- public function __construct( $results, $result, SearchConfig $config ) {
+ public function __construct( $results, $result ) {
global $wgCirrusSearchDevelOptions;
$this->ignoreMissingRev = isset(
$wgCirrusSearchDevelOptions['ignore_missing_rev'] );
- $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 );
+ $this->mTitle = TitleHelper::makeTitle( $result );
if ( $this->getTitle()->getNamespace() == NS_FILE ) {
$this->mImage = wfFindFile( $this->mTitle );
}
@@ -231,7 +225,7 @@
);
return null;
}
- return $this->makeRedirectTitle( $result, $best['title'],
$best['namespace'] );
+ return TitleHelper::makeRedirectTitle( $result, $best['title'],
$best['namespace'] );
}
/**
@@ -364,12 +358,5 @@
*/
public function getExplanation() {
return $this->explanation;
- }
-
- /**
- * @return SearchConfig $config
- */
- public function getConfig() {
- return $this->config;
}
}
diff --git a/includes/Search/ResultSet.php b/includes/Search/ResultSet.php
index aaec649..9fae631 100644
--- a/includes/Search/ResultSet.php
+++ b/includes/Search/ResultSet.php
@@ -3,7 +3,6 @@
namespace CirrusSearch\Search;
use CirrusSearch\Searcher;
-use CirrusSearch\SearchConfig;
use LinkBatch;
use SearchResultSet;
@@ -26,7 +25,6 @@
* http://www.gnu.org/copyleft/gpl.html
*/
class ResultSet extends SearchResultSet {
- use TitleHelper;
/**
* @var \Elastica\ResultSet
@@ -59,11 +57,6 @@
private $searchContainedSyntax;
/**
- * @var SearchConfig
- */
- private $config;
-
- /**
* @var array
*/
private $interwikiResults = [];
@@ -83,14 +76,12 @@
* @param string[] $suggestSuffixes
* @param \Elastica\ResultSet $res
* @param bool $searchContainedSyntax
- * @param SearchConfig $config
*/
- public function __construct( array $suggestPrefixes, array
$suggestSuffixes, \Elastica\ResultSet $res, $searchContainedSyntax,
SearchConfig $config ) {
+ public function __construct( array $suggestPrefixes, array
$suggestSuffixes, \Elastica\ResultSet $res, $searchContainedSyntax ) {
$this->result = $res;
$this->searchContainedSyntax = $searchContainedSyntax;
$this->hits = $res->count();
$this->totalHits = $res->getTotalHits();
- $this->config = $config;
$this->preCacheContainedTitles( $this->result );
$suggestion = $this->findSuggestion();
if ( $suggestion && !
$this->resultContainsFullyHighlightedMatch() ) {
@@ -197,7 +188,7 @@
// We can only pull in information about the local wiki
$lb = new LinkBatch;
foreach ( $resultSet->getResults() as $result ) {
- if ( !$this->isExternal( $result ) ) {
+ if ( !TitleHelper::isExternal( $result ) ) {
$lb->add( $result->namespace, $result->title );
}
@@ -250,7 +241,7 @@
$current = $this->result->current();
if ( $current ) {
$this->result->next();
- $result = new Result( $this->result, $current,
$this->config );
+ $result = new Result( $this->result, $current );
$this->augmentResult( $result );
return $result;
}
@@ -321,12 +312,5 @@
*/
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 c2451ca..0b59b4b 100644
--- a/includes/Search/ResultsType.php
+++ b/includes/Search/ResultsType.php
@@ -3,7 +3,6 @@
namespace CirrusSearch\Search;
use CirrusSearch\Searcher;
-use CirrusSearch\SearchConfig;
use MediaWiki\Logger\LoggerFactory;
use Title;
@@ -64,25 +63,6 @@
}
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
@@ -119,7 +99,7 @@
public function transformElasticsearchResult( SearchContext $context,
\Elastica\ResultSet $resultSet ) {
$results = [];
foreach( $resultSet->getResults() as $r ) {
- $results[] = $this->makeTitle( $r );
+ $results[] = TitleHelper::makeTitle( $r );
}
return $results;
}
@@ -143,11 +123,9 @@
* 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( SearchConfig $config, $matchedAnalyzer ) {
- parent::__construct( $config );
+ public function __construct( $matchedAnalyzer ) {
$this->matchedAnalyzer = $matchedAnalyzer;
}
@@ -212,7 +190,7 @@
public function transformElasticsearchResult( SearchContext $context,
\Elastica\ResultSet $resultSet ) {
$results = [];
foreach( $resultSet->getResults() as $r ) {
- $title = $this->makeTitle( $r );
+ $title = TitleHelper::makeTitle( $r );
$highlights = $r->getHighlights();
$resultForTitle = [];
@@ -244,7 +222,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 =
$this->makeRedirectTitle( $r, $redirectTitle, $r->namespace );
+ $redirectTitle =
TitleHelper::makeRedirectTitle( $r, $redirectTitle, $r->namespace );
$resultForTitle[ 'redirectMatches' ][]
= $redirectTitle;
}
}
@@ -294,11 +272,9 @@
private $highlightingConfig;
/**
- * @param SearchConfig $config
* @param int $highlightingConfig Bitmask, see HIGHLIGHT_* consts
*/
- public function __construct( SearchConfig $config, $highlightingConfig
) {
- parent::__construct( $config );
+ public function __construct( $highlightingConfig ) {
$this->highlightingConfig = $highlightingConfig;
}
@@ -477,8 +453,7 @@
$context->getSuggestPrefixes(),
$context->getSuggestSuffixes(),
$result,
- $context->isSpecialKeywordUsed(),
- $this->getConfig()
+ $context->isSpecialKeywordUsed()
);
}
diff --git a/includes/Search/TitleHelper.php b/includes/Search/TitleHelper.php
index 9d77b71..0c52f46 100644
--- a/includes/Search/TitleHelper.php
+++ b/includes/Search/TitleHelper.php
@@ -2,18 +2,23 @@
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
+ * Utility class build MW Title from elastica Result/ResultSet classes
+ * This class can be used in all classes that need to build a Title
* by reading the elasticsearch output.
*/
-trait TitleHelper {
+class TitleHelper {
+
+ /**
+ * Utility class, should not be instantiated
+ */
+ private function __construct() {
+ }
+
/**
* 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,
@@ -21,11 +26,11 @@
* 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 \Elastica\Result $r int $namespace
* @return Title
*/
- public function makeTitle( Result $r ) {
- $iwPrefix = $this->identifyInterwikiPrefix( $r );
+ public static function makeTitle( \Elastica\Result $r ) {
+ $iwPrefix = self::identifyInterwikiPrefix( $r );
if ( empty( $iwPrefix ) ) {
return Title::makeTitle( $r->namespace, $r->title );
} else {
@@ -40,13 +45,13 @@
* 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 \Elastica\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 );
+ public static function makeRedirectTitle( \Elastica\Result $r,
$redirectText, $redirNamespace ) {
+ $iwPrefix = self::identifyInterwikiPrefix( $r );
if ( empty( $iwPrefix ) ) {
return Title::makeTitle( $redirNamespace, $redirectText
);
}
@@ -67,37 +72,29 @@
}
/**
- * @param Result $r
+ * @param \Elastica\Result $r
* @return bool true if this result refers to an external Title
*/
- public function isExternal( Result $r ) {
+ public static function isExternal( \Elastica\Result $r ) {
if ( isset ( $r->wiki ) && $r->wiki !== wfWikiID() ) {
return true;
}
- // TODO: replace by return false when wiki is populated
- /** @suppress PhanDeprecatedFunction */
- return !empty( $this->getConfig()->getWikiCode() );
+ // no wiki is suspicious, should we log a warning?
+ return false;
}
/**
- * @param Result $r
+ * @param \Elastica\Result $r
* @return string|null the interwiki prefix for this result or null or
* empty if local.
*/
- public function identifyInterwikiPrefix( $r ) {
+ public static 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
- /** @suppress PhanDeprecatedFunction */
- return $this->getConfig()->getWikiCode();
+ // no wiki is suspicious, should we log something?
+ return null;
}
-
- /**
- * TODO: remove when getWikiCode is removed
- * @return SearchConfig
- */
- public abstract function getConfig();
}
diff --git a/includes/SearchConfig.php b/includes/SearchConfig.php
index 10dd844..e8ad504 100644
--- a/includes/SearchConfig.php
+++ b/includes/SearchConfig.php
@@ -67,17 +67,14 @@
* 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( $overrideName = null, $fullLoad = true ) {
+ public function __construct( $overrideName = null ) {
if ( $overrideName && $overrideName != wfWikiID() ) {
$this->wikiId = $overrideName;
- 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' ) ) );
- }
+ $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;
}
$this->source = new \GlobalVarConfig();
@@ -333,37 +330,5 @@
return true;
}
return false;
- }
-
- /**
- * Build a new SearchConfig based on $wiki
- * TODO: remove $fullLoad
- * @param $wiki dbname of the target wiki
- * @param bool $fullLoad
- * @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 9db56c6..3c8c50b 100644
--- a/includes/Searcher.php
+++ b/includes/Searcher.php
@@ -513,7 +513,7 @@
protected function buildSearch() {
if ( $this->resultsType === null ) {
- $this->resultsType = new FullTextResultsType(
$this->config, FullTextResultsType::HIGHLIGHT_ALL );
+ $this->resultsType = new FullTextResultsType(
FullTextResultsType::HIGHLIGHT_ALL );
}
$query = new \Elastica\Query();
@@ -1096,7 +1096,7 @@
*/
public function searchArchive( $term ) {
list( $term, $fuzzyUnused ) =
$this->searchContext->escaper()->fixupWholeQueryString( $term );
- $this->setResultsType( new TitleResultsType( $this->config ) );
+ $this->setResultsType( new TitleResultsType() );
$this->pageType = $this->connection->getArchiveType(
$this->indexBaseName );
diff --git a/tests/unit/Search/ResultTest.php b/tests/unit/Search/ResultTest.php
index 652c2eb..dd91223 100644
--- a/tests/unit/Search/ResultTest.php
+++ b/tests/unit/Search/ResultTest.php
@@ -29,6 +29,7 @@
'namespace' => NS_MAIN,
'namespace_text' => '',
'title' => 'Main Page',
+ 'wiki' => 'eswiki',
'redirect' => [
[
'title' => 'Main',
@@ -41,22 +42,6 @@
'heading' => [ '...' ],
],
];
- $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(),
'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,
diff --git a/tests/unit/Search/ResultsTypeTest.php
b/tests/unit/Search/ResultsTypeTest.php
index 1c1ea5c..c5f1589 100644
--- a/tests/unit/Search/ResultsTypeTest.php
+++ b/tests/unit/Search/ResultsTypeTest.php
@@ -35,10 +35,7 @@
array $expected
) {
$this->setMwGlobals(
'wgCirrusSearchUseExperimentalHighlighter', $useExperimentalHighlighter );
- $config = \MediaWiki\MediaWikiServices::getInstance()
- ->getConfigFactory()
- ->makeConfig( 'CirrusSearch' );
- $type = new FullTextResultsType( $config, $highlightingConfig );
+ $type = new FullTextResultsType( $highlightingConfig );
$this->assertEquals( $expected,
$type->getHighlightingConfiguration( $highlightSource ) );
}
--
To view, visit https://gerrit.wikimedia.org/r/320788
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9070e311ab1906038196dfda45ca1750cdceb9d0
Gerrit-PatchSet: 30
Gerrit-Project: mediawiki/extensions/CirrusSearch
Gerrit-Branch: master
Gerrit-Owner: DCausse <[email protected]>
Gerrit-Reviewer: Cindy-the-browser-test-bot <[email protected]>
Gerrit-Reviewer: DCausse <[email protected]>
Gerrit-Reviewer: EBernhardson <[email protected]>
Gerrit-Reviewer: Gehel <[email protected]>
Gerrit-Reviewer: Manybubbles <[email protected]>
Gerrit-Reviewer: Smalyshev <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
