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 <addshorew...@gmail.com> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits