Merge "Revert "Adding sanity check to Title::isRedirect().""
[lhc/web/wiklou.git] / includes / filerepo / backend / FileOp.php
index 312a531..d9fbafd 100644 (file)
@@ -1,17 +1,35 @@
 <?php
 /**
+ * Helper class for representing operations with transaction support.
+ *
+ * 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
  * @ingroup FileBackend
  * @author Aaron Schulz
  */
 
 /**
- * Helper class for representing operations with transaction support.
+ * FileBackend helper class for representing operations.
  * Do not use this class from places outside FileBackend.
  *
- * Methods called from attemptBatch() should avoid throwing exceptions at all costs.
- * FileOp objects should be lightweight in order to support large arrays in memory.
- * 
+ * Methods called from FileOpBatch::attempt() should avoid throwing
+ * exceptions at all costs. FileOp objects should be lightweight in order
+ * to support large arrays in memory and serialization.
+ *
  * @ingroup FileBackend
  * @since 1.19
  */
@@ -23,7 +41,9 @@ abstract class FileOp {
 
        protected $state = self::STATE_NEW; // integer
        protected $failed = false; // boolean
+       protected $async = false; // boolean
        protected $useLatest = true; // boolean
+       protected $batchId; // string
 
        protected $sourceSha1; // string
        protected $destSameAsSource; // boolean
@@ -33,10 +53,6 @@ abstract class FileOp {
        const STATE_CHECKED = 2;
        const STATE_ATTEMPTED = 3;
 
-       /* Timeout related parameters */
-       const MAX_BATCH_SIZE = 1000;
-       const TIME_LIMIT_SEC = 300; // 5 minutes
-
        /**
         * Build a new file operation transaction
         *
@@ -63,101 +79,28 @@ abstract class FileOp {
        }
 
        /**
-        * Allow stale data for file reads and existence checks
+        * Set the batch UUID this operation belongs to
         *
+        * @param $batchId string
         * @return void
         */
-       final protected function allowStaleReads() {
-               $this->useLatest = false;
+       final public function setBatchId( $batchId ) {
+               $this->batchId = $batchId;
        }
 
        /**
-        * Attempt a series of file operations.
-        * Callers are responsible for handling file locking.
-        * 
-        * $opts is an array of options, including:
-        * 'force'      : Errors that would normally cause a rollback do not.
-        *                The remaining operations are still attempted if any fail.
-        * 'allowStale' : Don't require the latest available data.
-        *                This can increase performance for non-critical writes.
-        *                This has no effect unless the 'force' flag is set.
+        * Whether to allow stale data for file reads and stat checks
         *
-        * The resulting Status will be "OK" unless:
-        *     a) unexpected operation errors occurred (network partitions, disk full...)
-        *     b) significant operation errors occured and 'force' was not set
-        * 
-        * @param $performOps Array List of FileOp operations
-        * @param $opts Array Batch operation options
-        * @return Status 
-        */
-       final public static function attemptBatch( array $performOps, array $opts ) {
-               $status = Status::newGood();
-
-               $allowStale = !empty( $opts['allowStale'] );
-               $ignoreErrors = !empty( $opts['force'] );
-
-               $n = count( $performOps );
-               if ( $n > self::MAX_BATCH_SIZE ) {
-                       $status->fatal( 'backend-fail-batchsize', $n, self::MAX_BATCH_SIZE );
-                       return $status;
-               }
-
-               $predicates = FileOp::newPredicates(); // account for previous op in prechecks
-               // Do pre-checks for each operation; abort on failure...
-               foreach ( $performOps as $index => $fileOp ) {
-                       if ( $allowStale ) {
-                               $fileOp->allowStaleReads(); // allow potentially stale reads
-                       }
-                       $subStatus = $fileOp->precheck( $predicates );
-                       $status->merge( $subStatus );
-                       if ( !$subStatus->isOK() ) { // operation failed?
-                               $status->success[$index] = false;
-                               ++$status->failCount;
-                               if ( !$ignoreErrors ) {
-                                       return $status; // abort
-                               }
-                       }
-               }
-
-               if ( $ignoreErrors ) {
-                       # Treat all precheck() fatals as merely warnings
-                       $status->setResult( true, $status->value );
-               }
-
-               // Restart PHP's execution timer and set the timeout to safe amount.
-               // This handles cases where the operations take a long time or where we are
-               // already running low on time left. The old timeout is restored afterwards.
-               # @TODO: re-enable this for when the number of batches is high.
-               #$scopedTimeLimit = new FileOpScopedPHPTimeout( self::TIME_LIMIT_SEC );
-
-               // Attempt each operation...
-               foreach ( $performOps as $index => $fileOp ) {
-                       if ( $fileOp->failed() ) {
-                               continue; // nothing to do
-                       }
-                       $subStatus = $fileOp->attempt();
-                       $status->merge( $subStatus );
-                       if ( $subStatus->isOK() ) {
-                               $status->success[$index] = true;
-                               ++$status->successCount;
-                       } else {
-                               $status->success[$index] = false;
-                               ++$status->failCount;
-                               // We can't continue (even with $ignoreErrors) as $predicates is wrong.
-                               // Log the remaining ops as failed for recovery...
-                               for ( $i = ($index + 1); $i < count( $performOps ); $i++ ) {
-                                       $performOps[$i]->logFailure( 'attempt_aborted' );
-                               }
-                               return $status; // bail out
-                       }
-               }
-
-               return $status;
+        * @param $allowStale bool
+        * @return void
+        */
+       final public function allowStaleReads( $allowStale ) {
+               $this->useLatest = !$allowStale;
        }
 
        /**
         * Get the value of the parameter with the given name
-        * 
+        *
         * @param $name string
         * @return mixed Returns null if the parameter is not set
         */
@@ -167,8 +110,8 @@ abstract class FileOp {
 
        /**
         * Check if this operation failed precheck() or attempt()
-        * 
-        * @return bool 
+        *
+        * @return bool
         */
        final public function failed() {
                return $this->failed;
@@ -177,12 +120,90 @@ abstract class FileOp {
        /**
         * Get a new empty predicates array for precheck()
         *
-        * @return Array 
+        * @return Array
         */
        final public static function newPredicates() {
                return array( 'exists' => array(), 'sha1' => array() );
        }
 
+       /**
+        * Get a new empty dependency tracking array for paths read/written to
+        *
+        * @return Array
+        */
+       final public static function newDependencies() {
+               return array( 'read' => array(), 'write' => array() );
+       }
+
+       /**
+        * Update a dependency tracking array to account for this operation
+        *
+        * @param $deps Array Prior path reads/writes; format of FileOp::newPredicates()
+        * @return Array
+        */
+       final public function applyDependencies( array $deps ) {
+               $deps['read']  += array_fill_keys( $this->storagePathsRead(), 1 );
+               $deps['write'] += array_fill_keys( $this->storagePathsChanged(), 1 );
+               return $deps;
+       }
+
+       /**
+        * Check if this operation changes files listed in $paths
+        *
+        * @param $paths Array Prior path reads/writes; format of FileOp::newPredicates()
+        * @return boolean
+        */
+       final public function dependsOn( array $deps ) {
+               foreach ( $this->storagePathsChanged() as $path ) {
+                       if ( isset( $deps['read'][$path] ) || isset( $deps['write'][$path] ) ) {
+                               return true; // "output" or "anti" dependency
+                       }
+               }
+               foreach ( $this->storagePathsRead() as $path ) {
+                       if ( isset( $deps['write'][$path] ) ) {
+                               return true; // "flow" dependency
+                       }
+               }
+               return false;
+       }
+
+       /**
+        * Get the file journal entries for this file operation
+        *
+        * @param $oPredicates Array Pre-op info about files (format of FileOp::newPredicates)
+        * @param $nPredicates Array Post-op info about files (format of FileOp::newPredicates)
+        * @return Array
+        */
+       final public function getJournalEntries( array $oPredicates, array $nPredicates ) {
+               $nullEntries = array();
+               $updateEntries = array();
+               $deleteEntries = array();
+               $pathsUsed = array_merge( $this->storagePathsRead(), $this->storagePathsChanged() );
+               foreach ( $pathsUsed as $path ) {
+                       $nullEntries[] = array( // assertion for recovery
+                               'op'      => 'null',
+                               'path'    => $path,
+                               'newSha1' => $this->fileSha1( $path, $oPredicates )
+                       );
+               }
+               foreach ( $this->storagePathsChanged() as $path ) {
+                       if ( $nPredicates['sha1'][$path] === false ) { // deleted
+                               $deleteEntries[] = array(
+                                       'op'      => 'delete',
+                                       'path'    => $path,
+                                       'newSha1' => ''
+                               );
+                       } else { // created/updated
+                               $updateEntries[] = array(
+                                       'op'      => $this->fileExists( $path, $oPredicates ) ? 'update' : 'create',
+                                       'path'    => $path,
+                                       'newSha1' => $nPredicates['sha1'][$path]
+                               );
+                       }
+               }
+               return array_merge( $nullEntries, $updateEntries, $deleteEntries );
+       }
+
        /**
         * Check preconditions of the operation without writing anything
         *
@@ -202,7 +223,14 @@ abstract class FileOp {
        }
 
        /**
-        * Attempt the operation, backing up files as needed; this must be reversible
+        * @return Status
+        */
+       protected function doPrecheck( array &$predicates ) {
+               return Status::newGood();
+       }
+
+       /**
+        * Attempt the operation
         *
         * @return Status
         */
@@ -221,52 +249,83 @@ abstract class FileOp {
                return $status;
        }
 
+       /**
+        * @return Status
+        */
+       protected function doAttempt() {
+               return Status::newGood();
+       }
+
+       /**
+        * Attempt the operation in the background
+        *
+        * @return Status
+        */
+       final public function attemptAsync() {
+               $this->async = true;
+               $result = $this->attempt();
+               $this->async = false;
+               return $result;
+       }
+
        /**
         * Get the file operation parameters
-        * 
+        *
         * @return Array (required params list, optional params list)
         */
        protected function allowedParams() {
                return array( array(), array() );
        }
 
+       /**
+        * Adjust params to FileBackendStore internal file calls
+        *
+        * @param $params Array
+        * @return Array (required params list, optional params list)
+        */
+       protected function setFlags( array $params ) {
+               return array( 'async' => $this->async ) + $params;
+       }
+
        /**
         * Get a list of storage paths read from for this operation
         *
         * @return Array
         */
-       public function storagePathsRead() {
-               return array();
+       final public function storagePathsRead() {
+               return array_map( 'FileBackend::normalizeStoragePath', $this->doStoragePathsRead() );
        }
 
        /**
-        * Get a list of storage paths written to for this operation
-        *
+        * @see FileOp::storagePathsRead()
         * @return Array
         */
-       public function storagePathsChanged() {
+       protected function doStoragePathsRead() {
                return array();
        }
 
        /**
-        * @return Status
+        * Get a list of storage paths written to for this operation
+        *
+        * @return Array
         */
-       protected function doPrecheck( array &$predicates ) {
-               return Status::newGood();
+       final public function storagePathsChanged() {
+               return array_map( 'FileBackend::normalizeStoragePath', $this->doStoragePathsChanged() );
        }
 
        /**
-        * @return Status
+        * @see FileOp::storagePathsChanged()
+        * @return Array
         */
-       protected function doAttempt() {
-               return Status::newGood();
+       protected function doStoragePathsChanged() {
+               return array();
        }
 
        /**
         * Check for errors with regards to the destination file already existing.
         * This also updates the destSameAsSource and sourceSha1 member variables.
         * A bad status will be returned if there is no chance it can be overwritten.
-        * 
+        *
         * @param $predicates Array
         * @return Status
         */
@@ -313,10 +372,10 @@ abstract class FileOp {
 
        /**
         * Check if a file will exist in storage when this operation is attempted
-        * 
+        *
         * @param $source string Storage path
         * @param $predicates Array
-        * @return bool 
+        * @return bool
         */
        final protected function fileExists( $source, array $predicates ) {
                if ( isset( $predicates['exists'][$source] ) ) {
@@ -329,7 +388,7 @@ abstract class FileOp {
 
        /**
         * Get the SHA-1 of a file in storage when this operation is attempted
-        * 
+        *
         * @param $source string Storage path
         * @param $predicates Array
         * @return string|bool False on failure
@@ -343,81 +402,33 @@ abstract class FileOp {
                }
        }
 
+       /**
+        * Get the backend this operation is for
+        *
+        * @return FileBackendStore
+        */
+       public function getBackend() {
+               return $this->backend;
+       }
+
        /**
         * Log a file operation failure and preserve any temp files
-        * 
+        *
         * @param $action string
         * @return void
         */
-       final protected function logFailure( $action ) {
+       final public function logFailure( $action ) {
                $params = $this->params;
                $params['failedAction'] = $action;
                try {
-                       wfDebugLog( 'FileOperation',
-                               get_class( $this ) . ' failed: ' . FormatJson::encode( $params ) );
+                       wfDebugLog( 'FileOperation', get_class( $this ) .
+                               " failed (batch #{$this->batchId}): " . FormatJson::encode( $params ) );
                } catch ( Exception $e ) {
                        // bad config? debug log error?
                }
        }
 }
 
-/**
- * FileOp helper class to expand PHP execution time for a function.
- * On construction, set_time_limit() is called and set to $seconds.
- * When the object goes out of scope, the timer is restarted, with
- * the original time limit minus the time the object existed.
- */
-class FileOpScopedPHPTimeout {
-       protected $startTime; // float; seconds
-       protected $oldTimeout; // integer; seconds
-
-       protected static $stackDepth = 0; // integer
-       protected static $totalCalls = 0; // integer
-       protected static $totalElapsed = 0; // float; seconds
-
-       /* Prevent callers in infinite loops from running forever */
-       const MAX_TOTAL_CALLS = 1000000;
-       const MAX_TOTAL_TIME = 300; // seconds
-
-       /**
-        * @param $seconds integer
-        */
-       public function __construct( $seconds ) {
-               if ( ini_get( 'max_execution_time' ) > 0 ) { // CLI uses 0
-                       if ( self::$totalCalls >= self::MAX_TOTAL_CALLS ) {
-                               trigger_error( "Maximum invocations of " . __CLASS__ . " exceeded." );
-                       } elseif ( self::$totalElapsed >= self::MAX_TOTAL_TIME ) {
-                               trigger_error( "Time limit within invocations of " . __CLASS__ . " exceeded." );
-                       } elseif ( self::$stackDepth > 0 ) { // recursion guard
-                               trigger_error( "Resursive invocation of " . __CLASS__ . " attempted." );
-                       } else {
-                               $this->oldTimeout = ini_set( 'max_execution_time', $seconds );
-                               $this->startTime = microtime( true );
-                               ++self::$stackDepth;
-                               ++self::$totalCalls; // proof against < 1us scopes
-                       }
-               }
-       }
-
-       /**
-        * Restore the original timeout.
-        * This does not account for the timer value on __construct().
-        */
-       public function __destruct() {
-               if ( $this->oldTimeout ) {
-                       $elapsed = microtime( true ) - $this->startTime;
-                       // Note: a limit of 0 is treated as "forever"
-                       set_time_limit( max( 1, $this->oldTimeout - (int)$elapsed ) );
-                       // If each scoped timeout is for less than one second, we end up
-                       // restoring the original timeout without any decrease in value.
-                       // Thus web scripts in an infinite loop can run forever unless we 
-                       // take some measures to prevent this. Track total time and calls.
-                       self::$totalElapsed += $elapsed;
-                       --self::$stackDepth;
-               }
-       }
-}
-
 /**
  * Store a file into the backend from a file on the file system.
  * Parameters similar to FileBackendStore::storeInternal(), which include:
@@ -427,10 +438,18 @@ class FileOpScopedPHPTimeout {
  *     overwriteSame : override any existing file at destination
  */
 class StoreFileOp extends FileOp {
+
+       /**
+        * @return array
+        */
        protected function allowedParams() {
                return array( array( 'src', 'dst' ), array( 'overwrite', 'overwriteSame' ) );
        }
 
+       /**
+        * @param $predicates array
+        * @return Status
+        */
        protected function doPrecheck( array &$predicates ) {
                $status = Status::newGood();
                // Check if the source file exists on the file system
@@ -439,10 +458,13 @@ class StoreFileOp extends FileOp {
                        return $status;
                // Check if the source file is too big
                } elseif ( filesize( $this->params['src'] ) > $this->backend->maxFileSizeInternal() ) {
+                       $status->fatal( 'backend-fail-maxsize',
+                               $this->params['dst'], $this->backend->maxFileSizeInternal() );
                        $status->fatal( 'backend-fail-store', $this->params['src'], $this->params['dst'] );
                        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'] );
                        $status->fatal( 'backend-fail-store', $this->params['src'], $this->params['dst'] );
                        return $status;
                }
@@ -456,15 +478,20 @@ class StoreFileOp extends FileOp {
                return $status; // safe to call attempt()
        }
 
+       /**
+        * @return Status
+        */
        protected function doAttempt() {
-               $status = Status::newGood();
                // Store the file at the destination
                if ( !$this->destSameAsSource ) {
-                       $status->merge( $this->backend->storeInternal( $this->params ) );
+                       return $this->backend->storeInternal( $this->setFlags( $this->params ) );
                }
-               return $status;
+               return Status::newGood();
        }
 
+       /**
+        * @return bool|string
+        */
        protected function getSourceSha1Base36() {
                wfSuppressWarnings();
                $hash = sha1_file( $this->params['src'] );
@@ -475,7 +502,7 @@ class StoreFileOp extends FileOp {
                return $hash;
        }
 
-       public function storagePathsChanged() {
+       protected function doStoragePathsChanged() {
                return array( $this->params['dst'] );
        }
 }
@@ -497,10 +524,13 @@ class CreateFileOp extends FileOp {
                $status = Status::newGood();
                // Check if the source data is too big
                if ( strlen( $this->getParam( 'content' ) ) > $this->backend->maxFileSizeInternal() ) {
+                       $status->fatal( 'backend-fail-maxsize',
+                               $this->params['dst'], $this->backend->maxFileSizeInternal() );
                        $status->fatal( 'backend-fail-create', $this->params['dst'] );
                        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'] );
                        $status->fatal( 'backend-fail-create', $this->params['dst'] );
                        return $status;
                }
@@ -514,20 +544,28 @@ class CreateFileOp extends FileOp {
                return $status; // safe to call attempt()
        }
 
+       /**
+        * @return Status
+        */
        protected function doAttempt() {
-               $status = Status::newGood();
-               // Create the file at the destination
                if ( !$this->destSameAsSource ) {
-                       $status->merge( $this->backend->createInternal( $this->params ) );
+                       // Create the file at the destination
+                       return $this->backend->createInternal( $this->setFlags( $this->params ) );
                }
-               return $status;
+               return Status::newGood();
        }
 
+       /**
+        * @return bool|String
+        */
        protected function getSourceSha1Base36() {
                return wfBaseConvert( sha1( $this->params['content'] ), 16, 36, 31 );
        }
 
-       public function storagePathsChanged() {
+       /**
+        * @return array
+        */
+       protected function doStoragePathsChanged() {
                return array( $this->params['dst'] );
        }
 }
@@ -541,10 +579,18 @@ class CreateFileOp extends FileOp {
  *     overwriteSame : override any existing file at destination
  */
 class CopyFileOp extends FileOp {
+
+       /**
+        * @return array
+        */
        protected function allowedParams() {
                return array( array( 'src', 'dst' ), array( 'overwrite', 'overwriteSame' ) );
        }
 
+       /**
+        * @param $predicates array
+        * @return Status
+        */
        protected function doPrecheck( array &$predicates ) {
                $status = Status::newGood();
                // Check if the source file exists
@@ -553,6 +599,7 @@ class CopyFileOp extends FileOp {
                        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'] );
                        $status->fatal( 'backend-fail-copy', $this->params['src'], $this->params['dst'] );
                        return $status;
                }
@@ -566,23 +613,31 @@ class CopyFileOp extends FileOp {
                return $status; // safe to call attempt()
        }
 
+       /**
+        * @return Status
+        */
        protected function doAttempt() {
-               $status = Status::newGood();
                // Do nothing if the src/dst paths are the same
                if ( $this->params['src'] !== $this->params['dst'] ) {
                        // Copy the file into the destination
                        if ( !$this->destSameAsSource ) {
-                               $status->merge( $this->backend->copyInternal( $this->params ) );
+                               return $this->backend->copyInternal( $this->setFlags( $this->params ) );
                        }
                }
-               return $status;
+               return Status::newGood();
        }
 
-       public function storagePathsRead() {
+       /**
+        * @return array
+        */
+       protected function doStoragePathsRead() {
                return array( $this->params['src'] );
        }
 
-       public function storagePathsChanged() {
+       /**
+        * @return array
+        */
+       protected function doStoragePathsChanged() {
                return array( $this->params['dst'] );
        }
 }
@@ -596,10 +651,17 @@ class CopyFileOp extends FileOp {
  *     overwriteSame : override any existing file at destination
  */
 class MoveFileOp extends FileOp {
+       /**
+        * @return array
+        */
        protected function allowedParams() {
                return array( array( 'src', 'dst' ), array( 'overwrite', 'overwriteSame' ) );
        }
 
+       /**
+        * @param $predicates array
+        * @return Status
+        */
        protected function doPrecheck( array &$predicates ) {
                $status = Status::newGood();
                // Check if the source file exists
@@ -608,6 +670,7 @@ class MoveFileOp extends FileOp {
                        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'] );
                        $status->fatal( 'backend-fail-move', $this->params['src'], $this->params['dst'] );
                        return $status;
                }
@@ -623,28 +686,36 @@ class MoveFileOp extends FileOp {
                return $status; // safe to call attempt()
        }
 
+       /**
+        * @return Status
+        */
        protected function doAttempt() {
-               $status = Status::newGood();
                // Do nothing if the src/dst paths are the same
                if ( $this->params['src'] !== $this->params['dst'] ) {
                        if ( !$this->destSameAsSource ) {
                                // Move the file into the destination
-                               $status->merge( $this->backend->moveInternal( $this->params ) );
+                               return $this->backend->moveInternal( $this->setFlags( $this->params ) );
                        } else {
                                // Just delete source as the destination needs no changes
                                $params = array( 'src' => $this->params['src'] );
-                               $status->merge( $this->backend->deleteInternal( $params ) );
+                               return $this->backend->deleteInternal( $this->setFlags( $params ) );
                        }
                }
-               return $status;
+               return Status::newGood();
        }
 
-       public function storagePathsRead() {
+       /**
+        * @return array
+        */
+       protected function doStoragePathsRead() {
                return array( $this->params['src'] );
        }
 
-       public function storagePathsChanged() {
-               return array( $this->params['dst'] );
+       /**
+        * @return array
+        */
+       protected function doStoragePathsChanged() {
+               return array( $this->params['src'], $this->params['dst'] );
        }
 }
 
@@ -655,12 +726,19 @@ class MoveFileOp extends FileOp {
  *     ignoreMissingSource : don't return an error if the file does not exist
  */
 class DeleteFileOp extends FileOp {
+       /**
+        * @return array
+        */
        protected function allowedParams() {
                return array( array( 'src' ), array( 'ignoreMissingSource' ) );
        }
 
        protected $needsDelete = true;
 
+       /**
+        * @param array $predicates
+        * @return Status
+        */
        protected function doPrecheck( array &$predicates ) {
                $status = Status::newGood();
                // Check if the source file exists
@@ -677,16 +755,21 @@ class DeleteFileOp extends FileOp {
                return $status; // safe to call attempt()
        }
 
+       /**
+        * @return Status
+        */
        protected function doAttempt() {
-               $status = Status::newGood();
                if ( $this->needsDelete ) {
                        // Delete the source file
-                       $status->merge( $this->backend->deleteInternal( $this->params ) );
+                       return $this->backend->deleteInternal( $this->setFlags( $this->params ) );
                }
-               return $status;
+               return Status::newGood();
        }
 
-       public function storagePathsChanged() {
+       /**
+        * @return array
+        */
+       protected function doStoragePathsChanged() {
                return array( $this->params['src'] );
        }
 }