[FileBackend] Avoid an extra RTT on Swift listing operations.

* Also tweaked timestamp tolerance in tests, intended for Swift
* Also made a few tweaks to speed up tests a bit

Change-Id: Ibdee36d3bf86089b027dc74bb2582acc1ab4b96b

diff --git a/includes/filerepo/backend/SwiftFileBackend.php b/includes/filerepo/backend/SwiftFileBackend.php
index 2520813..36d4334 100644 (file)
--- a/includes/filerepo/backend/SwiftFileBackend.php
+++ b/includes/filerepo/backend/SwiftFileBackend.php
@@ -741,9 +741,12 @@ class SwiftFileBackend extends FileBackendStore {
         * @return Array List of relative paths of dirs directly under $dir
         */
        public function getDirListPageInternal( $fullCont, $dir, &$after, $limit, array $params ) {
+               $dirs = array();
+               if ( $after === INF ) {
+                       return $dirs; // nothing more
+               }
                wfProfileIn( __METHOD__ . '-' . $this->name );
 
-               $dirs = array();
                try {
                        $container = $this->getContainer( $fullCont );
                        $prefix = ( $dir == '' ) ? null : "{$dir}/";
@@ -754,7 +757,6 @@ class SwiftFileBackend extends FileBackendStore {
                                        if ( substr( $object, -1 ) === '/' ) {
                                                $dirs[] = $object; // directories end in '/'
                                        }
-                                       $after = $object; // update last item
                                }
                        // Recursive: list all dirs under $dir and its subdirs
                        } else {
@@ -780,9 +782,13 @@ class SwiftFileBackend extends FileBackendStore {
                                                }
                                                $lastDir = $objectDir;
                                        }
-                                       $after = $object; // update last item
                                }
                        }
