jenkins-bot has submitted this change and it was merged.
Change subject: Move UpdateRepoOnMoveJob into repo using JobSpecification in
client
......................................................................
Move UpdateRepoOnMoveJob into repo using JobSpecification in client
JobSpecification allows us to make repo run this job without having
to instantiate it on client.
Bug: 60400
Change-Id: Iee0efb55303e501d8cfc7617cd6c2cd75581305a
---
M client/WikibaseClient.hooks.php
M client/includes/UpdateRepo.php
M client/includes/UpdateRepoOnMove.php
M client/tests/phpunit/includes/UpdateRepoOnMoveTest.php
M lib/WikibaseLib.php
M lib/tests/phpunit/NoBadDependencyUsageTest.php
D lib/tests/phpunit/UpdateRepoOnMoveJobTest.php
M repo/Wikibase.php
R repo/includes/UpdateRepoOnMoveJob.php
A repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
10 files changed, 161 insertions(+), 198 deletions(-)
Approvals:
WikidataJenkins: Verified
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/WikibaseClient.hooks.php b/client/WikibaseClient.hooks.php
index 899b195..6d1e9bd 100644
--- a/client/WikibaseClient.hooks.php
+++ b/client/WikibaseClient.hooks.php
@@ -797,7 +797,7 @@
return true;
}
- $repoDB =$settings->getSetting( 'repoDatabase' );
+ $repoDB = $settings->getSetting( 'repoDatabase' );
$siteLinkLookup =
$wikibaseClient->getStore()->getSiteLinkTable();
$jobQueueGroup = JobQueueGroup::singleton( $repoDB );
diff --git a/client/includes/UpdateRepo.php b/client/includes/UpdateRepo.php
index df6f140..6e560d4 100644
--- a/client/includes/UpdateRepo.php
+++ b/client/includes/UpdateRepo.php
@@ -3,12 +3,13 @@
namespace Wikibase;
use CentralAuthUser;
-use Job;
+use IJobSpecification;
use JobQueueGroup;
use RuntimeException;
use Title;
use User;
-use Wikibase\DataModel\SimpleSiteLink;
+use Wikibase\DataModel\SiteLink;
+use JobSpecification;
/**
* Provides logic to update the repo after certain changes have been
@@ -47,13 +48,24 @@
protected $title;
/**
+ * @var EntityId|null|bool
+ */
+ private $entityId = false;
+
+ /**
* @param string $repoDB Database name of the repo
* @param SiteLinkLookup $siteLinkLookup
* @param User $user
* @param string $siteId Global id of the client wiki
* @param Title $title Title in the client that has been changed
*/
- public function __construct( $repoDB, $siteLinkLookup, $user, $siteId,
$title ) {
+ public function __construct(
+ $repoDB,
+ SiteLinkLookup $siteLinkLookup,
+ User $user,
+ $siteId,
+ Title $title
+ ) {
$this->repoDB = $repoDB;
$this->siteLinkLookup = $siteLinkLookup;
$this->user = $user;
@@ -67,12 +79,16 @@
* @return EntityId|null
*/
public function getEntityId() {
- return $this->siteLinkLookup->getEntityIdForSiteLink(
- new SimpleSiteLink(
- $this->siteId,
- $this->title->getFullText()
- )
- );
+ if ( $this->entityId === false ) {
+ $this->entityId =
$this->siteLinkLookup->getEntityIdForSiteLink(
+ new SiteLink(
+ $this->siteId,
+ $this->title->getFullText()
+ )
+ );
+ }
+
+ return $this->entityId;
}
/**
@@ -94,7 +110,6 @@
return false;
}
- // XXX: repoDatabase == CentralAuth site id?!!
if ( !$caUser->isAttached() || !$caUser->attachedOn(
$this->repoDB ) ) {
// Either the user account on this wiki or the one on
the repo do not exist
// or they aren't connected
@@ -131,7 +146,32 @@
/**
* Returns a new job for updating the repo.
*
- * @return Job
+ * @return IJobSpecification
*/
- abstract public function createJob();
+ public function createJob() {
+ wfProfileIn( __METHOD__ );
+
+ $job = new JobSpecification(
+ $this->getJobName(),
+ $this->getJobParameters()
+ );
+
+ wfProfileOut( __METHOD__ );
+
+ return $job;
+ }
+
+ /**
+ * Get the parameters for creating a new IJobSpecification
+ *
+ * @return array
+ */
+ abstract protected function getJobParameters();
+
+ /**
+ * Get the name of the Job that should be run on the repo
+ *
+ * @return string
+ */
+ abstract protected function getJobName();
}
diff --git a/client/includes/UpdateRepoOnMove.php
b/client/includes/UpdateRepoOnMove.php
index 9dc9fc0..a13e429 100644
--- a/client/includes/UpdateRepoOnMove.php
+++ b/client/includes/UpdateRepoOnMove.php
@@ -1,6 +1,8 @@
<?php
namespace Wikibase;
+use Title;
+use User;
/**
* Provides logic to update the repo after page moves in the client.
@@ -13,41 +15,50 @@
class UpdateRepoOnMove extends UpdateRepo {
/**
- * @var \Title
+ * @var Title
*/
protected $newTitle;
/**
* @param string $repoDB Database name of the repo
* @param SiteLinkLookup $siteLinkLookup
- * @param \User $user
+ * @param User $user
* @param string $siteId Global id of the client wiki
- * @param \Title $oldTitle
- * @param \Title $newTitle
+ * @param Title $oldTitle
+ * @param Title $newTitle
*/
- public function __construct( $repoDB, $siteLinkLookup, $user, $siteId,
$oldTitle, $newTitle ) {
+ public function __construct(
+ $repoDB,
+ SiteLinkLookup $siteLinkLookup,
+ User $user, $siteId,
+ Title $oldTitle,
+ Title $newTitle
+ ) {
parent::__construct( $repoDB, $siteLinkLookup, $user, $siteId,
$oldTitle );
$this->newTitle = $newTitle;
}
/**
- * Returns a new job for updating the repo.
+ * Get the name of the Job that should be run on the repo
*
- * @return \Job
+ * @return string
*/
- public function createJob() {
- wfProfileIn( __METHOD__ );
+ protected function getJobName() {
+ return 'UpdateRepoOnMove';
+ }
- $job = UpdateRepoOnMoveJob::newFromMove(
- $this->title,
- $this->newTitle,
- $this->getEntityId(),
- $this->user,
- $this->siteId
+ /**
+ * Get the parameters for creating a new JobSpecification
+ *
+ * @return array
+ */
+ protected function getJobParameters() {
+ return array(
+ 'siteId' => $this->siteId,
+ 'entityId' => $this->getEntityId()->getSerialization(),
+ 'oldTitle' => $this->title->getPrefixedText(),
+ 'newTitle' => $this->newTitle->getPrefixedText(),
+ 'user' => $this->user->getName()
);
-
- wfProfileOut( __METHOD__ );
-
- return $job;
}
}
diff --git a/client/tests/phpunit/includes/UpdateRepoOnMoveTest.php
b/client/tests/phpunit/includes/UpdateRepoOnMoveTest.php
index 951038f..100b2f0 100644
--- a/client/tests/phpunit/includes/UpdateRepoOnMoveTest.php
+++ b/client/tests/phpunit/includes/UpdateRepoOnMoveTest.php
@@ -4,7 +4,6 @@
use Wikibase\DataModel\Entity\ItemId;
use Wikibase\UpdateRepoOnMove;
-use Wikibase\Client\WikibaseClient;
/**
* @covers Wikibase\UpdateRepoOnMove
@@ -24,33 +23,22 @@
* @return array
*/
protected function getFakeMoveData() {
- static $ret = array();
+ $entityId = new ItemId( 'Q123' );
- if ( !$ret ) {
- $entityId = new ItemId( 'Q123' );
+ $siteLinkLookupMock = $this->getMock(
'\Wikibase\SiteLinkLookup' );
- $siteLinkLookupMock = $this->getMockBuilder(
'\Wikibase\SiteLinkLookup' )
- ->disableOriginalConstructor()
- ->getMock();
+ $siteLinkLookupMock->expects( $this->any() )
+ ->method( 'getEntityIdForSiteLink' )
+ ->will( $this->returnValue( $entityId ) );
- $siteLinkLookupMock->expects( $this->any() )
- ->method( 'getEntityIdForSiteLink' )
- ->will( $this->returnValue( $entityId ) );
-
- $siteId =
WikibaseClient::getDefaultInstance()->getSettings()
- ->getSetting( 'siteGlobalID' );
-
- $ret = array(
- 'repoDB' => wfWikiID(),
- 'siteLinkLookup' => $siteLinkLookupMock,
- 'user' => \User::newFromName(
'RandomUserWhichDoesntExist' ),
- 'siteId' => $siteId,
- 'oldTitle' => \Title::newFromText(
'ThisOneDoesntExist' ),
- 'newTitle' => \Title::newFromText( 'Bar' )
- );
- }
-
- return $ret;
+ return array(
+ 'repoDB' => 'wikidata',
+ 'siteLinkLookup' => $siteLinkLookupMock,
+ 'user' => \User::newFromName(
'RandomUserWhichDoesntExist' ),
+ 'siteId' => 'whatever',
+ 'oldTitle' => \Title::newFromText( 'ThisOneDoesntExist'
),
+ 'newTitle' => \Title::newFromText( 'Bar' )
+ );
}
/**
@@ -59,16 +47,22 @@
* @return UpdateRepoOnMove
*/
protected function getNewLocal() {
- $moveData = $this->getFakeMoveData();
+ static $updateRepo = null;
- $updateRepo = new UpdateRepoOnMove(
- $moveData['repoDB'],
- $moveData['siteLinkLookup'],
- $moveData['user'],
- $moveData['siteId'],
- $moveData['oldTitle'],
- $moveData['newTitle']
- );
+ if ( !$updateRepo ) {
+ $moveData = $this->getFakeMoveData();
+
+ $updateRepo = new UpdateRepoOnMove(
+ $moveData['repoDB'],
+ // Nobody knows why we need to clone over here,
but it's not working
+ // without... PHP is fun!
+ clone $moveData['siteLinkLookup'],
+ $moveData['user'],
+ $moveData['siteId'],
+ $moveData['oldTitle'],
+ $moveData['newTitle']
+ );
+ }
return $updateRepo;
}
@@ -106,9 +100,10 @@
public function testCreateJob() {
$updateRepo = $this->getNewLocal();
$job = $updateRepo->createJob();
+ $itemId = new ItemId( 'Q123' );
$moveData = $this->getFakeMoveData();
- $this->assertInstanceOf( 'Job', $job );
+ $this->assertInstanceOf( 'IJobSpecification', $job );
$this->assertEquals( 'UpdateRepoOnMove', $job->getType() );
$params = $job->getParams();
@@ -116,7 +111,7 @@
$this->assertEquals( $moveData['oldTitle'], $params['oldTitle']
);
$this->assertEquals( $moveData['newTitle'], $params['newTitle']
);
$this->assertEquals( $moveData['user'], $params['user'] );
- $this->assertTrue( array_key_exists( 'entityId', $params ) );
+ $this->assertEquals( $itemId->getSerialization(),
$params['entityId'] );
}
public function testInjectJob() {
diff --git a/lib/WikibaseLib.php b/lib/WikibaseLib.php
index e3b3581..8e5e43d 100644
--- a/lib/WikibaseLib.php
+++ b/lib/WikibaseLib.php
@@ -63,7 +63,6 @@
$wgExtensionMessagesFiles['WikibaseLib'] = __DIR__ .
'/WikibaseLib.i18n.php';
$wgJobClasses['ChangeNotification'] = 'Wikibase\ChangeNotificationJob';
- $wgJobClasses['UpdateRepoOnMove'] = 'Wikibase\UpdateRepoOnMoveJob';
// Hooks
$wgHooks['UnitTestsList'][]
= 'Wikibase\LibHooks::registerPhpUnitTests';
diff --git a/lib/tests/phpunit/NoBadDependencyUsageTest.php
b/lib/tests/phpunit/NoBadDependencyUsageTest.php
index 24ae283..10363e3 100644
--- a/lib/tests/phpunit/NoBadDependencyUsageTest.php
+++ b/lib/tests/phpunit/NoBadDependencyUsageTest.php
@@ -16,8 +16,8 @@
public function testNoRepoUsageInLib() {
// Increasing this allowance is forbidden
- $this->assertStringNotInLib( 'WikibaseRepo' . '::', 4 );
- $this->assertStringNotInLib( 'Wikibase\\Repo\\', 4 );
+ $this->assertStringNotInLib( 'WikibaseRepo' . '::', 3 );
+ $this->assertStringNotInLib( 'Wikibase\\Repo\\', 3 );
}
public function testNoClientUsageInLib() {
diff --git a/lib/tests/phpunit/UpdateRepoOnMoveJobTest.php
b/lib/tests/phpunit/UpdateRepoOnMoveJobTest.php
deleted file mode 100644
index 3b61957..0000000
--- a/lib/tests/phpunit/UpdateRepoOnMoveJobTest.php
+++ /dev/null
@@ -1,84 +0,0 @@
-<?php
-
-namespace Wikibase\Test;
-
-use Title;
-use User;
-use Wikibase\DataModel\Entity\ItemId;
-use Wikibase\UpdateRepoOnMoveJob;
-
-/**
- * @covers Wikibase\UpdateRepoOnMoveJob
- *
- * @group Wikibase
- * @group WikibaseLib
- *
- * @licence GNU GPL v2+
- * @author Marius Hoch < [email protected] >
- */
-class UpdateRepoOnMoveJobTest extends \MediaWikiTestCase {
-
- /**
- * @param array $moveData
- *
- * @return UpdateRepoOnMoveJob
- */
- public function getNewFromMove( $moveData ) {
- return UpdateRepoOnMoveJob::newFromMove(
- $moveData['oldTitle'],
- $moveData['newTitle'],
- $moveData['entityId'],
- $moveData['user'],
- $moveData['siteId']
- );
- }
-
- /**
- * @return array
- */
- public function getSampleData() {
- return array(
- 'oldTitle' => Title::newFromText( 'Foo' ),
- 'newTitle' => Title::newFromText( 'Bar' ),
- 'entityId' => new ItemId( 'q123' ),
- 'user' => User::newFromName(
'RandomUserWhichDoesntExist' ),
- 'siteId' => wfWikiID() // Doesn't really matter what we
use here
- );
- }
-
- /**
- * Checks the type and params of a job created from a move
- */
- public function testNewFromMove() {
- $moveData = $this->getSampleData();
- $job = $this->getNewFromMove( $moveData );
-
- $this->assertInstanceOf( 'Job', $job );
- $this->assertEquals( 'UpdateRepoOnMove', $job->getType() );
-
- $params = $job->getParams();
- $this->assertEquals( $moveData['siteId'], $params['siteId'] );
- $this->assertEquals( $moveData['entityId'], $params['entityId']
);
- $this->assertEquals( $moveData['oldTitle'], $params['oldTitle']
);
- $this->assertEquals( $moveData['newTitle'], $params['newTitle']
);
- $this->assertEquals( $moveData['user'], $params['user'] );
- }
-
- public function testGetSummary() {
- if ( !defined( 'WB_VERSION' ) ) {
- $this->markTestSkipped( 'Wikibase\Summary is only
available on repo' );
- }
- $moveData = $this->getSampleData();
- $job = $this->getNewFromMove( $moveData );
-
- $summary = $job->getSummary( 'SiteID', 'Test', 'MoarTest' );
-
- $this->assertEquals( 'clientsitelink-update',
$summary->getMessageKey() );
- $this->assertEquals( 'SiteID', $summary->getLanguageCode() );
- $this->assertEquals(
- array( 'SiteID:Test', 'SiteID:MoarTest' ),
- $summary->getCommentArgs()
- );
- }
-
-}
diff --git a/repo/Wikibase.php b/repo/Wikibase.php
index e04ea70..b9d4252 100644
--- a/repo/Wikibase.php
+++ b/repo/Wikibase.php
@@ -38,7 +38,7 @@
call_user_func( function() {
global $wgExtensionCredits, $wgGroupPermissions,
$wgExtensionMessagesFiles;
global $wgAPIModules, $wgSpecialPages, $wgSpecialPageGroups, $wgHooks,
$wgContentHandlers;
- global $wgWBStores, $wgWBRepoSettings, $wgResourceModules,
$wgValueParsers;
+ global $wgWBStores, $wgWBRepoSettings, $wgResourceModules,
$wgValueParsers, $wgJobClasses;
$wgExtensionCredits['wikibase'][] = array(
'path' => __DIR__,
@@ -149,6 +149,9 @@
$wgSpecialPageGroups['MyLanguageFallbackChain'] =
'wikibaserepo';
$wgSpecialPageGroups['MergeItems']
= 'wikibaserepo';
+ // Jobs
+ $wgJobClasses['UpdateRepoOnMove'] = 'Wikibase\UpdateRepoOnMoveJob';
+
// Hooks
$wgHooks['BeforePageDisplay'][]
= 'Wikibase\RepoHooks::onBeforePageDisplay';
$wgHooks['LoadExtensionSchemaUpdates'][] =
'Wikibase\RepoHooks::onSchemaUpdate';
diff --git a/lib/includes/UpdateRepoOnMoveJob.php
b/repo/includes/UpdateRepoOnMoveJob.php
similarity index 79%
rename from lib/includes/UpdateRepoOnMoveJob.php
rename to repo/includes/UpdateRepoOnMoveJob.php
index 7881960..e967566 100644
--- a/lib/includes/UpdateRepoOnMoveJob.php
+++ b/repo/includes/UpdateRepoOnMoveJob.php
@@ -41,39 +41,6 @@
}
/**
- * Creates a UpdateRepoOnMoveJob representing the given move.
- *
- * @param \Title $oldTitle
- * @param \Title $newTitle
- * @param EntityId entityId
- * @param \User $user User who moved the page
- * @param string $globalId Global id of the site from which the is
coming
- * @param array|bool $params extra job parameters, see Job::__construct
(default: false).
- *
- * @return \Wikibase\UpdateRepoOnMoveJob: the job
- */
- public static function newFromMove( $oldTitle, $newTitle, $entityId,
$user, $globalId, $params = false ) {
- wfProfileIn( __METHOD__ );
-
- if ( $params === false ) {
- $params = array();
- }
-
- $params['siteId'] = $globalId;
- $params['entityId'] = $entityId;
- $params['oldTitle'] = $oldTitle->getPrefixedText();
- $params['newTitle'] = $newTitle->getPrefixedText();
- $params['user'] = $user->getName();
-
- // The Title object isn't really being used but \Job demands
it... so we just insert something
- // A Title belonging to the entity on the repo would be more
sane, but it doesn't really matter
- $job = new self( $newTitle, $params );
-
- wfProfileOut( __METHOD__ );
- return $job;
- }
-
- /**
* @return EntityContentFactory
*/
protected function getEntityContentFactory() {
@@ -161,20 +128,21 @@
* Update the siteLink on the repo to reflect the change in the client
*
* @param string $siteId Id of the client the change comes from
- * @param EntityId $entityId
+ * @param string $itemId
* @param string $oldPage
* @param string $newPage
* @param \User $user User who we'll attribute the update to
*
* @return bool Whether something changed
*/
- public function updateSiteLink( $siteId, $entityId, $oldPage, $newPage,
$user ) {
+ public function updateSiteLink( $siteId, $itemId, $oldPage, $newPage,
$user ) {
wfProfileIn( __METHOD__ );
- $item = $this->getEntityRevisionLookup()->getEntity( $entityId
);
+ $itemId = new ItemId( $itemId );
+ $item = $this->getEntityRevisionLookup()->getEntity( $itemId );
if ( !$item ) {
// The entity assigned with the moved page can't be
found
- wfDebugLog( __CLASS__, __FUNCTION__ . ": entity with id
" . $entityId->getPrefixedId() . " not found" );
+ wfDebugLog( __CLASS__, __FUNCTION__ . ": entity with id
" . $itemId->getPrefixedId() . " not found" );
wfProfileOut( __METHOD__ );
return false;
}
@@ -269,12 +237,11 @@
return true;
}
- if ( !( $params['entityId'] instanceof ItemId ) ) {
- // Forward compatibility as this is going to be called
with the serialized
- // item id, instead of a real object.
- // This is a short term fix to make this work side by
side with the new
+ if ( $params['entityId'] instanceof ItemId ) {
+ // Backward compatibility switch.
+ // This is a short term fix to make this work side by
side with the old
// version of the job.
- $params['entityId'] = new ItemId( $params['entityId'] );
+ $params['entityId'] =
$params['entityId']->getSerialization();
}
$this->updateSiteLink(
diff --git a/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
b/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
new file mode 100644
index 0000000..b794c79
--- /dev/null
+++ b/repo/tests/phpunit/includes/UpdateRepoOnMoveJobTest.php
@@ -0,0 +1,32 @@
+<?php
+
+namespace Wikibase\Test;
+
+use Title;
+use Wikibase\UpdateRepoOnMoveJob;
+
+/**
+ * @covers Wikibase\UpdateRepoOnMoveJob
+ *
+ * @group Wikibase
+ * @group WikibaseLib
+ *
+ * @licence GNU GPL v2+
+ * @author Marius Hoch < [email protected] >
+ */
+class UpdateRepoOnMoveJobTest extends \MediaWikiTestCase {
+
+ public function testGetSummary() {
+ $job = new UpdateRepoOnMoveJob( Title::newMainPage() );
+
+ $summary = $job->getSummary( 'SiteID', 'Test', 'MoarTest' );
+
+ $this->assertEquals( 'clientsitelink-update',
$summary->getMessageKey() );
+ $this->assertEquals( 'SiteID', $summary->getLanguageCode() );
+ $this->assertEquals(
+ array( 'SiteID:Test', 'SiteID:MoarTest' ),
+ $summary->getCommentArgs()
+ );
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/119306
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Iee0efb55303e501d8cfc7617cd6c2cd75581305a
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Aude <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Hoo man <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: WikidataJenkins <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
