(bug 39665) optimize API query generator list
authorReedy <reedy@wikimedia.org>
Thu, 6 Sep 2012 18:36:53 +0000 (19:36 +0100)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 12 Oct 2012 20:24:15 +0000 (20:24 +0000)
List of query generators is now not built using reflection, instead it
is defined in code. Per Domas, make this a hard coded list instead of
loading all the child classes.

Added $wgAPIGeneratorModules for people to register their API generator
modules.

Change-Id: I12da92da33527e414c9b125a50b82c9bdbb3ed99

RELEASE-NOTES-1.20
includes/DefaultSettings.php
includes/api/ApiQuery.php
tests/phpunit/includes/api/ApiGeneratorTest.php [new file with mode: 0644]

index 926cbcf..52c4e86 100644 (file)
@@ -298,8 +298,8 @@ upgrade PHP if you have not done so prior to upgrading MediaWiki.
 * (bug 38904) prop=revisions&rvstart=... no longer blows up when continuing.
 * (bug 39032) ApiQuery generates help in constructor.
 * (bug 11142) Improve file extension blacklist error reporting in API upload.
-* (bug 39665) Cache AllowedGenerator array so it doesn't autoload all query classes
-  on every request.
+* (bug 39665) List of query generators is now not built using reflection, instead it is
+  defined in code.
 
 === Languages updated in 1.20 ===
 
index 159ed29..10b7324 100644 (file)
@@ -5932,6 +5932,7 @@ $wgAPIModules = array();
 $wgAPIMetaModules = array();
 $wgAPIPropModules = array();
 $wgAPIListModules = array();
