Jeroen De Dauw has submitted this change and it was merged.
Change subject: Improvements to ChangeOpSiteLink and its test
......................................................................
Improvements to ChangeOpSiteLink and its test
Change-Id: Ie2b2aea6442ad104a24a9e74fea360c0855d74e3
---
M repo/includes/api/EditEntity.php
M repo/includes/changeop/ChangeOpSiteLink.php
M repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
3 files changed, 53 insertions(+), 52 deletions(-)
Approvals:
Daniel Kinzler: Looks good to me, approved
jenkins-bot: Verified
diff --git a/repo/includes/api/EditEntity.php b/repo/includes/api/EditEntity.php
index 466cb98..52c45b0 100644
--- a/repo/includes/api/EditEntity.php
+++ b/repo/includes/api/EditEntity.php
@@ -337,7 +337,7 @@
}
if ( array_key_exists( 'remove', $arg ) ||
$arg['title'] === "" ) {
- $siteLinksChangeOps[] = new ChangeOpSiteLink(
$linkSite, null );
+ $siteLinksChangeOps[] = new ChangeOpSiteLink(
$globalSiteId, null );
} else {
$linkPage = $linkSite->normalizePageName(
Utils::trimWhitespace( $arg['title'] ) );
@@ -346,7 +346,7 @@
$this->dieUsage( $this->msg(
'wikibase-api-no-external-page' )->text(), 'add-sitelink-failed' );
}
- $siteLinksChangeOps[] = new ChangeOpSiteLink(
$linkSite, $linkPage );
+ $siteLinksChangeOps[] = new ChangeOpSiteLink(
$globalSiteId, $linkPage );
}
}
diff --git a/repo/includes/changeop/ChangeOpSiteLink.php
b/repo/includes/changeop/ChangeOpSiteLink.php
index b6dbe9b..6e9165a 100644
--- a/repo/includes/changeop/ChangeOpSiteLink.php
+++ b/repo/includes/changeop/ChangeOpSiteLink.php
@@ -4,6 +4,7 @@
use InvalidArgumentException;
use Site;
+use Wikibase\DataModel\SimpleSiteLink;
/**
* Class for sitelink change operation
@@ -35,32 +36,36 @@
/**
* @since 0.4
*
- * @var Site
+ * @var string
*/
- protected $linkSite;
+ protected $siteId;
/**
* @since 0.4
*
* @var string|null
*/
- protected $linkPage;
+ protected $pageName;
/**
* @since 0.4
*
- * @param Site $linkSite
- * @param string|null $linkPage
+ * @param string $siteId
+ * @param string|null $pageName Null in case the link with the provided
siteId should be removed
*
* @throws InvalidArgumentException
*/
- public function __construct( Site $linkSite, $linkPage ) {
- if ( !is_string( $linkPage ) && $linkPage !==null ) {
+ public function __construct( $siteId, $pageName ) {
+ if ( !is_string( $siteId ) ) {
+ throw new InvalidArgumentException( '$siteId needs to
be a string' );
+ }
+
+ if ( !is_string( $pageName ) && $pageName !==null ) {
throw new InvalidArgumentException( '$linkPage needs to
be a string|null' );
}
- $this->linkSite = $linkSite;
- $this->linkPage = $linkPage;
+ $this->siteId = $siteId;
+ $this->pageName = $pageName;
}
/**
@@ -70,15 +75,18 @@
*
* @param Entity $entity
*
- * @return SiteLink|bool
+ * @throws InvalidArgumentException
*/
public function apply( Entity $entity ) {
- if ( $this->linkPage === null ) {
- $entity->removeSiteLink( $this->linkSite->getGlobalId()
);
- return true; // don't show an error when the sitelink
we try to remove does not exist
- } else {
- $siteLink = new SiteLink( $this->linkSite,
$this->linkPage );
- return $entity->addSiteLink( $siteLink,'set' );
+ if ( !( $entity instanceof Item ) ) {
+ throw new InvalidArgumentException( 'ChangeOpSiteLink
can only be applied to Item instances' );
+ }
+
+ if ( $this->pageName === null ) {
+ $entity->removeSiteLink( $this->siteId );
+ }
+ else {
+ $entity->addSimpleSiteLink( new SimpleSiteLink(
$this->siteId, $this->pageName ) );
}
}
diff --git a/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
b/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
index 31467fe..544daa5 100644
--- a/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
+++ b/repo/tests/phpunit/includes/changeop/ChangeOpSiteLinkTest.php
@@ -2,8 +2,10 @@
namespace Wikibase\Test;
+use Site;
use Wikibase\ChangeOpSiteLink;
-use Wikibase\ItemContent;
+use Wikibase\DataModel\SimpleSiteLink;
+use Wikibase\Item;
use InvalidArgumentException;
use Wikibase\SiteLink;
@@ -39,39 +41,36 @@
*/
class ChangeOpSiteLinkTest extends \PHPUnit_Framework_TestCase {
- public function invalidConstructorProvider() {
- $args = array();
- $args[] = array( $this->getSite( 'enwiki' ), 1234 );
-
- return $args;
- }
-
/**
* @dataProvider invalidConstructorProvider
* @expectedException InvalidArgumentException
- *
- * @param Site $linkSite
- * @param string|null $aliases
*/
- public function testInvalidConstruct( $linkSite, $linkPage ) {
- $changeOpSiteLink = new ChangeOpSiteLink( $linkSite, $linkPage
);
+ public function testConstructorWithInvalidArguments( $siteId, $linkPage
) {
+ new ChangeOpSiteLink( $siteId, $linkPage );
+ }
+
+ public function invalidConstructorProvider() {
+ $argLists = array();
+
+ $argLists[] = array( 'enwiki', 1234 );
+ $argLists[] = array( 1234, 'Berlin' );
+
+ return $argLists;
}
public function changeOpSiteLinkProvider() {
- $enWiki = $this->getSite( 'enwiki' );
- $deWiki = $this->getSite( 'dewiki' );
- $existingSiteLinks = array( new SiteLink( $deWiki, 'Berlin' ) );
- $enSiteLink = new SiteLink( $enWiki, 'Berlin' );
+ $existingSiteLinks = array( new SimpleSiteLink( 'dewiki',
'Berlin' ) );
+ $enSiteLink = new SimpleSiteLink( 'enwiki', 'Berlin' );
- $item = ItemContent::newEmpty();
- $entity = $item->getEntity();
+ $item = Item::newEmpty();
+
foreach ( $existingSiteLinks as $siteLink ) {
- $entity->addSiteLink( $siteLink );
+ $item->addSimpleSiteLink( $siteLink );
}
$args = array();
- $args[] = array ( clone $entity, new ChangeOpSiteLink( $enWiki,
'Berlin' ), array_merge( $existingSiteLinks, array ( $enSiteLink ) ) );
- $args[] = array ( clone $entity, new ChangeOpSiteLink( $deWiki,
null ), array() );
+ $args[] = array ( clone $item, new ChangeOpSiteLink( 'enwiki',
'Berlin' ), array_merge( $existingSiteLinks, array ( $enSiteLink ) ) );
+ $args[] = array ( clone $item, new ChangeOpSiteLink( 'dewiki',
null ), array() );
return $args;
}
@@ -79,23 +78,17 @@
/**
* @dataProvider changeOpSiteLinkProvider
*
- * @param Entity $entity
+ * @param Item $entity
* @param ChangeOpSiteLink $changeOpSiteLink
- * @param string $expectedSiteLinks
+ * @param SimpleSiteLink[] $expectedSiteLinks
*/
- public function testApply( $entity, $changeOpSiteLink,
$expectedSiteLinks ) {
+ public function testApply( Item $entity, ChangeOpSiteLink
$changeOpSiteLink, array $expectedSiteLinks ) {
$changeOpSiteLink->apply( $entity );
+
$this->assertEquals(
- SiteLink::siteLinksToArray( $expectedSiteLinks ),
- SiteLink::siteLinksToArray( $entity->getSiteLinks() )
+ $expectedSiteLinks,
+ $entity->getSimpleSiteLinks()
);
- }
-
- protected function getSite( $globalId ) {
- $site = new \MediaWikiSite();
- $site->setGlobalId( $globalId );
-
- return $site;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/67506
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie2b2aea6442ad104a24a9e74fea360c0855d74e3
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Daniel Kinzler <[email protected]>
Gerrit-Reviewer: Jeroen De Dauw <[email protected]>
Gerrit-Reviewer: Tobias Gritschacher <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
