From 6d315fb7f74a966c06788a21baf89cc81ed12679 Mon Sep 17 00:00:00 2001 From: daniel Date: Tue, 5 Mar 2019 22:07:13 +0100 Subject: [PATCH] Make LinksDeletionUpdate a subclass of LinksUpdate This is proposed as an alternative to I0b636dc144f34bb. The idea is to ensure that page deletion causes the exact same database updates to be performed, and the exact same hooks to be fired, as a page edit. Bug: T216249 Change-Id: I665320e27da8edc2867b47d181cc0f324e75d102 --- RELEASE-NOTES-1.33 | 5 + includes/deferred/LinksDeletionUpdate.php | 141 +++------------------- includes/deferred/LinksUpdate.php | 8 +- 3 files changed, 29 insertions(+), 125 deletions(-) diff --git a/RELEASE-NOTES-1.33 b/RELEASE-NOTES-1.33 index cfaf82d7fd..d3a09c5e78 100644 --- a/RELEASE-NOTES-1.33 +++ b/RELEASE-NOTES-1.33 @@ -412,6 +412,11 @@ because of Phabricator reports. changed to explicitly cast. Subclasses relying on the base-class implementation should check whether they need to override it now. * BagOStuff::add is now abstract and must explicitly be defined in subclasses. +* LinksDeletionUpdate is now a subclass of LinksUpdate. As a consequence, + the following hooks will now be triggered upon page deletion in addition + to page updates: LinksUpdateConstructed, LinksUpdate, LinksUpdateComplete. + LinksUpdateAfterInsert is not triggered since deletions do not cause + insertions into links tables. == Compatibility == MediaWiki 1.33 requires PHP 7.0.13 or later. Although HHVM 3.18.5 or later is diff --git a/includes/deferred/LinksDeletionUpdate.php b/includes/deferred/LinksDeletionUpdate.php index 4444bac45c..0e24134770 100644 --- a/includes/deferred/LinksDeletionUpdate.php +++ b/includes/deferred/LinksDeletionUpdate.php @@ -21,22 +21,16 @@ */ use MediaWiki\MediaWikiServices; use Wikimedia\ScopedCallback; -use Wikimedia\Rdbms\IDatabase; /** * Update object handling the cleanup of links tables after a page was deleted. */ -class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate { +class LinksDeletionUpdate extends LinksUpdate implements EnqueueableDataUpdate { /** @var WikiPage */ protected $page; - /** @var int */ - protected $pageId; /** @var string */ protected $timestamp; - /** @var IDatabase */ - private $db; - /** * @param WikiPage $page Page we are updating * @param int|null $pageId ID of the page we are updating [optional] @@ -44,63 +38,37 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate { * @throws MWException */ function __construct( WikiPage $page, $pageId = null, $timestamp = null ) { - parent::__construct(); - $this->page = $page; if ( $pageId ) { - $this->pageId = $pageId; // page ID at time of deletion + $this->mId = $pageId; // page ID at time of deletion } elseif ( $page->exists() ) { - $this->pageId = $page->getId(); + $this->mId = $page->getId(); } else { throw new InvalidArgumentException( "Page ID not known. Page doesn't exist?" ); } $this->timestamp = $timestamp ?: wfTimestampNow(); + + $fakePO = new ParserOutput(); + $fakePO->setCacheTime( $timestamp ); + parent::__construct( $page->getTitle(), $fakePO, false ); } - public function doUpdate() { + protected function doIncrementalUpdate() { $services = MediaWikiServices::getInstance(); $config = $services->getMainConfig(); $lbFactory = $services->getDBLoadBalancerFactory(); $batchSize = $config->get( 'UpdateRowsPerQuery' ); - // Page may already be deleted, so don't just getId() - $id = $this->pageId; - - if ( $this->ticket ) { - // Make sure all links update threads see the changes of each other. - // This handles the case when updates have to batched into several COMMITs. - $scopedLock = LinksUpdate::acquirePageLock( $this->getDB(), $id ); - if ( !$scopedLock ) { - throw new RuntimeException( "Could not acquire lock for page ID '{$id}'." ); - } - } + $id = $this->mId; + $title = $this->mTitle; - $title = $this->page->getTitle(); $dbw = $this->getDB(); // convenience - // Delete restrictions for it - $dbw->delete( 'page_restrictions', [ 'pr_page' => $id ], __METHOD__ ); + parent::doIncrementalUpdate(); - // Fix category table counts - $cats = $dbw->selectFieldValues( - 'categorylinks', - 'cl_to', - [ 'cl_from' => $id ], - __METHOD__ - ); - $catBatches = array_chunk( $cats, $batchSize ); - foreach ( $catBatches as $catBatch ) { - $this->page->updateCategoryCounts( [], $catBatch, $id ); - if ( count( $catBatches ) > 1 ) { - // Only sacrifice atomicity if necessary due to size - $lbFactory->commitAndWaitForReplication( - __METHOD__, $this->ticket, [ 'domain' => $dbw->getDomainID() ] - ); - } - } - - // Refresh counts on categories that should be empty now + // Typically, a category is empty when deleted, so check that we don't leave + // spurious row in the category table. if ( $title->getNamespace() === NS_CATEGORY ) { // T166757: do the update after the main job DB commit DeferredUpdates::addCallableUpdate( function () use ( $title ) { @@ -109,52 +77,11 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate { } ); } - $this->batchDeleteByPK( - 'pagelinks', - [ 'pl_from' => $id ], - [ 'pl_from', 'pl_namespace', 'pl_title' ], - $batchSize - ); - $this->batchDeleteByPK( - 'imagelinks', - [ 'il_from' => $id ], - [ 'il_from', 'il_to' ], - $batchSize - ); - $this->batchDeleteByPK( - 'categorylinks', - [ 'cl_from' => $id ], - [ 'cl_from', 'cl_to' ], - $batchSize - ); - $this->batchDeleteByPK( - 'templatelinks', - [ 'tl_from' => $id ], - [ 'tl_from', 'tl_namespace', 'tl_title' ], - $batchSize - ); - $this->batchDeleteByPK( - 'externallinks', - [ 'el_from' => $id ], - [ 'el_id' ], - $batchSize - ); - $this->batchDeleteByPK( - 'langlinks', - [ 'll_from' => $id ], - [ 'll_from', 'll_lang' ], - $batchSize - ); - $this->batchDeleteByPK( - 'iwlinks', - [ 'iwl_from' => $id ], - [ 'iwl_from', 'iwl_prefix', 'iwl_title' ], - $batchSize - ); + // Delete restrictions for the deleted page + $dbw->delete( 'page_restrictions', [ 'pr_page' => $id ], __METHOD__ ); - // Delete any redirect entry or page props entries + // Delete any redirect entry $dbw->delete( 'redirect', [ 'rd_from' => $id ], __METHOD__ ); - $dbw->delete( 'page_props', [ 'pp_page' => $id ], __METHOD__ ); // Find recentchanges entries to clean up... $rcIdsForTitle = $dbw->selectFieldValues( @@ -191,46 +118,14 @@ class LinksDeletionUpdate extends DataUpdate implements EnqueueableDataUpdate { ScopedCallback::consume( $scopedLock ); } - private function batchDeleteByPK( $table, array $conds, array $pk, $bSize ) { - $services = MediaWikiServices::getInstance(); - $lbFactory = $services->getDBLoadBalancerFactory(); - $dbw = $this->getDB(); // convenience - - $res = $dbw->select( $table, $pk, $conds, __METHOD__ ); - - $pkDeleteConds = []; - foreach ( $res as $row ) { - $pkDeleteConds[] = $dbw->makeList( (array)$row, LIST_AND ); - if ( count( $pkDeleteConds ) >= $bSize ) { - $dbw->delete( $table, $dbw->makeList( $pkDeleteConds, LIST_OR ), __METHOD__ ); - $lbFactory->commitAndWaitForReplication( - __METHOD__, $this->ticket, [ 'domain' => $dbw->getDomainID() ] - ); - $pkDeleteConds = []; - } - } - - if ( $pkDeleteConds ) { - $dbw->delete( $table, $dbw->makeList( $pkDeleteConds, LIST_OR ), __METHOD__ ); - } - } - - protected function getDB() { - if ( !$this->db ) { - $this->db = wfGetDB( DB_MASTER ); - } - - return $this->db; - } - public function getAsJobSpecification() { return [ 'domain' => $this->getDB()->getDomainID(), 'job' => new JobSpecification( 'deleteLinks', - [ 'pageId' => $this->pageId, 'timestamp' => $this->timestamp ], + [ 'pageId' => $this->mId, 'timestamp' => $this->timestamp ], [ 'removeDuplicates' => true ], - $this->page->getTitle() + $this->mTitle ) ]; } diff --git a/includes/deferred/LinksUpdate.php b/includes/deferred/LinksUpdate.php index 045795e0cd..14f86b7c5e 100644 --- a/includes/deferred/LinksUpdate.php +++ b/includes/deferred/LinksUpdate.php @@ -122,7 +122,11 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { parent::__construct(); $this->mTitle = $title; - $this->mId = $title->getArticleID( Title::GAID_FOR_UPDATE ); + + if ( !$this->mId ) { + // NOTE: subclasses may initialize mId before calling this constructor! + $this->mId = $title->getArticleID( Title::GAID_FOR_UPDATE ); + } if ( !$this->mId ) { throw new InvalidArgumentException( @@ -1180,7 +1184,7 @@ class LinksUpdate extends DataUpdate implements EnqueueableDataUpdate { /** * @return IDatabase */ - private function getDB() { + protected function getDB() { if ( !$this->db ) { $this->db = wfGetDB( DB_MASTER ); } -- 2.20.1