Addshore has uploaded a new change for review.
https://gerrit.wikimedia.org/r/106897
Change subject: Merge Reference is Statements are the same
......................................................................
Merge Reference is Statements are the same
If statements have the same main snak and qualifier
hashs when merging then rather than create a new
statement we should just merge the references accross!
Change-Id: I256c3e757f7bed8f2e3b731e9ad5f26d507690d3
---
M repo/includes/ChangeOp/ChangeOpReference.php
M repo/includes/ChangeOp/ChangeOpsMerge.php
M repo/tests/phpunit/includes/api/MergeItemsTest.php
M repo/tests/phpunit/includes/api/WikibaseApiTestCase.php
4 files changed, 91 insertions(+), 27 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Wikibase
refs/changes/97/106897/1
diff --git a/repo/includes/ChangeOp/ChangeOpReference.php
b/repo/includes/ChangeOp/ChangeOpReference.php
index b77b265..6756e29 100644
--- a/repo/includes/ChangeOp/ChangeOpReference.php
+++ b/repo/includes/ChangeOp/ChangeOpReference.php
@@ -55,7 +55,7 @@
*
* @param string $claimGuid
* @param Reference|null $reference
- * @param string $referenceHash
+ * @param string $referenceHash (if empty '' a new reference will be
created)
* @param int|null $index
*
* @throws \InvalidArgumentException
diff --git a/repo/includes/ChangeOp/ChangeOpsMerge.php
b/repo/includes/ChangeOp/ChangeOpsMerge.php
index 7eb5869..cbb97e3 100644
--- a/repo/includes/ChangeOp/ChangeOpsMerge.php
+++ b/repo/includes/ChangeOp/ChangeOpsMerge.php
@@ -3,6 +3,9 @@
namespace Wikibase\ChangeOp;
use InvalidArgumentException;
+use Wikibase\DataModel\Claim\Claim;
+use Wikibase\DataModel\Claim\Statement;
+use Wikibase\DataModel\Reference;
use Wikibase\ItemContent;
use Wikibase\Lib\ClaimGuidGenerator;
@@ -115,7 +118,7 @@
}
private function generateClaimsChangeOps() {
- foreach( $this->fromItemContent->getItem()->getClaims() as
$fromClaim ){
+ foreach( $this->fromItemContent->getItem()->getClaims() as
$fromClaim ) {
$this->fromChangeOps->add( new ChangeOpMainSnak(
$fromClaim->getGuid(),
null,
@@ -124,12 +127,44 @@
$toClaim = clone $fromClaim;
$toClaim->setGuid( null );
+ $claimMoved = false;
- $this->toChangeOps->add( new ChangeOpClaim(
- $toClaim ,
- new ClaimGuidGenerator(
$this->toItemContent->getItem()->getId() )
- ) );
+ if( $toClaim instanceof Statement ) {
+ $claimMoved = $this->getReferenceChangeOps(
$toClaim );
+ }
+
+ if( !$claimMoved ) {
+ $this->toChangeOps->add( new ChangeOpClaim(
+ $toClaim ,
+ new ClaimGuidGenerator(
$this->toItemContent->getItem()->getId() )
+ ) );
+ }
}
}
+ /**
+ * @param Statement $fromStatement
+ *
+ * @return bool Could be more the references?
+ */
+ private function getReferenceChangeOps( $fromStatement ) {
+ /** @var $claim Claim */
+ foreach( $this->toItemContent->getItem()->getClaims() as $claim
) {
+ $fromHash = $fromStatement->getMainSnak()->getHash() .
$fromStatement->getQualifiers()->getHash();
+ $toHash = $claim->getMainSnak()->getHash() .
$claim->getQualifiers()->getHash();
+ if( $toHash === $fromHash ) {
+ /** @var $reference Reference */
+ foreach ( $fromStatement->getReferences() as
$reference ) {
+ $this->toChangeOps->add( new
ChangeOpReference(
+ $claim->getGuid(),
+ $reference,
+ '' // empty hash will create a
new reference
+ ) );
+ }
+ return true;
+ }
+ }
+ return false;
+ }
+
}
\ No newline at end of file
diff --git a/repo/tests/phpunit/includes/api/MergeItemsTest.php
b/repo/tests/phpunit/includes/api/MergeItemsTest.php
index 4167628..49b21c2 100644
--- a/repo/tests/phpunit/includes/api/MergeItemsTest.php
+++ b/repo/tests/phpunit/includes/api/MergeItemsTest.php
@@ -2,9 +2,8 @@
namespace Wikibase\Test\Api;
-use Wikibase\DataModel\Entity\ItemId;
-use Wikibase\DataModel\Entity\PropertyId;
-use Wikibase\EntityId;
+use LogicException;
+use Wikibase\DataModel\Entity\EntityId;
use Wikibase\ItemContent;
use Wikibase\PropertyContent;
@@ -167,18 +166,43 @@
array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => '{Prop}', 'datavalue' => array( 'value' => 'imastring2',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ),
array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => '{Prop}', '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( '{Prop}' => array( array(
'mainsnak' => array(
- 'snaktype' => 'value', 'property' => '{Prop}',
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' => 'normal' ) ) )
),
- array( 'claims' => array( '{Prop}' => array( array(
'mainsnak' => array(
- 'snaktype' => 'value', 'property' => '{Prop}',
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' ) ),
- 'type' => 'statement', 'rank' => 'normal' ) ) )
),
- array(),
- array( 'claims' => array(
- array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => '{Prop}', 'datavalue' => array( 'value' => 'imastring',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ),
- array( 'mainsnak' => array( 'snaktype' =>
'value', 'property' => '{Prop}', 'datavalue' => array( 'value' => 'imastring',
'type' => 'string' ) ), 'type' => 'statement', 'rank' => 'normal' ) ) ),
+ //Identical claims (mainsnak and qualifiers) should merge
references
+ $testCases['identicalClaimMergeReferences'] = array(
+ array( 'claims' =>
+ array( '{Prop}' =>
+ array( array( 'mainsnak' =>
+ array( 'snaktype' => 'value',
'property' => '{Prop}', 'datavalue' =>
+ array( 'value' =>
'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement',
+ 'rank' => 'normal',
+ 'references' => array(
+ array( 'snaks' =>
array( '{Prop}' => array(
+ array(
'snaktype' => 'value',
+
'property' => '{Prop}',
+
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' )
+ ) ) ) ) ) ) ) ) ),
+ array( 'claims' =>
+ array( '{Prop}' =>
+ array( array( 'mainsnak' =>
+ array( 'snaktype' => 'value',
'property' => '{Prop}', 'datavalue' =>
+ array( 'value' =>
'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement',
+ 'rank' => 'normal'
+ ) ) ) ),
+ array(), //empty
+ array( 'claims' =>
+ array( '{Prop}' =>
+ array( array( 'mainsnak' =>
+ array( 'snaktype' => 'value',
'property' => '{Prop}', 'datavalue' =>
+ array( 'value' =>
'imastring', 'type' => 'string' ) ),
+ 'type' => 'statement',
+ 'rank' => 'normal',
+ 'references' => array(
+ array( 'snaks' =>
array( '{Prop}' => array(
+ array(
'snaktype' => 'value',
+
'property' => '{Prop}',
+
'datavalue' => array( 'value' => 'imastring', 'type' => 'string' )
+ ) ) ) ) ) ) ) ) ),
);
return $testCases;
}
@@ -186,11 +210,11 @@
/**
* @dataProvider provideData
*/
- function testMergeRequest( $pre1, $pre2, $expected1, $expected2,
$ignoreConflicts = null ){
+ function testMergeRequest( $pre1, $pre2, $expectedFrom, $expectedTo,
$ignoreConflicts = null ){
$this->injectIds( $pre1 );
$this->injectIds( $pre2 );
- $this->injectIds( $expected1 );
- $this->injectIds( $expected2 );
+ $this->injectIds( $expectedFrom );
+ $this->injectIds( $expectedTo );
// -- set up params ---------------------------------
$params = array(
@@ -229,8 +253,10 @@
$this->assertGreaterThan( 0, $result['to']['lastrevid'] );
// -- check the items
--------------------------------------------
- $this->assertEntityEquals( $expected1, $this->loadEntity(
$result['from']['id'] ) );
- $this->assertEntityEquals( $expected2, $this->loadEntity(
$result['to']['id'] ) );
+ $actualFrom = $this->loadEntity( $result['from']['id'] );
+ $this->assertEntityEquals( $expectedFrom, $actualFrom );
+ $actualTo = $this->loadEntity( $result['to']['id'] );
+ $this->assertEntityEquals( $expectedTo, $actualTo );
// -- check the edit summaries
--------------------------------------------
$this->assertRevisionSummary( array( 'wbmergeitems' ),
$result['from']['lastrevid'] );
@@ -341,10 +367,12 @@
* Applies self::$idMap to all data in the given data structure,
recursively.
*
* @param $data
+ *
+ * @throws LogicException
*/
protected function injectIds( &$data ) {
if ( !self::$hasSetup ) {
- throw new \LogicException( 'setUp() was not yet
completed.' );
+ throw new LogicException( 'setUp() was not yet
completed.' );
}
$idMap = array(
diff --git a/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php
b/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php
index 4dccfa0..fe30661 100644
--- a/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php
+++ b/repo/tests/phpunit/includes/api/WikibaseApiTestCase.php
@@ -279,6 +279,7 @@
unset( $data['id'] );
unset( $data['hash'] );
unset( $data['qualifiers-order'] );
+ unset( $data['snaks-order'] );
$this->assertArrayEquals( $exp, $data, false,
true );
}
}
--
To view, visit https://gerrit.wikimedia.org/r/106897
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I256c3e757f7bed8f2e3b731e9ad5f26d507690d3
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
