jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/298511 )
Change subject: Rewrite of translatable page moving code
......................................................................
Rewrite of translatable page moving code
Get rid of some ugly old code by using MovePage. Instead of splitting
moves into many many jobs, use one job that does it all. This allows
to simplify many things.
Old job class to be removed later.
Change-Id: Idbbdd09dcc214fecabc28607cc3207324fcb7fee
---
M Autoload.php
M TranslateHooks.php
M i18n/pagetranslation/en.json
M tag/PageTranslationHooks.php
M tag/SpecialPageTranslationMovePage.php
A tag/TranslatablePageMoveJob.php
6 files changed, 185 insertions(+), 92 deletions(-)
Approvals:
jenkins-bot: Verified
Siebrand: Looks good to me, approved
diff --git a/Autoload.php b/Autoload.php
index 11b6106..7c1f72a 100644
--- a/Autoload.php
+++ b/Autoload.php
@@ -132,6 +132,7 @@
* @ingroup PageTranslation
* @{
*/
+$al['TranslatablePageMoveJob'] = "$dir/tag/TranslatablePageMoveJob.php";
$al['TranslateDeleteJob'] = "$dir/tag/TranslateDeleteJob.php";
$al['TranslateMoveJob'] = "$dir/tag/TranslateMoveJob.php";
$al['SpecialPageMigration'] = "$dir/tag/SpecialPageMigration.php";
diff --git a/TranslateHooks.php b/TranslateHooks.php
index 9b49eeb..0bf9c7f 100644
--- a/TranslateHooks.php
+++ b/TranslateHooks.php
@@ -110,6 +110,7 @@
global $wgJobClasses;
$wgJobClasses['TranslateRenderJob'] =
'TranslateRenderJob';
$wgJobClasses['RenderJob'] = 'TranslateRenderJob';
+ $wgJobClasses['TranslatablePageMoveJob'] =
'TranslatablePageMoveJob';
$wgJobClasses['TranslateMoveJob'] = 'TranslateMoveJob';
$wgJobClasses['MoveJob'] = 'TranslateMoveJob';
$wgJobClasses['TranslateDeleteJob'] =
'TranslateDeleteJob';
diff --git a/i18n/pagetranslation/en.json b/i18n/pagetranslation/en.json
index 09881f8..f1b2023 100644
--- a/i18n/pagetranslation/en.json
+++ b/i18n/pagetranslation/en.json
@@ -147,7 +147,7 @@
"pt-movepage-action-other": "Change target",
"pt-movepage-intro": "This special page allows you to move pages which
are marked for translation.\nThe move action will not be instant, because many
pages will need to be moved.\nWhile the pages are being moved, it is not
possible to interact with the pages in question.\nFailures will be logged in
the [[Special:Log/pagetranslation|page translation log]] and they need to be
repaired by hand.",
"pt-movepage-logreason": "Part of translatable page \"$1\"",
- "pt-movepage-started": "The base page is now moved.\nPlease check the
[[Special:Log/pagetranslation|page translation log]] for errors and completion
message.",
+ "pt-movepage-started": "Please check the
[[Special:Log/pagetranslation|page translation log]] in a while for errors and
completion message.",
"pt-locked-page": "This page is locked because the translatable page is
currently being moved.",
"pt-deletepage-lang-title": "Deleting translation page \"$1\"",
"pt-deletepage-full-title": "Deleting translatable page \"$1\"",
diff --git a/tag/PageTranslationHooks.php b/tag/PageTranslationHooks.php
index 55ad8e9..8770876 100644
--- a/tag/PageTranslationHooks.php
+++ b/tag/PageTranslationHooks.php
@@ -826,8 +826,7 @@
$cache = wfGetCache( CACHE_ANYTHING );
$key = wfMemcKey( 'pt-lock', sha1( $title->getPrefixedText() )
);
- // At least memcached mangles true to "1"
- if ( $cache->get( $key ) !== false ) {
+ if ( $cache->get( $key ) === 'locked' ) {
$result = [ 'pt-locked-page' ];
return false;
diff --git a/tag/SpecialPageTranslationMovePage.php
b/tag/SpecialPageTranslationMovePage.php
index a84afbe..df422f0 100644
--- a/tag/SpecialPageTranslationMovePage.php
+++ b/tag/SpecialPageTranslationMovePage.php
@@ -402,114 +402,42 @@
}
protected function performAction() {
- $jobs = [];
- $user = $this->getUser();
$target = $this->newTitle;
$base = $this->oldTitle->getPrefixedText();
- $oldLatest = $this->oldTitle->getLatestRevID();
- $params = [
- 'base-source' => $this->oldTitle->getPrefixedText(),
- 'base-target' => $this->newTitle->getPrefixedText(),
- ];
+ $moves = [];
+ $moves[$base] = $target->getPrefixedText();
- $translationPages = $this->getTranslationPages();
- foreach ( $translationPages as $old ) {
- $to = $this->newPageTitle( $base, $old, $target );
- $jobs[$old->getPrefixedText()] =
TranslateMoveJob::newJob( $old, $to, $params, $user );
+ foreach ( $this->getTranslationPages() as $from ) {
+ $to = $this->newPageTitle( $base, $from, $target );
+ $moves[$from->getPrefixedText()] =
$to->getPrefixedText();
}
- $sectionPages = $this->getSectionPages();
- foreach ( $sectionPages as $old ) {
- $to = $this->newPageTitle( $base, $old, $target );
- $jobs[$old->getPrefixedText()] =
TranslateMoveJob::newJob( $old, $to, $params, $user );
+ foreach ( $this->getSectionPages() as $from ) {
+ $to = $this->newPageTitle( $base, $from, $target );
+ $moves[$from->getPrefixedText()] =
$to->getPrefixedText();
}
if ( $this->moveSubpages ) {
$subpages = $this->getSubpages();
- foreach ( $subpages as $old ) {
- if ( TranslatablePage::isTranslationPage( $old
) ) {
+ foreach ( $subpages as $from ) {
+ if ( TranslatablePage::isTranslationPage( $from
) ) {
continue;
}
- $to = $this->newPageTitle( $base, $old, $target
);
- $jobs[$old->getPrefixedText()] =
TranslateMoveJob::newJob(
- $old,
- $to,
- $params,
- $user
- );
+ $to = $this->newPageTitle( $base, $from,
$target );
+ $moves[$from->getPrefixedText()] =
$to->getPrefixedText();
}
}
- // This is used by TranslateMoveJob
- wfGetCache( CACHE_ANYTHING )->set( wfMemcKey(
'translate-pt-move', $base ), count( $jobs ) );
- JobQueueGroup::singleton()->push( $jobs );
+ $summary = $this->msg( 'pt-movepage-logreason', $base
)->inContentLanguage()->text();
+ $job = TranslatablePageMoveJob::newJob(
+ $this->oldTitle, $this->newTitle, $moves, $summary,
$this->getUser()
+ );
- TranslateMoveJob::forceRedirects( false );
-
- $errors = $this->oldTitle->moveTo( $this->newTitle, true,
$this->reason, false );
- if ( is_array( $errors ) ) {
- $this->showErrors( $errors );
- }
-
- TranslateMoveJob::forceRedirects( true );
-
- $newTpage = TranslatablePage::newFromTitle( $this->newTitle );
- $newTpage->addReadyTag( $this->newTitle->getLatestRevID(
Title::GAID_FOR_UPDATE ) );
-
- if ( $newTpage->getMarkedTag() === $oldLatest ) {
- $newTpage->addMarkedTag(
$this->newTitle->getLatestRevID( Title::GAID_FOR_UPDATE ) );
- }
-
- // remove the entries from metadata table.
- $oldGroupId = $this->page->getMessageGroupId();
- $newGroupId = $newTpage->getMessageGroupId();
- $this->moveMetadata( $oldGroupId, $newGroupId );
-
- MessageGroups::singleton()->recache();
- MessageIndexRebuildJob::newJob()->insert();
+ JobQueueGroup::singleton()->push( $job );
$this->getOutput()->addWikiMsg( 'pt-movepage-started' );
- }
-
- protected function moveMetadata( $oldGroupId, $newGroupId ) {
- $prioritylangs = TranslateMetadata::get( $oldGroupId,
'prioritylangs' );
- $priorityforce = TranslateMetadata::get( $oldGroupId,
'priorityforce' );
- $priorityreason = TranslateMetadata::get( $oldGroupId,
'priorityreason' );
- TranslateMetadata::set( $oldGroupId, 'prioritylangs', false );
- TranslateMetadata::set( $oldGroupId, 'priorityforce', false );
- TranslateMetadata::set( $oldGroupId, 'priorityreason', false );
- if ( $prioritylangs ) {
- TranslateMetadata::set( $newGroupId, 'prioritylangs',
$prioritylangs );
- }
- if ( $priorityforce ) {
- TranslateMetadata::set( $newGroupId, 'priorityforce',
$priorityforce );
- }
- if ( $priorityreason !== false ) {
- TranslateMetadata::set( $newGroupId, 'priorityreason',
$priorityreason );
- }
- // make the changes in aggregate groups metadata, if present in
any of them.
- $groups = MessageGroups::getAllGroups();
- foreach ( $groups as $group ) {
- if ( $group instanceof AggregateMessageGroup ) {
- $subgroups = TranslateMetadata::get(
$group->getId(), 'subgroups' );
- if ( $subgroups !== false ) {
- $subgroups = explode( ',', $subgroups );
- $subgroups = array_flip( $subgroups );
- if ( isset( $subgroups[$oldGroupId] ) )
{
- $subgroups[$newGroupId] =
$subgroups[$oldGroupId];
- unset( $subgroups[$oldGroupId]
);
- $subgroups = array_flip(
$subgroups );
- TranslateMetadata::set(
- $group->getId(),
- 'subgroups',
- implode( ',',
$subgroups )
- );
- }
- }
- }
- }
}
protected function checkMoveBlockers() {
diff --git a/tag/TranslatablePageMoveJob.php b/tag/TranslatablePageMoveJob.php
new file mode 100644
index 0000000..52ebaf7
--- /dev/null
+++ b/tag/TranslatablePageMoveJob.php
@@ -0,0 +1,164 @@
+<?php
+/**
+ * Contains class with job for moving translation pages.
+ *
+ * @file
+ * @author Niklas Laxström
+ * @license GPL-2.0+
+ */
+
+/**
+ * Contains class with job for moving translation pages. Used together with
+ * SpecialPageTranslationMovePage class.
+ *
+ * @ingroup PageTranslation JobQueue
+ */
+class TranslatablePageMoveJob extends Job {
+ /**
+ * @param $target Title
+ * @param $params array, should include base-source and base-target
+ * @param $performer
+ * @return TranslateMoveJob
+ */
+ public static function newJob(
+ Title $source, Title $target, array $moves, $summary, User
$performer
+ ) {
+ $params = [
+ 'source' => $source->getPrefixedText(),
+ 'target' => $target->getPrefixedText(),
+ 'moves' => $moves,
+ 'summary' => $summary,
+ 'performer' => $performer->getName(),
+ ];
+
+ $self = new self( $target, $params );
+ $self->lock( array_keys( $moves ) );
+ $self->lock( array_values( $moves ) );
+
+ return $self;
+ }
+
+ public function __construct( $title, $params = [] ) {
+ parent::__construct( __CLASS__, $title, $params );
+ $this->params = $params;
+ }
+
+ public function run() {
+ $sourceTitle = Title::newFromText( $this->params['source'] );
+ $targetTitle = Title::newFromText( $this->params['target'] );
+
+ $this->doMoves();
+
+ $this->moveMetadata(
+ TranslatablePage::newFromTitle( $sourceTitle
)->getMessageGroupId(),
+ TranslatablePage::newFromTitle( $targetTitle
)->getMessageGroupId()
+ );
+
+ $entry = new ManualLogEntry( 'pagetranslation', 'moveok' );
+ $entry->setPerformer( User::newFromName(
$this->params['performer'] ) );
+ $entry->setParameters( [ 'target' => $this->params['target'] ]
);
+ $entry->setTarget( $sourceTitle );
+ $logid = $entry->insert();
+ $entry->publish( $logid );
+
+ // Re-render the pages to get everything in sync
+ MessageGroups::singleton()->recache();
+
+ $job = new TranslationsUpdateJob( $targetTitle, [ 'sections' =>
[] ] );
+ JobQueueGroup::singleton()->push( $job );
+
+ return true;
+ }
+
+ protected function doMoves() {
+ $fuzzybot = FuzzyBot::getUser();
+ $performer = User::newFromName( $this->params['performer'] );
+
+ PageTranslationHooks::$allowTargetEdit = true;
+ PageTranslationHooks::$jobQueueRunning = true;
+
+ foreach ( $this->params['moves'] as $source => $target ) {
+ $sourceTitle = Title::newFromText( $source );
+ $targetTitle = Title::newFromText( $target );
+
+ if ( $source === $this->params['source'] ) {
+ $user = $performer;
+ } else {
+ $user = $fuzzybot;
+ }
+
+ $mover = new MovePage( $sourceTitle, $targetTitle );
+ $status = $mover->move( $user,
$this->params['summary'], false );
+ if ( !$status->isOK() ) {
+ $entry = new ManualLogEntry( 'pagetranslation',
'movenok' );
+ $entry->setPerformer( $performer );
+ $entry->setTarget( $sourceTitle );
+ $entry->setParameters( [
+ 'target' => $target,
+ 'error' => $status->getErrorsArray(),
+ ] );
+ $logid = $entry->insert();
+ $entry->publish( $logid );
+ }
+
+ $this->unlock( [ $source, $target ] );
+ }
+
+ PageTranslationHooks::$allowTargetEdit = false;
+ PageTranslationHooks::$jobQueueRunning = false;
+ }
+
+ protected function moveMetadata( $oldGroupId, $newGroupId ) {
+ $types = [ 'prioritylangs', 'priorityforce', 'priorityreason' ];
+
+ foreach ( $types as $type ) {
+ $value = TranslateMetadata::get( $oldGroupId, $type );
+ if ( $value !== false ) {
+ TranslateMetadata::set( $oldGroupId, $type,
false );
+ TranslateMetadata::set( $newGroupId, $type,
$value );
+ }
+ }
+
+ // Make the changes in aggregate groups metadata, if present in
any of them.
+ $groups = MessageGroups::getAllGroups();
+ foreach ( $groups as $group ) {
+ if ( !$group instanceof AggregateMessageGroup ) {
+ continue;
+ }
+
+ $subgroups = TranslateMetadata::get( $group->getId(),
'subgroups' );
+ if ( $subgroups === false ) {
+ continue;
+ }
+
+ $subgroups = explode( ',', $subgroups );
+ $subgroups = array_flip( $subgroups );
+ if ( isset( $subgroups[$oldGroupId] ) ) {
+ $subgroups[$newGroupId] =
$subgroups[$oldGroupId];
+ unset( $subgroups[$oldGroupId] );
+ $subgroups = array_flip( $subgroups );
+ TranslateMetadata::set(
+ $group->getId(),
+ 'subgroups',
+ implode( ',', $subgroups )
+ );
+ }
+ }
+ }
+
+ private function lock( array $titles ) {
+ $cache = wfGetCache( CACHE_ANYTHING );
+ $data = [];
+ foreach ( $titles as $title ) {
+ $data[wfMemcKey( 'pt-lock', sha1( $title ) )] =
'locked';
+ }
+ $cache->setMulti( $data );
+ }
+
+ private function unlock( array $titles ) {
+ $cache = wfGetCache( CACHE_ANYTHING );
+ foreach ( $titles as $title ) {
+ $cache->delete( wfMemcKey( 'pt-lock', sha1( $title ) )
);
+ }
+ }
+}
--
To view, visit https://gerrit.wikimedia.org/r/298511
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idbbdd09dcc214fecabc28607cc3207324fcb7fee
Gerrit-PatchSet: 12
Gerrit-Project: mediawiki/extensions/Translate
Gerrit-Branch: master
Gerrit-Owner: Nikerabbit <niklas.laxst...@gmail.com>
Gerrit-Reviewer: Amire80 <amir.ahar...@mail.huji.ac.il>
Gerrit-Reviewer: Glaisher <glaisher.w...@gmail.com>
Gerrit-Reviewer: Legoktm <lego...@member.fsf.org>
Gerrit-Reviewer: Nikerabbit <niklas.laxst...@gmail.com>
Gerrit-Reviewer: Santhosh <santhosh.thottin...@gmail.com>
Gerrit-Reviewer: Siebrand <siebr...@kitano.nl>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
