From ae7912737ca60aa95df081beb18d795820227f74 Mon Sep 17 00:00:00 2001
From: Max Semenik <maxsem@users.mediawiki.org>
Date: Fri, 26 Feb 2010 20:14:28 +0000
Subject: [PATCH] (bug 17560) Half-broken deletion moved image files to
 deletion archive without updating DB

---
 RELEASE-NOTES               |  1 +
 includes/Article.php        |  6 ++++--
 includes/FileDeleteForm.php | 30 +++++++++++++++++++++---------
 3 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/RELEASE-NOTES b/RELEASE-NOTES
index 70fa22dd45..d9bb77e2e3 100644
--- a/RELEASE-NOTES
+++ b/RELEASE-NOTES
@@ -21,6 +21,7 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN
 === Configuration changes in 1.17 ===
 
 === Bug fixes in 1.17 ===
+* (bug 17560) Half-broken deletion moved image files to deletion archive without updating DB
 
 == API changes in 1.17 ==
 
diff --git a/includes/Article.php b/includes/Article.php
index 17922b05bd..78d7485ddf 100644
--- a/includes/Article.php
+++ b/includes/Article.php
@@ -2859,7 +2859,7 @@ class Article {
 	 * Deletes the article with database consistency, writes logs, purges caches
 	 * Returns success
 	 */
-	public function doDeleteArticle( $reason, $suppress = false, $id = 0 ) {
+	public function doDeleteArticle( $reason, $suppress = false, $id = 0, $commit = true ) {
 		global $wgUseSquid, $wgDeferredUpdateList;
 		global $wgUseTrackbacks;
 
@@ -2984,7 +2984,9 @@ class Article {
 		# Make sure logging got through
 		$log->addEntry( 'delete', $this->mTitle, $reason, array() );
 
-		$dbw->commit();
+		if ( $commit ) {
+			$dbw->commit();
+		}
 
 		return true;
 	}
diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php
index dad19524c3..6d24cf794e 100644
--- a/includes/FileDeleteForm.php
+++ b/includes/FileDeleteForm.php
@@ -102,6 +102,8 @@ class FileDeleteForm {
 	public static function doDelete( &$title, &$file, &$oldimage, $reason, $suppress ) {
 		global $wgUser;
 		$article = null;
+		$status = Status::newFatal( 'error' );
+
 		if( $oldimage ) {
 			$status = $file->deleteOld( $oldimage, $reason, $suppress );
 			if( $status->ok ) {
@@ -113,23 +115,33 @@ class FileDeleteForm {
 					$log->addEntry( 'delete', $title, $logComment );
 			}
 		} else {
-			$status = $file->delete( $reason, $suppress );
-			if( $status->ok ) {
-				$id = $title->getArticleID( GAID_FOR_UPDATE );
-				// Need to delete the associated article
-				$article = new Article( $title );
-				$error = '';
-				if( wfRunHooks('ArticleDelete', array(&$article, &$wgUser, &$reason, &$error)) ) {
-					if( $article->doDeleteArticle( $reason, $suppress, $id ) ) {
+			$id = $title->getArticleID( GAID_FOR_UPDATE );
+			$article = new Article( $title );
+			$error = '';
+			$dbw = wfGetDB( DB_MASTER );
+			try {
+				if( wfRunHooks( 'ArticleDelete', array( &$article, &$wgUser, &$reason, &$error ) ) ) {
+					// delete the associated article first
+					if( $article->doDeleteArticle( $reason, $suppress, $id, false ) ) {
 						global $wgRequest;
 						if( $wgRequest->getCheck( 'wpWatch' ) && $wgUser->isLoggedIn() ) {
 							$article->doWatch();
 						} elseif( $title->userIsWatching() ) {
 							$article->doUnwatch();
 						}
-						wfRunHooks('ArticleDeleteComplete', array(&$article, &$wgUser, $reason, $id));
+						$status = $file->delete( $reason, $suppress );
+						if( $status->ok ) {
+							$dbw->commit();
+							wfRunHooks( 'ArticleDeleteComplete', array( &$article, &$wgUser, $reason, $id ) );
+						} else {
+							$dbw->rollback();
+						}
 					}
 				}
+			} catch ( MWException $e ) {
+				// rollback before returning to prevent UI from displaying incorrect "View or restore N deleted edits?"
+				//$dbw->rollback();
+				throw $e;
 			}
 		}
 		if( $status->isGood() ) 
-- 
2.20.1