Skip to content

Commit

Permalink
Streamline FingerprintPatcher
Browse files Browse the repository at this point in the history
  • Loading branch information
@thiemowmde
thiemowmde committed Mar 9, 2016
1 parent 971b470 commit 7ce5b8a
Show file tree
Hide file tree
Showing 2 changed files with 243 additions and 66 deletions.
169 changes: 107 additions & 62 deletions src/Diff/Internal/FingerprintPatcher.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
namespace Wikibase\DataModel\Services\Diff\Internal;

use Diff\DiffOp\Diff\Diff;
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
use Diff\Patcher\MapPatcher;
use InvalidArgumentException;
use Diff\Patcher\PatcherException;
use Wikibase\DataModel\Services\Diff\EntityDiff;
use Wikibase\DataModel\Term\AliasGroup;
use Wikibase\DataModel\Term\AliasGroupList;
use Wikibase\DataModel\Term\Fingerprint;
use Wikibase\DataModel\Term\TermList;
Expand All @@ -16,89 +18,132 @@
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroendedauw@gmail.com >
* @author Thiemo Mättig
*/
class FingerprintPatcher {

/**
* @var MapPatcher
*/
private $patcher;

public function __construct() {
$this->patcher = new MapPatcher();
}

/**
* @param Fingerprint $fingerprint
* @param EntityDiff $patch
*
* @throws InvalidArgumentException
* @throws PatcherException
*/
public function patchFingerprint( Fingerprint $fingerprint, EntityDiff $patch ) {
$labels = $this->patcher->patch(
$fingerprint->getLabels()->toTextArray(),
$patch->getLabelsDiff()
);

$fingerprint->setLabels( $this->newTermListFromArray( $labels ) );

$descriptions = $this->patcher->patch(
$fingerprint->getDescriptions()->toTextArray(),
$patch->getDescriptionsDiff()
);

$fingerprint->setDescriptions( $this->newTermListFromArray( $descriptions ) );

$this->patchAliases( $fingerprint, $patch->getAliasesDiff() );
$this->patchTermList( $fingerprint->getLabels(), $patch->getLabelsDiff() );
$this->patchTermList( $fingerprint->getDescriptions(), $patch->getDescriptionsDiff() );
$this->patchAliasGroupList( $fingerprint->getAliasGroups(), $patch->getAliasesDiff() );
}

/**
* @param string[] $termArray
* @see MapPatcher
*
* @return TermList
* @param TermList $labels
* @param Diff $diff
*
* @throws PatcherException
*/
private function newTermListFromArray( array $termArray ) {
$termList = new TermList();

foreach ( $termArray as $language => $labelText ) {
$termList->setTextForLanguage( $language, $labelText );
private function patchTermList( TermList $labels, Diff $diff ) {
foreach ( $diff as $languageCode => $diffOp ) {
$hasTerm = $labels->hasTermForLanguage( $languageCode );

if ( $diffOp instanceof DiffOpAdd ) {
if ( !$hasTerm ) {
$labels->setTextForLanguage( $languageCode, $diffOp->getNewValue() );
}
} elseif ( $diffOp instanceof DiffOpChange ) {
if ( $hasTerm
&& $labels->getByLanguage( $languageCode )->getText() === $diffOp->getOldValue()
) {
$labels->setTextForLanguage( $languageCode, $diffOp->getNewValue() );
}
} elseif ( $diffOp instanceof DiffOpRemove ) {
if ( $hasTerm
&& $labels->getByLanguage( $languageCode )->getText() === $diffOp->getOldValue()
) {
$labels->removeByLanguage( $languageCode );
}
} else {
throw new PatcherException( 'Invalid TermList diff' );
}
}

return $termList;
}

private function patchAliases( Fingerprint $fingerprint, Diff $aliasesDiff ) {
$patchedAliases = $this->patcher->patch(
$this->getAliasesArrayForPatching( $fingerprint->getAliasGroups() ),
$aliasesDiff
);

$fingerprint->setAliasGroups( $this->getAliasesFromArrayForPatching( $patchedAliases ) );
}

private function getAliasesArrayForPatching( AliasGroupList $aliases ) {
$textLists = array();

/**
* @var AliasGroup $aliasGroup
*/
foreach ( $aliases as $languageCode => $aliasGroup ) {
$textLists[$languageCode] = $aliasGroup->getAliases();
/**
* @see MapPatcher
*
* @param AliasGroupList $groups
* @param Diff $diff
*
* @throws PatcherException
*/
private function patchAliasGroupList( AliasGroupList $groups, Diff $diff ) {
foreach ( $diff as $languageCode => $diffOp ) {
$hasGroup = $groups->hasGroupForLanguage( $languageCode );

if ( $diffOp instanceof DiffOpAdd ) {
if ( !$hasGroup ) {
$groups->setAliasesForLanguage( $languageCode, $diffOp->getNewValue() );
}
} elseif ( $diffOp instanceof DiffOpChange ) {
if ( $hasGroup
&& $groups->getByLanguage( $languageCode )->getAliases() === $diffOp->getOldValue()
) {
$groups->setAliasesForLanguage( $languageCode, $diffOp->getNewValue() );
}
} elseif ( $diffOp instanceof DiffOpRemove ) {
if ( $hasGroup
&& $groups->getByLanguage( $languageCode )->getAliases() === $diffOp->getOldValue()
) {
$groups->removeByLanguage( $languageCode );
}
} elseif ( $diffOp instanceof Diff ) {
if ( $hasGroup
|| $diffOp->getChanges() === array() && $diffOp->getRemovals() === array()
) {
$aliases = $hasGroup
? $groups->getByLanguage( $languageCode )->getAliases()
: array();

$groups->setAliasesForLanguage(
$languageCode,
$this->getPatchedAliases( $aliases, $diffOp )
);
}
} else {
throw new PatcherException( 'Invalid AliasGroupList diff' );
}
}

return $textLists;
}

/**
* @param array[] $patchedAliases
* @see ListPatcher
*
* @return AliasGroupList
* @param string[] $aliases
* @param Diff $diff
*
* @throws PatcherException
* @return string[]
*/
private function getAliasesFromArrayForPatching( array $patchedAliases ) {
$aliases = new AliasGroupList();

foreach( $patchedAliases as $languageCode => $aliasList ) {
$aliases->setAliasesForLanguage( $languageCode, $aliasList );
private function getPatchedAliases( array $aliases, Diff $diff ) {
foreach ( $diff as $diffOp ) {
if ( $diffOp instanceof DiffOpAdd ) {
$aliases[] = $diffOp->getNewValue();
} elseif ( $diffOp instanceof DiffOpChange ) {
$key = array_search( $diffOp->getOldValue(), $aliases, true );

if ( $key !== false ) {
unset( $aliases[$key] );
$aliases[] = $diffOp->getNewValue();
}
} elseif ( $diffOp instanceof DiffOpRemove ) {
$key = array_search( $diffOp->getOldValue(), $aliases, true );

if ( $key !== false ) {
unset( $aliases[$key] );
}
} else {
throw new PatcherException( 'Invalid aliases diff' );
}
}

return $aliases;
Expand Down
140 changes: 136 additions & 4 deletions tests/unit/Diff/Internal/FingerprintPatcherTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Diff\DiffOp\Diff\Diff;
use Diff\DiffOp\DiffOpAdd;
use Diff\DiffOp\DiffOpChange;
use Diff\DiffOp\DiffOpRemove;
use Wikibase\DataModel\Services\Diff\EntityDiff;
use Wikibase\DataModel\Services\Diff\Internal\FingerprintPatcher;
use Wikibase\DataModel\Term\Fingerprint;
Expand All @@ -14,6 +15,7 @@
*
* @licence GNU GPL v2+
* @author Jeroen De Dauw < jeroendedauw@gmail.com >
* @author Thiemo Mättig
*/
class FingerprintPatcherTest extends \PHPUnit_Framework_TestCase {

Expand All @@ -34,6 +36,7 @@ private function newSimpleFingerprint() {
}

private function assertFingerprintResultsFromPatch( Fingerprint $expected, Fingerprint $original, EntityDiff $patch ) {
$this->assertEquals( $expected, $this->getPatchedFingerprint( $original, $patch ) );
$this->assertTrue( $expected->equals( $this->getPatchedFingerprint( $original, $patch ) ) );
}

Expand Down Expand Up @@ -80,19 +83,148 @@ public function testDescriptionDiffOnlyAffectsDescriptions() {
$this->assertFingerprintResultsFromPatch( $expectedFingerprint, $fingerprint, $patch );
}

public function testAliasDiffOnlyAffectsAliases() {
public function aliasDiffProvider() {
return array(
'diffs containing add/remove ops (default)' => array( array(
'de' => new Diff( array( new DiffOpAdd( 'foo' ) ) ),
'en' => new Diff( array( new DiffOpRemove( 'en-old' ), new DiffOpAdd( 'en-new' ) ) ),
'fa' => new Diff( array( new DiffOpRemove( 'fa-old' ) ) ),
) ),
'diffs containing atomic ops' => array( array(
'de' => new Diff( array( new DiffOpAdd( 'foo' ) ) ),
'en' => new Diff( array( new DiffOpChange( 'en-old', 'en-new' ) ) ),
'fa' => new Diff( array( new DiffOpRemove( 'fa-old' ) ) ),
) ),
'atomic diff ops' => array( array(
'de' => new DiffOpAdd( array( 'foo' ) ),
'en' => new DiffOpChange( array( 'en-old' ), array( 'en-new' ) ),
'fa' => new DiffOpRemove( array( 'fa-old' ) ),
) ),
);
}

/**
* @dataProvider aliasDiffProvider
*/
public function testAliasDiffOnlyAffectsAliases( array $diffOps ) {
$fingerprint = $this->newSimpleFingerprint();
$fingerprint->setAliasGroup( 'en', array( 'en-old' ) );
$fingerprint->setAliasGroup( 'fa', array( 'fa-old' ) );

$patch = new EntityDiff( array(
'aliases' => new Diff( array(
'de' => new Diff( array( new DiffOpAdd( 'foo' ) ), true ),
), true )
'aliases' => new Diff( $diffOps ),
) );

$expectedFingerprint = $this->newSimpleFingerprint();
$expectedFingerprint->setAliasGroup( 'de', array( 'foo' ) );
$expectedFingerprint->setAliasGroup( 'en', array( 'en-new' ) );

$this->assertFingerprintResultsFromPatch( $expectedFingerprint, $fingerprint, $patch );
}

public function conflictingEditProvider() {
return array(
'does not add existing label language' => array( array(
'label' => new Diff( array(
'en' => new DiffOpAdd( 'added' ),
) ),
) ),
'does not change modified label' => array( array(
'label' => new Diff( array(
'en' => new DiffOpChange( 'original', 'changed' ),
) ),
) ),
'does not change missing label' => array( array(
'label' => new Diff( array(
'de' => new DiffOpChange( 'original', 'changed' ),
) ),
) ),
'does not remove modified label' => array( array(
'label' => new Diff( array(
'en' => new DiffOpRemove( 'original' ),
) ),
) ),
'removing missing label is no-op' => array( array(
'label' => new Diff( array(
'de' => new DiffOpRemove( 'original' ),
) ),
) ),

'does not add existing description language' => array( array(
'description' => new Diff( array(
'en' => new DiffOpAdd( 'added' ),
) ),
) ),
'does not change modified description' => array( array(
'description' => new Diff( array(
'en' => new DiffOpChange( 'original', 'changed' ),
) ),
) ),
'changing missing description is no-op' => array( array(
'description' => new Diff( array(
'de' => new DiffOpChange( 'original', 'changed' ),
) ),
) ),
'does not remove modified description' => array( array(
'description' => new Diff( array(
'en' => new DiffOpRemove( 'original' ),
) ),
) ),
'removing missing description is no-op' => array( array(
'description' => new Diff( array(
'de' => new DiffOpRemove( 'original' ),
) ),
) ),

'does not add existing aliases language' => array( array(
'aliases' => new Diff( array(
'en' => new DiffOpAdd( array( 'added' ) ),
) ),
) ),
'does not change missing aliases language' => array( array(
'aliases' => new Diff( array(
'de' => new Diff( array( new DiffOpRemove( 'original' ), new DiffOpAdd( 'changed' ) ) ),
) ),
) ),
'changing missing aliases is no-op' => array( array(
'aliases' => new Diff( array(
'de' => new Diff( array( new DiffOpChange( 'original', 'changed' ) ) ),
'en' => new Diff( array( new DiffOpChange( 'original', 'changed' ) ) ),
) ),
) ),
'changing missing aliases is no-op (atomic)' => array( array(
'aliases' => new Diff( array(
'de' => new DiffOpChange( array( 'original' ), array( 'changed' ) ),
'en' => new DiffOpChange( array( 'original' ), array( 'changed' ) ),
) ),
) ),
'removing missing aliases is no-op' => array( array(
'aliases' => new Diff( array(
'de' => new Diff( array( new DiffOpRemove( 'original' ) ) ),
'en' => new Diff( array( new DiffOpRemove( 'original' ) ) ),
) ),
) ),
'removing missing aliases is no-op (atomic)' => array( array(
'aliases' => new Diff( array(
'de' => new DiffOpRemove( array( 'original' ) ),
'en' => new DiffOpRemove( array( 'original' ) ),
) ),
) ),
);
}

/**
* @dataProvider conflictingEditProvider
*/
public function testGivenConflictingEdit_fingerprintIsReturnedAsIs( array $diffOps ) {
$fingerprint = new Fingerprint();
$fingerprint->setLabel( 'en', 'conflict' );
$fingerprint->setDescription( 'en', 'conflict' );
$fingerprint->setAliasGroup( 'en', array( 'conflict' ) );

$patch = new EntityDiff( $diffOps );

$this->assertFingerprintResultsFromPatch( $fingerprint, $fingerprint, $patch );
}

}

0 comments on commit 7ce5b8a

Please sign in to comment.