From ebd345996ff8c5320f824790d92995d2af1ad98f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thiemo=20M=C3=A4ttig?= <thiemo.maettig@wikimedia.de>
Date: Mon, 25 Apr 2016 15:46:32 +0200
Subject: [PATCH] Introduce StatementListPatcher::patchStatementList

---
 src/Diff/Internal/StatementListPatcher.php    | 68 +++++++++------
 .../Internal/StatementListPatcherTest.php     | 87 +++++++++++++++++++
 2 files changed, 129 insertions(+), 26 deletions(-)

diff --git a/src/Diff/Internal/StatementListPatcher.php b/src/Diff/Internal/StatementListPatcher.php
index cdbbd812..a60ee126 100644
--- a/src/Diff/Internal/StatementListPatcher.php
+++ b/src/Diff/Internal/StatementListPatcher.php
@@ -2,9 +2,12 @@
 
 namespace Wikibase\DataModel\Services\Diff\Internal;
 
-use Diff\Comparer\CallbackComparer;
 use Diff\DiffOp\Diff\Diff;
-use Diff\Patcher\MapPatcher;
+use Diff\DiffOp\DiffOp;
+use Diff\DiffOp\DiffOpAdd;
+use Diff\DiffOp\DiffOpChange;
+use Diff\DiffOp\DiffOpRemove;
+use Diff\Patcher\PatcherException;
 use InvalidArgumentException;
 use Wikibase\DataModel\Statement\Statement;
 use Wikibase\DataModel\Statement\StatementList;
