diff --git a/src/Diff/Internal/StatementListPatcher.php b/src/Diff/Internal/StatementListPatcher.php index a5547a7f..d007a1d6 100644 --- a/src/Diff/Internal/StatementListPatcher.php +++ b/src/Diff/Internal/StatementListPatcher.php @@ -40,15 +40,21 @@ public function patchStatementList( StatementList $statements, Diff $patch ) { switch ( true ) { case $diffOp instanceof DiffOpAdd: /** @var DiffOpAdd $diffOp */ - $statements->addStatement( $diffOp->getNewValue() ); + /** @var Statement $statement */ + $statement = $diffOp->getNewValue(); + $guid = $statement->getGuid(); + if ( $statements->getFirstStatementWithGuid( $guid ) === null ) { + $statements->addStatement( $statement ); + } break; case $diffOp instanceof DiffOpChange: /** @var DiffOpChange $diffOp */ - /** @var Statement $statement */ - $statement = $diffOp->getOldValue(); - $statements->removeStatementsWithGuid( $statement->getGuid() ); - $statements->addStatement( $diffOp->getNewValue() ); + /** @var Statement $oldStatement */ + /** @var Statement $newStatement */ + $oldStatement = $diffOp->getOldValue(); + $newStatement = $diffOp->getNewValue(); + $this->changeStatement( $statements, $oldStatement->getGuid(), $newStatement ); break; case $diffOp instanceof DiffOpRemove: @@ -64,6 +70,39 @@ public function patchStatementList( StatementList $statements, Diff $patch ) { } } + /** + * @param StatementList $statements + * @param string|null $oldGuid + * @param Statement $newStatement + */ + private function changeStatement( StatementList $statements, $oldGuid, Statement $newStatement ) { + $replacements = array(); + + foreach ( $statements->toArray() as $statement ) { + $guid = $statement->getGuid(); + + // Collect all elements starting from the first with the same GUID + if ( $replacements !== array() ) { + $guid === null + ? $replacements[] = $statement + : $replacements[$guid] = $statement; + } elseif ( $guid === $oldGuid ) { + $guid === null + ? $replacements[] = $newStatement + : $replacements[$guid] = $newStatement; + } + } + + // Remove all starting from the one that should be replaced + foreach ( $replacements as $guid => $statement ) { + $statements->removeStatementsWithGuid( is_int( $guid ) ? null : $guid ); + } + // Re-add all starting from the new one + foreach ( $replacements as $statement ) { + $statements->addStatement( $statement ); + } + } + /** * @deprecated since 3.6, use patchStatementList instead * diff --git a/tests/unit/Diff/Internal/StatementListPatcherTest.php b/tests/unit/Diff/Internal/StatementListPatcherTest.php index f41e9066..6d973304 100644 --- a/tests/unit/Diff/Internal/StatementListPatcherTest.php +++ b/tests/unit/Diff/Internal/StatementListPatcherTest.php @@ -106,6 +106,134 @@ public function testPatchStatementList( $this->assertEquals( $expected, $statements ); } + /** + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + */ + public function statementOrderProvider() { + $statement1 = new Statement( new PropertyNoValueSnak( 1 ), null, null, 's1' ); + $statement2 = new Statement( new PropertyNoValueSnak( 2 ), null, null, 's2' ); + $statement3 = new Statement( new PropertyNoValueSnak( 3 ), null, null, 's3' ); + + return array( + 'Simple associative add' => array( + new StatementList(), + new Diff( array( + 's1' => new DiffOpAdd( $statement1 ), + ), true ), + array( 's1' ) + ), + 'Simple non-associative add' => array( + new StatementList(), + new Diff( array( + 's1' => new DiffOpAdd( $statement1 ), + ), false ), + array( 's1' ) + ), + 'Simple associative remove' => array( + new StatementList( $statement1 ), + new Diff( array( + 's1' => new DiffOpRemove( $statement1 ), + ), true ), + array() + ), + 'Simple non-associative remove' => array( + new StatementList( $statement1 ), + new Diff( array( + 's1' => new DiffOpRemove( $statement1 ), + ), false ), + array() + ), + + // Change operations + 'Remove and add' => array( + new StatementList( $statement1 ), + new Diff( array( + 's1' => new DiffOpRemove( $statement1 ), + 's2' => new DiffOpAdd( $statement2 ), + ) ), + array( 's2' ) + ), + 'Add and remove' => array( + new StatementList( $statement1 ), + new Diff( array( + 's2' => new DiffOpAdd( $statement2 ), + 's1' => new DiffOpRemove( $statement1 ), + ) ), + array( 's2' ) + ), + 'Simple associative replace' => array( + new StatementList( $statement1 ), + new Diff( array( + 's1' => new DiffOpChange( $statement1, $statement2 ), + ), true ), + array( 's2' ) + ), + 'Simple non-associative replace' => array( + new StatementList( $statement1 ), + new Diff( array( + 's1' => new DiffOpChange( $statement1, $statement2 ), + ), false ), + array( 's2' ) + ), + 'Replacing first element retains order' => array( + new StatementList( $statement1, $statement2 ), + new Diff( array( + 's1' => new DiffOpChange( $statement1, $statement3 ), + ) ), + array( 's3', 's2' ) + ), + 'Replacing last element retains order' => array( + new StatementList( $statement1, $statement2 ), + new Diff( array( + 's2' => new DiffOpChange( $statement2, $statement3 ), + ) ), + array( 's1', 's3' ) + ), + + // No-ops + 'Empty diff' => array( + new StatementList( $statement1 ), + new Diff(), + array( 's1' ) + ), + 'Adding existing element is no-op' => array( + new StatementList( $statement1 ), + new Diff( array( + 's1' => new DiffOpAdd( $statement1 ), + ) ), + array( 's1' ) + ), + 'Removing non-existing element is no-op' => array( + new StatementList( $statement1 ), + new Diff( array( + 's2' => new DiffOpRemove( $statement2 ), + ) ), + array( 's1' ) + ), + 'Replacing non-existing element is no-op' => array( + new StatementList( $statement1 ), + new Diff( array( + 's2' => new DiffOpChange( $statement2, $statement3 ), + ) ), + array( 's1' ) + ), + ); + } + + /** + * @dataProvider statementOrderProvider + */ + public function testStatementOrder( StatementList $statements, Diff $patch, array $expectedGuids ) { + $patcher = new StatementListPatcher(); + $patchedStatements = $patcher->getPatchedStatementList( $statements, $patch ); + + $guids = array(); + foreach ( $patchedStatements->toArray() as $statement ) { + $guids[] = $statement->getGuid(); + } + $this->assertSame( $expectedGuids, $guids ); + } + public function testGivenEmptyDiff_listIsReturnedAsIs() { $statements = new StatementList();