From 3115b3202d50d830a1db70d43bc32f6afb236930 Mon Sep 17 00:00:00 2001 From: David Causse Date: Mon, 9 Jul 2018 18:55:54 +0200 Subject: [PATCH] Unify SearchEngine normalizeNamespace and parseNamespacePrefixes These methods are very similar there should be no need to have two differents way to extract the namespace prefix. Bug: T198860 Change-Id: I22802278452559d35a3d8f6068549c1fef1a5e86 --- includes/PrefixSearch.php | 46 +------ includes/search/SearchEngine.php | 89 +++++++------- .../includes/search/SearchEngineTest.php | 116 ++++++++++++++++++ .../search/MockCompletionSearchEngine.php | 3 +- 4 files changed, 163 insertions(+), 91 deletions(-) diff --git a/includes/PrefixSearch.php b/includes/PrefixSearch.php index 5127158e69..63a4d9c620 100644 --- a/includes/PrefixSearch.php +++ b/includes/PrefixSearch.php @@ -58,54 +58,14 @@ abstract class PrefixSearch { return []; // Return empty result } - $hasNamespace = $this->extractNamespace( $search ); - if ( $hasNamespace ) { - list( $namespace, $search ) = $hasNamespace; - $namespaces = [ $namespace ]; - } else { - $namespaces = $this->validateNamespaces( $namespaces ); - Hooks::run( 'PrefixSearchExtractNamespace', [ &$namespaces, &$search ] ); + $hasNamespace = SearchEngine::parseNamespacePrefixes( $search, false, true ); + if ( $hasNamespace !== false ) { + list( $search, $namespaces ) = $hasNamespace; } return $this->searchBackend( $namespaces, $search, $limit, $offset ); } - /** - * Figure out if given input contains an explicit namespace. - * - * @param string $input - * @return false|array Array of namespace and remaining text, or false if no namespace given. - */ - protected function extractNamespace( $input ) { - if ( strpos( $input, ':' ) === false ) { - return false; - } - - // Namespace prefix only - $title = Title::newFromText( $input . 'Dummy' ); - if ( - $title && - $title->getText() === 'Dummy' && - !$title->inNamespace( NS_MAIN ) && - !$title->isExternal() - ) { - return [ $title->getNamespace(), '' ]; - } - - // Namespace prefix with additional input - $title = Title::newFromText( $input ); - if ( - $title && - !$title->inNamespace( NS_MAIN ) && - !$title->isExternal() - ) { - // getText provides correct capitalization - return [ $title->getNamespace(), $title->getText() ]; - } - - return false; - } - /** * Do a prefix search for all possible variants of the prefix * @param string $search diff --git a/includes/search/SearchEngine.php b/includes/search/SearchEngine.php index ea86a4b580..d46918f0ec 100644 --- a/includes/search/SearchEngine.php +++ b/includes/search/SearchEngine.php @@ -404,11 +404,21 @@ abstract class SearchEngine { * or namespace names * * @param string $query + * @param bool $withAllKeyword activate support of the "all:" keyword and its + * translations to activate searching on all namespaces. + * @param bool $withPrefixSearchExtractNamespaceHook call the PrefixSearchExtractNamespace hook + * if classic namespace identification did not match. * @return false|array false if no namespace was extracted, an array * with the parsed query at index 0 and an array of namespaces at index * 1 (or null for all namespaces). - */ - public static function parseNamespacePrefixes( $query ) { + * @throws FatalError + * @throws MWException + */ + public static function parseNamespacePrefixes( + $query, + $withAllKeyword = true, + $withPrefixSearchExtractNamespaceHook = false + ) { global $wgContLang; $parsed = $query; @@ -416,40 +426,48 @@ abstract class SearchEngine { return false; } $extractedNamespace = null; - $allkeywords = []; - - $allkeywords[] = wfMessage( 'searchall' )->inContentLanguage()->text() . ":"; - // force all: so that we have a common syntax for all the wikis - if ( !in_array( 'all:', $allkeywords ) ) { - $allkeywords[] = 'all:'; - } $allQuery = false; - foreach ( $allkeywords as $kw ) { - if ( strncmp( $query, $kw, strlen( $kw ) ) == 0 ) { - $extractedNamespace = null; - $parsed = substr( $query, strlen( $kw ) ); - $allQuery = true; - break; + if ( $withAllKeyword ) { + $allkeywords = []; + + $allkeywords[] = wfMessage( 'searchall' )->inContentLanguage()->text() . ":"; + // force all: so that we have a common syntax for all the wikis + if ( !in_array( 'all:', $allkeywords ) ) { + $allkeywords[] = 'all:'; + } + + foreach ( $allkeywords as $kw ) { + if ( strncmp( $query, $kw, strlen( $kw ) ) == 0 ) { + $extractedNamespace = null; + $parsed = substr( $query, strlen( $kw ) ); + $allQuery = true; + break; + } } } if ( !$allQuery && strpos( $query, ':' ) !== false ) { - // TODO: should we unify with PrefixSearch::extractNamespace ? $prefix = str_replace( ' ', '_', substr( $query, 0, strpos( $query, ':' ) ) ); $index = $wgContLang->getNsIndex( $prefix ); if ( $index !== false ) { $extractedNamespace = [ $index ]; $parsed = substr( $query, strlen( $prefix ) + 1 ); + } elseif ( $withPrefixSearchExtractNamespaceHook ) { + $hookNamespaces = [ NS_MAIN ]; + $hookQuery = $query; + Hooks::run( 'PrefixSearchExtractNamespace', [ &$hookNamespaces, &$hookQuery ] ); + if ( $hookQuery !== $query ) { + $parsed = $hookQuery; + $extractedNamespace = $hookNamespaces; + } else { + return false; + } } else { return false; } } - if ( trim( $parsed ) == '' ) { - $parsed = $query; // prefix was the whole query - } - return [ $parsed, $extractedNamespace ]; } @@ -532,34 +550,11 @@ abstract class SearchEngine { * @return string Simplified search string */ protected function normalizeNamespaces( $search ) { - // Find a Title which is not an interwiki and is in NS_MAIN - $title = Title::newFromText( $search ); - $ns = $this->namespaces; - if ( $title && !$title->isExternal() ) { - $ns = [ $title->getNamespace() ]; - $search = $title->getText(); - if ( $ns[0] == NS_MAIN ) { - $ns = $this->namespaces; // no explicit prefix, use default namespaces - Hooks::run( 'PrefixSearchExtractNamespace', [ &$ns, &$search ] ); - } - } else { - $title = Title::newFromText( $search . 'Dummy' ); - if ( $title && $title->getText() == 'Dummy' - && $title->getNamespace() != NS_MAIN - && !$title->isExternal() - ) { - $ns = [ $title->getNamespace() ]; - $search = ''; - } else { - Hooks::run( 'PrefixSearchExtractNamespace', [ &$ns, &$search ] ); - } + $queryAndNs = self::parseNamespacePrefixes( $search, false, true ); + if ( $queryAndNs !== false ) { + $this->setNamespaces( $queryAndNs[1] ); + return $queryAndNs[0]; } - - $ns = array_map( function ( $space ) { - return $space == NS_MEDIA ? NS_FILE : $space; - }, $ns ); - - $this->setNamespaces( $ns ); return $search; } diff --git a/tests/phpunit/includes/search/SearchEngineTest.php b/tests/phpunit/includes/search/SearchEngineTest.php index 5884d19343..c1f5cf8835 100644 --- a/tests/phpunit/includes/search/SearchEngineTest.php +++ b/tests/phpunit/includes/search/SearchEngineTest.php @@ -340,4 +340,120 @@ class SearchEngineTest extends MediaWikiLangTestCase { EDIT_NEW | EDIT_SUPPRESS_RC ); } + + public function provideDataForParseNamespacePrefix() { + return [ + 'noop' => [ + [ + 'query' => 'foo', + ], + false + ], + 'empty' => [ + [ + 'query' => '', + ], + false, + ], + 'namespace prefix' => [ + [ + 'query' => 'help:test', + ], + [ 'test', [ NS_HELP ] ], + ], + 'accented namespace prefix with hook' => [ + [ + 'query' => 'hélp:test', + 'withHook' => true, + ], + [ 'test', [ NS_HELP ] ], + ], + 'accented namespace prefix without hook' => [ + [ + 'query' => 'hélp:test', + 'withHook' => false, + ], + false, + ], + 'all with all keyword allowed' => [ + [ + 'query' => 'all:test', + 'withAll' => true, + ], + [ 'test', null ], + ], + 'all with all keyword disallowed' => [ + [ + 'query' => 'all:test', + 'withAll' => false, + ], + false + ], + 'ns only' => [ + [ + 'query' => 'help:', + ], + [ '', [ NS_HELP ] ] + ], + 'all only' => [ + [ + 'query' => 'all:', + 'withAll' => true, + ], + [ '', null ] + ], + 'all wins over namespace when first' => [ + [ + 'query' => 'all:help:test', + 'withAll' => true, + ], + [ 'help:test', null ] + ], + 'ns wins over all when first' => [ + [ + 'query' => 'help:all:test', + 'withAll' => true, + ], + [ 'all:test', [ NS_HELP ] ] + ], + ]; + } + + /** + * @dataProvider provideDataForParseNamespacePrefix + * @param array $params + * @param array|false $expected + * @throws FatalError + * @throws MWException + */ + public function testParseNamespacePrefix( array $params, $expected ) { + $this->setTemporaryHook( 'PrefixSearchExtractNamespace', function ( &$namespaces, &$query ) { + if ( strpos( $query, 'hélp:' ) === 0 ) { + $namespaces = [ NS_HELP ]; + $query = substr( $query, strlen( 'hélp:' ) ); + } + return false; + } ); + $testSet = []; + if ( isset( $params['withAll'] ) && isset( $params['withHook'] ) ) { + $testSet[] = $params; + } elseif ( isset( $params['withAll'] ) ) { + $testSet[] = $params + [ 'withHook' => true ]; + $testSet[] = $params + [ 'withHook' => false ]; + } elseif ( isset( $params['withHook'] ) ) { + $testSet[] = $params + [ 'withAll' => true ]; + $testSet[] = $params + [ 'withAll' => false ]; + } else { + $testSet[] = $params + [ 'withAll' => true, 'withHook' => true ]; + $testSet[] = $params + [ 'withAll' => true, 'withHook' => false ]; + $testSet[] = $params + [ 'withAll' => false, 'withHook' => false ]; + $testSet[] = $params + [ 'withAll' => true, 'withHook' => false ]; + } + + foreach ( $testSet as $test ) { + $actual = SearchEngine::parseNamespacePrefixes( $test['query'], + $test['withAll'], $test['withHook'] ); + $this->assertEquals( $expected, $actual, 'with params: ' . print_r( $test, true ) ); + } + } } diff --git a/tests/phpunit/mocks/search/MockCompletionSearchEngine.php b/tests/phpunit/mocks/search/MockCompletionSearchEngine.php index ac8a5dc702..13d16df737 100644 --- a/tests/phpunit/mocks/search/MockCompletionSearchEngine.php +++ b/tests/phpunit/mocks/search/MockCompletionSearchEngine.php @@ -27,11 +27,12 @@ class MockCompletionSearchEngine extends SearchEngine { */ public static function addMockResults( $query, array $result ) { // Leading : ensures we don't treat another : as a namespace separator - $normalized = Title::newFromText( ":$query" )->getText(); + $normalized = mb_strtolower( Title::newFromText( ":$query" )->getText() ); self::$results[$normalized] = $result; } public function completionSearchBackend( $search ) { + $search = mb_strtolower( $search ); if ( !isset( self::$results[$search] ) ) { return SearchSuggestionSet::emptySuggestionSet(); } -- 2.20.1