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 b7eef8f..598d956 100644 (file)
--- 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 90ee1bb..ce9e1d6 100644 (file)
--- 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() {