Push common search api parameters into SearchApi class
authorErik Bernhardson <ebernhardson@wikimedia.org>
Thu, 2 Jun 2016 20:32:59 +0000 (13:32 -0700)
committerErik Bernhardson <ebernhardson@wikimedia.org>
Tue, 26 Jul 2016 15:56:00 +0000 (08:56 -0700)
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

includes/api/ApiOpenSearch.php
includes/api/ApiQueryPrefixSearch.php
includes/api/ApiQuerySearch.php
includes/api/SearchApi.php
tests/phpunit/includes/api/ApiOpenSearchTest.php [new file with mode: 0644]

index 066aaa3..b13ba58 100644 (file)
@@ -272,20 +272,7 @@ class ApiOpenSearch extends ApiBase {
                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 @@ class ApiOpenSearch extends ApiBase {
                        '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() {
index 46538e0..5bd451e 100644 (file)
@@ -106,42 +106,18 @@ class ApiQueryPrefixSearch extends ApiQueryGeneratorBase {
                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() {
index 80798a1..a05f6be 100644 (file)
@@ -37,14 +37,6 @@ class ApiQuerySearch extends ApiQueryGeneratorBase {
        /** @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' );
        }
@@ -65,10 +57,6 @@ class ApiQuerySearch extends ApiQueryGeneratorBase {
                global $wgContLang;
                $params = $this->extractRequestParams();
 
-               if ( isset( $params['backend'] ) && $params['backend'] == self::BACKEND_NULL_PARAM ) {
-                       unset( $params['backend'] );
-               }
-
                // Extract parameters
                $query = $params['search'];
                $what = $params['what'];
@@ -309,16 +297,7 @@ class ApiQuerySearch extends ApiQueryGeneratorBase {
                        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 @@ class ApiQuerySearch extends ApiQueryGeneratorBase {
                                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() {
index 139793d..8ae1192 100644 (file)
@@ -26,26 +26,89 @@ use MediaWiki\MediaWikiServices;
  * @ingroup API
  */
 trait SearchApi {
+
+       /**
+        * 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
+        */
+       private static $BACKEND_NULL_PARAM = 'database-backed';
+
        /**
-        * Build the profile api param definitions.
+        * 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 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.
+        * @param bool $isScrollable True if the api offers scrolling
+        * @return array
         */
-       public function buildProfileApiParam( $profileType, $helpMsg, $backendType = null ) {
-               $searchEngine = null;
-               if ( $backendType !== null ) {
-                       $searchEngine = MediaWikiServices::getInstance()
-                               ->getSearchEngineFactory()->create( $backendType );
+       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',
+                       ];
+               }
+
+               $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 {
-                       $searchEngine = MediaWikiServices::getInstance()->newSearchEngine();
+                       $params += $this->buildProfileApiParam();
                }
 
-               $profiles = $searchEngine->getProfiles( $profileType );
-               if ( $profiles ) {
+               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 @@ trait SearchApi {
                                        $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 @@ trait SearchApi {
        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 @@ trait SearchApi {
                                $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 @@ trait SearchApi {
        }
 
        /**
-        * @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 (file)
index 0000000..fd5b3a9
--- /dev/null
@@ -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', '' );
+       }
+}