Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/87586
Change subject: Refactor some of MergeItems
......................................................................
Refactor some of MergeItems
This should fix the random failing test on travis
As far as I can tell this was caused by the item used
when testing exceptions not actually existing and
therefor triggering a different error than expected
i.e. item doesnt exist instead of something else
Also tidied some other things up
Change-Id: I9c39ea216f707e339ae9754278a78adbe8cfaf9b
---
M repo/includes/api/MergeItems.php
M repo/tests/phpunit/includes/api/MergeItemsTest.php
2 files changed, 35 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/86/87586/1
diff --git a/repo/includes/api/MergeItems.php b/repo/includes/api/MergeItems.php
index ee66057..1756d24 100644
--- a/repo/includes/api/MergeItems.php
+++ b/repo/includes/api/MergeItems.php
@@ -25,11 +25,11 @@
*
* @licence GNU GPL v2+
* @author Adam Shorland
+ *
+ * @todo allow merging of specific parts of an item only (eg.
sitelinks,aliases,claims)
+ * @todo allow optional deletion after merging (for admins)
*/
class MergeItems extends ApiWikibase {
-
- //todo allow merging of specific parts of an item only (eg.
sitelinks,aliases,claims)
- //todo allow optional deletion after merging (for admins)
/**
* @see \Wikibase\Api\Api::getRequiredPermissions()
@@ -53,6 +53,10 @@
$toEntityContent = $this->getEntityContentFromIdString(
$params['toid'] );
$this->validateEntityContents( $fromEntityContent,
$toEntityContent );
+ /**
+ * @var ItemContent $fromEntityContent
+ * @var ItemContent $toEntityContent
+ */
try{
$changeOps = new ChangeOpsMerge( $fromEntityContent,
$toEntityContent );
$changeOps->apply();
@@ -84,38 +88,36 @@
private function getEntityContentFromIdString( $idString ) {
$entityIdParser =
WikibaseRepo::getDefaultInstance()->getEntityIdParser();
$entityContentFactory =
WikibaseRepo::getDefaultInstance()->getEntityContentFactory();
- $entityContent = null;
try{
$entityId = $entityIdParser->parse( $idString );
- $entityContent = $entityContentFactory->getFromId(
$entityId );
+ return $entityContentFactory->getFromId( $entityId );
}
catch ( ParseException $e ){
$this->dieUsage( 'You must provide valid ids' ,
'param-invalid' );
}
-
- if( $entityContent === null ){
- $this->dieUsage( 'You must provide valid ids' ,
'param-invalid' );
- }
-
- return $entityContent;
+ return null;
}
/**
- * @param EntityContent $fromEntityContent
- * @param EntityContent $toEntityContent
+ * @param EntityContent|null $fromEntityContent
+ * @param EntityContent|null $toEntityContent
*/
private function validateEntityContents( $fromEntityContent,
$toEntityContent ) {
+ if( $fromEntityContent === null || $toEntityContent === null ){
+ $this->dieUsage( 'One of more of the ids provided do
not exist' , 'no-such-entity-id' );
+ }
+
if ( !( $fromEntityContent instanceof ItemContent &&
$toEntityContent instanceof ItemContent ) ) {
$this->dieUsage( "One or more of the entities are not
items", "not-item" );
}
- if( $fromEntityContent->getEntity()->getId()->getPrefixedId()
=== $toEntityContent->getEntity()->getId()->getPrefixedId() ){
+ if( $toEntityContent->getEntity()->getId()->equals(
$fromEntityContent->getEntity()->getId() ) ){
$this->dieUsage( 'You must provide unique ids' ,
'param-invalid' );
}
}
- private function validateParams( $params ) {
+ private function validateParams( array $params ) {
if ( empty( $params['fromid'] ) || empty( $params['toid'] ) ){
$this->dieUsage( 'You must provide a fromid and a toid'
, 'param-missing' );
}
@@ -165,6 +167,7 @@
public function getPossibleErrors() {
return array_merge( parent::getPossibleErrors(), array(
array( 'code' => 'not-item', 'info' => $this->msg(
'wikibase-api-not-item' )->text() ),
+ array( 'code' => 'no-such-entity-id', 'info' =>
$this->msg( 'wikibase-api-no-such-entity-id' )->text() ),
) );
}
diff --git a/repo/tests/phpunit/includes/api/MergeItemsTest.php
b/repo/tests/phpunit/includes/api/MergeItemsTest.php
index 463742d..a91a5bb 100644
--- a/repo/tests/phpunit/includes/api/MergeItemsTest.php
+++ b/repo/tests/phpunit/includes/api/MergeItemsTest.php
@@ -4,7 +4,9 @@
use Wikibase\Claim;
use Wikibase\DataModel\Entity\EntityId;
+use Wikibase\DataModel\Entity\ItemId;
use Wikibase\DataModel\Entity\PropertyId;
+use Wikibase\ItemContent;
use Wikibase\Property;
use Wikibase\PropertyContent;
@@ -35,11 +37,14 @@
parent::setUp();
if( !isset( self::$hasSetup ) ){
$this->initTestEntities( array( 'Empty', 'Empty2' ) );
- $prop42 = PropertyId::newFromNumber( 56 );
+
$prop = PropertyContent::newEmpty();
- $prop->getEntity()->setId( $prop42 );
+ $prop->getEntity()->setId( PropertyId::newFromNumber(
56 ) );
$prop->getEntity()->setDataTypeId( 'string' );
- $prop->save( 'testing' );
+ $prop->save( 'mergeitemstest' );
+ $item = ItemContent::newEmpty();
+ $item->getEntity()->setId( ItemId::newFromNumber( 999 )
);
+ $item->save( 'mergeitemstest' );
}
self::$hasSetup = true;
}
@@ -173,25 +178,25 @@
'p' => array( ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-missing' ) ) ),
array( //1 only from id
- 'p' => array( 'fromid' => 'q1' ),
+ 'p' => array( 'fromid' => 'q999' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-missing' ) ) ),
array( //2 only to id
- 'p' => array( 'toid' => 'q1' ),
+ 'p' => array( 'toid' => 'q999' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-missing' ) ) ),
array( //3 toid bad
- 'p' => array( 'fromid' => 'q1', 'toid' =>
'ABCDE' ),
+ 'p' => array( 'fromid' => 'q999', 'toid' =>
'ABCDE' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid' ) ) ),
array( //4 fromid bad
- 'p' => array( 'fromid' => 'ABCDE', 'toid' =>
'q1' ),
+ 'p' => array( 'fromid' => 'ABCDE', 'toid' =>
'q999' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid' ) ) ),
- array( //5 bot same id
- 'p' => array( 'fromid' => 'q1', 'toid' => 'q1'
),
- 'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid' ) ) ),
+ array( //5 both same id
+ 'p' => array( 'fromid' => 'Q999', 'toid' =>
'q999' ),
+ 'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid', 'message' => 'You must provide
unique ids' ) ) ),
array( //6 from id is property
- 'p' => array( 'fromid' => 'p56', 'toid' => 'q2'
),
+ 'p' => array( 'fromid' => 'p56', 'toid' =>
'q999' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'not-item' ) ) ),
array( //7 to id is property
- 'p' => array( 'fromid' => 'q2', 'toid' => 'p56'
),
+ 'p' => array( 'fromid' => 'q999', 'toid' =>
'p56' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'not-item' ) ) ),
);
}
--
To view, visit https://gerrit.wikimedia.org/r/87586
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9c39ea216f707e339ae9754278a78adbe8cfaf9b
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Wikibase
Gerrit-Branch: master
Gerrit-Owner: Addshore <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
