Daniel Kinzler has uploaded a new change for review. https://gerrit.wikimedia.org/r/170961
Change subject: Determin update actions based on usage aspects. ...................................................................... Determin update actions based on usage aspects. This greatly simplifies test cases for ChangeHandler. Change-Id: I4ef7a9315b110caf29ae43c30c582fe6623e2581 --- M client/includes/AffectedPagesFinder.php M client/includes/ChangeHandler.php M client/includes/PageUpdater.php M client/includes/Usage/EntityUsage.php M client/includes/WikiPageUpdater.php M client/includes/WikibaseClient.php M client/tests/phpunit/MockPageUpdater.php M client/tests/phpunit/includes/ChangeHandlerTest.php 8 files changed, 336 insertions(+), 425 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase refs/changes/61/170961/1 diff --git a/client/includes/AffectedPagesFinder.php b/client/includes/AffectedPagesFinder.php index 0a174f5..146436f 100644 --- a/client/includes/AffectedPagesFinder.php +++ b/client/includes/AffectedPagesFinder.php @@ -103,7 +103,7 @@ /** * @since 0.5 * - * @param Change $change + * @param EntityChange $change * * @return Iterator<PageEntityUsages> */ diff --git a/client/includes/ChangeHandler.php b/client/includes/ChangeHandler.php index c2f192d..95cc27b 100644 --- a/client/includes/ChangeHandler.php +++ b/client/includes/ChangeHandler.php @@ -2,16 +2,16 @@ namespace Wikibase; +use Exception; use InvalidArgumentException; use MWException; use Title; use Wikibase\Client\Changes\AffectedPagesFinder; use Wikibase\Client\Store\TitleFactory; -use Wikibase\DataModel\Entity\Diff\EntityDiff; -use Wikibase\DataModel\Entity\Diff\ItemDiff; +use Wikibase\Client\Usage\EntityUsage; +use Wikibase\Client\Usage\PageEntityUsages; use Wikibase\Lib\Changes\EntityChangeFactory; use Wikibase\Lib\Store\EntityRevisionLookup; -use Wikibase\Lib\Store\StorageException; /** * Interface for change handling. Whenever a change is detected, @@ -31,28 +31,28 @@ /** * The change requites any rendered version of the page to be purged from the parser cache. */ - const PARSER_PURGE_ACTION = 1; + const PARSER_PURGE_ACTION = 'parser'; /** * The change requites a LinksUpdate job to be scheduled to update any links * associated with the page. */ - const LINKS_UPDATE_ACTION = 2; + const LINKS_UPDATE_ACTION = 'links'; /** * The change requites any HTML output generated from the page to be purged from web cached. */ - const WEB_PURGE_ACTION = 4; + const WEB_PURGE_ACTION = 'web'; /** * The change requites an entry to be injected into the recentchanges table. */ - const RC_ENTRY_ACTION = 8; + const RC_ENTRY_ACTION = 'rc'; /** * The change requites an entry to be injected into the revision table. */ - const HISTORY_ENTRY_ACTION = 16; + const HISTORY_ENTRY_ACTION = 'history'; /** * @var EntityChangeFactory @@ -91,8 +91,7 @@ PageUpdater $updater, EntityRevisionLookup $entityRevisionLookup, $localSiteId, - $injectRC, - $allowDataTransclusion + $injectRC ) { $this->changeFactory = $changeFactory; $this->affectedPagesFinder = $affectedPagesFinder; @@ -108,13 +107,8 @@ throw new InvalidArgumentException( '$injectRC must be a bool' ); } - if ( !is_bool( $allowDataTransclusion ) ) { - throw new InvalidArgumentException( '$allowDataTransclusion must be a bool' ); - } - $this->localSiteId = $localSiteId; $this->injectRC = (bool)$injectRC; - $this->dataTransclusionAllowed = $allowDataTransclusion; $this->mirrorUpdater = null; } @@ -419,7 +413,7 @@ /** * Main entry point for handling changes * - * @see https://www.mediawiki.org/wiki/Manual:Hooks/WikibasePollHandle + * @todo: process multiple changes at once! * * @since 0.1 * @@ -436,120 +430,150 @@ wfDebugLog( __CLASS__, __FUNCTION__ . ": handling change #$chid" . " (" . $change->getType() . ")" ); - //TODO: Actions may be per-title, depending on how the change applies to that page. - // We'll need on list of titles per action. - $actions = $this->getActions( $change ); + $usagesPerPage = $this->affectedPagesFinder->getPagesToUpdate( $change ); - if ( $actions === 0 ) { - // nothing to do - wfDebugLog( __CLASS__, __FUNCTION__ . ": No actions to take for change #$chid." ); - wfProfileOut( __METHOD__ ); - return false; - } - - if ( $this->mirrorUpdater !== null && ( $change instanceof EntityChange ) ) { - // keep local mirror up to date - $this->mirrorUpdater->handleChange( $change ); - } - - $titlesToUpdate = $this->getPagesToUpdate( $change ); - - if ( empty( $titlesToUpdate ) ) { + if ( empty( $usagesPerPage ) ) { // nothing to do wfDebugLog( __CLASS__, __FUNCTION__ . ": No pages to update for change #$chid." ); wfProfileOut( __METHOD__ ); return false; } - wfDebugLog( __CLASS__, __FUNCTION__ . ": updating " . count( $titlesToUpdate ) - . " pages (actions: " . dechex( $actions ). ") for change #$chid." ); + wfDebugLog( __CLASS__, __FUNCTION__ . ": updating " . count( $usagesPerPage ) + . " page(s) for change #$chid." ); - $this->updatePages( $change, $actions, $titlesToUpdate ); + $actionBuckets = array(); + + /** @var PageEntityUsages $usages */ + foreach ( $usagesPerPage as $usages ) { + $actions = $this->getUpdateActions( $usages->getAspects() ); + $this->updateActionBuckets( $actionBuckets, $usages->getPageId(), $actions ); + } + + foreach ( $actionBuckets as $action => $bucket ) { + $this->applyUpdateAction( $action, $bucket, $change ); + } wfProfileOut( __METHOD__ ); return true; } /** - * Returns the pages that need some kind of updating given the change. + * @param string[] $aspects * - * @param Change $change - * - * @return Title[] the titles of the pages to update + * @return string[] A list of actions, as defined by the self::XXXX_ACTION constants. */ - public function getPagesToUpdate( Change $change ) { + public function getUpdateActions( $aspects ) { wfProfileIn( __METHOD__ ); - $usages = $this->affectedPagesFinder->getPagesToUpdate( $change ); - $pagesToUpdate = $this->getTitlesFromPageEntityUsages( $usages ); + $i = 0; + $actions = array(); + $aspects = array_flip( $aspects ); - wfProfileOut( __METHOD__ ); + $all = isset( $aspects[EntityUsage::ALL_USAGE] ); - return $pagesToUpdate; + if ( isset( $aspects[EntityUsage::SITELINK_USAGE] ) || $all ) { + // Link updates might be optimized to bypass parsing + $actions[self::LINKS_UPDATE_ACTION] = ++$i; + } + + if ( isset( $aspects[EntityUsage::LABEL_USAGE] ) || $all ) { + $actions[self::PARSER_PURGE_ACTION] = ++$i; + } + + if ( isset( $aspects[EntityUsage::TITLE_USAGE] ) || $all ) { + $actions[self::PARSER_PURGE_ACTION] = ++$i; + } + + if ( isset( $aspects[EntityUsage::OTHER_USAGE] ) || $all ) { + $actions[self::PARSER_PURGE_ACTION] = ++$i; + } + + // Purge caches and inject log entries if we have reason + // to update the cached ParserOutput object in some way. + if ( isset( $actions[self::PARSER_PURGE_ACTION] ) || isset( $actions[self::LINKS_UPDATE_ACTION] ) ) { + $actions[self::WEB_PURGE_ACTION] = ++$i; + $actions[self::RC_ENTRY_ACTION] = ++$i; + $actions[self::HISTORY_ENTRY_ACTION] = ++$i; + } + + // If we purge the parser cache, the links update is redundant. + if ( isset( $actions[self::PARSER_PURGE_ACTION] ) ) { + unset( $actions[self::LINKS_UPDATE_ACTION] ); + } + + return array_flip( $actions ); } /** - * @param PageEntityUsages[]|Iterator<PageEntityUsages> $pageIds + * @param array[] &$buckets Map of action names to lists of page IDs. To be updated. + * @param int $pageId The page ID + * @param string[] $actions Actions to perform on the page + */ + private function updateActionBuckets( &$buckets, $pageId, $actions ) { + foreach ( $actions as $action ) { + $buckets[$action][] = $pageId; + } + } + + /** + * @param string $action + * @param int[] $pageIds + * @param EntityChange $change + */ + private function applyUpdateAction( $action, array $pageIds, EntityChange $change ) { + wfProfileIn( __METHOD__ ); + + $titlesToUpdate = $this->getTitlesForPageIds( $pageIds ); + + if ( $action === self::PARSER_PURGE_ACTION ) { + $this->updater->purgeParserCache( $titlesToUpdate ); + } + + if ( $action === self::WEB_PURGE_ACTION ) { + $this->updater->purgeWebCache( $titlesToUpdate ); + } + + if ( $action === self::LINKS_UPDATE_ACTION ) { + $this->updater->scheduleRefreshLinks( $titlesToUpdate ); + } + + if ( $this->injectRC && ( $action === self::RC_ENTRY_ACTION ) > 0 ) { + $rcAttribs = $this->getRCAttributes( $change ); + + if ( $rcAttribs !== false ) { + //FIXME: The same change may be reported to several target pages; + // The comment we generate should be adapted to the role that page + // plays in the change, e.g. when a sitelink changes from one page to another, + // the link was effectively removed from one and added to the other page. + $this->updater->injectRCRecords( $titlesToUpdate, $rcAttribs ); + } + } + + //TODO: handling for self::HISTORY_ENTRY_ACTION goes here. + // should probably be $this->updater->injectHistoryRecords() or some such. + + wfProfileOut( __METHOD__ ); + } + + /** + * @param int[] $pageIds * * @return Title[] */ - private function getTitlesFromPageEntityUsages( $usages ) { + private function getTitlesForPageIds( $pageIds ) { $titles = array(); - foreach ( $usages as $pageEntityUsages ) { + foreach ( $pageIds as $id ) { try { - $pid = $pageEntityUsages->getPageId(); - $titles[] = $this->titleFactory->newFromID( $pid ); - } catch ( StorageException $ex ) { - // Page probably got deleted just now. Skip it. + $title = $this->titleFactory->newFromID( $id ); + $titles[] = $title; + } catch ( Exception $ex ) { + // never mind } } return $titles; - } - - /** - * Main entry point for handling changes - * - * @since 0.4 - * - * @param Change $change the change to apply to the pages - * @param int $actions a bit field of actions to take, as returned by getActions() - * @param Title[] $titlesToUpdate the pages to update - */ - public function updatePages( Change $change, $actions, array $titlesToUpdate ) { - wfProfileIn( __METHOD__ ); - - if ( ( $actions & self::PARSER_PURGE_ACTION ) > 0 ) { - $this->updater->purgeParserCache( $titlesToUpdate ); - } - - if ( ( $actions & self::WEB_PURGE_ACTION ) > 0 ) { - $this->updater->purgeWebCache( $titlesToUpdate ); - } - - if ( ( $actions & self::LINKS_UPDATE_ACTION ) > 0 ) { - $this->updater->scheduleRefreshLinks( $titlesToUpdate ); - } - - /* @var Title $title */ - foreach ( $titlesToUpdate as $title ) { - if ( $this->injectRC && ( $actions & self::RC_ENTRY_ACTION ) > 0 ) { - $rcAttribs = $this->getRCAttributes( $change ); - - if ( $rcAttribs !== false ) { - $this->updater->injectRCRecord( $title, $rcAttribs ); - } else { - trigger_error( "change #" . self::getChangeIdForLog( $change ) - . " did not provide RC info", E_USER_WARNING ); - } - } - - //TODO: handling for self::HISTORY_ENTRY_ACTION goes here. - // should probably be $this->updater->injectHistoryRecords() or some such. - } - - wfProfileOut( __METHOD__ ); } /** @@ -617,47 +641,6 @@ wfProfileOut( __METHOD__ ); return $params; - } - - /** - * Determine which actions to take for the given change. - * - * @since 0.4 - * - * @param Change $change the change to get the action for - * - * @return int actions to take, as a bit field using the XXX_ACTION flags - */ - public function getActions( Change $change ) { - wfProfileIn( __METHOD__ ); - - $actions = 0; - - if ( $change instanceof ItemChange ) { - $diff = $change->getDiff(); - - if ( $diff instanceof ItemDiff && !$diff->getSiteLinkDiff()->isEmpty() ) { - //TODO: make it so we don't have to re-render - // if only the site links changed (see bug 45534) - $actions |= self::PARSER_PURGE_ACTION | self::WEB_PURGE_ACTION | self::LINKS_UPDATE_ACTION - | self::RC_ENTRY_ACTION | self::HISTORY_ENTRY_ACTION; - } - - if ( $this->dataTransclusionAllowed ) { - if ( $diff instanceof EntityDiff && !$diff->getClaimsDiff()->isEmpty() ) { - $actions |= self::PARSER_PURGE_ACTION | self::WEB_PURGE_ACTION | self::LINKS_UPDATE_ACTION - | self::RC_ENTRY_ACTION | self::HISTORY_ENTRY_ACTION; - } - - if ( $diff instanceof EntityDiff && !$diff->getLabelsDiff()->isEmpty() ) { - $actions |= self::PARSER_PURGE_ACTION | self::WEB_PURGE_ACTION | self::LINKS_UPDATE_ACTION - | self::RC_ENTRY_ACTION | self::HISTORY_ENTRY_ACTION; - } - } - } - - wfProfileOut( __METHOD__ ); - return $actions; } /** diff --git a/client/includes/PageUpdater.php b/client/includes/PageUpdater.php index 9e02a72..4700cbe 100644 --- a/client/includes/PageUpdater.php +++ b/client/includes/PageUpdater.php @@ -46,10 +46,11 @@ /** * Injects an RC entry into the recentchanges, using the the given title and attribs * - * @param \Title $title - * @param array $attribs + * @since 0.5 * - * @return bool + * @param \Title[] $titles + * @param array $attribs */ - public function injectRCRecord( \Title $title, array $attribs ); + public function injectRCRecords( array $titles, array $attribs ); + } \ No newline at end of file diff --git a/client/includes/Usage/EntityUsage.php b/client/includes/Usage/EntityUsage.php index 73dc895..1531739 100644 --- a/client/includes/Usage/EntityUsage.php +++ b/client/includes/Usage/EntityUsage.php @@ -17,9 +17,12 @@ class EntityUsage { /** - * Usage flag indicating that the entity's sitelinks were used. + * Usage flag indicating that the entity's sitelinks were used as links. * This would be the case when generating language links or sister links from - * an entity's sitelinks. + * an entity's sitelinks, for display in the sidebar. + * + * @note: This does NOT cover sitelinks used in wikitext (e.g. via Lua). + * Use OTHER_USAGE for that. */ const SITELINK_USAGE = 'S'; diff --git a/client/includes/WikiPageUpdater.php b/client/includes/WikiPageUpdater.php index 2f7a02f..38a890d 100644 --- a/client/includes/WikiPageUpdater.php +++ b/client/includes/WikiPageUpdater.php @@ -81,30 +81,24 @@ /** * Injects an RC entry into the recentchanges, using the the given title and attribs * - * @param Title $title + * @param Title[] $titles * @param array $attribs - * - * @return bool */ - public function injectRCRecord( Title $title, array $attribs ) { + public function injectRCRecords( array $titles, array $attribs ) { wfProfileIn( __METHOD__ ); - if ( !$title->exists() ) { - wfProfileOut( __METHOD__ ); - return false; + foreach ( $titles as $title ) { + if ( !$title->exists() ) { + continue; + } + + $rc = ExternalRecentChange::newFromAttribs( $attribs, $title ); + + wfDebugLog( __CLASS__, __FUNCTION__ . ": saving RC entry for " . $title->getFullText() ); + $rc->save(); } - //FIXME: The same change may be reported to several target pages; - // The comment we generate should be adapted to the role that page - // plays in the change, e.g. when a sitelink changes from one page to another, - // the link was effectively removed from one and added to the other page. - $rc = ExternalRecentChange::newFromAttribs( $attribs, $title ); - - // @todo batch these - wfDebugLog( __CLASS__, __FUNCTION__ . ": saving RC entry for " . $title->getFullText() ); - $rc->save(); - wfProfileOut( __METHOD__ ); - return true; } + } diff --git a/client/includes/WikibaseClient.php b/client/includes/WikibaseClient.php index 20e3936..be9e728 100644 --- a/client/includes/WikibaseClient.php +++ b/client/includes/WikibaseClient.php @@ -773,8 +773,7 @@ new WikiPageUpdater(), $this->getStore()->getEntityRevisionLookup(), $this->getSite()->getGlobalId(), - $this->getSettings()->getSetting( 'injectRecentChanges' ), - $this->getSettings()->getSetting( 'allowDataTransclusion' ) + $this->getSettings()->getSetting( 'injectRecentChanges' ) ); } diff --git a/client/tests/phpunit/MockPageUpdater.php b/client/tests/phpunit/MockPageUpdater.php index c44e6c4..dd70f21 100644 --- a/client/tests/phpunit/MockPageUpdater.php +++ b/client/tests/phpunit/MockPageUpdater.php @@ -55,11 +55,15 @@ } } - public function injectRCRecord( Title $title, array $attribs ) { - $key = $title->getPrefixedDBkey(); - $this->updates['injectRCRecord'][ $key ] = $attribs; - - return true; + /** + * @param Title[] $titles + * @param array $attribs + */ + public function injectRCRecords( array $titles, array $attribs ) { + foreach ( $titles as $title ) { + $key = $title->getPrefixedDBkey(); + $this->updates['injectRCRecord'][ $key ] = $attribs; + } } public function getUpdates() { diff --git a/client/tests/phpunit/includes/ChangeHandlerTest.php b/client/tests/phpunit/includes/ChangeHandlerTest.php index b2dc354..6c75144 100644 --- a/client/tests/phpunit/includes/ChangeHandlerTest.php +++ b/client/tests/phpunit/includes/ChangeHandlerTest.php @@ -15,7 +15,6 @@ use Wikibase\Client\Usage\EntityUsage; use Wikibase\Client\Usage\PageEntityUsages; use Wikibase\Client\Usage\UsageLookup; -use Wikibase\Client\WikibaseClient; use Wikibase\DataModel\Entity\Diff\EntityDiff; use Wikibase\DataModel\Entity\Entity; use Wikibase\DataModel\Entity\Item; @@ -86,7 +85,6 @@ $updater, $repo, 'enwiki', - true, true ); @@ -800,87 +798,61 @@ // ========================================================================================== - public static function provideGetActions() { - $changes = TestChanges::getChanges(); - - $none = 0; - $any = 0xFFFF; - $all = ChangeHandler::HISTORY_ENTRY_ACTION - | ChangeHandler::LINKS_UPDATE_ACTION - | ChangeHandler::PARSER_PURGE_ACTION - | ChangeHandler::RC_ENTRY_ACTION - | ChangeHandler::WEB_PURGE_ACTION; - + public static function provideGetUpdateActions() { return array( - array( // #0 - $changes['property-creation'], $none, $any + 'empty' => array( + array(), + array(), ), - array( // #1 - $changes['property-deletion'], $none, $any + 'sitelink usage' => array( // #1 + array( EntityUsage::SITELINK_USAGE ), + array( ChangeHandler::LINKS_UPDATE_ACTION, ChangeHandler::WEB_PURGE_ACTION, ChangeHandler::RC_ENTRY_ACTION ), + array( ChangeHandler::PARSER_PURGE_ACTION ) ), - array( // #2 - $changes['property-set-label'], $none, $any + 'label usage' => array( + array( EntityUsage::LABEL_USAGE ), + array( ChangeHandler::PARSER_PURGE_ACTION, ChangeHandler::WEB_PURGE_ACTION, ChangeHandler::RC_ENTRY_ACTION ), + array( ChangeHandler::LINKS_UPDATE_ACTION ) ), - - array( // #3 - $changes['item-creation'], $none, $any + 'title usage' => array( + array( EntityUsage::TITLE_USAGE ), + array( ChangeHandler::PARSER_PURGE_ACTION, ChangeHandler::WEB_PURGE_ACTION, ChangeHandler::RC_ENTRY_ACTION ), + array( ChangeHandler::LINKS_UPDATE_ACTION ) ), - array( // #4 - $changes['item-deletion'], $none, $any + 'other usage' => array( + array( EntityUsage::OTHER_USAGE ), + array( ChangeHandler::PARSER_PURGE_ACTION, ChangeHandler::WEB_PURGE_ACTION, ChangeHandler::RC_ENTRY_ACTION ), + array( ChangeHandler::LINKS_UPDATE_ACTION ) ), - array( // #5 - $changes['item-deletion-linked'], $all, $none + 'all usage' => array( + array( EntityUsage::ALL_USAGE ), + array( ChangeHandler::PARSER_PURGE_ACTION, ChangeHandler::WEB_PURGE_ACTION, ChangeHandler::RC_ENTRY_ACTION ), + array( ChangeHandler::LINKS_UPDATE_ACTION ) ), - - array( // #6 - $changes['set-de-label'], $all, $none - ), - array( // #7 - $changes['set-en-label'], $all, $none // may change - ), - array( // #8 - $changes['set-en-aliases'], $none, $any - ), - - array( // #9 - $changes['add-claim'], $all, $none - ), - array( // #10 - $changes['remove-claim'], $all, $none - ), - - array( // #11 - $changes['set-dewiki-sitelink'], $all, $none // may change - ), - array( // #12 - $changes['set-enwiki-sitelink'], $all, $none // may change - ), - - array( // #13 - $changes['change-dewiki-sitelink'], $all, $none // may change - ), - array( // #14 - $changes['change-enwiki-sitelink'], $all, $none // may change - ), - - array( // #15 - $changes['remove-dewiki-sitelink'], $all, $none // may change - ), - array( // #16 - $changes['remove-enwiki-sitelink'], $all, $none // may change + 'sitelink and other usage (no redundant links update)' => array( + array( EntityUsage::SITELINK_USAGE, EntityUsage::OTHER_USAGE ), + array( ChangeHandler::PARSER_PURGE_ACTION, ChangeHandler::WEB_PURGE_ACTION, ChangeHandler::RC_ENTRY_ACTION ), + array( ChangeHandler::LINKS_UPDATE_ACTION ) ), ); } /** - * @dataProvider provideGetActions + * @dataProvider provideGetUpdateActions */ - public function testGetActions( Change $change, $expected, $unexpected ) { + public function testGetUpdateActions( $aspects, $expected, $not = array() ) { $handler = $this->newChangeHandler(); - $actions = $handler->getActions( $change ); + $actions = $handler->getUpdateActions( $aspects ); - $this->assertEquals( $expected, ( $actions & $expected ), "expected actions" ); - $this->assertEquals( 0, ( $actions & $unexpected ), "unexpected actions" ); + sort( $expected ); + sort( $actions ); + + // check that $actions contains AT LEAST $expected + $actual = array_intersect( $actions, $expected ); + $this->assertEquals( array_values( $expected ), array_values( $actual ), "expected actions" ); + + $unexpected = array_intersect( $actions, $not ); + $this->assertEmpty( array_values( $unexpected ), "unexpected actions: " . implode( '|', $unexpected ) ); } public static function provideGetEditComment() { @@ -1130,115 +1102,6 @@ } } - public static function provideGetPagesToUpdate() { - $changes = TestChanges::getChanges(); - - return array( - array( // #0 - $changes['property-creation'], - array( 'q100' => array() ), - array() - ), - array( // #1 - $changes['property-deletion'], - array( 'q100' => array() ), - array() - ), - array( // #2 - $changes['property-set-label'], - array( 'q100' => array() ), - array() - ), - - array( // #3 - $changes['item-creation'], - array( 'q100' => array() ), - array() - ), - array( // #4 - $changes['item-deletion'], - array( 'q100' => array() ), - array() - ), - array( // #5 - $changes['item-deletion-linked'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( 'Emmy2' ) - ), - - array( // #6 - $changes['set-de-label'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array(), // For the dummy page, only label and sitelink usage is defined. - ), - array( // #7 - $changes['set-en-label'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( 'Emmy2' ) - ), - array( // #8 - $changes['set-en-label'], - array( 'q100' => array( 'enwiki' => 'User:Emmy2' ) ), // bad namespace - array( ) - ), - array( // #9 - $changes['set-en-aliases'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array(), // For the dummy page, only label and sitelink usage is defined. - ), - - array( // #10 - $changes['add-claim'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( ) // statements are ignored - ), - array( // #11 - $changes['remove-claim'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( ) // statements are ignored - ), - - array( // #12 - $changes['set-dewiki-sitelink'], - array( 'q100' => array() ), - array( ) // not yet linked - ), - array( // #13 - $changes['set-enwiki-sitelink'], - array( 'q100' => array( 'enwiki' => 'Emmy' ) ), - array( 'Emmy' ) - ), - - array( // #14 - $changes['change-dewiki-sitelink'], - array( 'q100' => array( 'enwiki' => 'Emmy' ) ), - array( 'Emmy' ) - ), - array( // #15 - $changes['change-enwiki-sitelink'], - array( 'q100' => array( 'enwiki' => 'Emmy' ), 'q200' => array( 'enwiki' => 'Emmy2' ) ), - array( 'Emmy', 'Emmy2' ), - true - ), - array( // #16 - $changes['change-enwiki-sitelink-badges'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( 'Emmy2' ) // do we really want/need this to be updated? - ), - - array( // #17 - $changes['remove-dewiki-sitelink'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( 'Emmy2' ) - ), - array( // #18 - $changes['remove-enwiki-sitelink'], - array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), - array( 'Emmy2' ) - ), - ); - } - private function updateMockRepo( MockRepository $repo, $entities ) { foreach ( $entities as $id => $siteLinks ) { if ( !( $siteLinks instanceof Entity ) ) { @@ -1262,83 +1125,145 @@ } } - private function titles2strings( array $titles ) { - return array_map( - function ( Title $title ) { - return $title->getPrefixedDBKey(); - }, - $titles - ); - } - - /** - * @dataProvider provideGetPagesToUpdate - */ - public function testGetPagesToUpdate( Change $change, $entities, array $expected, $dummy = false ) { - $handler = $this->newChangeHandler( null, $entities ); - - $toUpdate = $handler->getPagesToUpdate( $change ); - $toUpdate = $this->titles2strings( $toUpdate ); - - $this->assertArrayEquals( $expected, $toUpdate ); - } - - public static function provideUpdatePages() { - $rc = WikibaseClient::getDefaultInstance()->getSettings() - ->getSetting( 'injectRecentChanges' ); - - $pto = self::provideGetPagesToUpdate(); - - $cases = array(); - - foreach ( $pto as $case ) { - $updated = $case[2]; - - $cases[] = array( - $case[0], // $change - $case[1], // $entities - array( // $expected // todo: depend on getAction() - 'purgeParserCache' => $updated, - 'purgeWebCache' => $updated, - 'scheduleRefreshLinks' => $updated, - 'injectRCRecord' => ( $rc ? $updated : array() ), - ) - ); - } - - return $cases; - } - - /** - * @dataProvider provideUpdatePages - */ - public function testUpdatePages( Change $change, $entities, array $expected ) { - $updater = new MockPageUpdater(); - $handler = $this->newChangeHandler( $updater, $entities ); - - $toUpdate = $handler->getPagesToUpdate( $change ); - $actions = $handler->getActions( $change ); - - $handler->updatePages( $change, $actions, $toUpdate ); - $updates = $updater->getUpdates(); - - foreach ( $expected as $k => $exp ) { - $up = array_keys( $updates[$k] ); - $this->assertArrayEquals( $exp, $up ); - } - - if ( isset( $updates['injectRCRecord'] ) ) { - foreach ( $updates['injectRCRecord'] as $rcAttr ) { - $this->assertType( 'array', $rcAttr ); - $this->assertArrayHasKey( 'wikibase-repo-change', $rcAttr ); - $this->assertType( 'array', $rcAttr['wikibase-repo-change'] ); - $this->assertArrayHasKey( 'entity_type', $rcAttr['wikibase-repo-change'] ); - } - } - } - public static function provideHandleChange() { - return self::provideUpdatePages(); + $changes = TestChanges::getChanges(); + + $empty = array( + 'purgeParserCache' => array(), + 'scheduleRefreshLinks' => array(), + 'purgeWebCache' => array(), + 'injectRCRecord' => array(), + ); + + $emmy2PurgeParser = array( + 'purgeParserCache' => array( 'Emmy2' => true ), + 'scheduleRefreshLinks' => array(), + 'purgeWebCache' => array( 'Emmy2' => true ), + 'injectRCRecord' => array( 'Emmy2' => true ), + ); + + $emmyUpdateLinks = array( + 'purgeParserCache' => array(), + 'scheduleRefreshLinks' => array( 'Emmy' => true ), + 'purgeWebCache' => array( 'Emmy' => true ), + 'injectRCRecord' => array( 'Emmy' => true ), + ); + + $emmy2UpdateLinks = array( + 'purgeParserCache' => array( ), + 'scheduleRefreshLinks' => array( 'Emmy2' => true ), + 'purgeWebCache' => array( 'Emmy2' => true ), + 'injectRCRecord' => array( 'Emmy2' => true ), + ); + + return array( + array( // #0 + $changes['property-creation'], + array( 'q100' => array() ), + $empty + ), + array( // #1 + $changes['property-deletion'], + array( 'q100' => array() ), + $empty + ), + array( // #2 + $changes['property-set-label'], + array( 'q100' => array() ), + $empty + ), + + array( // #3 + $changes['item-creation'], + array( 'q100' => array() ), + $empty + ), + array( // #4 + $changes['item-deletion'], + array( 'q100' => array() ), + $empty + ), + array( // #5 + $changes['item-deletion-linked'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $emmy2PurgeParser + ), + + array( // #6 + $changes['set-de-label'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $empty, // For the dummy page, only label and sitelink usage is defined. + ), + array( // #7 + $changes['set-en-label'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $emmy2PurgeParser + ), + array( // #8 + $changes['set-en-label'], + array( 'q100' => array( 'enwiki' => 'User:Emmy2' ) ), // bad namespace + $empty + ), + array( // #9 + $changes['set-en-aliases'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $empty, // For the dummy page, only label and sitelink usage is defined. + ), + + array( // #10 + $changes['add-claim'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $empty // statements are ignored + ), + array( // #11 + $changes['remove-claim'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $empty // statements are ignored + ), + + array( // #12 + $changes['set-dewiki-sitelink'], + array( 'q100' => array() ), + $empty // not yet linked + ), + array( // #13 + $changes['set-enwiki-sitelink'], + array( 'q100' => array( 'enwiki' => 'Emmy' ) ), + $emmyUpdateLinks + ), + + array( // #14 + $changes['change-dewiki-sitelink'], + array( 'q100' => array( 'enwiki' => 'Emmy' ) ), + $emmyUpdateLinks + ), + array( // #15 + $changes['change-enwiki-sitelink'], + array( 'q100' => array( 'enwiki' => 'Emmy' ), 'q200' => array( 'enwiki' => 'Emmy2' ) ), + array( + 'purgeParserCache' => array(), + 'scheduleRefreshLinks' => array( 'Emmy' => true, 'Emmy2' => true ), + 'purgeWebCache' => array( 'Emmy' => true, 'Emmy2' => true ), + 'injectRCRecord' => array( 'Emmy' => true, 'Emmy2' => true ), + ) + ), + array( // #16 + $changes['change-enwiki-sitelink-badges'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $emmy2UpdateLinks + ), + + array( // #17 + $changes['remove-dewiki-sitelink'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $emmy2UpdateLinks + ), + array( // #18 + $changes['remove-enwiki-sitelink'], + array( 'q100' => array( 'enwiki' => 'Emmy2' ) ), + $emmy2UpdateLinks + ), + ); } /** @@ -1351,9 +1276,11 @@ $handler->handleChange( $change ); $updates = $updater->getUpdates(); + $this->assertSameSize( $expected, $updates ); + foreach ( $expected as $k => $exp ) { - $up = array_keys( $updates[$k] ); - $this->assertArrayEquals( $exp, $up ); + $up = $updates[$k]; + $this->assertEquals( array_keys( $exp ), array_keys( $up ), $k ); } } -- To view, visit https://gerrit.wikimedia.org/r/170961 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: I4ef7a9315b110caf29ae43c30c582fe6623e2581 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