Hoo man has uploaded a new change for review.
https://gerrit.wikimedia.org/r/314016
Change subject: Move ChangeNotificationJob from lib to client
......................................................................
Move ChangeNotificationJob from lib to client
This was not possible back when the job was introduced,
but certainly is now.
Manually tested:
* Jobs created and run with the new code.
* Jobs created with the new code and run with the old code.
* Jobs created with the old code and run with the new code.
Change-Id: I43314602419012d0a261defa39602fe02f61169b
---
M client/WikibaseClient.php
R client/includes/ChangeNotificationJob.php
R client/tests/phpunit/includes/ChangeNotificationJobTest.php
M lib/WikibaseLib.php
M lib/tests/phpunit/NoBadDependencyUsageTest.php
M repo/includes/Notifications/JobQueueChangeNotificationSender.php
M
repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
7 files changed, 67 insertions(+), 65 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/16/314016/1
diff --git a/client/WikibaseClient.php b/client/WikibaseClient.php
index 7b34a44..ad37e9b 100644
--- a/client/WikibaseClient.php
+++ b/client/WikibaseClient.php
@@ -162,6 +162,7 @@
// job classes
$wgJobClasses['wikibase-addUsagesForPage'] =
Wikibase\Client\Store\AddUsagesForPageJob::class;
+ $wgJobClasses['ChangeNotification'] =
Wikibase\ChangeNotificationJob::class;
// api modules
$wgAPIMetaModules['wikibase'] = array(
diff --git a/lib/includes/ChangeNotificationJob.php
b/client/includes/ChangeNotificationJob.php
similarity index 71%
rename from lib/includes/ChangeNotificationJob.php
rename to client/includes/ChangeNotificationJob.php
index 19512a4..76b947f 100644
--- a/lib/includes/ChangeNotificationJob.php
+++ b/client/includes/ChangeNotificationJob.php
@@ -6,6 +6,7 @@
use Title;
use Wikibase\Client\Changes\ChangeHandler;
use Wikibase\Client\WikibaseClient;
+use Wikimedia\Assert\Assert;
/**
* Job for notifying a client wiki of a batch of changes on the repository.
@@ -28,46 +29,7 @@
private $changeHandler = null;
/**
- * Creates a ChangeNotificationJob representing the given changes.
- *
- * @param Change[] $changes The list of changes to be processed
- * @param string $repo The name of the repository the changes come
from (default: "").
- * @param array|bool $params extra job parameters, see
Job::__construct (default: false).
- *
- * @return self
- */
- public static function newFromChanges( array $changes, $repo = '',
$params = false ) {
- static $dummyTitle = null;
-
- // Note: we don't really care about the title and will use a
dummy
- if ( $dummyTitle === null ) {
- // The Job class wants a Title object for some reason.
Supply a dummy.
- $dummyTitle = Title::newFromText(
"ChangeNotificationJob", NS_SPECIAL );
- }
-
- // get the list of change IDs
- $changeIds = array_map(
- function ( Change $change ) {
- return $change->getId();
- },
- $changes
- );
-
- if ( $params === false ) {
- $params = array();
- }
-
- $params['repo'] = $repo;
- $params['changeIds'] = $changeIds;
-
- return new self( $dummyTitle, $params );
- }
-
- /**
* Constructs a ChangeNotificationJob representing the changes given by
$changeIds.
- *
- * @note: This is for use by Job::factory, don't call it directly;
- * use newFromChanges() instead.
*
* @note: the constructor's signature is dictated by Job::factory, so
we'll have to
* live with it even though it's rather ugly for our use case.
@@ -75,10 +37,23 @@
* @see Job::factory.
*
* @param Title $title
- * @param array|bool $params
+ * @param array|bool $params Needs to have two keys: "repo": the id of
the repository,
+ * "changeIds": array of change ids.
*/
public function __construct( Title $title, $params = false ) {
parent::__construct( 'ChangeNotification', $title, $params );
+
+ Assert::parameterType( 'array', $params, '$params' );
+ Assert::parameter(
+ isset( $params['repo'] ),
+ '$params',
+ '$params[\'repo\'] not set.'
+ );
+ Assert::parameter(
+ isset( $params['changeIds'] ) && is_array(
$params['changeIds'] ),
+ '$params',
+ '$params[\'changeIds\'] not set or not an array.'
+ );
}
/**
diff --git a/lib/tests/phpunit/ChangeNotificationJobTest.php
b/client/tests/phpunit/includes/ChangeNotificationJobTest.php
similarity index 61%
rename from lib/tests/phpunit/ChangeNotificationJobTest.php
rename to client/tests/phpunit/includes/ChangeNotificationJobTest.php
index 842ba02..1574660 100644
--- a/lib/tests/phpunit/ChangeNotificationJobTest.php
+++ b/client/tests/phpunit/includes/ChangeNotificationJobTest.php
@@ -2,7 +2,8 @@
namespace Wikibase\Test;
-use Wikibase\Change;
+use MediaWikiTestCase;
+use Title;
use Wikibase\ChangeNotificationJob;
/**
@@ -14,34 +15,35 @@
*
* @license GPL-2.0+
* @author Daniel Kinzler
+ * @author Marius Hoch
*/
-class ChangeNotificationJobTest extends \MediaWikiTestCase {
+class ChangeNotificationJobTest extends MediaWikiTestCase {
// TODO: testNewFromChanges
// TODO: testGetChanges
// TODO: testRun
public function provideToString() {
- return array(
- array( // #0: empty
- array(),
+ return [
+ 'empty' => [
+ [],
'/^ChangeNotification.*/'
- ),
- array( // #1: some changes
- array(
- $this->getMock( Change::class ),
- $this->getMock( Change::class ),
- ),
+ ],
+ 'some changes' => [
+ [ 5, 37 ],
'/^ChangeNotification/'
- ),
- );
+ ],
+ ];
}
/**
* @dataProvider provideToString
*/
- public function testToString( $changes, $regex ) {
- $job = ChangeNotificationJob::newFromChanges( $changes );
+ public function testToString( $changeIds, $regex ) {
+ $job = new ChangeNotificationJob(
+ Title::newMainPage(),
+ [ 'repo' => 'repo-db', 'changeIds' => $changeIds ]
+ );
// toString used to fail on some platforms if a job contained a
non-primitive parameter.
$s = $job->toString();
diff --git a/lib/WikibaseLib.php b/lib/WikibaseLib.php
index 41d6444..68a6573 100644
--- a/lib/WikibaseLib.php
+++ b/lib/WikibaseLib.php
@@ -73,8 +73,6 @@
// i18n
$wgMessagesDirs['WikibaseLib'] = __DIR__ . '/i18n';
- $wgJobClasses['ChangeNotification'] = 'Wikibase\ChangeNotificationJob';
-
// Hooks
$wgHooks['UnitTestsList'][] = 'Wikibase\LibHooks::registerPhpUnitTests';
$wgHooks['ResourceLoaderTestModules'][] =
'Wikibase\LibHooks::registerQUnitTests';
diff --git a/lib/tests/phpunit/NoBadDependencyUsageTest.php
b/lib/tests/phpunit/NoBadDependencyUsageTest.php
index 51cb0a2..d00de3d 100644
--- a/lib/tests/phpunit/NoBadDependencyUsageTest.php
+++ b/lib/tests/phpunit/NoBadDependencyUsageTest.php
@@ -24,8 +24,8 @@
public function testNoClientUsageInLib() {
// Increasing this allowance is forbidden
- $this->assertStringNotInLib( 'WikibaseClient' . '::', 3 );
- $this->assertStringNotInLib( 'Wikibase\\Client\\', 3 );
+ $this->assertStringNotInLib( 'WikibaseClient' . '::', 2 );
+ $this->assertStringNotInLib( 'Wikibase\\Client\\', 1 );
}
private function assertStringNotInLib( $string, $maxAllowance ) {
diff --git a/repo/includes/Notifications/JobQueueChangeNotificationSender.php
b/repo/includes/Notifications/JobQueueChangeNotificationSender.php
index b053e2f..a8fd76f 100644
--- a/repo/includes/Notifications/JobQueueChangeNotificationSender.php
+++ b/repo/includes/Notifications/JobQueueChangeNotificationSender.php
@@ -2,8 +2,8 @@
namespace Wikibase\Repo\Notifications;
+use JobSpecification;
use Wikibase\Change;
-use Wikibase\ChangeNotificationJob;
/**
* ChangeNotificationSender based on a JobQueueGroup and ChangeNotificationJob.
@@ -17,7 +17,7 @@
class JobQueueChangeNotificationSender implements ChangeNotificationSender {
/**
- * @var string
+ * @var string|bool
*/
private $repoDB;
@@ -37,7 +37,7 @@
private $jobQueueGroupFactory;
/**
- * @param string $repoDB
+ * @param string|bool $repoDB
* @param string[] $wikiDBNames An associative array mapping site IDs
to logical database names.
* @param int $batchSize Number of changes to push per job.
* @param callable|null $jobQueueGroupFactory Function that returns a
JobQueueGroup for a given wiki.
@@ -77,7 +77,7 @@
$jobs = [];
foreach ( $chunks as $chunk ) {
- $jobs[] = ChangeNotificationJob::newFromChanges(
$chunk, $this->repoDB );
+ $jobs[] = $this->getJobSpecification( $chunk );
}
$qgroup->push( $jobs );
@@ -88,4 +88,23 @@
);
}
+ private function getJobSpecification( array $changes ) {
+ $changeIds = array_map(
+ function ( Change $change ) {
+ return $change->getId();
+ },
+ $changes
+ );
+
+ $params = [
+ 'repo' => $this->repoDB,
+ 'changeIds' => $changeIds
+ ];
+
+ return new JobSpecification(
+ 'ChangeNotification',
+ $params
+ );
+ }
+
}
diff --git
a/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
b/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
index 74828e3..643afff 100644
---
a/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
+++
b/repo/tests/phpunit/includes/Notifications/JobQueueChangeNotificationSenderTest.php
@@ -3,9 +3,9 @@
namespace Wikibase\Repo\Tests\Notifications;
use JobQueueGroup;
+use JobSpecification;
use PHPUnit_Framework_TestCase;
use Wikibase\Change;
-use Wikibase\ChangeNotificationJob;
use Wikibase\Repo\Notifications\JobQueueChangeNotificationSender;
/**
@@ -36,9 +36,16 @@
function( array $jobs ) use ( $expectedChunks )
{
$this->assertCount( $expectedChunks,
$jobs );
$this->assertContainsOnlyInstancesOf(
- ChangeNotificationJob::class,
+ JobSpecification::class,
$jobs
);
+
+ foreach ( $jobs as $job ) {
+ $params = $job->getParams();
+
+ $this->assertSame( 'repo-db',
$params['repo'] );
+ $this->assertContainsOnly(
'int', $params['changeIds'] );
+ }
} )
);
--
To view, visit https://gerrit.wikimedia.org/r/314016
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I43314602419012d0a261defa39602fe02f61169b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Hoo man <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
