MediaWiki offers several extension interfaces based on registering classes to be used for a specific purpose, e.g. custom actions, special pages, api modules, etc. The problem with this approach is that the signature of the constructor has to be known to the framework, preventing us from moving away from global state towards using proper dependency injection via the constructor.
The alternative is to allow factory functions[1] to be registered instead of (or in addition to) class names. This is already supported for actions and config handlers, and I have now submitted a patch to also allow this for api modules <https://gerrit.wikimedia.org/r/#/c/149183/>. If this is accepted, I plan to do the same for special pages. Please have a look and comment. Let me give an example of why this is useful: For example, if we want to define a new api module ApiFoo which uses a DAO interface called FooLookup implemented by SqlFooLookup, we would have to use global state to get the instance of SqlFooLookup: class ApiFoo extends ApiBase { public function __construct( ApiMain $main, $name ) { parent::__construct( $main, $name ); $this->lookup = SqlFooLookup::singleton(); } ... } ... $wgAPIMOdules['foo'] = 'ApiFoo'; The API module would be bound to a global singleton, which makes testing and re-use a lot harder, and constitutes a hidden dependency. There is no way to control what implementation FooLookup is used, and the class can't operate without the SqlFooLookup singleton being there. If however we control the instantiation, we can use proper dependency injection: class ApiFoo extends ApiBase { public function __construct( FooLookup $lookup, ApiMain $main, $name ) { parent::__construct( $main, $name ); $this->lookup = $lookup; } ... } ... $wgAPIMOdules['foo'] = array( 'class' => 'ApiFoo', // This information is still needed! 'factory' => function( ApiMain $main, $name ) { $lookup = SqlFooLookup::singleton(); return new ApiFoo( $lookup, main, $name ); } ); Now, the dependency is controlled by the code that registers the API module (the bootstrap code), ApiFoo no longer knows anything about SqlFooLookup, and can easily be tested and re-used with different implementations of FooLookup. Essentially it means that we have less dependencies between implementations, and split the construction of the network of service objects from the actual logic of the individual components. Do you agree that this is a good approach? Do you see any problems with it? Perhaps we can discuss this some more at Wikimania (I assume there will be an architecture session there). Cheers, Daniel [1] We could also register factory objects instead of factory functions, following the abstract factory pattern. The main advantage of this pattern is type safety: the factory objects can be checked against an interface, while we have to just trust the factory functions to have the right signature. However, even with type hinting, PHP does not do type checks on return values, so we never know what the factory actually returns. Overall, individual factory objects seem a lot of overhead for very little benefit. See also the discussion on I5a5857fcfa075. _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
