From 783f7f0bd315e4b7ff0abe044fc751faf036a6ff Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Thu, 8 Mar 2012 22:31:04 +0000 Subject: [PATCH] [FileBackend] Made doOperations() Status handling align with documentation as well as what FileRepo is essentially expecting when using the 'force' option (it assumes fatals are for total batch failures, not just partial ones). The relevant documentation was also improved. --- includes/filerepo/backend/FileBackend.php | 5 +- includes/filerepo/backend/FileOp.php | 9 +++ .../includes/filerepo/FileBackendTest.php | 76 ++++++++++++++++++- 3 files changed, 86 insertions(+), 4 deletions(-) diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php index 227941e50b..7371cc9560 100644 --- 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 ad30f0f6d2..312a531006 100644 --- 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 356c6898d2..da44797a6a 100644 --- 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:, B:, C:, D: (file:) + // Now: A:, B:, C:, D: (file:) array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileA, 'overwriteSame' => 1 ), - // Now: A:, B:, C:, D: + // Now: A:, B:, C:, D: array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileD, 'overwrite' => 1 ), // Now: A:, B:, C:, D: 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:, B:, C:, D: (file:) + array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileA, 'overwriteSame' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'copy', 'src' => $fileB, 'dst' => $fileD, 'overwrite' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileD ), + // Now: A:, B:, C:, D: (failed) + array( 'op' => 'move', 'src' => $fileB, 'dst' => $fileC, 'overwriteSame' => 1 ), + // Now: A:, B:, C:, D: (failed) + array( 'op' => 'move', 'src' => $fileB, 'dst' => $fileA, 'overwrite' => 1 ), + // Now: A:, B:, C:, D: + array( 'op' => 'delete', 'src' => $fileD ), + // Now: A:, B:, C:, D: + 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(); -- 2.20.1