From 6cf900ffb0bec584e64202bc3a80ba0978646a75 Mon Sep 17 00:00:00 2001 From: addshore Date: Tue, 15 Mar 2016 00:36:02 +0000 Subject: [PATCH] Add clearWatchedItems to WatchedItemStore Change-Id: I67d1057c76ddccece4727f4df701a3ad14c3bbaa --- includes/ServiceWiring.php | 3 +- includes/watcheditem/WatchedItemStore.php | 70 +++++++++++++++-- .../WatchedItemStoreIntegrationTest.php | 17 +++++ .../watcheditem/WatchedItemStoreUnitTest.php | 75 ++++++++++++++++++- 4 files changed, 156 insertions(+), 9 deletions(-) diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php index 3a9474be1d..8b0452db3d 100644 --- a/includes/ServiceWiring.php +++ b/includes/ServiceWiring.php @@ -164,7 +164,8 @@ return [ $store = new WatchedItemStore( $services->getDBLoadBalancer(), new HashBagOStuff( [ 'maxKeys' => 100 ] ), - $services->getReadOnlyMode() + $services->getReadOnlyMode(), + $services->getMainConfig()->get( 'UpdateRowsPerQuery' ) ); $store->setStatsdDataFactory( $services->getStatsdDataFactory() ); diff --git a/includes/watcheditem/WatchedItemStore.php b/includes/watcheditem/WatchedItemStore.php index d6d9ff0eb9..35e824e036 100644 --- a/includes/watcheditem/WatchedItemStore.php +++ b/includes/watcheditem/WatchedItemStore.php @@ -13,10 +13,6 @@ use Wikimedia\Rdbms\LoadBalancer; * Database interaction & caching * TODO caching should be factored out into a CachingWatchedItemStore class * - * Uses database because this uses User::isAnon - * - * @group Database - * * @author Addshore * @since 1.27 */ @@ -55,6 +51,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac */ private $revisionGetTimestampFromIdCallback; + /** + * @var int + */ + private $updateRowsPerQuery; + /** * @var StatsdDataFactoryInterface */ @@ -64,18 +65,23 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac * @param LoadBalancer $loadBalancer * @param HashBagOStuff $cache * @param ReadOnlyMode $readOnlyMode + * @param int $updateRowsPerQuery */ public function __construct( LoadBalancer $loadBalancer, HashBagOStuff $cache, - ReadOnlyMode $readOnlyMode + ReadOnlyMode $readOnlyMode, + $updateRowsPerQuery ) { $this->loadBalancer = $loadBalancer; $this->cache = $cache; $this->readOnlyMode = $readOnlyMode; $this->stats = new NullStatsdDataFactory(); - $this->deferredUpdatesAddCallableUpdateCallback = [ DeferredUpdates::class, 'addCallableUpdate' ]; - $this->revisionGetTimestampFromIdCallback = [ Revision::class, 'getTimestampFromId' ]; + $this->deferredUpdatesAddCallableUpdateCallback = + [ DeferredUpdates::class, 'addCallableUpdate' ]; + $this->revisionGetTimestampFromIdCallback = + [ Revision::class, 'getTimestampFromId' ]; + $this->updateRowsPerQuery = $updateRowsPerQuery; } /** @@ -215,6 +221,56 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac return $this->loadBalancer->getConnectionRef( $dbIndex, [ 'watchlist' ] ); } + /** + * Deletes ALL watched items for the given user when under + * $updateRowsPerQuery entries exist. + * + * @since 1.30 + * + * @param User $user + * + * @return bool true on success, false when too many items are watched + */ + public function clearUserWatchedItems( User $user ) { + if ( $this->countWatchedItems( $user ) > $this->updateRowsPerQuery ) { + return false; + } + + $dbw = $this->loadBalancer->getConnectionRef( DB_MASTER ); + $dbw->delete( + 'watchlist', + [ 'wl_user' => $user->getId() ], + __METHOD__ + ); + $this->uncacheAllItemsForUser( $user ); + + return true; + } + + private function uncacheAllItemsForUser( User $user ) { + $userId = $user->getId(); + foreach ( $this->cacheIndex as $ns => $dbKeyIndex ) { + foreach ( $dbKeyIndex as $dbKey => $userIndex ) { + if ( array_key_exists( $userId, $userIndex ) ) { + $this->cache->delete( $userIndex[$userId] ); + unset( $this->cacheIndex[$ns][$dbKey][$userId] ); + } + } + } + + // Cleanup empty cache keys + foreach ( $this->cacheIndex as $ns => $dbKeyIndex ) { + foreach ( $dbKeyIndex as $dbKey => $userIndex ) { + if ( empty( $this->cacheIndex[$ns][$dbKey] ) ) { + unset( $this->cacheIndex[$ns][$dbKey] ); + } + } + if ( empty( $this->cacheIndex[$ns] ) ) { + unset( $this->cacheIndex[$ns] ); + } + } + } + /** * Queues a job that will clear the users watchlist using the Job Queue. * diff --git a/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php b/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php index 61b62aa66b..3102929ec7 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php @@ -106,6 +106,23 @@ class WatchedItemStoreIntegrationTest extends MediaWikiTestCase { ); } + public function testWatchBatchAndClearItems() { + $user = $this->getUser(); + $title1 = Title::newFromText( 'WatchedItemStoreIntegrationTestPage1' ); + $title2 = Title::newFromText( 'WatchedItemStoreIntegrationTestPage2' ); + $store = MediaWikiServices::getInstance()->getWatchedItemStore(); + + $store->addWatchBatchForUser( $user, [ $title1, $title2 ] ); + + $this->assertTrue( $store->isWatched( $user, $title1 ) ); + $this->assertTrue( $store->isWatched( $user, $title2 ) ); + + $store->clearUserWatchedItems( $user ); + + $this->assertFalse( $store->isWatched( $user, $title1 ) ); + $this->assertFalse( $store->isWatched( $user, $title2 ) ); + } + public function testUpdateResetAndSetNotificationTimestamp() { $user = $this->getUser(); $otherUser = ( new TestUser( 'WatchedItemStoreIntegrationTestUser_otherUser' ) )->getUser(); diff --git a/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php b/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php index 6dbb10697e..52e653cb4c 100644 --- a/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php +++ b/tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php @@ -2,6 +2,7 @@ use MediaWiki\Linker\LinkTarget; use Wikimedia\Rdbms\LoadBalancer; use Wikimedia\ScopedCallback; +use Wikimedia\TestingAccessWrapper; /** * @author Addshore @@ -104,10 +105,82 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase { return new WatchedItemStore( $loadBalancer, $cache, - $readOnlyMode + $readOnlyMode, + 1000 ); } + public function testClearWatchedItems() { + $user = $this->getMockNonAnonUserWithId( 7 ); + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'selectField' ) + ->with( + 'watchlist', + 'COUNT(*)', + [ + 'wl_user' => $user->getId(), + ], + $this->isType( 'string' ) + ) + ->will( $this->returnValue( 12 ) ); + $mockDb->expects( $this->once() ) + ->method( 'delete' ) + ->with( + 'watchlist', + [ 'wl_user' => 7 ], + $this->isType( 'string' ) + ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->never() )->method( 'get' ); + $mockCache->expects( $this->never() )->method( 'set' ); + $mockCache->expects( $this->once() ) + ->method( 'delete' ) + ->with( 'RM-KEY' ); + + $store = $this->newWatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache, + $this->getMockReadOnlyMode() + ); + TestingAccessWrapper::newFromObject( $store ) + ->cacheIndex = [ 0 => [ 'F' => [ 7 => 'RM-KEY', 9 => 'KEEP-KEY' ] ] ]; + + $this->assertTrue( $store->clearUserWatchedItems( $user ) ); + } + + public function testClearWatchedItems_tooManyItemsWatched() { + $user = $this->getMockNonAnonUserWithId( 7 ); + + $mockDb = $this->getMockDb(); + $mockDb->expects( $this->once() ) + ->method( 'selectField' ) + ->with( + 'watchlist', + 'COUNT(*)', + [ + 'wl_user' => $user->getId(), + ], + $this->isType( 'string' ) + ) + ->will( $this->returnValue( 99999 ) ); + + $mockCache = $this->getMockCache(); + $mockCache->expects( $this->never() )->method( 'get' ); + $mockCache->expects( $this->never() )->method( 'set' ); + $mockCache->expects( $this->never() )->method( 'delete' ); + + $store = $this->newWatchedItemStore( + $this->getMockLoadBalancer( $mockDb ), + $mockCache, + $this->getMockReadOnlyMode() + ); + + $this->assertFalse( $store->clearUserWatchedItems( $user ) ); + } + public function testCountWatchedItems() { $user = $this->getMockNonAnonUserWithId( 1 ); -- 2.20.1