From 8641c9af9de6441d0c6a8a0093192a513888a8a9 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 22 Sep 2012 11:01:58 -0700 Subject: [PATCH] [FileBackend] Added getFileContentsMulti() and improved it for Swift. Change-Id: I6a2173eccda8fe7d4e2e779421e6edf05c8201b4 --- includes/filebackend/FileBackend.php | 24 ++++- .../filebackend/FileBackendMultiWrite.php | 12 ++- includes/filebackend/FileBackendStore.php | 35 +++++--- includes/filebackend/SwiftFileBackend.php | 89 ++++++++++++++----- .../includes/filerepo/FileBackendTest.php | 38 +++++--- 5 files changed, 147 insertions(+), 51 deletions(-) diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index 94c068cb65..364f2183fc 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -720,7 +720,29 @@ abstract class FileBackend { * - latest : use the latest available data * @return string|bool Returns false on failure */ - abstract public function getFileContents( array $params ); + final public function getFileContents( array $params ) { + $contents = $this->getFileContentsMulti( + array( 'srcs' => array( $params['src'] ) ) + $params ); + + return $contents[$params['src']]; + } + + /** + * Like getFileContents() except it takes an array of storage paths + * and returns a map of storage paths to strings (or null on failure). + * The map keys (paths) are in the same order as the provided list of paths. + * + * @see FileBackend::getFileContents() + * + * @param $params Array + * $params include: + * - srcs : list of source storage paths + * - latest : use the latest available data + * - parallelize : try to do operations in parallel when possible + * @return Array Map of (path name => string or false on failure) + * @since 1.20 + */ + abstract public function getFileContentsMulti( array $params ); /** * Get the size (bytes) of a file at a storage path in the backend. diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php index 4203878b4c..eec471fe37 100644 --- a/includes/filebackend/FileBackendMultiWrite.php +++ b/includes/filebackend/FileBackendMultiWrite.php @@ -572,13 +572,19 @@ class FileBackendMultiWrite extends FileBackend { } /** - * @see FileBackend::getFileContents() + * @see FileBackend::getFileContentsMulti() * @param $params array * @return bool|string */ - public function getFileContents( array $params ) { + public function getFileContentsMulti( array $params ) { $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); - return $this->backends[$this->masterIndex]->getFileContents( $realParams ); + $contentsM = $this->backends[$this->masterIndex]->getFileContentsMulti( $realParams ); + + $contents = array(); // (path => FSFile) mapping using the proxy backend's name + foreach ( $contentsM as $path => $data ) { + $contents[$this->unsubstPaths( $path )] = $data; + } + return $contents; } /** diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 21832be1fc..112f247614 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -636,24 +636,33 @@ abstract class FileBackendStore extends FileBackend { abstract protected function doGetFileStat( array $params ); /** - * @see FileBackend::getFileContents() - * @return bool|string + * @see FileBackend::getFileContentsMulti() + * @return Array */ - public function getFileContents( array $params ) { + public function getFileContentsMulti( array $params ) { wfProfileIn( __METHOD__ ); wfProfileIn( __METHOD__ . '-' . $this->name ); - $tmpFile = $this->getLocalReference( $params ); - if ( !$tmpFile ) { - wfProfileOut( __METHOD__ . '-' . $this->name ); - wfProfileOut( __METHOD__ ); - return false; - } - wfSuppressWarnings(); - $data = file_get_contents( $tmpFile->getPath() ); - wfRestoreWarnings(); + + $params = $this->setConcurrencyFlags( $params ); + $contents = $this->doGetFileContentsMulti( $params ); + wfProfileOut( __METHOD__ . '-' . $this->name ); wfProfileOut( __METHOD__ ); - return $data; + return $contents; + } + + /** + * @see FileBackendStore::getFileContentsMulti() + * @return Array + */ + protected function doGetFileContentsMulti( array $params ) { + $contents = array(); + wfSuppressWarnings(); + foreach ( $this->doGetLocalReferenceMulti( $params ) as $path => $fsFile ) { + $contents[$path] = $fsFile ? file_get_contents( $fsFile->getPath() ) : false; + } + wfRestoreWarnings(); + return $contents; } /** diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php index 0b74f56996..a1f5c74857 100644 --- a/includes/filebackend/SwiftFileBackend.php +++ b/includes/filebackend/SwiftFileBackend.php @@ -801,27 +801,77 @@ class SwiftFileBackend extends FileBackendStore { } /** - * @see FileBackend::getFileContents() - * @return bool|string + * @see FileBackendStore::doGetFileContentsMulti() + * @return Array */ - public function getFileContents( array $params ) { - list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] ); - if ( $srcRel === null ) { - return false; // invalid storage path - } + protected function doGetFileContentsMulti( array $params ) { + $contents = array(); - $data = false; - try { - $sContObj = $this->getContainer( $srcCont ); - $obj = new CF_Object( $sContObj, $srcRel, false, false ); // skip HEAD - $data = $obj->read( $this->headersFromParams( $params ) ); - } catch ( NoSuchContainerException $e ) { - } catch ( NoSuchObjectException $e ) { - } catch ( CloudFilesException $e ) { // some other exception? - $this->handleException( $e, null, __METHOD__, $params ); + $ep = array_diff_key( $params, array( 'srcs' => 1 ) ); // for error logging + // Blindly create tmp files and stream to them, catching any exception if the file does + // not exist. Doing a stat here is useless causes infinite loops in addMissingMetadata(). + foreach ( array_chunk( $params['srcs'], $params['concurrency'] ) as $pathBatch ) { + $cfOps = array(); // (path => CF_Async_Op) + + foreach ( $pathBatch as $path ) { // each path in this concurrent batch + list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $path ); + if ( $srcRel === null ) { + $contents[$path] = false; + continue; + } + $data = false; + try { + $sContObj = $this->getContainer( $srcCont ); + $obj = new CF_Object( $sContObj, $srcRel, false, false ); // skip HEAD + // Get source file extension + $ext = FileBackend::extensionFromPath( $path ); + // Create a new temporary memory file... + $handle = fopen( 'php://temp', 'wb' ); + if ( $handle ) { + $headers = $this->headersFromParams( $params ); + if ( count( $pathBatch ) > 1 ) { + $cfOps[$path] = $obj->stream_async( $handle, $headers ); + $cfOps[$path]->_file_handle = $handle; // close this later + } else { + $obj->stream( $handle, $headers ); + rewind( $handle ); // start from the beginning + $data = stream_get_contents( $handle ); + fclose( $handle ); + } + } else { + $data = false; + } + } catch ( NoSuchContainerException $e ) { + $data = false; + } catch ( NoSuchObjectException $e ) { + $data = false; + } catch ( CloudFilesException $e ) { // some other exception? + $data = false; + $this->handleException( $e, null, __METHOD__, array( 'src' => $path ) + $ep ); + } + $contents[$path] = $data; + } + + $batch = new CF_Async_Op_Batch( $cfOps ); + $cfOps = $batch->execute(); + foreach ( $cfOps as $path => $cfOp ) { + try { + $cfOp->getLastResponse(); + rewind( $cfOp->_file_handle ); // start from the beginning + $contents[$path] = stream_get_contents( $cfOp->_file_handle ); + } catch ( NoSuchContainerException $e ) { + $contents[$path] = false; + } catch ( NoSuchObjectException $e ) { + $contents[$path] = false; + } catch ( CloudFilesException $e ) { // some other exception? + $contents[$path] = false; + $this->handleException( $e, null, __METHOD__, array( 'src' => $path ) + $ep ); + } + fclose( $cfOp->_file_handle ); // close open handle + } } - return $data; + return $contents; } /** @@ -1041,7 +1091,6 @@ class SwiftFileBackend extends FileBackendStore { foreach ( array_chunk( $params['srcs'], $params['concurrency'] ) as $pathBatch ) { $cfOps = array(); // (path => CF_Async_Op) - $handles = array(); // open file handles for async ops foreach ( $pathBatch as $path ) { // each path in this concurrent batch list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $path ); if ( $srcRel === null ) { @@ -1062,7 +1111,7 @@ class SwiftFileBackend extends FileBackendStore { $headers = $this->headersFromParams( $params ); if ( count( $pathBatch ) > 1 ) { $cfOps[$path] = $obj->stream_async( $handle, $headers ); - $handles[] = $handle; // close this later + $cfOps[$path]->_file_handle = $handle; // close this later } else { $obj->stream( $handle, $headers ); fclose( $handle ); @@ -1095,8 +1144,8 @@ class SwiftFileBackend extends FileBackendStore { $tmpFiles[$path] = null; $this->handleException( $e, null, __METHOD__, array( 'src' => $path ) + $ep ); } + fclose( $cfOp->_file_handle ); // close open handle } - array_map( 'fclose', $handles ); // close open handles } return $tmpFiles; diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php index 33f1727e85..2de569b584 100644 --- a/tests/phpunit/includes/filerepo/FileBackendTest.php +++ b/tests/phpunit/includes/filerepo/FileBackendTest.php @@ -937,21 +937,27 @@ class FileBackendTest extends MediaWikiTestCase { private function doTestGetFileContents( $source, $content ) { $backendName = $this->backendClass(); - $this->prepare( array( 'dir' => dirname( $source ) ) ); - - $status = $this->backend->doOperation( - array( 'op' => 'create', 'content' => $content, 'dst' => $source ) ); - $this->assertGoodStatus( $status, - "Creation of file at $source succeeded ($backendName)." ); - $this->assertEquals( true, $status->isOK(), - "Creation of file at $source succeeded with OK status ($backendName)." ); - - $newContents = $this->backend->getFileContents( array( 'src' => $source, 'latest' => 1 ) ); - $this->assertNotEquals( false, $newContents, - "Read of file at $source succeeded ($backendName)." ); + $srcs = (array)$source; + foreach ( $srcs as $src ) { + $this->prepare( array( 'dir' => dirname( $src ) ) ); + $status = $this->backend->doOperation( + array( 'op' => 'create', 'content' => $content, 'dst' => $src ) ); + $this->assertGoodStatus( $status, + "Creation of file at $src succeeded ($backendName)." ); + } - $this->assertEquals( $content, $newContents, - "Contents read match data at $source ($backendName)." ); + if ( is_array( $source ) ) { + $contents = $this->backend->getFileContentsMulti( array( 'srcs' => $source ) ); + foreach ( $contents as $path => $data ) { + $this->assertNotEquals( false, $data, "Contents of $path exists ($backendName)." ); + $this->assertEquals( $content, $data, "Contents of $path is correct ($backendName)." ); + } + $this->assertEquals( $source, array_keys( $contents ), "Contents in right order ($backendName)." ); + } else { + $data = $this->backend->getFileContents( array( 'src' => $source ) ); + $this->assertNotEquals( false, $data, "Contents of $source exists ($backendName)." ); + $this->assertEquals( $content, $data, "Contents of $source is correct ($backendName)." ); + } } function provider_testGetFileContents() { @@ -960,6 +966,10 @@ class FileBackendTest extends MediaWikiTestCase { $base = $this->baseStorePath(); $cases[] = array( "$base/unittest-cont1/e/b/z/some_file.txt", "some file contents" ); $cases[] = array( "$base/unittest-cont1/e/b/some-other_file.txt", "more file contents" ); + $cases[] = array( + array( "$base/unittest-cont1/e/a/x.txt", "$base/unittest-cont1/e/a/y.txt", + "$base/unittest-cont1/e/a/z.txt" ), + "some clone file contents" ); return $cases; } -- 2.20.1