* r107986: Added readOnly checks to prepare(), secure(), and clean() in FileBackendBase.

* Added some prepare()/clean() tests.

diff --git a/includes/filerepo/backend/FSFileBackend.php b/includes/filerepo/backend/FSFileBackend.php
index 7c1cca4..a0caebe 100644 (file)
--- a/includes/filerepo/backend/FSFileBackend.php
+++ b/includes/filerepo/backend/FSFileBackend.php
@@ -274,9 +274,9 @@ class FSFileBackend extends FileBackend {
        }
 
        /**
-        * @see FileBackend::doPrepare()
+        * @see FileBackend::doPrepareInternal()
         */
-       protected function doPrepare( $container, $dir, array $params ) {
+       protected function doPrepareInternal( $container, $dir, array $params ) {
                $status = Status::newGood();
                if ( !wfMkdirParents( $dir ) ) {
                        $status->fatal( 'directorycreateerror', $params['dir'] );
@@ -289,9 +289,9 @@ class FSFileBackend extends FileBackend {
        }
 
        /**
-        * @see FileBackend::doSecure()
+        * @see FileBackend::doSecureInternal()
         */
-       protected function doSecure( $container, $dir, array $params ) {
+       protected function doSecureInternal( $container, $dir, array $params ) {
                $status = Status::newGood();
                if ( !wfMkdirParents( $dir ) ) {
                        $status->fatal( 'directorycreateerror', $params['dir'] );
@@ -308,25 +308,27 @@ class FSFileBackend extends FileBackend {
                        }
                }
                // Add a .htaccess file to the root of the container...
-               list( $b, $container, $r ) = FileBackend::splitStoragePath( $params['dir'] );
-               $dirRoot = $this->containerPaths[$container]; // real path
-               if ( !empty( $params['noAccess'] ) && !file_exists( "{$dirRoot}/.htaccess" ) ) {
-                       wfSuppressWarnings();
-                       $ok = file_put_contents( "{$dirRoot}/.htaccess", "Deny from all\n" );
-                       wfRestoreWarnings();
-                       if ( !$ok ) {
-                               $storeDir = "mwstore://{$this->name}/{$container}";
-                               $status->fatal( 'backend-fail-create', "$storeDir/.htaccess" );
-                               return $status;
+               if ( !empty( $params['noAccess'] ) ) {
+                       list( $b, $container, $r ) = FileBackend::splitStoragePath( $params['dir'] );
+                       $dirRoot = $this->containerPaths[$container]; // real path
+                       if ( !file_exists( "{$dirRoot}/.htaccess" ) ) {
+                               wfSuppressWarnings();
+                               $ok = file_put_contents( "{$dirRoot}/.htaccess", "Deny from all\n" );
+                               wfRestoreWarnings();
+                               if ( !$ok ) {
+                                       $storeDir = "mwstore://{$this->name}/{$container}";
+                                       $status->fatal( 'backend-fail-create', "$storeDir/.htaccess" );
+                                       return $status;
+                               }
                        }
                }
                return $status;
        }
 
        /**
-        * @see FileBackend::doClean()
+        * @see FileBackend::doCleanInternal()
         */
-       protected function doClean( $container, $dir, array $params ) {
+       protected function doCleanInternal( $container, $dir, array $params ) {
                $status = Status::newGood();
                wfSuppressWarnings();
                if ( is_dir( $dir ) ) {

diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php
index 5befbe0..fefb2c9 100644 (file)
--- a/includes/filerepo/backend/FileBackend.php
+++ b/includes/filerepo/backend/FileBackend.php
@@ -292,7 +292,17 @@ abstract class FileBackendBase {
         * @param $params Array
         * @return Status
         */
-       abstract public function prepare( array $params );
+       final public function prepare( array $params ) {
+               if ( $this->readOnly != '' ) {
+                       return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly );
+               }
+               return $this->doPrepare( $params );
+       }
+
+       /**
+        * @see FileBackendBase::prepare()
+        */
+       abstract protected function doPrepare( array $params );
 
        /**
         * Take measures to block web access to a directory and
@@ -309,7 +319,17 @@ abstract class FileBackendBase {
         * @param $params Array
         * @return Status
         */
-       abstract public function secure( array $params );
+       final public function secure( array $params ) {
+               if ( $this->readOnly != '' ) {
+                       return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly );
+               }
+               return $this->doSecure( $params );
+       }
+
+       /**
+        * @see FileBackendBase::secure()
+        */
+       abstract protected function doSecure( array $params );
 
        /**
         * Clean up an empty storage directory.
@@ -321,7 +341,17 @@ abstract class FileBackendBase {
         * @param $params Array
         * @return Status
         */
-       abstract public function clean( array $params );
+       final public function clean( array $params ) {
+               if ( $this->readOnly != '' ) {
+                       return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly );
+               }
+               return $this->doClean( $params );
+       }
+
+       /**
+        * @see FileBackendBase::clean()
+        */
+       abstract protected function doClean( array $params );
 
        /**
         * Check if a file exists at a storage path in the backend.
@@ -747,9 +777,9 @@ abstract class FileBackend extends FileBackendBase {
        }
 
        /**
-        * @see FileBackendBase::prepare()
+        * @see FileBackendBase::doPrepare()
         */
-       final public function prepare( array $params ) {
+       final protected function doPrepare( array $params ) {
                $status = Status::newGood();
                list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] );
                if ( $dir === null ) {
@@ -757,28 +787,28 @@ abstract class FileBackend extends FileBackendBase {
                        return $status; // invalid storage path
                }
                if ( $shard !== null ) { // confined to a single container/shard
-                       $status->merge( $this->doPrepare( $fullCont, $dir, $params ) );
+                       $status->merge( $this->doPrepareInternal( $fullCont, $dir, $params ) );
                } else { // directory is on several shards
                        wfDebug( __METHOD__ . ": iterating over all container shards.\n" );
                        list( $b, $shortCont, $r ) = self::splitStoragePath( $params['dir'] );
                        foreach ( $this->getContainerSuffixes( $shortCont ) as $suffix ) {
-                               $status->merge( $this->doPrepare( "{$fullCont}{$suffix}", $dir, $params ) );
+                               $status->merge( $this->doPrepareInternal( "{$fullCont}{$suffix}", $dir, $params ) );
                        }
                }
                return $status;
        }
 
        /**
-        * @see FileBackend::prepare()
+        * @see FileBackend::doPrepare()
         */
-       protected function doPrepare( $container, $dir, array $params ) {
+       protected function doPrepareInternal( $container, $dir, array $params ) {
                return Status::newGood();
        }
 
        /**
-        * @see FileBackendBase::secure()
+        * @see FileBackendBase::doSecure()
         */
-       final public function secure( array $params ) {
+       final protected function doSecure( array $params ) {
                $status = Status::newGood();
                list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] );
                if ( $dir === null ) {
@@ -786,28 +816,28 @@ abstract class FileBackend extends FileBackendBase {
                        return $status; // invalid storage path
                }
                if ( $shard !== null ) { // confined to a single container/shard
-                       $status->merge( $this->doSecure( $fullCont, $dir, $params ) );
+                       $status->merge( $this->doSecureInternal( $fullCont, $dir, $params ) );
                } else { // directory is on several shards
                        wfDebug( __METHOD__ . ": iterating over all container shards.\n" );
                        list( $b, $shortCont, $r ) = self::splitStoragePath( $params['dir'] );
                        foreach ( $this->getContainerSuffixes( $shortCont ) as $suffix ) {
-                               $status->merge( $this->doSecure( "{$fullCont}{$suffix}", $dir, $params ) );
+                               $status->merge( $this->doSecureInternal( "{$fullCont}{$suffix}", $dir, $params ) );
                        }
                }
                return $status;
        }
 
        /**
-        * @see FileBackend::secure()
+        * @see FileBackend::doSecure()
         */
-       protected function doSecure( $container, $dir, array $params ) {
+       protected function doSecureInternal( $container, $dir, array $params ) {
                return Status::newGood();
        }
 
        /**
-        * @see FileBackendBase::clean()
+        * @see FileBackendBase::doClean()
         */
-       final public function clean( array $params ) {
+       final protected function doClean( array $params ) {
                $status = Status::newGood();
                list( $fullCont, $dir, $shard ) = $this->resolveStoragePath( $params['dir'] );
                if ( $dir === null ) {
@@ -815,21 +845,21 @@ abstract class FileBackend extends FileBackendBase {
                        return $status; // invalid storage path
                }
                if ( $shard !== null ) { // confined to a single container/shard
-                       $status->merge( $this->doClean( $fullCont, $dir, $params ) );
+                       $status->merge( $this->doCleanInternal( $fullCont, $dir, $params ) );
                } else { // directory is on several shards
                        wfDebug( __METHOD__ . ": iterating over all container shards.\n" );
                        list( $b, $shortCont, $r ) = self::splitStoragePath( $params['dir'] );
                        foreach ( $this->getContainerSuffixes( $shortCont ) as $suffix ) {
-                               $status->merge( $this->doClean( "{$fullCont}{$suffix}", $dir, $params ) );
+                               $status->merge( $this->doCleanInternal( "{$fullCont}{$suffix}", $dir, $params ) );
                        }
                }
                return $status;
        }
 
        /**
-        * @see FileBackend::clean()
+        * @see FileBackend::doClean()
         */
-       protected function doClean( $container, $dir, array $params ) {
+       protected function doCleanInternal( $container, $dir, array $params ) {
                return Status::newGood();
        }
 

diff --git a/includes/filerepo/backend/FileBackendMultiWrite.php b/includes/filerepo/backend/FileBackendMultiWrite.php
index b71b631..4258d13 100644 (file)
--- a/includes/filerepo/backend/FileBackendMultiWrite.php
+++ b/includes/filerepo/backend/FileBackendMultiWrite.php
@@ -251,37 +251,37 @@ class FileBackendMultiWrite extends FileBackendBase {
        }
 
        /**
-        * @see FileBackendBase::prepare()
+        * @see FileBackendBase::doPrepare()
         */
-       public function prepare( array $params ) {
+       public function doPrepare( array $params ) {
                $status = Status::newGood();
                foreach ( $this->backends as $backend ) {
                        $realParams = $this->substOpPaths( $params, $backend );
-                       $status->merge( $backend->prepare( $realParams ) );
+                       $status->merge( $backend->doPrepare( $realParams ) );
                }
                return $status;
        }
 
        /**
-        * @see FileBackendBase::secure()
+        * @see FileBackendBase::doSecure()
         */
-       public function secure( array $params ) {
+       public function doSecure( array $params ) {
                $status = Status::newGood();
                foreach ( $this->backends as $backend ) {
                        $realParams = $this->substOpPaths( $params, $backend );
-                       $status->merge( $backend->secure( $realParams ) );
+                       $status->merge( $backend->doSecure( $realParams ) );
                }
                return $status;
        }
 
        /**
-        * @see FileBackendBase::clean()
+        * @see FileBackendBase::doClean()
         */
-       public function clean( array $params ) {
+       public function doClean( array $params ) {
                $status = Status::newGood();
                foreach ( $this->backends as $backend ) {
                        $realParams = $this->substOpPaths( $params, $backend );
-                       $status->merge( $backend->clean( $realParams ) );
+                       $status->merge( $backend->doClean( $realParams ) );
                }
                return $status;
        }

diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php
index c9f5f3a..df9651d 100644 (file)
--- a/tests/phpunit/includes/filerepo/FileBackendTest.php
+++ b/tests/phpunit/includes/filerepo/FileBackendTest.php
@@ -690,11 +690,53 @@ class FileBackendTest extends MediaWikiTestCase {
                return $cases;
        }
 
-       // @TODO: testPrepare
+       /**
+        * @dataProvider provider_testPrepareAndClean
+        */
+       public function testPrepareAndClean( $path, $isOK ) {
+               $this->backend = $this->singleBackend;
+               $this->doTestPrepareAndClean( $path, $isOK );
 
-       // @TODO: testSecure
+               $this->backend = $this->multiBackend;
+               $this->doTestPrepareAndClean( $path, $isOK );
+       }
+
+       function provider_testPrepareAndClean() {
+               $base = $this->baseStorePath();
+               return array(
+                       array( "$base/cont1/a/z/some_file1.txt", true ),
+                       array( "$base/cont2/a/z/some_file2.txt", true ),
+                       array( "$base/cont3/a/z/some_file3.txt", false ),
+               );
+       }
+
+       function doTestPrepareAndClean( $path, $isOK ) {
+               $backendName = $this->backendClass();
+
+               $status = $this->backend->prepare( array( 'dir' => $path ) );
+               if ( $isOK ) {
+                       $this->assertEquals( array(), $status->errors,
+                               "Preparing dir $path succeeded without warnings ($backendName)." );
+                       $this->assertEquals( true, $status->isOK(),
+                               "Preparing dir $path succeeded ($backendName)." );
+               } else {
+                       $this->assertEquals( false, $status->isOK(),
+                               "Preparing dir $path failed ($backendName)." );
+               }
 
-       // @TODO: testClean
+               $status = $this->backend->clean( array( 'dir' => $path ) );
+               if ( $isOK ) {
+                       $this->assertEquals( array(), $status->errors,
+                               "Cleaning dir $path succeeded without warnings ($backendName)." );
+                       $this->assertEquals( true, $status->isOK(),
+                               "Cleaning dir $path succeeded ($backendName)." );
+               } else {
+                       $this->assertEquals( false, $status->isOK(),
+                               "Cleaning dir $path failed ($backendName)." );
+               }
+       }
+
+       // @TODO: testSecure
 
        // @TODO: testDoOperations