From: Aaron Schulz Date: Sun, 13 Dec 2015 12:01:03 +0000 (-0800) Subject: Convert page creation to using startAtomic()/endAtomic() X-Git-Tag: 1.31.0-rc.0~8184 X-Git-Url: http://git.cyclocoop.org/%22%2C%20generer_url_ecrire%28?a=commitdiff_plain;h=36a87a890291b651d332c510ed9ed791472e4f44;p=lhc%2Fweb%2Fwiklou.git Convert page creation to using startAtomic()/endAtomic() A few semantic changes result from this: * If multiple pages are created 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 an extension set some extra Status info or errors via the PageContentInsertComplete hook, they will not be seen by the caller (unless it was a CLI script possibly). Few callers use $status at all, and I did not see any that mutated it. Since the page is already committed when this hooks run (as has always been) they cannot veto edits and callers do not care or know what to do with random hook-set status errors; there was never much use in changing the Status anyway. Bug: T120718 Change-Id: Ieba35056be31b2f648c57f59d19d3cbbe58f1b05 --- diff --git a/includes/db/IDatabase.php b/includes/db/IDatabase.php index c72218aad1..cecb6438c7 100644 --- a/includes/db/IDatabase.php +++ b/includes/db/IDatabase.php @@ -1219,6 +1219,8 @@ interface IDatabase { * after the database is updated so that the jobs will see the data when they actually run. * It can also be used for updates that easily cause deadlocks if locks are held too long. * + * Updates will execute in the order they were enqueued. + * * @param callable $callback * @since 1.20 */ @@ -1232,6 +1234,8 @@ interface IDatabase { * This is useful for updates that easily cause deadlocks if locks are held too long * but where atomicity is strongly desired for these updates and some related updates. * + * Updates will execute in the order they were enqueued. + * * @param callable $callback * @since 1.22 */ diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 6f4c296709..c02f97506e 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -1823,7 +1823,7 @@ class WikiPage implements Page, IDBAccessObject { $revisionId = $revision->insertOn( $dbw ); // Update page_latest and friends to reflect the new revision if ( !$this->updateRevisionOn( $dbw, $revision, null, $meta['oldIsRedirect'] ) ) { - $dbw->rollback( __METHOD__ ); + $dbw->rollback( __METHOD__ ); // sanity; this should never happen throw new MWException( "Failed to update page row to use new revision." ); } @@ -1921,12 +1921,12 @@ class WikiPage implements Page, IDBAccessObject { } $dbw = wfGetDB( DB_MASTER ); - $dbw->begin( __METHOD__ ); + $dbw->startAtomic( __METHOD__ ); // Add the page record unless one already exists for the title $newid = $this->insertOn( $dbw ); if ( $newid === false ) { - $dbw->commit( __METHOD__ ); // nothing inserted + $dbw->endAtomic( __METHOD__ ); // nothing inserted $status->fatal( 'edit-already-exists' ); return $status; // nothing done @@ -1956,7 +1956,7 @@ class WikiPage implements Page, IDBAccessObject { $revisionId = $revision->insertOn( $dbw ); // Update the page record with revision data if ( !$this->updateRevisionOn( $dbw, $revision, 0 ) ) { - $dbw->rollback( __METHOD__ ); + $dbw->rollback( __METHOD__ ); // sanity; this should never happen throw new MWException( "Failed to update page row to use new revision." ); } @@ -1984,25 +1984,34 @@ class WikiPage implements Page, IDBAccessObject { $user->incEditCount(); - $dbw->commit( __METHOD__ ); + $dbw->endAtomic( __METHOD__ ); $this->mTimestamp = $now; - // Update links, etc. - $this->doEditUpdates( $revision, $user, array( 'created' => true ) ); - - $hook_args = array( &$this, &$user, $content, $summary, - $flags & EDIT_MINOR, null, null, &$flags, $revision ); - ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $hook_args ); - Hooks::run( 'PageContentInsertComplete', $hook_args ); - // Return the new revision to the caller $status->value['revision'] = $revision; - // 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 ( + &$that, $dbw, $revision, &$user, $content, $summary, &$flags, $meta, &$status + ) { + // Do per-page updates in a transaction + $dbw->setFlag( DBO_TRX ); + // Update links, etc. + $that->doEditUpdates( $revision, $user, array( 'created' => true ) ); + // Trigger post-create hook + $params = array( &$that, &$user, $content, $summary, + $flags & EDIT_MINOR, null, null, &$flags, $revision ); + ContentHandler::runLegacyHooks( 'ArticleInsertComplete', $params ); + Hooks::run( 'PageContentInsertComplete', $params ); + // Trigger post-save hook + $params = array_merge( $params, array( &$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 804b6ba405..f113b1f72a 100644 --- a/tests/phpunit/includes/EditPageTest.php +++ b/tests/phpunit/includes/EditPageTest.php @@ -272,6 +272,25 @@ class EditPageTest extends MediaWikiLangTestCase { public function testCreatePage( $desc, $pageTitle, $user, $editText, $expectedCode, $expectedText, $ignoreBlank = false ) { + $checkId = null; + + $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 ( &$checkId ) { + $checkId = $status->value['revision']->getId(); + // types/refs checked + } ), + ) ); + $edit = array( 'wpTextbox1' => $editText ); if ( $ignoreBlank ) { $edit['wpIgnoreBlankArticle'] = 1; @@ -280,7 +299,67 @@ class EditPageTest extends MediaWikiLangTestCase { $page = $this->assertEdit( $pageTitle, null, $user, $edit, $expectedCode, $expectedText, $desc ); if ( $expectedCode != EditPage::AS_BLANK_ARTICLE ) { + $latest = $page->getLatest(); + $page->doDeleteArticleReal( $pageTitle ); + + $this->assertGreaterThan( 0, $latest, "Page revision ID updated in object" ); + $this->assertEquals( $latest, $checkId, "Revision in Status for hook" ); + } + } + + /** + * @dataProvider provideCreatePages + * @covers EditPage + */ + public function testCreatePageTrx( + $desc, $pageTitle, $user, $editText, $expectedCode, $expectedText, $ignoreBlank = false + ) { + $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 + } ), + ) ); + + wfGetDB( DB_MASTER )->begin( __METHOD__ ); + + $edit = array( 'wpTextbox1' => $editText ); + if ( $ignoreBlank ) { + $edit['wpIgnoreBlankArticle'] = 1; + } + + $page = $this->assertEdit( + $pageTitle, null, $user, $edit, $expectedCode, $expectedText, $desc ); + + $pageTitle2 = (string)$pageTitle . '/x'; + $page2 = $this->assertEdit( + $pageTitle2, null, $user, $edit, $expectedCode, $expectedText, $desc ); + + wfGetDB( DB_MASTER )->commit( __METHOD__ ); + + if ( $expectedCode != EditPage::AS_BLANK_ARTICLE ) { + $latest = $page->getLatest(); $page->doDeleteArticleReal( $pageTitle ); + + $this->assertGreaterThan( 0, $latest, "Page #1 revision ID updated in object" ); + $this->assertEquals( $latest, $checkIds[0], "Revision #1 in Status for hook" ); + + $latest2 = $page2->getLatest(); + $page2->doDeleteArticleReal( $pageTitle2 ); + + $this->assertGreaterThan( 0, $latest2, "Page #2 revision ID updated in object" ); + $this->assertEquals( $latest2, $checkIds[1], "Revision #2 in Status for hook" ); } }