Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/94328
Change subject: Add ignoreconflicts to wbmergeitems api module
......................................................................
Add ignoreconflicts to wbmergeitems api module
Change-Id: I03684ea387e818641d26cda9f1613822bc5e2534
---
M repo/includes/api/MergeItems.php
M repo/tests/phpunit/includes/api/MergeItemsTest.php
2 files changed, 167 insertions(+), 102 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/28/94328/1
diff --git a/repo/includes/api/MergeItems.php b/repo/includes/api/MergeItems.php
index a220be4..6f7b09d 100644
--- a/repo/includes/api/MergeItems.php
+++ b/repo/includes/api/MergeItems.php
@@ -2,26 +2,19 @@
namespace Wikibase\Api;
-use ApiBase, User, Status, SiteList;
+use ApiBase;
+use Deserializers\Exceptions\InvalidAttributeException;
+use Status;
use ValueParsers\ParseException;
use Wikibase\ChangeOp\ChangeOpException;
use Wikibase\ChangeOp\ChangeOpsMerge;
-use Wikibase\Claim;
-use Wikibase\DataModel\Entity\EntityId;
-use Wikibase\EntityContentFactory;
use Wikibase\EntityContent;
-use Wikibase\Item;
use Wikibase\ItemContent;
-use Wikibase\Property;
use Wikibase\Repo\WikibaseRepo;
use Wikibase\Summary;
-use Wikibase\Utils;
/**
* @since 0.5
- *
- * @ingroup WikibaseRepo
- * @ingroup API
*
* @licence GNU GPL v2+
* @author Adam Shorland
@@ -60,19 +53,35 @@
$this->dieUsage( $status->getMessage(),
'permissiondenied');
}
+ $ignoreConflicts = $this->getIgnoreConflicts( $params );
+
/**
* @var ItemContent $fromEntityContent
* @var ItemContent $toEntityContent
*/
try{
- $changeOps = new ChangeOpsMerge( $fromEntityContent,
$toEntityContent );
+ $changeOps = new ChangeOpsMerge(
+ $fromEntityContent,
+ $toEntityContent,
+ $ignoreConflicts
+ );
$changeOps->apply();
}
- catch( ChangeOpException $e ){
- $this->dieUsage( $e->getMessage(), 'failed-save');
+ catch( InvalidAttributeException $e ) {
+ $this->dieUsage( $e->getMessage(), 'param-invalid' );
+ }
+ catch( ChangeOpException $e ) {
+ $this->dieUsage( $e->getMessage(), 'failed-save' );
}
$this->attemptSaveMerge( $fromEntityContent, $toEntityContent,
$params );
+ }
+
+ protected function getIgnoreConflicts( $params ) {
+ if( isset( $params['ignoreconflicts'] ) ){
+ return $params['ignoreconflicts'];
+ }
+ return array();
}
protected function addEntityToOutput( EntityContent $entityContent,
Status $status, $name ) {
@@ -167,8 +176,9 @@
$this->addEntityToOutput( $toItemContent, $toStatus,
'to' );
$this->getResult()->addValue( null, 'success', 1 );
-
+ } else {
//todo if the second result is not a success we should
probably undo the first change
+ $this->getResult()->addValue( null, 'success', 0 );
}
}
@@ -195,6 +205,11 @@
'toid' => array(
ApiBase::PARAM_TYPE => 'string',
),
+ 'ignoreconflicts' => array(
+ ApiBase::PARAM_ISMULTI => true,
+ ApiBase::PARAM_TYPE => 'string',
+ ApiBase::PARAM_REQUIRED => false,
+ ),
'summary' => array(
ApiBase::PARAM_TYPE => 'string',
),
@@ -212,6 +227,7 @@
array(
'fromid' => array( 'The id to merge from' ),
'toid' => array( 'The id to merge to' ),
+ 'ignoreconflicts' => array( 'Array of elements
of the item to ignore conflicts for, can only contain values of "label" and or
"description"' ),
'token' => 'An "edittoken" token previously
obtained through the token module (prop=info).',
'summary' => array( 'Summary for the edit.',
"Will be prepended by an automatically
generated comment. The length limit of the
@@ -236,8 +252,14 @@
*/
protected function getExamples() {
return array(
- 'api.php?action=wbmergeitems&fromid=Q42&toid=Q222' =>
'Merges data from Q42 into Q222',
- 'api.php?action=wbmergeitems&fromid=Q555&toid=Q3' =>
'Merges data from Q555 into Q3',
+ 'api.php?action=wbmergeitems&fromid=Q42&toid=Q222' =>
+ 'Merges data from Q42 into Q222',
+ 'api.php?action=wbmergeitems&fromid=Q555&toid=Q3' =>
+ 'Merges data from Q555 into Q3',
+
'api.php?action=wbmergeitems&fromid=Q66&toid=Q99&ignoreconflicts=label' =>
+ 'Merges data from Q66 into Q99 ignoring any
conflicting labels',
+
'api.php?action=wbmergeitems&fromid=Q66&toid=Q99&ignoreconflicts=label|description'
=>
+ 'Merges data from Q66 into Q99 ignoring any
conflicting labels and descriptions',
);
}
diff --git a/repo/tests/phpunit/includes/api/MergeItemsTest.php
b/repo/tests/phpunit/includes/api/MergeItemsTest.php
index a91a5bb..130ab65 100644
--- a/repo/tests/phpunit/includes/api/MergeItemsTest.php
+++ b/repo/tests/phpunit/includes/api/MergeItemsTest.php
@@ -2,16 +2,13 @@
namespace Wikibase\Test\Api;
-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;
/**
- * Unit tests for the Wikibase\Repo\Api\MergeItems class.
+ * @covers Wikibase\Api\MergeItems
*
* @since 0.5
*
@@ -28,8 +25,6 @@
* @author Adam Shorland
*/
class MergeItemsTest extends WikibaseApiTestCase {
-
- //todo add check merge conflicts are thrown
private static $hasSetup;
@@ -50,86 +45,127 @@
}
public static function provideData(){
- return array(
- //check all elements move individually
- array(
- array( 'labels' => array( 'en' => array(
'language' => 'en', 'value' => 'foo' ) ) ),
- array(),
- array(),
- array( 'labels' => array( 'en' => array(
'language' => 'en', 'value' => 'foo' ) ) ),
- ),
- array(//make sure items with the same labels are still
merged
- array( 'labels' => array( 'en' => array(
'language' => 'en', 'value' => 'foo' ) ) ),
- array( 'labels' => array( 'en' => array(
'language' => 'en', 'value' => 'foo' ) ) ),
- array(),
- array( 'labels' => array( 'en' => array(
'language' => 'en', 'value' => 'foo' ) ) ),
- ),
- array(
- array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
- array(),
- array(),
- array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
- ),
- array(
- array( 'aliases' => array( array( "language" =>
"nl", "value" => "Dickes B" ) ) ),
- array(),
- array(),
- array( 'aliases' => array( array( "language" =>
"nl", "value" => "Dickes B" ) ) ),
- ),
- array(
- array( 'sitelinks' => array( 'dewiki' => array(
'site' => 'dewiki', 'title' => 'Foo' ) ) ),
- array(),
- array(),
- array( 'sitelinks' => array( 'dewiki' => array(
'site' => 'dewiki', 'title' => 'Foo' ) ) ),
- ),
- array(
- array( 'claims' => array( 'P56' => array(
array( 'mainsnak' => array(
- 'snaktype' => 'value', 'property' =>
'P56', 'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' =>
'normal' ) ) ) ),
- array(),
- array(),
- array( 'claims' => array( array( 'mainsnak' =>
array(
- 'snaktype' => 'value', 'property' =>
'P56', 'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' =>
'normal' ) ) ),
- ),
- //check merges of elements work as expected
- array(
- array( 'aliases' => array( array( "language" =>
"nl", "value" => "Ali1" ) ) ),
- array( 'aliases' => array( array( "language" =>
"nl", "value" => "Ali2" ) ) ),
- array(),
- array( 'aliases' => array( array( "language" =>
"nl", "value" => "Ali2" ),array( "language" => "nl", "value" => "Ali1" ) ) ),
- ),
- array(
- array( 'claims' => array( 'P56' => array(
array( 'mainsnak' => array(
- 'snaktype' => 'value', 'property' =>
'P56', 'datavalue' => array( 'value' => 'imastring1', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' =>
'normal' ) ) ) ),
- array( 'claims' => array( 'P56' => array(
array( 'mainsnak' => array(
- 'snaktype' => 'value', 'property' =>
'P56', 'datavalue' => array( 'value' => 'imastring2', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' =>
'normal' ) ) ) ),
- array(),
- array( 'claims' => array(
- array( 'mainsnak' => array( 'snaktype'
=> 'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring2',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ),
- array( 'mainsnak' => array( 'snaktype'
=> 'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring1',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ) ) ),
- ),
- array(
- array( 'claims' => array( 'P56' => array(
array( 'mainsnak' => array(
- 'snaktype' => 'value', 'property' =>
'P56', 'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' =>
'normal' ) ) ) ),
- array( 'claims' => array( 'P56' => array(
array( 'mainsnak' => array(
- 'snaktype' => 'value', 'property' =>
'P56', 'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' =>
'normal' ) ) ) ),
- array(),
- array( 'claims' => array(
- array( 'mainsnak' => array( 'snaktype'
=> 'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ),
- array( 'mainsnak' => array( 'snaktype'
=> 'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ) ) ),
- ),
+ $testCases = array();
+ $testCases['labelMerge'] = array(
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'foo' ) ) ),
+ array(),
+ array(),
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'foo' ) ) ),
);
+ $testCases['identicalLabelMerge'] = array(
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'foo' ) ) ),
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'foo' ) ) ),
+ array(),
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'foo' ) ) ),
+ );
+ $testCases['ignoreConflictLabelMerge'] = array(
+ array( 'labels' => array(
+ 'en' => array( 'language' => 'en', 'value' =>
'foo' ),
+ 'de' => array( 'language' => 'de', 'value' =>
'berlin' )
+ ) ),
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'bar' ) ) ),
+ array( 'labels' => array( 'en' => array( 'language' =>
'en', 'value' => 'foo' ) ) ),
+ array( 'labels' => array(
+ 'en' => array( 'language' => 'en', 'value' =>
'bar' ),
+ 'de' => array( 'language' => 'de', 'value' =>
'berlin' )
+ ) ),
+ 'label'
+ );
+ $testCases['descriptionMerge'] = array(
+ array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
+ array(),
+ array(),
+ array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
+ );
+ $testCases['identicalDescriptionMerge'] = array(
+ array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
+ array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
+ array(),
+ array( 'descriptions' => array( 'de' => array(
'language' => 'de', 'value' => 'foo' ) ) ),
+ );
+ $testCases['ignoreConflictDescriptionMerge'] = array(
+ array( 'descriptions' => array(
+ 'en' => array( 'language' => 'en', 'value' =>
'foo' ),
+ 'de' => array( 'language' => 'de', 'value' =>
'berlin' )
+ ) ),
+ array( 'descriptions' => array( 'en' => array(
'language' => 'en', 'value' => 'bar' ) ) ),
+ array( 'descriptions' => array( 'en' => array(
'language' => 'en', 'value' => 'foo' ) ) ),
+ array( 'descriptions' => array(
+ 'en' => array( 'language' => 'en', 'value' =>
'bar' ),
+ 'de' => array( 'language' => 'de', 'value' =>
'berlin' )
+ ) ),
+ 'description'
+ );
+ $testCases['aliasesMerge'] = array(
+ array( 'aliases' => array( array( "language" => "nl",
"value" => "Dickes B" ) ) ),
+ array(),
+ array(),
+ array( 'aliases' => array( array( "language" => "nl",
"value" => "Dickes B" ) ) ),
+ );
+ $testCases[] = array(
+ array( 'aliases' => array( array( "language" => "nl",
"value" => "Ali1" ) ) ),
+ array( 'aliases' => array( array( "language" => "nl",
"value" => "Ali2" ) ) ),
+ array(),
+ array( 'aliases' => array( array( "language" => "nl",
"value" => "Ali2" ),array( "language" => "nl", "value" => "Ali1" ) ) ),
+ );
+ $testCases['sitelinksMerge'] = array(
+ array( 'sitelinks' => array( 'dewiki' => array( 'site'
=> 'dewiki', 'title' => 'Foo' ) ) ),
+ array(),
+ array(),
+ array( 'sitelinks' => array( 'dewiki' => array( 'site'
=> 'dewiki', 'title' => 'Foo' ) ) ),
+ );
+ $testCases['claimMerge'] = array(
+ array( 'claims' => array( 'P56' => array( array(
'mainsnak' => array(
+ 'snaktype' => 'value', 'property' => 'P56',
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement', 'rank' => 'normal' ) ) )
),
+ array(),
+ array(),
+ array( 'claims' => array( array( 'mainsnak' => array(
+ 'snaktype' => 'value', 'property' => 'P56',
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement', 'rank' => 'normal' ) ) ),
+ );
+ $testCases['claimMerge'] = array(
+ array( 'claims' => array( 'P56' => array( array(
'mainsnak' => array(
+ 'snaktype' => 'value', 'property' => 'P56',
'datavalue' => array( 'value' => 'imastring1', 'type' => 'string' ) ),
+ 'type' => 'statement', 'rank' => 'normal' ) ) )
),
+ array( 'claims' => array( 'P56' => array( array(
'mainsnak' => array(
+ 'snaktype' => 'value', 'property' => 'P56',
'datavalue' => array( 'value' => 'imastring2', 'type' => 'string' ) ),
+ 'type' => 'statement', 'rank' => 'normal' ) ) )
),
+ array(),
+ array( 'claims' => array(
+ array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring2',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ),
+ array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring1',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ) ) ),
+ );
+ //Identical claims should not be replaced but duplicated instead
+ $testCases['identicalClaimMerge'] = array(
+ array( 'claims' => array( 'P56' => array( array(
'mainsnak' => array(
+ 'snaktype' => 'value', 'property' => 'P56',
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement', 'rank' => 'normal' ) ) )
),
+ array( 'claims' => array( 'P56' => array( array(
'mainsnak' => array(
+ 'snaktype' => 'value', 'property' => 'P56',
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement', 'rank' => 'normal' ) ) )
),
+ array(),
+ array( 'claims' => array(
+ array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ),
+ array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => 'P56', 'datavalue' => array( 'value' => 'imastring',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ) ) ),
+ );
+ return $testCases;
}
/**
* @dataProvider provideData
*/
- function testMergeRequest( $pre1, $pre2, $expected1, $expected2 ){
+ function testMergeRequest( $pre1, $pre2, $expected1, $expected2,
$ignoreConflicts = null ){
+ // -- set up params ---------------------------------
+ $params = array(
+ 'action' => 'wbmergeitems',
+ 'fromid' => EntityTestHelper::getId( 'Empty' ),
+ 'toid' => EntityTestHelper::getId( 'Empty2' ),
+ 'summary' => 'CustomSummary!',
+ );
+ if( $ignoreConflicts !== null ){
+ $params['ignoreconflicts'] = $ignoreConflicts;
+ }
// -- prefill the entities
--------------------------------------------
$this->doApiRequestWithToken( array(
'action' => 'wbeditentity',
@@ -143,12 +179,7 @@
'data' => json_encode( $pre2 ) ) );
// -- do the request
--------------------------------------------
- list( $result,, ) = $this->doApiRequestWithToken( array(
- 'action' => 'wbmergeitems',
- 'fromid' => EntityTestHelper::getId( 'Empty' ),
- 'toid' => EntityTestHelper::getId( 'Empty2' ),
- 'summary' => 'CustomSummary!',
- ) );
+ list( $result,, ) = $this->doApiRequestWithToken( $params );
// -- check the result
--------------------------------------------
$this->assertResultSuccess( $result );
@@ -198,6 +229,12 @@
array( //7 to id is property
'p' => array( 'fromid' => 'q999', 'toid' =>
'p56' ),
'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'not-item' ) ) ),
+ array( //8 bad ignoreconficts (GETVALIDID is replaced
by a valid id)
+ 'p' => array( 'fromid' => 'GETVALIDID', 'toid'
=> 'GETVALIDID', 'ignoreconflicts' => 'foo' ),
+ 'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid' ) ) ),
+ array( //9 bad ignoreconficts (GETVALIDID is replaced
by a valid id)
+ 'p' => array( 'fromid' => 'GETVALIDID', 'toid'
=> 'GETVALIDID', 'ignoreconflicts' => 'label|foo' ),
+ 'e' => array( 'exception' => array( 'type' =>
'UsageException', 'code' => 'param-invalid' ) ) ),
);
}
@@ -207,6 +244,12 @@
public function testMergeItemsParamsExceptions( $params, $expected ){
// -- set any defaults ------------------------------------
$params['action'] = 'wbmergeitems';
+ if( isset( $params['from'] ) && $params['from'] ===
'GETVALIDID' ){
+ $params['from'] = EntityTestHelper::getId( 'Empty' );
+ }
+ if( isset( $params['to'] ) && $params['to'] === 'GETVALIDID' ){
+ $params['to'] = EntityTestHelper::getId( 'Empty2' );
+ }
$this->doTestQueryExceptions( $params, $expected['exception'] );
}
--
To view, visit https://gerrit.wikimedia.org/r/94328
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I03684ea387e818641d26cda9f1613822bc5e2534
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
