From 5ad5cb4f0a64bbf52e7b8e77e44dd6968d0624a6 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Tue, 22 Jul 2008 22:44:34 +0000 Subject: [PATCH] * (bug 4578) Automatically fix redirects broken by a page move. Works via the job queue, controllable by a checkbox on Special:Movepage. * Renamed some excessively short variables in SpecialMovepage.php * Allow $wgReservedUsernames to be localised using "msg:..." syntax --- RELEASE-NOTES | 1 + includes/AutoLoader.php | 1 + includes/DefaultSettings.php | 2 + includes/JobQueue.php | 28 +++++-- includes/User.php | 19 +++-- includes/specials/SpecialMovepage.php | 102 ++++++++++++++++---------- languages/messages/MessagesEn.php | 4 + 7 files changed, 109 insertions(+), 48 deletions(-) diff --git a/RELEASE-NOTES b/RELEASE-NOTES index fa69fa8d39..779585056a 100644 --- a/RELEASE-NOTES +++ b/RELEASE-NOTES @@ -194,6 +194,7 @@ it from source control: http://www.mediawiki.org/wiki/Download_from_SVN * Special:Recentchangeslinked now includes changes to transcluded pages and displayed images; also, the "Show changes to pages linked" checkbox now works on category pages too, showing all links that are not categorizations +* (bug 4578) Automatically fix redirects broken by a page move === Bug fixes in 1.13 === diff --git a/includes/AutoLoader.php b/includes/AutoLoader.php index e90491b6e6..4f36784a78 100644 --- a/includes/AutoLoader.php +++ b/includes/AutoLoader.php @@ -43,6 +43,7 @@ class AutoLoader { '_DiffOp' => 'includes/DifferenceEngine.php', 'DjVuImage' => 'includes/DjVuImage.php', 'DoubleReplacer' => 'includes/StringUtils.php', + 'DoubleRedirectJob' => 'includes/DoubleRedirectJob.php', 'Dump7ZipOutput' => 'includes/Export.php', 'DumpBZip2Output' => 'includes/Export.php', 'DumpFileOutput' => 'includes/Export.php', diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php index 3a13d3221d..dfb4707eee 100644 --- a/includes/DefaultSettings.php +++ b/includes/DefaultSettings.php @@ -1595,6 +1595,7 @@ $wgJobClasses = array( 'html_cache_update' => 'HTMLCacheUpdateJob', // backwards-compatible 'sendMail' => 'EmaillingJob', 'enotifNotify' => 'EnotifNotifyJob', + 'fixDoubleRedirect' => 'DoubleRedirectJob', ); /** @@ -3078,6 +3079,7 @@ $wgReservedUsernames = array( 'Conversion script', // Used for the old Wikipedia software upgrade 'Maintenance script', // Maintenance scripts which perform editing, image import script 'Template namespace initialisation script', // Used in 1.2->1.3 upgrade + 'msg:double-redirect-fixer', // Automatic double redirect fix ); /** diff --git a/includes/JobQueue.php b/includes/JobQueue.php index 23fabd74f5..8bfd1b3e59 100644 --- a/includes/JobQueue.php +++ b/includes/JobQueue.php @@ -163,7 +163,10 @@ abstract class Job { $job = Job::factory( $row->job_cmd, $title, Job::extractBlob( $row->job_params ), $row->job_id ); // Remove any duplicates it may have later in the queue + // Deadlock prone section + $dbw->begin(); $dbw->delete( 'job', $job->insertFields(), __METHOD__ ); + $dbw->commit(); wfProfileOut( __METHOD__ ); return $job; @@ -213,12 +216,23 @@ abstract class Job { * @param $jobs array of Job objects */ static function batchInsert( $jobs ) { - if( count( $jobs ) ) { - $dbw = wfGetDB( DB_MASTER ); - $dbw->begin(); - foreach( $jobs as $job ) { - $rows[] = $job->insertFields(); + if( !count( $jobs ) ) { + return; + } + $dbw = wfGetDB( DB_MASTER ); + $rows = array(); + foreach( $jobs as $job ) { + $rows[] = $job->insertFields(); + if ( count( $rows ) >= 50 ) { + # Do a small transaction to avoid slave lag + $dbw->begin(); + $dbw->insert( 'job', $rows, __METHOD__, 'IGNORE' ); + $dbw->commit(); + $rows = array(); } + } + if ( $rows ) { + $dbw->begin(); $dbw->insert( 'job', $rows, __METHOD__, 'IGNORE' ); $dbw->commit(); } @@ -288,6 +302,10 @@ abstract class Job { } } + protected function setLastError( $error ) { + $this->error = $error; + } + function getLastError() { return $this->error; } diff --git a/includes/User.php b/includes/User.php index ed5257f8e4..5c12981970 100644 --- a/includes/User.php +++ b/includes/User.php @@ -497,12 +497,21 @@ class User { */ static function isUsableName( $name ) { global $wgReservedUsernames; - return - // Must be a valid username, obviously ;) - self::isValidUserName( $name ) && + // Must be a valid username, obviously ;) + if ( !self::isValidUserName( $name ) ) { + return false; + } - // Certain names may be reserved for batch processes. - !in_array( $name, $wgReservedUsernames ); + // Certain names may be reserved for batch processes. + foreach ( $wgReservedUsernames as $reserved ) { + if ( substr( $reserved, 0, 4 ) == 'msg:' ) { + $reserved = wfMsgForContent( substr( $reserved, 4 ) ); + } + if ( $reserved == $name ) { + return false; + } + } + return true; } /** diff --git a/includes/specials/SpecialMovepage.php b/includes/specials/SpecialMovepage.php index caba8c4df0..cbf9045113 100644 --- a/includes/specials/SpecialMovepage.php +++ b/includes/specials/SpecialMovepage.php @@ -17,36 +17,35 @@ function wfSpecialMovepage( $par = null ) { } $target = isset( $par ) ? $par : $wgRequest->getVal( 'target' ); - $oldTitle = $wgRequest->getText( 'wpOldTitle', $target ); - $newTitle = $wgRequest->getText( 'wpNewTitle' ); + $oldTitleText = $wgRequest->getText( 'wpOldTitle', $target ); + $newTitleText = $wgRequest->getText( 'wpNewTitle' ); - # Variables beginning with 'o' for old article 'n' for new article - $ot = Title::newFromText( $oldTitle ); - $nt = Title::newFromText( $newTitle ); + $oldTitle = Title::newFromText( $oldTitleText ); + $newTitle = Title::newFromText( $newTitleText ); - if( is_null( $ot ) ) { + if( is_null( $oldTitle ) ) { $wgOut->showErrorPage( 'notargettitle', 'notargettext' ); return; } - if( !$ot->exists() ) { + if( !$oldTitle->exists() ) { $wgOut->showErrorPage( 'nopagetitle', 'nopagetext' ); return; } # Check rights - $permErrors = $ot->getUserPermissionsErrors( 'move', $wgUser ); + $permErrors = $oldTitle->getUserPermissionsErrors( 'move', $wgUser ); if( !empty( $permErrors ) ) { $wgOut->showPermissionsErrorPage( $permErrors ); return; } - $f = new MovePageForm( $ot, $nt ); + $form = new MovePageForm( $oldTitle, $newTitle ); if ( 'submit' == $action && $wgRequest->wasPosted() && $wgUser->matchEditToken( $wgRequest->getVal( 'wpEditToken' ) ) ) { - $f->doSubmit(); + $form->doSubmit(); } else { - $f->showForm( '' ); + $form->showForm( '' ); } } @@ -56,7 +55,7 @@ function wfSpecialMovepage( $par = null ) { */ class MovePageForm { var $oldTitle, $newTitle, $reason; # Text input - var $moveTalk, $deleteAndMove, $moveSubpages; + var $moveTalk, $deleteAndMove, $moveSubpages, $fixRedirects; private $watch = false; @@ -73,17 +72,17 @@ class MovePageForm { } $this->moveSubpages = $wgRequest->getBool( 'wpMovesubpages', false ); $this->deleteAndMove = $wgRequest->getBool( 'wpDeleteAndMove' ) && $wgRequest->getBool( 'wpConfirm' ); + $this->fixRedirects = $wgRequest->getBool( 'wpFixRedirects', true ); $this->watch = $wgRequest->getCheck( 'wpWatch' ); } function showForm( $err, $hookErr = '' ) { global $wgOut, $wgUser; - $ot = $this->oldTitle; - $sk = $wgUser->getSkin(); + $skin = $wgUser->getSkin(); - $oldTitleLink = $sk->makeLinkObj( $ot ); - $oldTitle = $ot->getPrefixedText(); + $oldTitleLink = $skin->makeLinkObj( $this->oldTitle ); + $oldTitle = $this->oldTitle->getPrefixedText(); $wgOut->setPagetitle( wfMsg( 'move-page', $oldTitle ) ); $wgOut->setSubtitle( wfMsg( 'move-page-backlink', $oldTitleLink ) ); @@ -99,7 +98,7 @@ class MovePageForm { # If a title was supplied, probably from the move log revert # link, check for validity. We can then show some diagnostic # information and save a click. - $newerr = $ot->isValidMoveOperation( $nt ); + $newerr = $this->oldTitle->isValidMoveOperation( $nt ); if( is_string( $newerr ) ) { $err = $newerr; } @@ -127,9 +126,16 @@ class MovePageForm { $confirm = false; } - $oldTalk = $ot->getTalkPage(); - $considerTalk = ( !$ot->isTalkPage() && $oldTalk->exists() ); + $oldTalk = $this->oldTitle->getTalkPage(); + $considerTalk = ( !$this->oldTitle->isTalkPage() && $oldTalk->exists() ); + $dbr = wfGetDB( DB_SLAVE ); + $hasRedirects = $dbr->selectField( 'redirect', '1', + array( + 'rd_namespace' => $this->oldTitle->getNamespace(), + 'rd_title' => $this->oldTitle->getDBkey(), + ) , __METHOD__ ); + if ( $considerTalk ) { $wgOut->addWikiMsg( 'movepagetalktext' ); } @@ -190,28 +196,41 @@ class MovePageForm { ); } - if( ($ot->hasSubpages() || $ot->getTalkPage()->hasSubpages()) - && $ot->userCan( 'move-subpages' ) ) { + if ( $hasRedirects ) { + $wgOut->addHTML( " + + + " . + Xml::checkLabel( wfMsg( 'fix-double-redirects' ), 'wpFixRedirects', + 'wpFixRedirects', $this->fixRedirects ) . + " + " + ); + } + + if( ($this->oldTitle->hasSubpages() || $this->oldTitle->getTalkPage()->hasSubpages()) + && $this->oldTitle->userCan( 'move-subpages' ) ) { $wgOut->addHTML( " " . Xml::checkLabel( wfMsgHtml( - $ot->hasSubpages() + $this->oldTitle->hasSubpages() ? 'move-subpages' : 'move-talk-subpages' ), 'wpMovesubpages', 'wpMovesubpages', # Don't check the box if we only have talk subpages to # move and we aren't moving the talk page. - $this->moveSubpages && ($ot->hasSubpages() || $this->moveTalk) + $this->moveSubpages && ($this->oldTitle->hasSubpages() || $this->moveTalk) ) . " " ); } - $watchChecked = $this->watch || $wgUser->getBoolOption( 'watchmoves' ) || $ot->userIsWatching(); + $watchChecked = $this->watch || $wgUser->getBoolOption( 'watchmoves' ) + || $this->oldTitle->userIsWatching(); $wgOut->addHTML( " @@ -233,7 +252,7 @@ class MovePageForm { "\n" ); - $this->showLogFragment( $ot, $wgOut ); + $this->showLogFragment( $this->oldTitle, $wgOut ); } @@ -277,6 +296,10 @@ class MovePageForm { return; } + if ( $this->fixRedirects ) { + DoubleRedirectJob::fixRedirects( 'move', $ot, $nt ); + } + wfRunHooks( 'SpecialMovepageAfterMove', array( &$this , &$ot , &$nt ) ) ; $wgOut->setPagetitle( wfMsg( 'pagemovedsub' ) ); @@ -358,40 +381,43 @@ class MovePageForm { continue; } - $oldPage = Title::newFromRow( $row ); + $oldSubpage = Title::newFromRow( $row ); $newPageName = preg_replace( '#^'.preg_quote( $ot->getDBKey(), '#' ).'#', $nt->getDBKey(), - $oldPage->getDBKey() + $oldSubpage->getDBKey() ); - if( $oldPage->isTalkPage() ) { + if( $oldSubpage->isTalkPage() ) { $newNs = $nt->getTalkPage()->getNamespace(); } else { $newNs = $nt->getSubjectPage()->getNamespace(); } # Bug 14385: we need makeTitleSafe because the new page names may # be longer than 255 characters. - $newPage = Title::makeTitleSafe( $newNs, $newPageName ); - if( !$newPage ) { - $oldLink = $skin->makeKnownLinkObj( $oldPage ); + $newSubpage = Title::makeTitleSafe( $newNs, $newPageName ); + if( !$newSubpage ) { + $oldLink = $skin->makeKnownLinkObj( $oldSubpage ); $extraOutput []= wfMsgHtml( 'movepage-page-unmoved', $oldLink, htmlspecialchars(Title::makeName( $newNs, $newPageName ))); continue; } # This was copy-pasted from Renameuser, bleh. - if ( $newPage->exists() && !$oldPage->isValidMoveTarget( $newPage ) ) { - $link = $skin->makeKnownLinkObj( $newPage ); + if ( $newSubpage->exists() && !$oldSubpage->isValidMoveTarget( $newSubpage ) ) { + $link = $skin->makeKnownLinkObj( $newSubpage ); $extraOutput []= wfMsgHtml( 'movepage-page-exists', $link ); } else { - $success = $oldPage->moveTo( $newPage, true, $this->reason ); + $success = $oldSubpage->moveTo( $newSubpage, true, $this->reason ); if( $success === true ) { - $oldLink = $skin->makeKnownLinkObj( $oldPage, '', 'redirect=no' ); - $newLink = $skin->makeKnownLinkObj( $newPage ); + if ( $this->fixRedirects ) { + DoubleRedirectJob::fixRedirects( 'move', $oldSubpage, $newSubpage ); + } + $oldLink = $skin->makeKnownLinkObj( $oldSubpage, '', 'redirect=no' ); + $newLink = $skin->makeKnownLinkObj( $newSubpage ); $extraOutput []= wfMsgHtml( 'movepage-page-moved', $oldLink, $newLink ); } else { - $oldLink = $skin->makeKnownLinkObj( $oldPage ); - $newLink = $skin->makeLinkObj( $newPage ); + $oldLink = $skin->makeKnownLinkObj( $oldSubpage ); + $newLink = $skin->makeLinkObj( $newSubpage ); $extraOutput []= wfMsgHtml( 'movepage-page-unmoved', $oldLink, $newLink ); } } diff --git a/languages/messages/MessagesEn.php b/languages/messages/MessagesEn.php index 96f0c4ad38..1d080ce51f 100644 --- a/languages/messages/MessagesEn.php +++ b/languages/messages/MessagesEn.php @@ -1871,6 +1871,9 @@ A page is treated as disambiguation page if it uses a template which is linked f 'doubleredirectstext' => 'This page lists pages which redirect to other redirect pages. Each row contains links to the first and second redirect, as well as the target of the second redirect, which is usually "real" target page, which the first redirect should point to.', +'double-redirect-fixed-move' => '[[$1]] has been moved, it is now just a redirect to [[$2]]', +'double-redirect-fixer' => 'Redirect fixer', + 'brokenredirects' => 'Broken redirects', 'brokenredirects-summary' => '', # do not translate or duplicate this message to other languages 'brokenredirectstext' => 'The following redirects link to non-existent pages:', @@ -2500,6 +2503,7 @@ cannot move pages from and into that namespace.', 'imagenocrossnamespace' => 'Cannot move file to non-file namespace', 'imagetypemismatch' => 'The new file extension does not match its type', 'imageinvalidfilename' => 'The target file name is invalid', +'fix-double-redirects' => 'Update any redirects that point to the original title', # Export 'export' => 'Export pages', -- 2.20.1