| Index: trunk/phase3/tests/phpunit/includes/filerepo/FileBackendTest.php |
| — | — | @@ -884,11 +884,13 @@ |
| 885 | 885 | // Does nothing |
| 886 | 886 | array( 'op' => 'move', 'src' => $fileC, 'dst' => $fileC, 'overwriteSame' => 1 ), |
| 887 | 887 | // Does nothing |
| | 888 | + array( 'op' => 'null' ), |
| | 889 | + // Does nothing |
| 888 | 890 | ) ); |
| 889 | 891 | |
| 890 | 892 | $this->assertEquals( array(), $status->errors, "Operation batch succeeded" ); |
| 891 | 893 | $this->assertEquals( true, $status->isOK(), "Operation batch succeeded" ); |
| 892 | | - $this->assertEquals( 12, count( $status->success ), |
| | 894 | + $this->assertEquals( 13, count( $status->success ), |
| 893 | 895 | "Operation batch has correct success array" ); |
| 894 | 896 | |
| 895 | 897 | $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileA ) ), |
| Index: trunk/phase3/includes/filerepo/backend/FileOp.php |
| — | — | @@ -7,12 +7,11 @@ |
| 8 | 8 | |
| 9 | 9 | /** |
| 10 | 10 | * Helper class for representing operations with transaction support. |
| 11 | | - * FileBackend::doOperations() will require these classes for supported operations. |
| 12 | 11 | * Do not use this class from places outside FileBackend. |
| | 12 | + * |
| | 13 | + * Methods called from attemptBatch() should avoid throwing exceptions at all costs. |
| | 14 | + * FileOp objects should be lightweight in order to support large arrays in memory. |
| 13 | 15 | * |
| 14 | | - * Use of large fields should be avoided as we want to support |
| 15 | | - * potentially many FileOp classes in large arrays in memory. |
| 16 | | - * |
| 17 | 16 | * @ingroup FileBackend |
| 18 | 17 | * @since 1.19 |
| 19 | 18 | */ |
| — | — | @@ -29,6 +28,10 @@ |
| 30 | 29 | protected $sourceSha1; // string |
| 31 | 30 | protected $destSameAsSource; // boolean |
| 32 | 31 | |
| | 32 | + /* Operation parameters */ |
| | 33 | + protected static $requiredParams = array(); |
| | 34 | + protected static $optionalParams = array(); |
| | 35 | + |
| 33 | 36 | /* Object life-cycle */ |
| 34 | 37 | const STATE_NEW = 1; |
| 35 | 38 | const STATE_CHECKED = 2; |
| — | — | @@ -43,14 +46,23 @@ |
| 44 | 47 | * |
| 45 | 48 | * @params $backend FileBackend |
| 46 | 49 | * @params $params Array |
| | 50 | + * @throws MWException |
| 47 | 51 | */ |
| 48 | 52 | final public function __construct( FileBackendBase $backend, array $params ) { |
| 49 | 53 | $this->backend = $backend; |
| 50 | | - foreach ( $this->allowedParams() as $name ) { |
| | 54 | + $class = get_class( $this ); // simulate LSB |
| | 55 | + foreach ( $class::$requiredParams as $name ) { |
| 51 | 56 | if ( isset( $params[$name] ) ) { |
| 52 | 57 | $this->params[$name] = $params[$name]; |
| | 58 | + } else { |
| | 59 | + throw new MWException( "File operation missing parameter '$name'." ); |
| 53 | 60 | } |
| 54 | 61 | } |
| | 62 | + foreach ( $class::$optionalParams as $name ) { |
| | 63 | + if ( isset( $params[$name] ) ) { |
| | 64 | + $this->params[$name] = $params[$name]; |
| | 65 | + } |
| | 66 | + } |
| 55 | 67 | $this->params = $params; |
| 56 | 68 | } |
| 57 | 69 | |
| — | — | @@ -223,13 +235,6 @@ |
| 224 | 236 | } |
| 225 | 237 | |
| 226 | 238 | /** |
| 227 | | - * @return Array List of allowed parameters |
| 228 | | - */ |
| 229 | | - protected function allowedParams() { |
| 230 | | - return array(); |
| 231 | | - } |
| 232 | | - |
| 233 | | - /** |
| 234 | 239 | * @return Status |
| 235 | 240 | */ |
| 236 | 241 | protected function doPrecheck( array &$predicates ) { |
| — | — | @@ -382,9 +387,8 @@ |
| 383 | 388 | * overwriteSame : override any existing file at destination |
| 384 | 389 | */ |
| 385 | 390 | class StoreFileOp extends FileOp { |
| 386 | | - protected function allowedParams() { |
| 387 | | - return array( 'src', 'dst', 'overwrite', 'overwriteSame' ); |
| 388 | | - } |
| | 391 | + protected static $requiredParams = array( 'src', 'dst' ); |
| | 392 | + protected static $optionalParams = array( 'overwrite', 'overwriteSame' ); |
| 389 | 393 | |
| 390 | 394 | protected function doPrecheck( array &$predicates ) { |
| 391 | 395 | $status = Status::newGood(); |
| — | — | @@ -438,15 +442,14 @@ |
| 439 | 443 | /** |
| 440 | 444 | * Create a file in the backend with the given content. |
| 441 | 445 | * Parameters similar to FileBackend::createInternal(), which include: |
| 442 | | - * content : a string of raw file content. Let unset to create an empty file. |
| | 446 | + * content : the raw file contents |
| 443 | 447 | * dst : destination storage path |
| 444 | 448 | * overwrite : do nothing and pass if an identical file exists at destination |
| 445 | 449 | * overwriteSame : override any existing file at destination |
| 446 | 450 | */ |
| 447 | 451 | class CreateFileOp extends FileOp { |
| 448 | | - protected function allowedParams() { |
| 449 | | - return array( 'content', 'dst', 'overwrite', 'overwriteSame' ); |
| 450 | | - } |
| | 452 | + protected static $requiredParams = array( 'content', 'dst' ); |
| | 453 | + protected static $optionalParams = array( 'overwrite', 'overwriteSame' ); |
| 451 | 454 | |
| 452 | 455 | protected function doPrecheck( array &$predicates ) { |
| 453 | 456 | $status = Status::newGood(); |
| — | — | @@ -496,9 +499,8 @@ |
| 497 | 500 | * overwriteSame : override any existing file at destination |
| 498 | 501 | */ |
| 499 | 502 | class CopyFileOp extends FileOp { |
| 500 | | - protected function allowedParams() { |
| 501 | | - return array( 'src', 'dst', 'overwrite', 'overwriteSame' ); |
| 502 | | - } |
| | 503 | + protected static $requiredParams = array( 'src', 'dst' ); |
| | 504 | + protected static $optionalParams = array( 'overwrite', 'overwriteSame' ); |
| 503 | 505 | |
| 504 | 506 | protected function doPrecheck( array &$predicates ) { |
| 505 | 507 | $status = Status::newGood(); |
| — | — | @@ -551,9 +553,8 @@ |
| 552 | 554 | * overwriteSame : override any existing file at destination |
| 553 | 555 | */ |
| 554 | 556 | class MoveFileOp extends FileOp { |
| 555 | | - protected function allowedParams() { |
| 556 | | - return array( 'src', 'dst', 'overwrite', 'overwriteSame' ); |
| 557 | | - } |
| | 557 | + protected static $requiredParams = array( 'src', 'dst' ); |
| | 558 | + protected static $optionalParams = array( 'overwrite', 'overwriteSame' ); |
| 558 | 559 | |
| 559 | 560 | protected function doPrecheck( array &$predicates ) { |
| 560 | 561 | $status = Status::newGood(); |
| — | — | @@ -610,12 +611,11 @@ |
| 611 | 612 | * ignoreMissingSource : don't return an error if the file does not exist |
| 612 | 613 | */ |
| 613 | 614 | class DeleteFileOp extends FileOp { |
| | 615 | + protected static $requiredParams = array( 'src' ); |
| | 616 | + protected static $optionalParams = array( 'ignoreMissingSource' ); |
| | 617 | + |
| 614 | 618 | protected $needsDelete = true; |
| 615 | 619 | |
| 616 | | - protected function allowedParams() { |
| 617 | | - return array( 'src', 'ignoreMissingSource' ); |
| 618 | | - } |
| 619 | | - |
| 620 | 620 | protected function doPrecheck( array &$predicates ) { |
| 621 | 621 | $status = Status::newGood(); |
| 622 | 622 | // Check if the source file exists |