Merge "Ignore reuseConnection() errors after LoadBalancer/LBFactory destruction"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 6 Oct 2016 21:41:38 +0000 (21:41 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 6 Oct 2016 21:41:38 +0000 (21:41 +0000)
15 files changed:
RELEASE-NOTES-1.28
includes/EditPage.php
includes/Revision.php
includes/WatchedItemStore.php
includes/content/ContentHandler.php
includes/diff/DifferenceEngine.php
includes/filerepo/FileRepo.php
includes/filerepo/ForeignDBRepo.php
includes/filerepo/ForeignDBViaLBRepo.php
includes/filerepo/LocalRepo.php
includes/filerepo/file/File.php
includes/filerepo/file/ForeignDBFile.php
includes/filerepo/file/LocalFile.php
includes/filerepo/file/OldLocalFile.php
includes/page/Article.php

index 27e02a5..1ece2a4 100644 (file)
@@ -214,6 +214,8 @@ changes to languages because of Phabricator reports.
 * IP::isConfiguredProxy() and IP::isTrustedProxy() were removed. Callers should
   migrate to using the same functions on a ProxyLookup instance, obtainable from
   MediaWikiServices.
+* The ArticleViewCustom, EditPageGetDiffText and EditPageGetPreviewText hooks will
+  now emit deprecation warnings if used.
 
 == Compatibility ==
 
index c0c0048..c1ba3ce 100644 (file)
@@ -3416,7 +3416,7 @@ HTML
                }
 
                if ( $newContent ) {
-                       ContentHandler::runLegacyHooks( 'EditPageGetDiffText', [ $this, &$newContent ] );
+                       ContentHandler::runLegacyHooks( 'EditPageGetDiffText', [ $this, &$newContent ], '1.21' );
                        Hooks::run( 'EditPageGetDiffContent', [ $this, &$newContent ] );
 
                        $popts = ParserOptions::newFromUserAndLang( $wgUser, $wgContLang );
@@ -3829,7 +3829,7 @@ HTML
                        }
 
                        $hook_args = [ $this, &$content ];
-                       ContentHandler::runLegacyHooks( 'EditPageGetPreviewText', $hook_args );
+                       ContentHandler::runLegacyHooks( 'EditPageGetPreviewText', $hook_args, '1.25' );
                        Hooks::run( 'EditPageGetPreviewContent', $hook_args );
 
                        $parserResult = $this->doPreviewParse( $content );
