Thiemo Kreuz (WMDE) has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/405697 )
Change subject: Fix incomplete Form diffing not covering adding/removal of Forms
......................................................................
Fix incomplete Form diffing not covering adding/removal of Forms
Forms can not only change, Forms can also be added and removed. This
was just not covered by the code yet. The diff failed hard with quite
extreme exceptions.
This becomes obvious very easily if you try to compare two different
Lexemes, e.g. …?oldid=…&diff=… with two revision IDs from two different
Lexemes. (Which is a quite typical use case, e.g. if two entities are
duplicates and the user wants to compare the two.)
Bug: T185477
Change-Id: I9ba8622f5b060d76a7915d9bd74383b39d76fb3f
---
M src/DataModel/Services/Diff/FormDiffer.php
M src/DataModel/Services/Diff/LexemeDiffer.php
M src/Diff/FormDiffView.php
3 files changed, 58 insertions(+), 24 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/WikibaseLexeme
refs/changes/97/405697/1
diff --git a/src/DataModel/Services/Diff/FormDiffer.php
b/src/DataModel/Services/Diff/FormDiffer.php
index f869063..6f2fa4e 100644
--- a/src/DataModel/Services/Diff/FormDiffer.php
+++ b/src/DataModel/Services/Diff/FormDiffer.php
@@ -4,13 +4,15 @@
use Diff\Differ\MapDiffer;
use Diff\DiffOp\Diff\Diff;
-use DomainException;
use InvalidArgumentException;
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\DataModel\Term\Term;
+use Wikibase\DataModel\Term\TermList;
use Wikibase\Lexeme\DataModel\Form;
+use Wikibase\Lexeme\DataModel\FormId;
/**
* @license GPL-2.0+
@@ -42,8 +44,8 @@
}
/**
- * @param EntityDocument $from
- * @param EntityDocument $to
+ * @param Form $from
+ * @param Form $to
*
* @return EntityDiff
* @throws InvalidArgumentException
@@ -57,30 +59,40 @@
}
/**
- * @param EntityDocument $entity
+ * @param Form $form
*
* @return EntityDiff
* @throws InvalidArgumentException
*/
- public function getConstructionDiff( EntityDocument $entity ) {
- throw new DomainException( 'Forms aren\'t stored as separate
wiki pages, and can only show '
- . 'up in regular diffs that add or remove a Form' );
+ public function getConstructionDiff( EntityDocument $form ) {
+ if ( !( $form instanceof Form ) ) {
+ throw new InvalidArgumentException( 'Can only diff
Forms' );
+ }
+
+ return $this->diff( $this->newEmptyForm( $form->getId() ),
$form );
}
/**
- * @param EntityDocument $entity
+ * @param Form $form
*
* @return EntityDiff
* @throws InvalidArgumentException
*/
- public function getDestructionDiff( EntityDocument $entity ) {
- throw new DomainException( 'Forms aren\'t stored as separate
wiki pages, and can only show '
- . 'up in regular diffs that add or remove a Form' );
+ public function getDestructionDiff( EntityDocument $form ) {
+ if ( !( $form instanceof Form ) ) {
+ throw new InvalidArgumentException( 'Can only diff
Forms' );
+ }
+
+ return $this->diff( $form, $this->newEmptyForm( $form->getId()
) );
+ }
+
+ private function newEmptyForm( FormId $id ) {
+ $form = new Form( $id, new TermList( [ new Term( 'dummy',
'dummy' ) ] ), [] );
+ $form->getRepresentations()->clear();
+ return $form;
}
/**
- * @deprecated use self::diffEntities instead
- *
* @param Form $old
* @param Form $new
*
diff --git a/src/DataModel/Services/Diff/LexemeDiffer.php
b/src/DataModel/Services/Diff/LexemeDiffer.php
index 1494c7e..e59fdce 100644
--- a/src/DataModel/Services/Diff/LexemeDiffer.php
+++ b/src/DataModel/Services/Diff/LexemeDiffer.php
@@ -4,7 +4,10 @@
use Diff\Differ\MapDiffer;
use Diff\DiffOp\Diff\Diff;
+use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
+use Diff\DiffOp\DiffOpRemove;
+use LogicException;
use UnexpectedValueException;
use Wikibase\DataModel\Entity\EntityDocument;
use Wikibase\DataModel\Services\Diff\EntityDiff;
@@ -153,7 +156,7 @@
* @param FormSet $from
* @param FormSet $to
*
- * @return Diff;
+ * @return Diff
*/
private function getFormsDiff( FormSet $from, FormSet $to ) {
$differ = new MapDiffer();
@@ -167,13 +170,28 @@
$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()
- );
+ switch ( true ) {
+ case $formDiffOp instanceof DiffOpChange:
+ $formDiffOps[$index] =
$this->formDiffer->diffEntities(
+ $formDiffOp->getOldValue(),
+ $formDiffOp->getNewValue()
+ );
+ break;
+ case $formDiffOp instanceof DiffOpAdd:
+ $formDiffOps[$index] =
$this->formDiffer->getConstructionDiff(
+ $formDiffOp->getNewValue()
+ );
+ break;
+
+ case $formDiffOp instanceof DiffOpRemove:
+ $formDiffOps[$index] =
$this->formDiffer->getDestructionDiff(
+ $formDiffOp->getOldValue()
+ );
+ break;
+
+ default:
+ throw new LogicException( 'Invalid form
diff' );
}
}
diff --git a/src/Diff/FormDiffView.php b/src/Diff/FormDiffView.php
index 1d82223..46484bb 100644
--- a/src/Diff/FormDiffView.php
+++ b/src/Diff/FormDiffView.php
@@ -65,22 +65,26 @@
* @throws MWException
*/
protected function generateOpHtml( array $path, DiffOp $op ) {
- $html = '';
if ( $op->isAtomic() ) {
return parent::generateOpHtml( $path, $op );
}
+
+ $html = '';
+
foreach ( $op as $key => $subOp ) {
- if ( !$subOp instanceof ChangeFormDiffOp ) {
- $html .= $this->generateOpHtml( array_merge(
$path, [ $key ] ), $subOp );
- } else {
+ if ( $subOp instanceof ChangeFormDiffOp ) {
$html .= $this->generateFormOpHtml( $path,
$subOp, $key );
+ } else {
+ $html .= $this->generateOpHtml( array_merge(
$path, [ $key ] ), $subOp );
}
}
+
return $html;
}
private function generateFormOpHtml( $path, ChangeFormDiffOp $op, $key
) {
$html = '';
+
foreach ( $op->getStatementsDiffOps() as $claimDiffOp ) {
$html .= $this->getClaimDiffHtml(
$claimDiffOp,
--
To view, visit https://gerrit.wikimedia.org/r/405697
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9ba8622f5b060d76a7915d9bd74383b39d76fb3f
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/WikibaseLexeme
Gerrit-Branch: master
Gerrit-Owner: Thiemo Kreuz (WMDE) <thiemo.kr...@wikimedia.de>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
