Skip to content

Commit d0200ce

Browse files
committed
feat: Save a checksum for documents and use it to detect conflicts
Signed-off-by: Benjamin Frueh <[email protected]>
1 parent 2545f19 commit d0200ce

File tree

5 files changed

+73
-9
lines changed

5 files changed

+73
-9
lines changed

composer/composer/autoload_classmap.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@
6262
'OCA\\Text\\Migration\\Version030701Date20230207131313' => $baseDir . '/../lib/Migration/Version030701Date20230207131313.php',
6363
'OCA\\Text\\Migration\\Version030901Date20231114150437' => $baseDir . '/../lib/Migration/Version030901Date20231114150437.php',
6464
'OCA\\Text\\Migration\\Version040100Date20240611165300' => $baseDir . '/../lib/Migration/Version040100Date20240611165300.php',
65+
'OCA\\Text\\Migration\\Version070000Date20250925110024' => $baseDir . '/../lib/Migration/Version070000Date20250925110024.php',
6566
'OCA\\Text\\Notification\\Notifier' => $baseDir . '/../lib/Notification/Notifier.php',
6667
'OCA\\Text\\Service\\ApiService' => $baseDir . '/../lib/Service/ApiService.php',
6768
'OCA\\Text\\Service\\AttachmentService' => $baseDir . '/../lib/Service/AttachmentService.php',

composer/composer/autoload_static.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ class ComposerStaticInitText
7777
'OCA\\Text\\Migration\\Version030701Date20230207131313' => __DIR__ . '/..' . '/../lib/Migration/Version030701Date20230207131313.php',
7878
'OCA\\Text\\Migration\\Version030901Date20231114150437' => __DIR__ . '/..' . '/../lib/Migration/Version030901Date20231114150437.php',
7979
'OCA\\Text\\Migration\\Version040100Date20240611165300' => __DIR__ . '/..' . '/../lib/Migration/Version040100Date20240611165300.php',
80+
'OCA\\Text\\Migration\\Version070000Date20250925110024' => __DIR__ . '/..' . '/../lib/Migration/Version070000Date20250925110024.php',
8081
'OCA\\Text\\Notification\\Notifier' => __DIR__ . '/..' . '/../lib/Notification/Notifier.php',
8182
'OCA\\Text\\Service\\ApiService' => __DIR__ . '/..' . '/../lib/Service/ApiService.php',
8283
'OCA\\Text\\Service\\AttachmentService' => __DIR__ . '/..' . '/../lib/Service/AttachmentService.php',

