Refactor MediaWikiServices::forceGlobalInstance to return void instead of old instance.
Motivation:
Right now code contains a lot of legacy calls without Dependency Inversion to singleton MediaWikiService instance like:
MediaWikiService::getInstance()->getService() ->doSomething();
Example:
interface IColor {
function getName(): string
}
interfact IColorer {
function getColor(string $color): ?IColor
}
class ItemHandler {
function coloredTo(string $color): void {
$colorer = MediaWikiService::getInstance()->getColorer();
$this->color = $colorer->getColor($color)
}
function getColor(): IColor { }
}To cover ItemHandler::coloredTo by pure unit test we have to mock MediaWikiService somehow.
class ItemHandlerTest ... {
function testColoredTo() {
$item = new SomeClass;
$this->assertNull($item->getColor())
$item->coloredTo('red')
$this->assertSame(
'Red'
$item->getColor()->getName()
)
}
}We can't do that now, because MediaWikiServices::forceGlobalInstance has
$old = self::getInstance();
Line which on first call initialises filly-working service with valid config.
Note. This line violated the Single Responsibility Principle and causes side-effects.
To fix this problem we
- can re-engineer
MediaWikiServices::forceGlobalInstance($): MediaWikiServices
to
MediaWikiServices::forceGlobalInstance($): void
remove the line:
$old = self::getInstance();
And change return void;
- reengineer each call to it.
from
$oldInstance = MediaWikiServices::forceGlobalInstance($newInstance);
to
$oldInstance = MediaWikiServices::getInstance(); MediaWikiServices::forceGlobalInstance($newInstance);
Below is the draft of implementation:
class MockMediaWikiServicesRegistration {
public static function register( TestCase $testInstance ) {
$mock = new MockBuilder( $testInstance, MediaWikiServices::class );
$service = $mock
->disableOriginalConstructor()
->getMock();
MediaWikiServices::forceGlobalInstance( $service );
}
}
class MediaWikiServicesTest extends MediaWikiUnitTestCase {
public function setUp() {
parent::setUp();
MockMediaWikiServicesRegistration::register($this);
}
public function tearDown() {
parent::tearDown();
MockMediaWikiServicesRegistration::destroy($);
}
public function testGetInstance() {
MediaWikiServices::getInstance();
}
}Then tests get into:
class ItemTest extends MediaWikiUnitTestCase {
public function setUp() {
parent::setUp();
MockMediaWikiServicesRegistration::register( $this );
}
public function tearDown() {
parent::tearDown();
MockMediaWikiServices::destroy();
}
function testColoredTo() {
$colorer = $this->createMock( IColorer::class )
$colorer->method( 'getName' )->willReturn( 'mock color' );
$this->registerColorer($colorer);
$item = new SomeClass;
$this->assertNull($item->getColor())
$item->coloredTo('red')
$this->assertSame(
'mock color'
$item->getColor()->getName()
)
}
private function registerColorer(IColorer $colorer) {
/**
* @var MediaWikiServices|MockObject $service
*/
$service = MediaWikiServices::getInstance();
$service
->method( 'getColorer' )
->willReturn( $colorer );
}
}