jenkins-bot has submitted this change and it was merged.
Change subject: Split ChangeHandler::getPagesToUpdate into new class with tests
......................................................................
Split ChangeHandler::getPagesToUpdate into new class with tests
Change-Id: I86a78a1a275c7deb7a822e1188a1a04c9c37bcf5
---
M client/WikibaseClient.classes.php
M client/includes/ChangeHandler.php
A client/includes/ReferencedPagesFinder.php
A client/tests/phpunit/includes/ReferencedPagesFinderTest.php
4 files changed, 416 insertions(+), 84 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/client/WikibaseClient.classes.php
b/client/WikibaseClient.classes.php
index ad29c64..e554ee4 100644
--- a/client/WikibaseClient.classes.php
+++ b/client/WikibaseClient.classes.php
@@ -17,6 +17,7 @@
'Wikibase\LangLinkHandler' => 'includes/LangLinkHandler.php',
'Wikibase\ChangeHandler' => 'includes/ChangeHandler.php',
'Wikibase\NamespaceChecker' => 'includes/NamespaceChecker.php',
+ 'Wikibase\ReferencedPagesFinder' =>
'includes/ReferencedPagesFinder.php',
'Wikibase\RepoItemLinkGenerator' =>
'includes/RepoItemLinkGenerator.php',
'Wikibase\RepoLinker' => 'includes/RepoLinker.php',
'Wikibase\Client\WikibaseClient' =>
'includes/WikibaseClient.php',
diff --git a/client/includes/ChangeHandler.php
b/client/includes/ChangeHandler.php
index e840f2f..287a31f 100644
--- a/client/includes/ChangeHandler.php
+++ b/client/includes/ChangeHandler.php
@@ -1,4 +1,5 @@
<?php
+
namespace Wikibase;
use Diff\Diff;
@@ -17,25 +18,7 @@
* it should be fed to this service which then takes care handling
* it.
*
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License along
- * with this program; if not, write to the Free Software Foundation, Inc.,
- * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
- * http://www.gnu.org/copyleft/gpl.html
- *
* @since 0.1
- *
- * @file
- * @ingroup WikibaseLib
*
* @licence GNU GPL v2+
* @author Daniel Kinzler
@@ -100,9 +83,14 @@
protected $entityLookup;
/**
- * @var \Site $site
+ * @var Site $site
*/
protected $site;
+
+ /**
+ * @var string
+ */
+ protected $siteId;
/**
* @var NamespaceChecker $namespaceChecker
@@ -151,6 +139,8 @@
}
$this->site = $localSite;
+ $this->siteId = $localSite->getGlobalId();
+
$this->updater = $updater;
$this->entityLookup = $entityLookup;
$this->entityUsageIndex = $entityUsageIndex;
@@ -549,8 +539,6 @@
/**
* Returns the pages that need some kind of updating given the change.
*
- * @todo: determine actions for each page!
- *
* @param Change $change
*
* @return Title[] the titles of the pages to update
@@ -558,72 +546,19 @@
public function getPagesToUpdate( Change $change ) {
wfProfileIn( __METHOD__ );
- $pagesToUpdate = array();
+ // todo inject!
+ $referencedPagesFinder = new ReferencedPagesFinder(
+ $this->entityUsageIndex,
+ $this->namespaceChecker,
+ $this->siteId,
+ $this->checkPageExistence
+ );
- if ( $change instanceof ItemChange ) {
- // update local pages connected to a relevant data item.
-
- $itemId = $change->getEntityId();
-
- $siteGlobalId = $this->site->getGlobalId();
-
- $usedOnPages = $this->entityUsageIndex->getEntityUsage(
array( $itemId ) );
- $pagesToUpdate = array_merge( $pagesToUpdate,
$usedOnPages );
-
- // if an item's sitelinks change, update the old and
the new target
- $siteLinkDiff = $change->getSiteLinkDiff();
-
- if ( isset( $siteLinkDiff[$siteGlobalId] ) ) {
-
- // $siteLinkDiff changed from containing atomic
diffs to
- // containing map diffs. For B/C, handle both
cases.
- $siteLinkDiffOp = $siteLinkDiff[$siteGlobalId];
-
- if ( $siteLinkDiffOp instanceof Diff ) {
- if ( array_key_exists( 'name',
$siteLinkDiffOp ) ) {
- $siteLinkDiffOp =
$siteLinkDiffOp['name'];
- } else {
- // change to badges only
- $siteLinkDiffOp = null;
- }
- }
-
- if ( $siteLinkDiffOp === null ) {
- // do nothing, only badges were changed
- }elseif ( $siteLinkDiffOp instanceof DiffOpAdd
) {
- $pagesToUpdate[] =
$siteLinkDiffOp->getNewValue();
- } elseif ( $siteLinkDiffOp instanceof
DiffOpRemove ) {
- $pagesToUpdate[] =
$siteLinkDiffOp->getOldValue();
- } elseif ( $siteLinkDiffOp instanceof
DiffOpChange ) {
- $pagesToUpdate[] =
$siteLinkDiffOp->getNewValue();
- $pagesToUpdate[] =
$siteLinkDiffOp->getOldValue();
- } else {
- wfWarn( "Unknown change operation: " .
get_class( $siteLinkDiffOp ) . ")" );
- }
- }
- }
-
- $pagesToUpdate = array_unique( $pagesToUpdate );
- $titlesToUpdate = array();
-
- foreach ( $pagesToUpdate as $page ) {
- $title = Title::newFromText( $page );
-
- if ( !$title->exists() && $this->checkPageExistence ) {
- continue;
- }
-
- $ns = $title->getNamespace();
-
- if ( !is_int( $ns ) ||
!$this->namespaceChecker->isWikibaseEnabled( $ns ) ) {
- continue;
- }
-
- $titlesToUpdate[] = $title;
- }
+ $pagesToUpdate = $referencedPagesFinder->getPages( $change );
wfProfileOut( __METHOD__ );
- return $titlesToUpdate;
+
+ return $pagesToUpdate;
}
/**
diff --git a/client/includes/ReferencedPagesFinder.php
b/client/includes/ReferencedPagesFinder.php
new file mode 100644
index 0000000..0c7017c
--- /dev/null
+++ b/client/includes/ReferencedPagesFinder.php
@@ -0,0 +1,189 @@
+<?php
+
+namespace Wikibase;
+
+use Diff\Diff;
+use Diff\DiffOpAdd;
+use Diff\DiffOpChange;
+use Diff\DiffOpRemove;
+use Title;
+use Wikibase\DataModel\Entity\ItemId;
+
+/**
+ * Interface for change handling. Whenever a change is detected,
+ * it should be fed to this service which then takes care handling
+ * it.
+ *
+ * @since 0.1
+ *
+ * @licence GNU GPL v2+
+ * @author Daniel Kinzler
+ * @author Katie Filbert < aude.w...@gmail.com >
+ */
+class ReferencedPagesFinder {
+
+ /**
+ * @var ItemUsageIndex
+ */
+ private $itemUsageIndex;
+
+ /**
+ * @var NamespaceChecker
+ */
+ private $namespaceChecker;
+
+ /**
+ * @var string
+ */
+ private $siteId;
+
+ /**
+ * @var boolean
+ */
+ private $checkPageExistence;
+
+ /**
+ * @param ItemUsageIndex $itemUsageIndex
+ * @param NamespaceChecker $namespaceChecker
+ * @param string $siteId
+ * @param boolean $checkPageExistence
+ */
+ public function __construct( ItemUsageIndex $itemUsageIndex,
NamespaceChecker $namespaceChecker,
+ $siteId, $checkPageExistence = true ) {
+ $this->itemUsageIndex = $itemUsageIndex;
+ $this->namespaceChecker = $namespaceChecker;
+ $this->siteId = $siteId;
+ $this->checkPageExistence = $checkPageExistence;
+ }
+
+ /**
+ * @since 0.5
+ *
+ * @param Change $change
+ *
+ * @return Title[]
+ */
+ public function getPages( Change $change ) {
+ if ( ! ( $change instanceof ItemChange ) ) {
+ return array();
+ }
+
+ $pages = $this->getReferencedPages( $change );
+
+ return $this->getTitlesToUpdate( $pages );
+ }
+
+ /**
+ * Returns the pages that need some kind of updating given the change.
+ *
+ * @param Change $change
+ *
+ * @return Title[] the titles of the pages to update
+ */
+ private function getReferencedPages( Change $change ) {
+ $itemId = $change->getEntityId();
+
+ $pages = $this->itemUsageIndex->getEntityUsage( array( $itemId
) );
+
+ $siteLinkDiff = $change->getSiteLinkDiff();
+
+ if ( $this->isRelevantSiteLinkChange( $siteLinkDiff ) ) {
+ $pages = $this->addSiteLinkDiffPages( $siteLinkDiff,
$pages );
+ }
+
+ return array_unique( $pages );
+ }
+
+ /**
+ * @param Diff $siteLinkDiff
+ *
+ * @return boolean
+ */
+ private function isRelevantSiteLinkChange( Diff $siteLinkDiff ) {
+ return isset( $siteLinkDiff[$this->siteId] ) &&
!$this->isBadgesOnlyChange( $siteLinkDiff );
+ }
+
+ /**
+ * @param Diff $siteLinkDiff
+ * @param string[] $pages
+ *
+ * @return string[]
+ */
+ private function addSiteLinkDiffPages( Diff $siteLinkDiff, array $pages
) {
+ return array_merge(
+ $pages,
+ $this->getPagesReferencedInDiff( $siteLinkDiff )
+ );
+ }
+
+ /**
+ * @param Diff $siteLinkDiff
+ *
+ * @return array
+ */
+ private function getPagesReferencedInDiff( Diff $siteLinkDiff ) {
+ $pagesToUpdate = array();
+
+ // $siteLinkDiff changed from containing atomic diffs to
+ // containing map diffs. For B/C, handle both cases.
+ $siteLinkDiffOp = $siteLinkDiff[$this->siteId];
+
+ if ( ( $siteLinkDiffOp instanceof Diff ) && ( array_key_exists(
'name', $siteLinkDiffOp ) ) ) {
+ $siteLinkDiffOp = $siteLinkDiffOp['name'];
+ }
+
+ if ( $siteLinkDiffOp instanceof DiffOpAdd ) {
+ $pagesToUpdate[] = $siteLinkDiffOp->getNewValue();
+ } elseif ( $siteLinkDiffOp instanceof DiffOpRemove ) {
+ $pagesToUpdate[] = $siteLinkDiffOp->getOldValue();
+ } elseif ( $siteLinkDiffOp instanceof DiffOpChange ) {
+ $pagesToUpdate[] = $siteLinkDiffOp->getNewValue();
+ $pagesToUpdate[] = $siteLinkDiffOp->getOldValue();
+ } else {
+ throw new UnexpectedValueException(
+ "Unknown change operation: " . get_class(
$siteLinkDiffOp ) . ")"
+ );
+ }
+
+ return $pagesToUpdate;
+ }
+
+ /**
+ * @param Diff $siteLinkDiff
+ *
+ * @return boolean
+ */
+ private function isBadgesOnlyChange( Diff $siteLinkDiff ) {
+ $siteLinkDiffOp = $siteLinkDiff[$this->siteId];
+
+ return ( $siteLinkDiffOp instanceof Diff && !array_key_exists(
'name', $siteLinkDiffOp ) );
+ }
+
+ /**
+ * @param array $pagesToUpdate
+ *
+ * @return Title[]
+ */
+ private function getTitlesToUpdate( array $pagesToUpdate ) {
+ $titlesToUpdate = array();
+
+ foreach ( $pagesToUpdate as $page ) {
+ $title = Title::newFromText( $page );
+
+ if ( !$title->exists() && $this->checkPageExistence ) {
+ continue;
+ }
+
+ $ns = $title->getNamespace();
+
+ if ( !is_int( $ns ) ||
!$this->namespaceChecker->isWikibaseEnabled( $ns ) ) {
+ continue;
+ }
+
+ $titlesToUpdate[] = $title;
+ }
+
+ return $titlesToUpdate;
+ }
+
+}
diff --git a/client/tests/phpunit/includes/ReferencedPagesFinderTest.php
b/client/tests/phpunit/includes/ReferencedPagesFinderTest.php
new file mode 100644
index 0000000..c0d7a65
--- /dev/null
+++ b/client/tests/phpunit/includes/ReferencedPagesFinderTest.php
@@ -0,0 +1,207 @@
+<?php
+
+namespace Wikibase\Test;
+
+use ContentHandler;
+use Title;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\DataModel\SimpleSiteLink;
+use Wikibase\Item;
+use Wikibase\ItemChange;
+use Wikibase\ReferencedPagesFinder;
+use WikiPage;
+
+/**
+ * @covers Wikibase\ReferencedPagesFinder
+ *
+ * @since 0.5
+ *
+ * @group WikibaseClient
+ *
+ * @licence GNU GPL v2+
+ * @author Katie Filbert < aude.w...@gmail.com >
+ */
+class ReferencedPagesFinderTest extends \PHPUnit_Framework_TestCase {
+
+ static protected $titles;
+
+ public function setUp() {
+ parent::setUp();
+
+ if ( !self::$titles ) {
+ self::$titles = array(
+ Title::newFromText( 'Berlin' ),
+ Title::newFromText( 'Rome' )
+ );
+
+ foreach( self::$titles as $title ) {
+ $content = ContentHandler::makeContent( 'edit
page', $title );
+ $page = WikiPage::factory( $title );
+ $page->doEditContent( $content, 'edit page' );
+ }
+ }
+ }
+
+ /**
+ * @dataProvider getPagesProvider
+ */
+ public function testGetPages( $expected, $usage, $change, $message ) {
+ $itemUsageIndex = $this->getMockBuilder(
'\Wikibase\ItemUsageIndex' )
+
->disableOriginalConstructor()->getMock();
+
+ $itemUsageIndex->expects( $this->any() )
+ ->method( 'getEntityUsage' )
+ ->will( $this->returnValue( $usage ) );
+
+ $namespaceChecker = $this->getMockBuilder(
'\Wikibase\NamespaceChecker' )
+
->disableOriginalConstructor()->getMock();
+
+ $namespaceChecker->expects( $this->any() )
+ ->method( 'isWikibaseEnabled' )
+ ->will( $this->returnValue( true ) );
+
+ $referencedPagesFinder = new ReferencedPagesFinder(
$itemUsageIndex, $namespaceChecker, 'enwiki' );
+
+ $this->assertEquals( $expected,
$referencedPagesFinder->getPages( $change ), $message );
+ }
+
+ public function getPagesProvider() {
+ $berlin = Title::newFromText( 'Berlin' );
+ $rome = Title::newFromText( 'Rome' );
+
+ $cases = array();
+
+ $cases[] = array(
+ array( $berlin ),
+ array(),
+ ItemChange::newFromUpdate(
+ ItemChange::ADD,
+ null,
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Berlin' ) )
+ ),
+ 'created item with site link to client'
+ );
+
+ $cases[] = array(
+ array( $berlin ),
+ array(),
+ ItemChange::newFromUpdate(
+ ItemChange::UPDATE,
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Berlin' ) ),
+ $this->getEmptyItem()
+ ),
+ 'removed site link to client'
+ );
+
+ $cases[] = array(
+ array( $rome ),
+ array(),
+ ItemChange::newFromUpdate(
+ ItemChange::UPDATE,
+ $this->getEmptyItem(),
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Rome' ) )
+ ),
+ 'added site link to client'
+ );
+
+ $cases[] = array(
+ array( $berlin, $rome ),
+ array(),
+ ItemChange::newFromUpdate(
+ ItemChange::UPDATE,
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Rome' ) ),
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Berlin' ) )
+ ),
+ 'changed client site link'
+ );
+
+ $cases[] = array(
+ array( $rome ),
+ array(),
+ ItemChange::newFromUpdate(
+ ItemChange::REMOVE,
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Rome' ) ),
+ null
+ ),
+ 'item connected to client was deleted'
+ );
+
+ $cases[] = array(
+ array( $rome ),
+ array( 'Rome' ),
+ ItemChange::newFromUpdate(
+ ItemChange::UPDATE,
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Rome' ) ),
+ $this->getItemWithSiteLinks( array(
+ 'enwiki' => 'Rome',
+ 'itwiki' => 'Roma'
+ ) )
+ ),
+ 'added site link on connected item'
+ );
+
+ $cases[] = array(
+ array(),
+ array(),
+ ItemChange::newFromUpdate(
+ ItemChange::UPDATE,
+ $this->getEmptyItem(),
+ $this->getItemWithLabel( 'de', 'Berlin' )
+ ),
+ 'unrelated label change'
+ );
+
+ $connectedItem = $this->getItemWithSiteLinks( array( 'enwiki'
=> 'Berlin' ) );
+ $connectedItemWithLabel = $this->getItemWithSiteLinks( array(
'enwiki' => 'Berlin' ) );
+ $connectedItemWithLabel->setLabel( 'enwiki', 'Berlin' );
+
+ $cases[] = array(
+ array( $berlin ),
+ array( 'Berlin' ),
+ ItemChange::newFromUpdate( ItemChange::UPDATE,
$connectedItem, $connectedItemWithLabel ),
+ 'connected item label change'
+ );
+
+ $itemWithBadge = $this->getEmptyItem();
+ $badges = array( new ItemId( 'Q34' ) );
+ $itemWithBadge->addSimpleSiteLink( new SimpleSiteLink(
'enwiki', 'Rome', $badges ) );
+
+ $cases[] = array(
+ array(),
+ array(),
+ ItemChange::newFromUpdate( ItemChange::UPDATE,
+ $this->getItemWithSiteLinks( array( 'enwiki' =>
'Rome' ) ),
+ $itemWithBadge ),
+ 'badge change'
+ );
+
+ return $cases;
+ }
+
+ private function getEmptyItem() {
+ $item = Item::newEmpty();
+ $item->setId( 2 );
+
+ return $item->copy();
+ }
+
+ private function getItemWithSiteLinks( $links ) {
+ $item = $this->getEmptyItem();
+
+ foreach( $links as $siteId => $page ) {
+ $item->addSimpleSiteLink(
+ new SimpleSiteLink( $siteId, $page )
+ );
+ }
+
+ return $item->copy();
+ }
+
+ private function getItemWithLabel( $lang, $label ) {
+ $item = $this->getEmptyItem();
+ $item->setLabel( $lang, $label );
+
+ return $item;
+ }
+
+}
--
To view, visit https://gerrit.wikimedia.org/r/96384
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I86a78a1a275c7deb7a822e1188a1a04c9c37bcf5
Gerrit-PatchSet: 9
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Aude <aude.w...@gmail.com>
Gerrit-Reviewer: Daniel Kinzler <daniel.kinz...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
