Fixes for r106752:

* Make sure FSFileIterator::current() directly returns the string path and that it is relative to the directory being searched.
* Fixed silly bug in testGetFileList that masked any bugs.
* Added a few code comments.

diff --git a/includes/filerepo/backend/FSFileBackend.php b/includes/filerepo/backend/FSFileBackend.php
index 5f2d04a..f6c7d48 100644 (file)
--- a/includes/filerepo/backend/FSFileBackend.php
+++ b/includes/filerepo/backend/FSFileBackend.php
@@ -446,6 +446,7 @@ class FSFileBackend extends FileBackend {
 class FSFileIterator implements Iterator {
        /** @var RecursiveIteratorIterator */
        protected $iter;
+       protected $suffixStart; // integer
 
        /**
         * Get an FSFileIterator from a file system directory
@@ -453,6 +454,7 @@ class FSFileIterator implements Iterator {
         * @param $dir string
         */
        public function __construct( $dir ) {
+               $this->suffixStart = strlen( realpath( $dir ) ) + 1; // size of "path/to/dir/"
                try {
                        $this->iter = new RecursiveIteratorIterator( new RecursiveDirectoryIterator( $dir ) );
                } catch ( UnexpectedValueException $e ) {
@@ -461,7 +463,8 @@ class FSFileIterator implements Iterator {
        }
 
        public function current() {
-               return $this->iter->current();
+               // Return only the relative path and normalize slashes to FileBackend-style
+               return str_replace( '\\', '/', substr( $this->iter->current(), $this->suffixStart ) );
        }
 
        public function key() {

diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php
index 902f4cc..b5b2244 100644 (file)
--- a/includes/filerepo/backend/FileBackend.php
+++ b/includes/filerepo/backend/FileBackend.php
@@ -1368,6 +1368,7 @@ class ContainerShardListIterator implements Iterator {
        /**
         * If the iterator for this container shard is out of items,
         * then move on to the next container that has items.
+        * If there are none, then it advances to the last container.
         */
        protected function nextShardIteratorIfNotValid() {
                while ( !$this->valid() ) {

diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php
index c1cc55f..3748deb 100644 (file)
--- a/tests/phpunit/includes/filerepo/FileBackendTest.php
+++ b/tests/phpunit/includes/filerepo/FileBackendTest.php
@@ -556,7 +556,7 @@ class FileBackendTest extends MediaWikiTestCase {
                        "subdir1/test1.txt",
                        "subdir1/test2.txt",
                        "subdir2/test3.txt",
-                       "subdir2/test1.txt",
+                       "subdir2/test4.txt",
                        "subdir2/subdir/test1.txt",
                        "subdir2/subdir/test2.txt",
                        "subdir2/subdir/test3.txt",
@@ -565,7 +565,7 @@ class FileBackendTest extends MediaWikiTestCase {
                        "subdir2/subdir/sub/test0.txt",
                        "subdir2/subdir/sub/120-px-file.txt",
                );
-               $expected = sort( $expected );
+               sort( $expected );
 
                // Actual listing (no trailing slash)
                $list = array();
@@ -573,8 +573,9 @@ class FileBackendTest extends MediaWikiTestCase {
                foreach ( $iter as $file ) {
                        $list[] = $file;
                }
+               sort( $list );
 
-               $this->assertEquals( $expected, sort( $list ), "Correct file listing." );
+               $this->assertEquals( $expected, $list, "Correct file listing." );
 
                // Actual listing (with trailing slash)
                $list = array();
@@ -582,8 +583,9 @@ class FileBackendTest extends MediaWikiTestCase {
                foreach ( $iter as $file ) {
                        $list[] = $file;
                }
+               sort( $list );
 
-               $this->assertEquals( $expected, sort( $list ), "Correct file listing." );
+               $this->assertEquals( $expected, $list, "Correct file listing." );
 
                foreach ( $files as $file ) {
                        $this->backend->doOperation( array( 'op' => 'delete', 'src' => "$base/$file" ) );