* Fixed 'success' value of doOperations() Status to match documentation.

* Made 'success', 'successCount', and 'failCount' fields reflect the overall operation in FileBackendMultiWrite::doOperationsInternal(). This makes it match up with single-write backends.
* Made FileBackend::clearCache() part of the public API.

diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php
index 613447d..5befbe0 100644 (file)
--- a/includes/filerepo/backend/FileBackend.php
+++ b/includes/filerepo/backend/FileBackend.php
@@ -479,6 +479,15 @@ abstract class FileBackendBase {
         */
        abstract public function getFileList( array $params );
 
+       /**
+        * Invalidate any in-process file existence and property cache.
+        * If $paths is given, then only the cache for those files will be cleared.
+        *
+        * @param $paths Array Storage paths
+        * @return void
+        */
+       abstract public function clearCache( array $paths = null );
+
        /**
         * Lock the files at the given storage paths in the backend.
         * This will either lock all the files or none (on failure).
@@ -1094,17 +1103,19 @@ abstract class FileBackend extends FileBackendBase {
 
                // Clear any cache entries (after locks acquired)
                $this->clearCache();
+
                // Actually attempt the operation batch...
-               $status->merge( FileOp::attemptBatch( $performOps, $opts ) );
+               $subStatus = FileOp::attemptBatch( $performOps, $opts );
+
+               // Merge errors into status fields
+               $status->merge( $subStatus );
+               $status->success = $subStatus->success; // not done in merge()
 
                return $status;
        }
 
        /**
-        * Invalidate the file existence and property cache
-        *
-        * @param $paths Array Clear cache for specific files
-        * @return void
+        * @see FileBackendBase::clearCache()
         */
        final public function clearCache( array $paths = null ) {
                if ( $paths === null ) {

diff --git a/includes/filerepo/backend/FileBackendMultiWrite.php b/includes/filerepo/backend/FileBackendMultiWrite.php
index 10ebd7e..b71b631 100644 (file)
--- a/includes/filerepo/backend/FileBackendMultiWrite.php
+++ b/includes/filerepo/backend/FileBackendMultiWrite.php
@@ -14,7 +14,7 @@
  * Only use this class when transitioning from one storage system to another.
  *
  * Read operations are only done on the 'master' backend for consistency.
- * All write operations are performed on all backends, in the order defined.
+ * Write operations are performed on all backends, in the order defined.
  * If an operation fails on one backend it will be rolled back from the others.
  *
  * @ingroup FileBackend
@@ -86,8 +86,8 @@ class FileBackendMultiWrite extends FileBackendBase {
                                        $filesChanged = array_merge( $filesChanged, $fileOp->storagePathsChanged() );
                                }
                                // Get the paths under the proxy backend's name
-                               $this->unsubstPaths( $filesRead );
-                               $this->unsubstPaths( $filesChanged );
+                               $filesRead = $this->unsubstPaths( $filesRead );
+                               $filesChanged = $this->unsubstPaths( $filesChanged );
                        }
                }
 
@@ -103,9 +103,7 @@ class FileBackendMultiWrite extends FileBackendBase {
                }
 
                // Clear any cache entries (after locks acquired)
-               foreach ( $this->backends as $backend ) {
-                       $backend->clearCache();
-               }
+               $this->clearCache();
 
                // Do a consistency check to see if the backends agree
                if ( count( $this->backends ) > 1 ) {
@@ -116,7 +114,32 @@ class FileBackendMultiWrite extends FileBackendBase {
                }
 
                // Actually attempt the operation batch...
-               $status->merge( FileOp::attemptBatch( $performOps, $opts ) );
+               $subStatus = FileOp::attemptBatch( $performOps, $opts );
+
+               $success = array();
+               $failCount = $successCount = 0;
+               // Make 'success', 'successCount', and 'failCount' fields reflect
+               // the overall operation, rather than all the batches for each backend.
+               // Do this by only using success values from the master backend's batch.
+               $batchStart = $this->masterIndex * count( $ops );
+               $batchEnd = $batchStart + count( $ops ) - 1;
+               for ( $i = $batchStart; $i <= $batchEnd; $i++ ) {
+                       if ( !isset( $subStatus->success[$i] ) ) {
+                               break; // failed out before trying this op
+                       } elseif ( $subStatus->success[$i] ) {
+                               ++$successCount;
+                       } else {
+                               ++$failCount;
+                       }
+                       $success[] = $subStatus->success[$i];
+               }
+               $subStatus->success = $success;
+               $subStatus->successCount = $successCount;
+               $subStatus->failCount = $failCount;
+
+               // Merge errors into status fields
+               $status->merge( $subStatus );
+               $status->success = $subStatus->success; // not done in merge()
 
                return $status;
        }
@@ -166,7 +189,7 @@ class FileBackendMultiWrite extends FileBackendBase {
 
        /**
         * Substitute the backend name in storage path parameters
-        * for a set of operations with that of a given backend.
+        * for a set of operations with that of a given internal backend.
         * 
         * @param $ops Array List of file operation arrays
         * @param $backend FileBackend
@@ -177,12 +200,8 @@ class FileBackendMultiWrite extends FileBackendBase {
                foreach ( $ops as $op ) {
                        $newOp = $op; // operation
                        foreach ( array( 'src', 'srcs', 'dst', 'dir' ) as $par ) {
-                               if ( isset( $newOp[$par] ) ) {
-                                       $newOp[$par] = preg_replace(
-                                               '!^mwstore://' . preg_quote( $this->name ) . '/!',
-                                               'mwstore://' . $backend->getName() . '/',
-                                               $newOp[$par] // string or array
-                                       );
+                               if ( isset( $newOp[$par] ) ) { // string or array
+                                       $newOp[$par] = $this->substPaths( $newOp[$par], $backend );
                                }
                        }
                        $newOps[] = $newOp;
@@ -203,15 +222,32 @@ class FileBackendMultiWrite extends FileBackendBase {
        }
 
        /**
-        * Replace the backend part of storage paths with this backend's name
+        * Substitute the backend of storage paths with an internal backend's name
         * 
-        * @param &$paths Array
-        * @return void 
+        * @param $paths Array|string List of paths or single string path
+        * @param $backend FileBackend
+        * @return Array|string
         */
-       protected function unsubstPaths( array &$paths ) {
-               foreach ( $paths as &$path ) {
-                       $path = preg_replace( '!^mwstore://([^/]+)!', "mwstore://{$this->name}", $path );
-               }
+       protected function substPaths( $paths, FileBackend $backend ) {
+               return preg_replace(
+                       '!^mwstore://' . preg_quote( $this->name ) . '/!',
+                       'mwstore://' . $backend->getName() . '/',
+                       $paths // string or array
+               );
+       }
+
+       /**
+        * Substitute the backend of internal storage paths with the proxy backend's name
+        * 
+        * @param $paths Array|string List of paths or single string path
+        * @return Array|string
+        */
+       protected function unsubstPaths( $paths ) {
+               return preg_replace(
+                       '!^mwstore://([^/]+)!',
+                       "mwstore://{$this->name}",
+                       $paths // string or array
+               );
        }
 
        /**
@@ -346,4 +382,14 @@ class FileBackendMultiWrite extends FileBackendBase {
                $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] );
                return $this->backends[$this->masterIndex]->getFileList( $realParams );
        }
+
+       /**
+        * @see FileBackendBase::clearCache()
+        */
+       public function clearCache( array $paths = null ) {
+               foreach ( $this->backends as $backend ) {
+                       $realPaths = is_array( $paths ) ? $this->substPaths( $paths ) : null;
+                       $backend->clearCache( $realPaths );
+               }
+       }
 }

diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php
index aef7729..c9f5f3a 100644 (file)
--- a/tests/phpunit/includes/filerepo/FileBackendTest.php
+++ b/tests/phpunit/includes/filerepo/FileBackendTest.php
@@ -76,6 +76,8 @@ class FileBackendTest extends MediaWikiTestCase {
                        "Store from $source to $dest succeeded without warnings ($backendName)." );
                $this->assertEquals( true, $status->isOK(),
                        "Store from $source to $dest succeeded ($backendName)." );
+               $this->assertEquals( array( 0 => true ), $status->success,
+                       "Store from $source to $dest has proper 'success' field in Status ($backendName)." );
                $this->assertEquals( true, file_exists( $source ),
                        "Source file $source still exists ($backendName)." );
                $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ),
@@ -142,6 +144,8 @@ class FileBackendTest extends MediaWikiTestCase {
                        "Copy from $source to $dest succeeded without warnings ($backendName)." );
                $this->assertEquals( true, $status->isOK(),
                        "Copy from $source to $dest succeeded ($backendName)." );
+               $this->assertEquals( array( 0 => true ), $status->success,
+                       "Copy from $source to $dest has proper 'success' field in Status ($backendName)." );
                $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $source ) ),
                        "Source file $source still exists ($backendName)." );
                $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ),
