using SecondaryDataUpdate to clean up after deletion
authordaniel <daniel.kinzler@wikimedia.de>
Mon, 30 Apr 2012 13:32:31 +0000 (15:32 +0200)
committerdaniel <daniel.kinzler@wikimedia.de>
Mon, 30 Apr 2012 13:32:31 +0000 (15:32 +0200)
includes/AutoLoader.php
includes/ContentHandler.php
includes/LinksUpdate.php
includes/SecondaryDataUpdate.php
includes/WikiPage.php
tests/phpunit/includes/WikiPageTest.php

index 0f163f6..5d9f0b3 100644 (file)
@@ -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',
index 7e11cdd..ac54be3 100644 (file)
@@ -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 ),
+               );
+       }
 }
 
 
index 4097055..4717d83 100644 (file)
@@ -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
index 43e7497..e02b2b3 100644 (file)
  *
  * 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.
         *
index c1af18f..92f1d93 100644 (file)
@@ -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;
        }
 }
+
index 6ecda15..357de0c 100644 (file)
@@ -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() {