From 4538a5f4bc56ca5d9637bc05db72bd8266e0a586 Mon Sep 17 00:00:00 2001 From: daniel Date: Mon, 30 Apr 2012 15:32:31 +0200 Subject: [PATCH] using SecondaryDataUpdate to clean up after deletion --- includes/AutoLoader.php | 1 + includes/ContentHandler.php | 11 ++++ includes/LinksUpdate.php | 80 +++++++++++++++++++++++- includes/SecondaryDataUpdate.php | 10 ++- includes/WikiPage.php | 82 +++++-------------------- tests/phpunit/includes/WikiPageTest.php | 51 +++++++++++---- 6 files changed, 151 insertions(+), 84 deletions(-) diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index 0f163f6ef4..5d9f0b32d3 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -144,6 +144,7 @@ $wgAutoloadLocalClasses = array( 'Linker' => 'includes/Linker.php', 'LinkFilter' => 'includes/LinkFilter.php', 'LinksUpdate' => 'includes/LinksUpdate.php', + 'LinksDeletionUpdate' => 'includes/LinksUpdate.php', 'LocalisationCache' => 'includes/LocalisationCache.php', 'LocalisationCache_BulkLoad' => 'includes/LocalisationCache.php', 'MagicWord' => 'includes/MagicWord.php', diff --git a/includes/ContentHandler.php b/includes/ContentHandler.php index 7e11cdd77f..ac54be3686 100644 --- a/includes/ContentHandler.php +++ b/includes/ContentHandler.php @@ -655,6 +655,17 @@ abstract class ContentHandler { public function isParserCacheSupported() { return true; } + + /** + * @param $page WikiPage the page that was deleted (note: $page->getId() must still return the old page ID!) + * + * @return array a list of SecondaryDataUpdate instances that will clean up the database ofter deletion. + */ + public function getDeletionUpdates( WikiPage $page ) { + return array( + new LinksDeletionUpdate( $page ), + ); + } } diff --git a/includes/LinksUpdate.php b/includes/LinksUpdate.php index 40970556e7..4717d83ff5 100644 --- a/includes/LinksUpdate.php +++ b/includes/LinksUpdate.php @@ -52,10 +52,11 @@ class LinksUpdate extends SecondaryDBDataUpdate { "Please see Article::editUpdates() for an invocation example.\n" ); } - if ( !is_object( $title ) ) { + if ( !is_object( $parserOutput ) ) { throw new MWException( "The calling convention to LinksUpdate::__construct() has changed. " . "Please see WikiPage::doEditUpdates() for an invocation example.\n" ); - } + } + $this->mTitle = $title; $this->mId = $title->getArticleID(); @@ -801,3 +802,78 @@ class LinksUpdate extends SecondaryDBDataUpdate { } } } + +/** + * Update object handling the cleanup of links tables after a page was deleted. + **/ +class LinksDeletionUpdate extends SecondaryDBDataUpdate { + + /**@{{ + * @private + */ + var $mWikiPage; //!< WikiPage the wikipage that was deleted + /**@}}*/ + + /** + * Constructor + * + * @param $title Title of the page we're updating + * @param $parserOutput ParserOutput: output from a full parse of this page + * @param $recursive Boolean: queue jobs for recursive updates? + */ + function __construct( WikiPage $page ) { + parent::__construct( ); + + $this->mPage = $page; + } + + /** + * Do some database updates after deletion + */ + public function doUpdate() { + $title = $this->mPage->getTitle(); + $id = $this->mPage->getId(); + + # Delete restrictions for it + $this->mDb->delete( 'page_restrictions', array ( 'pr_page' => $id ), __METHOD__ ); + + # Fix category table counts + $cats = array(); + $res = $this->mDb->select( 'categorylinks', 'cl_to', array( 'cl_from' => $id ), __METHOD__ ); + + foreach ( $res as $row ) { + $cats [] = $row->cl_to; + } + + $this->mPage->updateCategoryCounts( array(), $cats ); + + # If using cascading deletes, we can skip some explicit deletes + if ( !$this->mDb->cascadingDeletes() ) { + $this->mDb->delete( 'revision', array( 'rev_page' => $id ), __METHOD__ ); + + # Delete outgoing links + $this->mDb->delete( 'pagelinks', array( 'pl_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'imagelinks', array( 'il_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'categorylinks', array( 'cl_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'templatelinks', array( 'tl_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'externallinks', array( 'el_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'langlinks', array( 'll_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'iwlinks', array( 'iwl_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'redirect', array( 'rd_from' => $id ), __METHOD__ ); + $this->mDb->delete( 'page_props', array( 'pp_page' => $id ), __METHOD__ ); + } + + # If using cleanup triggers, we can skip some manual deletes + if ( !$this->mDb->cleanupTriggers() ) { + # Clean up recentchanges entries... + $this->mDb->delete( 'recentchanges', + array( 'rc_type != ' . RC_LOG, + 'rc_namespace' => $title->getNamespace(), + 'rc_title' => $title->getDBkey() ), + __METHOD__ ); + $this->mDb->delete( 'recentchanges', + array( 'rc_type != ' . RC_LOG, 'rc_cur_id' => $id ), + __METHOD__ ); + } + } +} \ No newline at end of file diff --git a/includes/SecondaryDataUpdate.php b/includes/SecondaryDataUpdate.php index 43e7497583..e02b2b3104 100644 --- a/includes/SecondaryDataUpdate.php +++ b/includes/SecondaryDataUpdate.php @@ -19,8 +19,11 @@ * * Abstract base class for update jobs that do something with some secondary * data extracted from article. + * + * @todo: add support for transactions + * */ -abstract class SecondaryDataUpdate { +abstract class SecondaryDataUpdate implements DeferrableUpdate { /** * Constructor @@ -29,11 +32,6 @@ abstract class SecondaryDataUpdate { # noop } - /** - * Perform update. - */ - public abstract function doUpdate(); - /** * Conveniance method, calls doUpdate() on every element in the array. * diff --git a/includes/WikiPage.php b/includes/WikiPage.php index c1af18f8c8..92f1d93b44 100644 --- a/includes/WikiPage.php +++ b/includes/WikiPage.php @@ -2278,7 +2278,22 @@ class WikiPage extends Page { return WikiPage::DELETE_NO_REVISIONS; } - $this->doDeleteUpdates( $id ); + # update site status + DeferredUpdates::addUpdate( new SiteStatsUpdate( 0, 1, - (int)$this->isCountable(), -1 ) ); + + # remove secondary indexes, etc + $handler = $this->getContentHandler(); + $updates = $handler->getDeletionUpdates( $this ); + SecondaryDataUpdate::runUpdates( $updates ); + + # Clear caches + WikiPage::onArticleDelete( $this->mTitle ); + + # Reset this object + $this->clear(); + + # Clear the cached article id so the interface doesn't act like we exist + $this->mTitle->resetArticleID( 0 ); # Log the deletion, if the page was suppressed, log it at Oversight instead $logtype = $suppress ? 'suppress' : 'delete'; @@ -2298,70 +2313,6 @@ class WikiPage extends Page { return WikiPage::DELETE_SUCCESS; } - /** - * Do some database updates after deletion - * - * @param $id Int: page_id value of the page being deleted - */ - public function doDeleteUpdates( $id ) { - DeferredUpdates::addUpdate( new SiteStatsUpdate( 0, 1, - (int)$this->isCountable(), -1 ) ); - - $dbw = wfGetDB( DB_MASTER ); - - # Delete restrictions for it - $dbw->delete( 'page_restrictions', array ( 'pr_page' => $id ), __METHOD__ ); - - # Fix category table counts - $cats = array(); - $res = $dbw->select( 'categorylinks', 'cl_to', array( 'cl_from' => $id ), __METHOD__ ); - - foreach ( $res as $row ) { - $cats [] = $row->cl_to; - } - - $this->updateCategoryCounts( array(), $cats ); - - #TODO: move this to an Update object! - - # If using cascading deletes, we can skip some explicit deletes - if ( !$dbw->cascadingDeletes() ) { - $dbw->delete( 'revision', array( 'rev_page' => $id ), __METHOD__ ); - - # Delete outgoing links - $dbw->delete( 'pagelinks', array( 'pl_from' => $id ), __METHOD__ ); - $dbw->delete( 'imagelinks', array( 'il_from' => $id ), __METHOD__ ); - $dbw->delete( 'categorylinks', array( 'cl_from' => $id ), __METHOD__ ); - $dbw->delete( 'templatelinks', array( 'tl_from' => $id ), __METHOD__ ); - $dbw->delete( 'externallinks', array( 'el_from' => $id ), __METHOD__ ); - $dbw->delete( 'langlinks', array( 'll_from' => $id ), __METHOD__ ); - $dbw->delete( 'iwlinks', array( 'iwl_from' => $id ), __METHOD__ ); - $dbw->delete( 'redirect', array( 'rd_from' => $id ), __METHOD__ ); - $dbw->delete( 'page_props', array( 'pp_page' => $id ), __METHOD__ ); - } - - # If using cleanup triggers, we can skip some manual deletes - if ( !$dbw->cleanupTriggers() ) { - # Clean up recentchanges entries... - $dbw->delete( 'recentchanges', - array( 'rc_type != ' . RC_LOG, - 'rc_namespace' => $this->mTitle->getNamespace(), - 'rc_title' => $this->mTitle->getDBkey() ), - __METHOD__ ); - $dbw->delete( 'recentchanges', - array( 'rc_type != ' . RC_LOG, 'rc_cur_id' => $id ), - __METHOD__ ); - } - - # Clear caches - self::onArticleDelete( $this->mTitle ); - - # Reset this object - $this->clear(); - - # Clear the cached article id so the interface doesn't act like we exist - $this->mTitle->resetArticleID( 0 ); - } - /** * Roll back the most recent consecutive set of edits to a page * from the same user; fails if there are no eligible edits to @@ -3227,3 +3178,4 @@ class PoolWorkArticleView extends PoolCounterWork { return false; } } + diff --git a/tests/phpunit/includes/WikiPageTest.php b/tests/phpunit/includes/WikiPageTest.php index 6ecda15f3f..357de0cfc1 100644 --- a/tests/phpunit/includes/WikiPageTest.php +++ b/tests/phpunit/includes/WikiPageTest.php @@ -51,7 +51,7 @@ class WikiPageTest extends MediaWikiTestCase { $page = $this->newPage( $title ); - $content = ContentHandler::makeContent( "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam " + $content = ContentHandler::makeContent( "[[Lorem ipsum]] dolor sit amet, consetetur sadipscing elitr, sed diam " . " nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat.", $title ); @@ -60,6 +60,8 @@ class WikiPageTest extends MediaWikiTestCase { $this->assertTrue( $title->exists(), "Title object should indicate that the page now exists" ); $this->assertTrue( $page->exists(), "WikiPage object should indicate that the page now exists" ); + $id = $page->getId(); + # ------------------------ $page = new WikiPage( $title ); @@ -67,8 +69,8 @@ class WikiPageTest extends MediaWikiTestCase { $this->assertTrue( $content->equals( $retrieved ), 'retrieved content doesn\'t equal original' ); # ------------------------ - $content = ContentHandler::makeContent( "At vero eos et accusam et justo duo dolores et ea rebum. " - . "Stet clita kasd gubergren, no sea takimata sanctus est.", + $content = ContentHandler::makeContent( "At vero eos et accusam et justo duo [[dolores]] et ea rebum. " + . "Stet clita kasd [[gubergren]], no sea takimata sanctus est.", $title ); $page->doEditContent( $content, "testing 2" ); @@ -78,6 +80,14 @@ class WikiPageTest extends MediaWikiTestCase { $retrieved = $page->getContent(); $this->assertTrue( $content->equals( $retrieved ), 'retrieved content doesn\'t equal original' ); + + # ------------------------ + $dbr = wfGetDB( DB_SLAVE ); + $res = $dbr->select( 'pagelinks', array( 'pl_from' => $id ) ); + $n = $res->numRows(); + $res->free(); + + $this->assertEquals( 2, $n, 'pagelinks should contain two links from the page' ); } public function testDoEdit() { @@ -85,7 +95,7 @@ class WikiPageTest extends MediaWikiTestCase { $page = $this->newPage( $title ); - $text = "Lorem ipsum dolor sit amet, consetetur sadipscing elitr, sed diam " + $text = "[[Lorem ipsum]] dolor sit amet, consetetur sadipscing elitr, sed diam " . " nonumy eirmod tempor invidunt ut labore et dolore magna aliquyam erat."; $page->doEdit( $text, "testing 1" ); @@ -93,6 +103,8 @@ class WikiPageTest extends MediaWikiTestCase { $this->assertTrue( $title->exists(), "Title object should indicate that the page now exists" ); $this->assertTrue( $page->exists(), "WikiPage object should indicate that the page now exists" ); + $id = $page->getId(); + # ------------------------ $page = new WikiPage( $title ); @@ -100,8 +112,8 @@ class WikiPageTest extends MediaWikiTestCase { $this->assertEquals( $text, $retrieved, 'retrieved text doesn\'t equal original' ); # ------------------------ - $text = "At vero eos et accusam et justo duo dolores et ea rebum. " - . "Stet clita kasd gubergren, no sea takimata sanctus est."; + $text = "At vero eos et accusam et justo duo [[dolores]] et ea rebum. " + . "Stet clita kasd [[gubergren]], no sea takimata sanctus est."; $page->doEdit( $text, "testing 2" ); @@ -110,6 +122,14 @@ class WikiPageTest extends MediaWikiTestCase { $retrieved = $page->getText(); $this->assertEquals( $text, $retrieved, 'retrieved text doesn\'t equal original' ); + + # ------------------------ + $dbr = wfGetDB( DB_SLAVE ); + $res = $dbr->select( 'pagelinks', array( 'pl_from' => $id ) ); + $n = $res->numRows(); + $res->free(); + + $this->assertEquals( 2, $n, 'pagelinks should contain two links from the page' ); } public function testDoQuickEdit() { @@ -139,17 +159,26 @@ class WikiPageTest extends MediaWikiTestCase { } public function testDoDeleteArticle() { - $page = $this->createPage( "WikiPageTest_testDoDeleteArticle", "original text" ); + $page = $this->createPage( "WikiPageTest_testDoDeleteArticle", "[[original text]] foo" ); + $id = $page->getId(); $page->doDeleteArticle( "testing deletion" ); - $this->assertFalse( $page->exists() ); + $this->assertFalse( $page->exists(), "WikiPage::exists should return false after page was deleted" ); - $this->assertNull( $page->getContent() ); - $this->assertFalse( $page->getText() ); + $this->assertNull( $page->getContent(), "WikiPage::getContent should return null after page was deleted" ); + $this->assertFalse( $page->getText(), "WikiPage::getText should return false after page was deleted" ); $t = Title::newFromText( $page->getTitle()->getPrefixedText() ); - $this->assertFalse( $t->exists() ); + $this->assertFalse( $t->exists(), "Title::exists should return false after page was deleted" ); + + # ------------------------ + $dbr = wfGetDB( DB_SLAVE ); + $res = $dbr->select( 'pagelinks', array( 'pl_from' => $id ) ); + $n = $res->numRows(); + $res->free(); + + $this->assertEquals( 0, $n, 'pagelinks should contain no more links from the page' ); } public function testGetRevision() { -- 2.20.1