From 99e8d45b50993113b26ced7c8bb91e44c7d908b1 Mon Sep 17 00:00:00 2001 From: addshore Date: Wed, 6 Apr 2016 11:46:50 +0100 Subject: [PATCH] Make WatchedItemStore use MediaWikiServices Change-Id: I5c8c1a652a35e055d8d65753a60c0d1684e1c37d --- includes/MediaWikiServices.php | 9 ++++ includes/ServiceWiring.php | 9 ++++ includes/WatchedItemStore.php | 42 ++--------------- .../includes/MediaWikiServicesTest.php | 1 + .../includes/WatchedItemStoreUnitTest.php | 14 +----- .../phpunit/includes/WatchedItemUnitTest.php | 45 +++++++++++-------- 6 files changed, 50 insertions(+), 70 deletions(-) diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index f5ae2d517c..dd66967d92 100644 --- a/includes/MediaWikiServices.php +++ b/includes/MediaWikiServices.php @@ -34,6 +34,8 @@ use SiteStore; use SpecialPageFactory; use Title; use User; +use WatchedItemStore; +use Wikimedia\Assert\Assert; /** * Service locator for MediaWiki core services. @@ -448,6 +450,13 @@ 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 5ec1d64c7b..dc2c66b6b8 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -92,6 +92,15 @@ 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 8ae7932be0..603bacd1cd 100644 --- a/includes/WatchedItemStore.php +++ b/includes/WatchedItemStore.php @@ -1,6 +1,7 @@ getWatchedItemStore() * @return self */ public static function getDefaultInstance() { - if ( !self::$instance ) { - self::$instance = new self( - wfGetLB(), - new HashBagOStuff( [ 'maxKeys' => 100 ] ) - ); - self::$instance->setStatsdDataFactory( RequestContext::getMain()->getStats() ); - } - return self::$instance; + return MediaWikiServices::getInstance()->getWatchedItemStore(); } private function getCacheKey( User $user, LinkTarget $target ) { diff --git a/tests/phpunit/includes/MediaWikiServicesTest.php b/tests/phpunit/includes/MediaWikiServicesTest.php index 53cd0b12cf..96abf97e41 100644 --- a/tests/phpunit/includes/MediaWikiServicesTest.php +++ b/tests/phpunit/includes/MediaWikiServicesTest.php @@ -230,6 +230,7 @@ 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 9dd38df069..78231c2418 100644 --- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php @@ -1,12 +1,11 @@ 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 b4eaa765e9..db7f16c655 100644 --- a/tests/phpunit/includes/WatchedItemUnitTest.php +++ b/tests/phpunit/includes/WatchedItemUnitTest.php @@ -5,13 +5,30 @@ * * @covers WatchedItem */ -class WatchedItemUnitTest extends PHPUnit_Framework_TestCase { +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; + } public function provideUserTitleTimestamp() { + $user = $this->getMockUser( 111 ); return [ - [ User::newFromId( 111 ), Title::newFromText( 'SomeTitle' ), null ], - [ User::newFromId( 111 ), Title::newFromText( 'SomeTitle' ), '20150101010101' ], - [ User::newFromId( 111 ), new TitleValue( 0, 'TVTitle', 'frag' ), '20150101010101' ], + [ $user, Title::newFromText( 'SomeTitle' ), null ], + [ $user, Title::newFromText( 'SomeTitle' ), '20150101010101' ], + [ $user, new TitleValue( 0, 'TVTitle', 'frag' ), '20150101010101' ], ]; } @@ -51,15 +68,13 @@ class WatchedItemUnitTest extends PHPUnit_Framework_TestCase { ->method( 'loadWatchedItem' ) ->with( $user, $linkTarget ) ->will( $this->returnValue( new WatchedItem( $user, $linkTarget, $timestamp ) ) ); - $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); + $this->setService( 'WatchedItemStore', $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 ); } /** @@ -85,12 +100,10 @@ class WatchedItemUnitTest extends PHPUnit_Framework_TestCase { return true; } ) ); - $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); + $this->setService( 'WatchedItemStore', $store ); $item = new WatchedItem( $user, $linkTarget, $timestamp ); $item->resetNotificationTimestamp( $force, $oldid ); - - ScopedCallback::consume( $scopedOverride ); } public function testAddWatch() { @@ -157,17 +170,15 @@ class WatchedItemUnitTest extends PHPUnit_Framework_TestCase { $store->expects( $this->once() ) ->method( 'duplicateAllAssociatedEntries' ) ->with( $oldTitle, $newTitle ); - $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); + $this->setService( 'WatchedItemStore', $store ); WatchedItem::duplicateEntries( $oldTitle, $newTitle ); - - ScopedCallback::consume( $scopedOverride ); } public function testBatchAddWatch() { - $itemOne = new WatchedItem( User::newFromId( 1 ), new TitleValue( 0, 'Title1' ), null ); + $itemOne = new WatchedItem( $this->getMockUser( 1 ), new TitleValue( 0, 'Title1' ), null ); $itemTwo = new WatchedItem( - User::newFromId( 3 ), + $this->getMockUser( 3 ), Title::newFromText( 'Title2' ), '20150101010101' ); @@ -193,11 +204,9 @@ class WatchedItemUnitTest extends PHPUnit_Framework_TestCase { $itemTwo->getTitle()->getTalkPage(), ] ); - $scopedOverride = WatchedItemStore::overrideDefaultInstance( $store ); + $this->setService( 'WatchedItemStore', $store ); WatchedItem::batchAddWatch( [ $itemOne, $itemTwo ] ); - - ScopedCallback::consume( $scopedOverride ); } } -- 2.20.1