From: Aaron Schulz Date: Sun, 13 Dec 2015 12:18:56 +0000 (-0800) Subject: Convert page modification to using startAtomic()/endAtomic() X-Git-Tag: 1.31.0-rc.0~8052^2 X-Git-Url: http://git.cyclocoop.org/%22.%24image2.%22?a=commitdiff_plain;h=f4a44574035d22b771aa61161a122381fd3d9cc1;p=lhc%2Fweb%2Fwiklou.git Convert page modification to using startAtomic()/endAtomic() A few semantic changes result from this: * If multiple pages are edited in a request, the updates happen in the same order relative to each other, but all in one second step instead of after each page edit. * If the same page is edited twice in a request, the WikiPage hook argument will reflect the last request edit, not always the edit that fired the hook. Bug: T120718 Change-Id: I9429f29e5a90f24e4d7af5797a80e63a9cc34146 --- diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index b7eef8faa3..598d9560e0 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -1793,6 +1793,8 @@ class WikiPage implements Page, IDBAccessObject { $changed = !$content->equals( $oldContent ); + $dbw = wfGetDB( DB_MASTER ); + if ( $changed ) { $prepStatus = $content->prepareSave( $this, $flags, $oldid, $user ); $status->merge( $prepStatus ); @@ -1800,14 +1802,13 @@ class WikiPage implements Page, IDBAccessObject { return $status; } - $dbw = wfGetDB( DB_MASTER ); - $dbw->begin( __METHOD__ ); + $dbw->startAtomic( __METHOD__ ); // Get the latest page_latest value while locking it. // Do a CAS style check to see if it's the same as when this method // started. If it changed then bail out before touching the DB. $latestNow = $this->lockAndGetLatest(); if ( $latestNow != $oldid ) { - $dbw->commit( __METHOD__ ); + $dbw->endAtomic( __METHOD__ ); // Page updated or deleted in the mean time $status->fatal( 'edit-conflict' ); @@ -1855,7 +1856,7 @@ class WikiPage implements Page, IDBAccessObject { $user->incEditCount(); - $dbw->commit( __METHOD__ ); + $dbw->endAtomic( __METHOD__ ); $this->mTimestamp = $now; } else { // Bug 32948: revision ID must be set to page {{REVISIONID}} and @@ -1863,17 +1864,6 @@ class WikiPage implements Page, IDBAccessObject { $revision->setId( $this->getLatest() ); } - // Update links tables, site stats, etc. - $this->doEditUpdates( - $revision, - $user, - array( - 'changed' => $changed, - 'oldcountable' => $meta['oldCountable'], - 'oldrevision' => $meta['oldRevision'] - ) - ); - if ( $changed ) { // Return the new revision to the caller $status->value['revision'] = $revision; @@ -1884,11 +1874,32 @@ class WikiPage implements Page, IDBAccessObject { $this->mTitle->invalidateCache( $now ); } - // Trigger post-save hook - $hook_args = array( &$this, &$user, $content, $summary, - $flags & EDIT_MINOR, null, null, &$flags, $revision, &$status, $meta['baseRevId'] ); - ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $hook_args ); - Hooks::run( 'PageContentSaveComplete', $hook_args ); + // Do secondary updates once the main changes have been committed... + $that = $this; + $dbw->onTransactionIdle( + function () use ( + $dbw, &$that, $revision, &$user, $content, $summary, &$flags, + $changed, $meta, &$status + ) { + // Do per-page updates in a transaction + $dbw->setFlag( DBO_TRX ); + // Update links tables, site stats, etc. + $that->doEditUpdates( + $revision, + $user, + array( + 'changed' => $changed, + 'oldcountable' => $meta['oldCountable'], + 'oldrevision' => $meta['oldRevision'] + ) + ); + // Trigger post-save hook + $params = array( &$that, &$user, $content, $summary, $flags & EDIT_MINOR, + null, null, &$flags, $revision, &$status, $meta['baseRevId'] ); + ContentHandler::runLegacyHooks( 'ArticleSaveComplete', $params ); + Hooks::run( 'PageContentSaveComplete', $params ); + } + ); return $status; } diff --git a/tests/phpunit/includes/EditPageTest.php b/tests/phpunit/includes/EditPageTest.php index 90ee1bb8f8..ce9e1d61b9 100644 --- a/tests/phpunit/includes/EditPageTest.php +++ b/tests/phpunit/includes/EditPageTest.php @@ -364,6 +364,25 @@ class EditPageTest extends MediaWikiLangTestCase { } public function testUpdatePage() { + $checkIds = array(); + + $this->setMwGlobals( 'wgHooks', array( + 'PageContentInsertComplete' => array( function ( + WikiPage &$page, User &$user, Content $content, + $summary, $minor, $u1, $u2, &$flags, Revision $revision + ) { + // types/refs checked + } ), + 'PageContentSaveComplete' => array( function ( + WikiPage &$page, User &$user, Content $content, + $summary, $minor, $u1, $u2, &$flags, Revision $revision, + Status &$status, $baseRevId + ) use ( &$checkIds ) { + $checkIds[] = $status->value['revision']->getId(); + // types/refs checked + } ), + ) ); + $text = "one"; $edit = array( 'wpTextbox1' => $text, @@ -373,6 +392,7 @@ class EditPageTest extends MediaWikiLangTestCase { $page = $this->assertEdit( 'EditPageTest_testUpdatePage', "zero", null, $edit, EditPage::AS_SUCCESS_UPDATE, $text, "expected successfull update with given text" ); + $this->assertGreaterThan( 0, $checkIds[0], "First event rev ID set" ); $this->forceRevisionDate( $page, '20120101000000' ); @@ -385,6 +405,62 @@ class EditPageTest extends MediaWikiLangTestCase { $this->assertEdit( 'EditPageTest_testUpdatePage', null, null, $edit, EditPage::AS_SUCCESS_UPDATE, $text, "expected successfull update with given text" ); + $this->assertGreaterThan( 0, $checkIds[1], "Second edit hook rev ID set" ); + $this->assertGreaterThan( $checkIds[0], $checkIds[1], "Second event rev ID is higher" ); + } + + public function testUpdatePageTrx() { + $text = "one"; + $edit = array( + 'wpTextbox1' => $text, + 'wpSummary' => 'first update', + ); + + $page = $this->assertEdit( 'EditPageTest_testTrxUpdatePage', "zero", null, $edit, + EditPage::AS_SUCCESS_UPDATE, $text, + "expected successfull update with given text" ); + + $this->forceRevisionDate( $page, '20120101000000' ); + + $checkIds = array(); + $this->setMwGlobals( 'wgHooks', array( + 'PageContentSaveComplete' => array( function ( + WikiPage &$page, User &$user, Content $content, + $summary, $minor, $u1, $u2, &$flags, Revision $revision, + Status &$status, $baseRevId + ) use ( &$checkIds ) { + $checkIds[] = $status->value['revision']->getId(); + // types/refs checked + } ), + ) ); + + wfGetDB( DB_MASTER )->begin( __METHOD__ ); + + $text = "two"; + $edit = array( + 'wpTextbox1' => $text, + 'wpSummary' => 'second update', + ); + + $this->assertEdit( 'EditPageTest_testTrxUpdatePage', null, null, $edit, + EditPage::AS_SUCCESS_UPDATE, $text, + "expected successfull update with given text" ); + + $text = "three"; + $edit = array( + 'wpTextbox1' => $text, + 'wpSummary' => 'third update', + ); + + $this->assertEdit( 'EditPageTest_testTrxUpdatePage', null, null, $edit, + EditPage::AS_SUCCESS_UPDATE, $text, + "expected successfull update with given text" ); + + wfGetDB( DB_MASTER )->commit( __METHOD__ ); + + $this->assertGreaterThan( 0, $checkIds[0], "First event rev ID set" ); + $this->assertGreaterThan( 0, $checkIds[1], "Second edit hook rev ID set" ); + $this->assertGreaterThan( $checkIds[0], $checkIds[1], "Second event rev ID is higher" ); } public static function provideSectionEdit() {