Merge "editTests: Use the correct list of parser test files"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Sat, 10 Feb 2018 00:30:08 +0000 (00:30 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Sat, 10 Feb 2018 00:30:08 +0000 (00:30 +0000)
12 files changed:
includes/MediaWiki.php
includes/ServiceWiring.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
includes/parser/ParserOutput.php
includes/specials/SpecialEditWatchlist.php
includes/user/User.php
includes/watcheditem/WatchedItemStore.php
languages/i18n/en.json
languages/i18n/qqq.json
tests/phpunit/includes/parser/ParserOutputTest.php
tests/phpunit/includes/watcheditem/WatchedItemStoreIntegrationTest.php
tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php

index 150c72f..371f2cb 100644 (file)
@@ -548,6 +548,9 @@ class MediaWiki {
                                }
                        }
 
+                       MWExceptionHandler::handleException( $e );
+               } catch ( Error $e ) {
+                       // Type errors and such: at least handle it now and clean up the LBFactory state
                        MWExceptionHandler::handleException( $e );
                }
 
index 3a9474b..8b0452d 100644 (file)
@@ -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() );
 
index d070c1e..04b3ea3 100644 (file)
@@ -49,7 +49,7 @@ class LoadBalancer implements ILoadBalancer {
        /** @var bool Whether to disregard replica DB lag as a factor in replica DB selection */
        private $mAllowLagged;
        /** @var int Seconds to spend waiting on replica DB lag to resolve */
-       private $mWaitTimeout;
+       private $waitTimeout;
        /** @var array The LoadMonitor configuration */
        private $loadMonitorConfig;
        /** @var array[] $aliases Map of (table => (dbname, schema, prefix) map) */
@@ -153,7 +153,7 @@ class LoadBalancer implements ILoadBalancer {
                        : DatabaseDomain::newUnspecified();
                $this->setLocalDomain( $localDomain );
 
-               $this->mWaitTimeout = isset( $params['waitTimeout'] )
+               $this->waitTimeout = isset( $params['waitTimeout'] )
                        ? $params['waitTimeout']
                        : self::MAX_WAIT_DEFAULT;
 
@@ -515,7 +515,7 @@ class LoadBalancer implements ILoadBalancer {
        }
 
        public function waitForAll( $pos, $timeout = null ) {
-               $timeout = $timeout ?: $this->mWaitTimeout;
+               $timeout = $timeout ?: $this->waitTimeout;
 
                $oldPos = $this->mWaitForPos;
                try {
@@ -575,11 +575,11 @@ class LoadBalancer implements ILoadBalancer {
         * Wait for a given replica DB to catch up to the master pos stored in $this
         * @param int $index Server index
         * @param bool $open Check the server even if a new connection has to be made
-        * @param int $timeout Max seconds to wait; default is mWaitTimeout
+        * @param int $timeout Max seconds to wait; default is "waitTimeout" given to __construct()
         * @return bool
         */
        protected function doWait( $index, $open = false, $timeout = null ) {
-               $timeout = max( 1, $timeout ?: $this->mWaitTimeout );
+               $timeout = max( 1, $timeout ?: $this->waitTimeout );
 
                // Check if we already know that the DB has reached this point
                $server = $this->getServerName( $index );
@@ -1372,7 +1372,7 @@ class LoadBalancer implements ILoadBalancer {
                $this->trxRoundId = false;
                $this->forEachOpenMasterConnection(
                        function ( IDatabase $conn ) use ( $fname, $restore ) {
-                               if ( $conn->writesOrCallbacksPending() ) {
+                               if ( $conn->writesOrCallbacksPending() || $conn->explicitTrxActive() ) {
                                        $conn->rollback( $fname, $conn::FLUSHING_ALL_PEERS );
                                }
                                if ( $restore ) {
@@ -1448,7 +1448,7 @@ class LoadBalancer implements ILoadBalancer {
        }
 
        public function hasOrMadeRecentMasterChanges( $age = null ) {
-               $age = ( $age === null ) ? $this->mWaitTimeout : $age;
+               $age = ( $age === null ) ? $this->waitTimeout : $age;
 
                return ( $this->hasMasterChanges()
                        || $this->lastMasterChangeTimestamp() > microtime( true ) - $age );
@@ -1658,10 +1658,12 @@ class LoadBalancer implements ILoadBalancer {
        /**
         * @param IDatabase $conn
         * @param DBMasterPos|bool $pos
-        * @param int $timeout
+        * @param int|null $timeout
         * @return bool
         */
-       public function safeWaitForMasterPos( IDatabase $conn, $pos = false, $timeout = 10 ) {
+       public function safeWaitForMasterPos( IDatabase $conn, $pos = false, $timeout = null ) {
+               $timeout = max( 1, $timeout ?: $this->waitTimeout );
+
                if ( $this->getServerCount() <= 1 || !$conn->getLBInfo( 'replica' ) ) {
                        return true; // server is not a replica DB
                }
index e2efaff..346a151 100644 (file)
@@ -301,10 +301,16 @@ class ParserOutput extends CacheTime {
                        ] );
                        $startLen = strlen( $start );
                        $end = Html::closeElement( 'div' );
+                       $endPos = strrpos( $text, $end );
                        $endLen = strlen( $end );
 
-                       if ( substr( $text, 0, $startLen ) === $start && substr( $text, -$endLen ) === $end ) {
-                               $text = substr( $text, $startLen, -$endLen );
+                       if ( substr( $text, 0, $startLen ) === $start && $endPos !== false
+                               // if the closing div is followed by real content, bail out of unwrapping
+                               && preg_match( '/^(?>\s*<!--.*?-->)*\s*$/s', substr( $text, $endPos + $endLen ) )
+                       ) {
+                               $text = substr( $text, $startLen );
+                               $text = substr( $text, 0, $endPos - $startLen )
+                                       . substr( $text, $endPos - $startLen + $endLen );
                        }
                }
 
index e3c5d8c..0a38ad1 100644 (file)
@@ -29,7 +29,6 @@
 use MediaWiki\Linker\LinkRenderer;
 use MediaWiki\Linker\LinkTarget;
 use MediaWiki\MediaWikiServices;
-use Wikimedia\Rdbms\DBReadOnlyError;
 
 /**
  * Provides the UI through which users can perform editing
@@ -244,17 +243,12 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
                                $this->showTitles( $toUnwatch, $this->successMessage );
                        }
                } else {
-                       $this->clearWatchlist();
-                       $this->getUser()->invalidateCache();
 
-                       if ( count( $current ) > 0 ) {
-                               $this->successMessage = $this->msg( 'watchlistedit-raw-done' )->parse();
-                       } else {
+                       if ( count( $current ) === 0 ) {
                                return false;
                        }
 
-                       $this->successMessage .= ' ' . $this->msg( 'watchlistedit-raw-removed' )
-                               ->numParams( count( $current ) )->parse();
+                       $this->clearUserWatchedItems( $current, 'raw' );
                        $this->showTitles( $current, $this->successMessage );
                }
 
@@ -263,16 +257,28 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
 
        public function submitClear( $data ) {
                $current = $this->getWatchlist();
-               $this->clearWatchlist();
-               $this->getUser()->invalidateCache();
-               $this->successMessage = $this->msg( 'watchlistedit-clear-done' )->parse();
-               $this->successMessage .= ' ' . $this->msg( 'watchlistedit-clear-removed' )
-                       ->numParams( count( $current ) )->parse();
+               $this->clearUserWatchedItems( $current, 'clear' );
                $this->showTitles( $current, $this->successMessage );
-
                return true;
        }
 
+       /**
+        * @param array $current
+        * @param string $messageFor 'raw' or 'clear'
+        */
+       private function clearUserWatchedItems( $current, $messageFor ) {
+               $watchedItemStore = MediaWikiServices::getInstance()->getWatchedItemStore();
+               if ( $watchedItemStore->clearUserWatchedItems( $this->getUser() ) ) {
+                       $this->successMessage = $this->msg( 'watchlistedit-' . $messageFor . '-done' )->parse();
+                       $this->successMessage .= ' ' . $this->msg( 'watchlistedit-' . $messageFor . '-removed' )
+                                       ->numParams( count( $current ) )->parse();
+                       $this->getUser()->invalidateCache();
+               } else {
+                       $watchedItemStore->clearUserWatchedItemsUsingJobQueue( $this->getUser() );
+                       $this->successMessage = $this->msg( 'watchlistedit-clear-jobqueue' )->parse();
+               }
+       }
+
        /**
         * Print out a list of linked titles
         *
@@ -448,22 +454,6 @@ class SpecialEditWatchlist extends UnlistedSpecialPage {
                } );
        }
 
-       /**
-        * Remove all titles from a user's watchlist
-        */
-       private function clearWatchlist() {
-               if ( $this->getConfig()->get( 'ReadOnlyWatchedItemStore' ) ) {
-                       throw new DBReadOnlyError( null, 'The watchlist is currently readonly.' );
-               }
-
-               $dbw = wfGetDB( DB_MASTER );
-               $dbw->delete(
-                       'watchlist',
-                       [ 'wl_user' => $this->getUser()->getId() ],
-                       __METHOD__
-               );
-       }
-
        /**
         * Add a list of targets to a user's watchlist
         *
index 97035c2..eeade49 100644 (file)
@@ -2509,12 +2509,17 @@ class User implements IDBAccessObject, UserIdentity {
                if ( $mode === 'refresh' ) {
                        $cache->delete( $key, 1 );
                } else {
-                       wfGetDB( DB_MASTER )->onTransactionPreCommitOrIdle(
-                               function () use ( $cache, $key ) {
-                                       $cache->delete( $key );
-                               },
-                               __METHOD__
-                       );
+                       $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+                       if ( $lb->hasOrMadeRecentMasterChanges() ) {
+                               $lb->getConnection( DB_MASTER )->onTransactionPreCommitOrIdle(
+                                       function () use ( $cache, $key ) {
+                                               $cache->delete( $key );
+                                       },
+                                       __METHOD__
+                               );
+                       } else {
+                               $cache->delete( $key );
+                       }
                }
        }
 
index d6d9ff0..35e824e 100644 (file)
@@ -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.
         *
index 7316448..9d06c96 100644 (file)
        "watchlistedit-clear-titles": "Titles:",
        "watchlistedit-clear-submit": "Clear the watchlist (This is permanent!)",
        "watchlistedit-clear-done": "Your watchlist has been cleared.",
+       "watchlistedit-clear-jobqueue": "Your watchlist is being cleared. This may take some time!",
        "watchlistedit-clear-removed": "{{PLURAL:$1|1 title was|$1 titles were}} removed:",
        "watchlistedit-too-many": "There are too many pages to display here.",
        "watchlisttools-clear": "Clear the watchlist",
index a4f1d64..f3cbb7b 100644 (file)
        "watchlistedit-clear-titles": "Text above edit box containing items being watched on [[Special:Watchlist/clear]].\n{{Identical|Title}}",
        "watchlistedit-clear-submit": "Text of submit button on [[Special:Watchlist/clear]].\n{{Identical|Clear watchlist}}",
        "watchlistedit-clear-done": "A message which appears after the watchlist has been cleared using [[Special:Watchlist/clear]].",
+       "watchlistedit-clear-jobqueue": "A message which appears after the watchlist has been scheduled to be cleared using [[Special:Watchlist/clear]] and the Job Queue.",
        "watchlistedit-clear-removed": "Message on [[Special:EditWatchlist/clear]].\n\nThe message appears once the watchlist has been cleared.",
        "watchlistedit-too-many": "Message on [[Special:EditWatchlist]] that is used when there are too many titles to display.\n\nShown instead of list of the pages.",
        "watchlisttools-clear": "[[Special:Watchlist]]: Navigation link under the title.\n{{Identical|Clear watchlist}}",
index efcc4e0..1f3ee67 100644 (file)
@@ -349,6 +349,11 @@ EOF
                        'Unwrap without a mw-parser-output wrapper' => [
                                [ 'unwrap' => true ], [], '<div class="foobar">Content</div>', '<div class="foobar">Content</div>'
                        ],
+                       'Unwrap with extra comment at end' => [
+                               [ 'unwrap' => true ], [], '<div class="mw-parser-output"><p>Test document.</p></div>
+<!-- Saved in parser cache... -->', '<p>Test document.</p>
+<!-- Saved in parser cache... -->'
+                       ],
                ];
                // phpcs:enable
        }
index 61b62aa..3102929 100644 (file)
@@ -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();
index 6dbb106..52e653c 100644 (file)
@@ -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 );