@@ -16,25 +19,51 @@
  *
  * @license GPL-2.0+
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ * @author Thiemo Mättig
  */
 class StatementListPatcher {
 
 	/**
-	 * @var MapPatcher
+	 * @since 3.6
+	 *
+	 * @param StatementList $statements
+	 * @param Diff $patch
+	 *
+	 * @throws PatcherException
 	 */
-	private $patcher;
+	public function patchStatementList( StatementList $statements, Diff $patch ) {
+		/** @var DiffOp $diffOp */
+		foreach ( $patch as $diffOp ) {
+			switch ( true ) {
+				case $diffOp instanceof DiffOpAdd:
+					/** @var DiffOpAdd $diffOp */
+					$statements->addStatement( $diffOp->getNewValue() );
+					break;
 
-	public function __construct() {
-		$this->patcher = new MapPatcher();
+				case $diffOp instanceof DiffOpChange:
+					/** @var DiffOpChange $diffOp */
+					/** @var Statement $statement */
+					$statement = $diffOp->getOldValue();
+					$statements->removeStatementsWithGuid( $statement->getGuid() );
+					$statements->addStatement( $diffOp->getNewValue() );
+					break;
 
-		$this->patcher->setValueComparer( new CallbackComparer(
-			function( Statement $firstStatement, Statement $secondStatement ) {
-				return $firstStatement->equals( $secondStatement );
+				case $diffOp instanceof DiffOpRemove:
+					/** @var DiffOpRemove $diffOp */
+					/** @var Statement $statement */
+					$statement = $diffOp->getOldValue();
+					$statements->removeStatementsWithGuid( $statement->getGuid() );
+					break;
+
+				default:
+					throw new PatcherException( 'Invalid statement list diff' );
 			}
-		) );
+		}
 	}
 
 	/**
+	 * @deprecated since 3.6, use patchStatementList instead
+	 *
 	 * @param StatementList $statements
 	 * @param Diff $patch
 	 *
@@ -42,22 +71,9 @@ function( Statement $firstStatement, Statement $secondStatement ) {
 	 * @return StatementList
 	 */
 	public function getPatchedStatementList( StatementList $statements, Diff $patch ) {
-		$statementsByGuid = array();
-
-		/**
-		 * @var Statement $statement
-		 */
-		foreach ( $statements as $statement ) {
-			$statementsByGuid[$statement->getGuid()] = $statement;
-		}
-
-		$patchedList = new StatementList();
-
-		foreach ( $this->patcher->patch( $statementsByGuid, $patch ) as $statement ) {
-			$patchedList->addStatement( $statement );
-		}
-
-		return $patchedList;
+		$patched = clone $statements;
+		$this->patchStatementList( $patched, $patch );
+		return $patched;
 	}
 
 }
diff --git a/tests/unit/Diff/Internal/StatementListPatcherTest.php b/tests/unit/Diff/Internal/StatementListPatcherTest.php
index 7555d636..f41e9066 100644
--- a/tests/unit/Diff/Internal/StatementListPatcherTest.php
+++ b/tests/unit/Diff/Internal/StatementListPatcherTest.php
@@ -5,6 +5,7 @@
 use DataValues\StringValue;
 use Diff\DiffOp\Diff\Diff;
 use Diff\DiffOp\DiffOpAdd;
+use Diff\DiffOp\DiffOpChange;
 use Diff\DiffOp\DiffOpRemove;
 use Wikibase\DataModel\Services\Diff\Internal\StatementListPatcher;
 use Wikibase\DataModel\Snak\PropertyNoValueSnak;
@@ -18,9 +19,93 @@
  *
  * @license GPL-2.0+
  * @author Jeroen De Dauw < jeroendedauw@gmail.com >
+ * @author Thiemo Mättig
  */
 class StatementListPatcherTest extends \PHPUnit_Framework_TestCase {
 
+	public function patchStatementListProvider() {
+		$statement1 = new Statement( new PropertyNoValueSnak( 1 ) );
+		$statement2 = new Statement( new PropertyNoValueSnak( 2 ) );
+
+		return array(
+			// Empty diffs
+			array(
+				new StatementList(),
+				new Diff(),
+				new StatementList()
+			),
+			array(
+				new StatementList( $statement1 ),
+				new Diff(),
+				new StatementList( $statement1 )
+			),
+
+			// Add operations
+			array(
+				new StatementList(),
+				new Diff( array( new DiffOpAdd( $statement1 ) ), true ),
+				new StatementList( $statement1 )
+			),
+			array(
+				new StatementList(),
+				new Diff( array( new DiffOpAdd( $statement1 ) ), false ),
+				new StatementList( $statement1 )
+			),
+			array(
+				new StatementList(),
+				new Diff( array( new DiffOpAdd( $statement1 ) ) ),
+				new StatementList( $statement1 )
+			),
+
+			// Remove operations
+			array(
+				new StatementList( $statement1 ),
+				new Diff( array( new DiffOpRemove( $statement1 ) ), true ),
+				new StatementList()
+			),
+			array(
+				new StatementList( $statement1 ),
+				new Diff( array( new DiffOpRemove( $statement1 ) ), false ),
+				new StatementList()
+			),
+			array(
+				new StatementList( $statement1 ),
+				new Diff( array( new DiffOpRemove( $statement1 ) ) ),
+				new StatementList()
+			),
+
+			// Mixed operations
+			array(
+				new StatementList( $statement1 ),
+				new Diff( array(
+					new DiffOpRemove( $statement1 ),
+					new DiffOpAdd( $statement2 ),
+				) ),
+				new StatementList( $statement2 )
+			),
+			array(
+				new StatementList( $statement1 ),
+				new Diff( array(
+					new DiffOpChange( $statement1, $statement2 ),
+				) ),
+				new StatementList( $statement2 )
+			),
+		);
+	}
+
+	/**
+	 * @dataProvider patchStatementListProvider
+	 */
+	public function testPatchStatementList(
+		StatementList $statements,
+		Diff $patch,
+		StatementList $expected
+	) {
+		$patcher = new StatementListPatcher();
+		$patcher->patchStatementList( $statements, $patch );
+		$this->assertEquals( $expected, $statements );
+	}
+
 	public function testGivenEmptyDiff_listIsReturnedAsIs() {
 		$statements = new StatementList();
 
@@ -29,7 +114,9 @@ public function testGivenEmptyDiff_listIsReturnedAsIs() {
 
 	private function assertListResultsFromPatch( StatementList $expected, StatementList $original, Diff $patch ) {
 		$patcher = new StatementListPatcher();
+		$clone = clone $original;
 		$this->assertEquals( $expected, $patcher->getPatchedStatementList( $original, $patch ) );
+		$this->assertEquals( $clone, $original, 'original must not change' );
 	}
 
 	public function testFoo() {