ulysia/szurubooru
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Changed snapshot merging to work for deletions

Browse source 
Creating and deleting stuff will remove history snapshots if it occurs
within five minute gap. This is to prevent spamming history with
tag names that are introduced by an accident and removed shortly after.
This commit is contained in:
Marcin Kurczewski 2014-11-19 19:07:29 +01:00
parent 847f248829
commit 44a4184eb8
3 changed files with 103 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
src/Dao/SnapshotDao.php
View file

@ -48,7 +48,7 @@ class SnapshotDao extends AbstractDao
private function getUser(Snapshot $snapshot)
{
$userId = $snapshot->getUserId();
$userId = $snapshot->getUserId();
return $this->userDao->findById($userId);
}
}

57
src/Services/HistoryService.php
View file

@ -42,28 +42,43 @@ class HistoryService
$snapshot->setTime($this->timeService->getCurrentTime());
$snapshot->setUser($this->authService->getLoggedInUser());
$lastSnapshot = $this->getLastSnapshot($snapshot);
$earlierSnapshots = array_values($this->snapshotDao->findEarlierSnapshots($snapshot));
$snapshotsLeft = count($earlierSnapshots);
$dataDifference = $this->getSnapshotDataDifference($snapshot, $lastSnapshot);
$snapshot->setDataDifference($dataDifference);
if ($snapshot->getOperation() !== Snapshot::OPERATION_DELETE && $lastSnapshot !== null)
while (true)
{
//don't save if nothing changed
if (empty($dataDifference['+']) && empty($dataDifference['-']))
{
if ($snapshot->getId())
$this->snapshotDao->deleteById($snapshot->getId());
return $lastSnapshot;
}
$lastSnapshot = array_shift($earlierSnapshots);
$dataDifference = $this->getSnapshotDataDifference($snapshot, $lastSnapshot);
$snapshot->setDataDifference($dataDifference);
//merge recent edits
if ($lastSnapshot === null)
break;
//recent snapshots should be merged
$isFresh = ((strtotime($snapshot->getTime()) - strtotime($lastSnapshot->getTime())) <= 5 * 60);
if ($isFresh && $lastSnapshot->getUserId() === $snapshot->getUserId())
{
$lastSnapshot->setData($snapshot->getData());
return $this->saveSnapshot($lastSnapshot);
}
if (!$isFresh || $lastSnapshot->getUserId() !== $snapshot->getUserId())
break;
//delete the current snapshot
if ($snapshot->getId())
$this->snapshotDao->deleteById($snapshot->getId());
//become previous snapshot, but keep the current data
$snapshot->setId($lastSnapshot->getId());
if ($snapshot->getOperation() !== Snapshot::OPERATION_DELETE)
$snapshot->setOperation($lastSnapshot->getOperation());
-- $snapshotsLeft;
}
$onlyRemovalLeft = (!$snapshotsLeft && $snapshot->getOperation() === Snapshot::OPERATION_DELETE);
$emptyDiff = (empty($dataDifference['+']) && empty($dataDifference['-']));
if ($onlyRemovalLeft || $emptyDiff)
{
if ($snapshot->getId())
$this->snapshotDao->deleteById($snapshot->getId());
return array_shift($earlierSnapshots);
}
return $this->snapshotDao->save($snapshot);
@ -110,10 +125,4 @@ class HistoryService
'-' => $diffFunction($oldData, $newData),
];
}
private function getLastSnapshot(Snapshot $reference)
{
$earlierSnapshots = $this->snapshotDao->findEarlierSnapshots($reference);
return empty($earlierSnapshots) ? null : array_shift($earlierSnapshots);
}
}

78
tests/Services/HistoryServiceTest.php
View file