lib/Db/Document.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
* @method setLastSavedVersionEtag(string $etag): void
2424
* @method getBaseVersionEtag(): string
2525
* @method setBaseVersionEtag(string $etag): void
26+
* @method getChecksum(): ?string
27+
* @method setChecksum(?string $checksum): void
2628
*/
2729
class Document extends Entity implements \JsonSerializable {
2830
public $id = null;
@@ -33,13 +35,15 @@ class Document extends Entity implements \JsonSerializable {
3335
protected int $lastSavedVersionTime = 0;
3436
protected string $lastSavedVersionEtag = '';
3537
protected string $baseVersionEtag = '';
38+
protected ?string $checksum = null;
3639

3740
public function __construct() {
3841
$this->addType('id', 'integer');
3942
$this->addType('currentVersion', 'integer');
4043
$this->addType('lastSavedVersion', 'integer');
4144
$this->addType('lastSavedVersionTime', 'integer');
4245
$this->addType('initialVersion', 'integer');
46+
$this->addType('checksum', 'string');
4347
}
4448

4549
public function jsonSerialize(): array {
@@ -48,7 +52,8 @@ public function jsonSerialize(): array {
4852
'lastSavedVersion' => $this->lastSavedVersion,
4953
'lastSavedVersionTime' => $this->lastSavedVersionTime,
5054
'baseVersionEtag' => $this->baseVersionEtag,
51-
'initialVersion' => $this->initialVersion
55+
'initialVersion' => $this->initialVersion,
56+
'checksum' => $this->checksum
5257
];
5358
}
5459
}
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace OCA\Text\Migration;
6+
7+
use Closure;
8+
use OCP\DB\ISchemaWrapper;
9+
use OCP\DB\Types;
10+
use OCP\Migration\Attributes\AddColumn;
11+
use OCP\Migration\Attributes\ColumnType;
12+
use OCP\Migration\IOutput;
13+
use OCP\Migration\SimpleMigrationStep;
14+
15+
#[AddColumn(table: 'text_documents', name: 'checksum', type: ColumnType::STRING, description: 'CRC32 checksum of document content')]
16+
class Version070000Date20250925110024 extends SimpleMigrationStep {
17+
public function changeSchema(IOutput $output, Closure $schemaClosure, array $options) {
18+
/** @var ISchemaWrapper $schema */
19+
$schema = $schemaClosure();
20+
21+
$table = $schema->getTable('text_documents');
22+
if (!$table->hasColumn('checksum')) {
23+
$table->addColumn('checksum', Types::STRING, [
24+
'notnull' => false,
25+
'length' => 8,
26+
]);
27+
return $schema;
28+
}
29+
30+
return null;
31+
}
32+
}

lib/Service/DocumentService.php

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -143,6 +143,7 @@ public function createDocument(File $file): Document {
143143
$document->setLastSavedVersionTime($file->getMTime());
144144
$document->setLastSavedVersionEtag($file->getEtag());
145145
$document->setBaseVersionEtag(uniqid());
146+
$document->setChecksum($this->computeCheckSum($file->getContent()));
146147
try {
147148
/** @var Document $document */
148149
$document = $this->documentMapper->insert($document);
@@ -310,25 +311,48 @@ public function getSteps(int $documentId, int $lastVersion): array {
310311
return $this->stepMapper->find($documentId, $lastVersion);
311312
}
312313

314+
315+
313316
/**
314317
* @throws DocumentSaveConflictException
315318
* @throws InvalidPathException
316319
* @throws NotFoundException
317320
*/
318321
public function assertNoOutsideConflict(Document $document, File $file, bool $force = false, ?string $shareToken = null): void {
319322
$documentId = $document->getId();
320-
$savedEtag = $file->getEtag();
321323
$lastMTime = $document->getLastSavedVersionTime();
324+
$lastEtag = $document->getLastSavedVersionEtag();
325+
326+
if ($lastMTime <= 0 || $force || $this->isReadOnly($file, $shareToken) || $this->cache->get('document-save-lock-' . $documentId)) {
327+
return;
328+
}
329+
330+
$fileMtime = $file->getMtime();
331+
$fileEtag = $file->getEtag();
332+
333+
if ($lastEtag === $fileEtag && $lastMTime === $fileMtime) {
334+
return;
335+
}
336+
337+
$storedChecksum = $document->getChecksum();
338+
$fileContent = $file->getContent();
339+
$fileChecksum = $this->computeChecksum($fileContent);
322340

323-
if ($lastMTime > 0
324-
&& $force === false
325-
&& !$this->isReadOnly($file, $shareToken)
326-
&& $savedEtag !== $document->getLastSavedVersionEtag()
327-
&& $lastMTime !== $file->getMtime()
328-
&& !$this->cache->get('document-save-lock-' . $documentId)
329-
) {
341+
if ($storedChecksum !== $fileChecksum) {
330342
throw new DocumentSaveConflictException('File changed in the meantime from outside');
331343
}
344+
345+
$document->setLastSavedVersionTime($fileMtime);
346+
$document->setLastSavedVersionEtag($fileEtag);
347+
$this->documentMapper->update($document);
348+
}
349+
350+
/**
351+
* @param string $content
352+
* @return string
353+
*/
354+
private function computeCheckSum(string $content): string {
355+
return hash('crc32', $content);
332356
}
333357

334358
/**
@@ -414,6 +438,7 @@ public function autosave(Document $document, ?File $file, int $version, ?string
414438
$document->setLastSavedVersion($stepsVersion);
415439
$document->setLastSavedVersionTime($file->getMTime());
416440
$document->setLastSavedVersionEtag($file->getEtag());
441+
$document->setChecksum($this->computeCheckSum($autoSaveDocument));
417442
$this->documentMapper->update($document);
418443
} catch (LockedException $e) {
419444
// Ignore lock since it might occur when multiple people save at the same time

0 commit comments

Comments
 (0)