}
}
- /**
- * Roll back the DB transaction and mark the image unlocked
- */
- function unlockAndRollback() {
- $this->locked = false;
- $dbw = $this->repo->getMasterDB();
- $dbw->rollback( __METHOD__ );
- $this->lockedOwnTrx = false;
- }
-
/**
* @return Status
*/
public function execute() {
$repo = $this->file->repo;
$status = $repo->newGood();
+ $destFile = wfLocalFile( $this->target );
+
+ $this->file->lock(); // begin
+ $destFile->lock(); // quickly fail if destination is not available
$triplets = $this->getMoveTriplets();
$checkStatus = $this->removeNonexistentFiles( $triplets );
if ( !$checkStatus->isGood() ) {
- $status->merge( $checkStatus );
+ $destFile->unlock();
+ $this->file->unlock();
+ $status->merge( $checkStatus ); // couldn't talk to file backend
return $status;
}
$triplets = $checkStatus->value;
- $destFile = wfLocalFile( $this->target );
- $this->file->lock(); // begin
- $destFile->lock(); // quickly fail if destination is not available
- // Rename the file versions metadata in the DB.
- // This implicitly locks the destination file, which avoids race conditions.
- // If we moved the files from A -> C before DB updates, another process could
- // move files from B -> C at this point, causing storeBatch() to fail and thus
- // cleanupTarget() to trigger. It would delete the C files and cause data loss.
- $statusDb = $this->doDBUpdates();
+ // Verify the file versions metadata in the DB.
+ $statusDb = $this->verifyDBUpdates();
if ( !$statusDb->isGood() ) {
$destFile->unlock();
- $this->file->unlockAndRollback();
+ $this->file->unlock();
$statusDb->ok = false;
return $statusDb;
}
- wfDebugLog( 'imagemove', "Renamed {$this->file->getName()} in database: " .
- "{$statusDb->successCount} successes, {$statusDb->failCount} failures" );
if ( !$repo->hasSha1Storage() ) {
// Copy the files into their new location.
// Delete any files copied over (while the destination is still locked)
$this->cleanupTarget( $triplets );
$destFile->unlock();
- $this->file->unlockAndRollback(); // unlocks the destination
+ $this->file->unlock();
wfDebugLog( 'imagemove', "Error in moving files: "
. $statusMove->getWikiText( false, false, 'en' ) );
$statusMove->ok = false;
$status->merge( $statusMove );
}
+ // Rename the file versions metadata in the DB.
+ $this->doDBUpdates();
+
+ wfDebugLog( 'imagemove', "Renamed {$this->file->getName()} in database: " .
+ "{$statusDb->successCount} successes, {$statusDb->failCount} failures" );
+
$destFile->unlock();
$this->file->unlock(); // done
}
/**
- * Do the database updates and return a new FileRepoStatus indicating how
- * many rows where updated.
+ * Verify the database updates and return a new FileRepoStatus indicating how
+ * many rows would be updated.
*
* @return FileRepoStatus
*/
- protected function doDBUpdates() {
+ protected function verifyDBUpdates() {
$repo = $this->file->repo;
$status = $repo->newGood();
$dbw = $this->db;
- // Update current image
- $dbw->update(
+ $hasCurrent = $dbw->selectField(
'image',
- [ 'img_name' => $this->newName ],
+ '1',
[ 'img_name' => $this->oldName ],
- __METHOD__
+ __METHOD__,
+ [ 'FOR UPDATE' ]
+ );
+ $oldRowCount = $dbw->selectField(
+ 'oldimage',
+ 'COUNT(*)',
+ [ 'oi_name' => $this->oldName ],
+ __METHOD__,
+ [ 'FOR UPDATE' ]
);
- if ( $dbw->affectedRows() ) {
+ if ( $hasCurrent ) {
$status->successCount++;
} else {
$status->failCount++;
- $status->fatal( 'imageinvalidfilename' );
-
- return $status;
}
+ $status->successCount += $oldRowCount;
+ // Bug 34934: oldCount is based on files that actually exist.
+ // There may be more DB rows than such files, in which case $affected
+ // can be greater than $total. We use max() to avoid negatives here.
+ $status->failCount += max( 0, $this->oldCount - $oldRowCount );
+ if ( $status->failCount ) {
+ $status->error( 'imageinvalidfilename' );
+ }
+
+ return $status;
+ }
+
+ /**
+ * Do the database updates and return a new FileRepoStatus indicating how
+ * many rows where updated.
+ */
+ protected function doDBUpdates() {
+ $dbw = $this->db;
+ // Update current image
+ $dbw->update(
+ 'image',
+ [ 'img_name' => $this->newName ],
+ [ 'img_name' => $this->oldName ],
+ __METHOD__
+ );
// Update old images
$dbw->update(
'oldimage',
[ 'oi_name' => $this->oldName ],
__METHOD__
);
-
- $affected = $dbw->affectedRows();
- $total = $this->oldCount;
- $status->successCount += $affected;
- // Bug 34934: $total is based on files that actually exist.
- // There may be more DB rows than such files, in which case $affected
- // can be greater than $total. We use max() to avoid negatives here.
- $status->failCount += max( 0, $total - $affected );
- if ( $status->failCount ) {
- $status->error( 'imageinvalidfilename' );
- }
-
- return $status;
}
/**