@@ -210,6 +214,8 @@ class FileBackendTest extends MediaWikiTestCase {
                        "Move from $source to $dest succeeded without warnings ($backendName)." );
                $this->assertEquals( true, $status->isOK(),
                        "Move from $source to $dest succeeded ($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 still exists ($backendName)." );
                $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $dest ) ),
@@ -282,6 +288,8 @@ class FileBackendTest extends MediaWikiTestCase {
                                "Deletion of file at $source succeeded without warnings ($backendName)." );
                        $this->assertEquals( true, $status->isOK(),
                                "Deletion of file at $source succeeded ($backendName)." );
+                       $this->assertEquals( array( 0 => true ), $status->success,
+                               "Deletion of file at $source has proper 'success' field in Status ($backendName)." );
                } else {
                        $this->assertEquals( false, $status->isOK(),
                                "Deletion of file at $source failed ($backendName)." );
@@ -362,6 +370,8 @@ class FileBackendTest extends MediaWikiTestCase {
                                "Creation of file at $dest succeeded without warnings ($backendName)." );
                        $this->assertEquals( true, $status->isOK(),
                                "Creation of file at $dest succeeded ($backendName)." );
+                       $this->assertEquals( array( 0 => true ), $status->success,
+                               "Creation of file at $dest has proper 'success' field in Status ($backendName)." );
                } else {
                        $this->assertEquals( false, $status->isOK(),
                                "Creation of file at $dest failed ($backendName)." );