Daniel Kinzler has uploaded a new change for review.
https://gerrit.wikimedia.org/r/203049
Change subject: Do not skip non-item changes in ChangeDispatcher
......................................................................
Do not skip non-item changes in ChangeDispatcher
Change-Id: I037f9bb5335a1efbc280a70e3b7044be5de8ee89
---
M repo/includes/ChangeDispatcher.php
M repo/tests/phpunit/includes/ChangeDispatcherTest.php
2 files changed, 38 insertions(+), 34 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/49/203049/1
diff --git a/repo/includes/ChangeDispatcher.php
b/repo/includes/ChangeDispatcher.php
index 8067e5b..4113164 100644
--- a/repo/includes/ChangeDispatcher.php
+++ b/repo/includes/ChangeDispatcher.php
@@ -5,7 +5,7 @@
use Wikibase\Change;
use Wikibase\ChunkAccess;
use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\EntityChange;
use Wikibase\ItemChange;
use Wikibase\Lib\Reporting\ExceptionHandler;
use Wikibase\Lib\Reporting\LogWarningExceptionHandler;
@@ -338,8 +338,8 @@
/**
* Filters a list of changes, removing changes not relevant to the
given client wiki.
*
- * Currently, we only keep ItemChanges for items that have a sitelink
to the
- * target client wiki.
+ * Currently, we keep EntityChanges for entities the client wiki is
subscribed to, or
+ * that modify a sitelink to the client wiki.
*
* @param string $siteID : The client wiki's global site identifier,
as used by sitelinks.
* @param Change[] $changes: The list of changes to filter.
@@ -351,40 +351,43 @@
*/
private function filterChanges( $siteID, $changes, $limit ) {
// collect all item IDs mentioned in the changes
- $itemSet = array();
+ $entitySet = array();
foreach ( $changes as $change ) {
- if ( $change instanceof ItemChange ) {
- $id = $change->getEntityId();
- $itemId = $id->getNumericId();
- $itemSet[$itemId] = $id;
+ if ( !( $change instanceof EntityChange ) ) {
+ continue;
}
+
+ $id = $change->getEntityId();
+ $idString = $id->getSerialization();
+ $entitySet[$idString] = $id;
}
- $this->trace( "Checking sitelinks to $siteID for " . count(
$itemSet ) . " items." );
+ $this->trace( "Checking sitelinks to $siteID for " . count(
$entitySet ) . " entities." );
- $linkedItems = $this->subscriptionLookup->getSubscriptions(
$siteID, $itemSet );
- $linkedItems = $this->reIndexEntityIds( $linkedItems );
+ $subscribedEntities =
$this->subscriptionLookup->getSubscriptions( $siteID, $entitySet );
+ $subscribedEntities = $this->reIndexEntityIds(
$subscribedEntities );
- $this->trace( "Retaining changes for " . count( $linkedItems )
. " relevant items." );
+ $this->trace( "Retaining changes for " . count(
$subscribedEntities ) . " relevant entities." );
// find all changes that relate to an item that has a sitelink
to $siteID.
$filteredChanges = array();
$numberOfChangesFound = 0;
$lastIdSeen = 0;
foreach ( $changes as $change ) {
+ if ( !( $change instanceof EntityChange ) ) {
+ continue;
+ }
+
$lastIdSeen = $change->getId();
+ $idString = $change->getEntityId()->getSerialization();
- if ( $change instanceof ItemChange) {
- $itemId =
$change->getEntityId()->getNumericId();
+ // The change is relevant if it alters any sitelinks
referring to $siteID,
+ // or the item currently links to $siteID.
+ if ( isset( $subscribedEntities[$idString] )
+ || $this->isRelevantChange( $change, $siteID )
) {
- // The change is relevant if it alters any
sitelinks referring to $siteID,
- // or the item currently links to $siteID.
- if ( isset( $linkedItems[$itemId] )
- || $this->isRelevantChange( $change,
$siteID ) ) {
-
- $filteredChanges[] = $change;
- $numberOfChangesFound++;
- }
+ $filteredChanges[] = $change;
+ $numberOfChangesFound++;
}
if ( $numberOfChangesFound >= $limit ) {
@@ -400,16 +403,14 @@
/**
* @param EntityId[] $entityIds
*
- * @return ItemId[] The ItemIds from EntityId[], keyed by numeric id.
+ * @return EntityId[] $entityIds re-keyed by id string.
*/
private function reIndexEntityIds( array $entityIds ) {
$reindexed = array();
foreach ( $entityIds as $id ) {
- if ( $id instanceof ItemId ) {
- $key = $id->getNumericId();
- $reindexed[$key] = $id;
- }
+ $key = $id->getSerialization();
+ $reindexed[$key] = $id;
}
return $reindexed;
diff --git a/repo/tests/phpunit/includes/ChangeDispatcherTest.php
b/repo/tests/phpunit/includes/ChangeDispatcherTest.php
index f8fdfda..5c8fb57 100644
--- a/repo/tests/phpunit/includes/ChangeDispatcherTest.php
+++ b/repo/tests/phpunit/includes/ChangeDispatcherTest.php
@@ -7,11 +7,12 @@
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
-use Diff\Tests\DiffOp\DiffOpAddTest;
use Wikibase\Change;
use Wikibase\ChunkAccess;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\Item;
use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\Entity\PropertyId;
use Wikibase\Repo\ChangeDispatcher;
use Wikibase\Repo\Notifications\ChangeNotificationSender;
use Wikibase\Store\ChangeDispatchCoordinator;
@@ -138,8 +139,8 @@
return array(
// index 0 is ignored, or used as the base change.
$this->newChange( 0, new ItemId( 'Q99999' ), sprintf(
'201403030100', 0 ) ),
- $this->newChange( ++$changeId, new ItemId( 'Q11' ),
sprintf( '2014030301%02d', $changeId ) ),
- $this->newChange( ++$changeId, new ItemId( 'Q11' ),
sprintf( '2014030301%02d', $changeId ) ),
+ $this->newChange( ++$changeId, new PropertyId( 'P11' ),
sprintf( '2014030301%02d', $changeId ) ),
+ $this->newChange( ++$changeId, new PropertyId( 'P11' ),
sprintf( '2014030301%02d', $changeId ) ),
$this->newChange( ++$changeId, new ItemId( 'Q22' ),
sprintf( '2014030301%02d', $changeId ) ),
$this->newChange( ++$changeId, new ItemId( 'Q22' ),
sprintf( '2014030301%02d', $changeId ) ),
$this->newChange( ++$changeId, new ItemId( 'Q33' ),
sprintf( '2014030301%02d', $changeId ) , $addEn ),
@@ -151,7 +152,7 @@
public function setUp() {
$this->subscriptions['enwiki'] = array(
- new ItemId( 'Q11' ),
+ new PropertyId( 'P11' ),
new ItemId( 'Q22' ),
// changes to Q33 are relevant because they affect
enwiki
);
@@ -173,8 +174,10 @@
* @return Change
*/
private function newChange( $changeId, EntityId $entityId, $time, Diff
$siteLinkDiff = null ) {
- //FIXME: test non-items too!
- $change = $this->getMockBuilder( 'Wikibase\ItemChange' )
+ $changeClass = ( $entityId->getEntityType() ===
Item::ENTITY_TYPE )
+ ? 'Wikibase\ItemChange' : 'Wikibase\EntityChange';
+
+ $change = $this->getMockBuilder( $changeClass )
->disableOriginalConstructor()
->getMock();
@@ -202,7 +205,7 @@
$change->expects( $this->any() )
->method( 'getObjectId' )
- ->will( $this->returnValue( $entityId->getNumericId()
) );
+ ->will( $this->returnValue(
$entityId->getSerialization() ) );
$change->expects( $this->any() )
->method( 'getEntityId' )
--
To view, visit https://gerrit.wikimedia.org/r/203049
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I037f9bb5335a1efbc280a70e3b7044be5de8ee89
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Daniel Kinzler <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
