diff options
| -rw-r--r-- | includes/MediaWikiServices.php | 9 | ||||
| -rw-r--r-- | includes/ServiceWiring.php | 9 | ||||
| -rw-r--r-- | includes/WatchedItemStore.php | 42 | ||||
| -rw-r--r-- | tests/phpunit/includes/MediaWikiServicesTest.php | 1 | ||||
| -rw-r--r-- | tests/phpunit/includes/WatchedItemStoreUnitTest.php | 14 | ||||
| -rw-r--r-- | tests/phpunit/includes/WatchedItemUnitTest.php | 45 |
6 files changed, 70 insertions, 50 deletions
diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php index dd66967d92ca..f5ae2d517c10 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 dc2c66b6b831..5ec1d64c7ba9 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 603bacd1cd64..8ae7932be0b4 100644 --- a/includes/WatchedItemStore.php +++ b/includes/WatchedItemStore.php @@ -1,7 +1,6 @@ <?php use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface; -use MediaWiki\MediaWikiServices; use Wikimedia\Assert\Assert; /** @@ -51,6 +50,11 @@ class WatchedItemStore implements StatsdAwareInterface { private $stats; /** + * @var self|null + */ + private static $instance; + + /** * @param LoadBalancer $loadBalancer * @param HashBagOStuff $cache */ @@ -121,11 +125,43 @@ class WatchedItemStore implements StatsdAwareInterface { } /** - * @deprecated use MediaWikiServices::getInstance()->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 96abf97e41fb..53cd0b12cfdb 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 78231c2418e3..9dd38df0693b 100644 --- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php @@ -1,11 +1,12 @@ <?php +use Liuggio\StatsdClient\Factory\StatsdDataFactory; /** * @author Addshore * * @covers WatchedItemStore */ -class WatchedItemStoreUnitTest extends MediaWikiTestCase { +class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase { /** * @return PHPUnit_Framework_MockObject_MockObject|IDatabase @@ -95,6 +96,17 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { $this->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 db7f16c655a9..b4eaa765e9c3 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 ); } } |