Added snapshot merging

src/Dao/EntityConverters/SnapshotEntityConverter.php

@@ -25,7 +25,7 @@ class SnapshotEntityConverter extends AbstractEntityConverter implements IEntity
 		$entity->setTime($this->dbTimeToEntityTime($array['time']));
 		$entity->setType(intval($array['type']));
 		$entity->setPrimaryKey($array['primaryKey']);
-		$entity->setUserId($array['userId']);
+		$entity->setUserId(intval($array['userId']));
 		$entity->setOperation($array['operation']);
 		$entity->setData(json_decode($array['data'], true));
 		$entity->setDataDifference(json_decode($array['dataDifference'], true));

src/Dao/SnapshotDao.php

@@ -22,13 +22,17 @@ class SnapshotDao extends AbstractDao
 		$this->userDao = $userDao;
 	}
 
-	public function findByTypeAndKey($type, $primaryKey)
+	public function findEarlierSnapshots(Snapshot $snapshot)
 	{
 		$query = $this->pdo
 			->from($this->tableName)
-			->where('type', $type)
+			->where('type', $snapshot->getType())
-			->where('primaryKey', $primaryKey)
+			->where('primaryKey', $snapshot->getPrimaryKey())
 			->orderBy('time DESC');
+
+		if ($snapshot->getId())
+			$query->where('id < ?', $snapshot->getId());
+
 		return $this->arrayToEntities(iterator_to_array($query));
 	}
 

src/Services/HistoryService.php

@@ -39,29 +39,46 @@ class HistoryService
 	{
 		$transactionFunc = function() use ($snapshot)
 		{
-			$otherSnapshots = $this->snapshotDao->findByTypeAndKey($snapshot->getType(), $snapshot->getPrimaryKey());
-			if ($otherSnapshots)
-			{
-				$lastSnapshot = array_shift($otherSnapshots);
-				$dataDifference = $this->getSnapshotDataDifference($snapshot->getData(), $lastSnapshot->getData());
-				$snapshot->setDataDifference($dataDifference);
-				if (empty($dataDifference['+']) && empty($dataDifference['-']))
-					return $lastSnapshot;
-			}
-			else
-			{
-				$dataDifference = $this->getSnapshotDataDifference($snapshot->getData(), []);
-				$snapshot->setDataDifference($dataDifference);
-			}
-
 			$snapshot->setTime($this->timeService->getCurrentTime());
 			$snapshot->setUser($this->authService->getLoggedInUser());
+
+			$lastSnapshot = $this->getLastSnapshot($snapshot);
+
+			$dataDifference = $this->getSnapshotDataDifference($snapshot, $lastSnapshot);
+			$snapshot->setDataDifference($dataDifference);
+
+			if ($snapshot->getOperation() !== Snapshot::OPERATION_DELETE && $lastSnapshot !== null)
+			{
+				//don't save if nothing changed
+				if (empty($dataDifference['+']) && empty($dataDifference['-']))
+				{
+					if ($snapshot->getId())
+						$this->snapshotDao->deleteById($snapshot->getId());
+					return $lastSnapshot;
+				}
+
+				//merge recent edits
+				$isFresh = ((strtotime($snapshot->getTime()) - strtotime($lastSnapshot->getTime())) <= 5 * 60);
+				if ($isFresh && $lastSnapshot->getUserId() === $snapshot->getUserId())
+				{
+					$lastSnapshot->setData($snapshot->getData());
+					return $this->saveSnapshot($lastSnapshot);
+				}
+			}
+
 			return $this->snapshotDao->save($snapshot);
 		};
 		return $this->transactionManager->commit($transactionFunc);
 	}
 
-	public function getSnapshotDataDifference($newData, $oldData)
+	public function getSnapshotDataDifference(Snapshot $newSnapshot, Snapshot $oldSnapshot = null)
+	{
+		return $this->getDataDifference(
+			$newSnapshot->getData(),
+			$oldSnapshot ? $oldSnapshot->getData() : []);
+	}
+
+	public function getDataDifference($newData, $oldData)
 	{
 		$diffFunction = function($base, $other)
 		{
@@ -90,4 +107,10 @@ class HistoryService
 			'-' => $diffFunction($oldData, $newData),
 		];
 	}
+
+	private function getLastSnapshot(Snapshot $reference)
+	{
+		$earlierSnapshots = $this->snapshotDao->findEarlierSnapshots($reference);
+		return empty($earlierSnapshots) ? null : array_shift($earlierSnapshots);
+	}
 }

tests/Services/HistoryServiceTest.php

@@ -1,5 +1,6 @@
 <?php
 namespace Szurubooru\Tests\Services;
+use Szurubooru\Entities\Snapshot;
 use Szurubooru\Dao\SnapshotDao;
 use Szurubooru\Dao\TransactionManager;
 use Szurubooru\Services\AuthService;
@@ -14,7 +15,7 @@ final class HistoryServiceTest extends AbstractTestCase
 	private $authServiceMock;
 	private $transactionManagerMock;
 
