From: Catrope Date: Mon, 11 Apr 2016 20:25:47 +0000 (+0000) Subject: Revert "Make WatchedItemStore use MediaWikiServices" X-Git-Tag: 1.31.0-rc.0~7324^2 X-Git-Url: http://git.cyclocoop.org/%24image?a=commitdiff_plain;h=deb22bced75964dc1d5b8783ab0f6a7e66ee945a;p=lhc%2Fweb%2Fwiklou.git Revert "Make WatchedItemStore use MediaWikiServices" In order to cleanly revert "Allow reset of global services" which breaks login. This reverts commit 99e8d45b50993113b26ced7c8bb91e44c7d908b1. Change-Id: Iae7f7b7b7083345dd50c313ee68e2e814f130f1a --- diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index dd66967d92..f5ae2d517c 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -34,8 +34,6 @@ use SiteStore; use SpecialPageFactory; use Title; use User; -use WatchedItemStore; -use Wikimedia\Assert\Assert; /** * Service locator for MediaWiki core services. @@ -450,13 +448,6 @@ class MediaWikiServices extends ServiceContainer { return $this->getService( 'DBLoadBalancer' ); } - /** - * @return WatchedItemStore - */ - public function getWatchedItemStore() { - return $this->getService( 'WatchedItemStore' ); - } - /////////////////////////////////////////////////////////////////////////// // NOTE: When adding a service getter here, don't forget to add a test // case for it in MediaWikiServicesTest::provideGetters() and in diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index dc2c66b6b8..5ec1d64c7b 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -92,15 +92,6 @@ return [ return $services->getConfigFactory()->makeConfig( 'main' ); }, - 'WatchedItemStore' => function( MediaWikiServices $services ) { - $store = new WatchedItemStore( - $services->getDBLoadBalancer(), - new HashBagOStuff( [ 'maxKeys' => 100 ] ) - ); - $store->setStatsdDataFactory( RequestContext::getMain()->getStats() ); - return $store; - } - /////////////////////////////////////////////////////////////////////////// // NOTE: When adding a service here, don't forget to add a getter function // in the MediaWikiServices class. The convenience getter should just call diff --git a/includes/WatchedItemStore.php b/includes/WatchedItemStore.php index 603bacd1cd..8ae7932be0 100644 --- a/includes/WatchedItemStore.php +++ b/includes/WatchedItemStore.php @@ -1,7 +1,6 @@ getWatchedItemStore() + * Overrides the default instance of this class + * This is intended for use while testing and will fail if MW_PHPUNIT_TEST is not defined. + * + * If this method is used it MUST also be called with null after a test to ensure a new + * default instance is created next time getDefaultInstance is called. + * + * @param WatchedItemStore|null $store + * + * @return ScopedCallback to reset the overridden value + * @throws MWException + */ + public static function overrideDefaultInstance( WatchedItemStore $store = null ) { + if ( !defined( 'MW_PHPUNIT_TEST' ) ) { + throw new MWException( + 'Cannot override ' . __CLASS__ . 'default instance in operation.' + ); + } + + $previousValue = self::$instance; + self::$instance = $store; + return new ScopedCallback( function() use ( $previousValue ) { + self::$instance = $previousValue; + } ); + } + + /** * @return self */ public static function getDefaultInstance() { - return MediaWikiServices::getInstance()->getWatchedItemStore(); + if ( !self::$instance ) { + self::$instance = new self( + wfGetLB(), + new HashBagOStuff( [ 'maxKeys' => 100 ] ) + ); + self::$instance->setStatsdDataFactory( RequestContext::getMain()->getStats() ); + } + return self::$instance; } private function getCacheKey( User $user, LinkTarget $target ) { diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index 96abf97e41..53cd0b12cf 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -230,7 +230,6 @@ class MediaWikiServicesTest extends PHPUnit_Framework_TestCase { 'SiteLookup' => [ 'SiteLookup', SiteLookup::class ], 'DBLoadBalancerFactory' => [ 'DBLoadBalancerFactory', 'LBFactory' ], 'DBLoadBalancer' => [ 'DBLoadBalancer', 'LoadBalancer' ], - 'WatchedItemStore' => [ 'WatchedItemStore', WatchedItemStore::class ], ]; } diff --git a/tests/phpunit/includes/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/WatchedItemStoreUnitTest.php index 78231c2418..9dd38df069 100644 --- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php @@ -1,11 +1,12 @@ assertSame( $instanceOne, $instanceTwo ); } + public function testOverrideDefaultInstance() { + $instance = WatchedItemStore::getDefaultInstance(); + $scopedOverride = $instance->overrideDefaultInstance( null ); + + $this->assertNotSame( $instance, WatchedItemStore::getDefaultInstance() ); + + unset( $scopedOverride ); + + $this->assertSame( $instance, WatchedItemStore::getDefaultInstance() ); + } + public function testCountWatchedItems() { $user = $this->getMockNonAnonUserWithId( 1 ); diff --git a/tests/phpunit/includes/WatchedItemUnitTest.php b/tests/phpunit/includes/WatchedItemUnitTest.php index db7f16c655..b4eaa765e9 100644 --- a/tests/phpunit/includes/WatchedItemUnitTest.php +++ b/tests/phpunit/includes/WatchedItemUnitTest.php @@ -5,30 +5,13 @@ * * @covers WatchedItem */ -class WatchedItemUnitTest extends MediaWikiTestCase { - - /** - * @param int $id - * - * @return PHPUnit_Framework_MockObject_MockObject|User - */ - private function getMockUser( $id ) { - $user = $this->getMock( User::class ); - $user->expects( $this->any() ) - ->method( 'getId' ) - ->will( $this->returnValue( $id ) ); - $user->expects( $this->any() ) - ->method( 'isAllowed' ) - ->will( $this->returnValue( true ) ); - return $user; - } +class WatchedItemUnitTest extends PHPUnit_Framework_TestCase { public function provideUserTitleTimestamp() { - $user = $this->getMockUser( 111 ); return [ - [ $user, Title::newFromText( 'SomeTitle' ), null ], - [ $user, Title::newFromText( 'SomeTitle' ), '20150101010101' ], - [ $user, new TitleValue( 0, 'TVTitle', 'frag' ), '20150101010101' ], + [ User::newFromId( 111 ), Title::newFromText( 'SomeTitle' ), null ], + [ User::newFromId( 111 ), Title::newFromText( 'SomeTitle' ), '20150101010101' ], + [ User::newFromId( 111 ), new TitleValue( 0, 'TVTitle', 'frag' ), '20150101010101' ], ]; } @@ -68,13 +51,15 @@ class WatchedItemUnitTest extends MediaWikiTestCase { ->method( 'loadWatchedItem' ) ->with( $user, $linkTarget ) ->will( $this->returnValue( new WatchedItem( $user, $linkTarget, $timestamp ) ) ); - $this->setService( 'WatchedItemStore', $store ); + $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); $item = WatchedItem::fromUserTitle( $user, $linkTarget, User::IGNORE_USER_RIGHTS ); $this->assertEquals( $user, $item->getUser() ); $this->assertEquals( $linkTarget, $item->getLinkTarget() ); $this->assertEquals( $timestamp, $item->getNotificationTimestamp() ); + + ScopedCallback::consume( $scopedOverride ); } /** @@ -100,10 +85,12 @@ class WatchedItemUnitTest extends MediaWikiTestCase { return true; } ) ); - $this->setService( 'WatchedItemStore', $store ); + $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); $item = new WatchedItem( $user, $linkTarget, $timestamp ); $item->resetNotificationTimestamp( $force, $oldid ); + + ScopedCallback::consume( $scopedOverride ); } public function testAddWatch() { @@ -170,15 +157,17 @@ class WatchedItemUnitTest extends MediaWikiTestCase { $store->expects( $this->once() ) ->method( 'duplicateAllAssociatedEntries' ) ->with( $oldTitle, $newTitle ); - $this->setService( 'WatchedItemStore', $store ); + $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); WatchedItem::duplicateEntries( $oldTitle, $newTitle ); + + ScopedCallback::consume( $scopedOverride ); } public function testBatchAddWatch() { - $itemOne = new WatchedItem( $this->getMockUser( 1 ), new TitleValue( 0, 'Title1' ), null ); + $itemOne = new WatchedItem( User::newFromId( 1 ), new TitleValue( 0, 'Title1' ), null ); $itemTwo = new WatchedItem( - $this->getMockUser( 3 ), + User::newFromId( 3 ), Title::newFromText( 'Title2' ), '20150101010101' ); @@ -204,9 +193,11 @@ class WatchedItemUnitTest extends MediaWikiTestCase { $itemTwo->getTitle()->getTalkPage(), ] ); - $this->setService( 'WatchedItemStore', $store ); + $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); WatchedItem::batchAddWatch( [ $itemOne, $itemTwo ] ); + + ScopedCallback::consume( $scopedOverride ); } }