diff --git a/Classes/Backend/Controller/PersistenceController.php b/Classes/Backend/Controller/PersistenceController.php index 66b2d2e..cca32c7 100644 --- a/Classes/Backend/Controller/PersistenceController.php +++ b/Classes/Backend/Controller/PersistenceController.php @@ -8,8 +8,11 @@ use Psr\Http\Message\ServerRequestInterface; use RuntimeException; use TYPO3\CMS\Backend\Attribute\AsController; +use TYPO3\CMS\Core\Crypto\Random; +use TYPO3\CMS\Core\DataHandling\Model\CorrelationId; use TYPO3\CMS\Core\Http\JsonResponse; use TYPO3\CMS\VisualEditor\Service\DataHandlerService; +use TYPO3\CMS\VisualEditor\SysHistory\SysHistoryCombiner; use function array_keys; use function implode; @@ -20,6 +23,8 @@ { public function __construct( private DataHandlerService $dataHandlerService, + private SysHistoryCombiner $sysHistoryCombiner, + private Random $randomGenerator, ) { } @@ -44,12 +49,16 @@ public function saveAction(ServerRequestInterface $request): ResponseInterface } $GLOBALS['TYPO3_REQUEST'] = $request; - $errorLog = $this->dataHandlerService->run($data, []); + $correlationId = CorrelationId::forScope($this->randomGenerator->generateRandomBase64String(32)) + ->withAspects(SysHistoryCombiner::CORRELATION_ASPECT); + $errorLog = $this->dataHandlerService->run($data, [], $correlationId); foreach ($cmdArray as $cmd) { - $errorLog = [...$errorLog, ...$this->dataHandlerService->run([], $cmd)]; + $errorLog = [...$errorLog, ...$this->dataHandlerService->run([], $cmd, $correlationId)]; } + $this->sysHistoryCombiner->combine(); + if ($errorLog) { return new JsonResponse(['success' => false, 'errorLog' => $errorLog], 500); } diff --git a/Classes/Service/DataHandlerService.php b/Classes/Service/DataHandlerService.php index cb8d494..216e014 100644 --- a/Classes/Service/DataHandlerService.php +++ b/Classes/Service/DataHandlerService.php @@ -6,6 +6,7 @@ use RuntimeException; use TYPO3\CMS\Core\DataHandling\DataHandler; +use TYPO3\CMS\Core\DataHandling\Model\CorrelationId; use TYPO3\CMS\Core\Schema\TcaSchemaFactory; use TYPO3\CMS\Core\Utility\GeneralUtility; @@ -26,15 +27,17 @@ public function __construct( * * @return list */ - public function run(array $data, array $cmd): array + public function run(array $data, array $cmd, CorrelationId $correlationId): array { $this->validateData($data); $this->validateCmd($cmd); $dataHandler = GeneralUtility::makeInstance(DataHandler::class); // never use DataHandler over DI!! $dataHandler->start($data, $cmd); + $dataHandler->setCorrelationId($correlationId); $dataHandler->process_datamap(); $dataHandler->process_cmdmap(); + return $dataHandler->errorLog; } diff --git a/Classes/SysHistory/SysHistoryCombiner.php b/Classes/SysHistory/SysHistoryCombiner.php new file mode 100644 index 0000000..5c6cb76 --- /dev/null +++ b/Classes/SysHistory/SysHistoryCombiner.php @@ -0,0 +1,170 @@ +getMappedRows($timeToCombine); + foreach ($mapped as $rowsToCombine) { + $toBeDeletedUids = [...$toBeDeletedUids, ...$this->combineRows($rowsToCombine)]; + } + + $this->sysHistoryRepository->deleteEntries($toBeDeletedUids); + } + + /** + * @return array>> + */ + private function getMappedRows(int $timeToCombine): array + { + $mapped = []; + $groupedPerRecord = []; + foreach ($this->sysHistoryRepository->fetchSysHistory($timeToCombine) as $row) { + $key = implode('-', [ + $row['recuid'], + $row['tablename'], + $row['workspace'], + ]); + $groupedPerRecord[$key] ??= []; + $groupedPerRecord[$key][] = $row; + } + + foreach ($groupedPerRecord as $parentKey => $rows) { + $lastKey = null; + $counter = 0; + foreach ($rows as $row) { + if (!$this->isVisualEditorModification($row)) { + $lastKey = null; + $counter++; + continue; + } + + $key = implode('-', [ + $row['actiontype'], + $row['usertype'], + $row['userid'], + $row['originaluserid'], + ]); + if ($key !== $lastKey) { + $lastKey = $key; + $counter++; + } + + $mapped[$counter . '-' . $parentKey . '-' . $key][] = $row; + } + } + + return $mapped; + } + + /** + * @param array $row + */ + private function isVisualEditorModification(array $row): bool + { + if ((int)$row['actiontype'] !== RecordHistoryStore::ACTION_MODIFY) { + return false; + } + + try { + return CorrelationId::fromString((string)$row['correlation_id'])->getAspects() === [self::CORRELATION_ASPECT]; + } catch (InvalidArgumentException) { + return false; + } + } + + /** + * @param list> $rowsToCombine + * @return list + */ + private function combineRows(array $rowsToCombine): array + { + if (count($rowsToCombine) <= 1) { + return []; + } + + // first is the Oldest, last is the Newest + $first = $rowsToCombine[0]; + $last = $rowsToCombine[array_key_last($rowsToCombine)]; + $newHistoryData = json_decode($first['history_data'], true, flags: JSON_THROW_ON_ERROR); + foreach (array_slice($rowsToCombine, 1) as $rowToCombine) { + $currentHistoryData = json_decode($rowToCombine['history_data'], true, flags: JSON_THROW_ON_ERROR); + + $newHistoryData['oldRecord'] = [ + ...$currentHistoryData['oldRecord'], + ...$newHistoryData['oldRecord'], + ]; + $newHistoryData['newRecord'] = [ + ...$newHistoryData['newRecord'], + ...$currentHistoryData['newRecord'], + ]; + } + + // if one field is the same, we can remove it from the diff, because it is not a change + $newHistoryData = $this->reduce($newHistoryData); + + // if you change the text and than change it back, the resulting diff will be empty, so we can just delete all history entries + if ($this->isTheSame($newHistoryData['oldRecord'], $newHistoryData['newRecord'])) { + return array_map(intval(...), array_column($rowsToCombine, 'uid')); + } + + $this->sysHistoryRepository->updateHistoryData((int)$last['uid'], $newHistoryData); + + return array_map(intval(...), array_column(array_slice($rowsToCombine, 0, -1), 'uid')); + } + + /** + * @param array $oldRecord + * @param array $newRecord + */ + private function isTheSame(array $oldRecord, array $newRecord): bool + { + unset($oldRecord['l18n_diffsource'], $newRecord['l18n_diffsource']); + + $normalizeScalarValue = static fn(mixed $value): mixed => is_scalar($value) ? (string)$value : $value; + $oldRecord = array_map($normalizeScalarValue, $oldRecord); + $newRecord = array_map($normalizeScalarValue, $newRecord); + + return $oldRecord === $newRecord; + } + + /** + * @param array{oldRecord: array, newRecord: array} $newHistoryData + * @return array{oldRecord: array, newRecord: array} + */ + private function reduce(array $newHistoryData): array + { + foreach ($newHistoryData['oldRecord'] as $field => $value) { + if (isset($newHistoryData['newRecord'][$field]) && $newHistoryData['newRecord'][$field] === $value) { + unset($newHistoryData['oldRecord'][$field], $newHistoryData['newRecord'][$field]); + } + } + + return $newHistoryData; + } +} diff --git a/Classes/SysHistory/SysHistoryRepository.php b/Classes/SysHistory/SysHistoryRepository.php new file mode 100644 index 0000000..129efd4 --- /dev/null +++ b/Classes/SysHistory/SysHistoryRepository.php @@ -0,0 +1,61 @@ +> + */ + public function fetchSysHistory(int $timeToFetch): array + { + $queryBuilder = $this->connectionPool->getQueryBuilderForTable('sys_history'); + $result = $queryBuilder + ->select('*') + ->from('sys_history') + ->where($queryBuilder->expr()->gte('tstamp', (new DateTimeImmutable())->getTimestamp() - $timeToFetch)) + ->orderBy('uid', 'ASC') + ->executeQuery(); + return $result->fetchAllAssociative(); + } + + /** + * @param list $toBeDeletedUids + */ + public function deleteEntries(array $toBeDeletedUids): void + { + if ($toBeDeletedUids) { + $queryBuilder = $this->connectionPool->getQueryBuilderForTable('sys_history'); + $queryBuilder + ->delete('sys_history') + ->where($queryBuilder->expr()->in('uid', array_map(intval(...), $toBeDeletedUids))) + ->executeStatement(); + } + } + + /** + * @param array $newHistoryData + */ + public function updateHistoryData(int $uid, array $newHistoryData): void + { + $this->connectionPool->getConnectionForTable('sys_history')->update( + 'sys_history', + ['history_data' => json_encode($newHistoryData, JSON_THROW_ON_ERROR)], + ['uid' => $uid], + ); + } +} diff --git a/Classes/SysHistory/SysHistoryRepositoryInterface.php b/Classes/SysHistory/SysHistoryRepositoryInterface.php new file mode 100644 index 0000000..b560d1a --- /dev/null +++ b/Classes/SysHistory/SysHistoryRepositoryInterface.php @@ -0,0 +1,23 @@ +> + */ + public function fetchSysHistory(int $timeToFetch): array; + + /** + * @param list $toBeDeletedUids + */ + public function deleteEntries(array $toBeDeletedUids): void; + + /** + * @param array $newHistoryData + */ + public function updateHistoryData(int $uid, array $newHistoryData): void; +} diff --git a/Resources/Public/JavaScript/Backend/components/ve-auto-save-toggle.js b/Resources/Public/JavaScript/Backend/components/ve-auto-save-toggle.js index e80a10c..62877e6 100644 --- a/Resources/Public/JavaScript/Backend/components/ve-auto-save-toggle.js +++ b/Resources/Public/JavaScript/Backend/components/ve-auto-save-toggle.js @@ -49,7 +49,7 @@ export class VeAutoSaveToggle extends LitElement { super.connectedCallback(); if (!this.disposeUpdateEditorStateListener) { - this.disposeUpdateEditorStateListener = onMessageDebounced('updateEditorState', this.#onEditorStateMessage.bind(this), 300); + this.disposeUpdateEditorStateListener = onMessageDebounced('updateEditorState', this.#onEditorStateMessage.bind(this), 1000); } this.addEventListener('click', this.onClick); diff --git a/Tests/Unit/SysHistory/RecordingSysHistoryRepository.php b/Tests/Unit/SysHistory/RecordingSysHistoryRepository.php new file mode 100644 index 0000000..6a1414a --- /dev/null +++ b/Tests/Unit/SysHistory/RecordingSysHistoryRepository.php @@ -0,0 +1,51 @@ +> */ + public array $recordedDeleteEntries = []; + + /** @var array> */ + public array $recordedUpdateHistoryData = []; + + /** + * @param list> $sysHistoryData + */ + public function __construct(private readonly array $sysHistoryData = []) + { + } + + /** + * @return list> + */ + public function fetchSysHistory(int $timeToFetch): array + { + return $this->sysHistoryData; + } + + /** + * @param list $toBeDeletedUids + */ + public function deleteEntries(array $toBeDeletedUids): void + { + $this->recordedDeleteEntries[] = $toBeDeletedUids; + } + + /** + * @param array $newHistoryData + */ + public function updateHistoryData(int $uid, array $newHistoryData): void + { + $this->recordedUpdateHistoryData[$uid] = $newHistoryData; + } +} diff --git a/Tests/Unit/SysHistory/SysHistoryCombinerTest.php b/Tests/Unit/SysHistory/SysHistoryCombinerTest.php new file mode 100644 index 0000000..01472d3 --- /dev/null +++ b/Tests/Unit/SysHistory/SysHistoryCombinerTest.php @@ -0,0 +1,243 @@ +> $sysHistoryData + * @param list> $recordedDeleteEntries + * @param array> $recordedUpdateHistoryData + */ + #[Test] + #[DataProvider('provideCombineData')] + public function combine( + array $sysHistoryData, + array $recordedDeleteEntries = [], + array $recordedUpdateHistoryData = [], + int $timeToCombine = 60 + ): void { + $recordings = new RecordingSysHistoryRepository($sysHistoryData); + $combiner = new SysHistoryCombiner($recordings); + $combiner->combine($timeToCombine); + self::assertSame($recordedDeleteEntries, $recordings->recordedDeleteEntries, 'Delete entries do not match'); + self::assertSame($recordedUpdateHistoryData, $recordings->recordedUpdateHistoryData, 'Update history data does not match'); + } + + public static function provideCombineData(): Generator + { + yield 'no data' => [ + 'sysHistoryData' => [], + 'recordedDeleteEntries' => [[]], + 'recordedUpdateHistoryData' => [], + ]; + yield 'simple' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B'), + self::createRow(2, 'B', 'C'), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult('A', 'C'), + ], + ]; + yield 'visual editor correlations combine across scopes' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', correlationId: '0400$scope-1:subject/visual-editor'), + self::createRow(2, 'B', 'C', correlationId: '0400$scope-2:subject/visual-editor'), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult('A', 'C'), + ], + ]; + yield 'non visual editor correlations are rollback boundaries' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', correlationId: '0400$scope-1:subject/redirects/slug'), + self::createRow(2, 'B', 'C', correlationId: '0400$scope-2:subject/redirects/slug'), + ], + 'recordedDeleteEntries' => [[]], + 'recordedUpdateHistoryData' => [], + ]; + yield 'non visual editor correlations separate visual editor modifications' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', correlationId: '0400$scope-1:subject/visual-editor'), + self::createRow(2, 'B', 'B', correlationId: '0400$scope-2:subject/visual-editor/redirects/slug'), + self::createRow(3, 'B', 'C', correlationId: '0400$scope-3:subject/visual-editor'), + ], + 'recordedDeleteEntries' => [[]], + 'recordedUpdateHistoryData' => [], + ]; + yield 'change to and back with type missmatch' => [ + 'sysHistoryData' => [ + self::createRow(1, 1, 2), + self::createRow(2, 2, '1'), + ], + 'recordedDeleteEntries' => [[1, 2]], + 'recordedUpdateHistoryData' => [], + ]; + yield 'structured values are not treated as identical' => [ + 'sysHistoryData' => [ + self::createRow(1, ['a'], ['b']), + self::createRow(2, ['b'], ['c']), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult(['a'], ['c']), + ], + ]; + yield 'null and empty strings are not treated as identical' => [ + 'sysHistoryData' => [ + self::createRow(1, null, 'value'), + self::createRow(2, 'value', ''), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult(null, ''), + ], + ]; + yield 'different Users' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', 2), + self::createRow(2, 'B', 'C', 3), + ], + 'recordedDeleteEntries' => [[]], + 'recordedUpdateHistoryData' => [], + ]; + yield 'different Users still combining' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', 2), + self::createRow(2, 'B', 'C', 2), + self::createRow(3, 'C', 'D', 3), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult('A', 'C'), + ], + ]; + yield 'not combining over other users entries' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', userid: 2), + self::createRow(2, 'B', 'C', userid: 2), + self::createRow(3, 'C', 'D', userid: 3), + self::createRow(4, 'D', 'E', userid: 2), + self::createRow(5, 'E', 'F', userid: 2), + ], + 'recordedDeleteEntries' => [[1, 4]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult('A', 'C'), + 5 => self::createResult('D', 'F'), + ], + ]; + yield 'not combining over intervening actions' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B'), + self::createRow(2, 'B', 'B', actiontype: 3), + self::createRow(3, 'B', 'C'), + ], + 'recordedDeleteEntries' => [[]], + 'recordedUpdateHistoryData' => [], + ]; + yield 'combining over another record by the same user' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', recuid: 9), + self::createRow(2, 'A', 'B', recuid: 10), + self::createRow(3, 'B', 'C', recuid: 9), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 3 => self::createResult('A', 'C'), + ], + ]; + yield 'combining over another workspace by the same user' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', workspace: 0), + self::createRow(2, 'A', 'B', workspace: 2), + self::createRow(3, 'B', 'C', workspace: 0), + ], + 'recordedDeleteEntries' => [[1]], + 'recordedUpdateHistoryData' => [ + 3 => self::createResult('A', 'C', workspace: 0), + ], + ]; + yield 'not combining over other users entries second user also combines' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B', userid: 2), + self::createRow(2, 'B', 'C', userid: 2), + self::createRow(3, 'C', 'D', userid: 3), + self::createRow(4, 'D', 'E', userid: 3), + self::createRow(5, 'E', 'F', userid: 2), + ], + 'recordedDeleteEntries' => [[1, 3]], + 'recordedUpdateHistoryData' => [ + 2 => self::createResult('A', 'C'), + 4 => self::createResult('C', 'E'), + ], + ]; + yield 'lot of changes' => [ + 'sysHistoryData' => [ + self::createRow(1, 'A', 'B'), + self::createRow(2, 'B', 'C'), + self::createRow(3, 'C', 'D'), + self::createRow(4, 'D', 'E'), + self::createRow(5, 'E', 'F'), + ], + 'recordedDeleteEntries' => [[1, 2, 3, 4]], + 'recordedUpdateHistoryData' => [ + 5 => self::createResult('A', 'F'), + ], + ]; + } + + /** + * @return array + */ + public static function createRow(int $uid, mixed $oldValue, mixed $newValue, int $userid = 1, int $recuid = 9, int $workspace = 3, int $actiontype = 2, string $correlationId = '0400$scope:subject/visual-editor'): array + { + return [ + 'uid' => $uid, + 'tstamp' => $uid, + 'actiontype' => $actiontype, + 'usertype' => 'BE', + 'userid' => $userid, + 'originaluserid' => 0, + 'recuid' => $recuid, + 'tablename' => 'tt_content', + 'history_data' => json_encode([ + 'oldRecord' => [ + 'header' => $oldValue, + ], + 'newRecord' => [ + 'header' => $newValue, + ], + 'workspace' => $workspace, + ]), + 'workspace' => $workspace, + 'correlation_id' => $correlationId, + ]; + } + + /** + * @return array{oldRecord: array{header: mixed}, newRecord: array{header: mixed}, workspace: int} + */ + public static function createResult(mixed $oldValue, mixed $newValue, int $workspace = 3): array + { + return [ + 'oldRecord' => [ + 'header' => $oldValue, + ], + 'newRecord' => [ + 'header' => $newValue, + ], + 'workspace' => $workspace, + ]; + } +} diff --git a/ext_tables.sql b/ext_tables.sql new file mode 100644 index 0000000..189c0d3 --- /dev/null +++ b/ext_tables.sql @@ -0,0 +1,3 @@ +CREATE TABLE sys_history ( + KEY visual_editor_tstamp_uid (tstamp,uid) +);