* Added FileRepo::SKIP_LOCKING constant and made storeBatch() check it.
authorAaron Schulz <aaron@users.mediawiki.org>
Wed, 21 Dec 2011 20:39:50 +0000 (20:39 +0000)
committerAaron Schulz <aaron@users.mediawiki.org>
Wed, 21 Dec 2011 20:39:50 +0000 (20:39 +0000)
* Made File::maybeDoTransform() use the FileRepo::store() and a new File::getThumbRel() function. Looks cleaner and loosens FileBackend coupling.
* Also made storeTemp() use FileRepo::SKIP_LOCKING for performance.
* Killed some useless initZones() calls in FileRepo. Extensions may not even use these zones. Likewise, it could make tests fail even though they don't those zones. We already do the sanity with some prepare() calls in storeBatch().
* Removed FileRepo::SKIP_VALIDATION, not used by anything now.
* Moved getUrlRel() down a bit.

includes/filerepo/FileRepo.php
includes/filerepo/file/File.php

index 4097db1..1bebf4a 100644 (file)
  */
 class FileRepo {
        const FILES_ONLY = 1;
+
        const DELETE_SOURCE = 1;
        const OVERWRITE = 2;
        const OVERWRITE_SAME = 4;
-       const SKIP_VALIDATION = 8;
+       const SKIP_LOCKING = 8;
 
        /** @var FileBackendBase */
        protected $backend;
@@ -622,6 +623,7 @@ class FileRepo {
         *     self::OVERWRITE         Overwrite an existing destination file instead of failing
         *     self::OVERWRITE_SAME    Overwrite the file if the destination exists and has the
         *                             same contents as the source
+        *     self::SKIP_LOCKING      Skip any file locking when doing the store
         * @return FileRepoStatus
         */
        public function store( $srcPath, $dstZone, $dstRel, $flags = 0 ) {
@@ -641,17 +643,12 @@ class FileRepo {
         *     self::OVERWRITE         Overwrite an existing destination file instead of failing
         *     self::OVERWRITE_SAME    Overwrite the file if the destination exists and has the
         *                             same contents as the source
+        *     self::SKIP_LOCKING      Skip any file locking when doing the store
         * @return FileRepoStatus
         */
        public function storeBatch( $triplets, $flags = 0 ) {
                $backend = $this->backend; // convenience
 
-               // Try creating directories
-               $status = $this->initZones();
-               if ( !$status->isOK() ) {
-                       return $status;
-               }
-
                $status = $this->newGood();
 
                $operations = array();
@@ -705,6 +702,9 @@ class FileRepo {
 
                // Execute the store operation for each triplet
                $opts = array( 'ignoreErrors' => true );
+               if ( $flags & self::SKIP_LOCKING ) {
+                       $opts['nonLocking'] = true;
+               }
                $status->merge( $backend->doOperations( $operations, $opts ) );
                // Cleanup for disk source files...
                foreach ( $sourceFSFilesToDelete as $file ) {
@@ -778,7 +778,7 @@ class FileRepo {
                $dstRel = "{$hashPath}{$date}!{$originalName}";
                $dstUrlRel = $hashPath . $date . '!' . rawurlencode( $originalName );
 
-               $result = $this->store( $srcPath, 'temp', $dstRel );
+               $result = $this->store( $srcPath, 'temp', $dstRel, self::SKIP_LOCKING );
                $result->value = $this->getVirtualUrl( 'temp' ) . '/' . $dstUrlRel;
                return $result;
        }
@@ -1005,9 +1005,6 @@ class FileRepo {
         * @return Either array of files and existence flags, or false
         */
        public function fileExistsBatch( $files, $flags = 0 ) {
-               if ( !$this->initZones() ) {
-                       return false;
-               }
                $result = array();
                foreach ( $files as $key => $file ) {
                        if ( self::isVirtualUrl( $file ) ) {
index 7aef11b..9fc552e 100644 (file)
@@ -799,12 +799,11 @@ abstract class File {
                                $thumb = $this->handler->getTransform( $this, $tmpThumbPath, $thumbUrl, $params );
                        }
                } elseif ( $thumb->hasFile() && !$thumb->fileIsSource() ) {
-                       // @TODO: use a FileRepo store function
-                       $op = array( 'op' => 'store',
-                               'src' => $tmpThumbPath, 'dst' => $thumbPath, 'overwriteDest' => true );
                        // Copy any thumbnail from the FS into storage at $dstpath
-                       $opts = array( 'ignoreErrors' => true, 'nonLocking' => true ); // performance
-                       if ( !$this->getRepo()->getBackend()->doOperation( $op, $opts )->isOK() ) {
+                       $status = $this->repo->store(
+                               $tmpThumbPath, 'thumb', $this->getThumbRel( $thumbName ),
+                               FileRepo::OVERWRITE | FileRepo::SKIP_LOCKING );
+                       if ( !$status->isOK() ) {
                                return new MediaTransformError( 'thumbnail_error',
                                        $params['width'], 0, wfMsg( 'thumbnail-dest-create' ) );
                        }
@@ -1013,7 +1012,8 @@ abstract class File {
        }
 
        /**
-        * Get the path of the file relative to the public zone root
+        * Get the path of the file relative to the public zone root.
+        * This function is overriden in OldLocalFile to be like getArchiveRel().
         *
         * @return string
         */
@@ -1022,16 +1022,7 @@ abstract class File {
        }
 
        /**
-        * Get urlencoded relative path of the file
-        *
-        * @return string
-        */
-       function getUrlRel() {
-               return $this->getHashPath() . rawurlencode( $this->getName() );
-       }
-
-       /**
-        * Get the relative path for an archived file
+        * Get the path of an archived file relative to the public zone root
         *
         * @param $suffix bool|string if not false, the name of an archived thumbnail file
         *
@@ -1048,7 +1039,33 @@ abstract class File {
        }
 
        /**
-        * Get the relative path for an archived file's thumbs directory
+        * Get the path, relative to the thumbnail zone root, of the
+        * thumbnail directory or a particular file if $suffix is specified
+        *
+        * @param $suffix bool|string if not false, the name of a thumbnail file
+        *
+        * @return string
+        */
+       function getThumbRel( $suffix = false ) {
+               $path = $this->getRel();
+               if ( $suffix !== false ) {
+                       $path .= '/' . $suffix;
+               }
+               return $path;
+       }
+
+       /**
+        * Get urlencoded path of the file relative to the public zone root.
+        * This function is overriden in OldLocalFile to be like getArchiveUrl().
+        *
+        * @return string
+        */
+       function getUrlRel() {
+               return $this->getHashPath() . rawurlencode( $this->getName() );
+       }
+
+       /**
+        * Get the path, relative to the thumbnail zone root, for an archived file's thumbs directory
         * or a specific thumb if the $suffix is given.
         *
         * @param $archiveName string the timestamped name of an archived image
@@ -1079,7 +1096,7 @@ abstract class File {
        }
 
        /**
-        * Get the path of the archived file's thumbs, or a particular thumb if $suffix is specified
+        * Get the path of an archived file's thumbs, or a particular thumb if $suffix is specified
         *
         * @param $archiveName string the timestamped name of an archived image
         * @param $suffix bool|string if not false, the name of a thumbnail file
@@ -1101,11 +1118,7 @@ abstract class File {
         */
        function getThumbPath( $suffix = false ) {
                $this->assertRepoDefined();
-               $path = $this->repo->getZonePath( 'thumb' ) . '/' . $this->getRel();
-               if ( $suffix !== false ) {
-                       $path .= '/' . $suffix;
-               }
-               return $path;
+               return $this->repo->getZonePath( 'thumb' ) . '/' . $this->getThumbRel( $suffix );
        }
 
        /**