Thiemo Mättig (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/328374 )
Change subject: Rewrite ItemId patching in LexemePatcher
......................................................................
Rewrite ItemId patching in LexemePatcher
This avoids calling methods dynamically.
Bug: T153675
Change-Id: Ib504a2cb302d298fec6c76174a03fe1a6ddb25b4
---
M src/DataModel/Lexeme.php
M src/DataModel/Services/Diff/LexemePatcher.php
M tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
3 files changed, 49 insertions(+), 43 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseLexeme
refs/changes/74/328374/1
diff --git a/src/DataModel/Lexeme.php b/src/DataModel/Lexeme.php
index 6b04cd7..3cf81f0 100644
--- a/src/DataModel/Lexeme.php
+++ b/src/DataModel/Lexeme.php
@@ -238,7 +238,7 @@
/**
* @param ItemId|null $lexicalCategory
*/
- public function setLexicalCategory( $lexicalCategory ) {
+ public function setLexicalCategory( ItemId $lexicalCategory = null ) {
$this->lexicalCategory = $lexicalCategory;
}
@@ -252,7 +252,7 @@
/**
* @param ItemId|null $language
*/
- public function setLanguage( $language ) {
+ public function setLanguage( ItemId $language = null ) {
$this->language = $language;
}
diff --git a/src/DataModel/Services/Diff/LexemePatcher.php
b/src/DataModel/Services/Diff/LexemePatcher.php
index f36279d..d2a4571 100644
--- a/src/DataModel/Services/Diff/LexemePatcher.php
+++ b/src/DataModel/Services/Diff/LexemePatcher.php
@@ -3,7 +3,6 @@
namespace Wikibase\Lexeme\DataModel\Services\Diff;
use Diff\DiffOp\Diff\Diff;
-use Diff\DiffOp\DiffOp;
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
@@ -22,6 +21,7 @@
/**
* @license GPL-2.0+
* @author Amir Sarabadani <[email protected]>
+ * @author Thiemo Mättig
*/
class LexemePatcher implements EntityPatcherStrategy {
@@ -72,50 +72,44 @@
$patch->getClaimsDiff()
);
- $this->patchItemId(
- $entity,
- $patch->getLexicalCategoryDiff(),
- 'setLexicalCategory',
- 'lexical category'
- );
+ $itemId = $this->getPatchedItemId(
$patch->getLexicalCategoryDiff() );
+ if ( $itemId !== false ) {
+ $entity->setLexicalCategory( $itemId );
+ }
- $this->patchItemId(
- $entity,
- $patch->getLanguageDiff(),
- 'setLanguage',
- 'language'
- );
+ $itemId = $this->getPatchedItemId( $patch->getLanguageDiff() );
+ if ( $itemId !== false ) {
+ $entity->setLanguage( $itemId );
+ }
}
- private function patchItemId( Lexeme $lexeme, Diff $patch, $setMethod,
$attrName ) {
- /** @var DiffOp $diffOp */
- foreach ( $patch as $diffOp ) {
- switch ( true ) {
- case $diffOp instanceof DiffOpAdd:
- /** @var DiffOpAdd $diffOp */
- call_user_func(
- [ $lexeme, $setMethod ],
- new ItemId(
$diffOp->getNewValue() )
- );
- break;
-
- case $diffOp instanceof DiffOpChange:
- /** @var DiffOpAdd $diffOp */
- call_user_func(
- [ $lexeme, $setMethod ],
- new ItemId(
$diffOp->getNewValue() )
- );
- break;
-
- case $diffOp instanceof DiffOpRemove:
- /** @var DiffOpRemove $diffOp */
- $lexeme->setLanguage( null );
- break;
-
- default:
- throw new PatcherException( "Invalid
$attrName diff" );
- }
+ /**
+ * @param Diff $patch
+ *
+ * @throws PatcherException
+ * @return ItemId|null|false False in case the diff is valid, but does
not contain a change.
+ */
+ private function getPatchedItemId( Diff $patch ) {
+ if ( $patch->isEmpty() ) {
+ return false;
}
+
+ $diffOp = reset( $patch );
+
+ switch ( true ) {
+ case $diffOp instanceof DiffOpAdd:
+ /** @var DiffOpAdd $diffOp */
+ return new ItemId( $diffOp->getNewValue() );
+
+ case $diffOp instanceof DiffOpChange:
+ /** @var DiffOpChange $diffOp */
+ return new ItemId( $diffOp->getNewValue() );
+
+ case $diffOp instanceof DiffOpRemove:
+ return null;
+ }
+
+ throw new PatcherException( 'Invalid ItemId diff' );
}
}
diff --git
a/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
b/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
index af534d8..13f019a 100644
--- a/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
+++ b/tests/phpunit/composer/DataModel/Services/Diff/LexemePatcherTest.php
@@ -144,4 +144,16 @@
$this->assertTrue( $expected->equals( $lexeme ) );
}
+ public function testElementsNotInDiffAreNotRemoved() {
+ $lexeme = new Lexeme();
+ $lexeme->setLexicalCategory( new ItemId( 'Q1' ) );
+ $lexeme->setLanguage( new ItemId( 'Q2' ) );
+
+ $patcher = new LexemePatcher();
+ $patcher->patchEntity( $lexeme, new LexemeDiff() );
+
+ $this->assertNotNull( $lexeme->getLexicalCategory() );
+ $this->assertNotNull( $lexeme->getLanguage() );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/328374
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib504a2cb302d298fec6c76174a03fe1a6ddb25b4
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseLexeme
Gerrit-Branch: master
Gerrit-Owner: Thiemo Mättig (WMDE) <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
