From a0133ea632eeeeae401e74c8000825a1f4a310bf Mon Sep 17 00:00:00 2001 From: Marcin Kurczewski Date: Tue, 18 Nov 2014 21:22:46 +0100 Subject: [PATCH] Added snapshot merging --- .../SnapshotEntityConverter.php | 2 +- src/Dao/SnapshotDao.php | 10 +- src/Services/HistoryService.php | 55 +++++--- tests/Services/HistoryServiceTest.php | 126 ++++++++++++++++-- 4 files changed, 162 insertions(+), 31 deletions(-) diff --git a/src/Dao/EntityConverters/SnapshotEntityConverter.php b/src/Dao/EntityConverters/SnapshotEntityConverter.php index e5a8378d..938f18fe 100644 --- a/src/Dao/EntityConverters/SnapshotEntityConverter.php +++ b/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)); diff --git a/src/Dao/SnapshotDao.php b/src/Dao/SnapshotDao.php index 3dad4f47..57e5fafb 100644 --- a/src/Dao/SnapshotDao.php +++ b/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('primaryKey', $primaryKey) + ->where('type', $snapshot->getType()) + ->where('primaryKey', $snapshot->getPrimaryKey()) ->orderBy('time DESC'); + + if ($snapshot->getId()) + $query->where('id < ?', $snapshot->getId()); + return $this->arrayToEntities(iterator_to_array($query)); } diff --git a/src/Services/HistoryService.php b/src/Services/HistoryService.php index 657f6017..cbb29d6a 100644 --- a/src/Services/HistoryService.php +++ b/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); + } } diff --git a/tests/Services/HistoryServiceTest.php b/tests/Services/HistoryServiceTest.php index c372d126..ef359613 100644 --- a/tests/Services/HistoryServiceTest.php +++ b/tests/Services/HistoryServiceTest.php @@ -1,5 +1,6 @@ [], '-' => []] + [], + [], + ['+' => [], '-' => []] ]; yield [ - ['key' => 'unchangedValue'], - ['key' => 'unchangedValue'], - ['+' => [], '-' => []] + ['key' => 'unchangedValue'], + ['key' => 'unchangedValue'], + ['+' => [], '-' => []] ]; 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()