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 dd66967..f5ae2d5 100644 (file)
--- 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 dc2c66b..5ec1d64 100644 (file)
--- 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 603bacd..8ae7932 100644 (file)
--- 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;
 
 /**
@@ -50,6 +49,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 96abf97..53cd0b1 100644 (file)
--- 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 78231c2..9dd38df 100644 (file)
--- 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 db7f16c..b4eaa76 100644 (file)
--- 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 );
        }
 
 }