From baa11f7430971936c9add1605fc3ed193bcfa5f4 Mon Sep 17 00:00:00 2001 From: daniel Date: Fri, 25 Jul 2014 00:37:52 +0200 Subject: [PATCH] Allow factory functions for creating API modules. 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 | 8 + includes/DefaultSettings.php | 41 ++- includes/api/ApiModuleManager.php | 116 +++++++- .../includes/api/ApiModuleManagerTest.php | 274 ++++++++++++++++++ .../includes/api/PrefixUniquenessTest.php | 11 +- 5 files changed, 428 insertions(+), 22 deletions(-) create mode 100644 tests/phpunit/includes/api/ApiModuleManagerTest.php diff --git a/RELEASE-NOTES-1.24 b/RELEASE-NOTES-1.24 index ec4bc0fa0c..96432382d1 100644 --- a/RELEASE-NOTES-1.24 +++ b/RELEASE-NOTES-1.24 @@ -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 === diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index cf6a95d0f7..e36ac1dbb7 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -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. diff --git a/includes/api/ApiModuleManager.php b/includes/api/ApiModuleManager.php index 822652977e..fdf3f02ed4 100644 --- a/includes/api/ApiModuleManager.php +++ b/includes/api/ApiModuleManager.php @@ -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 index 0000000000..1ac9b9df6c --- /dev/null +++ b/tests/phpunit/includes/api/ApiModuleManagerTest.php @@ -0,0 +1,274 @@ + 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 ); + } + +} diff --git a/tests/phpunit/includes/api/PrefixUniquenessTest.php b/tests/phpunit/includes/api/PrefixUniquenessTest.php index 38beb87208..13da33c700 100644 --- a/tests/phpunit/includes/api/PrefixUniquenessTest.php +++ b/tests/phpunit/includes/api/PrefixUniquenessTest.php @@ -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]}" ); -- 2.20.1