+                       if ( count( $objects ) < $limit ) {
+                               $after = INF; // avoid a second RTT
+                       } else {
+                               $after = end( $objects ); // update last item
+                       }
                } catch ( NoSuchContainerException $e ) {
                } catch ( CloudFilesException $e ) { // some other exception?
                        $this->handleException( $e, null, __METHOD__,
@@ -808,9 +814,12 @@ class SwiftFileBackend extends FileBackendStore {
         * @return Array List of relative paths of files under $dir
         */
        public function getFileListPageInternal( $fullCont, $dir, &$after, $limit, array $params ) {
+               $files = array();
+               if ( $after === INF ) {
+                       return $files; // nothing more
+               }
                wfProfileIn( __METHOD__ . '-' . $this->name );
 
-               $files = array();
                try {
                        $container = $this->getContainer( $fullCont );
                        $prefix = ( $dir == '' ) ? null : "{$dir}/";
@@ -824,10 +833,14 @@ class SwiftFileBackend extends FileBackendStore {
                                }
                        // Recursive: list all files under $dir and its subdirs
                        } else { // files
-                               $files = $container->list_objects( $limit, $after, $prefix );
+                               $objects = $container->list_objects( $limit, $after, $prefix );
+                               $files = $objects;
+                       }
+                       if ( count( $objects ) < $limit ) {
+                               $after = INF; // avoid a second RTT
+                       } else {
+                               $after = end( $objects ); // update last item
                        }
-                       $after = end( $files ); // update last item
-                       reset( $files ); // reset pointer
                } catch ( NoSuchContainerException $e ) {
                } catch ( CloudFilesException $e ) { // some other exception?
                        $this->handleException( $e, null, __METHOD__,

diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php
index 2a9608e..2b94778 100644 (file)
--- a/tests/phpunit/includes/filerepo/FileBackendTest.php
+++ b/tests/phpunit/includes/filerepo/FileBackendTest.php
@@ -850,7 +850,7 @@ class FileBackendTest extends MediaWikiTestCase {
 
                if ( $alreadyExists ) {
                        $this->prepare( array( 'dir' => dirname( $path ) ) );
-                       $status = $this->backend->create( array( 'dst' => $path, 'content' => $content ) );
+                       $status = $this->create( array( 'dst' => $path, 'content' => $content ) );
                        $this->assertGoodStatus( $status,
                                "Creation of file at $path succeeded ($backendName)." );
 
@@ -860,14 +860,14 @@ class FileBackendTest extends MediaWikiTestCase {
 
                        $this->assertEquals( strlen( $content ), $size,
                                "Correct file size of '$path'" );
-                       $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 5,
+                       $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 10,
                                "Correct file timestamp of '$path'" );
 
                        $size = $stat['size'];
                        $time = $stat['mtime'];
                        $this->assertEquals( strlen( $content ), $size,
                                "Correct file size of '$path'" );
-                       $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 5,
+                       $this->assertTrue( abs( time() - wfTimestamp( TS_UNIX, $time ) ) < 10,
                                "Correct file timestamp of '$path'" );
                } else {
                        $size = $this->backend->getFileSize( array( 'src' => $path ) );
@@ -999,8 +999,7 @@ class FileBackendTest extends MediaWikiTestCase {
 
                $this->prepare( array( 'dir' => dirname( $source ) ) );
 
-               $status = $this->backend->doOperation(
-                       array( 'op' => 'create', 'content' => $content, 'dst' => $source ) );
+               $status = $this->create( array( 'content' => $content, 'dst' => $source ) );
                $this->assertGoodStatus( $status,
                        "Creation of file at $source succeeded ($backendName)." );
 
@@ -1169,11 +1168,11 @@ class FileBackendTest extends MediaWikiTestCase {
                $fileD = "$base/unittest-cont1/a/b/fileD.txt";
 
                $this->prepare( array( 'dir' => dirname( $fileA ) ) );
-               $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
+               $this->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
                $this->prepare( array( 'dir' => dirname( $fileB ) ) );
-               $this->backend->create( array( 'dst' => $fileB, 'content' => $fileBContents ) );
+               $this->create( array( 'dst' => $fileB, 'content' => $fileBContents ) );
                $this->prepare( array( 'dir' => dirname( $fileC ) ) );
-               $this->backend->create( array( 'dst' => $fileC, 'content' => $fileCContents ) );
+               $this->create( array( 'dst' => $fileC, 'content' => $fileCContents ) );
                $this->prepare( array( 'dir' => dirname( $fileD ) ) );
 
                $status = $this->backend->doOperations( array(
@@ -1255,7 +1254,7 @@ class FileBackendTest extends MediaWikiTestCase {
                $fileD = "$base/unittest-cont1/a/b/fileD.txt";
 
                $this->prepare( array( 'dir' => dirname( $fileA ) ) );
-               $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
+               $this->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
                $this->prepare( array( 'dir' => dirname( $fileB ) ) );
                $this->prepare( array( 'dir' => dirname( $fileC ) ) );
                $this->prepare( array( 'dir' => dirname( $fileD ) ) );
@@ -1329,11 +1328,11 @@ class FileBackendTest extends MediaWikiTestCase {
                $fileD = "$base/unittest-cont2/a/b/fileD.txt";
 
                $this->prepare( array( 'dir' => dirname( $fileA ) ) );
-               $this->backend->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
+               $this->create( array( 'dst' => $fileA, 'content' => $fileAContents ) );
                $this->prepare( array( 'dir' => dirname( $fileB ) ) );
-               $this->backend->create( array( 'dst' => $fileB, 'content' => $fileBContents ) );
+               $this->create( array( 'dst' => $fileB, 'content' => $fileBContents ) );
                $this->prepare( array( 'dir' => dirname( $fileC ) ) );
-               $this->backend->create( array( 'dst' => $fileC, 'content' => $fileCContents ) );
+               $this->create( array( 'dst' => $fileC, 'content' => $fileCContents ) );
 
                $status = $this->backend->doOperations( array(
                        array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC, 'overwrite' => 1 ),
@@ -1418,7 +1417,7 @@ class FileBackendTest extends MediaWikiTestCase {
                        $this->prepare( array( 'dir' => dirname( $file ) ) );
                        $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file );
                }
-               $status = $this->backend->doOperations( $ops );
+               $status = $this->backend->doQuickOperations( $ops );
                $this->assertGoodStatus( $status,
                        "Creation of files succeeded ($backendName)." );
                $this->assertEquals( true, $status->isOK(),
@@ -1571,7 +1570,7 @@ class FileBackendTest extends MediaWikiTestCase {
                        $this->prepare( array( 'dir' => dirname( $file ) ) );
                        $ops[] = array( 'op' => 'create', 'content' => 'xxy', 'dst' => $file );
                }
-               $status = $this->backend->doOperations( $ops );
+               $status = $this->backend->doQuickOperations( $ops );
                $this->assertGoodStatus( $status,
                        "Creation of files succeeded ($backendName)." );
                $this->assertEquals( true, $status->isOK(),
@@ -1771,6 +1770,12 @@ class FileBackendTest extends MediaWikiTestCase {
                return $this->backend->prepare( $params );
        }
 
+       // test helper wrapper for backend prepare() function
+       private function create( array $params ) {
+               $params['op'] = 'create';
+               return $this->backend->doQuickOperations( array( $params ) );
+       }
+
        function tearDownFiles() {
                foreach ( $this->filesToPrune as $file ) {
                        @unlink( $file );