Allow factory functions for creating API modules.
authordaniel <daniel.kinzler@wikimedia.de>
Thu, 24 Jul 2014 22:37:52 +0000 (00:37 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Wed, 6 Aug 2014 20:58:14 +0000 (22:58 +0200)
This enables factory functions to be registered for API modules,
in addition to the module class itself. This allows modules to
use proper dependency injection via the modules constructor.

Example:

  $wgAPIModules['foo'] = array(
    'class' => 'ApiFoo',
    'factory' => function( $main, $action ) { ... }
  )

Change-Id: Ieb85493a7765f466317f5fa74b0b0e262220deab

RELEASE-NOTES-1.24
includes/DefaultSettings.php
includes/api/ApiModuleManager.php
tests/phpunit/includes/api/ApiModuleManagerTest.php [new file with mode: 0644]
tests/phpunit/includes/api/PrefixUniquenessTest.php

index ec4bc0f..9643238 100644 (file)
@@ -196,6 +196,14 @@ production.
 * (bug 60734) Actions that use ApiPageSet (e.g. purge, watch,
   setnotificationtimestamp) will now include continuation information when
   using a generator.
+* $wgAPIModules (and the related $wgAPIFormatModules, $wgAPIMetaModules,
+  $wgAPIPropModules, and $wgAPIListModules settings) now allow API modules
+  to be specified using a "module spec" array instead of a plain class name.
+  A "module spec" is an associative array containing at least the 'class' key
+  for the module's class, and optionally a 'factory' key for the factory function
+  to use for the module. This is intended for extensions that want control over
+  the instantiation of their API modules, to allow for proper dependency
+  injection.
 
 === Languages updated in 1.24 ===
 
index cf6a95d..e36ac1d 100644 (file)
@@ -6841,16 +6841,45 @@ $wgDebugAPI = false;
 
 /**
  * API module extensions.
- * Associative array mapping module name to class name.
- * Extension modules may override the core modules.
  *
+ * Associative array mapping module name to modules specs;
+ * Each module spec is an associative array containing at least
+ * the 'class' key for the module's class, and optionally a
+ * 'factory' key for the factory function to use for the module.
+ *
+ * That factory function will be called with two parameters,
+ * the parent module (an instance of ApiBase, usually ApiMain)
+ * and the name the module was registered under. The return
+ * value must be an instance of the class given in the 'class'
+ * field.
+ *
+ * For backward compatibility, the module spec may also be a
+ * simple string containing the module's class name. In that
+ * case, the class' constructor will be called with the parent
+ * module and module name as parameters, as described above.
+ *
+ * Examples for registering API modules:
+ *
+ * @code
+ *  $wgAPIModules['foo'] = 'ApiFoo';
+ *  $wgAPIModules['bar'] = array(
+ *    'class' => 'ApiBar',
+ *    'factory' => function( $main, $name ) { ... }
+ *  );
+ *  $wgAPIModules['xyzzy'] = array(
+ *    'class' => 'ApiXyzzy',
+ *    'factory' => array( 'XyzzyFactory', 'newApiModule' )
+ *  );
+ * @endcode
+ *
+ * Extension modules may override the core modules.
  * See ApiMain::$Modules for a list of the core modules.
  */
 $wgAPIModules = array();
 
 /**
  * API format module extensions.
- * Associative array mapping format module name to class name.
+ * Associative array mapping format module name to module specs (see $wgAPIModules).
  * Extension modules may override the core modules.
  *
  * See ApiMain::$Formats for a list of the core format modules.
@@ -6859,7 +6888,7 @@ $wgAPIFormatModules = array();
 
 /**
  * API Query meta module extensions.
- * Associative array mapping meta module name to class name.
+ * Associative array mapping meta module name to module specs (see $wgAPIModules).
  * Extension modules may override the core modules.
  *
  * See ApiQuery::$QueryMetaModules for a list of the core meta modules.
@@ -6868,7 +6897,7 @@ $wgAPIMetaModules = array();
 
 /**
  * API Query prop module extensions.
- * Associative array mapping properties module name to class name.
+ * Associative array mapping prop module name to module specs (see $wgAPIModules).
  * Extension modules may override the core modules.
  *
  * See ApiQuery::$QueryPropModules for a list of the core prop modules.
@@ -6877,7 +6906,7 @@ $wgAPIPropModules = array();
 
 /**
  * API Query list module extensions.
- * Associative array mapping list module name to class name.
+ * Associative array mapping list module name to module specs (see $wgAPIModules).
  * Extension modules may override the core modules.
  *
  * See ApiQuery::$QueryListModules for a list of the core list modules.
index 8226529..fdf3f02 100644 (file)
@@ -59,13 +59,55 @@ class ApiModuleManager extends ContextSource {
        }
 
        /**
-        * Add a list of modules to the manager
-        * @param array $modules A map of ModuleName => ModuleClass
+        * Add a list of modules to the manager. Each module is described
+        * by a module spec.
+        *
+        * Each module spec is an associative array containing at least
+        * the 'class' key for the module's class, and optionally a
+        * 'factory' key for the factory function to use for the module.
+        *
+        * That factory function will be called with two parameters,
+        * the parent module (an instance of ApiBase, usually ApiMain)
+        * and the name the module was registered under. The return
+        * value must be an instance of the class given in the 'class'
+        * field.
+        *
+        * For backward compatibility, the module spec may also be a
+        * simple string containing the module's class name. In that
+        * case, the class' constructor will be called with the parent
+        * module and module name as parameters, as described above.
+        *
+        * Examples for defining module specs:
+        *
+        * @code
+        *  $modules['foo'] = 'ApiFoo';
+        *  $modules['bar'] = array(
+        *      'class' => 'ApiBar',
+        *      'factory' => function( $main, $name ) { ... }
+        *  );
+        *  $modules['xyzzy'] = array(
+        *      'class' => 'ApiXyzzy',
+        *      'factory' => array( 'XyzzyFactory', 'newApiModule' )
+        *  );
+        * @endcode
+        *
+        * @param array $modules A map of ModuleName => ModuleSpec; The ModuleSpec
+        *        is either a string containing the module's class name, or an associative
+        *        array (see above for details).
         * @param string $group Which group modules belong to (action,format,...)
         */
        public function addModules( array $modules, $group ) {
-               foreach ( $modules as $name => $class ) {
-                       $this->addModule( $name, $group, $class );
+
+               foreach ( $modules as $name => $moduleSpec ) {
+                       if ( is_array( $moduleSpec ) ) {
+                               $class = $moduleSpec['class'];
+                               $factory = ( isset( $moduleSpec['factory'] ) ? $moduleSpec['factory'] : null );
+                       } else {
+                               $class = $moduleSpec;
+                               $factory = null;
+                       }
+
+                       $this->addModule( $name, $group, $class, $factory );
                }
        }
 
@@ -74,37 +116,61 @@ class ApiModuleManager extends ContextSource {
         * classes who wish to add their own modules to their lexicon or override the
         * behavior of inherent ones.
         *
-        * @param string $group Name of the module group
         * @param string $name The identifier for this module.
+        * @param string $group Name of the module group
         * @param string $class The class where this module is implemented.
+        * @param callable|null $factory Callback for instantiating the module.
+        *
+        * @throws InvalidArgumentException
         */
-       public function addModule( $name, $group, $class ) {
+       public function addModule( $name, $group, $class, $factory = null ) {
+               if ( !is_string( $name ) ) {
+                       throw new InvalidArgumentException( '$name must be a string' );
+               }
+
+               if ( !is_string( $group ) ) {
+                       throw new InvalidArgumentException( '$group must be a string' );
+               }
+
+               if ( !is_string( $class ) ) {
+                       throw new InvalidArgumentException( '$class must be a string' );
+               }
+
+               if ( $factory !== null && !is_callable( $factory ) ) {
+                       throw new InvalidArgumentException( '$factory must be a callable (or null)' );
+               }
+
                $this->mGroups[$group] = null;
-               $this->mModules[$name] = array( $group, $class );
+               $this->mModules[$name] = array( $group, $class, $factory );
        }
 
        /**
         * Get module instance by name, or instantiate it if it does not exist
+        *
         * @param string $moduleName Module name
         * @param string $group Optionally validate that the module is in a specific group
         * @param bool $ignoreCache If true, force-creates a new instance and does not cache it
-        * @return mixed The new module instance, or null if failed
+        *
+        * @return ApiBase|null The new module instance, or null if failed
         */
        public function getModule( $moduleName, $group = null, $ignoreCache = false ) {
                if ( !isset( $this->mModules[$moduleName] ) ) {
                        return null;
                }
-               $grpCls = $this->mModules[$moduleName];
-               if ( $group !== null && $grpCls[0] !== $group ) {
+
+               list( $moduleGroup, $moduleClass, $moduleFactory ) = $this->mModules[$moduleName];
+
+               if ( $group !== null && $moduleGroup !== $group ) {
                        return null;
                }
+
                if ( !$ignoreCache && isset( $this->mInstances[$moduleName] ) ) {
                        // already exists
                        return $this->mInstances[$moduleName];
                } else {
                        // new instance
-                       $class = $grpCls[1];
-                       $instance = new $class ( $this->mParent, $moduleName );
+                       $instance = $this->instantiateModule( $moduleName, $moduleClass, $moduleFactory );
+
                        if ( !$ignoreCache ) {
                                // cache this instance in case it is needed later
                                $this->mInstances[$moduleName] = $instance;
@@ -114,6 +180,32 @@ class ApiModuleManager extends ContextSource {
                }
        }
 
+       /**
+        * Instantiate the module using the given class or factory function.
+        *
+        * @param string $name The identifier for this module.
+        * @param string $class The class where this module is implemented.
+        * @param callable|null $factory Callback for instantiating the module.
+        *
+        * @throws MWException
+        * @return ApiBase
+        */
+       private function instantiateModule( $name, $class, $factory = null ) {
+               if ( $factory !== null ) {
+                       // create instance from factory
+                       $instance = call_user_func( $factory, $this->mParent, $name );
+
+                       if ( ! $instance instanceof $class ) {
+                               throw new MWException( "The factory function for module $name did not return an instance of $class!" );
+                       }
+               } else {
+                       // create instance from class name
+                       $instance = new $class( $this->mParent, $name );
+               }
+
+               return $instance;
+       }
+
        /**
         * Get an array of modules in a specific group or all if no group is set.
         * @param string $group Optional group filter
diff --git a/tests/phpunit/includes/api/ApiModuleManagerTest.php b/tests/phpunit/includes/api/ApiModuleManagerTest.php
new file mode 100644 (file)
index 0000000..1ac9b9d
--- /dev/null
@@ -0,0 +1,274 @@
+<?php
+
+/**
+ * @covers ApiModuleManager
+ *
+ * @group API
+ * @group Database
+ * @group medium
+ */
+class ApiModuleManagerTest extends MediaWikiTestCase {
+
+       private function getModuleManager() {
+               $request = new FauxRequest();
+               $main = new ApiMain( $request );
+               return new ApiModuleManager( $main );
+       }
+
+       public function newApiLogin( $main, $action ) {
+               return new ApiLogin( $main, $action );
+       }
+
+       public function addModuleProvider() {
+               return array(
+                       'plain class' => array(
+                               'login',
+                               'action',
+                               'ApiLogin',
+                               null,
+                       ),
+
+                       'with factory' => array(
+                               'login',
+                               'action',
+                               'ApiLogin',
+                               array( $this, 'newApiLogin' ),
+                       ),
+
+                       'with closure' => array(
+                               'logout',
+                               'action',
+                               'ApiLogout',
+                               function( ApiMain $main, $action ) {
+                                               return new ApiLogout( $main, $action );
+                               },
+                       ),
+               );
+       }
+
+       /**
+        * @dataProvider addModuleProvider
+        */
+       public function testAddModule( $name, $group, $class, $factory = null ) {
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModule( $name, $group, $class, $factory );
+
+               $this->assertTrue( $moduleManager->isDefined( $name, $group ), 'isDefined' );
+               $this->assertNotNull( $moduleManager->getModule( $name, $group, true ), 'getModule' );
+       }
+
+       public function addModulesProvider() {
+               return array(
+                       'empty' => array(
+                               array(),
+                               'action',
+                       ),
+
+                       'simple' => array(
+                               array(
+                                       'login' => 'ApiLogin',
+                                       'logout' => 'ApiLogout',
+                               ),
+                               'action',
+                       ),
+
+                       'with factories' => array(
+                               array(
+                                       'login' => array(
+                                               'class' => 'ApiLogin',
+                                               'factory' => array( $this, 'newApiLogin' ),
+                                       ),
+                                       'logout' => array(
+                                               'class' => 'ApiLogout',
+                                               'factory' => function( ApiMain $main, $action ) {
+                                                       return new ApiLogout( $main, $action );
+                                               },
+                                       ),
+                               ),
+                               'action',
+                       ),
+               );
+       }
+
+       /**
+        * @dataProvider addModulesProvider
+        */
+       public function testAddModules( array $modules, $group ) {
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $modules, $group );
+
+               foreach ( array_keys( $modules ) as $name ) {
+                       $this->assertTrue( $moduleManager->isDefined( $name, $group ), 'isDefined' );
+                       $this->assertNotNull( $moduleManager->getModule( $name, $group, true ), 'getModule' );
+               }
+       }
+
+       public function getModuleProvider() {
+               $modules = array(
+                       'feedrecentchanges' => 'ApiFeedRecentChanges',
+                       'feedcontributions' => array( 'class' => 'ApiFeedContributions' ),
+                       'login' => array(
+                               'class' => 'ApiLogin',
+                               'factory' => array( $this, 'newApiLogin' ),
+                       ),
+                       'logout' => array(
+                               'class' => 'ApiLogout',
+                               'factory' => function( ApiMain $main, $action ) {
+                                               return new ApiLogout( $main, $action );
+                                       },
+                       ),
+               );
+
+               return array(
+                       'legacy entry' => array(
+                               $modules,
+                               'feedrecentchanges',
+                               'ApiFeedRecentChanges',
+                       ),
+
+                       'just a class' => array(
+                               $modules,
+                               'feedcontributions',
+                               'ApiFeedContributions',
+                       ),
+
+                       'with factory' => array(
+                               $modules,
+                               'login',
+                               'ApiLogin',
+                       ),
+
+                       'with closure' => array(
+                               $modules,
+                               'logout',
+                               'ApiLogout',
+                       ),
+               );
+       }
+
+       /**
+        * @dataProvider getModuleProvider
+        */
+       public function testGetModule( $modules, $name, $expectedClass ) {
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $modules, 'test' );
+
+               // should return the right module
+               $module1 = $moduleManager->getModule( $name, null, false );
+               $this->assertInstanceOf( $expectedClass, $module1 );
+
+               // should pass group check (with caching disabled)
+               $module2 = $moduleManager->getModule( $name, 'test', true );
+               $this->assertNotNull( $module2 );
+
+               // should use cached instance
+               $module3 = $moduleManager->getModule( $name, null, false );
+               $this->assertSame( $module1, $module3 );
+
+               // should not use cached instance if caching is disabled
+               $module4 = $moduleManager->getModule( $name, null, true );
+               $this->assertNotSame( $module1, $module4 );
+       }
+
+       public function testGetModule_null() {
+               $modules = array(
+                       'login' => 'ApiLogin',
+                       'logout' => 'ApiLogout',
+               );
+
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $modules, 'test' );
+
+               $this->assertNull( $moduleManager->getModule( 'quux' ), 'unknown name' );
+               $this->assertNull( $moduleManager->getModule( 'login', 'bla' ), 'wrong group' );
+       }
+
+       public function testGetNames() {
+               $fooModules = array(
+                       'login' => 'ApiLogin',
+                       'logout' => 'ApiLogout',
+               );
+
+               $barModules = array(
+                       'feedcontributions' => array( 'class' => 'ApiFeedContributions' ),
+                       'feedrecentchanges' => array( 'class' => 'ApiFeedRecentChanges' ),
+               );
+
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $fooModules, 'foo' );
+               $moduleManager->addModules( $barModules, 'bar' );
+
+               $fooNames = $moduleManager->getNames( 'foo' );
+               $this->assertArrayEquals( array_keys( $fooModules ), $fooNames );
+
+               $allNames = $moduleManager->getNames();
+               $allModules = array_merge( $fooModules, $barModules );
+               $this->assertArrayEquals( array_keys( $allModules ), $allNames );
+       }
+
+       public function testGetNamesWithClasses() {
+               $fooModules = array(
+                       'login' => 'ApiLogin',
+                       'logout' => 'ApiLogout',
+               );
+
+               $barModules = array(
+                       'feedcontributions' => array( 'class' => 'ApiFeedContributions' ),
+                       'feedrecentchanges' => array( 'class' => 'ApiFeedRecentChanges' ),
+               );
+
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $fooModules, 'foo' );
+               $moduleManager->addModules( $barModules, 'bar' );
+
+               $fooNamesWithClasses = $moduleManager->getNamesWithClasses( 'foo' );
+               $this->assertArrayEquals( $fooModules, $fooNamesWithClasses );
+
+               $allNamesWithClasses = $moduleManager->getNamesWithClasses();
+               $allModules = array_merge( $fooModules, array(
+                       'feedcontributions' => 'ApiFeedContributions',
+                       'feedrecentchanges' => 'ApiFeedRecentChanges',
+               ) );
+               $this->assertArrayEquals( $allModules, $allNamesWithClasses );
+       }
+
+       public function testGetModuleGroup() {
+               $fooModules = array(
+                       'login' => 'ApiLogin',
+                       'logout' => 'ApiLogout',
+               );
+
+               $barModules = array(
+                       'feedcontributions' => array( 'class' => 'ApiFeedContributions' ),
+                       'feedrecentchanges' => array( 'class' => 'ApiFeedRecentChanges' ),
+               );
+
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $fooModules, 'foo' );
+               $moduleManager->addModules( $barModules, 'bar' );
+
+               $this->assertEquals( 'foo', $moduleManager->getModuleGroup( 'login' ) );
+               $this->assertEquals( 'bar', $moduleManager->getModuleGroup( 'feedrecentchanges' ) );
+               $this->assertNull( $moduleManager->getModuleGroup( 'quux' ) );
+       }
+
+       public function testGetGroups() {
+               $fooModules = array(
+                       'login' => 'ApiLogin',
+                       'logout' => 'ApiLogout',
+               );
+
+               $barModules = array(
+                       'feedcontributions' => array( 'class' => 'ApiFeedContributions' ),
+                       'feedrecentchanges' => array( 'class' => 'ApiFeedRecentChanges' ),
+               );
+
+               $moduleManager = $this->getModuleManager();
+               $moduleManager->addModules( $fooModules, 'foo' );
+               $moduleManager->addModules( $barModules, 'bar' );
+
+               $groups = $moduleManager->getGroups();
+               $this->assertArrayEquals( array( 'foo', 'bar' ), $groups );
+       }
+
+}
index 38beb87..13da33c 100644 (file)
@@ -10,12 +10,15 @@ class PrefixUniquenessTest extends MediaWikiTestCase {
        public function testPrefixes() {
                $main = new ApiMain( new FauxRequest() );
                $query = new ApiQuery( $main, 'foo', 'bar' );
-               $modules = $query->getModuleManager()->getNamesWithClasses();
+               $moduleManager = $query->getModuleManager();
+
+               $modules = $moduleManager->getNames();
                $prefixes = array();
 
-               foreach ( $modules as $name => $class ) {
-                       /** @var ApiQueryBase $module */
-                       $module = new $class( $query, $name );
+               foreach ( $modules as $name ) {
+                       $module = $moduleManager->getModule( $name );
+                       $class = get_class( $module );
+
                        $prefix = $module->getModulePrefix();
                        if ( isset( $prefixes[$prefix] ) ) {
                                $this->fail( "Module prefix '{$prefix}' is shared between {$class} and {$prefixes[$prefix]}" );