From aa3befb2e053a99128e0040dce4664ce619f185e Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Mon, 26 Dec 2011 23:35:40 +0000 Subject: [PATCH] In LocalFile: * Removed bogus rmdir() call. * Added lock() calls to upgradeRow()/getSha1(). * Use FileRepo getFileSha1() in getSha1() rather than via FSFile. * Made purgeThumbList()/migrateThumbFile() use FileRepo::cleanupBatch(). * A few other minor cleanups. * w/s cleanup in recordUpload2(). In FSRepo: * Removed deleted zone config code from constructor; useless since r107028. In FileBackend: * Make sure 'latest' param gets passed through via $params for some functions. * Cleaned up doMoveInternal() to use *_Internal() functions. --- includes/filerepo/FSRepo.php | 5 - includes/filerepo/backend/FileBackend.php | 16 +- .../filerepo/backend/FileBackendGroup.php | 10 +- includes/filerepo/file/LocalFile.php | 161 +++++++++--------- 4 files changed, 95 insertions(+), 97 deletions(-) diff --git a/includes/filerepo/FSRepo.php b/includes/filerepo/FSRepo.php index 92e7295083..e7a1545c69 100644 --- a/includes/filerepo/FSRepo.php +++ b/includes/filerepo/FSRepo.php @@ -44,11 +44,6 @@ class FSRepo extends FileRepo { ) ); // Update repo config to use this backend $info['backend'] = $backend; - // Set "deleted" zone in repo if deletedDir is configured - if ( $deletedDir !== false ) { - $info['zones']['deleted'] = array( - 'container' => 'media-deleted', 'directory' => '' ); - } } parent::__construct( $info ); diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php index 4bb7332321..23035a43ec 100644 --- a/includes/filerepo/backend/FileBackend.php +++ b/includes/filerepo/backend/FileBackend.php @@ -108,7 +108,7 @@ abstract class FileBackendBase { * 'src' => , * 'ignoreMissingSource' => * ) - * f) Concatenate a list of files into a single file within storage + * f) Concatenate a list of files within storage into a single temp file * array( * 'op' => 'concatenate', * 'srcs' => , @@ -314,7 +314,7 @@ abstract class FileBackendBase { * latest : use the latest available data * * @param $params Array - * @return bool + * @return bool|null Returns null on failure */ abstract public function fileExists( array $params ); @@ -584,12 +584,12 @@ abstract class FileBackend extends FileBackendBase { */ protected function doMoveInternal( array $params ) { // Copy source to dest - $status = $this->backend->copyInternal( $params ); + $status = $this->backend->doCopyInternal( $params ); if ( !$status->isOK() ) { return $status; } // Delete source (only fails due to races or medium going down) - $status->merge( $this->backend->deleteInternal( array( 'src' => $params['src'] ) ) ); + $status->merge( $this->backend->doDeleteInternal( array( 'src' => $params['src'] ) ) ); $status->setResult( true, $status->value ); // ignore delete() errors return $status; } @@ -693,7 +693,7 @@ abstract class FileBackend extends FileBackendBase { if ( isset( $this->cache[$path]['sha1'] ) ) { return $this->cache[$path]['sha1']; } - $fsFile = $this->getLocalReference( array( 'src' => $path ) ); + $fsFile = $this->getLocalReference( $params ); if ( !$fsFile ) { return false; } else { @@ -710,7 +710,7 @@ abstract class FileBackend extends FileBackendBase { * @see FileBackendBase::getFileProps() */ public function getFileProps( array $params ) { - $fsFile = $this->getLocalReference( array( 'src' => $params['src'] ) ); + $fsFile = $this->getLocalReference( $params ); if ( !$fsFile ) { return FSFile::placeholderProps(); } else { @@ -728,10 +728,10 @@ abstract class FileBackend extends FileBackendBase { /** * @see FileBackendBase::streamFile() */ - function streamFile( array $params ) { + public function streamFile( array $params ) { $status = Status::newGood(); - $fsFile = $this->getLocalReference( array( 'src' => $params['src'] ) ); + $fsFile = $this->getLocalReference( $params ); if ( !$fsFile ) { $status->fatal( 'backend-fail-stream', $params['src'] ); return $status; diff --git a/includes/filerepo/backend/FileBackendGroup.php b/includes/filerepo/backend/FileBackendGroup.php index fae777ce8d..d3849e7a1b 100644 --- a/includes/filerepo/backend/FileBackendGroup.php +++ b/includes/filerepo/backend/FileBackendGroup.php @@ -13,7 +13,7 @@ class FileBackendGroup { protected static $instance = null; - /** @var Array of (name => ('class' =>, 'config' =>, 'instance' =>)) */ + /** @var Array (name => ('class' => string, 'config' => array, 'instance' => object)) */ protected $backends = array(); protected function __construct() {} @@ -75,10 +75,10 @@ class FileBackendGroup { 'class' => 'FSFileBackend', 'lockManager' => 'fsLockManager', 'containerPaths' => array( - "media-public" => "{$directory}", - "media-thumb" => $thumbDir, - "media-deleted" => $deletedDir, - "media-temp" => "{$directory}/temp" + 'media-public' => "{$directory}", + 'media-thumb' => $thumbDir, + 'media-deleted' => $deletedDir, + 'media-temp' => "{$directory}/temp" ), 'fileMode' => $fileMode, ); diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index d0d5659d6d..251d325058 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -385,6 +385,8 @@ class LocalFile extends File { function upgradeRow() { wfProfileIn( __METHOD__ ); + $this->lock(); // begin + $this->loadFromFile(); # Don't destroy file info of missing files @@ -405,19 +407,23 @@ class LocalFile extends File { $dbw->update( 'image', array( - 'img_width' => $this->width, - 'img_height' => $this->height, - 'img_bits' => $this->bits, + 'img_width' => $this->width, + 'img_height' => $this->height, + 'img_bits' => $this->bits, 'img_media_type' => $this->media_type, 'img_major_mime' => $major, 'img_minor_mime' => $minor, - 'img_metadata' => $this->metadata, - 'img_sha1' => $this->sha1, - ), array( 'img_name' => $this->getName() ), + 'img_metadata' => $this->metadata, + 'img_sha1' => $this->sha1, + ), + array( 'img_name' => $this->getName() ), __METHOD__ ); $this->saveToCache(); + + $this->unlock(); // done + wfProfileOut( __METHOD__ ); } @@ -458,7 +464,7 @@ class LocalFile extends File { function isMissing() { if ( $this->missing === null ) { - list( $fileExists ) = $this->repo->fileExistsBatch( array( $this->getVirtualUrl() ), FileRepo::FILES_ONLY ); + list( $fileExists ) = $this->repo->fileExists( $this->getVirtualUrl(), FileRepo::FILES_ONLY ); $this->missing = !$fileExists; } return $this->missing; @@ -603,9 +609,8 @@ class LocalFile extends File { */ if ( $this->repo->fileExists( $thumbDir, FileRepo::FILES_ONLY ) ) { - // File where directory should be - $op = array( 'op' => 'delete', 'src' => $thumbDir ); - $this->repo->getBackend()->doOperation( $op ); + // Delete file where directory should be + $this->repo->cleanupBatch( array( $thumbDir ) ); } } @@ -690,15 +695,6 @@ class LocalFile extends File { $dir = array_shift( $files ); $this->purgeThumbList( $dir, $files ); - // Directory should be empty, delete it too. This will probably suck on - // something like NFS or if the directory isn't actually empty, so hide - // the warnings :D - wfSuppressWarnings(); - if( !rmdir( $dir ) ) { - wfDebug( __METHOD__ . ": unable to remove archive directory: $dir\n" ); - } - wfRestoreWarnings(); - // Purge any custom thumbnail caches wfRunHooks( 'LocalFilePurgeThumbnails', array( $this, $archiveName ) ); @@ -757,17 +753,19 @@ class LocalFile extends File { ); wfDebug( __METHOD__ . ": $fileListDebug\n" ); - $backend = $this->repo->getBackend(); + $purgeList = array(); foreach ( $files as $file ) { # Check that the base file name is part of the thumb name # This is a basic sanity check to avoid erasing unrelated directories if ( strpos( $file, $this->getName() ) !== false ) { - $op = array( 'op' => 'delete', 'src' => "{$dir}/{$file}" ); - $backend->doOperation( $op ); + $purgeList[] = "{$dir}/{$file}"; } } - # Clear out directory if empty - $backend->clean( array( 'dir' => $dir ) ); + + # Delete the thumbnails + $this->repo->cleanupBatch( $purgeList ); + # Clear out the thumbnail directory if empty + $this->repo->getBackend()->clean( array( 'dir' => $dir ) ); } /** purgeDescription inherited */ @@ -906,7 +904,7 @@ class LocalFile extends File { // non-nicely (dangling multi-byte chars, non-truncated // version in cache). $comment = $wgContLang->truncate( $comment, 255 ); - $this->lock(); + $this->lock(); // begin $status = $this->publish( $srcPath, $flags ); if ( $status->ok ) { @@ -915,7 +913,7 @@ class LocalFile extends File { } } - $this->unlock(); + $this->unlock(); // done return $status; } @@ -986,20 +984,20 @@ class LocalFile extends File { # doesn't deadlock. SELECT FOR UPDATE causes a deadlock for every race condition. $dbw->insert( 'image', array( - 'img_name' => $this->getName(), - 'img_size' => $this->size, - 'img_width' => intval( $this->width ), - 'img_height' => intval( $this->height ), - 'img_bits' => $this->bits, - 'img_media_type' => $this->media_type, - 'img_major_mime' => $this->major_mime, - 'img_minor_mime' => $this->minor_mime, - 'img_timestamp' => $timestamp, + 'img_name' => $this->getName(), + 'img_size' => $this->size, + 'img_width' => intval( $this->width ), + 'img_height' => intval( $this->height ), + 'img_bits' => $this->bits, + 'img_media_type' => $this->media_type, + 'img_major_mime' => $this->major_mime, + 'img_minor_mime' => $this->minor_mime, + 'img_timestamp' => $timestamp, 'img_description' => $comment, - 'img_user' => $user->getId(), - 'img_user_text' => $user->getName(), - 'img_metadata' => $this->metadata, - 'img_sha1' => $this->sha1 + 'img_user' => $user->getId(), + 'img_user_text' => $user->getName(), + 'img_metadata' => $this->metadata, + 'img_sha1' => $this->sha1 ), __METHOD__, 'IGNORE' @@ -1014,43 +1012,45 @@ class LocalFile extends File { # Insert previous contents into oldimage $dbw->insertSelect( 'oldimage', 'image', array( - 'oi_name' => 'img_name', + 'oi_name' => 'img_name', 'oi_archive_name' => $dbw->addQuotes( $oldver ), - 'oi_size' => 'img_size', - 'oi_width' => 'img_width', - 'oi_height' => 'img_height', - 'oi_bits' => 'img_bits', - 'oi_timestamp' => 'img_timestamp', - 'oi_description' => 'img_description', - 'oi_user' => 'img_user', - 'oi_user_text' => 'img_user_text', - 'oi_metadata' => 'img_metadata', - 'oi_media_type' => 'img_media_type', - 'oi_major_mime' => 'img_major_mime', - 'oi_minor_mime' => 'img_minor_mime', - 'oi_sha1' => 'img_sha1' - ), array( 'img_name' => $this->getName() ), __METHOD__ + 'oi_size' => 'img_size', + 'oi_width' => 'img_width', + 'oi_height' => 'img_height', + 'oi_bits' => 'img_bits', + 'oi_timestamp' => 'img_timestamp', + 'oi_description' => 'img_description', + 'oi_user' => 'img_user', + 'oi_user_text' => 'img_user_text', + 'oi_metadata' => 'img_metadata', + 'oi_media_type' => 'img_media_type', + 'oi_major_mime' => 'img_major_mime', + 'oi_minor_mime' => 'img_minor_mime', + 'oi_sha1' => 'img_sha1' + ), + array( 'img_name' => $this->getName() ), + __METHOD__ ); # Update the current image row $dbw->update( 'image', array( /* SET */ - 'img_size' => $this->size, - 'img_width' => intval( $this->width ), - 'img_height' => intval( $this->height ), - 'img_bits' => $this->bits, - 'img_media_type' => $this->media_type, - 'img_major_mime' => $this->major_mime, - 'img_minor_mime' => $this->minor_mime, - 'img_timestamp' => $timestamp, + 'img_size' => $this->size, + 'img_width' => intval( $this->width ), + 'img_height' => intval( $this->height ), + 'img_bits' => $this->bits, + 'img_media_type' => $this->media_type, + 'img_major_mime' => $this->major_mime, + 'img_minor_mime' => $this->minor_mime, + 'img_timestamp' => $timestamp, 'img_description' => $comment, - 'img_user' => $user->getId(), - 'img_user_text' => $user->getName(), - 'img_metadata' => $this->metadata, - 'img_sha1' => $this->sha1 - ), array( /* WHERE */ - 'img_name' => $this->getName() - ), __METHOD__ + 'img_user' => $user->getId(), + 'img_user_text' => $user->getName(), + 'img_metadata' => $this->metadata, + 'img_sha1' => $this->sha1 + ), + array( 'img_name' => $this->getName() ), + __METHOD__ ); } else { # This is a new file @@ -1160,7 +1160,7 @@ class LocalFile extends File { * archive name, or an empty string if it was a new file. */ function publishTo( $srcPath, $dstRel, $flags = 0 ) { - $this->lock(); + $this->lock(); // begin $archiveName = wfTimestamp( TS_MW ) . '!'. $this->getName(); $archiveRel = 'archive/' . $this->getHashPath() . $archiveName; @@ -1173,7 +1173,7 @@ class LocalFile extends File { $status->value = $archiveName; } - $this->unlock(); + $this->unlock(); // done return $status; } @@ -1197,7 +1197,7 @@ class LocalFile extends File { */ function move( $target ) { wfDebugLog( 'imagemove', "Got request to move {$this->name} to " . $target->getText() ); - $this->lock(); + $this->lock(); // begin $batch = new LocalFileMoveBatch( $this, $target ); $batch->addCurrent(); @@ -1207,7 +1207,7 @@ class LocalFile extends File { wfDebugLog( 'imagemove', "Finished moving {$this->name}" ); $this->purgeEverything(); - $this->unlock(); + $this->unlock(); // done if ( $status->isOk() ) { // Now switch the object @@ -1235,7 +1235,7 @@ class LocalFile extends File { * @return FileRepoStatus object. */ function delete( $reason, $suppress = false ) { - $this->lock(); + $this->lock(); // begin $batch = new LocalFileDeleteBatch( $this, $reason, $suppress ); $batch->addCurrent(); @@ -1258,7 +1258,7 @@ class LocalFile extends File { $this->purgeEverything(); } - $this->unlock(); + $this->unlock(); // done return $status; } @@ -1278,14 +1278,14 @@ class LocalFile extends File { * @return FileRepoStatus object. */ function deleteOld( $archiveName, $reason, $suppress = false ) { - $this->lock(); + $this->lock(); // begin $batch = new LocalFileDeleteBatch( $this, $reason, $suppress ); $batch->addOld( $archiveName ); $this->purgeOldThumbnails( $archiveName ); $status = $batch->execute(); - $this->unlock(); + $this->unlock(); // done if ( $status->ok ) { $this->purgeDescription(); @@ -1370,8 +1370,9 @@ class LocalFile extends File { $this->load(); // Initialise now if necessary if ( $this->sha1 == '' && $this->fileExists ) { - $tmpPath = $this->getLocalRefPath(); - $this->sha1 = FSFile::getSha1Base36FromPath( $tmpPath ); + $this->lock(); // begin + + $this->sha1 = $this->repo->getFileSha1( $this->getPath() ); if ( !wfReadOnly() && strval( $this->sha1 ) != '' ) { $dbw = $this->repo->getMasterDB(); $dbw->update( 'image', @@ -1380,6 +1381,8 @@ class LocalFile extends File { __METHOD__ ); $this->saveToCache(); } + + $this->unlock(); // done } return $this->sha1; -- 2.20.1