-	public static function snapshotDataDifferenceProvider()
+	public static function dataDifferenceProvider()
 	{
 		yield
 		[
@@ -105,18 +106,121 @@ final class HistoryServiceTest extends AbstractTestCase
 	{
 		parent::setUp();
 		$this->snapshotDaoMock = $this->mock(SnapshotDao::class);
-		$this->transactionManagerMock = $this->mock(TransactionManager::class);
+		$this->transactionManagerMock = $this->mockTransactionManager();
 		$this->timeServiceMock = $this->mock(TimeService::class);
 		$this->authServiceMock = $this->mock(AuthService::class);
 	}
 
 	/**
-	 * @dataProvider snapshotDataDifferenceProvider
+	 * @dataProvider dataDifferenceProvider
 	 */
-	public function testSnapshotDataDifference($newData, $oldData, $expectedResult)
+	public function testDataDifference($newData, $oldData, $expectedResult)
 	{
 		$historyService = $this->getHistoryService();
-		$this->assertEquals($expectedResult, $historyService->getSnapshotDataDifference($newData, $oldData));
+		$this->assertEquals($expectedResult, $historyService->getDataDifference($newData, $oldData));
+	}
+
+	public static function mergingProvider()
+	{
+		{
+			$oldSnapshot = new Snapshot(1);
+			$oldSnapshot->setTime(date('c', 1));
+			$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
+			$oldSnapshot->setData(['old' => '1']);
+
+			$newSnapshot = new Snapshot(2);
+			$newSnapshot->setTime(date('c', 2));
+			$newSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
+			$newSnapshot->setData(['new' => '2']);
+
+			$expectedSnapshot = new Snapshot(1);
+			$expectedSnapshot->setTime(date('c', 3));
+			$expectedSnapshot->setOperation(Snapshot::OPERATION_CREATION);
+			$expectedSnapshot->setData(['new' => '2']);
+			$expectedSnapshot->setDataDifference(['+' => [['new', '2']], '-' => []]);
+
+			yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, date('c', 3)];
+		}
+
+		{
+			$oldSnapshot = new Snapshot(1);
+			$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
+			$oldSnapshot->setData(['old' => '1']);
+
+			$newSnapshot = new Snapshot(2);
+			$newSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
+			$newSnapshot->setData(['new' => '2']);
+
+			$expectedSnapshot = new Snapshot(2);
+			$expectedSnapshot->setTime(date('c', 3000));
+			$expectedSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
+			$expectedSnapshot->setData(['new' => '2']);
+			$expectedSnapshot->setDataDifference(['+' => [['new', '2']], '-' => [['old', '1']]]);
+
+			yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, date('c', 3000)];
+		}
+
+		{
+			$oldSnapshot = new Snapshot(1);
+			$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
+			$oldSnapshot->setData(['old' => '1']);
+			$oldSnapshot->setUserId(1);
+
+			$newSnapshot = new Snapshot(2);
+			$newSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
+			$newSnapshot->setData(['new' => '2']);
+			$newSnapshot->setUserId(2);
+
+			$expectedSnapshot = new Snapshot(2);
+			$expectedSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
+			$expectedSnapshot->setData(['new' => '2']);
+			$expectedSnapshot->setDataDifference(['+' => [['new', '2']], '-' => [['old', '1']]]);
+			$expectedSnapshot->setUserId(null);
+
+			yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, null];
+		}
+
+		{
+			$oldSnapshot = new Snapshot(1);
+			$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
+			$oldSnapshot->setData(['old' => '1']);
+
+			$newSnapshot = new Snapshot(2);
+			$newSnapshot->setOperation(Snapshot::OPERATION_DELETE);
+			$newSnapshot->setData(['new' => '2']);
+
+			$expectedSnapshot = new Snapshot(2);
+			$expectedSnapshot->setOperation(Snapshot::OPERATION_DELETE);
+			$expectedSnapshot->setData(['new' => '2']);
+			$expectedSnapshot->setDataDifference(['+' => [['new', '2']], '-' => [['old', '1']]]);
+
+			yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, null];
+		}
+	}
+
+	/**
+	 * @dataProvider mergingProvider
+	 */
+	public function testMerging($oldSnapshot, $newSnapshot, $expectedSnapshot, $currentTime)
+	{
+		$this->timeServiceMock->method('getCurrentTime')->willReturn($currentTime);
+
+		$this->snapshotDaoMock
+			->method('findEarlierSnapshots')
+			->will(
+				$this->onConsecutiveCalls([$oldSnapshot], null));
+
+		$this->snapshotDaoMock
+			->expects($this->once())
+			->method('save')
+			->will($this->returnCallback(function($param) use (&$actualSnapshot)
+				{
+					$actualSnapshot = $param;
+				}));
+
+		$historyService = $this->getHistoryService();
+		$historyService->saveSnapshot($newSnapshot);
+		$this->assertEntitiesEqual($expectedSnapshot, $actualSnapshot);
 	}
 
 	private function getHistoryService()