Merge "rdbms: lower the log channel severity of LoadMonitor::getServerStates"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Fri, 15 Mar 2019 22:57:31 +0000 (22:57 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Fri, 15 Mar 2019 22:57:31 +0000 (22:57 +0000)
29 files changed:
RELEASE-NOTES-1.33
autoload.php
includes/ForeignResourceManager.php
includes/Storage/DerivedPageDataUpdater.php
includes/deferred/CdnCacheUpdate.php
includes/deferred/LinksUpdate.php
includes/deferred/RefreshSecondaryDataUpdate.php [new file with mode: 0644]
includes/libs/objectcache/APCBagOStuff.php
includes/libs/objectcache/APCUBagOStuff.php
includes/libs/objectcache/BagOStuff.php
includes/libs/objectcache/CachedBagOStuff.php
includes/libs/objectcache/HashBagOStuff.php
includes/libs/objectcache/RESTBagOStuff.php
includes/libs/objectcache/ReplicatedBagOStuff.php
includes/libs/objectcache/WinCacheBagOStuff.php
includes/libs/rdbms/database/Database.php
includes/page/WikiPage.php
includes/specials/SpecialWatchlist.php
includes/watcheditem/WatchedItemStore.php
maintenance/cleanupPreferences.php
maintenance/findHooks.php
maintenance/generateJsonI18n.php
maintenance/hhvm/makeRepo.php
tests/phpunit/documentation/ReleaseNotesTest.php
tests/phpunit/includes/LinkerTest.php
tests/phpunit/includes/changetags/ChangeTagsTest.php
tests/phpunit/includes/db/DatabaseTestHelper.php
tests/phpunit/includes/password/Pbkdf2PasswordFallbackTest.php
tests/phpunit/includes/watcheditem/WatchedItemStoreUnitTest.php

index f8a4db1..27b1c71 100644 (file)
@@ -287,6 +287,8 @@ because of Phabricator reports.
 * BagOStuff::modifySimpleRelayEvent() method has been removed.
 * ParserOutput::getLegacyOptions, deprecated in 1.30, has been removed.
   Use ParserOutput::allCacheVaryingOptions instead.
+* CdnCacheUpdate::newSimplePurge, deprecated in 1.27, has been removed.
+  Use CdnCacheUpdate::newFromTitles() instead.
 
 === Deprecations in 1.33 ===
 * The configuration option $wgUseESI has been deprecated, and is expected
@@ -352,6 +354,7 @@ because of Phabricator reports.
 * The implementation of buildStringCast() in Wikimedia\Rdbms\Database has
   changed to explicitly cast. Subclasses relying on the base-class
   implementation should check whether they need to override it now.
+* BagOStuff::add is now abstract and must explicitly be defined in subclasses.
 
 == Compatibility ==
 MediaWiki 1.33 requires PHP 7.0.13 or later. Although HHVM 3.18.5 or later is
index 9dad6f2..bb4de22 100644 (file)
@@ -1215,6 +1215,7 @@ $wgAutoloadLocalClasses = [
        'RefreshImageMetadata' => __DIR__ . '/maintenance/refreshImageMetadata.php',
        'RefreshLinks' => __DIR__ . '/maintenance/refreshLinks.php',
        'RefreshLinksJob' => __DIR__ . '/includes/jobqueue/jobs/RefreshLinksJob.php',
+       'RefreshSecondaryDataUpdate' => __DIR__ . '/includes/deferred/RefreshSecondaryDataUpdate.php',
        'RegexlikeReplacer' => __DIR__ . '/includes/libs/replacers/RegexlikeReplacer.php',
        'RemexStripTagHandler' => __DIR__ . '/includes/parser/RemexStripTagHandler.php',
        'RemoveInvalidEmails' => __DIR__ . '/maintenance/removeInvalidEmails.php',
index 18014fa..e0d088a 100644 (file)
@@ -234,6 +234,7 @@ class ForeignResourceManager {
                                                $from,
                                                RecursiveDirectoryIterator::SKIP_DOTS
                                        ) );
+                                       /** @var SplFileInfo $file */
                                        foreach ( $rii as $file ) {
                                                $remote = $file->getPathname();
                                                $local = strtr( $remote, [ $from => $to ] );
index 9ce12b4..8dedc70 100644 (file)
@@ -24,6 +24,7 @@ namespace MediaWiki\Storage;
 
 use ApiStashEdit;
 use CategoryMembershipChangeJob;
+use RefreshSecondaryDataUpdate;
 use Content;
 use ContentHandler;
 use DataUpdate;
@@ -1590,14 +1591,31 @@ class DerivedPageDataUpdater implements IDBAccessObject {
                                $update->setRevision( $legacyRevision );
                                $update->setTriggeringUser( $triggeringUser );
                        }
-                       if ( $options['defer'] === false ) {
-                               if ( $options['transactionTicket'] !== null ) {
+               }
+
+               if ( $options['defer'] === false ) {
+                       foreach ( $updates as $update ) {
+                               if ( $update instanceof DataUpdate && $options['transactionTicket'] !== null ) {
                                        $update->setTransactionTicket( $options['transactionTicket'] );
                                }
                                $update->doUpdate();
-                       } else {
-                               DeferredUpdates::addUpdate( $update, $options['defer'] );
                        }
+               } else {
+                       $cacheTime = $this->getCanonicalParserOutput()->getCacheTime();
+                       // Bundle all of the data updates into a single deferred update wrapper so that
+                       // any failure will cause at most one refreshLinks job to be enqueued by
+                       // DeferredUpdates::doUpdates(). This is hard to do when there are many separate
+                       // updates that are not defined as being related.
+                       $update = new RefreshSecondaryDataUpdate(
+                               $this->wikiPage,
+                               $updates,
+                               $options,
+                               $cacheTime,
+                               $this->loadbalancerFactory->getLocalDomainID()
+                       );
+                       $update->setRevision( $legacyRevision );
+                       $update->setTriggeringUser( $triggeringUser );
+                       DeferredUpdates::addUpdate( $update, $options['defer'] );
                }
        }
 
index 5329ca7..6f961e8 100644 (file)
@@ -62,15 +62,6 @@ class CdnCacheUpdate implements DeferrableUpdate, MergeableUpdate {
                return new CdnCacheUpdate( $urlArr );
        }
 
-       /**
-        * @param Title $title
-        * @return CdnCacheUpdate
-        * @deprecated since 1.27
-        */
-       public static function newSimplePurge( Title $title ) {
-               return new CdnCacheUpdate( $title->getCdnUrls() );
-       }
-
        /**
         * Purges the list of URLs passed to the constructor.
         */
index 7a31e26..101a1e2 100644 (file)
@@ -32,7 +32,7 @@ use Wikimedia\ScopedCallback;
  *
  * See docs/deferred.txt
  */
-class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate {
+class LinksUpdate extends DataUpdate {
        // @todo make members protected, but make sure extensions don't break
 
        /** @var int Page ID of the article linked from */
@@ -1187,39 +1187,4 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate {
 
                return $this->db;
        }
-
-       public function getAsJobSpecification() {
-               if ( $this->user ) {
-                       $userInfo = [
-                               'userId' => $this->user->getId(),
-                               'userName' => $this->user->getName(),
-                       ];
-               } else {
-                       $userInfo = false;
-               }
-
-               if ( $this->mRevision ) {
-                       $triggeringRevisionId = $this->mRevision->getId();
-               } else {
-                       $triggeringRevisionId = false;
-               }
-
-               return [
-                       'wiki' => WikiMap::getWikiIdFromDbDomain( $this->getDB()->getDomainID() ),
-                       'job'  => new JobSpecification(
-                               'refreshLinksPrioritized',
-                               [
-                                       // Reuse the parser cache if it was saved
-                                       'rootJobTimestamp' => $this->mParserOutput->getCacheTime(),
-                                       'useRecursiveLinksUpdate' => $this->mRecursive,
-                                       'triggeringUser' => $userInfo,
-                                       'triggeringRevisionId' => $triggeringRevisionId,
-                                       'causeAction' => $this->getCauseAction(),
-                                       'causeAgent' => $this->getCauseAgent()
-                               ],
-                               [ 'removeDuplicates' => true ],
-                               $this->getTitle()
-                       )
-               ];
-       }
 }
diff --git a/includes/deferred/RefreshSecondaryDataUpdate.php b/includes/deferred/RefreshSecondaryDataUpdate.php
new file mode 100644 (file)
index 0000000..8086a70
--- /dev/null
@@ -0,0 +1,117 @@
+<?php
+/**
+ * Updater for secondary data after a page edit.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ */
+
+/**
+ * Update object handling the cleanup of secondary data after a page was edited.
+ *
+ * This makes makes it possible for DeferredUpdates to have retry logic using a single
+ * refreshLinks job if any of the bundled updates fail.
+ */
+class RefreshSecondaryDataUpdate extends DataUpdate implements EnqueueableDataUpdate {
+       /** @var WikiPage */
+       private $page;
+       /** @var DeferrableUpdate[] */
+       private $updates;
+       /** @var bool */
+       private $recursive;
+       /** @var string */
+       private $cacheTimestamp;
+       /** @var string Database domain ID */
+       private $domain;
+
+       /** @var Revision|null */
+       private $revision;
+       /** @var User|null */
+       private $user;
+
+       /**
+        * @param WikiPage $page Page we are updating
+        * @param DeferrableUpdate[] $updates Updates from DerivedPageDataUpdater::getSecondaryUpdates()
+        * @param array $options Options map (causeAction, causeAgent, recursive)
+        * @param string $cacheTime Result of ParserOutput::getCacheTime() for the source output
+        * @param string $domain The database domain ID of the wiki the update is for
+        */
+       function __construct(
+               WikiPage $page,
+               array $updates,
+               array $options,
+               $cacheTime,
+               $domain
+       ) {
+               parent::__construct();
+
+               $this->page = $page;
+               $this->updates = $updates;
+               $this->causeAction = $options['causeAction'] ?? 'unknown';
+               $this->causeAgent = $options['causeAgent'] ?? 'unknown';
+               $this->recursive = !empty( $options['recursive'] );
+               $this->cacheTimestamp = $cacheTime;
+               $this->domain = $domain;
+       }
+
+       public function doUpdate() {
+               foreach ( $this->updates as $update ) {
+                       $update->doUpdate();
+               }
+       }
+
+       /**
+        * Set the revision corresponding to this LinksUpdate
+        * @param Revision $revision
+        */
+       public function setRevision( Revision $revision ) {
+               $this->revision = $revision;
+       }
+
+       /**
+        * Set the User who triggered this LinksUpdate
+        * @param User $user
+        */
+       public function setTriggeringUser( User $user ) {
+               $this->user = $user;
+       }
+
+       public function getAsJobSpecification() {
+               return [
+                       'wiki' => WikiMap::getWikiIdFromDomain( $this->domain ),
+                       'job'  => new JobSpecification(
+                               'refreshLinksPrioritized',
+                               [
+                                       // Reuse the parser cache if it was saved
+                                       'rootJobTimestamp' => $this->cacheTimestamp,
+                                       'useRecursiveLinksUpdate' => $this->recursive,
+                                       'triggeringUser' => $this->user
+                                               ? [
+                                                       'userId' => $this->user->getId(),
+                                                       'userName' => $this->user->getName()
+                                               ]
+                                               : false,
+                                       'triggeringRevisionId' => $this->revision ? $this->revision->getId() : false,
+                                       'causeAction' => $this->getCauseAction(),
+                                       'causeAgent' => $this->getCauseAgent()
+                               ],
+                               [ 'removeDuplicates' => true ],
+                               $this->page->getTitle()
+                       )
+               ];
+       }
+}
index 1fedfaf..847a1eb 100644 (file)
@@ -97,6 +97,14 @@ class APCBagOStuff extends BagOStuff {
                return true;
        }
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               return apc_add(
+                       $key . self::KEY_SUFFIX,
+                       $this->setSerialize( $value ),
+                       $exptime
+               );
+       }
+
        protected function setSerialize( $value ) {
                if ( !$this->nativeSerialize && !$this->isInteger( $value ) ) {
                        $value = serialize( $value );
index fb43d4d..d5f1edc 100644 (file)
@@ -55,6 +55,14 @@ class APCUBagOStuff extends APCBagOStuff {
                return true;
        }
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               return apcu_add(
+                       $key . self::KEY_SUFFIX,
+                       $this->setSerialize( $value ),
+                       $exptime
+               );
+       }
+
        public function delete( $key, $flags = 0 ) {
                apcu_delete( $key . self::KEY_SUFFIX );
 
index 9c75a2f..c439f9b 100644 (file)
@@ -267,6 +267,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         * (which will be false if not present), and takes the arguments:
         * (this BagOStuff, cache key, current value, TTL).
         * The TTL parameter is reference set to $exptime. It can be overriden in the callback.
+        * If the callback returns false, then the current value will be unchanged (including TTL).
         *
         * @param string $key
         * @param callable $callback Callback method to be executed
@@ -630,14 +631,7 @@ abstract class BagOStuff implements IExpiringStore, LoggerAwareInterface {
         * @param int $flags Bitfield of BagOStuff::WRITE_* constants (since 1.33)
         * @return bool Success
         */
-       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
-               // @note: avoid lock() here since that method uses *this* method by default
-               if ( $this->get( $key ) === false ) {
-                       return $this->set( $key, $value, $exptime, $flags );
-               }
-
-               return false; // key already set
-       }
+       abstract public function add( $key, $value, $exptime = 0, $flags = 0 );
 
        /**
         * Increase stored value of $key by $value while preserving its TTL
index 25fcdb0..95b12b4 100644 (file)
@@ -101,6 +101,14 @@ class CachedBagOStuff extends HashBagOStuff {
        // These just call the backend (tested elsewhere)
        // @codeCoverageIgnoreStart
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               if ( $this->get( $key ) === false ) {
+                       return $this->set( $key, $value, $exptime, $flags );
+               }
+
+               return false; // key already set
+       }
+
        public function lock( $key, $timeout = 6, $expiry = 6, $rclass = '' ) {
                return $this->backend->lock( $key, $timeout, $expiry, $rclass );
        }
index 7d074fa..f88f567 100644 (file)
@@ -106,6 +106,14 @@ class HashBagOStuff extends BagOStuff {
                return true;
        }
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               if ( $this->get( $key ) === false ) {
+                       return $this->set( $key, $value, $exptime, $flags );
+               }
+
+               return false; // key already set
+       }
+
        public function delete( $key, $flags = 0 ) {
                unset( $this->bag[$key] );
 
index b0b82d8..135556a 100644 (file)
@@ -138,6 +138,14 @@ class RESTBagOStuff extends BagOStuff {
                return $this->handleError( "Failed to store $key", $rcode, $rerr );
        }
 
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               if ( $this->get( $key ) === false ) {
+                       return $this->set( $key, $value, $exptime, $flags );
+               }
+
+               return false; // key already set
+       }
+
        public function delete( $key, $flags = 0 ) {
                // @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
                $req = [
index ea380a6..14e2fef 100644 (file)
@@ -103,7 +103,7 @@ class ReplicatedBagOStuff extends BagOStuff {
        }
 
        public function add( $key, $value, $exptime = 0, $flags = 0 ) {
-               return $this->writeStore->add( $key, $value, $exptime );
+               return $this->writeStore->add( $key, $value, $exptime, $flags );
        }
 
        public function incr( $key, $value = 1 ) {
index 6b0bec0..cae0280 100644 (file)
@@ -40,9 +40,13 @@ class WinCacheBagOStuff extends BagOStuff {
        public function set( $key, $value, $expire = 0, $flags = 0 ) {
                $result = wincache_ucache_set( $key, serialize( $value ), $expire );
 
-               /* wincache_ucache_set returns an empty array on success if $value
-                * was an array, bool otherwise */
-               return ( is_array( $result ) && $result === [] ) || $result;
+               return ( $result === [] || $result === true );
+       }
+
+       public function add( $key, $value, $exptime = 0, $flags = 0 ) {
+               $result = wincache_ucache_add( $key, serialize( $value ), $exptime );
+
+               return ( $result === [] || $result === true );
        }
 
        public function delete( $key, $flags = 0 ) {
index 13bf8f0..224bcf2 100644 (file)
@@ -992,16 +992,37 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        /**
-        * Make sure isOpen() returns true as a sanity check
+        * Make sure there is an open connection handle (alive or not) as a sanity check
+        *
+        * This guards against fatal errors to the binding handle not being defined
+        * in cases where open() was never called or close() was already called
         *
         * @throws DBUnexpectedError
         */
-       protected function assertOpen() {
+       protected function assertHasConnectionHandle() {
                if ( !$this->isOpen() ) {
                        throw new DBUnexpectedError( $this, "DB connection was already closed." );
                }
        }
 
+       /**
+        * Make sure that this server is not marked as a replica nor read-only as a sanity check
+        *
+        * @throws DBUnexpectedError
+        */
+       protected function assertIsWritableMaster() {
+               if ( $this->getLBInfo( 'replica' ) === true ) {
+                       throw new DBUnexpectedError(
+                               $this,
+                               'Write operations are not allowed on replica database connections.'
+                       );
+               }
+               $reason = $this->getReadOnlyReason();
+               if ( $reason !== false ) {
+                       throw new DBReadOnlyError( $this, "Database is read-only: $reason" );
+               }
+       }
+
        /**
         * Closes underlying database connection
         * @since 1.20
@@ -1145,92 +1166,65 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
        public function query( $sql, $fname = __METHOD__, $tempIgnore = false ) {
                $this->assertTransactionStatus( $sql, $fname );
+               $this->assertHasConnectionHandle();
 
-               # Avoid fatals if close() was called
-               $this->assertOpen();
-
+               $priorTransaction = $this->trxLevel;
                $priorWritesPending = $this->writesOrCallbacksPending();
                $this->lastQuery = $sql;
 
-               $isWrite = $this->isWriteQuery( $sql );
-               if ( $isWrite ) {
-                       $isNonTempWrite = !$this->registerTempTableOperation( $sql );
-               } else {
-                       $isNonTempWrite = false;
-               }
-
-               if ( $isWrite ) {
-                       if ( $this->getLBInfo( 'replica' ) === true ) {
-                               throw new DBError(
-                                       $this,
-                                       'Write operations are not allowed on replica database connections.'
-                               );
-                       }
+               if ( $this->isWriteQuery( $sql ) ) {
                        # In theory, non-persistent writes are allowed in read-only mode, but due to things
                        # like https://bugs.mysql.com/bug.php?id=33669 that might not work anyway...
-                       $reason = $this->getReadOnlyReason();
-                       if ( $reason !== false ) {
-                               throw new DBReadOnlyError( $this, "Database is read-only: $reason" );
-                       }
-                       # Set a flag indicating that writes have been done
-                       $this->lastWriteTime = microtime( true );
+                       $this->assertIsWritableMaster();
+                       # Avoid treating temporary table operations as meaningful "writes"
+                       $isEffectiveWrite = !$this->registerTempTableOperation( $sql );
+               } else {
+                       $isEffectiveWrite = false;
                }
 
                # Add trace comment to the begin of the sql string, right after the operator.
                # Or, for one-word queries (like "BEGIN" or COMMIT") add it to the end (T44598)
                $commentedSql = preg_replace( '/\s|$/', " /* $fname {$this->agent} */ ", $sql, 1 );
 
-               # Start implicit transactions that wrap the request if DBO_TRX is enabled
-               if ( !$this->trxLevel && $this->getFlag( self::DBO_TRX )
-                       && $this->isTransactableQuery( $sql )
-               ) {
-                       $this->begin( __METHOD__ . " ($fname)", self::TRANSACTION_INTERNAL );
-                       $this->trxAutomatic = true;
-               }
-
-               # Keep track of whether the transaction has write queries pending
-               if ( $this->trxLevel && !$this->trxDoneWrites && $isWrite ) {
-                       $this->trxDoneWrites = true;
-                       $this->trxProfiler->transactionWritingIn(
-                               $this->server, $this->getDomainID(), $this->trxShortId );
-               }
-
-               if ( $this->getFlag( self::DBO_DEBUG ) ) {
-                       $this->queryLogger->debug( "{$this->getDomainID()} {$commentedSql}" );
-               }
-
                # Send the query to the server and fetch any corresponding errors
-               $ret = $this->doProfiledQuery( $sql, $commentedSql, $isNonTempWrite, $fname );
+               $ret = $this->attemptQuery( $sql, $commentedSql, $isEffectiveWrite, $fname );
                $lastError = $this->lastError();
                $lastErrno = $this->lastErrno();
 
-               # Try reconnecting if the connection was lost
+               $recoverableSR = false; // recoverable statement rollback?
+               $recoverableCL = false; // recoverable connection loss?
+
                if ( $ret === false && $this->wasConnectionLoss() ) {
-                       # Check if any meaningful session state was lost
-                       $recoverable = $this->canRecoverFromDisconnect( $sql, $priorWritesPending );
+                       # Check if no meaningful session state was lost
+                       $recoverableCL = $this->canRecoverFromDisconnect( $sql, $priorWritesPending );
                        # Update session state tracking and try to restore the connection
                        $reconnected = $this->replaceLostConnection( __METHOD__ );
                        # Silently resend the query to the server if it is safe and possible
-                       if ( $reconnected && $recoverable ) {
-                               $ret = $this->doProfiledQuery( $sql, $commentedSql, $isNonTempWrite, $fname );
+                       if ( $recoverableCL && $reconnected ) {
+                               $ret = $this->attemptQuery( $sql, $commentedSql, $isEffectiveWrite, $fname );
                                $lastError = $this->lastError();
                                $lastErrno = $this->lastErrno();
 
                                if ( $ret === false && $this->wasConnectionLoss() ) {
                                        # Query probably causes disconnects; reconnect and do not re-run it
                                        $this->replaceLostConnection( __METHOD__ );
+                               } else {
+                                       $recoverableCL = false; // connection does not need recovering
+                                       $recoverableSR = $this->wasKnownStatementRollbackError();
                                }
                        }
+               } else {
+                       $recoverableSR = $this->wasKnownStatementRollbackError();
                }
 
                if ( $ret === false ) {
-                       if ( $this->trxLevel ) {
-                               if ( $this->wasKnownStatementRollbackError() ) {
+                       if ( $priorTransaction ) {
+                               if ( $recoverableSR ) {
                                        # We're ignoring an error that caused just the current query to be aborted.
                                        # But log the cause so we can log a deprecation notice if a caller actually
                                        # does ignore it.
                                        $this->trxStatusIgnoredCause = [ $lastError, $lastErrno, $fname ];
-                               } else {
+                               } elseif ( !$recoverableCL ) {
                                        # Either the query was aborted or all queries after BEGIN where aborted.
                                        # In the first case, the only options going forward are (a) ROLLBACK, or
                                        # (b) ROLLBACK TO SAVEPOINT (if one was set). If the later case, the only
@@ -1254,12 +1248,28 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         *
         * @param string $sql Original SQL query
         * @param string $commentedSql SQL query with debugging/trace comment
-        * @param bool $isWrite Whether the query is a (non-temporary) write operation
+        * @param bool $isEffectiveWrite Whether the query is a (non-temporary table) write
         * @param string $fname Name of the calling function
         * @return bool|ResultWrapper True for a successful write query, ResultWrapper
         *     object for a successful read query, or false on failure
         */
-       private function doProfiledQuery( $sql, $commentedSql, $isWrite, $fname ) {
+       private function attemptQuery( $sql, $commentedSql, $isEffectiveWrite, $fname ) {
+               $this->beginIfImplied( $sql, $fname );
+
+               # Keep track of whether the transaction has write queries pending
+               if ( $isEffectiveWrite ) {
+                       $this->lastWriteTime = microtime( true );
+                       if ( $this->trxLevel && !$this->trxDoneWrites ) {
+                               $this->trxDoneWrites = true;
+                               $this->trxProfiler->transactionWritingIn(
+                                       $this->server, $this->getDomainID(), $this->trxShortId );
+                       }
+               }
+
+               if ( $this->getFlag( self::DBO_DEBUG ) ) {
+                       $this->queryLogger->debug( "{$this->getDomainID()} {$commentedSql}" );
+               }
+
                $isMaster = !is_null( $this->getLBInfo( 'master' ) );
                # generalizeSQL() will probably cut down the query to reasonable
                # logging size most of the time. The substr is really just a sanity check.
@@ -1282,7 +1292,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 
                if ( $ret !== false ) {
                        $this->lastPing = $startTime;
-                       if ( $isWrite && $this->trxLevel ) {
+                       if ( $isEffectiveWrite && $this->trxLevel ) {
                                $this->updateTrxWriteQueryTime( $sql, $queryRuntime, $this->affectedRows() );
                                $this->trxWriteCallers[] = $fname;
                        }
@@ -1295,8 +1305,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                $this->trxProfiler->recordQueryCompletion(
                        $queryProf,
                        $startTime,
-                       $isWrite,
-                       $isWrite ? $this->affectedRows() : $this->numRows( $ret )
+                       $isEffectiveWrite,
+                       $isEffectiveWrite ? $this->affectedRows() : $this->numRows( $ret )
                );
                $this->queryLogger->debug( $sql, [
                        'method' => $fname,
@@ -1307,6 +1317,23 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                return $ret;
        }
 
+       /**
+        * Start an implicit transaction if DBO_TRX is enabled and no transaction is active
+        *
+        * @param string $sql
+        * @param string $fname
+        */
+       private function beginIfImplied( $sql, $fname ) {
+               if (
+                       !$this->trxLevel &&
+                       $this->getFlag( self::DBO_TRX ) &&
+                       $this->isTransactableQuery( $sql )
+               ) {
+                       $this->begin( __METHOD__ . " ($fname)", self::TRANSACTION_INTERNAL );
+                       $this->trxAutomatic = true;
+               }
+       }
+
        /**
         * Update the estimated run-time of a query, not counting large row lock times
         *
@@ -1386,8 +1413,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        /**
-        * Determine whether or not it is safe to retry queries after a database
-        * connection is lost
+        * Determine whether it is safe to retry queries after a database connection is lost
         *
         * @param string $sql SQL query
         * @param bool $priorWritesPending Whether there is a transaction open with
@@ -1436,6 +1462,15 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
         * Clean things up after transaction loss
         */
        private function handleTransactionLoss() {
+               if ( $this->trxDoneWrites ) {
+                       $this->trxProfiler->transactionWritingOut(
+                               $this->server,
+                               $this->getDomainID(),
+                               $this->trxShortId,
+                               $this->pendingWriteQueryDuration( self::ESTIMATE_TOTAL ),
+                               $this->trxWriteAffectedRows
+                       );
+               }
                $this->trxLevel = 0;
                $this->trxAtomicCounter = 0;
                $this->trxIdleCallbacks = []; // T67263; transaction already lost
@@ -3284,7 +3319,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
        }
 
        /**
-        * @return bool Whether it is safe to assume the given error only caused statement rollback
+        * @return bool Whether it is known that the last query error only caused statement rollback
         * @note This is for backwards compatibility for callers catching DBError exceptions in
         *   order to ignore problems like duplicate key errors or foriegn key violations
         * @since 1.31
@@ -3845,8 +3880,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        throw new DBUnexpectedError( $this, $msg );
                }
 
-               // Avoid fatals if close() was called
-               $this->assertOpen();
+               $this->assertHasConnectionHandle();
 
                $this->doBegin( $fname );
                $this->trxStatus = self::STATUS_TRX_OK;
@@ -3922,8 +3956,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                        }
                }
 
-               // Avoid fatals if close() was called
-               $this->assertOpen();
+               $this->assertHasConnectionHandle();
 
                $this->runOnTransactionPreCommitCallbacks();
 
@@ -3975,8 +4008,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
                }
 
                if ( $trxActive ) {
-                       // Avoid fatals if close() was called
-                       $this->assertOpen();
+                       $this->assertHasConnectionHandle();
 
                        $this->doRollback( $fname );
                        $this->trxStatus = self::STATUS_TRX_NONE;
index cd0fa8b..4b0e503 100644 (file)
@@ -50,13 +50,23 @@ class WikiPage implements Page, IDBAccessObject {
         */
        public $mTitle = null;
 
-       /**@{{
+       /**
+        * @var bool
+        * @protected
+        */
+       public $mDataLoaded = false;
+
+       /**
+        * @var bool
+        * @protected
+        */
+       public $mIsRedirect = false;
+
+       /**
+        * @var int|false False means "not loaded"
         * @protected
         */
-       public $mDataLoaded = false;         // !< Boolean
-       public $mIsRedirect = false;         // !< Boolean
-       public $mLatest = false;             // !< Integer (false means "not loaded")
-       /**@}}*/
+       public $mLatest = false;
 
        /** @var PreparedEdit Map of cache fields (text, parser output, ect) for a proposed/new edit */
        public $mPreparedEdit = false;
index 4d2fc62..6defc9d 100644 (file)
@@ -37,12 +37,16 @@ class SpecialWatchlist extends ChangesListSpecialPage {
        protected static $limitPreferenceName = 'wllimit';
        protected static $collapsedPreferenceName = 'rcfilters-wl-collapsed';
 
+       /** @var float|int */
        private $maxDays;
+       /** WatchedItemStore */
+       private $watchStore;
 
        public function __construct( $page = 'Watchlist', $restriction = 'viewmywatchlist' ) {
                parent::__construct( $page, $restriction );
 
                $this->maxDays = $this->getConfig()->get( 'RCMaxAge' ) / ( 3600 * 24 );
+               $this->watchStore = MediaWikiServices::getInstance()->getWatchedItemStore();
        }
 
        public function doesWrites() {
@@ -187,9 +191,13 @@ class SpecialWatchlist extends ChangesListSpecialPage {
                                        'label' => 'rcfilters-filter-watchlistactivity-unseen-label',
                                        'description' => 'rcfilters-filter-watchlistactivity-unseen-description',
                                        'cssClassSuffix' => 'watchedunseen',
-                                       'isRowApplicableCallable' => function ( $ctx, $rc ) {
+                                       'isRowApplicableCallable' => function ( $ctx, RecentChange $rc ) {
                                                $changeTs = $rc->getAttribute( 'rc_timestamp' );
-                                               $lastVisitTs = $rc->getAttribute( 'wl_notificationtimestamp' );
+                                               $lastVisitTs = $this->watchStore->getLatestNotificationTimestamp(
+                                                       $rc->getAttribute( 'wl_notificationtimestamp' ),
+                                                       $rc->getPerformer(),
+                                                       $rc->getTitle()
+                                               );
                                                return $lastVisitTs !== null && $changeTs >= $lastVisitTs;
                                        },
                                ],
index 274a35d..e092859 100644 (file)
@@ -966,14 +966,31 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
                        }
                }
 
+               // Get the timestamp (TS_MW) of this revision to track the latest one seen
+               $seenTime = call_user_func(
+                       $this->revisionGetTimestampFromIdCallback,
+                       $title,
+                       $oldid ?: $title->getLatestRevID()
+               );
+
                // Mark the item as read immediately in lightweight storage
                $this->stash->merge(
                        $this->getPageSeenTimestampsKey( $user ),
-                       function ( $cache, $key, $current ) use ( $time, $title ) {
+                       function ( $cache, $key, $current ) use ( $title, $seenTime ) {
                                $value = $current ?: new MapCacheLRU( 300 );
-                               $value->set( $this->getPageSeenKey( $title ), wfTimestamp( TS_MW, $time ) );
-
-                               $this->latestUpdateCache->set( $key, $value, IExpiringStore::TTL_PROC_LONG );
+                               $subKey = $this->getPageSeenKey( $title );
+
+                               if ( $seenTime > $value->get( $subKey ) ) {
+                                       // Revision is newer than the last one seen
+                                       $value->set( $subKey, $seenTime );
+                                       $this->latestUpdateCache->set( $key, $value, IExpiringStore::TTL_PROC_LONG );
+                               } elseif ( $seenTime === false ) {
+                                       // Revision does not exist
+                                       $value->set( $subKey, wfTimestamp( TS_MW ) );
+                                       $this->latestUpdateCache->set( $key, $value, IExpiringStore::TTL_PROC_LONG );
+                               } else {
+                                       return false; // nothing to update
+                               }
 
                                return $value;
                        },
@@ -1000,7 +1017,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
 
        /**
         * @param User $user
-        * @return MapCacheLRU|null
+        * @return MapCacheLRU|null The map contains prefixed title keys and TS_MW values
         */
        private function getPageSeenTimestamps( User $user ) {
                $key = $this->getPageSeenTimestampsKey( $user );
index a6399f6..66fc6d3 100644 (file)
@@ -99,9 +99,6 @@ class CleanupPreferences extends Maintenance {
                }
        }
 
-       /**
-        *
-        */
        private function deleteByWhere( $dbw, $startMessage, $where ) {
                $this->output( $startMessage . "...\n" );
                $total = 0;
index ed7a762..900752f 100644 (file)
@@ -320,6 +320,7 @@ class FindHooks extends Maintenance {
                        $iterator = new DirectoryIterator( $dir );
                }
 
+               /** @var SplFileInfo $info */
                foreach ( $iterator as $info ) {
                        // Ignore directories, work only on php files,
                        if ( $info->isFile() && in_array( $info->getExtension(), [ 'php', 'inc' ] )
index a7224b4..e9f4eca 100644 (file)
@@ -79,6 +79,7 @@ class GenerateJsonI18n extends Maintenance {
                        $dir_iterator = new RecursiveDirectoryIterator( dirname( $phpfile ) );
                        $iterator = new RecursiveIteratorIterator(
                                $dir_iterator, RecursiveIteratorIterator::LEAVES_ONLY );
+                       /** @var SplFileInfo $fileObject */
                        foreach ( $iterator as $path => $fileObject ) {
                                if ( fnmatch( "*.i18n.php", $fileObject->getFilename() ) ) {
                                        $this->output( "Converting $path.\n" );
index cef0dad..a8a0c71 100644 (file)
@@ -149,6 +149,7 @@ class HHVMMakeRepo extends Maintenance {
                        ),
                        RecursiveIteratorIterator::LEAVES_ONLY
                );
+               /** @var SplFileInfo $fileInfo */
                foreach ( $iter as $file => $fileInfo ) {
                        if ( $fileInfo->isFile() ) {
                                $files[] = $file;
index 2789571..019a13e 100644 (file)
@@ -41,8 +41,6 @@ class ReleaseNotesTest extends MediaWikiTestCase {
                }
        }
 
-       /**
-        */
        private function assertFileLength( $type, $fileName ) {
                $file = file( $fileName, FILE_IGNORE_NEW_LINES );
 
index 0e209d5..1405680 100644 (file)
@@ -592,7 +592,13 @@ class ChangeTagsTest extends MediaWikiTestCase {
                                'ctd_user_defined' => 1
                        ],
                ];
-               $res = $dbr->select( 'change_tag_def', [ 'ctd_name', 'ctd_user_defined' ], '' );
+               $res = $dbr->select(
+                       'change_tag_def',
+                       [ 'ctd_name', 'ctd_user_defined' ],
+                       '',
+                       __METHOD__,
+                       [ 'ORDER BY' => 'ctd_name' ]
+               );
                $this->assertEquals( $expected, iterator_to_array( $res, false ) );
        }
 }
index ff10fef..9679c6c 100644 (file)
@@ -108,7 +108,11 @@ class DatabaseTestHelper extends Database {
 
                // Handle some internal calls from the Database class
                $check = $fname;
-               if ( preg_match( '/^Wikimedia\\\\Rdbms\\\\Database::query \((.+)\)$/', $fname, $m ) ) {
+               if ( preg_match(
+                       '/^Wikimedia\\\\Rdbms\\\\Database::(?:query|beginIfImplied) \((.+)\)$/',
+                       $fname,
+                       $m
+               ) ) {
                        $check = $m[1];
                }
 
index 280ad90..6249c49 100644 (file)
@@ -2388,7 +2388,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
                                $oldid
                        )
                );
-               $this->assertEquals( 1, $getTimestampCallCounter );
+               $this->assertEquals( 2, $getTimestampCallCounter );
 
                ScopedCallback::consume( $scopedOverrideRevision );
        }
@@ -2530,7 +2530,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
                                $oldid
                        )
                );
-               $this->assertEquals( 1, $getTimestampCallCounter );
+               $this->assertEquals( 2, $getTimestampCallCounter );
 
                ScopedCallback::consume( $scopedOverrideRevision );
        }
@@ -2609,7 +2609,7 @@ class WatchedItemStoreUnitTest extends MediaWikiTestCase {
                                $oldid
                        )
                );
-               $this->assertEquals( 1, $getTimestampCallCounter );
+               $this->assertEquals( 2, $getTimestampCallCounter );
 
                ScopedCallback::consume( $scopedOverrideRevision );
        }