Skip to content

Commit 51750af

Browse files
committed
fix(save): autosave only every ten seconds
Abort autosave if another save is still under way. We observed save requests taking up to 30 seconds. In this timespan autosave requests would still be processed as the lastSavedTime in the database has not been updated yet. Signed-off-by: Max <max@nextcloud.com>
1 parent d513f67 commit 51750af

2 files changed

Lines changed: 144 additions & 0 deletions

File tree

lib/Service/DocumentService.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,11 @@ public function autosave(Document $document, File $file, int $version, string $a
364364

365365
$this->assertNoOutsideConflict($document, $file, $force);
366366

367+
// Abort autosave if already saving.
368+
if ($this->cache->get('document-save-lock-' . $documentId) && $manualSave === false) {
369+
return $document;
370+
}
371+
367372
// Do not save if newer version already saved
368373
// Note that $version is the version of the steps the client has fetched.
369374
// It may have added steps on top of that - so if the versions match we still save.
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
<?php
2+
3+
/**
4+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
5+
* SPDX-License-Identifier: AGPL-3.0-or-later
6+
*/
7+
8+
namespace OCA\Text\Tests;
9+
10+
use OCA\Text\Db\Document;
11+
use OCA\Text\Db\DocumentMapper;
12+
use OCA\Text\Db\SessionMapper;
13+
use OCA\Text\Db\StepMapper;
14+
use OCA\Text\Exception\DocumentSaveConflictException;
15+
use OCA\Text\Service\DocumentService;
16+
use OCA\Text\Service\FileService;
17+
use OCA\Text\Service\LockService;
18+
use OCP\DirectEditing\IManager;
19+
use OCP\Files\Config\IUserMountCache;
20+
use OCP\Files\File;
21+
use OCP\Files\IAppData;
22+
use OCP\Files\IRootFolder;
23+
use OCP\ICache;
24+
use OCP\ICacheFactory;
25+
use OCP\IConfig;
26+
use OCP\IRequest;
27+
use Psr\Log\LoggerInterface;
28+
29+
class DocumentServiceTest extends \PHPUnit\Framework\TestCase {
30+
private DocumentService $documentService;
31+
32+
private DocumentMapper $documentMapper;
33+
private FileService $fileService;
34+
private ICache $cache;
35+
36+
public function setUp(): void {
37+
$this->documentMapper = $this->createMock(DocumentMapper::class);
38+
$this->fileService = $this->createMock(FileService::class);
39+
$this->cache = $this->createMock(ICache::class);
40+
$cacheFactory = $this->createMock(ICacheFactory::class);
41+
$cacheFactory->method('createDistributed')->willReturn($this->cache);
42+
$request = $this->createMock(IRequest::class);
43+
$request->method('getParam')->willReturn(null);
44+
45+
$this->fileService->method('isReadOnly')->willReturn(false);
46+
47+
$this->documentService = new DocumentService(
48+
$this->documentMapper,
49+
$this->fileService,
50+
$this->createMock(StepMapper::class),
51+
$this->createMock(SessionMapper::class),
52+
$this->createMock(IAppData::class),
53+
'admin',
54+
$this->createMock(IRootFolder::class),
55+
$cacheFactory,
56+
$this->createMock(LoggerInterface::class),
57+
$this->createMock(LockService::class),
58+
$request,
59+
$this->createMock(IManager::class),
60+
$this->createMock(IUserMountCache::class),
61+
$this->createMock(IConfig::class),
62+
);
63+
}
64+
65+
private function createDocument(string $etag, int $mtime, string $content): Document {
66+
$document = new Document();
67+
$document->setId(123);
68+
$document->setLastSavedVersionEtag($etag);
69+
$document->setLastSavedVersionTime($mtime);
70+
$document->setChecksum(DocumentService::computeCheckSum($content));
71+
return $document;
72+
}
73+
74+
private function mockFile(string $etag, int $mtime, string $content): File {
75+
$file = $this->createMock(File::class);
76+
$file->method('getEtag')->willReturn($etag);
77+
$file->method('getMtime')->willReturn($mtime);
78+
$file->method('getContent')->willReturn($content);
79+
return $file;
80+
}
81+
82+
public function testNoConflictWhenVersionInfoMatches(): void {
83+
$document = $this->createDocument('etag1', 1000, 'content');
84+
$file = $this->mockFile('etag1', 1000, 'content');
85+
86+
$this->documentMapper->expects(self::never())->method('update');
87+
$this->documentService->assertNoOutsideConflict($document, $file);
88+
}
89+
90+
public function testRefreshesVersionInfoWhenContentMatches(): void {
91+
$document = $this->createDocument('etag1', 1000, 'content');
92+
$file = $this->mockFile('etag2', 2000, 'content');
93+
94+
$this->documentMapper->expects(self::never())->method('find');
95+
$this->documentMapper->expects(self::once())
96+
->method('update')
97+
->with($document);
98+
99+
$this->documentService->assertNoOutsideConflict($document, $file);
100+
self::assertSame('etag2', $document->getLastSavedVersionEtag());
101+
self::assertSame(2000, $document->getLastSavedVersionTime());
102+
}
103+
104+
public function testConflictWhenFileChangedFromOutside(): void {
105+
$document = $this->createDocument('etag1', 1000, 'old content');
106+
$file = $this->mockFile('etag2', 2000, 'outside content');
107+
108+
$this->expectException(DocumentSaveConflictException::class);
109+
$this->documentService->assertNoOutsideConflict($document, $file);
110+
}
111+
112+
public function testNoConflictWhileSaveLockIsHeld(): void {
113+
$document = $this->createDocument('etag1', 1000, 'old content');
114+
$file = $this->mockFile('etag2', 2000, 'new content');
115+
116+
$this->cache->method('get')
117+
->with('document-save-lock-123')
118+
->willReturn(true);
119+
$this->documentMapper->expects(self::never())->method('find');
120+
$this->documentMapper->expects(self::never())->method('update');
121+
122+
$this->documentService->assertNoOutsideConflict($document, $file);
123+
}
124+
125+
126+
public function testNoAutosavingWhileSaveIsUnderWay(): void {
127+
$document = $this->createDocument('etag1', 1000, 'new content');
128+
$file = $this->mockFile('etag1', 1000, 'old content');
129+
$this->cache->method('get')
130+
->with('document-save-lock-123')
131+
->willReturn(true);
132+
$this->documentMapper->expects(self::never())->method('update');
133+
134+
$result = $this->documentService->autosave($document, $file, 1234, 'new content', 'doc state');
135+
self::assertSame($result, $document);
136+
}
137+
138+
}
139+
}

0 commit comments

Comments
 (0)