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

Reply via email to