[FileBackend] Made doOperations() Status handling align with documentation as well...

diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php
index 227941e..7371cc9 100644 (file)
--- a/includes/filerepo/backend/FileBackend.php
+++ b/includes/filerepo/backend/FileBackend.php
@@ -186,8 +186,9 @@ abstract class FileBackend {
         * Return value:
         * This returns a Status, which contains all warnings and fatals that occured
         * during the operation. The 'failCount', 'successCount', and 'success' members
-        * will reflect each operation attempted. The status will be "OK" unless any
-        * of the operations failed and the 'force' parameter was not set.
+        * will reflect each operation attempted. The 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 $ops Array List of operations to execute in order
         * @param $opts Array Batch operation options

diff --git a/includes/filerepo/backend/FileOp.php b/includes/filerepo/backend/FileOp.php
index ad30f0f..312a531 100644 (file)
--- a/includes/filerepo/backend/FileOp.php
+++ b/includes/filerepo/backend/FileOp.php
@@ -81,6 +81,10 @@ abstract class FileOp {
         * '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.
+        *
+        * 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
@@ -115,6 +119,11 @@ abstract class FileOp {
                        }
                }
 
+               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.

diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php
index 356c689..da44797 100644 (file)
--- a/tests/phpunit/includes/filerepo/FileBackendTest.php
+++ b/tests/phpunit/includes/filerepo/FileBackendTest.php
@@ -1034,6 +1034,16 @@ class FileBackendTest extends MediaWikiTestCase {
                $this->doTestDoOperations();
                $this->tearDownFiles();
 
+               $this->backend = $this->singleBackend;
+               $this->tearDownFiles();
+               $this->doTestDoOperationsFailing();
+               $this->tearDownFiles();
+
+               $this->backend = $this->multiBackend;
+               $this->tearDownFiles();
+               $this->doTestDoOperationsFailing();
+               $this->tearDownFiles();
+
                // @TODO: test some cases where the ops should fail
        }
 
@@ -1057,9 +1067,9 @@ class FileBackendTest extends MediaWikiTestCase {
 
                $status = $this->backend->doOperations( array(
                        array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC, 'overwrite' => 1 ),
-                       // Now: A:<A>, B:<B>, C:<A>, D:<D> (file:<orginal contents>)
+                       // Now: A:<A>, B:<B>, C:<A>, D:<empty> (file:<orginal contents>)
                        array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileA, 'overwriteSame' => 1 ),
-                       // Now: A:<A>, B:<B>, C:<A>, D:<D>
+                       // Now: A:<A>, B:<B>, C:<A>, D:<empty>
                        array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileD, 'overwrite' => 1 ),
                        // Now: A:<A>, B:<B>, C:<empty>, D:<A>
                        array( 'op' => 'move', 'src' => $fileB, 'dst' => $fileC ),
@@ -1109,6 +1119,68 @@ class FileBackendTest extends MediaWikiTestCase {
                        "Correct file SHA-1 of $fileC" );
        }
 
+       function doTestDoOperationsFailing() {
+               $base = $this->baseStorePath();
+
+               $fileA = "$base/unittest-cont2/a/b/fileA.txt";
+               $fileAContents = '3tqtmoeatmn4wg4qe-mg3qt3 tq';
+               $fileB = "$base/unittest-cont2/a/b/fileB.txt";
+               $fileBContents = 'g-jmq3gpqgt3qtg q3GT ';
+               $fileC = "$base/unittest-cont2/a/b/fileC.txt";
+               $fileCContents = 'eigna[ogmewt 3qt g3qg flew[ag';
+               $fileD = "$base/unittest-cont2/a/b/fileD.txt";
+
+               $this->prepare( array( 'dir' => dirname( $fileA ) ) );
+               $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
+               $this->prepare( array( 'dir' => dirname( $fileB ) ) );
+               $this->backend->create( array( 'dst' => $fileB, 'content' => $fileBContents ) );
+               $this->prepare( array( 'dir' => dirname( $fileC ) ) );
+               $this->backend->create( array( 'dst' => $fileC, 'content' => $fileCContents ) );
+
+               $status = $this->backend->doOperations( array(
+                       array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC, 'overwrite' => 1 ),
+                       // Now: A:<A>, B:<B>, C:<A>, D:<empty> (file:<orginal contents>)
+                       array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileA, 'overwriteSame' => 1 ),
+                       // Now: A:<A>, B:<B>, C:<A>, D:<empty>
+                       array( 'op' => 'copy', 'src' => $fileB, 'dst' => $fileD, 'overwrite' => 1 ),
+                       // Now: A:<A>, B:<B>, C:<A>, D:<B>
+                       array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileD ),
+                       // Now: A:<A>, B:<B>, C:<A>, D:<empty> (failed)
+                       array( 'op' => 'move', 'src' => $fileB, 'dst' => $fileC, 'overwriteSame' => 1 ),
+                       // Now: A:<A>, B:<B>, C:<A>, D:<empty> (failed)
+                       array( 'op' => 'move', 'src' => $fileB, 'dst' => $fileA, 'overwrite' => 1 ),
+                       // Now: A:<B>, B:<empty>, C:<A>, D:<empty>
+                       array( 'op' => 'delete', 'src' => $fileD ),
+                       // Now: A:<B>, B:<empty>, C:<A>, D:<empty>
+                       array( 'op' => 'null' ),
+                       // Does nothing
+               ), array( 'force' => 1 ) );
+
+               $this->assertNotEquals( array(), $status->errors, "Operation had warnings" );
+               $this->assertEquals( true, $status->isOK(), "Operation batch succeeded" );
+               $this->assertEquals( 8, count( $status->success ),
+                       "Operation batch has correct success array" );
+
+               $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileB ) ),
+                       "File does not exist at $fileB" );
+               $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileD ) ),
+                       "File does not exist at $fileD" );
+
+               $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $fileA ) ),
+                       "File does not exist at $fileA" );
+               $this->assertEquals( true, $this->backend->fileExists( array( 'src' => $fileC ) ),
+                       "File exists at $fileC" );
+               $this->assertEquals( $fileBContents,
+                       $this->backend->getFileContents( array( 'src' => $fileA ) ),
+                       "Correct file contents of $fileA" );
+               $this->assertEquals( strlen( $fileBContents ),
+                       $this->backend->getFileSize( array( 'src' => $fileA ) ),
+                       "Correct file size of $fileA" );
+               $this->assertEquals( wfBaseConvert( sha1( $fileBContents ), 16, 36, 31 ),
+                       $this->backend->getFileSha1Base36( array( 'src' => $fileA ) ),
+                       "Correct file SHA-1 of $fileA" );
+       }
+
        public function testGetFileList() {
                $this->backend = $this->singleBackend;
                $this->tearDownFiles();