Fixes and cleanups to FileOpBatch

* Fixed bug where operations that failed after precheck() would
  not properly update the Status. This could cause failed ops to
  be treated as successful.
* Removed special casing for 1-sized batches, this was mostly
  for Swift and is irrelevant after it was rewritten.
* Removed unused return value and fixed the docs

bug: 60318
Change-Id: I7f12ebf711bc196313745f943070f8bdb6335964

diff --git a/includes/filebackend/FileOpBatch.php b/includes/filebackend/FileOpBatch.php
index 9fd1eb5..b42e140 100644 (file)
--- a/includes/filebackend/FileOpBatch.php
+++ b/includes/filebackend/FileOpBatch.php
@@ -141,9 +141,8 @@ class FileOpBatch {
         * within any given sub-batch do not depend on each other.
         * This will abort remaining ops on failure.
         *
-        * @param array $pPerformOps
+        * @param array $pPerformOps Batches of file ops (batches use original indexes)
         * @param Status $status
-        * @return bool Success
         */
        protected static function runParallelBatches( array $pPerformOps, Status $status ) {
                $aborted = false; // set to true on unexpected errors
@@ -164,12 +163,8 @@ class FileOpBatch {
                        // If attemptAsync() returns a Status, it was either due to an error
                        // or the backend does not support async ops and did it synchronously.
                        foreach ( $performOpsBatch as $i => $fileOp ) {
-                               if ( !$fileOp->failed() ) { // failed => already has Status
-                                       // If the batch is just one operation, it's faster to avoid
-                                       // pipelining as that can involve creating new TCP connections.
-                                       $subStatus = ( count( $performOpsBatch ) > 1 )
-                                               ? $fileOp->attemptAsync()
-                                               : $fileOp->attempt();
+                               if ( !isset( $status->success[$i] ) ) { // didn't already fail in precheck()
+                                       $subStatus = $fileOp->attemptAsync();
                                        if ( $subStatus->value instanceof FileBackendStoreOpHandle ) {
                                                $opHandles[$i] = $subStatus->value; // deferred
                                        } else {
@@ -181,7 +176,7 @@ class FileOpBatch {
                        $statuses = $statuses + $backend->executeOpHandlesInternal( $opHandles );
                        // Marshall and merge all the responses (blocking)...
                        foreach ( $performOpsBatch as $i => $fileOp ) {
-                               if ( !$fileOp->failed() ) { // failed => already has Status
+                               if ( !isset( $status->success[$i] ) ) { // didn't already fail in precheck()
                                        $subStatus = $statuses[$i];
                                        $status->merge( $subStatus );
                                        if ( $subStatus->isOK() ) {
@@ -195,7 +190,5 @@ class FileOpBatch {
                                }
                        }
                }
-
-               return $status;
        }
 }