From: addshore <addshorewiki@gmail.com>
Date: Wed, 6 Apr 2016 10:46:50 +0000 (+0100)
Subject: Make WatchedItemStore use MediaWikiServices
X-Git-Tag: 1.31.0-rc.0~7078
X-Git-Url: http://git.cyclocoop.org/%24href?a=commitdiff_plain;h=8a71ee8ec3f259198e040b294d986f2b29dc2bf1;p=lhc%2Fweb%2Fwiklou.git

Make WatchedItemStore use MediaWikiServices

(This is a re-submi8t of I5c8c1a65 after it got reverted by Iae7f7b7)

Change-Id: I193de7ef1566336040463b71735b09db941d8ce1
---

diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index e48ea7993e..6c650aa641 100644
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -17,6 +17,7 @@ use SearchEngineConfig;
 use SearchEngineFactory;
 use SiteLookup;
 use SiteStore;
+use WatchedItemStore;
 use SkinFactory;
 
 /**
@@ -414,6 +415,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 1f26b583e5..8e95034195 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -130,6 +130,15 @@ return [
 		return $factory;
 	},
 
+	'WatchedItemStore' => function( MediaWikiServices $services ) {
+		$store = new WatchedItemStore(
+			$services->getDBLoadBalancer(),
+			new HashBagOStuff( [ 'maxKeys' => 100 ] )
+		);
+		$store->setStatsdDataFactory( $services->getStatsdDataFactory() );
+		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 fcf6d3b6e6..f3a076b52c 100644
--- a/includes/WatchedItemStore.php
+++ b/includes/WatchedItemStore.php
@@ -1,6 +1,7 @@
 <?php
 
 use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
+use MediaWiki\MediaWikiServices;
 use MediaWiki\Linker\LinkTarget;
 use Wikimedia\Assert\Assert;
 
@@ -50,11 +51,6 @@ class WatchedItemStore implements StatsdAwareInterface {
 	 */
 	private $stats;
 
-	/**
-	 * @var self|null
-	 */
-	private static $instance;
-
 	/**
 	 * @param LoadBalancer $loadBalancer
 	 * @param HashBagOStuff $cache
@@ -126,43 +122,11 @@ class WatchedItemStore implements StatsdAwareInterface {
 	}
 
 	/**
-	 * 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;
-		} );
-	}
-
-	/**
+	 * @deprecated use MediaWikiServices::getInstance()->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 df0ce8ec9f..6c38d503f7 100644
--- a/tests/phpunit/includes/MediaWikiServicesTest.php
+++ b/tests/phpunit/includes/MediaWikiServicesTest.php
@@ -237,6 +237,7 @@ class MediaWikiServicesTest extends PHPUnit_Framework_TestCase {
 			'SkinFactory' => [ 'SkinFactory', SkinFactory::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 9479a82864..108c4c5fb7 100644
--- a/tests/phpunit/includes/WatchedItemStoreUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemStoreUnitTest.php
@@ -6,7 +6,7 @@ use MediaWiki\Linker\LinkTarget;
  *
  * @covers WatchedItemStore
  */
-class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
+class WatchedItemStoreUnitTest extends MediaWikiTestCase {
 
 	/**
 	 * @return PHPUnit_Framework_MockObject_MockObject|IDatabase
@@ -100,17 +100,6 @@ class WatchedItemStoreUnitTest extends PHPUnit_Framework_TestCase {
 		$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 c33ba7ef68..0182eb7fc4 100644
--- a/tests/phpunit/includes/WatchedItemUnitTest.php
+++ b/tests/phpunit/includes/WatchedItemUnitTest.php
@@ -6,13 +6,30 @@ use MediaWiki\Linker\LinkTarget;
  *
  * @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' ],
 		];
 	}
 
@@ -52,15 +69,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 );
 	}
 
 	/**
@@ -86,12 +101,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() {
@@ -158,17 +171,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'
 		);
@@ -194,11 +205,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 );
 	}
 
 }