[stable35] fix/no conflicts on sync

lib/Service/ApiService.php

@@ -175,7 +175,6 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
 			// ensure file is still present and accessible
 			$file = $this->fileService->getFileForSession($session, $shareToken);
 			$result['readOnly'] = $this->fileService->isReadOnly($file, $shareToken);
-			$this->documentService->assertNoOutsideConflict($document, $file);
 		} catch (NotPermittedException|NotFoundException|InvalidPathException $e) {
 			$this->logger->info($e->getMessage(), ['exception' => $e]);
 			return new DataResponse([
@@ -186,16 +185,9 @@ public function sync(Session $session, Document $document, int $version = 0, ?st
 			return new DataResponse([
 				'message' => 'Document no longer exists'
 			], Http::STATUS_NOT_FOUND);
-		} catch (DocumentSaveConflictException) {
-			try {
-				/** @psalm-suppress PossiblyUndefinedVariable */
-				$result['outsideChange'] = $file->getContent();
-			} catch (LockedException) {
-				// Ignore locked exception since it might happen due to an autosave action happening at the same time
-			}
 		}
 
-		return new DataResponse($result, isset($result['outsideChange']) ? Http::STATUS_CONFLICT : Http::STATUS_OK);
+		return new DataResponse($result, Http::STATUS_OK);
 	}
 
 	public function save(Session $session, Document $document, int $version, string $autosaveContent, string $documentState, bool $force = false, bool $manualSave = false, ?string $shareToken = null): DataResponse {

lib/Service/DocumentService.php

@@ -364,6 +364,11 @@ public function autosave(Document $document, File $file, int $version, string $a
 
 		$this->assertNoOutsideConflict($document, $file, $force);
 
+		// Abort autosave if already saving.
+		if ($this->cache->get('document-save-lock-' . $documentId) && $manualSave === false) {
+			return $document;
+		}
+
 		// Do not save if newer version already saved
 		// Note that $version is the version of the steps the client has fetched.
 		// It may have added steps on top of that - so if the versions match we still save.
@@ -402,7 +407,7 @@ public function autosave(Document $document, File $file, int $version, string $a
 			return $document;
 		}
 
-		$this->cache->set('document-save-lock-' . $documentId, true, 10);
+		$this->cache->set('document-save-lock-' . $documentId, true, 60);
 		try {
 			$this->lockService->runInScope($file, function () use ($file, $autoSaveDocument, $documentState): void {
 				$this->saveFromText = true;

tests/unit/Service/DocumentServiceTest.php

@@ -0,0 +1,137 @@
+<?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('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 testConflictWhenFileChangedFromOutside(): void {
+		$document = $this->createDocument('etag1', 1000, 'old content');
+		$file = $this->mockFile('etag2', 2000, 'outside content');
+
+		$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);
+	}
+
+	public function testNoAutosavingWhileSaveIsUnderWay(): void {
+		$document = $this->createDocument('etag1', 1000, 'new content');
+		$file = $this->mockFile('etag1', 1000, 'old content');
+		$this->cache->method('get')
+			->with('document-save-lock-123')
+			->willReturn(true);
+		$this->documentMapper->expects(self::never())->method('update');
+
+		$result = $this->documentService->autosave($document, $file, 1234, 'new content', 'doc state');
+		self::assertSame($result, $document);
+	}
+
+}