diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 969d6bcd6f0..fe37d17bdd4 100644 --- a/lib/Service/DocumentService.php +++ b/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()) { diff --git a/playwright/e2e/synchronize-content/outside-change.spec.ts b/playwright/e2e/synchronize-content/outside-change.spec.ts new file mode 100644 index 00000000000..9271fbe2057 --- /dev/null +++ b/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() +}) diff --git a/playwright/support/fixtures/User.ts b/playwright/support/fixtures/User.ts index d451847d101..4d0f3ad0c5d 100644 --- a/playwright/support/fixtures/User.ts +++ b/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[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}`] }, + ) + } } diff --git a/src/services/SaveService.ts b/src/services/SaveService.ts index d6a9ea436de..95a87d922e5 100644 --- a/src/services/SaveService.ts +++ b/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, diff --git a/src/tests/services/SaveService.spec.ts b/src/tests/services/SaveService.spec.ts new file mode 100644 index 00000000000..6f77eb0b9ba --- /dev/null +++ b/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({ + 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' }, + }) + }) +}) diff --git a/tests/unit/Service/DocumentServiceTest.php b/tests/unit/Service/DocumentServiceTest.php index 64087365f0f..5ad1c93f693 100644 --- a/tests/unit/Service/DocumentServiceTest.php +++ b/tests/unit/Service/DocumentServiceTest.php @@ -64,6 +64,14 @@ private function createDocument(string $etag, int $mtime, string $content): Docu return $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 testGetAllPendingAttachmentCleanup(): void { $document = $this->createDocument('etag1', 1000, 'content'); $documents = (static function () use ($document): \Generator { @@ -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)); + } }