Merge "Push common search api parameters into SearchApi class"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Wed, 27 Jul 2016 09:18:33 +0000 (09:18 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 27 Jul 2016 09:18:33 +0000 (09:18 +0000)
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 39b0212..3bf6d3f 100644 (file)
@@ -105,42 +105,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 4377831..f46b5d2 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'];
@@ -308,16 +296,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',
@@ -354,52 +333,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', '' );
+       }
+}