jenkins-bot has submitted this change and it was merged.
Change subject: Push common search api parameters into SearchApi class
......................................................................
Push common search api parameters into SearchApi class
We have a number of parameters that are pretty much the same between
these different search api's. Lets make them actually the same by
sharing the definitions, and then letting individual classes tweak them
as needed by removing the offset, or adjusting the max limits as
necessary.
Change-Id: I6f987db8ecb63dc943b4d2518bfe3703c677448e
---
M includes/api/ApiOpenSearch.php
M includes/api/ApiQueryPrefixSearch.php
M includes/api/ApiQuerySearch.php
M includes/api/SearchApi.php
A tests/phpunit/includes/api/ApiOpenSearchTest.php
5 files changed, 192 insertions(+), 139 deletions(-)
Approvals:
Smalyshev: Looks good to me, but someone else must approve
DCausse: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/api/ApiOpenSearch.php b/includes/api/ApiOpenSearch.php
index 066aaa3..b13ba58 100644
--- a/includes/api/ApiOpenSearch.php
+++ b/includes/api/ApiOpenSearch.php
@@ -272,20 +272,7 @@
if ( $this->allowedParams !== null ) {
return $this->allowedParams;
}
- $this->allowedParams = [
- 'search' => null,
- 'limit' => [
- ApiBase::PARAM_DFLT => $this->getConfig()->get(
'OpenSearchDefaultLimit' ),
- ApiBase::PARAM_TYPE => 'limit',
- ApiBase::PARAM_MIN => 1,
- ApiBase::PARAM_MAX => 100,
- ApiBase::PARAM_MAX2 => 100
- ],
- 'namespace' => [
- ApiBase::PARAM_DFLT => NS_MAIN,
- ApiBase::PARAM_TYPE => 'namespace',
- ApiBase::PARAM_ISMULTI => true
- ],
+ $this->allowedParams = $this->buildCommonApiParams( false ) + [
'suggest' => false,
'redirects' => [
ApiBase::PARAM_TYPE => [ 'return', 'resolve' ],
@@ -297,19 +284,21 @@
'warningsaserror' => false,
];
- $profileParam = $this->buildProfileApiParam(
SearchEngine::COMPLETION_PROFILE_TYPE,
- 'apihelp-query+prefixsearch-param-profile' );
- if ( $profileParam ) {
- $this->allowedParams['profile'] = $profileParam;
- }
+ // Use open search specific default limit
+ $this->allowedParams['limit'][ApiBase::PARAM_DFLT] =
$this->getConfig()->get(
+ 'OpenSearchDefaultLimit'
+ );
+
return $this->allowedParams;
}
public function getSearchProfileParams() {
- if ( isset( $this->getAllowedParams()['profile'] ) ) {
- return [ SearchEngine::COMPLETION_PROFILE_TYPE =>
'profile' ];
- }
- return [];
+ return [
+ 'qiprofile' => [
+ 'profile-type' =>
SearchEngine::COMPLETION_PROFILE_TYPE,
+ 'help-message' =>
'apihelp-query+prefixsearch-param-profile'
+ ],
+ ];
}
protected function getExamplesMessages() {
diff --git a/includes/api/ApiQueryPrefixSearch.php
b/includes/api/ApiQueryPrefixSearch.php
index 46538e0..5bd451e 100644
--- a/includes/api/ApiQueryPrefixSearch.php
+++ b/includes/api/ApiQueryPrefixSearch.php
@@ -106,42 +106,18 @@
if ( $this->allowedParams !== null ) {
return $this->allowedParams;
}
- $this->allowedParams = [
- 'search' => [
- ApiBase::PARAM_TYPE => 'string',
- ApiBase::PARAM_REQUIRED => true,
- ],
- 'namespace' => [
- ApiBase::PARAM_DFLT => NS_MAIN,
- ApiBase::PARAM_TYPE => 'namespace',
- ApiBase::PARAM_ISMULTI => true,
- ],
- 'limit' => [
- ApiBase::PARAM_DFLT => 10,
- ApiBase::PARAM_TYPE => 'limit',
- ApiBase::PARAM_MIN => 1,
- // Non-standard value for compatibility with
action=opensearch
- ApiBase::PARAM_MAX => 100,
- ApiBase::PARAM_MAX2 => 200,
- ],
- 'offset' => [
- ApiBase::PARAM_DFLT => 0,
- ApiBase::PARAM_TYPE => 'integer',
- ],
- ];
- $profileParam = $this->buildProfileApiParam(
SearchEngine::COMPLETION_PROFILE_TYPE,
- 'apihelp-query+prefixsearch-param-profile' );
- if ( $profileParam ) {
- $this->allowedParams['profile'] = $profileParam;
- }
+ $this->allowedParams = $this->buildCommonApiParams();
+
return $this->allowedParams;
}
public function getSearchProfileParams() {
- if ( isset( $this->getAllowedParams()['profile'] ) ) {
- return [ SearchEngine::COMPLETION_PROFILE_TYPE =>
'profile' ];
- }
- return [];
+ return [
+ 'profile' => [
+ 'profile-type' =>
SearchEngine::COMPLETION_PROFILE_TYPE,
+ 'help-message' =>
'apihelp-query+prefixsearch-param-profile',
+ ],
+ ];
}
protected function getExamplesMessages() {
diff --git a/includes/api/ApiQuerySearch.php b/includes/api/ApiQuerySearch.php
index 80798a1..a05f6be 100644
--- a/includes/api/ApiQuerySearch.php
+++ b/includes/api/ApiQuerySearch.php
@@ -37,14 +37,6 @@
/** @var array list of api allowed params */
private $allowedParams;
- /**
- * When $wgSearchType is null, $wgSearchAlternatives[0] is null. Null
isn't
- * a valid option for an array for PARAM_TYPE, so we'll use a fake name
- * that can't possibly be a class name and describes what the null
behavior
- * does
- */
- const BACKEND_NULL_PARAM = 'database-backed';
-
public function __construct( ApiQuery $query, $moduleName ) {
parent::__construct( $query, $moduleName, 'sr' );
}
@@ -64,10 +56,6 @@
private function run( $resultPageSet = null ) {
global $wgContLang;
$params = $this->extractRequestParams();
-
- if ( isset( $params['backend'] ) && $params['backend'] ==
self::BACKEND_NULL_PARAM ) {
- unset( $params['backend'] );
- }
// Extract parameters
$query = $params['search'];
@@ -309,16 +297,7 @@
return $this->allowedParams;
}
- $this->allowedParams = [
- 'search' => [
- ApiBase::PARAM_TYPE => 'string',
- ApiBase::PARAM_REQUIRED => true
- ],
- 'namespace' => [
- ApiBase::PARAM_DFLT => NS_MAIN,
- ApiBase::PARAM_TYPE => 'namespace',
- ApiBase::PARAM_ISMULTI => true,
- ],
+ $this->allowedParams = $this->buildCommonApiParams() + [
'what' => [
ApiBase::PARAM_TYPE => [
'title',
@@ -355,52 +334,20 @@
ApiBase::PARAM_ISMULTI => true,
ApiBase::PARAM_HELP_MSG_PER_VALUE => [],
],
- 'offset' => [
- ApiBase::PARAM_DFLT => 0,
- ApiBase::PARAM_HELP_MSG =>
'api-help-param-continue',
- ],
- 'limit' => [
- ApiBase::PARAM_DFLT => 10,
- ApiBase::PARAM_TYPE => 'limit',
- ApiBase::PARAM_MIN => 1,
- ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1,
- ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2
- ],
'interwiki' => false,
'enablerewrites' => false,
];
-
- $searchConfig =
MediaWikiServices::getInstance()->getSearchEngineConfig();
- $alternatives = $searchConfig->getSearchTypes();
- if ( count( $alternatives ) > 1 ) {
- if ( $alternatives[0] === null ) {
- $alternatives[0] = self::BACKEND_NULL_PARAM;
- }
- $this->allowedParams['backend'] = [
- ApiBase::PARAM_DFLT =>
$searchConfig->getSearchType(),
- ApiBase::PARAM_TYPE => $alternatives,
- ];
- // @todo: support profile selection when multiple
- // backends are available. The solution could be to
- // merge all possible profiles and let ApiBase
- // subclasses do the check. Making ApiHelp and
ApiSandbox
- // comprehensive might be more difficult.
- } else {
- $profileParam = $this->buildProfileApiParam(
SearchEngine::FT_QUERY_INDEP_PROFILE_TYPE,
- 'apihelp-query+search-param-qiprofile' );
- if ( $profileParam ) {
- $this->allowedParams['qiprofile'] =
$profileParam;
- }
- }
return $this->allowedParams;
}
public function getSearchProfileParams() {
- if ( isset( $this->getAllowedParams()['qiprofile'] ) ) {
- return [ SearchEngine::FT_QUERY_INDEP_PROFILE_TYPE =>
'qiprofile' ];
- }
- return [];
+ return [
+ 'qiprofile' => [
+ 'profile-type' =>
SearchEngine::FT_QUERY_INDEP_PROFILE_TYPE,
+ 'help-message' =>
'apihelp-query+search-param-qiprofile',
+ ],
+ ];
}
protected function getExamplesMessages() {
diff --git a/includes/api/SearchApi.php b/includes/api/SearchApi.php
index 139793d..8ae1192 100644
--- a/includes/api/SearchApi.php
+++ b/includes/api/SearchApi.php
@@ -26,26 +26,89 @@
* @ingroup API
*/
trait SearchApi {
+
/**
- * Build the profile api param definitions.
- *
- * @param string $profileType type of profile to customize
- * @param string $helpMsg i18n message
- * @param string|null $backendType SearchEngine backend type or null
for default engine
- * @return array|null the api param definition or null if profiles are
- * not supported by the searchEngine implementation.
+ * When $wgSearchType is null, $wgSearchAlternatives[0] is null. Null
isn't
+ * a valid option for an array for PARAM_TYPE, so we'll use a fake name
+ * that can't possibly be a class name and describes what the null
behavior
+ * does
*/
- public function buildProfileApiParam( $profileType, $helpMsg,
$backendType = null ) {
- $searchEngine = null;
- if ( $backendType !== null ) {
- $searchEngine = MediaWikiServices::getInstance()
- ->getSearchEngineFactory()->create(
$backendType );
- } else {
- $searchEngine =
MediaWikiServices::getInstance()->newSearchEngine();
+ private static $BACKEND_NULL_PARAM = 'database-backed';
+
+ /**
+ * The set of api parameters that are shared between api calls that
+ * call the SearchEngine. Primarily this defines parameters that
+ * are utilized by self::buildSearchEngine().
+ *
+ * @param bool $isScrollable True if the api offers scrolling
+ * @return array
+ */
+ public function buildCommonApiParams( $isScrollable = true ) {
+ $params = [
+ 'search' => [
+ ApiBase::PARAM_TYPE => 'string',
+ ApiBase::PARAM_REQUIRED => true,
+ ],
+ 'namespace' => [
+ ApiBase::PARAM_DFLT => NS_MAIN,
+ ApiBase::PARAM_TYPE => 'namespace',
+ ApiBase::PARAM_ISMULTI => true,
+ ],
+ 'limit' => [
+ ApiBase::PARAM_DFLT => 10,
+ ApiBase::PARAM_TYPE => 'limit',
+ ApiBase::PARAM_MIN => 1,
+ ApiBase::PARAM_MAX => ApiBase::LIMIT_BIG1,
+ ApiBase::PARAM_MAX2 => ApiBase::LIMIT_BIG2,
+ ],
+ ];
+ if ( $isScrollable ) {
+ $params['offset'] = [
+ ApiBase::PARAM_DFLT => 0,
+ ApiBase::PARAM_TYPE => 'integer',
+ ApiBase::PARAM_HELP_MSG =>
'api-help-param-continue',
+ ];
}
- $profiles = $searchEngine->getProfiles( $profileType );
- if ( $profiles ) {
+ $searchConfig =
MediaWikiServices::getInstance()->getSearchEngineConfig();
+ $alternatives = $searchConfig->getSearchTypes();
+ if ( count( $alternatives ) > 1 ) {
+ if ( $alternatives[0] === null ) {
+ $alternatives[0] = self::$BACKEND_NULL_PARAM;
+ }
+ $this->allowedParams['backend'] = [
+ ApiBase::PARAM_DFLT =>
$searchConfig->getSearchType(),
+ ApiBase::PARAM_TYPE => $alternatives,
+ ];
+ // @todo: support profile selection when multiple
+ // backends are available. The solution could be to
+ // merge all possible profiles and let ApiBase
+ // subclasses do the check. Making ApiHelp and
ApiSandbox
+ // comprehensive might be more difficult.
+ } else {
+ $params += $this->buildProfileApiParam();
+ }
+
+ return $params;
+ }
+
+ /**
+ * Build the profile api param definitions. Makes bold assumption only
one search
+ * engine is available, ensure that is true before calling.
+ *
+ * @return array array containing available additional api param
definitions.
+ * Empty if profiles are not supported by the searchEngine
implementation.
+ */
+ private function buildProfileApiParam() {
+ $configs = $this->getSearchProfileParams();
+ $searchEngine =
MediaWikiServices::getInstance()->newSearchEngine();
+ $params = [];
+ foreach ( $configs as $paramName => $paramConfig ) {
+ $profiles = $searchEngine->getProfiles(
$paramConfig['profile-type'] );
+ if ( !$profiles ) {
+ continue;
+ }
+
$types = [];
$helpMessages = [];
$defaultProfile = null;
@@ -58,20 +121,23 @@
$defaultProfile = $profile['name'];
}
}
- return [
+
+ $params[$paramName] = [
ApiBase::PARAM_TYPE => $types,
- ApiBase::PARAM_HELP_MSG => $helpMsg,
+ ApiBase::PARAM_HELP_MSG =>
$paramConfig['help-message'],
ApiBase::PARAM_HELP_MSG_PER_VALUE =>
$helpMessages,
ApiBase::PARAM_DFLT => $defaultProfile,
];
}
- return null;
+
+ return $params;
}
/**
* Build the search engine to use.
* If $params is provided then the following searchEngine options
* will be set:
+ * - backend: which search backend to use
* - limit: mandatory
* - offset: optional, if set limit will be incremented by
* one ( to support the continue parameter )
@@ -84,6 +150,9 @@
public function buildSearchEngine( array $params = null ) {
if ( $params != null ) {
$type = isset( $params['backend'] ) ?
$params['backend'] : null;
+ if ( $type === self::$BACKEND_NULL_PARAM ) {
+ $type = null;
+ }
$searchEngine =
MediaWikiServices::getInstance()->getSearchEngineFactory()->create( $type );
$limit = $params['limit'];
$searchEngine->setNamespaces( $params['namespace'] );
@@ -97,9 +166,15 @@
$limit += 1;
}
$searchEngine->setLimitOffset( $limit, $offset );
- foreach ( $this->getSearchProfileParams() as $type =>
$param ) {
- if ( isset( $params[$param] ) ) {
- $searchEngine->setFeatureData( $type,
$params[$param] );
+
+ // Initialize requested search profiles.
+ $configs = $this->getSearchProfileParams();
+ foreach ( $configs as $paramName => $paramConfig ) {
+ if ( isset( $params[$paramName] ) ) {
+ $searchEngine->setFeatureData(
+ $paramConfig['profile-type'],
+ $params[$paramName]
+ );
}
}
} else {
@@ -109,8 +184,8 @@
}
/**
- * @return string[] the list of supported search profile types. Key is
- * the profile type and its associated value is the request param.
+ * @return array[] array of arrays mapping from parameter name to a two
value map
+ * containing 'help-message' and 'profile-type' keys.
*/
abstract public function getSearchProfileParams();
}
diff --git a/tests/phpunit/includes/api/ApiOpenSearchTest.php
b/tests/phpunit/includes/api/ApiOpenSearchTest.php
new file mode 100644
index 0000000..fd5b3a9
--- /dev/null
+++ b/tests/phpunit/includes/api/ApiOpenSearchTest.php
@@ -0,0 +1,66 @@
+<?php
+
+use MediaWiki\MediaWikiServices;
+
+class ApiOpenSearchTest extends MediaWikiTestCase {
+ public function testGetAllowedParams() {
+ $config = $this->replaceSearchEngineConfig();
+ $config->expects( $this->any() )
+ ->method( 'getSearchTypes' )
+ ->will( $this->returnValue( [ 'the one ring' ] ) );
+
+ $engine = $this->replaceSearchEngine();
+ $engine->expects( $this->any() )
+ ->method( 'getProfiles' )
+ ->will( $this->returnValueMap( [
+ [ SearchEngine::COMPLETION_PROFILE_TYPE, [
+ [
+ 'name' => 'normal',
+ 'desc-message' =>
'normal-message',
+ 'default' => true,
+ ],
+ [
+ 'name' => 'strict',
+ 'desc-message' =>
'strict-message',
+ ],
+ ] ],
+ ] ) );
+
+ $api = $this->createApi();
+ $params = $api->getAllowedParams();
+
+ $this->assertArrayNotHasKey( 'offset', $params );
+ $this->assertArrayHasKey( 'qiprofile', $params, print_r(
$params, true ) );
+ $this->assertEquals( 'normal',
$params['qiprofile'][ApiBase::PARAM_DFLT] );
+ }
+
+ private function replaceSearchEngineConfig() {
+ $config = $this->getMockBuilder( 'SearchEngineConfig' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $this->setService( 'SearchEngineConfig', $config );
+
+ return $config;
+ }
+
+ private function replaceSearchEngine() {
+ $engine = $this->getMockBuilder( 'SearchEngine' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $engineFactory = $this->getMockBuilder( 'SearchEngineFactory' )
+ ->disableOriginalConstructor()
+ ->getMock();
+ $engineFactory->expects( $this->any() )
+ ->method( 'create' )
+ ->will( $this->returnValue( $engine ) );
+ $this->setService( 'SearchEngineFactory', $engineFactory );
+
+ return $engine;
+ }
+
+ private function createApi() {
+ $ctx = new RequestContext();
+ $apiMain = new ApiMain( $ctx );
+ return new ApiOpenSearch( $apiMain, 'opensearch', '' );
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/292477
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I6f987db8ecb63dc943b4d2518bfe3703c677448e
Gerrit-PatchSet: 11
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: EBernhardson <[email protected]>
Gerrit-Reviewer: Anomie <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: DCausse <[email protected]>
Gerrit-Reviewer: EBernhardson <[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
