jenkins-bot has submitted this change and it was merged.
Change subject: Convert doDeleteArticleReal to startAtomic()/endAtomic()
......................................................................
Convert doDeleteArticleReal to startAtomic()/endAtomic()
* They no longer commit the update, but just make sure
it is part of a transaction. The BEGIN/COMMIT will
happen at request start/end given DBO_TRX or in this
method otherwise (like when in CLI mode). This avoids
premature committing of other transactions.
* FileDeleteForm was the only caller using $commit=false
for WikiPage::doDeleteArticleReal() and its proxies
WikiPage::doDeleteArticle() and Article::doDeleteArticle().
The ugly $commit flag is now removed.
* No caller was using $id for WikiPage::doDeleteArticleReal()
and its proxies WikiPage::doDeleteArticle() and
Article::doDeleteArticle(). It is now removed and we can
be sure the lock() and CAS logic always hit in the method.
The rollback() calls are not needed given the page row lock
and having them there could break outer transactions.
* Updated FileDeleteForm to use startAtomic()/endAtomic() so
the article and file delete are still wrapped in a
transaction. Note that LocalFile::delete() does reference
counting and trxLevel() checks so it will not try to begin()
and break FileDeleteForm's transaction via startAtomic().
* Moved less important 'page-recent-delete' key update down
for sanity in case it blows up.
Change-Id: Idb98510506c0edd02236c30badaec97d86e07696
---
M RELEASE-NOTES-1.27
M includes/FileDeleteForm.php
M includes/filerepo/file/File.php
M includes/page/Article.php
M includes/page/WikiPage.php
5 files changed, 54 insertions(+), 49 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27
index f8293b9..b3add69 100644
--- a/RELEASE-NOTES-1.27
+++ b/RELEASE-NOTES-1.27
@@ -90,6 +90,8 @@
=== Other changes in 1.27 ===
* ProfilerOutputUdp was removed. Note that there is a ProfilerOutputStats
class.
+* WikiPage::doDeleteArticleReal() and WikiPage::doDeleteArticle() now
+ ignore the 2nd and 3rd arguments (formerly $id and $commit).
== Compatibility ==
diff --git a/includes/FileDeleteForm.php b/includes/FileDeleteForm.php
index 5e7f5b2..8b41ad4 100644
--- a/includes/FileDeleteForm.php
+++ b/includes/FileDeleteForm.php
@@ -190,6 +190,7 @@
$page = WikiPage::factory( $title );
$dbw = wfGetDB( DB_MASTER );
try {
+ $dbw->startAtomic( __METHOD__ );
// delete the associated article first
$error = '';
$deleteStatus = $page->doDeleteArticleReal(
$reason, $suppress, 0, false, $error, $user );
@@ -198,11 +199,15 @@
if ( $deleteStatus->isOK() ) {
$status = $file->delete( $reason,
$suppress, $user );
if ( $status->isOK() ) {
- $dbw->commit( __METHOD__ );
$status->value =
$deleteStatus->value; // log id
+ $dbw->endAtomic( __METHOD__ );
} else {
+ // Page deleted but file still
there? rollback page delete
$dbw->rollback( __METHOD__ );
}
+ } else {
+ // Done; nothing changed
+ $dbw->endAtomic( __METHOD__ );
}
} catch ( Exception $e ) {
// Rollback before returning to prevent UI from
displaying
diff --git a/includes/filerepo/file/File.php b/includes/filerepo/file/File.php
index 588ae6b..5eda550 100644
--- a/includes/filerepo/file/File.php
+++ b/includes/filerepo/file/File.php
@@ -1907,7 +1907,7 @@
* @param string $reason
* @param bool $suppress Hide content from sysops?
* @param User|null $user
- * @return bool Boolean on success, false on some kind of failure
+ * @return FileRepoStatus
* STUB
* Overridden by LocalFile
*/
diff --git a/includes/page/Article.php b/includes/page/Article.php
index 43b1a47..5d6435e 100644
--- a/includes/page/Article.php
+++ b/includes/page/Article.php
@@ -2088,15 +2088,15 @@
/**
* @param string $reason
* @param bool $suppress
- * @param int $id
- * @param bool $commit
+ * @param int $u1 Unused
+ * @param bool $u2 Unused
* @param string $error
* @return bool
*/
- public function doDeleteArticle( $reason, $suppress = false, $id = 0,
- $commit = true, &$error = ''
+ public function doDeleteArticle(
+ $reason, $suppress = false, $u1 = null, $u2 = null, &$error = ''
) {
- return $this->mPage->doDeleteArticle( $reason, $suppress, $id,
$commit, $error );
+ return $this->mPage->doDeleteArticle( $reason, $suppress, $u1,
$u2, $error );
}
/**
diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index cdaab1a..244e469 100644
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -2736,16 +2736,16 @@
* @param string $reason Delete reason for deletion log
* @param bool $suppress Suppress all revisions and log the deletion in
* the suppression log instead of the deletion log
- * @param int $id Article ID
- * @param bool $commit Defaults to true, triggers transaction end
- * @param array &$error Array of errors to append to
+ * @param int $u1 Unused
+ * @param bool $u2 Unused
+ * @param array|string &$error Array of errors to append to
* @param User $user The deleting user
* @return bool True if successful
*/
public function doDeleteArticle(
- $reason, $suppress = false, $id = 0, $commit = true, &$error =
'', User $user = null
+ $reason, $suppress = false, $u1 = null, $u2 = null, &$error =
'', User $user = null
) {
- $status = $this->doDeleteArticleReal( $reason, $suppress, $id,
$commit, $error, $user );
+ $status = $this->doDeleteArticleReal( $reason, $suppress, $u1,
$u2, $error, $user );
return $status->isGood();
}
@@ -2758,16 +2758,16 @@
* @param string $reason Delete reason for deletion log
* @param bool $suppress Suppress all revisions and log the deletion in
* the suppression log instead of the deletion log
- * @param int $id Article ID
- * @param bool $commit Defaults to true, triggers transaction end
- * @param array &$error Array of errors to append to
+ * @param int $u1 Unused
+ * @param bool $u2 Unused
+ * @param array|string &$error Array of errors to append to
* @param User $user The deleting user
* @return Status Status object; if successful, $status->value is the
log_id of the
* deletion log entry. If the page couldn't be deleted because it
wasn't
* found, $status is a non-fatal 'cannotdelete' error
*/
public function doDeleteArticleReal(
- $reason, $suppress = false, $id = 0, $commit = true, &$error =
'', User $user = null
+ $reason, $suppress = false, $u1 = null, $u2 = null, &$error =
'', User $user = null
) {
global $wgUser, $wgContentHandlerUseDB;
@@ -2793,24 +2793,27 @@
}
$dbw = wfGetDB( DB_MASTER );
- $dbw->begin( __METHOD__ );
+ $dbw->startAtomic( __METHOD__ );
- if ( $id == 0 ) {
- $this->loadPageData( self::READ_LATEST );
- $id = $this->getID();
- // T98706: lock the page from various other updates but
avoid using
- // WikiPage::READ_LOCKING as that will carry over the
FOR UPDATE to
- // the revisions queries (which also JOIN on user).
Only lock the page
- // row and CAS check on page_latest to see if the trx
snapshot matches.
- $lockedLatest = $this->lock();
- if ( $id == 0 || $this->getLatest() != $lockedLatest ) {
- // Page not there or trx snapshot is stale
- $dbw->rollback( __METHOD__ );
- $status->error( 'cannotdelete',
- wfEscapeWikiText(
$this->getTitle()->getPrefixedText() ) );
- return $status;
- }
+ $this->loadPageData( self::READ_LATEST );
+ $id = $this->getID();
+ // T98706: lock the page from various other updates but avoid
using
+ // WikiPage::READ_LOCKING as that will carry over the FOR
UPDATE to
+ // the revisions queries (which also JOIN on user). Only lock
the page
+ // row and CAS check on page_latest to see if the trx snapshot
matches.
+ $lockedLatest = $this->lock();
+ if ( $id == 0 || $this->getLatest() != $lockedLatest ) {
+ $dbw->endAtomic( __METHOD__ );
+ // Page not there or trx snapshot is stale
+ $status->error( 'cannotdelete',
+ wfEscapeWikiText(
$this->getTitle()->getPrefixedText() ) );
+ return $status;
}
+
+ // At this point we are now comitted to returning an OK
+ // status unless some DB query error or other exception comes
up.
+ // This way callers don't have to call rollback() if $status is
bad
+ // unless they actually try to catch exceptions (which is rare).
// we need to remember the old content so we can use it to
generate all deletion updates.
$content = $this->getContent( Revision::RAW );
@@ -2864,24 +2867,20 @@
$row['ar_content_format'] = 'rev_content_format';
}
- $dbw->insertSelect( 'archive', array( 'page', 'revision' ),
+ // Copy all the page revisions into the archive table
+ $dbw->insertSelect(
+ 'archive',
+ array( 'page', 'revision' ),
$row,
array(
'page_id' => $id,
'page_id = rev_page'
- ), __METHOD__
+ ),
+ __METHOD__
);
// Now that it's safely backed up, delete it
$dbw->delete( 'page', array( 'page_id' => $id ), __METHOD__ );
- $ok = ( $dbw->affectedRows() > 0 ); // $id could be laggy
-
- if ( !$ok ) {
- $dbw->rollback( __METHOD__ );
- $status->error( 'cannotdelete',
- wfEscapeWikiText(
$this->getTitle()->getPrefixedText() ) );
- return $status;
- }
if ( !$dbw->cascadingDeletes() ) {
$dbw->delete( 'revision', array( 'rev_page' => $id ),
__METHOD__ );
@@ -2904,19 +2903,18 @@
$logEntry->publish( $logid );
} );
- if ( $commit ) {
- $dbw->commit( __METHOD__ );
- }
-
- // Show log excerpt on 404 pages rather than just a link
- $key = wfMemcKey( 'page-recent-delete', md5(
$logTitle->getPrefixedText() ) );
- ObjectCache::getMainStashInstance()->set( $key, 1, 86400 );
+ $dbw->endAtomic( __METHOD__ );
$this->doDeleteUpdates( $id, $content );
Hooks::run( 'ArticleDeleteComplete',
array( &$this, &$user, $reason, $id, $content,
$logEntry ) );
$status->value = $logid;
+
+ // Show log excerpt on 404 pages rather than just a link
+ $key = wfMemcKey( 'page-recent-delete', md5(
$logTitle->getPrefixedText() ) );
+ ObjectCache::getMainStashInstance()->set( $key, 1, 86400 );
+
return $status;
}
--
To view, visit https://gerrit.wikimedia.org/r/244250
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idb98510506c0edd02236c30badaec97d86e07696
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
