Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lib/Service/DocumentService.php
Original file line number Diff line number Diff line change
Expand Up @@ -307,6 +307,7 @@ public function getSteps(int $documentId, int $lastVersion): array {

/**
* @throws DocumentSaveConflictException
* @throws DoesNotExistException
* @throws InvalidPathException
* @throws NotFoundException
*/
Expand All @@ -331,7 +332,16 @@ public function assertNoOutsideConflict(Document $document, File $file, bool $fo
$fileChecksum = self::computeCheckSum($fileContent);

if ($storedChecksum !== $fileChecksum) {
throw new DocumentSaveConflictException('File changed in the meantime from outside');
// $document was loaded at the start of the request.
// A save request handled in the meantime is not reflected in it
// and would be mistaken for an outside change.
// Reload the document to compare against the latest saved state.
$document = $this->documentMapper->find($documentId);
if ($document->getChecksum() !== $fileChecksum) {
throw new DocumentSaveConflictException('File changed in the meantime from outside');
}
// The save request already stored the latest version info.
return;
}

$document->setLastSavedVersionTime($fileMtime);
Expand Down
150 changes: 150 additions & 0 deletions tests/unit/Service/DocumentServiceTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
<?php

/**
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\Text\Tests;

use OCA\Text\Db\Document;
use OCA\Text\Db\DocumentMapper;
use OCA\Text\Db\SessionMapper;
use OCA\Text\Db\StepMapper;
use OCA\Text\Exception\DocumentSaveConflictException;
use OCA\Text\Service\DocumentService;
use OCA\Text\Service\FileService;
use OCA\Text\Service\LockService;
use OCP\DirectEditing\IManager;
use OCP\Files\Config\IUserMountCache;
use OCP\Files\File;
use OCP\Files\IAppData;
use OCP\Files\IRootFolder;
use OCP\ICache;
use OCP\ICacheFactory;
use OCP\IConfig;
use OCP\IRequest;
use Psr\Log\LoggerInterface;

class DocumentServiceTest extends \PHPUnit\Framework\TestCase {
private DocumentService $documentService;

private DocumentMapper $documentMapper;
private FileService $fileService;
private ICache $cache;

public function setUp(): void {
$this->documentMapper = $this->createMock(DocumentMapper::class);
$this->fileService = $this->createMock(FileService::class);
$this->cache = $this->createMock(ICache::class);
$cacheFactory = $this->createMock(ICacheFactory::class);
$cacheFactory->method('createDistributed')->willReturn($this->cache);
$request = $this->createMock(IRequest::class);
$request->method('getParam')->willReturn(null);

$this->fileService->method('isReadOnly')->willReturn(false);

$this->documentService = new DocumentService(
$this->documentMapper,
$this->fileService,
$this->createMock(StepMapper::class),
$this->createMock(SessionMapper::class),
$this->createMock(IAppData::class),
'admin',
$this->createMock(IRootFolder::class),
$cacheFactory,
$this->createMock(LoggerInterface::class),
$this->createMock(LockService::class),
$request,
$this->createMock(IManager::class),
$this->createMock(IUserMountCache::class),
$this->createMock(IConfig::class),
);
}

private function createDocument(string $etag, int $mtime, string $content): Document {
$document = new Document();
$document->setId(123);
$document->setLastSavedVersionEtag($etag);
$document->setLastSavedVersionTime($mtime);
$document->setChecksum(DocumentService::computeCheckSum($content));
return $document;
}

private function mockFile(string $etag, int $mtime, string $content): File {
$file = $this->createMock(File::class);
$file->method('getEtag')->willReturn($etag);
$file->method('getMtime')->willReturn($mtime);
$file->method('getContent')->willReturn($content);
return $file;
}

public function testNoConflictWhenVersionInfoMatches(): void {
$document = $this->createDocument('etag1', 1000, 'content');
$file = $this->mockFile('etag1', 1000, 'content');

$this->documentMapper->expects(self::never())->method('find');
$this->documentMapper->expects(self::never())->method('update');

$this->documentService->assertNoOutsideConflict($document, $file);
}

public function testRefreshesVersionInfoWhenContentMatches(): void {
$document = $this->createDocument('etag1', 1000, 'content');
$file = $this->mockFile('etag2', 2000, 'content');

$this->documentMapper->expects(self::never())->method('find');
$this->documentMapper->expects(self::once())
->method('update')
->with($document);

$this->documentService->assertNoOutsideConflict($document, $file);
self::assertSame('etag2', $document->getLastSavedVersionEtag());
self::assertSame(2000, $document->getLastSavedVersionTime());
}

public function testNoConflictWhenOwnSaveFinishedInTheMeantime(): void {
// Loaded at the start of the request - stale by now.
$document = $this->createDocument('etag1', 1000, 'old content');
// A save request updated the file in the meantime ...
$file = $this->mockFile('etag2', 2000, 'new content');
// ... and stored the new version info in the document.
$freshDocument = $this->createDocument('etag2', 2000, 'new content');

$this->documentMapper->expects(self::once())
->method('find')
->with(123)
->willReturn($freshDocument);
$this->documentMapper->expects(self::never())->method('update');

$this->documentService->assertNoOutsideConflict($document, $file);
}

public function testConflictWhenFileChangedFromOutside(): void {
$document = $this->createDocument('etag1', 1000, 'old content');
$file = $this->mockFile('etag2', 2000, 'outside content');
// The latest saved state does not match the file either.
$freshDocument = $this->createDocument('etag1', 1000, 'old content');

$this->documentMapper->expects(self::once())
->method('find')
->with(123)
->willReturn($freshDocument);

$this->expectException(DocumentSaveConflictException::class);
$this->documentService->assertNoOutsideConflict($document, $file);
}

public function testNoConflictWhileSaveLockIsHeld(): void {
$document = $this->createDocument('etag1', 1000, 'old content');
$file = $this->mockFile('etag2', 2000, 'new content');

$this->cache->method('get')
->with('document-save-lock-123')
->willReturn(true);
$this->documentMapper->expects(self::never())->method('find');
$this->documentMapper->expects(self::never())->method('update');

$this->documentService->assertNoOutsideConflict($document, $file);
}
}
Loading