@ -123,6 +123,7 @@ final class HistoryServiceTest extends AbstractTestCase
public static function mergingProvider()
{
{
//basic merging
$oldSnapshot = new Snapshot(1);
$oldSnapshot->setTime(date('c', 1));
$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
@ -139,10 +140,11 @@ final class HistoryServiceTest extends AbstractTestCase
$expectedSnapshot->setData(['new' => '2']);
$expectedSnapshot->setDataDifference(['+' => ['new' => '2'], '-' => []]);
yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, date('c', 3)];
yield [[$oldSnapshot], $newSnapshot, $expectedSnapshot, date('c', 3), [2]];
}
{
//too big time gap for merge
$oldSnapshot = new Snapshot(1);
$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
$oldSnapshot->setData(['old' => '1']);
@ -157,10 +159,11 @@ final class HistoryServiceTest extends AbstractTestCase
$expectedSnapshot->setData(['new' => '2']);
$expectedSnapshot->setDataDifference(['+' => ['new' => '2'], '-' => ['old' => '1']]);
yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, date('c', 3000)];
yield [[$oldSnapshot], $newSnapshot, $expectedSnapshot, date('c', 3000), []];
}
{
//operations done by different user shouldn't be merged
$oldSnapshot = new Snapshot(1);
$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
$oldSnapshot->setData(['old' => '1']);
@ -177,10 +180,11 @@ final class HistoryServiceTest extends AbstractTestCase
$expectedSnapshot->setDataDifference(['+' => ['new' => '2'], '-' => ['old' => '1']]);
$expectedSnapshot->setUserId(null);
yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, null];
yield [[$oldSnapshot], $newSnapshot, $expectedSnapshot, null, []];
}
{
//merge that leaves only delete snapshot should be removed altogether
$oldSnapshot = new Snapshot(1);
$oldSnapshot->setOperation(Snapshot::OPERATION_CREATION);
$oldSnapshot->setData(['old' => '1']);
@ -189,35 +193,91 @@ final class HistoryServiceTest extends AbstractTestCase
$newSnapshot->setOperation(Snapshot::OPERATION_DELETE);
$newSnapshot->setData(['new' => '2']);
yield [[$oldSnapshot], $newSnapshot, null, null, [2, 1]];
}
{
//chaining to creation snapshot should preserve operation type
$oldestSnapshot = new Snapshot(1);
$oldestSnapshot->setOperation(Snapshot::OPERATION_CREATION);
$oldestSnapshot->setData(['oldest' => '0']);
$oldSnapshot = new Snapshot(2);
$oldSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
$oldSnapshot->setData(['old' => '1']);
$newSnapshot = new Snapshot(3);
$newSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
$newSnapshot->setData(['oldest' => '0', 'new' => '2']);
$expectedSnapshot = new Snapshot(1);
$expectedSnapshot->setOperation(Snapshot::OPERATION_CREATION);
$expectedSnapshot->setData(['oldest' => '0', 'new' => '2']);
$expectedSnapshot->setDataDifference(['+' => ['oldest' => '0', 'new' => '2'], '-' => []]);
yield [[$oldSnapshot, $oldestSnapshot], $newSnapshot, $expectedSnapshot, null, [3, 2]];
$newSnapshot = clone($newSnapshot);
$newSnapshot->setId(null);
yield [[$oldSnapshot, $oldestSnapshot], $newSnapshot, $expectedSnapshot, null, [2]];
}
{
//chaining to edit snapshot should update operation type
$oldestSnapshot = new Snapshot(1);
$oldestSnapshot->setOperation(Snapshot::OPERATION_CREATION);
$oldestSnapshot->setData(['oldest' => '0']);
$oldestSnapshot->setTime(date('c', 1));
$oldSnapshot = new Snapshot(2);
$oldSnapshot->setOperation(Snapshot::OPERATION_CHANGE);
$oldSnapshot->setData(['old' => '1']);
$oldSnapshot->setTime(date('c', 400));
$newSnapshot = new Snapshot(3);
$newSnapshot->setOperation(Snapshot::OPERATION_DELETE);
$newSnapshot->setData(['new' => '2']);
$newSnapshot->setTime(date('c', 401));
$expectedSnapshot = new Snapshot(2);
$expectedSnapshot->setOperation(Snapshot::OPERATION_DELETE);
$expectedSnapshot->setData(['new' => '2']);
$expectedSnapshot->setDataDifference(['+' => ['new' => '2'], '-' => ['old' => '1']]);
$expectedSnapshot->setDataDifference(['+' => ['new' => '2'], '-' => ['oldest' => '0']]);
$expectedSnapshot->setTime(date('c', 402));
yield [$oldSnapshot, $newSnapshot, $expectedSnapshot, null];
yield [[$oldSnapshot, $oldestSnapshot], $newSnapshot, $expectedSnapshot, date('c', 402), [3]];
$newSnapshot = clone($newSnapshot);
$newSnapshot->setId(null);
yield [[$oldSnapshot, $oldestSnapshot], $newSnapshot, $expectedSnapshot, date('c', 402), []];
}
}
/**
* @dataProvider mergingProvider
*/
public function testMerging($oldSnapshot, $newSnapshot, $expectedSnapshot, $currentTime)
public function testMerging($earlierSnapshots, $newSnapshot, $expectedSnapshot, $currentTime, $expectedDeletions = [])
{
$this->timeServiceMock->method('getCurrentTime')->willReturn($currentTime);
$this->snapshotDaoMock
->expects($this->once())
->method('findEarlierSnapshots')
->will(
$this->onConsecutiveCalls([$oldSnapshot], null));
->willReturn($earlierSnapshots);
$this->snapshotDaoMock
->expects($this->once())
->expects($this->exactly($expectedSnapshot === null ? 0 : 1))
->method('save')
->will($this->returnCallback(function($param) use (&$actualSnapshot)
{
$actualSnapshot = $param;
}));
$this->snapshotDaoMock
->expects($this->exactly(count($expectedDeletions)))
->method('deleteById')
->withConsecutive(...array_map(function($del) { return [$del]; }, $expectedDeletions));
$historyService = $this->getHistoryService();
$historyService->saveSnapshot($newSnapshot);
$this->assertEntitiesEqual($expectedSnapshot, $actualSnapshot);