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 <daniel.kinz...@wikimedia.de> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits