jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/376781 )
Change subject: Diffing and Patching Lexeme forms and nextFormId
......................................................................
Diffing and Patching Lexeme forms and nextFormId
Change-Id: Icfa5d5bfcbe7e48cab9f1d4c99eeba1bd656d39f
---
M src/DataModel/Services/Diff/LexemeDiff.php
M src/DataModel/Services/Diff/LexemeDiffer.php
M src/DataModel/Services/Diff/LexemePatcher.php
M tests/phpunit/composer/DataModel/Services/Diff/LexemeDifferPatcherTest.php
4 files changed, 259 insertions(+), 3 deletions(-)
Approvals:
WMDE-leszek: Looks good to me, approved
jenkins-bot: Verified
diff --git a/src/DataModel/Services/Diff/LexemeDiff.php
b/src/DataModel/Services/Diff/LexemeDiff.php
index ee9f4e3..cf452bd 100644
--- a/src/DataModel/Services/Diff/LexemeDiff.php
+++ b/src/DataModel/Services/Diff/LexemeDiff.php
@@ -20,6 +20,7 @@
* @param DiffOp[] $operations
*/
public function __construct( array $operations = [] ) {
+ //TODO Probably can be removed. Does it do anything useful?
$this->fixSubstructureDiff( $operations, 'lemmas' );
$this->fixSubstructureDiff( $operations, 'lexicalCategory' );
$this->fixSubstructureDiff( $operations, 'language' );
@@ -56,13 +57,24 @@
}
/**
+ * @return Diff
+ */
+ public function getFormsDiff() {
+ return isset( $this['forms'] ) ? $this['forms'] : new Diff( [],
true );
+ }
+
+ /**
* Returns if there are any changes (equivalent to: any differences
between the entities).
*
* @return bool
*/
public function isEmpty() {
return $this->getLemmasDiff()->isEmpty()
- && $this->getClaimsDiff()->isEmpty();
+ && $this->getClaimsDiff()->isEmpty();
+ }
+
+ public function getNextFormIdDiff() {
+ return isset( $this['nextFormId'] ) ? $this['nextFormId'] : new
Diff( [], true );
}
}
diff --git a/src/DataModel/Services/Diff/LexemeDiffer.php
b/src/DataModel/Services/Diff/LexemeDiffer.php
index 374c147..f33531f 100644
--- a/src/DataModel/Services/Diff/LexemeDiffer.php
+++ b/src/DataModel/Services/Diff/LexemeDiffer.php
@@ -3,11 +3,14 @@
namespace Wikibase\Lexeme\DataModel\Services\Diff;
use Diff\Differ\MapDiffer;
+use Diff\DiffOp\Diff\Diff;
+use Diff\DiffOp\DiffOpChange;
use UnexpectedValueException;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Services\Diff\EntityDiff;
use Wikibase\DataModel\Services\Diff\EntityDifferStrategy;
use Wikibase\DataModel\Services\Diff\StatementListDiffer;
+use Wikibase\Lexeme\DataModel\Form;
use Wikibase\Lexeme\DataModel\Lexeme;
use Wikimedia\Assert\Assert;
use InvalidArgumentException;
@@ -27,9 +30,15 @@
*/
private $recursiveMapDiffer;
+ /**
+ * @var FormDiffer
+ */
+ private $formDiffer;
+
public function __construct() {
$this->recursiveMapDiffer = new MapDiffer( true );
$this->statementListDiffer = new StatementListDiffer();
+ $this->formDiffer = new FormDiffer();
}
/**
@@ -73,6 +82,13 @@
$diffOps['claim'] = $this->statementListDiffer->getDiff(
$from->getStatements(),
$to->getStatements()
+ );
+
+ $diffOps['nextFormId'] = $this->getNextFormIdCounterDiff(
$from, $to );
+
+ $diffOps['forms'] = $this->getFormsDiff(
+ $from->getForms(),
+ $to->getForms()
);
return new LexemeDiff( $diffOps );
@@ -132,4 +148,55 @@
return $this->diffEntities( $entity, new Lexeme() );
}
+ /**
+ * @param Form[] $from
+ * @param Form[] $to
+ *
+ * @return Diff;
+ */
+ private function getFormsDiff( array $from, array $to ) {
+ $differ = new MapDiffer();
+
+ $differ->setComparisonCallback( function ( Form $from, Form $to
) {
+ return $from == $to;
+ } );
+
+ $from = $this->toFormsDiffArray( $from );
+ $to = $this->toFormsDiffArray( $to );
+ $formDiffOps = $differ->doDiff( $from, $to );
+
+ foreach ( $formDiffOps as $index => $formDiffOp ) {
+ if ( $formDiffOp instanceof DiffOpChange ) {
+ /** @var DiffOpChange $formDiffOp */
+ $formDiffOps[$index] = $this->formDiffer->diff(
+ $formDiffOp->getOldValue(),
+ $formDiffOp->getNewValue()
+ );
+
+ }
+ }
+
+ return new Diff( $formDiffOps, true );
+ }
+
+ /**
+ * @param Form[] $forms
+ */
+ private function toFormsDiffArray( array $forms ) {
+ $result = [];
+ foreach ( $forms as $form ) {
+ $result[$form->getId()->getSerialization()] = $form;
+ }
+
+ return $result;
+ }
+
+ private function getNextFormIdCounterDiff( Lexeme $from, Lexeme $to ) {
+ if ( $to->getNextFormId() <= $from->getNextFormId() ) {
+ return new Diff( [] );
+ }
+
+ return new Diff( [ new DiffOpChange( $from->getNextFormId(),
$to->getNextFormId() ) ] );
+ }
+
}
diff --git a/src/DataModel/Services/Diff/LexemePatcher.php
b/src/DataModel/Services/Diff/LexemePatcher.php
index 0ec031c..1c20b2c 100644
--- a/src/DataModel/Services/Diff/LexemePatcher.php
+++ b/src/DataModel/Services/Diff/LexemePatcher.php
@@ -6,6 +6,7 @@
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
+use Diff\Patcher\ListPatcher;
use Diff\Patcher\PatcherException;
use InvalidArgumentException;
use Wikibase\DataModel\Entity\EntityDocument;
@@ -15,7 +16,9 @@
use Wikibase\DataModel\Services\Diff\StatementListPatcher;
use Wikibase\DataModel\Services\Diff\TermListPatcher;
use Wikibase\DataModel\Term\TermList;
+use Wikibase\Lexeme\DataModel\Form;
use Wikibase\Lexeme\DataModel\Lexeme;
+use Wikibase\Lexeme\DataModel\LexemePatchAccess;
use Wikimedia\Assert\Assert;
/**
@@ -24,7 +27,7 @@
* @author Thiemo Mättig
*/
class LexemePatcher implements EntityPatcherStrategy {
-
+ private $formPatcher;
/**
* @var TermListPatcher
*/
@@ -38,6 +41,7 @@
public function __construct() {
$this->termListPatcher = new TermListPatcher();
$this->statementListPatcher = new StatementListPatcher();
+ $this->formPatcher = new FormPatcher();
}
/**
@@ -57,9 +61,11 @@
*/
public function patchEntity( EntityDocument $entity, EntityDiff $patch
) {
Assert::parameterType( Lexeme::class, $entity, '$entity' );
-
+ Assert::parameterType( LexemeDiff::class, $patch, '$patch' );
/** @var Lexeme $entity */
/** @var LexemeDiff $patch */
+
+ //TODO Lemmas can't be null. Redundant check
$lemmas = $entity->getLemmas() !== null ? $entity->getLemmas()
: new TermList();
$this->termListPatcher->patchTermList(
$lemmas,
@@ -81,6 +87,10 @@
if ( $itemId !== false ) {
$entity->setLanguage( $itemId );
}
+
+ $this->patchNextFormId( $entity, $patch );
+
+ $this->patchForms( $entity, $patch );
}
/**
@@ -112,4 +122,57 @@
throw new PatcherException( 'Invalid ItemId diff' );
}
+ private function patchNextFormId( Lexeme $entity, LexemeDiff $patch ) {
+ $nextFormIdDiffList = $patch->getNextFormIdDiff();
+ foreach ( $nextFormIdDiffList as $nextFormIdDiff ) {
+ switch ( get_class( $nextFormIdDiff ) ) {
+ case DiffOpChange::class:
+ /** @var DiffOpChange $nextFormIdDiff */
+ $newNumber =
$nextFormIdDiff->getNewValue();
+ if ( $newNumber >
$entity->getNextFormId() ) {
+ $entity->patch( function (
LexemePatchAccess $patchAccess ) use ( $newNumber ) {
+
$patchAccess->increaseNextFormIdTo( $newNumber );
+ } );
+ }
+ break;
+ default:
+ throw new PatcherException( 'Invalid
forms list diff' );
+ }
+ }
+ }
+
+ private function patchForms( Lexeme $entity, LexemeDiff $patch ) {
+ $formsDiff = $patch->getFormsDiff();
+ foreach ( $formsDiff as $formDiff ) {
+ switch ( get_class( $formDiff ) ) {
+ case DiffOpAdd::class:
+ /** @var DiffOpAdd $formDiff */
+ /** @var Form $form */
+ $form = $formDiff->getNewValue();
+ $entity->patch(
+ function ( LexemePatchAccess
$patchAccess ) use ( $form ) {
+ $patchAccess->addForm(
$form );
+ }
+ );
+ break;
+ case DiffOpRemove::class:
+ /** @var DiffOpRemove $formDiff */
+ /** @var Form $form */
+ $form = $formDiff->getOldValue();
+ $entity->removeForm( $form->getId() );
+ break;
+ case ChangeFormDiffOp::class:
+ /** @var ChangeFormDiffOp $formDiff */
+ /** @var Form $form */
+ //TODO: This implementation is
incomplete/incorrect.
+ //TODO: Proper implementation of Forms
patching is needed
+ $form = $entity->getForms()[0];
+ $this->formPatcher->patch( $form,
$formDiff );
+ break;
+ default:
+ throw new PatcherException( 'Invalid
forms list diff: ' . get_class( $formDiff ) );
+ }
+ }
+ }
+
}
diff --git
a/tests/phpunit/composer/DataModel/Services/Diff/LexemeDifferPatcherTest.php
b/tests/phpunit/composer/DataModel/Services/Diff/LexemeDifferPatcherTest.php
index 92325fb..9632933 100644
--- a/tests/phpunit/composer/DataModel/Services/Diff/LexemeDifferPatcherTest.php
+++ b/tests/phpunit/composer/DataModel/Services/Diff/LexemeDifferPatcherTest.php
@@ -2,12 +2,17 @@
namespace Wikibase\Lexeme\Tests\DataModel\Services\Diff;
+use Wikibase\DataModel\Entity\ItemId;
+use Wikibase\Lexeme\DataModel\Form;
+use Wikibase\Lexeme\DataModel\FormId;
use Eris\Facade;
use Wikibase\Lexeme\DataModel\Lexeme;
use Wikibase\Lexeme\DataModel\LexemeId;
use Wikibase\Lexeme\DataModel\Services\Diff\LexemeDiffer;
use Wikibase\Lexeme\DataModel\Services\Diff\LexemePatcher;
use Wikibase\Lexeme\Tests\ErisGenerators\WikibaseLexemeGenerators;
+use Wikibase\Lexeme\Tests\DataModel\NewForm;
+use Wikibase\Lexeme\Tests\DataModel\NewLexeme;
/**
* @covers \Wikibase\Lexeme\DataModel\Services\Diff\LexemeDiffer
@@ -45,4 +50,113 @@
} );
}
+ public function testAddedFormIsDiffedAndPatched() {
+ $differ = new LexemeDiffer();
+ $patcher = new LexemePatcher();
+ $initialLexeme = NewLexeme::create();
+ $lexemeWithoutForm = $initialLexeme->build();
+ $lexemeWithForm = $initialLexeme->withForm( NewForm::havingId(
'F1' ) )->build();
+
+ $diff = $differ->diffLexemes( $lexemeWithoutForm,
$lexemeWithForm );
+ $patcher->patchEntity( $lexemeWithoutForm, $diff );
+
+ $this->assertTrue(
+ $lexemeWithoutForm->equals( $lexemeWithForm ),
+ "Lexemes are not equal"
+ );
+ }
+
+ public function testRemovedFormIsDiffedAndPatched() {
+ $differ = new LexemeDiffer();
+ $patcher = new LexemePatcher();
+ $initialLexeme = NewLexeme::create();
+ $lexemeWithForm = $initialLexeme->withForm( NewForm::havingId(
'F1' ) )->build();
+ $lexemeWithoutForm = $initialLexeme->build();
+
+ $diff = $differ->diffLexemes( $lexemeWithForm,
$lexemeWithoutForm );
+ $patcher->patchEntity( $lexemeWithForm, $diff );
+
+ $this->assertEquals( [], $lexemeWithForm->getForms() );
+ }
+
+ public function testDiffAndPatchCanIncreaseNextFormIdCounter() {
+ $differ = new LexemeDiffer();
+ $patcher = new LexemePatcher();
+ $initialLexeme = NewLexeme::create();
+ $lexemeWithoutForm = $initialLexeme->build();
+ $lexemeThatHadForm = $initialLexeme->withForm(
NewForm::havingId( 'F1' ) )->build();
+ $lexemeThatHadForm->removeForm( new FormId( 'F1' ) );
+
+ $diff = $differ->diffLexemes( $lexemeWithoutForm,
$lexemeThatHadForm );
+ $patcher->patchEntity( $lexemeWithoutForm, $diff );
+
+ $this->assertEquals(
+ $lexemeThatHadForm->getNextFormId(),
+ $lexemeWithoutForm->getNextFormId()
+ );
+ $this->assertTrue(
+ $lexemeWithoutForm->equals( $lexemeThatHadForm ),
+ "Lexemes are not equal"
+ );
+ }
+
+ public function testDiffAndPatchCanChangeExistingFormRepresentations() {
+ $differ = new LexemeDiffer();
+ $patcher = new LexemePatcher();
+ $initialLexeme = NewLexeme::create();
+ $lexeme1 = $initialLexeme
+ ->withForm(
+ NewForm::havingId( 'F1' )
+ ->andRepresentation( 'en', 'cat' )
+ )->build();
+ $lexeme2 = $initialLexeme
+ ->withForm(
+ NewForm::havingId( 'F1' )
+ ->andRepresentation( 'en', 'goat' )
+ )->build();
+
+ $diff = $differ->diffLexemes( $lexeme1, $lexeme2 );
+ $patcher->patchEntity( $lexeme1, $diff );
+
+ $this->assertTrue(
+ $lexeme1->equals( $lexeme2 ),
+ "Lexemes are not equal"
+ );
+ }
+
+ public function
testDiffAndPatchCanAtomicallyChangeExistingFormRepresentations() {
+ $differ = new LexemeDiffer();
+ $patcher = new LexemePatcher();
+ $initialLexeme = NewLexeme::create();
+ $lexeme1 = $initialLexeme
+ ->withForm(
+ NewForm::havingId( 'F1' )
+ ->andRepresentation( 'en', 'en-value' )
+ )->build();
+ $lexeme2 = $initialLexeme
+ ->withForm(
+ NewForm::havingId( 'F1' )
+ ->andRepresentation( 'en', 'en-value' )
+ ->andRepresentation( 'fr', 'fr-value' )
+ )->build();
+ $latestLexeme = $initialLexeme
+ ->withForm(
+ NewForm::havingId( 'F1' )
+ ->andRepresentation( 'de', 'de-value' )
+ )->build();
+
+ $diff = $differ->diffLexemes( $lexeme1, $lexeme2 );
+ $patcher->patchEntity( $latestLexeme, $diff );
+
+ $form = $latestLexeme->getForms()[0];
+ $this->assertEquals(
+ 'fr-value',
+ $form->getRepresentations()->getByLanguage( 'fr'
)->getText()
+ );
+ $this->assertEquals(
+ 'de-value',
+ $form->getRepresentations()->getByLanguage( 'de'
)->getText()
+ );
+ }
+
}
--
To view, visit https://gerrit.wikimedia.org/r/376781
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Icfa5d5bfcbe7e48cab9f1d4c99eeba1bd656d39f
Gerrit-PatchSet: 17
Gerrit-Project: mediawiki/extensions/WikibaseLexeme
Gerrit-Branch: master
Gerrit-Owner: Aleksey Bekh-Ivanov (WMDE) <aleksey.bekh-iva...@wikimedia.de>
Gerrit-Reviewer: Aleksey Bekh-Ivanov (WMDE) <aleksey.bekh-iva...@wikimedia.de>
Gerrit-Reviewer: Jonas Kress (WMDE) <jonas.kr...@wikimedia.de>
Gerrit-Reviewer: Thiemo Mättig (WMDE) <thiemo.maet...@wikimedia.de>
Gerrit-Reviewer: WMDE-leszek <leszek.mani...@wikimedia.de>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
