diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index 13361cf8617..c84834f97d9 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -73,23 +73,19 @@ describe('The session Api', function () { cy.closeConnection(connection) }) - // Echoes all message types but queries - Object.entries(messages) - .filter(([key, _value]) => key !== 'query') - .forEach(([type, sample]) => { - it(`echos ${type} messages`, function () { - const steps = [sample] - const version = 0 - cy.pushSteps({ connection, steps, version }) - .its('version') - .should('eql', 0) - cy.syncSteps(connection) - .its('steps[0].data') - .should('eql', steps) - }) + // Echoes updates and responses + ;['update', 'response'].forEach((type) => { + it(`echos ${type} messages`, function () { + const steps = [messages[type]] + const version = 0 + cy.pushSteps({ connection, steps, version }) + .its('version') + .should('eql', 0) + cy.syncSteps(connection).its('steps[0].data').should('eql', steps) }) + }) - it('responds to queries', function () { + it('responds to queries with updates and responses', function () { const version = 0 Object.entries(messages).forEach(([type, sample]) => { cy.pushSteps({ connection, steps: [sample], version }) @@ -97,10 +93,13 @@ describe('The session Api', function () { cy.pushSteps({ connection, steps: [messages.query], version }).then( (response) => { cy.wrap(response).its('version').should('eql', 0) - cy.wrap(response).its('steps.length').should('eql', 1) + cy.wrap(response).its('steps.length').should('eql', 2) cy.wrap(response) .its('steps[0].data') .should('eql', [messages.update]) + cy.wrap(response) + .its('steps[1].data') + .should('eql', [messages.response]) }, ) }) @@ -111,7 +110,6 @@ describe('The session Api', function () { let connection let fileId let filePath - let joining beforeEach(function () { cy.testName().then((name) => { @@ -156,13 +154,10 @@ describe('The session Api', function () { manualSave: true, }) cy.openConnection({ fileId, filePath }) - .then(({ connection: con, data }) => { - joining = con - return data - }) - .its('documentState') + .as('joining') + .its('data.documentState') .should('eql', documentState) - cy.closeConnection(joining) + cy.get('@joining').its('connection').then(cy.closeConnection) }) afterEach(function () { @@ -175,7 +170,6 @@ describe('The session Api', function () { let connection let filePath let shareToken - let joining beforeEach(function () { cy.testName().then((name) => { @@ -232,13 +226,10 @@ describe('The session Api', function () { manualSave: true, }) cy.openConnection({ filePath: '', token: shareToken }) - .then(({ connection: con, data }) => { - joining = con - return data - }) - .its('documentState') + .as('joining') + .its('data.documentState') .should('eql', documentState) - cy.closeConnection(joining) + cy.get('@joining').its('connection').then(cy.closeConnection) }) }) diff --git a/cypress/e2e/api/SyncServiceProvider.spec.js b/cypress/e2e/api/SyncServiceProvider.spec.js index 8844754aab0..257315af0e4 100644 --- a/cypress/e2e/api/SyncServiceProvider.spec.js +++ b/cypress/e2e/api/SyncServiceProvider.spec.js @@ -43,15 +43,20 @@ describe('Sync service provider', function () { */ function createProvider(ydoc) { const relativePath = '.' - const { connection, openConnection, baseVersionEtag } = provideConnection({ - fileId, - relativePath, - }) - const { syncService } = provideSyncService( - connection, - openConnection, - baseVersionEtag, + let baseVersionEtag + const setBaseVersionEtag = (val) => { + baseVersionEtag = val + } + const getBaseVersionEtag = () => baseVersionEtag + const { connection, openConnection } = provideConnection( + { + fileId, + relativePath, + }, + getBaseVersionEtag, + setBaseVersionEtag, ) + const { syncService } = provideSyncService(connection, openConnection) const queue = [] syncService.bus.on('opened', () => syncService.startSync()) return createSyncServiceProvider({ diff --git a/cypress/e2e/conflict.spec.js b/cypress/e2e/conflict.spec.js deleted file mode 100644 index bd4cfde7c57..00000000000 --- a/cypress/e2e/conflict.spec.js +++ /dev/null @@ -1,180 +0,0 @@ -/** - * SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors - * SPDX-License-Identifier: AGPL-3.0-or-later - */ - -import { randUser } from '../utils/index.js' - -const variants = [ - { fixture: 'lines.txt', mime: 'text/plain' }, - { fixture: 'test.md', mime: 'text/markdown' }, -] -const getWrapper = () => cy.get('.text-editor__wrapper.has-conflicts') - -variants.forEach(function ({ fixture, mime }) { - const user = randUser() - const fileName = fixture - const prefix = mime.replaceAll('/', '-') - describe(`${mime} (${fileName})`, function () { - before(() => { - cy.createUser(user) - }) - - beforeEach(function () { - cy.login(user) - cy.createTestFolder() - }) - - it(prefix + ': no actual conflict - just reload', function () { - cy.testName().then((testName) => { - // start with different content - cy.uploadFile('frontmatter.md', mime, `${testName}/${fileName}`) - // just a read only session opened - cy.shareFile(`${testName}/${fileName}`).then((token) => { - cy.visit(`/s/${token}`) - }) - cy.getContent().should('contain', 'Heading') - - cy.uploadFile(fileName, mime, testName + '/' + fileName) - cy.get('#editor-container .document-status', { - timeout: 40000, - }).should('contain', 'session has expired') - - // Reload button works - cy.get('#editor-container .document-status button') - .contains('Reload') - .click() - getWrapper().should('not.exist') - cy.getContent().should('contain', 'Hello world') - cy.getContent().should('not.contain', 'Heading') - }) - }) - - it(prefix + ': displays conflicts', function () { - createConflict(fileName, 'edited-' + fileName, mime) - - cy.openFile(fileName) - - cy.get('.text-editor .document-status').should( - 'contain', - 'The file was overwritten.', - ) - getWrapper().find('#read-only-editor').should('contain', 'Hello world') - getWrapper() - .find('#read-only-editor') - .should('not.contain', 'cruel conflicting') - - getWrapper().find('.text-editor__main').should('contain', 'Hello world') - getWrapper() - .find('.text-editor__main') - .should('contain', 'cruel conflicting') - }) - - it( - prefix + ': resolves conflict using current editing session', - function () { - createConflict(fileName, 'edited-' + fileName, mime) - - cy.openFile(fileName) - cy.intercept({ method: 'POST', url: '**/session/*/push' }).as('push') - cy.wait('@push') - cy.get('[data-cy="resolveThisVersion"]').click() - - getWrapper().should('not.exist') - cy.get('[data-cy="resolveThisVersion"]').should('not.exist') - cy.getContent().should('contain', 'cruel conflicting') - }, - ) - - it(prefix + ': resolves conflict using server version', function () { - createConflict(fileName, 'edited-' + fileName, mime) - - cy.openFile(fileName) - cy.get('[data-cy="resolveServerVersion"]').click() - - getWrapper().should('not.exist') - cy.get('[data-cy="resolveThisVersion"]').should('not.exist') - cy.get('[data-cy="resolveServerVersion"]').should('not.exist') - cy.getContent().should('contain', 'Hello world') - cy.getContent().should('not.contain', 'cruel conflicting') - }) - - it(prefix + ': hides conflict in read only session', function () { - createConflict(fileName, 'edited-' + fileName, mime) - cy.testName().then((testName) => { - cy.shareFile(`/${testName}/${fileName}`).then((token) => { - cy.logout() - cy.visit(`/s/${token}`) - }) - }) - cy.getContent().should('contain', 'cruel conflicting') - getWrapper().should('not.exist') - }) - - it(prefix + ': no conflict when uploading same file content', function () { - createConflict(fileName, fileName, mime) - cy.openFile(fileName) - cy.getContent().should('contain', 'Hello world') - getWrapper().should('not.exist') - }) - }) -}) - -describe('conflict dialog scroll behaviour', function () { - const user = randUser() - const fileName = 'long.md' - - before(() => { - cy.createUser(user) - }) - - it('document status and collision resolution dialog elements are sticky', function () { - cy.login(user) - cy.createTestFolder() - - createConflict(fileName, 'edited-' + fileName, 'text/markdown') - - cy.openFile(fileName) - - cy.get('.text-editor .document-status').should( - 'contain', - 'The file was overwritten.', - ) - getWrapper() - .find('.text-editor__main h2') - .contains('Third subheading') - .scrollIntoView() - - cy.get('.text-editor #resolve-conflicts').should('be.visible') - cy.get('.text-editor .document-status').should('be.visible') - }) -}) - -/** - * @param {string} fileName1 - filename1 - * @param {string} fileName2 - filename2 - * @param {string} mime - mimetype - */ -function createConflict(fileName1, fileName2, mime) { - cy.testName().then((testName) => { - cy.uploadFile(fileName1, mime, `${testName}/${fileName1}`) - }) - cy.visitTestFolder() - cy.openFile(fileName1) - cy.log('Inspect editor') - cy.getEditor() - .find('.ProseMirror') - .should('have.attr', 'contenteditable', 'true') - - cy.getContent().type('Hello you cruel conflicting world') - - cy.testName().then((testName) => { - cy.uploadFile(fileName2, mime, testName + '/' + fileName1) - }) - - cy.intercept('POST', '**/session/*/sync').as('sync') - cy.wait('@sync', { timeout: 10000 }) - - cy.get('#viewer .modal-header button.header-close').click() - cy.get('#viewer').should('not.exist') -} diff --git a/cypress/e2e/sync.spec.js b/cypress/e2e/sync.spec.js index e02ced4c53f..d86b01c7174 100644 --- a/cypress/e2e/sync.spec.js +++ b/cypress/e2e/sync.spec.js @@ -108,7 +108,10 @@ describe('Sync', () => { 'contain', 'The document could not be loaded.', ) + cy.intercept('**/apps/text/session/*/create').as('create') cy.get('#editor-container .document-status').find('button').click() + // let first attempt fail + cy.wait('@create', { timeout: 10000 }) cy.get('#editor-container .document-status', { timeout: 30000 }).should( 'contain', 'The document could not be loaded.', @@ -117,13 +120,13 @@ describe('Sync', () => { cy.intercept('**/apps/text/session/*/*', (req) => { req.continue() }).as('alive') - cy.intercept('**/apps/text/session/*/create').as('create') cy.get('#editor-container .document-status').find('button').click() + // this is the create request... - now with the alive alias cy.wait('@alive', { timeout: 30000 }) - cy.wait('@create', { timeout: 10000 }) .its('request.body') .should('have.property', 'baseVersionEtag') .should('not.be.empty') + cy.getContent().should('contain', 'Hello world') }) it('recovers from a lost and closed connection', () => { @@ -176,18 +179,9 @@ describe('Sync', () => { cy.wait('@save') cy.uploadTestFile('test.md') - cy.get('#editor-container .document-status', { timeout: 30000 }).should( - 'contain', - 'Editing session has expired.', - ) - - // Reload button works - cy.get('#editor-container .document-status button') - .contains('Reload') - .click() - - cy.getContent() - cy.get('#editor-container .document-status .notecard').should('not.exist') + cy.getContent().should('not.exist') + cy.getContent().find('h2').should('contain', 'Hello world') + cy.getContent().find('li').should('not.exist') // was overwritten after the save }) it('passes the doc content from one session to the next', () => { diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index 111012362fe..800b659ced0 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -11,7 +11,6 @@ namespace OCA\Text\Cron; -use OCA\Text\Exception\DocumentHasUnsavedChangesException; use OCA\Text\Service\AttachmentService; use OCA\Text\Service\DocumentService; use OCA\Text\Service\SessionService; @@ -37,10 +36,6 @@ public function __construct( protected function run($argument): void { $this->logger->debug('Run cleanup job for text documents'); foreach ($this->documentService->getAllWithNoActiveSession() as $document) { - try { - $this->documentService->resetDocument($document->getId()); - } catch (DocumentHasUnsavedChangesException) { - } $this->attachmentService->cleanupAttachments($document->getId()); } diff --git a/lib/Listeners/BeforeNodeWrittenListener.php b/lib/Listeners/BeforeNodeWrittenListener.php index b634a9f396c..9a145639ed9 100644 --- a/lib/Listeners/BeforeNodeWrittenListener.php +++ b/lib/Listeners/BeforeNodeWrittenListener.php @@ -40,12 +40,12 @@ public function handle(Event $event): void { } // Reset document session to avoid manual conflict resolution if there's no unsaved steps try { - $this->documentService->resetDocument($node->getId()); + $this->documentService->resetDocument($node->getId(), true); } catch (DocumentHasUnsavedChangesException|NotFoundException $e) { // Do not throw during event handling in this is expected to happen // DocumentHasUnsavedChangesException: A document editing session is likely ongoing, someone can resolve the conflict // NotFoundException: The event was called oin a file that was just created so a NonExistingFile object is used that has no id yet - $this->logger->debug('Reset document skipped in BeforeNodeWrittenEvent', ['exception' => $e]); + $this->logger->warning('Reset document skipped in BeforeNodeWrittenEvent', ['exception' => $e]); } } } diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index d3c26ab7524..2408ab559f9 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -208,8 +208,12 @@ public function addStep(Document $document, Session $session, array $steps, int if ($readOnly && $message->isUpdate()) { continue; } + // Only accept sync protocol + if ($message->getYjsMessageType() !== YjsMessage::YJS_MESSAGE_SYNC) { + continue; + } // Filter out query steps as they would just trigger clients to send their steps again - if ($message->getYjsMessageType() === YjsMessage::YJS_MESSAGE_SYNC && $message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_STEP1) { + if ($message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_STEP1) { $stepsIncludeQuery = true; } else { $stepsToInsert[] = $step; @@ -249,7 +253,7 @@ public function addStep(Document $document, Session $session, array $steps, int $stepsToReturn = []; foreach ($allSteps as $step) { $message = YjsMessage::fromBase64($step->getData()); - if ($message->getYjsMessageType() === YjsMessage::YJS_MESSAGE_SYNC && $message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_UPDATE) { + if ($message->isUpdate()) { $stepsToReturn[] = $step; } } diff --git a/package-lock.json b/package-lock.json index aa7db21d982..3305b15039a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -83,6 +83,7 @@ "vue-click-outside": "^1.1.0", "vue-material-design-icons": "^5.3.1", "webdav": "^5.9.0", + "y-indexeddb": "^9.0.12", "y-prosemirror": "^1.3.7", "y-protocols": "^1.0.7", "yjs": "^13.6.30" @@ -21885,6 +21886,26 @@ "node": ">=0.4" } }, + "node_modules/y-indexeddb": { + "version": "9.0.12", + "resolved": "https://registry.npmjs.org/y-indexeddb/-/y-indexeddb-9.0.12.tgz", + "integrity": "sha512-9oCFRSPPzBK7/w5vOkJBaVCQZKHXB/v6SIT+WYhnJxlEC61juqG0hBrAf+y3gmSMLFLwICNH9nQ53uscuse6Hg==", + "license": "MIT", + "dependencies": { + "lib0": "^0.2.74" + }, + "engines": { + "node": ">=16.0.0", + "npm": ">=8.0.0" + }, + "funding": { + "type": "GitHub Sponsors ❤", + "url": "https://github.com/sponsors/dmonad" + }, + "peerDependencies": { + "yjs": "^13.0.0" + } + }, "node_modules/y-prosemirror": { "version": "1.3.7", "resolved": "https://registry.npmjs.org/y-prosemirror/-/y-prosemirror-1.3.7.tgz", diff --git a/package.json b/package.json index 6bc5dfa762b..7e2e907215f 100644 --- a/package.json +++ b/package.json @@ -103,6 +103,7 @@ "vue-click-outside": "^1.1.0", "vue-material-design-icons": "^5.3.1", "webdav": "^5.9.0", + "y-indexeddb": "^9.0.12", "y-prosemirror": "^1.3.7", "y-protocols": "^1.0.7", "yjs": "^13.6.30" diff --git a/playwright/e2e/autosave.spec.ts b/playwright/e2e/autosave.spec.ts new file mode 100644 index 00000000000..9a51a583709 --- /dev/null +++ b/playwright/e2e/autosave.spec.ts @@ -0,0 +1,65 @@ +/** + * SPDX-FileCopyrightText: 2025 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' +import { test as offlineTest } from '../support/fixtures/offline' +import { test as uploadFileTest } from '../support/fixtures/upload-file' + +const test = mergeTests(editorTest, offlineTest, uploadFileTest) + +// As we switch on and off the network +// we cannot run tests in parallel. +test.describe.configure({ mode: 'serial' }) + +test.beforeEach(async ({ open }) => { + await open() +}) + +test('saves after 30 seconds', async ({ editor, page }) => { + await page.clock.install() + await expect(editor.el).toBeVisible() + await editor.typeHeading('Hello world') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + await page.clock.fastForward(30_000) + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + // TODO: Why does this not work? await expect(await file.getContent()).toBe('## Hello world') +}) + +test('saves after being disconnected for 20 sec.', async ({ + editor, + page, + setOffline, + setOnline, +}) => { + await page.clock.install() + await expect(editor.el).toBeVisible() + await editor.typeHeading('Hello world') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + setOffline() + await page.clock.fastForward(20_000) + setOnline() + await page.clock.fastForward(20_000) + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + // TODO: Why does this not work? await expect(await file.getContent()).toBe('## Hello world') +}) + +test('saves after being disconnected for 2 minutes', async ({ + editor, + page, + setOffline, + setOnline, +}) => { + await page.clock.install() + await expect(editor.el).toBeVisible() + await editor.typeHeading('Hello world') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + setOffline() + await page.clock.fastForward(120_000) + setOnline() + await page.clock.fastForward(40_000) + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + // TODO: Why does this not work? await expect(await file.getContent()).toBe('## Hello world') +}) diff --git a/playwright/e2e/conflict.spec.ts b/playwright/e2e/conflict.spec.ts new file mode 100644 index 00000000000..acb21324677 --- /dev/null +++ b/playwright/e2e/conflict.spec.ts @@ -0,0 +1,264 @@ +/** + * 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' +import { test as offlineTest } from '../support/fixtures/offline' +import { test as randomUserTest } from '../support/fixtures/random-user' +import { test as uploadFileTest } from '../support/fixtures/upload-file' + +const test = mergeTests(editorTest, offlineTest, randomUserTest, uploadFileTest) + +// As we switch on and off the network +// we cannot run tests in parallel. +test.describe.configure({ mode: 'serial' }) + +test.beforeEach(async ({ open }) => { + await open() +}) + +const resolutionVariants = [ + { + source: 'local', + buttonName: /Overwrite the file and save /, + headingName: 'Hello world', + }, + { + source: 'server', + buttonName: /Discard the changes and edit /, + headingName: 'Good bye', + }, +] + +resolutionVariants.forEach(({ source, buttonName, headingName }) => { + test(`[conflict] manual resolution after typing while offline - resolve with ${source} version`, async ({ + container, + editor, + file, + reader, + setOffline, + setOnline, + user, + }) => { + await expect(editor.el).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await setOnline() + await user.uploadFile({ name: file.name, content: '## Good bye' }) + + // Verify both verisons are shown + await expect(editor.getHeading({ name: 'Good bye' })).toBeVisible() + await expect(reader.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() + }) +}) + +resolutionVariants.forEach(({ source, buttonName, headingName }) => { + test(`[conflict] manual resolution after typing while offline and reopening editor - resolve with ${source} version`, async ({ + close, + container, + editor, + file, + reader, + setOffline, + setOnline, + user, + }) => { + await expect(editor.el).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await close() + await setOnline() + await user.uploadFile({ name: file.name, content: '## Good bye' }) + await file.open() + + // Verify both verisons are shown + await expect(editor.getHeading({ name: 'Good bye' })).toBeVisible() + await expect(reader.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() + }) +}) + +resolutionVariants.forEach(({ source, buttonName, headingName }) => { + test(`[conflict] manual resolution after typing while online - resolve with ${source} version`, async ({ + container, + editor, + file, + reader, + user, + }) => { + await expect(editor.el).toBeVisible() + await editor.typeHeading('Hello world') + await user.uploadFile({ name: file.name, content: '## Good bye' }) + + // Verify both verisons are shown + await expect(editor.getHeading({ name: 'Good bye' })).toBeVisible() + await expect(reader.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() + }) +}) + +test.describe('Plaintext conflict resolution', () => { + test.use({ fileName: 'empty.txt' }) + const plaintextResolutionVariants = [ + { + source: 'local', + buttonName: /Overwrite the file and save /, + content: 'Hello world', + }, + { + source: 'server', + buttonName: /Discard the changes and edit /, + content: 'Good bye', + }, + ] + + plaintextResolutionVariants.forEach(({ source, buttonName, content }) => { + test(`[conflict, plaintext] manual resolution after typing while online - resolve with ${source} version`, async ({ + container, + editor, + file, + reader, + user, + }) => { + await expect(editor.el).toBeVisible() + await editor.type('Hello world') + await user.uploadFile({ name: file.name, content: 'Good bye' }) + + // Verify both verisons are shown + await expect(editor.content).toHaveText('Good bye') + await expect(reader.content).toHaveText('Hello world') + + // Resolve conflict + await container.getButton({ name: buttonName }).click() + await expect(reader.el).not.toBeVisible() + await expect(editor.content).toHaveText(content) + }) + }) +}) + +test('[conflict] automatic resolution if no unsaved changes', async ({ + container, + editor, + file, + page, + reader, + user, +}) => { + await expect(editor.el).toBeVisible() + await editor.typeHeading('Hello world') + const requestPromise = page.waitForRequest(/save/) + await editor.saveIndicator.click() + await requestPromise + await page.waitForTimeout(500) // More robust against 423 Locked + + await user.uploadFile({ name: file.name, content: '## Good bye' }) + + // Should show latest content, no conflict dialog + await expect(editor.getHeading({ name: 'Good bye' })).toBeVisible() + await expect(reader.content).not.toBeVisible() + await expect(container.getButton({ name: /Overwrite/ })).not.toBeVisible() +}) + +test('readonly session hides conflict dialog', async ({ + container, + close, + editor, + file, + page, + setOffline, + setOnline, + user, +}) => { + await expect(editor.el).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await close() + + await setOnline() + await user.uploadFile({ name: file.name, content: '## Good bye' }) + + // Share file and open as guest (read-only) + const { token } = await file.shareLink() + await page.goto(`/s/${token}`) + + // Should show latest content, no conflict dialog + + await expect(editor.getHeading({ name: 'Good bye' })).toBeVisible() + await expect(container.getButton({ name: /Overwrite/ })).not.toBeVisible() +}) + +test.skip('no conflict when uploading identical content', async ({ + close, + container, + editor, + file, + reader, + setOffline, + setOnline, + user, +}) => { + await expect(editor.el).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await close() + + await setOnline() + await user.uploadFile({ name: file.name, content: '## Hello world' }) + + await file.open() + + await expect(editor.getHeading({ name: 'Hello world' })).toBeVisible() + await expect(reader.getHeading({ name: 'Hello world' })).toBeVisible() + await expect(container.getButton({ name: /Overwrite/ })).not.toBeVisible() + await expect(editor.getHeading({ name: 'Hello world' })).toBeVisible() +}) + +test('conflict dialog is sticky when scrolling', async ({ + close, + container, + file, + editor, + page, + reader, + setOffline, + setOnline, + user, +}) => { + await expect(editor.el).toBeVisible() + await editor.typeHeading('Long content\n') + await setOffline() + + for (let i = 1; i < 8; i++) { + await editor.typeHeading(`Section ${i}`) + await editor.type('\n\nLorem ipsum dolor sit amet.\n\n') + } + await close() + + await setOnline() + await user.uploadFile({ name: file.name, content: '## Different content' }) + await file.open() + + await expect(editor.getHeading({ name: 'Different content' })).toBeVisible() + await expect(reader.getHeading({ name: 'Long content' })).toBeVisible() + + // Scroll down + await reader.getHeading({ name: 'Section 7' }).scrollIntoViewIfNeeded() + + await expect(container.getButton({ name: /Overwrite/ })).toBeVisible() + await expect(page.locator('.document-status')).toBeVisible() +}) diff --git a/playwright/e2e/indexed-db.spec.ts b/playwright/e2e/indexed-db.spec.ts new file mode 100644 index 00000000000..034b079704a --- /dev/null +++ b/playwright/e2e/indexed-db.spec.ts @@ -0,0 +1,56 @@ +/** + * SPDX-FileCopyrightText: 2025 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' +import { test as offlineTest } from '../support/fixtures/offline' +import { test as randomUserTest } from '../support/fixtures/random-user' +import { test as uploadFileTest } from '../support/fixtures/upload-file' + +const test = mergeTests(editorTest, offlineTest, randomUserTest, uploadFileTest) + +// As we switch on and off the network +// we cannot run tests in parallel. +test.describe.configure({ mode: 'serial' }) + +test.beforeEach(async ({ open }) => { + await open() +}) + +test('recovering from indexed db', async ({ + close, + editor, + file, + page, + setOffline, + setOnline, +}) => { + await expect(editor.el).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await close() + await setOnline() + const pushWithSteps = page.waitForRequest( + (request) => { + if (!request.url().includes('push')) { + return false + } + const data = request.postDataJSON() + return data.steps.length > 0 + }, + { timeout: 10_000 }, + ) + await file.open() + await expect(editor.getHeading({ name: 'Hello world' })).toBeVisible() + await expect(editor.offlineState).not.toBeVisible() + await pushWithSteps + await expect(editor.saveIndicator).not.toHaveAttribute( + 'title', + /Unsaved changes/, + ) + await expect + .poll(() => file.getContent(), { timeout: 10_000 }) + .toBe('## Hello world') +}) diff --git a/playwright/e2e/offline.spec.ts b/playwright/e2e/offline.spec.ts index 3492a24e3ac..8dbc0267e34 100644 --- a/playwright/e2e/offline.spec.ts +++ b/playwright/e2e/offline.spec.ts @@ -52,5 +52,5 @@ test('typing offline and coming back online', async ({ await editor.typeHeading('Hello world') await setOnline() await expect(editor.offlineState).not.toBeVisible() - await expect(editor.saveIndicator).toHaveAttribute('title', /Unsaved changes/) + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) }) diff --git a/playwright/e2e/reconnect.spec.ts b/playwright/e2e/reconnect.spec.ts new file mode 100644 index 00000000000..e2c4b1cbfe7 --- /dev/null +++ b/playwright/e2e/reconnect.spec.ts @@ -0,0 +1,40 @@ +/** + * SPDX-FileCopyrightText: 2025 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' +import { test as offlineTest } from '../support/fixtures/offline' +import { test as uploadFileTest } from '../support/fixtures/upload-file' + +const test = mergeTests(editorTest, offlineTest, uploadFileTest) + +// As we switch on and off the network +// we cannot run tests in parallel. +test.describe.configure({ mode: 'serial' }) + +test.beforeEach(async ({ open }) => { + await open() +}) + +test('opening a file with unsaved changes', async ({ + editor, + file, + open, + close, + setOffline, + setOnline, +}) => { + await expect(editor.el).toBeVisible() + await setOffline() + await editor.typeHeading('Hello world') + await expect(editor.saveIndicator).toHaveAccessibleName(/Unsaved changes/) + await close() + await setOnline() + await expect(await file.getContent()).toBe('') + await open() + await expect(editor.getHeading({ name: 'Hello world' })).toBeVisible() + await expect(editor.saveIndicator).not.toHaveAccessibleName(/Unsaved changes/) + await expect(await file.getContent()).toBe('## Hello world') +}) diff --git a/playwright/support/fixtures/Node.ts b/playwright/support/fixtures/Node.ts index 941140f15cb..82160da92ed 100644 --- a/playwright/support/fixtures/Node.ts +++ b/playwright/support/fixtures/Node.ts @@ -124,4 +124,9 @@ export class Node { return { token, id } } + async getContent() { + const response = await this.page.request.get(`/remote.php/webdav/${this.name}`) + return response.text() + } + } diff --git a/playwright/support/fixtures/editor.ts b/playwright/support/fixtures/editor.ts index bb217bffa9a..66d3734f49b 100644 --- a/playwright/support/fixtures/editor.ts +++ b/playwright/support/fixtures/editor.ts @@ -5,9 +5,13 @@ import { test as baseTest } from '@playwright/test' import { EditorSection } from '../sections/EditorSection' +import { ReaderSection } from '../sections/ReaderSection' +import { ContainerSection } from '../sections/ContainerSection' interface EditorFixture { editor: EditorSection + reader: ReaderSection + container: ContainerSection } export const test = baseTest.extend({ @@ -15,4 +19,12 @@ export const test = baseTest.extend({ const editor = new EditorSection(page) await use(editor) }, + reader: async ({ page }, use) => { + const reader = new ReaderSection(page) + await use(reader) + }, + container: async ({ page }, use) => { + const container = new ContainerSection(page) + await use(container) + }, }) diff --git a/playwright/support/fixtures/upload-file.ts b/playwright/support/fixtures/upload-file.ts index 578b5283e79..287c3934abd 100644 --- a/playwright/support/fixtures/upload-file.ts +++ b/playwright/support/fixtures/upload-file.ts @@ -39,5 +39,6 @@ export const test = base.extend({ open: ({ file }, use) => use(() => file.open()), viewer: ({ fileName, page }, use) => use(new ViewerSection(fileName, page)), + close: ({ viewer }, use) => use(() => viewer.close()), }) diff --git a/playwright/support/sections/ContainerSection.ts b/playwright/support/sections/ContainerSection.ts new file mode 100644 index 00000000000..adedbc2b432 --- /dev/null +++ b/playwright/support/sections/ContainerSection.ts @@ -0,0 +1,22 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { Locator, Page } from '@playwright/test' + +/** + * Container of the editor and possibly conflict dialog, reader for the other version etc. + * + * Currently used to test the other version in conflict dialogs + */ +export class ContainerSection { + public readonly el: Locator + + // eslint-disable-next-line no-useless-constructor + constructor(public readonly page: Page) { + this.el = this.page.locator('#editor-container').first() + } + + getButton = (options: object = {}) => this.el.getByRole('button', options) +} diff --git a/playwright/support/sections/ReaderSection.ts b/playwright/support/sections/ReaderSection.ts new file mode 100644 index 00000000000..95e76693c59 --- /dev/null +++ b/playwright/support/sections/ReaderSection.ts @@ -0,0 +1,24 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { Locator, Page } from '@playwright/test' + +/** + * Section for the read only editor + * + * Currently used to test the other version in conflict dialogs + */ +export class ReaderSection { + public readonly el: Locator + public readonly content: Locator + + // eslint-disable-next-line no-useless-constructor + constructor(public readonly page: Page) { + this.el = this.page.locator('#read-only-editor').first() + this.content = this.el.getByRole('textbox') + } + + getHeading = (options: object = {}) => this.content.getByRole('heading', options) +} diff --git a/src/EditorFactory.js b/src/EditorFactory.ts similarity index 70% rename from src/EditorFactory.js rename to src/EditorFactory.ts index 7b69352e0d7..88505340ce0 100644 --- a/src/EditorFactory.js +++ b/src/EditorFactory.ts @@ -5,21 +5,23 @@ import 'proxy-polyfill' -import { Editor } from '@tiptap/core' +import { Editor, Extension } from '@tiptap/core' import hljs from 'highlight.js/lib/core' import { createLowlight } from 'lowlight' +import type { Node } from '@tiptap/pm/model' +import type { Connection } from './composables/useConnection' import { FocusTrap, PlainText, RichText } from './extensions/index.js' -import { logger } from './helpers/logger.js' +import { logger } from './helpers/logger' const lowlight = createLowlight() -const loadSyntaxHighlight = async (language) => { +const loadSyntaxHighlight = async (language: string) => { const list = hljs.listLanguages() logger.debug('Supported languages', { list }) if (!lowlight.listLanguages().includes(language)) { try { - logger.debug('Loading language', language) + logger.debug('Loading language ' + language) // eslint-disable-next-line n/no-missing-import const syntax = await import( `../node_modules/highlight.js/lib/languages/${language}.js` @@ -43,6 +45,12 @@ const createRichEditor = ({ relativePath, isEmbedded = false, mentionSearch = undefined, +}: { + extensions?: Extension[] + connection?: Connection + relativePath?: string + isEmbedded?: boolean + mentionSearch?: (query: string) => Promise> } = {}) => { return new Editor({ editorProps, @@ -59,7 +67,10 @@ const createRichEditor = ({ }) } -const createPlainEditor = ({ language = 'plaintext', extensions = [] } = {}) => { +const createPlainEditor = ({ + language = 'plaintext', + extensions = [], +}: { language?: string; extensions?: Extension[] } = {}) => { return new Editor({ editorProps, extensions: [ @@ -74,7 +85,7 @@ const createPlainEditor = ({ language = 'plaintext', extensions = [] } = {}) => }) } -const serializePlainText = (doc) => { +const serializePlainText = (doc: Node) => { return doc.textContent } diff --git a/src/components/CollisionResolveDialog.vue b/src/components/CollisionResolveDialog.vue index f66a229e68a..82a72eb2ace 100644 --- a/src/components/CollisionResolveDialog.vue +++ b/src/components/CollisionResolveDialog.vue @@ -12,18 +12,16 @@ :wide="true" size="large" :disabled="clicked" - data-cy="resolveThisVersion" - @click="resolveThisVersion"> - {{ t('text', 'Overwrite the file and save the current changes') }} + data-cy="useEditorVersion" + @click="useEditorVersion"> + {{ textForSource[editorSource] }} - {{ - t('text', 'Discard the current changes and load the latest version') - }} + data-cy="useReaderVersion" + @click="useReaderVersion"> + {{ textForSource[readerSource] }} @@ -35,23 +33,38 @@ import { useEditor } from '../composables/useEditor.ts' import { useEditorMethods } from '../composables/useEditorMethods.ts' import { useSaveService } from '../composables/useSaveService.ts' import { useSyncService } from '../composables/useSyncService.ts' +import { logger } from '../helpers/logger.ts' export default { name: 'CollisionResolveDialog', components: { NcButton, }, props: { - syncError: { - type: Object, - default: null, + otherVersion: { + type: String, + required: true, + }, + readerSource: { + type: String, + required: true, }, }, - setup() { + setup(props) { + if (!['local', 'server'].includes(props.readerSource)) { + logger.warn('Invalid reader source', props) + } const { editor } = useEditor() const { syncService } = useSyncService() const { saveService } = useSaveService() const { setContent, setEditable } = useEditorMethods(editor) + const editorSource = props.readerSource === 'local' ? 'server' : 'local' + const textForSource = { + local: t('text', 'Overwrite the file and save the unsaved changes'), + server: t('text', 'Discard the changes and edit the latest version'), + } return { + editorSource, + textForSource, setContent, setEditable, saveService, @@ -65,17 +78,22 @@ export default { } }, methods: { - resolveThisVersion() { + useEditorVersion() { this.clicked = true - this.saveService.forceSave().then(() => this.syncService.syncUp()) + this.saveService.forceSave().then(() => { + this.syncService.syncUp() + this.$emit('resolved') + }) this.setEditable(!this.readOnly) }, - resolveServerVersion() { - const { outsideChange } = this.syncError.data + useReaderVersion() { this.clicked = true this.setEditable(!this.readOnly) - this.setContent(outsideChange) - this.saveService.forceSave().then(() => this.syncService.syncUp()) + this.setContent(this.otherVersion) + this.saveService.forceSave().then(() => { + this.syncService.syncUp() + this.$emit('resolved') + }) }, }, } diff --git a/src/components/Editor.js b/src/components/Editor.js new file mode 100644 index 00000000000..cb0bdcb6d20 --- /dev/null +++ b/src/components/Editor.js @@ -0,0 +1,34 @@ +/** + * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { defineComponent, h, nextTick, ref, watch } from 'vue' +import Editor from './Editor.vue' + +export default defineComponent({ + emits: ['focus', 'ready'], + props: Editor.props, + setup(props, { attrs, emit, slots }) { + const reloading = ref(false) + watch(reloading, (val) => { + if (val) { + nextTick(() => { + reloading.value = false + }) + } + }) + return () => + !reloading.value + && h(Editor, { + attrs, + props, + on: { + focus: () => emit('focus'), + ready: () => emit('ready'), + reload: () => (reloading.value = true), + }, + scopedSlots: slots, + }) + }, +}) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 8b8442567b7..3ec274bfa98 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -12,11 +12,15 @@ :class="{ 'is-mobile': isMobile }" tabindex="-1"> - + @@ -30,7 +34,7 @@ :document="document" :dirty="dirty" :sync-error="syncError" - :has-connection-issue="requireReconnect" /> + :has-connection-issue="displayConnectionIssue" /> @@ -47,7 +51,7 @@ :document="document" :dirty="dirty" :sync-error="syncError" - :has-connection-issue="requireReconnect" /> + :has-connection-issue="displayConnectionIssue" />