[stable35] fix: don't silently drop outside changes (e.g. from external storage)

lib/Service/DocumentService.php

@@ -102,8 +102,17 @@ public function isSaveFromText(): bool {
 	public function getOrCreateDocument(Document $document): Document {
 		$loaded = $this->documentMapper->load($document->getContextType(), $document->getContextId());
 		if ($loaded !== null) {
-			$this->logger->info('Keep previous document of ' . $document->toString());
-			return $loaded;
+			if ($loaded->getChecksum() === $document->getChecksum() || $this->hasUnsavedChanges($loaded)) {
+				$this->logger->info('Keep previous document of ' . $document->toString());
+				return $loaded;
+			}
+			// The file was changed without passing through Nextcloud, e.g. directly on external storage.
+			$this->logger->info('Reset outdated document of ' . $document->toString());
+			try {
+				$this->resetDocument($document->getContextType(), $document->getContextId());
+			} catch (DocumentHasUnsavedChangesException) {
+				return $loaded;
+			}
 		}
 
 		if (!$this->ensureDocumentsFolder()) {

playwright/e2e/synchronize-content/outside-change.spec.ts

@@ -0,0 +1,91 @@
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { expect, mergeTests } from '@playwright/test'
+import { test as editorTest } from '../../support/fixtures/editor.ts'
+import { test as randomUserTest } from '../../support/fixtures/random-user.ts'
+import { test as uploadFileTest } from '../../support/fixtures/upload-file.ts'
+
+const test = mergeTests(editorTest, randomUserTest, uploadFileTest)
+
+test.beforeEach(async ({ open }) => {
+	await open()
+})
+
+const resolutionVariants = [
+	{
+		source: 'local',
+		buttonName: /Keep my local changes and overwrite the remote version/,
+		headingName: 'Hello world',
+	},
+	{
+		source: 'server',
+		buttonName: /Discard my local changes and use the remote version/,
+		headingName: 'Good bye',
+	},
+]
+
+resolutionVariants.forEach(({ source, buttonName, headingName }) => {
+	test(`[conflict] saving after the file was changed outside of Nextcloud - resolve with ${source} version`, async ({
+		container,
+		editor,
+		file,
+		page,
+		reader,
+		user,
+	}) => {
+		await expect(editor.el).toBeVisible()
+		const pushPromise = page.waitForRequest(/push/)
+		await editor.typeHeading('Hello world')
+		await pushPromise
+		await user.writeFileOutsideNextcloud({ name: file.name, content: '## Good bye' })
+		// Clicking the save indicator would force the save and skip the conflict check.
+		await page.keyboard.press('ControlOrMeta+s')
+
+		// Verify both versions are shown
+		await expect(reader.getHeading({ name: 'Good bye' })).toBeVisible({
+			timeout: 10_000,
+		})
+		await expect(editor.getHeading({ name: 'Hello world' })).toBeVisible()
+
+		// Resolve conflict
+		await container.getButton({ name: buttonName }).click()
+		await expect(reader.el).not.toBeVisible()
+		await expect(editor.getHeading({ name: headingName })).toBeVisible()
+		await expect(async () => {
+			expect(await file.getContent()).toContain(headingName)
+		}).toPass()
+	})
+})
+
+test('reopening after the file was changed outside of Nextcloud shows the new content', async ({
+	close,
+	container,
+	editor,
+	file,
+	page,
+	reader,
+	user,
+}) => {
+	await expect(editor.el).toBeVisible()
+	const pushPromise = page.waitForRequest(/push/)
+	await editor.typeHeading('Hello world')
+	await pushPromise
+	await editor.saveIndicator.click()
+	await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/)
+	await close()
+
+	await user.writeFileOutsideNextcloud({ name: file.name, content: '## Good bye' })
+	await file.open()
+
+	await expect(editor.getHeading({ name: 'Good bye' })).toBeVisible({
+		timeout: 10_000,
+	})
+	await expect(editor.getHeading({ name: 'Hello world' })).not.toBeVisible()
+	await expect(reader.el).not.toBeVisible()
+	await expect(container.getButton({
+		name: /Keep my local changes and overwrite the remote version/,
+	})).not.toBeVisible()
+})

playwright/support/fixtures/User.ts

@@ -6,6 +6,7 @@
 import type { User as Account } from '@nextcloud/e2e-test-server'
 import type { Page } from '@playwright/test'
 
+import { runExec } from '@nextcloud/e2e-test-server/docker'
 import { uploadFile } from './Node.ts'
 
 export class User {
@@ -22,4 +23,19 @@ export class User {
 	uploadFile(upload: Omit<Parameters<typeof uploadFile>[0], 'owner'>) {
 		return uploadFile({ ...upload, owner: this })
 	}
+
+	/**
+	 * Write a file directly to the user's storage on the server, bypassing Nextcloud
+	 * like a change on external storage would.
+	 *
+	 * @param options options for the write
+	 * @param options.name Name of the file
+	 * @param options.content File content
+	 */
+	async writeFileOutsideNextcloud({ name, content }: { name: string, content: string }) {
+		await runExec(
+			['sh', '-c', 'printf "%s" "$CONTENT" > "data/$OWNER/files/$NAME"'],
+			{ env: [`CONTENT=${content}`, `OWNER=${this.account.userId}`, `NAME=${name}`] },
+		)
+	}
 }

src/services/SaveService.ts

@@ -87,12 +87,19 @@ class SaveService {
 		} catch (e) {
 			logger.error('Failed to save document.', { error: e })
 			const response = (
-				e as { response?: { status?: number, data?: { error?: string } } }
+				e as { response?: { status?: number, data?: { error?: string, outsideChange?: string } } }
 			).response
 			if (response?.status === 403) {
 				// Document is now read-only; permissionChange from sync will update the UI
 				return
 			}
+			if (response?.status === 409) {
+				this.bus.emit('error', {
+					type: ERROR_TYPE.SAVE_COLLISION,
+					data: { outsideChange: response.data?.outsideChange },
+				})
+				return
+			}
 			if (response?.status === 412) {
 				this.bus.emit('error', {
 					type: ERROR_TYPE.LOAD_ERROR,

src/tests/services/SaveService.spec.ts

@@ -0,0 +1,47 @@
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import type { Connection } from '../../types/Connection.ts'
+
+import { describe, expect, it, vi } from 'vitest'
+import { shallowRef } from 'vue'
+import * as saveApi from '../../apis/save.ts'
+import { SaveService } from '../../services/SaveService.ts'
+import { ERROR_TYPE } from '../../types/ErrorType.ts'
+
+vi.mock('../../apis/save.ts')
+
+const connection = shallowRef<Connection | undefined>({
+	documentId: '123',
+	sessionId: 345,
+	sessionToken: 'sessionToken',
+	filePath: './',
+	baseVersionEtag: 'etag',
+})
+function getSaveData() {
+	return {
+		version: 1,
+		autosaveContent: 'content',
+		documentState: 'state',
+	}
+}
+
+describe('SaveService', () => {
+	it('emits a save collision when the file was changed outside', async () => {
+		vi.mocked(saveApi.save).mockRejectedValue({
+			response: { status: 409, data: { outsideChange: 'outside content' } },
+		})
+		const saveService = new SaveService({ connection, getSaveData })
+		const onError = vi.fn()
+		saveService.bus.on('error', onError)
+
+		await saveService.save()
+
+		expect(onError).toHaveBeenCalledWith({
+			type: ERROR_TYPE.SAVE_COLLISION,
+			data: { outsideChange: 'outside content' },
+		})
+	})
+})

tests/unit/Service/DocumentServiceTest.php

@@ -122,6 +122,14 @@ public function testCannotCheckUnsavedChangesWithoutDocumentId(): void {
 		$this->documentService->hasUnsavedChanges(new Document());
 	}
 
+	private function createBuiltDocument(string $content): Document {
+		$document = new Document();
+		$document->setContextType('file');
+		$document->setContextId(42);
+		$document->setChecksum(DocumentService::computeCheckSum($content));
+		return $document;
+	}
+
 	public function testNoAutosavingWhileSaveIsUnderWay(): void {
 		$document = $this->createDocument('etag1', 1000, 'new content');
 		$context = $this->createMock(FileContext::class);
@@ -134,4 +142,35 @@ public function testNoAutosavingWhileSaveIsUnderWay(): void {
 		self::assertSame($result, $document);
 	}
 
+	public function testKeepsDocumentWhenFileIsUnchanged(): void {
+		$loaded = $this->createDocument('etag1', 1000, 'content');
+		$this->documentMapper->method('load')->willReturn($loaded);
+		$this->documentMapper->expects(self::never())->method('delete');
+		$this->documentMapper->expects(self::never())->method('insert');
+
+		self::assertSame($loaded, $this->documentService->getOrCreateDocument($this->createBuiltDocument('content')));
+	}
+
+	public function testKeepsDocumentWithUnsavedChangesWhenFileChangedOutside(): void {
+		$loaded = $this->createDocument('etag1', 1000, 'content');
+		$loaded->setLastSavedVersion(3);
+		$this->documentMapper->method('load')->willReturn($loaded);
+		$this->stepMapper->method('getLatestVersion')->willReturn(4);
+		$this->documentMapper->expects(self::never())->method('delete');
+		$this->documentMapper->expects(self::never())->method('insert');
+
+		self::assertSame($loaded, $this->documentService->getOrCreateDocument($this->createBuiltDocument('changed outside')));
+	}
+
+	public function testResetsDocumentWhenFileChangedOutside(): void {
+		$loaded = $this->createDocument('etag1', 1000, 'content');
+		$loaded->setLastSavedVersion(3);
+		$this->documentMapper->method('load')->willReturn($loaded);
+		$this->stepMapper->method('getLatestVersion')->willReturn(3);
+		$this->documentMapper->expects(self::once())->method('delete')->with($loaded);
+		$this->documentMapper->expects(self::once())->method('insert')->willReturnArgument(0);
+
+		$built = $this->createBuiltDocument('changed outside');
+		self::assertSame($built, $this->documentService->getOrCreateDocument($built));
+	}
 }