+$wgAPIGeneratorModules = array();
 
 /**
  * Maximum amount of rows to scan in a DB query in the API
index 64399b2..dff7524 100644 (file)
@@ -46,6 +46,10 @@ class ApiQuery extends ApiBase {
 
        private $params, $redirects, $convertTitles, $iwUrl;
 
+       /**
+        * List of Api Query prop modules
+        * @var array
+        */
        private $mQueryPropModules = array(
                'categories' => 'ApiQueryCategories',
                'categoryinfo' => 'ApiQueryCategoryInfo',
@@ -63,6 +67,10 @@ class ApiQuery extends ApiBase {
                'templates' => 'ApiQueryLinks',
        );
 
+       /**
+        * List of Api Query list modules
+        * @var array
+        */
        private $mQueryListModules = array(
                'allcategories' => 'ApiQueryAllCategories',
                'allimages' => 'ApiQueryAllImages',
@@ -92,16 +100,52 @@ class ApiQuery extends ApiBase {
                'watchlistraw' => 'ApiQueryWatchlistRaw',
        );
 
+       /**
+        * List of Api Query meta modules
+        * @var array
+        */
        private $mQueryMetaModules = array(
                'allmessages' => 'ApiQueryAllMessages',
                'siteinfo' => 'ApiQuerySiteinfo',
                'userinfo' => 'ApiQueryUserInfo',
        );
 
+       /**
+        * List of Api Query generator modules
+        * Defined in code, rather than being derived at runtime,
+        * due to performance reasons
+        * @var array
+        */
+       private $mQueryGenerators = array(
+               'allcategories' => 'ApiQueryAllCategories',
+               'allimages' => 'ApiQueryAllImages',
+               'alllinks' => 'ApiQueryAllLinks',
+               'allpages' => 'ApiQueryAllPages',
+               'backlinks' => 'ApiQueryBacklinks',
+               'categories' => 'ApiQueryCategories',
+               'categorymembers' => 'ApiQueryCategoryMembers',
+               'duplicatefiles' => 'ApiQueryDuplicateFiles',
+               'embeddedin' => 'ApiQueryBacklinks',
+               'exturlusage' => 'ApiQueryExtLinksUsage',
+               'images' => 'ApiQueryImages',
+               'imageusage' => 'ApiQueryBacklinks',
+               'iwbacklinks' => 'ApiQueryIWBacklinks',
+               'langbacklinks' => 'ApiQueryLangBacklinks',
+               'links' => 'ApiQueryLinks',
+               'protectedtitles' => 'ApiQueryProtectedTitles',
+               'querypage' => 'ApiQueryQueryPage',
+               'random' => 'ApiQueryRandom',
+               'recentchanges' => 'ApiQueryRecentChanges',
+               'search' => 'ApiQuerySearch',
+               'templates' => 'ApiQueryLinks',
+               'watchlist' => 'ApiQueryWatchlist',
+               'watchlistraw' => 'ApiQueryWatchlistRaw',
+       );
+
        private $mSlaveDB = null;
        private $mNamedDB = array();
 
-       protected $mAllowedGenerators = array();
+       protected $mAllowedGenerators;
 
        /**
         * @param $main ApiMain
@@ -111,32 +155,16 @@ class ApiQuery extends ApiBase {
                parent::__construct( $main, $action );
 
                // Allow custom modules to be added in LocalSettings.php
-               global $wgAPIPropModules, $wgAPIListModules, $wgAPIMetaModules,
-                       $wgMemc, $wgAPICacheHelpTimeout;
+               global $wgAPIPropModules, $wgAPIListModules, $wgAPIMetaModules, $wgAPIGeneratorModules;
                self::appendUserModules( $this->mQueryPropModules, $wgAPIPropModules );
                self::appendUserModules( $this->mQueryListModules, $wgAPIListModules );
                self::appendUserModules( $this->mQueryMetaModules, $wgAPIMetaModules );
+               self::appendUserModules( $this->mQueryGenerators, $wgAPIGeneratorModules );
 
                $this->mPropModuleNames = array_keys( $this->mQueryPropModules );
                $this->mListModuleNames = array_keys( $this->mQueryListModules );
                $this->mMetaModuleNames = array_keys( $this->mQueryMetaModules );
-
-               // Get array of query generators from cache if present
-               $key = wfMemcKey( 'apiquerygenerators', SpecialVersion::getVersion( 'nodb' ) );
-
-               if ( $wgAPICacheHelpTimeout > 0 ) {
-                       $cached = $wgMemc->get( $key );
-                       if ( $cached ) {
-                               $this->mAllowedGenerators = $cached;
-                               return;
-                       }
-               }
-               $this->makeGeneratorList( $this->mQueryPropModules );
-               $this->makeGeneratorList( $this->mQueryListModules );
-
-               if ( $wgAPICacheHelpTimeout > 0 ) {
-                       $wgMemc->set( $key, $this->mAllowedGenerators, $wgAPICacheHelpTimeout );
-               }
+               $this->mAllowedGenerators = array_keys( $this->mQueryGenerators );
        }
 
        /**
@@ -196,10 +224,18 @@ class ApiQuery extends ApiBase {
         * Get the array mapping module names to class names
         * @return array array(modulename => classname)
         */
-       function getModules() {
+       public function getModules() {
                return array_merge( $this->mQueryPropModules, $this->mQueryListModules, $this->mQueryMetaModules );
        }
 
+       /**
+        * Get the generators array mapping module names to class names
+        * @return array array(modulename => classname)
+        */
+       public function getGenerators() {
+               return $this->mQueryGenerators;
+       }
+
        /**
         * Get whether the specified module is a prop, list or a meta query module
         * @param $moduleName string Name of the module to find type for
@@ -680,19 +716,6 @@ class ApiQuery extends ApiBase {
                return implode( "\n", $moduleDescriptions );
        }
 
-       /**
-        * Adds any classes that are a subclass of ApiQueryGeneratorBase
-        * to the allowed generator list
-        * @param $moduleList array()
-        */
-       private function makeGeneratorList( $moduleList ) {
-               foreach( $moduleList as  $moduleName => $moduleClass ) {
-                       if ( is_subclass_of( $moduleClass, 'ApiQueryGeneratorBase'  ) ) {
-                               $this->mAllowedGenerators[] = $moduleName;
-                       }
-               }
-       }
-
        /**
         * Override to add extra parameters from PageSet
         * @return string
diff --git a/tests/phpunit/includes/api/ApiGeneratorTest.php b/tests/phpunit/includes/api/ApiGeneratorTest.php
new file mode 100644 (file)
index 0000000..60ae608
--- /dev/null
@@ -0,0 +1,76 @@
+<?php
+class ApiGeneratorTest extends MediaWikiTestCase {
+
+       /**
+        * Helper to easily get an ApiQuery object instance
+        */
+       function getApiQuery() {
+               // Initialize an ApiQuery object to play with
+               $main = new ApiMain( new FauxRequest() );
+               return new ApiQuery( $main, 'foo', 'bar' );
+       }
+
+       /**
+        * Test whether all registered query modules which are subclasses of
+        * ApiQueryGeneratorBase are listed as being a generator. Registration is
+        * done:
+        *  - for core: add it to ApiQuery::$mQueryGenerators
+        *  - for extension: by adding to $wgAPIGeneratorModules
+        *
+        * @dataProvider provideApiquerygeneratorbaseChilds
+        */
+       public function testApiquerygeneatorbaseModulesListedAsGenerators(
+               $moduleName, $moduleClass
+       ) {
+               $generators = $this->getApiQuery()->getGenerators();
+               $this->assertArrayHasKey( $moduleName, $generators,
+                       "API module '$moduleName' of class '$moduleClass' (an ApiQueryGeneratorBase subclass) must be listed in ApiQuery::\$mQueryGenerators or added to \$wgAPIGeneratorModules."
+               );
+       }
+
+       /**
+        * Returns API modules which are subclassing ApiQueryGeneratorBase.
+        * Case format is:
+        *      (moduleName, moduleClass)
+        */
+       public function provideApiquerygeneratorbaseChilds() {
+               $cases = array();
+               $modules = $this->getApiQuery()->getModules();
+               foreach( $modules as $moduleName => $moduleClass ) {
+                       if( !is_subclass_of( $moduleClass, 'ApiQueryGeneratorBase' ) ) {
+                               continue;
+                       }
+                       $cases[] = array( $moduleName, $moduleClass );
+               }
+               return $cases;
+       }
+
+       /**
+        * @dataProvider provideListedApiqueryGenerators
+        */
+       public function testGeneratorsAreApiquerygeneratorbaseSubclasses(
+               $generatorName, $generatorClass
+       ) {
+               $modules = $this->getApiQuery()->getModules();
+               $this->assertArrayHasKey( $generatorName, $modules,
+                       "Class '$generatorClass' of generator '$generatorName' must be a subclass of 'ApiQueryGeneratorBase'. Listed either in ApiQuery::\$mQueryGenerators or in \$wgAPIGeneratorModules."
+               );
+
+       }
+
+       /**
+        * Return ApiQuery generators, either listed in ApiQuery or registered
+        * via wgAPIGeneratorModules.
+        * Case format is:
+        *  (moduleName, $moduleClass).
+        */
+       public function provideListedApiqueryGenerators() {
+               $cases = array();
+               $generators = $this->getApiQuery()->getGenerators();
+               foreach( $generators as $generatorName => $generatorClass ) {
+                       $cases[] = array( $generatorName, $generatorClass );
+               }
+               return $cases;
+       }
+
+}