Merge "WikitextContentHandlerTest expects the messages to be in English."
authorNikerabbit <niklas.laxstrom@gmail.com>
Wed, 31 Oct 2012 07:27:01 +0000 (07:27 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Wed, 31 Oct 2012 07:27:01 +0000 (07:27 +0000)
includes/filebackend/FSFileBackend.php
includes/filebackend/FileBackend.php
includes/filebackend/FileBackendStore.php
includes/filebackend/FileOp.php
includes/filebackend/SwiftFileBackend.php
includes/filerepo/FileRepo.php
includes/job/JobQueueDB.php
includes/specials/SpecialVersion.php
tests/phpunit/includes/filerepo/FileBackendTest.php

index 97abc27..04cf29e 100644 (file)
@@ -250,6 +250,13 @@ class FSFileBackend extends FileBackendStore {
                        return $status;
                }
 
+               if ( !is_file( $source ) ) {
+                       if ( empty( $params['ignoreMissingSource'] ) ) {
+                               $status->fatal( 'backend-fail-copy', $params['src'] );
+                       }
+                       return $status; // do nothing; either OK or bad status
+               }
+
                if ( file_exists( $dest ) ) {
                        $ok = unlink( $dest );
                        if ( !$ok ) {
@@ -310,6 +317,13 @@ class FSFileBackend extends FileBackendStore {
                        return $status;
                }
 
+               if ( !is_file( $source ) ) {
+                       if ( empty( $params['ignoreMissingSource'] ) ) {
+                               $status->fatal( 'backend-fail-move', $params['src'] );
+                       }
+                       return $status; // do nothing; either OK or bad status
+               }
+
                if ( file_exists( $dest ) ) {
                        // Windows does not support moving over existing files
                        if ( wfIsWindows() ) {
index 0ef4cd0..e01bfc2 100644 (file)
@@ -206,6 +206,7 @@ abstract class FileBackend {
         *         'dst'                 => <storage path>,
         *         'overwrite'           => <boolean>,
         *         'overwriteSame'       => <boolean>,
+        *         'ignoreMissingSource' => <boolean>, # since 1.21
         *         'disposition'         => <Content-Disposition header value>
         *     )
         * @endcode
@@ -218,6 +219,7 @@ abstract class FileBackend {
         *         'dst'                 => <storage path>,
         *         'overwrite'           => <boolean>,
         *         'overwriteSame'       => <boolean>,
+        *         'ignoreMissingSource' => <boolean>, # since 1.21
         *         'disposition'         => <Content-Disposition header value>
         *     )
         * @endcode
@@ -427,6 +429,7 @@ abstract class FileBackend {
         *         'op'                  => 'copy',
         *         'src'                 => <storage path>,
         *         'dst'                 => <storage path>,
+        *         'ignoreMissingSource' => <boolean>, # since 1.21
         *         'disposition'         => <Content-Disposition header value>
         *     )
         * @endcode
@@ -436,6 +439,7 @@ abstract class FileBackend {
         *         'op'                  => 'move',
         *         'src'                 => <storage path>,
         *         'dst'                 => <storage path>,
+        *         'ignoreMissingSource' => <boolean>, # since 1.21
         *         'disposition'         => <Content-Disposition header value>
         *     )
         * @endcode
index dd0bec9..cb8be0b 100644 (file)
@@ -161,12 +161,13 @@ abstract class FileBackendStore extends FileBackend {
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
-        *   - src           : source storage path
-        *   - dst           : destination storage path
-        *   - disposition   : Content-Disposition header value for the destination
-        *   - async         : Status will be returned immediately if supported.
-        *                     If the status is OK, then its value field will be
-        *                     set to a FileBackendStoreOpHandle object.
+        *   - src                 : source storage path
+        *   - dst                 : destination storage path
+        *   - ignoreMissingSource : do nothing if the source file does not exist
+        *   - disposition         : Content-Disposition header value for the destination
+        *   - async               : Status will be returned immediately if supported.
+        *                           If the status is OK, then its value field will be
+        *                           set to a FileBackendStoreOpHandle object.
         *
         * @param $params Array
         * @return Status
@@ -223,12 +224,13 @@ abstract class FileBackendStore extends FileBackend {
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
-        *   - src           : source storage path
-        *   - dst           : destination storage path
-        *   - disposition   : Content-Disposition header value for the destination
-        *   - async         : Status will be returned immediately if supported.
-        *                     If the status is OK, then its value field will be
-        *                     set to a FileBackendStoreOpHandle object.
+        *   - src                 : source storage path
+        *   - dst                 : destination storage path
+        *   - ignoreMissingSource : do nothing if the source file does not exist
+        *   - disposition         : Content-Disposition header value for the destination
+        *   - async               : Status will be returned immediately if supported.
+        *                           If the status is OK, then its value field will be
+        *                           set to a FileBackendStoreOpHandle object.
         *
         * @param $params Array
         * @return Status
index 3ac90f2..20dfda2 100644 (file)
@@ -45,6 +45,7 @@ abstract class FileOp {
        protected $useLatest = true; // boolean
        protected $batchId; // string
 
+       protected $doOperation = true; // boolean; operation is not a no-op
        protected $sourceSha1; // string
        protected $destSameAsSource; // boolean
 
@@ -209,6 +210,9 @@ abstract class FileOp {
         * @return Array
         */
        final public function getJournalEntries( array $oPredicates, array $nPredicates ) {
+               if ( !$this->doOperation ) {
+                       return array(); // this is a no-op
+               }
                $nullEntries = array();
                $updateEntries = array();
                $deleteEntries = array();
@@ -239,7 +243,9 @@ abstract class FileOp {
        }
 
        /**
-        * Check preconditions of the operation without writing anything
+        * Check preconditions of the operation without writing anything.
+        * This must update $predicates for each path that the op can change
+        * except when a failing status object is returned.
         *
         * @param $predicates Array
         * @return Status
@@ -275,10 +281,14 @@ abstract class FileOp {
                        return Status::newFatal( 'fileop-fail-attempt-precheck' );
                }
                $this->state = self::STATE_ATTEMPTED;
-               $status = $this->doAttempt();
-               if ( !$status->isOK() ) {
-                       $this->failed = true;
-                       $this->logFailure( 'attempt' );
+               if ( $this->doOperation ) {
+                       $status = $this->doAttempt();
+                       if ( !$status->isOK() ) {
+                               $this->failed = true;
+                               $this->logFailure( 'attempt' );
+                       }
+               } else { // no-op
+                       $status = Status::newGood();
                }
                return $status;
        }
@@ -414,6 +424,8 @@ abstract class FileOp {
        final protected function fileSha1( $source, array $predicates ) {
                if ( isset( $predicates['sha1'][$source] ) ) {
                        return $predicates['sha1'][$source]; // previous op assures this
+               } elseif ( isset( $predicates['exists'][$source] ) && !$predicates['exists'][$source] ) {
+                       return false; // previous op assures this
                } else {
                        $params = array( 'src' => $source, 'latest' => $this->useLatest );
                        return $this->backend->getFileSha1Base36( $params );
@@ -591,7 +603,7 @@ class CopyFileOp extends FileOp {
         */
        protected function allowedParams() {
                return array( array( 'src', 'dst' ),
-                       array( 'overwrite', 'overwriteSame', 'disposition' ) );
+                       array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'disposition' ) );
        }
 
        /**
@@ -602,8 +614,16 @@ class CopyFileOp extends FileOp {
                $status = Status::newGood();
                // Check if the source file exists
                if ( !$this->fileExists( $this->params['src'], $predicates ) ) {
-                       $status->fatal( 'backend-fail-notexists', $this->params['src'] );
-                       return $status;
+                       if ( $this->getParam( 'ignoreMissingSource' ) ) {
+                               $this->doOperation = false; // no-op
+                               // Update file existence predicates (cache 404s)
+                               $predicates['exists'][$this->params['src']] = false;
+                               $predicates['sha1'][$this->params['src']] = false;
+                               return $status; // nothing to do
+                       } else {
+                               $status->fatal( 'backend-fail-notexists', $this->params['src'] );
+                               return $status;
+                       }
                // Check if a file can be placed at the destination
                } elseif ( !$this->backend->isPathUsableInternal( $this->params['dst'] ) ) {
                        $status->fatal( 'backend-fail-usable', $this->params['dst'] );
@@ -659,7 +679,7 @@ class MoveFileOp extends FileOp {
         */
        protected function allowedParams() {
                return array( array( 'src', 'dst' ),
-                       array( 'overwrite', 'overwriteSame', 'disposition' ) );
+                       array( 'overwrite', 'overwriteSame', 'ignoreMissingSource', 'disposition' ) );
        }
 
        /**
@@ -670,8 +690,16 @@ class MoveFileOp extends FileOp {
                $status = Status::newGood();
                // Check if the source file exists
                if ( !$this->fileExists( $this->params['src'], $predicates ) ) {
-                       $status->fatal( 'backend-fail-notexists', $this->params['src'] );
-                       return $status;
+                       if ( $this->getParam( 'ignoreMissingSource' ) ) {
+                               $this->doOperation = false; // no-op
+                               // Update file existence predicates (cache 404s)
+                               $predicates['exists'][$this->params['src']] = false;
+                               $predicates['sha1'][$this->params['src']] = false;
+                               return $status; // nothing to do
+                       } else {
+                               $status->fatal( 'backend-fail-notexists', $this->params['src'] );
+                               return $status;
+                       }
                // Check if a file can be placed at the destination
                } elseif ( !$this->backend->isPathUsableInternal( $this->params['dst'] ) ) {
                        $status->fatal( 'backend-fail-usable', $this->params['dst'] );
@@ -735,21 +763,24 @@ class DeleteFileOp extends FileOp {
                return array( array( 'src' ), array( 'ignoreMissingSource' ) );
        }
 
-       protected $needsDelete = true;
-
        /**
-        * @param array $predicates
+        * @param $predicates array
         * @return Status
         */
        protected function doPrecheck( array &$predicates ) {
                $status = Status::newGood();
                // Check if the source file exists
                if ( !$this->fileExists( $this->params['src'], $predicates ) ) {
-                       if ( !$this->getParam( 'ignoreMissingSource' ) ) {
+                       if ( $this->getParam( 'ignoreMissingSource' ) ) {
+                               $this->doOperation = false; // no-op
+                               // Update file existence predicates (cache 404s)
+                               $predicates['exists'][$this->params['src']] = false;
+                               $predicates['sha1'][$this->params['src']] = false;
+                               return $status; // nothing to do
+                       } else {
                                $status->fatal( 'backend-fail-notexists', $this->params['src'] );
                                return $status;
                        }
-                       $this->needsDelete = false;
                }
                // Update file existence predicates
                $predicates['exists'][$this->params['src']] = false;
@@ -761,11 +792,8 @@ class DeleteFileOp extends FileOp {
         * @return Status
         */
        protected function doAttempt() {
-               if ( $this->needsDelete ) {
-                       // Delete the source file
-                       return $this->backend->deleteInternal( $this->setFlags( $this->params ) );
-               }
-               return Status::newGood();
+               // Delete the source file
+               return $this->backend->deleteInternal( $this->setFlags( $this->params ) );
        }
 
        /**
index 5dfad9e..8441f8f 100644 (file)
@@ -400,7 +400,9 @@ class SwiftFileBackend extends FileBackendStore {
                } catch ( CDNNotEnabledException $e ) {
                        // CDN not enabled; nothing to see here
                } catch ( NoSuchObjectException $e ) { // source object does not exist
-                       $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
+                       if ( empty( $params['ignoreMissingSource'] ) ) {
+                               $status->fatal( 'backend-fail-copy', $params['src'], $params['dst'] );
+                       }
                } catch ( CloudFilesException $e ) { // some other exception?
                        $this->handleException( $e, $status, __METHOD__, $params );
                }
@@ -471,7 +473,9 @@ class SwiftFileBackend extends FileBackendStore {
                } catch ( CDNNotEnabledException $e ) {
                        // CDN not enabled; nothing to see here
                } catch ( NoSuchObjectException $e ) { // source object does not exist
-                       $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] );
+                       if ( empty( $params['ignoreMissingSource'] ) ) {
+                               $status->fatal( 'backend-fail-move', $params['src'], $params['dst'] );
+                       }
                } catch ( CloudFilesException $e ) { // some other exception?
                        $this->handleException( $e, $status, __METHOD__, $params );
                }
index e8aa5a6..6cd93cc 100644 (file)
@@ -1094,47 +1094,42 @@ class FileRepo {
                                return $this->newFatal( 'directorycreateerror', $archiveDir );
                        }
 
-                       // Archive destination file if it exists
-                       if ( $backend->fileExists( array( 'src' => $dstPath ) ) ) {
-                               // Check if the archive file exists
-                               // This is a sanity check to avoid data loss. In UNIX, the rename primitive
-                               // unlinks the destination file if it exists. DB-based synchronisation in
-                               // publishBatch's caller should prevent races. In Windows there's no
-                               // problem because the rename primitive fails if the destination exists.
-                               if ( $backend->fileExists( array( 'src' => $archivePath ) ) ) {
-                                       $operations[] = array( 'op' => 'null' );
-                                       continue;
-                               } else {
-                                       $operations[] = array(
-                                               'op'           => 'move',
-                                               'src'          => $dstPath,
-                                               'dst'          => $archivePath
-                                       );
-                               }
-                               $status->value[$i] = 'archived';
-                       } else {
-                               $status->value[$i] = 'new';
-                       }
+                       // Archive destination file if it exists.
+                       // This will check if the archive file also exists and fail if does.
+                       // This is a sanity check to avoid data loss. On Windows and Linux,
+                       // copy() will overwrite, so the existence check is vulnerable to
+                       // race conditions unless an functioning LockManager is used.
+                       // LocalFile also uses SELECT FOR UPDATE for synchronization.
+                       $operations[] = array(
+                               'op'                  => 'copy',
+                               'src'                 => $dstPath,
+                               'dst'                 => $archivePath,
+                               'ignoreMissingSource' => true
+                       );
+
                        // Copy (or move) the source file to the destination
                        if ( FileBackend::isStoragePath( $srcPath ) ) {
                                if ( $flags & self::DELETE_SOURCE ) {
                                        $operations[] = array(
                                                'op'           => 'move',
                                                'src'          => $srcPath,
-                                               'dst'          => $dstPath
+                                               'dst'          => $dstPath,
+                                               'overwrite'    => true // replace current
                                        );
                                } else {
                                        $operations[] = array(
                                                'op'           => 'copy',
                                                'src'          => $srcPath,
-                                               'dst'          => $dstPath
+                                               'dst'          => $dstPath,
+                                               'overwrite'    => true // replace current
                                        );
                                }
                        } else { // FS source path
                                $operations[] = array(
                                        'op'           => 'store',
                                        'src'          => $srcPath,
-                                       'dst'          => $dstPath
+                                       'dst'          => $dstPath,
+                                       'overwrite'    => true // replace current
                                );
                                if ( $flags & self::DELETE_SOURCE ) {
                                        $sourceFSFilesToDelete[] = $srcPath;
@@ -1143,8 +1138,17 @@ class FileRepo {
                }
 
                // Execute the operations for each triplet
-               $opts = array( 'force' => true );
-               $status->merge( $backend->doOperations( $operations, $opts ) );
+               $status->merge( $backend->doOperations( $operations ) );
+               // Find out which files were archived...
+               foreach ( $triplets as $i => $triplet ) {
+                       list( $srcPath, $dstRel, $archiveRel ) = $triplet;
+                       $archivePath = $this->getZonePath( 'public' ) . "/$archiveRel";
+                       if ( $this->fileExists( $archivePath ) ) {
+                               $status->value[$i] = 'archived';
+                       } else {
+                               $status->value[$i] = 'new';
+                       }
+               }
                // Cleanup for disk source files...
                foreach ( $sourceFSFilesToDelete as $file ) {
                        wfSuppressWarnings();
index f6003b2..d9465fb 100644 (file)
@@ -111,11 +111,14 @@ class JobQueueDB extends JobQueue {
        protected function doPop() {
                global $wgMemc;
 
-               $uuid = wfRandomString( 32 ); // pop attempt
+               if ( $wgMemc->get( $this->getEmptinessCacheKey() ) === 'true' ) {
+                       return false; // queue is empty
+               }
 
                $dbw = $this->getMasterDB();
                $dbw->commit( __METHOD__, 'flush' ); // flush existing transaction
 
+               $uuid = wfRandomString( 32 ); // pop attempt
                $job = false; // job popped off
                $autoTrx = $dbw->getFlag( DBO_TRX ); // automatic begin() enabled?
                $dbw->clearFlag( DBO_TRX ); // make each query its own transaction
index 87ea943..e0c6d12 100644 (file)
@@ -775,11 +775,23 @@ class SpecialVersion extends SpecialPage {
                        'version-entrypoints-load-php' => wfScript( 'load' ),
                );
 
+               $language = $this->getLanguage();
+               $thAttribures = array(
+                       'dir' => $language->getDir(),
+                       'lang' => $language->getCode()
+               );
                $out = Html::element( 'h2', array( 'id' => 'mw-version-entrypoints' ), $this->msg( 'version-entrypoints' )->text() ) .
-                       Html::openElement( 'table', array( 'class' => 'wikitable plainlinks', 'id' => 'mw-version-entrypoints-table' ) ) .
+                       Html::openElement( 'table',
+                               array(
+                                       'class' => 'wikitable plainlinks',
+                                       'id' => 'mw-version-entrypoints-table',
+                                       'dir' => 'ltr',
+                                       'lang' => 'en'
+                               )
+                       ) .
                        Html::openElement( 'tr' ) .
-                       Html::element( 'th', array(), $this->msg( 'version-entrypoints-header-entrypoint' )->text() ) .
-                       Html::element( 'th', array(), $this->msg( 'version-entrypoints-header-url' )->text() ) .
+                       Html::element( 'th', $thAttribures, $this->msg( 'version-entrypoints-header-entrypoint' )->text() ) .
+                       Html::element( 'th', $thAttribures, $this->msg( 'version-entrypoints-header-url' )->text() ) .
                        Html::closeElement( 'tr' );
 
                foreach ( $entryPoints as $message => $value ) {
index 7201eec..da36e90 100644 (file)
@@ -302,6 +302,19 @@ class FileBackendTest extends MediaWikiTestCase {
                $this->prepare( array( 'dir' => dirname( $source ) ) );
                $this->prepare( array( 'dir' => dirname( $dest ) ) );
 
+               if ( isset( $op['ignoreMissingSource'] ) ) {
+                       $status = $this->backend->doOperation( $op );
+                       $this->assertGoodStatus( $status,
+                               "Move from $source to $dest succeeded without warnings ($backendName)." );
+                       $this->assertEquals( array( 0 => true ), $status->success,
+                               "Move from $source to $dest has proper 'success' field in Status ($backendName)." );
+                       $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $source ) ),
+                               "Source file $source does not exist ($backendName)." );
+                       $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $dest ) ),
+                               "Destination file $dest does not exist ($backendName)." );
+                       return; // done
+               }
+
                $status = $this->backend->doOperation(
                        array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) );
                $this->assertGoodStatus( $status,
@@ -366,6 +379,14 @@ class FileBackendTest extends MediaWikiTestCase {
                        $dest, // dest
                );
 
+               $op2 = $op;
+               $op2['ignoreMissingSource'] = true;
+               $cases[] = array(
+                       $op2, // operation
+                       $source, // source
+                       $dest, // dest
+               );
+
                return $cases;
        }
 
@@ -392,6 +413,19 @@ class FileBackendTest extends MediaWikiTestCase {
                $this->prepare( array( 'dir' => dirname( $source ) ) );
                $this->prepare( array( 'dir' => dirname( $dest ) ) );
 
+               if ( isset( $op['ignoreMissingSource'] ) ) {
+                       $status = $this->backend->doOperation( $op );
+                       $this->assertGoodStatus( $status,
+                               "Move from $source to $dest succeeded without warnings ($backendName)." );
+                       $this->assertEquals( array( 0 => true ), $status->success,
+                               "Move from $source to $dest has proper 'success' field in Status ($backendName)." );
+                       $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $source ) ),
+                               "Source file $source does not exist ($backendName)." );
+                       $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $dest ) ),
+                               "Destination file $dest does not exist ($backendName)." );
+                       return; // done
+               }
+
                $status = $this->backend->doOperation(
                        array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) );
                $this->assertGoodStatus( $status,
@@ -457,6 +491,14 @@ class FileBackendTest extends MediaWikiTestCase {
                        $dest, // dest
                );
 
+               $op2 = $op;
+               $op2['ignoreMissingSource'] = true;
+               $cases[] = array(
+                       $op2, // operation
+                       $source, // source
+                       $dest, // dest
+               );
+
                return $cases;
        }