[FileBackend] Added support for changing headers on existing objects.

* Added a 'describe' file operation type to doOperations()/doQuickOperations().
  This can be used by scripts to fill in headers like X-Content-Duration for
  files that already exists.
* Removed wrong comments about removing headers (they don't get removed with null).
* Added some quick unit tests.

Change-Id: I43c5907b59421beaa9487eefac0cdbf8bc6c6d85

diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php
index a49d901..8e2447d 100644 (file)
--- a/includes/AutoLoader.php
+++ b/includes/AutoLoader.php
@@ -580,6 +580,7 @@ $wgAutoloadLocalClasses = array(
        'MoveFileOp' => 'includes/filebackend/FileOp.php',
        'DeleteFileOp' => 'includes/filebackend/FileOp.php',
        'CreateFileOp' => 'includes/filebackend/FileOp.php',
+       'DescribeFileOp' => 'includes/filebackend/FileOp.php',
        'NullFileOp' => 'includes/filebackend/FileOp.php',
 
        # includes/filerepo

diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php
index 25126a3..ef49fe5 100644 (file)
--- a/includes/filebackend/FileBackend.php
+++ b/includes/filebackend/FileBackend.php
@@ -172,6 +172,7 @@ abstract class FileBackend {
         *  - copy
         *  - move
         *  - delete
+        *  - describe (since 1.21)
         *  - null
         *
         * a) Create a new file in storage with the contents of a string
@@ -235,7 +236,17 @@ abstract class FileBackend {
         *     )
         * @endcode
         *
-        * f) Do nothing (no-op)
+        * f) Update metadata for a file within storage
+        * @code
+        *     array(
+        *         'op'                  => 'describe',
+        *         'src'                 => <storage path>,
+        *         'disposition'         => <Content-Disposition header value>,
+        *         'headers'             => <HTTP header name/value map>
+        *     )
+        * @endcode
+        *
+        * g) Do nothing (no-op)
         * @code
         *     array(
         *         'op'                  => 'null',
@@ -409,6 +420,7 @@ abstract class FileBackend {
         *  - copy
         *  - move
         *  - delete
+        *  - describe (since 1.21)
         *  - null
         *
         * a) Create a new file in storage with the contents of a string
@@ -421,6 +433,7 @@ abstract class FileBackend {
         *         'headers'             => <HTTP header name/value map> # since 1.21
         *     )
         * @endcode
+        *
         * b) Copy a file system file into storage
         * @code
         *     array(
@@ -431,6 +444,7 @@ abstract class FileBackend {
         *         'headers'             => <HTTP header name/value map> # since 1.21
         *     )
         * @endcode
+        *
         * c) Copy a file within storage
         * @code
         *     array(
@@ -441,6 +455,7 @@ abstract class FileBackend {
         *         'disposition'         => <Content-Disposition header value>
         *     )
         * @endcode
+        *
         * d) Move a file within storage
         * @code
         *     array(
@@ -451,6 +466,7 @@ abstract class FileBackend {
         *         'disposition'         => <Content-Disposition header value>
         *     )
         * @endcode
+        *
         * e) Delete a file within storage
         * @code
         *     array(
@@ -459,7 +475,18 @@ abstract class FileBackend {
         *         'ignoreMissingSource' => <boolean>
         *     )
         * @endcode
-        * f) Do nothing (no-op)
+        *
+        * f) Update metadata for a file within storage
+        * @code
+        *     array(
+        *         'op'                  => 'describe',
+        *         'src'                 => <storage path>,
+        *         'disposition'         => <Content-Disposition header value>,
+        *         'headers'             => <HTTP header name/value map>
+        *     )
+        * @endcode
+        *
+        * g) Do nothing (no-op)
         * @code
         *     array(
         *         'op'                  => 'null',

diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php
index 2d7fa07..9bbcc0a 100644 (file)
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -90,13 +90,13 @@ abstract class FileBackendStore extends FileBackend {
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
-        *   - content       : the raw file contents
-        *   - dst           : destination storage path
-        *   - disposition   : Content-Disposition header value for the destination
-        *   - headers       : HTTP header name/value map
-        *   - async         : Status will be returned immediately if supported.
-        *                     If the status is OK, then its value field will be
-        *                     set to a FileBackendStoreOpHandle object.
+        *   - content     : the raw file contents
+        *   - dst         : destination storage path
+        *   - disposition : Content-Disposition header value for the destination
+        *   - headers     : HTTP header name/value map
+        *   - async       : Status will be returned immediately if supported.
+        *                   If the status is OK, then its value field will be
+        *                   set to a FileBackendStoreOpHandle object.
         *
         * @param $params Array
         * @return Status
@@ -119,6 +119,7 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * @see FileBackendStore::createInternal()
+        * @return Status
         */
        abstract protected function doCreateInternal( array $params );
 
@@ -128,13 +129,13 @@ abstract class FileBackendStore extends FileBackend {
         * Do not call this function from places outside FileBackend and FileOp.
         *
         * $params include:
-        *   - src           : source path on disk
-        *   - dst           : destination storage path
-        *   - disposition   : Content-Disposition header value for the destination
-        *   - headers       : HTTP header name/value map
-        *   - async         : Status will be returned immediately if supported.
-        *                     If the status is OK, then its value field will be
-        *                     set to a FileBackendStoreOpHandle object.
+        *   - src         : source path on disk
+        *   - dst         : destination storage path
+        *   - disposition : Content-Disposition header value for the destination
+        *   - headers     : HTTP header name/value map
+        *   - async       : Status will be returned immediately if supported.
+        *                   If the status is OK, then its value field will be
+        *                   set to a FileBackendStoreOpHandle object.
         *
         * @param $params Array
         * @return Status
@@ -157,6 +158,7 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * @see FileBackendStore::storeInternal()
+        * @return Status
         */
        abstract protected function doStoreInternal( array $params );
 
@@ -190,6 +192,7 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * @see FileBackendStore::copyInternal()
+        * @return Status
         */
        abstract protected function doCopyInternal( array $params );
 
@@ -220,6 +223,7 @@ abstract class FileBackendStore extends FileBackend {
 
        /**
         * @see FileBackendStore::deleteInternal()
+        * @return Status
         */
        abstract protected function doDeleteInternal( array $params );
 
@@ -268,6 +272,40 @@ abstract class FileBackendStore extends FileBackend {
                return $status;
        }
 
+       /**
+        * Alter metadata for a file at the storage path.
+        * Do not call this function from places outside FileBackend and FileOp.
+        *
+        * $params include:
+        *   - src           : source storage path
+        *   - disposition   : Content-Disposition header value for the destination
+        *   - headers       : HTTP header name/value map
+        *   - async         : Status will be returned immediately if supported.
+        *                     If the status is OK, then its value field will be
+        *                     set to a FileBackendStoreOpHandle object.
+        *
+        * @param $params Array
+        * @return Status
+        */
+       final public function describeInternal( array $params ) {
+               wfProfileIn( __METHOD__ );
+               wfProfileIn( __METHOD__ . '-' . $this->name );
+               $status = $this->doDescribeInternal( $params );
+               $this->clearCache( array( $params['src'] ) );
+               $this->deleteFileCache( $params['src'] ); // persistent cache
+               wfProfileOut( __METHOD__ . '-' . $this->name );
+               wfProfileOut( __METHOD__ );
+               return $status;
+       }
+
+       /**
+        * @see FileBackendStore::describeInternal()
+        * @return Status
+        */
+       protected function doDescribeInternal( array $params ) {
+               return Status::newGood();
+       }
+
        /**
         * No-op file operation that does nothing.
         * Do not call this function from places outside FileBackend and FileOp.
@@ -997,12 +1035,13 @@ abstract class FileBackendStore extends FileBackend {
         */
        final public function getOperationsInternal( array $ops ) {
                $supportedOps = array(
-                       'store'       => 'StoreFileOp',
-                       'copy'        => 'CopyFileOp',
-                       'move'        => 'MoveFileOp',
-                       'delete'      => 'DeleteFileOp',
-                       'create'      => 'CreateFileOp',
-                       'null'        => 'NullFileOp'
+                       'store'    => 'StoreFileOp',
+                       'copy'     => 'CopyFileOp',
+                       'move'     => 'MoveFileOp',
+                       'delete'   => 'DeleteFileOp',
+                       'create'   => 'CreateFileOp',
+                       'describe' => 'DescribeFileOp',
+                       'null'     => 'NullFileOp'
                );
 
                $performOps = array(); // array of FileOp objects
@@ -1229,6 +1268,9 @@ abstract class FileBackendStore extends FileBackend {
                                if ( strlen( $name ) > 255 || strlen( $value ) > 255 ) {
                                        trigger_error( "Header '$name: $value' is too long." );
                                        unset( $op['headers'][$name] );
+                               } elseif ( !strlen( $value ) ) {
+                                       trigger_error( "Header value for '$name' is empty." );
+                                       unset( $op['headers'][$name] ); // ignore
                                }
                        }
                }

diff --git a/includes/filebackend/FileOp.php b/includes/filebackend/FileOp.php
index 60be5ee..bfa2757 100644 (file)
--- a/includes/filebackend/FileOp.php
+++ b/includes/filebackend/FileOp.php
@@ -809,6 +809,58 @@ class DeleteFileOp extends FileOp {
        }
 }
 
+/**
+ * Change metadata for a file at the given storage path in the backend.
+ * Parameters for this operation are outlined in FileBackend::doOperations().
+ */
+class DescribeFileOp extends FileOp {
+       /**
+        * @return array
+        */
+       protected function allowedParams() {
+               return array( array( 'src' ), array( 'disposition', 'headers' ) );
+       }
+
+       /**
+        * @param $predicates array
+        * @return Status
+        */
+       protected function doPrecheck( array &$predicates ) {
+               $status = Status::newGood();
+               // Check if the source file exists
+               if ( !$this->fileExists( $this->params['src'], $predicates ) ) {
+                       $status->fatal( 'backend-fail-notexists', $this->params['src'] );
+                       return $status;
+               // Check if a file can be placed/changed at the source
+               } elseif ( !$this->backend->isPathUsableInternal( $this->params['src'] ) ) {
+                       $status->fatal( 'backend-fail-usable', $this->params['src'] );
+                       $status->fatal( 'backend-fail-describe', $this->params['src'] );
+                       return $status;
+               }
+               // Update file existence predicates
+               $predicates['exists'][$this->params['src']] =
+                       $this->fileExists( $this->params['src'], $predicates );
+               $predicates['sha1'][$this->params['src']] =
+                       $this->fileSha1( $this->params['src'], $predicates );
+               return $status; // safe to call attempt()
+       }
+
+       /**
+        * @return Status
+        */
+       protected function doAttempt() {
+               // Update the source file's metadata
+               return $this->backend->describeInternal( $this->setFlags( $this->params ) );
+       }
+
+       /**
+        * @return array
+        */
+       public function storagePathsChanged() {
+               return array( $this->params['src'] );
+       }
+}
+
 /**
  * Placeholder operation that has no params and does nothing
  */

diff --git a/includes/filebackend/SwiftFileBackend.php b/includes/filebackend/SwiftFileBackend.php
index 624374b..6a27e9b 100644 (file)
--- a/includes/filebackend/SwiftFileBackend.php
+++ b/includes/filebackend/SwiftFileBackend.php
@@ -562,6 +562,45 @@ class SwiftFileBackend extends FileBackendStore {
                }
        }
 
+       /**
+        * @see FileBackendStore::doDescribeInternal()
+        * @return Status
+        */
+       protected function doDescribeInternal( array $params ) {
+               $status = Status::newGood();
+
+               list( $srcCont, $srcRel ) = $this->resolveStoragePathReal( $params['src'] );
+               if ( $srcRel === null ) {
+                       $status->fatal( 'backend-fail-invalidpath', $params['src'] );
+                       return $status;
+               }
+
+               $hdrs = isset( $params['headers'] ) ? $params['headers'] : array();
+               // Set the Content-Disposition header if requested
+               if ( isset( $params['disposition'] ) ) {
+                       $hdrs['Content-Disposition'] = $this->truncDisp( $params['disposition'] );
+               }
+
+               try {
+                       $sContObj = $this->getContainer( $srcCont );
+                       $srcObj = new CF_Object( $sContObj, $srcRel, false, false ); // skip HEAD
+                       // Merge in the new metadata and header values...
+                       $srcObj->headers = $hdrs;
+                       $srcObj->sync_metadata(); // save to Swift
+                       $this->purgeCDNCache( array( $srcObj ) );
+               } catch ( CDNNotEnabledException $e ) {
+                       // CDN not enabled; nothing to see here
+               } catch ( NoSuchContainerException $e ) {
+                       $status->fatal( 'backend-fail-describe', $params['src'] );
+               } catch ( NoSuchObjectException $e ) {
+                       $status->fatal( 'backend-fail-describe', $params['src'] );
+               } catch ( CloudFilesException $e ) { // some other exception?
+                       $this->handleException( $e, $status, __METHOD__, $params );
+               }
+
+               return $status;
+       }
+
        /**
         * @see FileBackendStore::doPrepareInternal()
         * @return Status

diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php
index 5e33d62..861b145 100644 (file)
--- a/languages/messages/MessagesEn.php
+++ b/languages/messages/MessagesEn.php
@@ -2315,6 +2315,7 @@ If the problem persists, contact an [[Special:ListUsers/sysop|administrator]].',
 'backend-fail-notsame'       => 'A non-identical file already exists at "$1".',
 'backend-fail-invalidpath'   => '"$1" is not a valid storage path.',
 'backend-fail-delete'        => 'Could not delete file "$1".',
+'backend-fail-describe'      => 'Could not change metadata for file "$1".',
 'backend-fail-alreadyexists' => 'The file "$1" already exists.',
 'backend-fail-store'         => 'Could not store file "$1" at "$2".',
 'backend-fail-copy'          => 'Could not copy file "$1" to "$2".',

diff --git a/languages/messages/MessagesQqq.php b/languages/messages/MessagesQqq.php
index 8429171..b6b3ecb 100644 (file)
--- a/languages/messages/MessagesQqq.php
+++ b/languages/messages/MessagesQqq.php
@@ -2106,6 +2106,8 @@ Extensions making use of it:
 * $1 is a storage path.',
 'backend-fail-delete' => 'Parameters:
 * $1 is a file path.',
+'backend-fail-describe' => 'Parameters:
+* $1 is a file path.',
 'backend-fail-alreadyexists' => 'Parameters:
 * $1 is a filename.',
 'backend-fail-store' => 'Parameters:
@@ -3694,7 +3696,7 @@ Parameters:
 Start with a lowercase letter, unless the first word is “SVG”.',
 'svg-long-desc-animated' => 'Displayed under an SVG image at the image description page if the image is animated. Non-animated images use {{msg-mw|svg-long-desc}}.
 * $1 is the width in pixels
-* $2 is the height in pixels, and 
+* $2 is the height in pixels, and
 * $3 is the file size including a unit (for example "10 KB").
 
 Start with a lowercase letter, unless the first word is “SVG”.',

diff --git a/maintenance/language/messages.inc b/maintenance/language/messages.inc
index d7a396f..ddf2369 100644 (file)
--- a/maintenance/language/messages.inc
+++ b/maintenance/language/messages.inc
@@ -1399,6 +1399,7 @@ $wgMessageStructure = array(
                'backend-fail-notsame',
                'backend-fail-invalidpath',
                'backend-fail-delete',
+               'backend-fail-describe',
                'backend-fail-alreadyexists',
                'backend-fail-store',
                'backend-fail-copy',

diff --git a/tests/phpunit/includes/filebackend/FileBackendTest.php b/tests/phpunit/includes/filebackend/FileBackendTest.php
index add7066..5369112 100644 (file)
--- a/tests/phpunit/includes/filebackend/FileBackendTest.php
+++ b/tests/phpunit/includes/filebackend/FileBackendTest.php
@@ -585,6 +585,73 @@ class FileBackendTest extends MediaWikiTestCase {
                return $cases;
        }
 
+       /**
+        * @dataProvider provider_testDescribe
+        */
+       public function testDescribe( $op, $withSource, $okStatus ) {
+               $this->backend = $this->singleBackend;
+               $this->tearDownFiles();
+               $this->doTestDescribe( $op, $withSource, $okStatus );
+               $this->tearDownFiles();
+
+               $this->backend = $this->multiBackend;
+               $this->tearDownFiles();
+               $this->doTestDescribe( $op, $withSource, $okStatus );
+               $this->tearDownFiles();
+       }
+
+       private function doTestDescribe( $op, $withSource, $okStatus ) {
+               $backendName = $this->backendClass();
+
+               $source = $op['src'];
+               $this->prepare( array( 'dir' => dirname( $source ) ) );
+
+               if ( $withSource ) {
+                       $status = $this->backend->doOperation(
+                               array( 'op' => 'create', 'content' => 'blahblah', 'dst' => $source ) );
+                       $this->assertGoodStatus( $status,
+                               "Creation of file at $source succeeded ($backendName)." );
+               }
+
+               $status = $this->backend->doOperation( $op );
+               if ( $okStatus ) {
+                       $this->assertGoodStatus( $status,
+                               "Describe of file at $source succeeded without warnings ($backendName)." );
+                       $this->assertEquals( true, $status->isOK(),
+                               "Describe of file at $source succeeded ($backendName)." );
+                       $this->assertEquals( array( 0 => true ), $status->success,
+                               "Describe of file at $source has proper 'success' field in Status ($backendName)." );
+               } else {
+                       $this->assertEquals( false, $status->isOK(),
+                               "Describe of file at $source failed ($backendName)." );
+               }
+
+               $this->assertBackendPathsConsistent( array( $source ) );
+       }
+
+       public static function provider_testDescribe() {
+               $cases = array();
+
+               $source = self::baseStorePath() . '/unittest-cont1/e/myfacefile.txt';
+
+               $op = array( 'op' => 'describe', 'src' => $source,
+                       'headers' => array( 'X-Content-Length' => '91.3' ),
+                       'disposition' => 'inline' );
+               $cases[] = array(
+                       $op, // operation
+                       true, // with source
+                       true // succeeds
+               );
+
+               $cases[] = array(
+                       $op, // operation
+                       false, // without source
+                       false // fails
+               );
+
+               return $cases;
+       }
+
        /**
         * @dataProvider provider_testCreate
         */
@@ -1362,6 +1429,8 @@ class FileBackendTest extends MediaWikiTestCase {
                $this->prepare( array( 'dir' => dirname( $fileD ) ) );
 
                $status = $this->backend->doOperations( array(
+                       array( 'op' => 'describe', 'src' => $fileA,
+                               'headers' => array( 'X-Content-Length' => '91.3' ), 'disposition' => 'inline' ),
                        array( 'op' => 'copy', 'src' => $fileA, 'dst' => $fileC, 'overwrite' => 1 ),
                        // Now: A:<A>, B:<B>, C:<A>, D:<empty> (file:<orginal contents>)
                        array( 'op' => 'copy', 'src' => $fileC, 'dst' => $fileA, 'overwriteSame' => 1 ),
@@ -1392,7 +1461,7 @@ class FileBackendTest extends MediaWikiTestCase {
 
                $this->assertGoodStatus( $status, "Operation batch succeeded" );
                $this->assertEquals( true, $status->isOK(), "Operation batch succeeded" );
-               $this->assertEquals( 13, count( $status->success ),
+               $this->assertEquals( 14, count( $status->success ),
                        "Operation batch has correct success array" );
 
                $this->assertEquals( false, $this->backend->fileExists( array( 'src' => $fileA ) ),