https://www.mediawiki.org/wiki/Special:Code/MediaWiki/108355
Revision: 108355
Author: aaron
Date: 2012-01-08 09:25:15 +0000 (Sun, 08 Jan 2012)
Log Message:
-----------
* Follow-up r107170: Moved FileBackend::concatenate() outside of doOperations()
as it's own separate operation. It does not mutate storage files like the
others and having it in doOperations() broke FileBackendMultiWrite. This change
also makes overriding doOperationsInternal() easier (suching as using a custom
batch operation storage API).
* Added sanity check to FileBackendMultiWrite constructor.
* Moved FileBackend::create() function up a bit.
Modified Paths:
--------------
trunk/phase3/includes/filerepo/FileRepo.php
trunk/phase3/includes/filerepo/backend/FileBackend.php
trunk/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
trunk/phase3/includes/filerepo/backend/FileOp.php
trunk/phase3/tests/phpunit/includes/filerepo/FileBackendTest.php
Modified: trunk/phase3/includes/filerepo/FileRepo.php
===================================================================
--- trunk/phase3/includes/filerepo/FileRepo.php 2012-01-08 09:22:16 UTC (rev
108354)
+++ trunk/phase3/includes/filerepo/FileRepo.php 2012-01-08 09:25:15 UTC (rev
108355)
@@ -791,8 +791,6 @@
*/
function concatenate( $srcPaths, $dstPath, $flags = 0 ) {
$status = $this->newGood();
- // Resolve target to a storage path if virtual
- $dest = $this->resolveToStoragePath( $dstPath );
$sources = array();
$deleteOperations = array(); // post-concatenate ops
@@ -805,9 +803,9 @@
}
}
- // Concatenate the chunks into one file
- $op = array( 'op' => 'concatenate', 'srcs' => $sources, 'dst'
=> $dest );
- $status->merge( $this->backend->doOperation( $op ) );
+ // Concatenate the chunks into one FS file
+ $params = array( 'srcs' => $sources, 'dst' => $dstPath );
+ $status->merge( $this->backend->concatenate( $params ) );
if ( !$status->isOK() ) {
return $status;
}
Modified: trunk/phase3/includes/filerepo/backend/FileBackend.php
===================================================================
--- trunk/phase3/includes/filerepo/backend/FileBackend.php 2012-01-08
09:22:16 UTC (rev 108354)
+++ trunk/phase3/includes/filerepo/backend/FileBackend.php 2012-01-08
09:25:15 UTC (rev 108355)
@@ -132,14 +132,8 @@
* 'src' => <storage path>,
* 'ignoreMissingSource' => <boolean>
* )
- * f) Concatenate a list of files within storage into a single temp file
+ * f) Do nothing (no-op)
* array(
- * 'op' => 'concatenate',
- * 'srcs' => <ordered array of storage paths>,
- * 'dst' => <file system path to 0-byte temp
file>
- * )
- * g) Do nothing (no-op)
- * array(
* 'op' => 'null',
* )
*
@@ -203,6 +197,21 @@
}
/**
+ * Performs a single create operation.
+ * This sets $params['op'] to 'create' and passes it to doOperation().
+ *
+ * @see FileBackendBase::doOperation()
+ *
+ * @param $params Array Operation parameters
+ * @param $opts Array Operation options
+ * @return Status
+ */
+ final public function create( array $params, array $opts = array() ) {
+ $params['op'] = 'create';
+ return $this->doOperation( $params, $opts );
+ }
+
+ /**
* Performs a single store operation.
* This sets $params['op'] to 'store' and passes it to doOperation().
*
@@ -263,36 +272,17 @@
}
/**
- * Performs a single create operation.
- * This sets $params['op'] to 'create' and passes it to doOperation().
+ * Concatenate a list of storage files into a single file on the file
system
+ * $params include:
+ * srcs : ordered source storage paths (e.g. chunk1,
chunk2, ...)
+ * dst : file system path to 0-byte temp file
*
- * @see FileBackendBase::doOperation()
- *
* @param $params Array Operation parameters
- * @param $opts Array Operation options
* @return Status
*/
- final public function create( array $params, array $opts = array() ) {
- $params['op'] = 'create';
- return $this->doOperation( $params, $opts );
- }
+ abstract public function concatenate( array $params );
/**
- * Performs a single concatenate operation.
- * This sets $params['op'] to 'concatenate' and passes it to
doOperation().
- *
- * @see FileBackendBase::doOperation()
- *
- * @param $params Array Operation parameters
- * @param $opts Array Operation options
- * @return Status
- */
- final public function concatenate( array $params, array $opts = array()
) {
- $params['op'] = 'concatenate';
- return $this->doOperation( $params, $opts );
- }
-
- /**
* Prepare a storage path for usage. This will create containers
* that don't yet exist or, on FS backends, create parent directories.
*
@@ -677,24 +667,27 @@
}
/**
- * Combines files from several storage paths into a new file in the
backend.
- * Do not call this function from places outside FileBackend and FileOp.
- * $params include:
- * srcs : ordered source storage paths (e.g. chunk1,
chunk2, ...)
- * dst : file system path to 0-byte temp file
- *
- * @param $params Array
- * @return Status
+ * @see FileBackendBase::concatenate()
*/
- final public function concatenateInternal( array $params ) {
- $status = $this->doConcatenateInternal( $params );
+ final public function concatenate( array $params ) {
+ $status = Status::newGood();
+
+ // Try to lock the source files for the scope of this function
+ $scopeLockS = $this->getScopedFileLocks( $params['srcs'],
LockManager::LOCK_UW, $status );
+ if ( !$status->isOK() ) {
+ return $status; // abort
+ }
+
+ // Actually do the concatenation
+ $status->merge( $this->doConcatenate( $params ) );
+
return $status;
}
/**
- * @see FileBackend::concatenateInternal()
+ * @see FileBackend::concatenate()
*/
- protected function doConcatenateInternal( array $params ) {
+ protected function doConcatenate( array $params ) {
$status = Status::newGood();
$tmpPath = $params['dst']; // convenience
@@ -1035,7 +1028,6 @@
'copy' => 'CopyFileOp',
'move' => 'MoveFileOp',
'delete' => 'DeleteFileOp',
- 'concatenate' => 'ConcatenateFileOp',
'create' => 'CreateFileOp',
'null' => 'NullFileOp'
);
Modified: trunk/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
===================================================================
--- trunk/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
2012-01-08 09:22:16 UTC (rev 108354)
+++ trunk/phase3/includes/filerepo/backend/FileBackendMultiWrite.php
2012-01-08 09:25:15 UTC (rev 108355)
@@ -36,9 +36,15 @@
*/
public function __construct( array $config ) {
parent::__construct( $config );
+ $namesUsed = array();
// Construct backends here rather than via registration
// to keep these backends hidden from outside the proxy.
foreach ( $config['backends'] as $index => $config ) {
+ $name = $config['name'];
+ if ( isset( $namesUsed[$name] ) ) { // don't break
FileOp predicates
+ throw new MWException( "Two or more backends
defined with the name $name." );
+ }
+ $namesUsed[$name] = 1;
if ( !isset( $config['class'] ) ) {
throw new MWException( 'No class given for a
backend config.' );
}
@@ -245,6 +251,15 @@
}
/**
+ * @see FileBackendBase::getFileList()
+ */
+ public function concatenate( array $params ) {
+ // We are writing to an FS file, so we don't need to do this
per-backend
+ $realParams = $this->substOpPaths( $params,
$this->backends[$this->masterIndex] );
+ return $this->backends[$this->masterIndex]->concatenate(
$realParams );
+ }
+
+ /**
* @see FileBackendBase::fileExists()
*/
public function fileExists( array $params ) {
Modified: trunk/phase3/includes/filerepo/backend/FileOp.php
===================================================================
--- trunk/phase3/includes/filerepo/backend/FileOp.php 2012-01-08 09:22:16 UTC
(rev 108354)
+++ trunk/phase3/includes/filerepo/backend/FileOp.php 2012-01-08 09:25:15 UTC
(rev 108355)
@@ -808,61 +808,6 @@
}
/**
- * Combines files from severals storage paths into a new file in the backend.
- * Parameters similar to FileBackend::concatenate(), which include:
- * srcs : ordered source storage paths (e.g. chunk1, chunk2, ...)
- * dst : destination file system path to 0-byte temp file
- */
-class ConcatenateFileOp extends FileOp {
- protected function allowedParams() {
- return array( 'srcs', 'dst' );
- }
-
- protected function doPrecheck( array &$predicates ) {
- $status = Status::newGood();
- // Check destination temp file
- wfSuppressWarnings();
- $ok = ( is_file( $this->params['dst'] ) && !filesize(
$this->params['dst'] ) );
- wfRestoreWarnings();
- if ( !$ok ) { // not present or not empty
- $status->fatal( 'backend-fail-opentemp',
$this->params['dst'] );
- return $status;
- }
- // Check that source files exists
- foreach ( $this->params['srcs'] as $source ) {
- if ( !$this->fileExists( $source, $predicates ) ) {
- $status->fatal( 'backend-fail-notexists',
$source );
- return $status;
- }
- }
- return $status;
- }
-
- protected function doAttempt() {
- $status = Status::newGood();
- // Concatenate the file at the destination
- $status->merge( $this->backend->concatenateInternal(
$this->params ) );
- return $status;
- }
-
- protected function doRevert() {
- $status = Status::newGood();
- // Clear out the temp file back to 0-bytes
- wfSuppressWarnings();
- $ok = file_put_contents( $this->params['dst'], '' );
- wfRestoreWarnings();
- if ( !$ok ) {
- $status->fatal( 'backend-fail-writetemp',
$this->params['dst'] );
- }
- return $status;
- }
-
- public function storagePathsRead() {
- return $this->params['srcs'];
- }
-}
-
-/**
* Delete a file at the storage path.
* Parameters similar to FileBackend::delete(), which include:
* src : source storage path
Modified: trunk/phase3/tests/phpunit/includes/filerepo/FileBackendTest.php
===================================================================
--- trunk/phase3/tests/phpunit/includes/filerepo/FileBackendTest.php
2012-01-08 09:22:16 UTC (rev 108354)
+++ trunk/phase3/tests/phpunit/includes/filerepo/FileBackendTest.php
2012-01-08 09:25:15 UTC (rev 108355)
@@ -437,13 +437,12 @@
$this->doTestConcatenate( $op, $srcs, $srcsContent,
$alreadyExists, $okStatus );
$this->tearDownFiles();
- # FIXME
- #$this->backend = $this->multiBackend;
- #$this->doTestConcatenate( $op, $srcs, $srcsContent,
$alreadyExists, $okStatus );
- #$this->tearDownFiles();
+ $this->backend = $this->multiBackend;
+ $this->doTestConcatenate( $op, $srcs, $srcsContent,
$alreadyExists, $okStatus );
+ $this->tearDownFiles();
}
- public function doTestConcatenate( $op, $srcs, $srcsContent,
$alreadyExists, $okStatus ) {
+ public function doTestConcatenate( $params, $srcs, $srcsContent,
$alreadyExists, $okStatus ) {
$backendName = $this->backendClass();
$expContent = '';
@@ -462,7 +461,7 @@
$this->assertEquals( true, $status->isOK(),
"Creation of source files succeeded ($backendName)." );
- $dest = $op['dst'];
+ $dest = $params['dst'];
if ( $alreadyExists ) {
$ok = file_put_contents( $dest,
'blah...blah...waahwaah' ) !== false;
$this->assertEquals( true, $ok,
@@ -473,8 +472,8 @@
"Creation of 0-byte file at $dest succeeded
($backendName)." );
}
- // Combine them
- $status = $this->backend->doOperation( $op );
+ // Combine the files into one
+ $status = $this->backend->concatenate( $params );
if ( $okStatus ) {
$this->assertEquals( array(), $status->errors,
"Creation of concat file at $dest succeeded
without warnings ($backendName)." );
@@ -534,10 +533,10 @@
'lkaem;a',
'legma'
);
- $op = array( 'op' => 'concatenate', 'srcs' => $srcs, 'dst' =>
$dest );
+ $params = array( 'srcs' => $srcs, 'dst' => $dest );
$cases[] = array(
- $op, // operation
+ $params, // operation
$srcs, // sources
$content, // content for each source
false, // no dest already exists
@@ -545,7 +544,7 @@
);
$cases[] = array(
- $op, // operation
+ $params, // operation
$srcs, // sources
$content, // content for each source
true, // dest already exists
_______________________________________________
MediaWiki-CVS mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-cvs