index 6acc528..208652f 100644 (file)
@@ -1046,11 +1046,10 @@ class Revision implements IDBAccessObject {
         *   to the $audience parameter
         *
         * @deprecated since 1.21, use getContent() instead
-        * @todo Replace usage in core
         * @return string
         */
        public function getText( $audience = self::FOR_PUBLIC, User $user = null ) {
-               ContentHandler::deprecated( __METHOD__, '1.21' );
+               wfDeprecated( __METHOD__, '1.21' );
 
                $content = $this->getContent( $audience, $user );
                return ContentHandler::getContentText( $content ); # returns the raw content text, if applicable
@@ -1396,6 +1395,11 @@ class Revision implements IDBAccessObject {
        public function insertOn( $dbw ) {
                global $wgDefaultExternalStore, $wgContentHandlerUseDB;
 
+               // We're inserting a new revision, so we have to use master anyway.
+               // If it's a null revision, it may have references to rows that
+               // are not in the replica yet (the text row).
+               $this->mQueryFlags |= self::READ_LATEST;
+
                // Not allowed to have rev_page equal to 0, false, etc.
                if ( !$this->mPage ) {
                        $title = $this->getTitle();
index 90d45ce..6c47cae 100644 (file)
@@ -3,6 +3,7 @@
 use Liuggio\StatsdClient\Factory\StatsdDataFactoryInterface;
 use MediaWiki\Linker\LinkTarget;
 use Wikimedia\Assert\Assert;
+use Wikimedia\ScopedCallback;
 
 /**
  * Storage layer class for WatchedItems.
index d709c5c..5755918 100644 (file)
@@ -243,7 +243,7 @@ abstract class ContentHandler {
                }
 
                // Hook can force JS/CSS
-               Hooks::run( 'TitleIsCssOrJsPage', [ $title, &$isCodePage ], '1.25' );
+               Hooks::run( 'TitleIsCssOrJsPage', [ $title, &$isCodePage ], '1.21' );
 
                // Is this a user subpage containing code?
                $isCodeSubpage = NS_USER == $ns
@@ -258,7 +258,7 @@ abstract class ContentHandler {
                $isWikitext = $isWikitext && !$isCodePage && !$isCodeSubpage;
 
                // Hook can override $isWikitext
-               Hooks::run( 'TitleIsWikitextPage', [ $title, &$isWikitext ], '1.25' );
+               Hooks::run( 'TitleIsWikitextPage', [ $title, &$isWikitext ], '1.21' );
 
                if ( !$isWikitext ) {
                        switch ( $ext ) {
index 1537535..f27db07 100644 (file)
@@ -620,7 +620,11 @@ class DifferenceEngine extends ContextSource {
                                }
                        } elseif ( !Hooks::run( 'ArticleContentViewCustom', [ $this->mNewContent, $this->mNewPage, $out ] ) ) {
                                // Handled by extension
-                       } elseif ( !ContentHandler::runLegacyHooks( 'ArticleViewCustom', [ $this->mNewContent, $this->mNewPage, $out ] ) ) {
+                       } elseif ( !ContentHandler::runLegacyHooks(
+                               'ArticleViewCustom',
+                               [ $this->mNewContent, $this->mNewPage, $out ],
+                               '1.21'
+                       ) ) {
                                // NOTE: deprecated hook, B/C only
                                // Handled by extension
                        } else {
index 1a6c818..66dab99 100644 (file)
@@ -393,7 +393,7 @@ class FileRepo {
                        if ( $this->oldFileFactory ) {
                                return call_user_func( $this->oldFileFactory, $title, $this, $time );
                        } else {
-                               return false;
+                               return null;
                        }
                } else {
                        return call_user_func( $this->fileFactory, $title, $this );
@@ -818,7 +818,7 @@ class FileRepo {
         *   self::OVERWRITE_SAME    Overwrite the file if the destination exists and has the
         *                           same contents as the source
         *   self::SKIP_LOCKING      Skip any file locking when doing the store
-        * @return FileRepoStatus
+        * @return Status
         */
        public function store( $srcPath, $dstZone, $dstRel, $flags = 0 ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -841,7 +841,7 @@ class FileRepo {
         *                           same contents as the source
         *   self::SKIP_LOCKING      Skip any file locking when doing the store
         * @throws MWException
-        * @return FileRepoStatus
+        * @return Status
         */
        public function storeBatch( array $triplets, $flags = 0 ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -912,7 +912,7 @@ class FileRepo {
         * @param array $files List of files to delete
         * @param int $flags Bitwise combination of the following flags:
         *   self::SKIP_LOCKING      Skip any file locking when doing the deletions
-        * @return FileRepoStatus
+        * @return Status
         */
        public function cleanupBatch( array $files, $flags = 0 ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -952,7 +952,7 @@ class FileRepo {
         * @param array|string|null $options An array consisting of a key named headers
         *   listing extra headers. If a string, taken as content-disposition header.
         *   (Support for array of options new in 1.23)
-        * @return FileRepoStatus
+        * @return Status
         */
        final public function quickImport( $src, $dst, $options = null ) {
                return $this->quickImportBatch( [ [ $src, $dst, $options ] ] );
@@ -964,7 +964,7 @@ class FileRepo {
         * This is intended for purging thumbnails.
         *
         * @param string $path Virtual URL or storage path
-        * @return FileRepoStatus
+        * @return Status
         */
        final public function quickPurge( $path ) {
                return $this->quickPurgeBatch( [ $path ] );
@@ -995,7 +995,7 @@ class FileRepo {
         * When "headers" are given they are used as HTTP headers if supported.
         *
         * @param array $triples List of (source path or FSFile, destination path, disposition)
-        * @return FileRepoStatus
+        * @return Status
         */
        public function quickImportBatch( array $triples ) {
                $status = $this->newGood();
@@ -1040,7 +1040,7 @@ class FileRepo {
         * This does no locking nor journaling and is intended for purging thumbnails.
         *
         * @param array $paths List of virtual URLs or storage paths
-        * @return FileRepoStatus
+        * @return Status
         */
        public function quickPurgeBatch( array $paths ) {
                $status = $this->newGood();
@@ -1065,7 +1065,7 @@ class FileRepo {
         * @param string $originalName The base name of the file as specified
         *   by the user. The file extension will be maintained.
         * @param string $srcPath The current location of the file.
-        * @return FileRepoStatus Object with the URL in the value.
+        * @return Status Object with the URL in the value.
         */
        public function storeTemp( $originalName, $srcPath ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -1107,7 +1107,7 @@ class FileRepo {
         * @param string $dstPath Target file system path
         * @param int $flags Bitwise combination of the following flags:
         *   self::DELETE_SOURCE     Delete the source files on success
-        * @return FileRepoStatus
+        * @return Status
         */
        public function concatenate( array $srcPaths, $dstPath, $flags = 0 ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -1156,7 +1156,7 @@ class FileRepo {
         * @param int $flags Bitfield, may be FileRepo::DELETE_SOURCE to indicate
         *   that the source file should be deleted if possible
         * @param array $options Optional additional parameters
-        * @return FileRepoStatus
+        * @return Status
         */
        public function publish(
                $src, $dstRel, $archiveRel, $flags = 0, array $options = []
@@ -1185,7 +1185,7 @@ class FileRepo {
         * @param int $flags Bitfield, may be FileRepo::DELETE_SOURCE to indicate
         *   that the source files should be deleted if possible
         * @throws MWException
-        * @return FileRepoStatus
+        * @return Status
         */
        public function publishBatch( array $ntuples, $flags = 0 ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -1322,7 +1322,10 @@ class FileRepo {
                        $params = [ 'noAccess' => true, 'noListing' => true ] + $params;
                }
 
-               return $this->backend->prepare( $params );
+               $status = $this->newGood();
+               $status->merge( $this->backend->prepare( $params ) );
+
+               return $status;
        }
 
        /**
@@ -1380,7 +1383,7 @@ class FileRepo {
         * @param mixed $srcRel Relative path for the file to be deleted
         * @param mixed $archiveRel Relative path for the archive location.
         *   Relative to a private archive directory.
-        * @return FileRepoStatus
+        * @return Status
         */
        public function delete( $srcRel, $archiveRel ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -1403,7 +1406,7 @@ class FileRepo {
         *   public root in the first element, and the archive file path relative
         *   to the deleted zone root in the second element.
         * @throws MWException
-        * @return FileRepoStatus
+        * @return Status
         */
        public function deleteBatch( array $sourceDestPairs ) {
                $this->assertWritableRepo(); // fail out if read-only
@@ -1599,7 +1602,10 @@ class FileRepo {
                $path = $this->resolveToStoragePath( $virtualUrl );
                $params = [ 'src' => $path, 'headers' => $headers, 'options' => $optHeaders ];
 
-               return $this->backend->streamFile( $params );
+               $status = $this->newGood();
+               $status->merge( $this->backend->streamFile( $params ) );
+
+               return $status;
        }
 
        /**
index be046bd..7fb7a0e 100644 (file)
@@ -51,9 +51,12 @@ class ForeignDBRepo extends LocalRepo {
        /** @var bool */
        protected $hasSharedCache;
 
-       # Other stuff
+       /** @var IDatabase */
        protected $dbConn;
+
+       /** @var callable */
        protected $fileFactory = [ 'ForeignDBFile', 'newFromTitle' ];
+       /** @var callable */
        protected $fileFromRowFactory = [ 'ForeignDBFile', 'newFromRow' ];
 
        /**
index 55df1af..129d55a 100644 (file)
@@ -59,23 +59,22 @@ class ForeignDBViaLBRepo extends LocalRepo {
         * @return IDatabase
         */
        function getMasterDB() {
-               return wfGetDB( DB_MASTER, [], $this->wiki );
+               return wfGetLB( $this->wiki )->getConnectionRef( DB_MASTER, [], $this->wiki );
        }
 
        /**
         * @return IDatabase
         */
        function getSlaveDB() {
-               return wfGetDB( DB_REPLICA, [], $this->wiki );
+               return wfGetLB( $this->wiki )->getConnectionRef( DB_REPLICA, [], $this->wiki );
        }
 
        /**
         * @return Closure
         */
        protected function getDBFactory() {
-               $wiki = $this->wiki;
-               return function( $index ) use ( $wiki ) {
-                       return wfGetDB( $index, [], $wiki );
+               return function( $index ) {
+                       return wfGetLB( $this->wiki )->getConnectionRef( $index, [], $this->wiki );
                };
        }
 
index 21492a5..c195241 100644 (file)
  * @ingroup FileRepo
  */
 class LocalRepo extends FileRepo {
-       /** @var array */
+       /** @var callable */
        protected $fileFactory = [ 'LocalFile', 'newFromTitle' ];
-
-       /** @var array */
+       /** @var callable */
        protected $fileFactoryKey = [ 'LocalFile', 'newFromKey' ];
-
-       /** @var array */
+       /** @var callable */
        protected $fileFromRowFactory = [ 'LocalFile', 'newFromRow' ];
-
-       /** @var array */
+       /** @var callable */
        protected $oldFileFromRowFactory = [ 'OldLocalFile', 'newFromRow' ];
-
-       /** @var array */
+       /** @var callable */
        protected $oldFileFactory = [ 'OldLocalFile', 'newFromTitle' ];
-
-       /** @var array */
+       /** @var callable */
        protected $oldFileFactoryKey = [ 'OldLocalFile', 'newFromKey' ];
 
        function __construct( array $info = null ) {
                parent::__construct( $info );
 
-               $this->hasSha1Storage = isset( $info['storageLayout'] ) && $info['storageLayout'] === 'sha1';
+               $this->hasSha1Storage = isset( $info['storageLayout'] )
+                       && $info['storageLayout'] === 'sha1';
 
                if ( $this->hasSha1Storage() ) {
                        $this->backend = new FileBackendDBRepoWrapper( [
@@ -93,7 +89,7 @@ class LocalRepo extends FileRepo {
         *
         * @param array $storageKeys
         *
-        * @return FileRepoStatus
+        * @return Status
         */
        function cleanupDeletedBatch( array $storageKeys ) {
                if ( $this->hasSha1Storage() ) {
@@ -454,7 +450,7 @@ class LocalRepo extends FileRepo {
 
        /**
         * Get a connection to the replica DB
-        * @return Database
+        * @return IDatabase
         */
        function getSlaveDB() {
                return wfGetDB( DB_REPLICA );
@@ -462,7 +458,7 @@ class LocalRepo extends FileRepo {
 
        /**
         * Get a connection to the master DB
-        * @return Database
+        * @return IDatabase
         */
        function getMasterDB() {
                return wfGetDB( DB_MASTER );
@@ -562,7 +558,7 @@ class LocalRepo extends FileRepo {
         *
         * @param string $function
         * @param array $args
-        * @return FileRepoStatus
+        * @return Status
         */
        protected function skipWriteOperationIfSha1( $function, array $args ) {
                $this->assertWritableRepo(); // fail out if read-only
index c48866b..c1d5573 100644 (file)
@@ -1805,7 +1805,7 @@ abstract class File implements IDBAccessObject {
         * @param int $flags A bitwise combination of:
         *   File::DELETE_SOURCE    Delete the source file, i.e. move rather than copy
         * @param array $options Optional additional parameters
-        * @return FileRepoStatus On success, the value member contains the
+        * @return Status On success, the value member contains the
         *   archive name, or an empty string if it was a new file.
         *
         * STUB
@@ -1905,7 +1905,7 @@ abstract class File implements IDBAccessObject {
         * and logging are caller's responsibility
         *
         * @param Title $target New file name
-        * @return FileRepoStatus
+        * @return Status
         */
        function move( $target ) {
                $this->readOnlyError();
@@ -1922,7 +1922,7 @@ abstract class File implements IDBAccessObject {
         * @param string $reason
         * @param bool $suppress Hide content from sysops?
         * @param User|null $user
-        * @return FileRepoStatus
+        * @return Status
         * STUB
         * Overridden by LocalFile
         */
index cf0045e..c041dea 100644 (file)
@@ -57,7 +57,7 @@ class ForeignDBFile extends LocalFile {
         * @param string $srcPath
         * @param int $flags
         * @param array $options
-        * @return FileRepoStatus
+        * @return Status
         * @throws MWException
         */
        function publish( $srcPath, $flags = 0, array $options = [] ) {
@@ -84,7 +84,7 @@ class ForeignDBFile extends LocalFile {
        /**
         * @param array $versions
         * @param bool $unsuppress
-        * @return FileRepoStatus
+        * @return Status
         * @throws MWException
         */
        function restore( $versions = [], $unsuppress = false ) {
@@ -95,7 +95,7 @@ class ForeignDBFile extends LocalFile {
         * @param string $reason
         * @param bool $suppress
         * @param User|null $user
-        * @return FileRepoStatus
+        * @return Status
         * @throws MWException
         */
        function delete( $reason, $suppress = false, $user = null ) {
@@ -104,7 +104,7 @@ class ForeignDBFile extends LocalFile {
 
        /**
         * @param Title $target
-        * @return FileRepoStatus
+        * @return Status
         * @throws MWException
         */
        function move( $target ) {
index 7ffb147..9df9360 100644 (file)
@@ -1160,7 +1160,7 @@ class LocalFile extends File {
         * @param User|null $user User object or null to use $wgUser
         * @param string[] $tags Change tags to add to the log entry and page revision.
         *   (This doesn't check $user's permissions.)
-        * @return FileRepoStatus On success, the value member contains the
+        * @return Status On success, the value member contains the
         *     archive name, or an empty string if it was a new file.
         */
        function upload( $src, $comment, $pageText, $flags = 0, $props = false,
@@ -1582,7 +1582,7 @@ class LocalFile extends File {
         * @param int $flags A bitwise combination of:
         *     File::DELETE_SOURCE    Delete the source file, i.e. move rather than copy
         * @param array $options Optional additional parameters
-        * @return FileRepoStatus On success, the value member contains the
+        * @return Status On success, the value member contains the
         *     archive name, or an empty string if it was a new file.
         */
        function publish( $src, $flags = 0, array $options = [] ) {
@@ -1601,7 +1601,7 @@ class LocalFile extends File {
         * @param int $flags A bitwise combination of:
         *     File::DELETE_SOURCE    Delete the source file, i.e. move rather than copy
         * @param array $options Optional additional parameters
-        * @return FileRepoStatus On success, the value member contains the
+        * @return Status On success, the value member contains the
         *     archive name, or an empty string if it was a new file.
         */
        function publishTo( $src, $dstRel, $flags = 0, array $options = [] ) {
@@ -1663,7 +1663,7 @@ class LocalFile extends File {
         * and logging are caller's responsibility
         *
         * @param Title $target New file name
-        * @return FileRepoStatus
+        * @return Status
         */
        function move( $target ) {
                if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -1722,7 +1722,7 @@ class LocalFile extends File {
         * @param string $reason
         * @param bool $suppress
         * @param User|null $user
-        * @return FileRepoStatus
+        * @return Status
         */
        function delete( $reason, $suppress = false, $user = null ) {
                if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -1780,7 +1780,7 @@ class LocalFile extends File {
         * @param bool $suppress
         * @param User|null $user
         * @throws MWException Exception on database or file store failure
-        * @return FileRepoStatus
+        * @return Status
         */
        function deleteOld( $archiveName, $reason, $suppress = false, $user = null ) {
                if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -1816,7 +1816,7 @@ class LocalFile extends File {
         * @param array $versions Set of record ids of deleted items to restore,
         *   or empty to restore all revisions.
         * @param bool $unsuppress
-        * @return FileRepoStatus
+        * @return Status
         */
        function restore( $versions = [], $unsuppress = false ) {
                if ( $this->getRepo()->getReadOnlyReason() !== false ) {
@@ -2346,7 +2346,7 @@ class LocalFileDeleteBatch {
 
        /**
         * Run the transaction
-        * @return FileRepoStatus
+        * @return Status
         */
        public function execute() {
                $repo = $this->file->getRepo();
@@ -2494,7 +2494,7 @@ class LocalFileRestoreBatch {
         * rows and there's no need to keep the image row locked while it's acquiring those locks
         * The caller may have its own transaction open.
         * So we save the batch and let the caller call cleanup()
-        * @return FileRepoStatus
+        * @return Status
         */
        public function execute() {
                /** @var Language */
@@ -2795,7 +2795,7 @@ class LocalFileRestoreBatch {
        /**
         * Delete unused files in the deleted zone.
         * This should be called from outside the transaction in which execute() was called.
-        * @return FileRepoStatus
+        * @return Status
         */
        public function cleanup() {
                if ( !$this->cleanupBatch ) {
@@ -2930,7 +2930,7 @@ class LocalFileMoveBatch {
 
        /**
         * Perform the move.
-        * @return FileRepoStatus
+        * @return Status
         */
        public function execute() {
                $repo = $this->file->repo;
@@ -3002,7 +3002,7 @@ class LocalFileMoveBatch {
         * Verify the database updates and return a new FileRepoStatus indicating how
         * many rows would be updated.
         *
-        * @return FileRepoStatus
+        * @return Status
         */
        protected function verifyDBUpdates() {
                $repo = $this->file->repo;
index 31e62ec..a17ca6e 100644 (file)
@@ -332,7 +332,7 @@ class OldLocalFile extends LocalFile {
         * @param string $timestamp
         * @param string $comment
         * @param User $user
-        * @return FileRepoStatus
+        * @return Status
         */
        function uploadOld( $srcPath, $archiveName, $timestamp, $comment, $user ) {
                $this->lock();
index 338b1ae..2e23e3d 100644 (file)
@@ -623,9 +623,11 @@ class Article implements Page {
 
                                                # Allow extensions do their own custom view for certain pages
                                                $outputDone = true;
-                                       } elseif ( !ContentHandler::runLegacyHooks( 'ArticleViewCustom',
-                                                       [ $this->fetchContentObject(), $this->getTitle(), $outputPage ] ) ) {
-
+                                       } elseif ( !ContentHandler::runLegacyHooks(
+                                               'ArticleViewCustom',
+                                               [ $this->fetchContentObject(), $this->getTitle(), $outputPage ],
+                                               '1.21'
+                                       ) ) {
                                                # Allow extensions do their own custom view for certain pages
                                                $outputDone = true;
                                        }
@@ -2496,6 +2498,7 @@ class Article implements Page {
 
        /**
         * Call to WikiPage function for backwards compatibility.
+        * @deprecated since 1.21, use prepareContentForEdit
         * @see WikiPage::prepareTextForEdit
         */
        public function prepareTextForEdit( $text, $revid = null, User $user = null ) {