diff --git a/composer/composer/autoload_classmap.php b/composer/composer/autoload_classmap.php index da85631f4b9..dd4f029eb45 100644 --- a/composer/composer/autoload_classmap.php +++ b/composer/composer/autoload_classmap.php @@ -31,6 +31,7 @@ 'OCA\\Text\\Event\\LoadEditor' => $baseDir . '/../lib/Event/LoadEditor.php', 'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => $baseDir . '/../lib/Exception/DocumentHasUnsavedChangesException.php', 'OCA\\Text\\Exception\\DocumentSaveConflictException' => $baseDir . '/../lib/Exception/DocumentSaveConflictException.php', + 'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => $baseDir . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php', 'OCA\\Text\\Exception\\InvalidSessionException' => $baseDir . '/../lib/Exception/InvalidSessionException.php', 'OCA\\Text\\Exception\\UploadException' => $baseDir . '/../lib/Exception/UploadException.php', 'OCA\\Text\\Exception\\VersionMismatchException' => $baseDir . '/../lib/Exception/VersionMismatchException.php', @@ -38,12 +39,14 @@ 'OCA\\Text\\Listeners\\BeforeAssistantNotificationListener' => $baseDir . '/../lib/Listeners/BeforeAssistantNotificationListener.php', 'OCA\\Text\\Listeners\\BeforeNodeDeletedListener' => $baseDir . '/../lib/Listeners/BeforeNodeDeletedListener.php', 'OCA\\Text\\Listeners\\BeforeNodeRenamedListener' => $baseDir . '/../lib/Listeners/BeforeNodeRenamedListener.php', + 'OCA\\Text\\Listeners\\BeforeNodeWrittenListener' => $baseDir . '/../lib/Listeners/BeforeNodeWrittenListener.php', 'OCA\\Text\\Listeners\\FilesLoadAdditionalScriptsListener' => $baseDir . '/../lib/Listeners/FilesLoadAdditionalScriptsListener.php', 'OCA\\Text\\Listeners\\FilesSharingLoadAdditionalScriptsListener' => $baseDir . '/../lib/Listeners/FilesSharingLoadAdditionalScriptsListener.php', 'OCA\\Text\\Listeners\\LoadEditorListener' => $baseDir . '/../lib/Listeners/LoadEditorListener.php', 'OCA\\Text\\Listeners\\LoadViewerListener' => $baseDir . '/../lib/Listeners/LoadViewerListener.php', 'OCA\\Text\\Listeners\\NodeCopiedListener' => $baseDir . '/../lib/Listeners/NodeCopiedListener.php', 'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => $baseDir . '/../lib/Listeners/RegisterDirectEditorEventListener.php', + 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentBaseVersionEtag' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php', 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSession.php', 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php', 'OCA\\Text\\Middleware\\SessionMiddleware' => $baseDir . '/../lib/Middleware/SessionMiddleware.php', diff --git a/composer/composer/autoload_static.php b/composer/composer/autoload_static.php index 8732ef4a1dc..aac06275a14 100644 --- a/composer/composer/autoload_static.php +++ b/composer/composer/autoload_static.php @@ -46,6 +46,7 @@ class ComposerStaticInitText 'OCA\\Text\\Event\\LoadEditor' => __DIR__ . '/..' . '/../lib/Event/LoadEditor.php', 'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => __DIR__ . '/..' . '/../lib/Exception/DocumentHasUnsavedChangesException.php', 'OCA\\Text\\Exception\\DocumentSaveConflictException' => __DIR__ . '/..' . '/../lib/Exception/DocumentSaveConflictException.php', + 'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => __DIR__ . '/..' . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php', 'OCA\\Text\\Exception\\InvalidSessionException' => __DIR__ . '/..' . '/../lib/Exception/InvalidSessionException.php', 'OCA\\Text\\Exception\\UploadException' => __DIR__ . '/..' . '/../lib/Exception/UploadException.php', 'OCA\\Text\\Exception\\VersionMismatchException' => __DIR__ . '/..' . '/../lib/Exception/VersionMismatchException.php', @@ -53,12 +54,14 @@ class ComposerStaticInitText 'OCA\\Text\\Listeners\\BeforeAssistantNotificationListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeAssistantNotificationListener.php', 'OCA\\Text\\Listeners\\BeforeNodeDeletedListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeNodeDeletedListener.php', 'OCA\\Text\\Listeners\\BeforeNodeRenamedListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeNodeRenamedListener.php', + 'OCA\\Text\\Listeners\\BeforeNodeWrittenListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeNodeWrittenListener.php', 'OCA\\Text\\Listeners\\FilesLoadAdditionalScriptsListener' => __DIR__ . '/..' . '/../lib/Listeners/FilesLoadAdditionalScriptsListener.php', 'OCA\\Text\\Listeners\\FilesSharingLoadAdditionalScriptsListener' => __DIR__ . '/..' . '/../lib/Listeners/FilesSharingLoadAdditionalScriptsListener.php', 'OCA\\Text\\Listeners\\LoadEditorListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadEditorListener.php', 'OCA\\Text\\Listeners\\LoadViewerListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadViewerListener.php', 'OCA\\Text\\Listeners\\NodeCopiedListener' => __DIR__ . '/..' . '/../lib/Listeners/NodeCopiedListener.php', 'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => __DIR__ . '/..' . '/../lib/Listeners/RegisterDirectEditorEventListener.php', + 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentBaseVersionEtag' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php', 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSession.php', 'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php', 'OCA\\Text\\Middleware\\SessionMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/SessionMiddleware.php', diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index 6b8ffe74314..0f4eaaf46dd 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -315,7 +315,7 @@ describe('The session Api', function() { }) }) - it('sends initial content if other session is alive but did not push any steps', function() { + it('does not send initial content if other session is alive but did not push any steps', function() { let joining cy.createTextSession(undefined, { filePath: '', shareToken }) .then(con => { @@ -323,7 +323,7 @@ describe('The session Api', function() { return con }) .its('state.documentSource') - .should('not.eql', '') + .should('eql', '## Hello world\n') .then(() => joining.close()) .then(() => connection.close()) }) @@ -339,11 +339,33 @@ describe('The session Api', function() { return con }) .its('state.documentSource') - .should('eql', '') + .should('eql', '## Hello world\n') .then(() => joining.close()) .then(() => connection.close()) }) + it('refuses create,push,sync,save with non-matching baseVersionEtag', function() { + cy.failToCreateTextSession(undefined, 'wrongBaseVersionEtag', { filePath: '', shareToken }) + .its('status') + .should('eql', 412) + + connection.setBaseVersionEtag('wrongBaseVersionEtag') + + cy.failToPushSteps({ connection, steps: [messages.update], version }) + .its('status') + .should('equal', 412) + + cy.failToSyncSteps(connection, { version: 0 }) + .its('status') + .should('equal', 412) + + cy.failToSave(connection) + .its('status') + .should('equal', 412) + + cy.then(() => connection.close()) + }) + it('recovers session even if last person leaves right after create', function() { let joining cy.log('Initial user pushes steps') diff --git a/cypress/e2e/conflict.spec.js b/cypress/e2e/conflict.spec.js index 9e5021dc6c1..1396f6f9bef 100644 --- a/cypress/e2e/conflict.spec.js +++ b/cypress/e2e/conflict.spec.js @@ -54,7 +54,8 @@ variants.forEach(function({ fixture, mime }) { cy.get('#viewer .modal-header button.header-close').click() cy.get('#viewer').should('not.exist') cy.openFile(fileName) - cy.get('.text-editor .document-status .icon-error') + cy.get('.text-editor .document-status') + .should('contain', 'Document has been changed outside of the editor.') getWrapper() .find('#read-only-editor') .should('contain', 'Hello world') diff --git a/cypress/e2e/initial.spec.js b/cypress/e2e/initial.spec.js new file mode 100644 index 00000000000..c9372c22ff7 --- /dev/null +++ b/cypress/e2e/initial.spec.js @@ -0,0 +1,160 @@ +/** + * @copyright Copyright (c) 2020 Julius Härtl + * + * @author Julius Härtl + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +import { randUser } from '../utils/index.js' + +const user = randUser() + +describe('Test state loading of documents', function() { + before(function() { + // Init user + cy.createUser(user) + cy.login(user) + cy.uploadFile('test.md', 'text/markdown') + cy.uploadFile('test.md', 'text/markdown', 'test2.md') + cy.uploadFile('test.md', 'text/markdown', 'test3.md') + }) + beforeEach(function() { + cy.login(user) + }) + + it('Initial content can not be undone', function() { + cy.shareFile('/test.md', { edit: true }) + .then((token) => { + cy.visit(`/s/${token}`) + }) + .then(() => { + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + + cy.getMenu().should('be.visible') + cy.getActionEntry('undo').should('be.visible').click() + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + }) + }) + + it('Consecutive sessions work properly', function() { + let readToken = null + let writeToken = null + cy.interceptCreate() + cy.shareFile('/test2.md') + .then((token) => { + readToken = token + cy.logout() + cy.visit(`/s/${readToken}`) + cy.wait('@create') + }) + .then(() => { + // Open read only for the first time + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.closeInterceptedSession(readToken) + + // Open read only for the second time + cy.reload() + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.closeInterceptedSession(readToken) + + cy.login(user) + cy.shareFile('/test2.md', { edit: true }) + .then((token) => { + writeToken = token + // Open write link and edit something + cy.visit(`/s/${writeToken}`) + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.getContent() + .type('Something new {end}') + cy.intercept({ method: 'POST', url: '**/session/*/push' }).as('push') + cy.intercept({ method: 'POST', url: '**/session/*/sync' }).as('sync') + cy.wait('@push') + cy.wait('@sync') + cy.closeInterceptedSession(writeToken) + + // Reopen read only link and check if changes are there + cy.visit(`/s/${readToken}`) + cy.getEditor().should('be.visible') + cy.getContent() + .find('h2').should('contain', 'Something new Hello world') + }) + }) + }) + + it('Load after state has been saved', function() { + let readToken = null + let writeToken = null + cy.interceptCreate() + cy.shareFile('/test3.md', { edit: true }) + .then((token) => { + writeToken = token + cy.logout() + cy.visit(`/s/${writeToken}`) + }) + .then(() => { + // Open a file, write and save + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Hello world') + cy.getContent() + .type('Something new {end}') + cy.intercept({ method: 'POST', url: '**/session/*/save' }).as('save') + cy.get('.save-status button').click() + cy.wait('@save', { timeout: 10000 }) + cy.closeInterceptedSession(writeToken) + + // Open writable file again and assert the content + cy.reload() + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Something new Hello world') + + cy.login(user) + cy.shareFile('/test3.md') + .then((token) => { + readToken = token + cy.logout() + cy.visit(`/s/${readToken}`) + }) + .then(() => { + // Open read only file again and assert the content + cy.getEditor().should('be.visible') + cy.getContent() + .should('contain', 'Hello world') + .find('h2').should('contain', 'Something new Hello world') + }) + }) + }) + +}) diff --git a/cypress/e2e/share.spec.js b/cypress/e2e/share.spec.js index 5eb124baf8c..e8282d5b775 100644 --- a/cypress/e2e/share.spec.js +++ b/cypress/e2e/share.spec.js @@ -151,7 +151,7 @@ describe('Open test.md in viewer', function() { cy.login(recipient) cy.visit('/apps/files') cy.openFile('test.md') - cy.getModal().find('.empty-content__name').should('contain', 'Failed to load file') + cy.getModal().find('.document-status').should('contain', 'This file cannot be displayed as download is disabled by the share') cy.getModal().getContent().should('not.exist') }) }) diff --git a/cypress/e2e/sync.spec.js b/cypress/e2e/sync.spec.js index 01a68ab7035..c6338d82804 100644 --- a/cypress/e2e/sync.spec.js +++ b/cypress/e2e/sync.spec.js @@ -74,7 +74,7 @@ describe('Sync', () => { }).as('sessionRequests') cy.wait('@dead', { timeout: 30000 }) cy.get('#editor-container .document-status', { timeout: 30000 }) - .should('contain', 'File could not be loaded') + .should('contain', 'Document could not be loaded.') .then(() => { reconnect = true }) @@ -83,7 +83,7 @@ describe('Sync', () => { .as('syncAfterRecovery') cy.wait('@syncAfterRecovery', { timeout: 30000 }) cy.get('#editor-container .document-status', { timeout: 30000 }) - .should('not.contain', 'File could not be loaded') + .should('not.contain', 'Document could not be loaded.') // FIXME: There seems to be a bug where typed words maybe lost if not waiting for the new session cy.wait('@syncAfterRecovery', { timeout: 10000 }) cy.getContent().type('* more content added after the lost connection{enter}') @@ -109,12 +109,12 @@ describe('Sync', () => { cy.wait('@sessionRequests', { timeout: 30000 }) cy.get('#editor-container .document-status', { timeout: 30000 }) - .should('contain', 'File could not be loaded') + .should('contain', 'Document could not be loaded.') cy.wait('@syncAfterRecovery', { timeout: 60000 }) cy.get('#editor-container .document-status', { timeout: 30000 }) - .should('not.contain', 'File could not be loaded') + .should('not.contain', 'Document could not be loaded.') // FIXME: There seems to be a bug where typed words maybe lost if not waiting for the new session cy.wait('@syncAfterRecovery', { timeout: 10000 }) cy.getContent().type('* more content added after the lost connection{enter}') @@ -126,6 +126,25 @@ describe('Sync', () => { .should('include', 'after the lost connection') }) + it('shows warning when document session got cleaned up', () => { + cy.get('.save-status button') + .click() + 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 a.button') + .contains('Reload') + .click() + + cy.getContent() + cy.get('#editor-container .document-status .notecard') + .should('not.exist') + }) + it('passes the doc content from one session to the next', () => { cy.closeFile() cy.intercept({ method: 'PUT', url: '**/apps/text/session/*/create' }) diff --git a/cypress/support/commands.js b/cypress/support/commands.js index edcd59375c8..0a716440c00 100644 --- a/cypress/support/commands.js +++ b/cypress/support/commands.js @@ -391,6 +391,36 @@ Cypress.Commands.add('closeFile', (params = {}) => { cy.wait('@close', { timeout: 7000 }) }) +let closeData = null +Cypress.Commands.add('interceptCreate', () => { + return cy.intercept({ method: 'PUT', url: '**/session/*/create' }, (req) => { + closeData = { + url: ('' + req.url).replace('create', 'close'), + } + req.continue((res) => { + closeData = { + ...closeData, + ...res.body, + } + }) + }).as('create') +}) + +Cypress.Commands.add('closeInterceptedSession', (shareToken = undefined) => { + return cy.window().then(win => { + return axios.post( + closeData.url, + { + documentId: closeData.session.documentId, + sessionId: closeData.session.id, + sessionToken: closeData.session.token, + token: shareToken, + }, + { headers: { requesttoken: win.OC.requestToken } }, + ) + }) +}) + Cypress.Commands.add('getFile', fileName => { return cy.get(`[data-cy-files-list] tr[data-cy-files-list-row-name="${fileName}"]`) diff --git a/cypress/support/sessions.js b/cypress/support/sessions.js index 947185529e5..8f598887481 100644 --- a/cypress/support/sessions.js +++ b/cypress/support/sessions.js @@ -36,13 +36,12 @@ Cypress.Commands.add('createTextSession', (fileId, options = {}) => { return api.open({ fileId }) }) -Cypress.Commands.add('failToCreateTextSession', (fileId) => { - const api = new SessionApi() - return api.open({ fileId }) +Cypress.Commands.add('failToCreateTextSession', (fileId, baseVersionEtag = null, options = {}) => { + const api = new SessionApi(options) + return api.open({ fileId, baseVersionEtag }) .then((response) => { throw new Error('Expected request to fail - but it succeeded!') - }) - .catch((err) => err.response) + }, (err) => err.response) }) Cypress.Commands.add('pushSteps', ({ connection, steps, version, awareness = '' }) => { @@ -50,16 +49,37 @@ Cypress.Commands.add('pushSteps', ({ connection, steps, version, awareness = '' .then(response => response.data) }) +Cypress.Commands.add('failToPushSteps', ({ connection, steps, version, awareness = '' }) => { + return connection.push({ steps, version, awareness }) + .then((response) => { + throw new Error('Expected request to fail - but it succeeded!') + }, (err) => err.response) +}) + Cypress.Commands.add('syncSteps', (connection, options = { version: 0 }) => { return connection.sync(options) .then(response => response.data) }) +Cypress.Commands.add('failToSyncSteps', (connection, options = { version: 0 }) => { + return connection.sync(options) + .then((response) => { + throw new Error('Expected request to fail - but it succeeded!') + }, (err) => err.response) +}) + Cypress.Commands.add('save', (connection, options = { version: 0 }) => { return connection.save(options) .then(response => response.data) }) +Cypress.Commands.add('failToSave', (connection, options = { version: 0 }) => { + return connection.save(options) + .then((response) => { + throw new Error('Expected request to fail - but it succeeded!') + }, (err) => err.response) +}) + // Used to test for race conditions between the last push and the close request Cypress.Commands.add('pushAndClose', ({ connection, steps, version, awareness = '' }) => { cy.log('Race between push and close') diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 7fdf15251a7..563117f61b0 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -32,6 +32,7 @@ use OCA\Text\Listeners\BeforeAssistantNotificationListener; use OCA\Text\Listeners\BeforeNodeDeletedListener; use OCA\Text\Listeners\BeforeNodeRenamedListener; +use OCA\Text\Listeners\BeforeNodeWrittenListener; use OCA\Text\Listeners\FilesLoadAdditionalScriptsListener; use OCA\Text\Listeners\FilesSharingLoadAdditionalScriptsListener; use OCA\Text\Listeners\LoadEditorListener; @@ -51,6 +52,7 @@ use OCP\DirectEditing\RegisterDirectEditorEvent; use OCP\Files\Events\Node\BeforeNodeDeletedEvent; use OCP\Files\Events\Node\BeforeNodeRenamedEvent; +use OCP\Files\Events\Node\BeforeNodeWrittenEvent; use OCP\Files\Events\Node\NodeCopiedEvent; use OCP\Files\Template\ITemplateManager; use OCP\Files\Template\TemplateFileCreator; @@ -71,6 +73,7 @@ public function register(IRegistrationContext $context): void { $context->registerEventListener(LoadEditor::class, LoadEditorListener::class); // for attachments $context->registerEventListener(NodeCopiedEvent::class, NodeCopiedListener::class); + $context->registerEventListener(BeforeNodeWrittenEvent::class, BeforeNodeWrittenListener::class); $context->registerEventListener(BeforeNodeRenamedEvent::class, BeforeNodeRenamedListener::class); $context->registerEventListener(BeforeNodeDeletedEvent::class, BeforeNodeDeletedListener::class); $context->registerEventListener(AddMissingIndicesEvent::class, AddMissingIndicesListener::class); diff --git a/lib/Command/ResetDocument.php b/lib/Command/ResetDocument.php index 65a614e2f1a..0864ccb68ec 100644 --- a/lib/Command/ResetDocument.php +++ b/lib/Command/ResetDocument.php @@ -23,9 +23,7 @@ namespace OCA\Text\Command; -use OCA\Text\Db\DocumentMapper; -use OCA\Text\Db\SessionMapper; -use OCA\Text\Db\StepMapper; +use OCA\Text\Exception\DocumentHasUnsavedChangesException; use OCA\Text\Service\DocumentService; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; @@ -34,33 +32,26 @@ class ResetDocument extends Command { protected DocumentService $documentService; - protected DocumentMapper $documentMapper; - protected StepMapper $stepMapper; - protected SessionMapper $sessionMapper; - public function __construct(DocumentService $documentService, DocumentMapper $documentMapper, StepMapper $stepMapper, SessionMapper $sessionMapper) { + public function __construct(DocumentService $documentService) { parent::__construct(); - $this->documentService = $documentService; - $this->documentMapper = $documentMapper; - $this->stepMapper = $stepMapper; - $this->sessionMapper = $sessionMapper; } protected function configure(): void { $this ->setName('text:reset') - ->setDescription('Reset a text document') + ->setDescription('Reset a text document session to the current file content') ->addArgument( 'file-id', InputArgument::REQUIRED, - 'File id of the document to rest' + 'File id of the document to reset' ) ->addOption( - 'full', + 'force', 'f', null, - 'Drop all existing steps and use the currently saved version' + 'Reset the document session even with unsaved changes' ) ; } @@ -72,27 +63,23 @@ protected function configure(): void { */ protected function execute(InputInterface $input, OutputInterface $output): int { $fileId = $input->getArgument('file-id'); - $fullReset = $input->getOption('full'); + $fullReset = $input->getOption('force'); if ($fullReset) { - $output->writeln('Full document reset'); + $output->writeln('Force-reset the document session for file ' . $fileId); $this->documentService->resetDocument($fileId, true); return 0; - } else { - $output->writeln('Trying to restore to last saved version'); - $document = $this->documentMapper->find($fileId); - $deleted = $this->stepMapper->deleteAfterVersion($fileId, $document->getLastSavedVersion()); - if ($deleted > 0) { - $this->sessionMapper->deleteByDocumentId($fileId); - $output->writeln('Reverted document to the last saved version'); - - return 0; - } else { - $output->writeln('Failed revert changes that are newer than the last saved version'); - } + } + $output->writeln('Reset the document session for file ' . $fileId); + try { + $this->documentService->resetDocument($fileId); + } catch (DocumentHasUnsavedChangesException) { + $output->writeln('Not resetting due to unsaved changes'); return 1; } + + return 0; } } diff --git a/lib/Controller/PublicSessionController.php b/lib/Controller/PublicSessionController.php index 889f6634eed..8dd157dbd93 100644 --- a/lib/Controller/PublicSessionController.php +++ b/lib/Controller/PublicSessionController.php @@ -25,6 +25,7 @@ namespace OCA\Text\Controller; +use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag; use OCA\Text\Middleware\Attribute\RequireDocumentSession; use OCA\Text\Service\ApiService; use OCP\AppFramework\Http\Attribute\NoAdminRequired; @@ -80,8 +81,8 @@ protected function isPasswordProtected(): bool { #[NoAdminRequired] #[PublicPage] - public function create(string $token, string $file = null, ?string $guestName = null): DataResponse { - return $this->apiService->create(null, $file, $token, $guestName); + public function create(string $token, ?string $file = null, ?string $baseVersionEtag = null, ?string $guestName = null): DataResponse { + return $this->apiService->create(null, $file, $baseVersionEtag, $token, $guestName); } #[NoAdminRequired] @@ -92,6 +93,7 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function push(int $documentId, int $sessionId, string $sessionToken, int $version, array $steps, string $awareness, string $token): DataResponse { return $this->apiService->push($this->getSession(), $this->getDocument(), $version, $steps, $awareness, $token); @@ -99,6 +101,7 @@ public function push(int $documentId, int $sessionId, string $sessionToken, int #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function sync(string $token, int $version = 0): DataResponse { return $this->apiService->sync($this->getSession(), $this->getDocument(), $version, $token); @@ -106,6 +109,7 @@ public function sync(string $token, int $version = 0): DataResponse { #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function save(string $token, int $version = 0, string $autosaveContent = null, string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse { return $this->apiService->save($this->getSession(), $this->getDocument(), $version, $autosaveContent, $documentState, $force, $manualSave, $token); diff --git a/lib/Controller/SessionController.php b/lib/Controller/SessionController.php index 9de585d394b..71452a79bd7 100644 --- a/lib/Controller/SessionController.php +++ b/lib/Controller/SessionController.php @@ -25,6 +25,7 @@ namespace OCA\Text\Controller; +use OCA\Text\Middleware\Attribute\RequireDocumentBaseVersionEtag; use OCA\Text\Middleware\Attribute\RequireDocumentSession; use OCA\Text\Service\ApiService; use OCA\Text\Service\NotificationService; @@ -57,8 +58,8 @@ public function __construct( } #[NoAdminRequired] - public function create(int $fileId = null, string $file = null): DataResponse { - return $this->apiService->create($fileId, $file, null, null); + public function create(?int $fileId = null, ?string $file = null, ?string $baseVersionEtag = null): DataResponse { + return $this->apiService->create($fileId, $file, $baseVersionEtag, null, null); } #[NoAdminRequired] @@ -69,6 +70,7 @@ public function close(int $documentId, int $sessionId, string $sessionToken): Da #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function push(int $version, array $steps, string $awareness): DataResponse { try { @@ -81,6 +83,7 @@ public function push(int $version, array $steps, string $awareness): DataRespons #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function sync(int $version = 0): DataResponse { try { @@ -93,6 +96,7 @@ public function sync(int $version = 0): DataResponse { #[NoAdminRequired] #[PublicPage] + #[RequireDocumentBaseVersionEtag] #[RequireDocumentSession] public function save(int $version = 0, string $autosaveContent = null, string $documentState = null, bool $force = false, bool $manualSave = false): DataResponse { try { diff --git a/lib/Cron/Cleanup.php b/lib/Cron/Cleanup.php index 19933cfc17c..e374e742f10 100644 --- a/lib/Cron/Cleanup.php +++ b/lib/Cron/Cleanup.php @@ -28,7 +28,7 @@ namespace OCA\Text\Cron; -use OCA\Text\Service\AttachmentService; +use OCA\Text\Exception\DocumentHasUnsavedChangesException; use OCA\Text\Service\DocumentService; use OCA\Text\Service\SessionService; use OCP\AppFramework\Utility\ITimeFactory; @@ -39,26 +39,22 @@ class Cleanup extends TimedJob { private SessionService $sessionService; private DocumentService $documentService; private LoggerInterface $logger; - private AttachmentService $attachmentService; public function __construct(ITimeFactory $time, SessionService $sessionService, DocumentService $documentService, - AttachmentService $attachmentService, LoggerInterface $logger) { parent::__construct($time); $this->sessionService = $sessionService; $this->documentService = $documentService; - $this->attachmentService = $attachmentService; $this->logger = $logger; $this->setInterval(SessionService::SESSION_VALID_TIME); } /** * @param array $argument - * @return void */ - protected function run($argument) { + protected function run($argument): void { $this->logger->debug('Run cleanup job for text documents'); $documents = $this->documentService->getAll(); foreach ($documents as $document) { @@ -69,11 +65,10 @@ protected function run($argument) { continue; } - if ($this->documentService->hasUnsavedChanges($document)) { - continue; + try { + $this->documentService->resetDocument($document->getId()); + } catch (DocumentHasUnsavedChangesException) { } - - $this->documentService->resetDocument($document->getId()); } $this->logger->debug('Run cleanup job for text sessions'); diff --git a/lib/Exception/InvalidDocumentBaseVersionEtagException.php b/lib/Exception/InvalidDocumentBaseVersionEtagException.php new file mode 100644 index 00000000000..1bb13ca65a2 --- /dev/null +++ b/lib/Exception/InvalidDocumentBaseVersionEtagException.php @@ -0,0 +1,9 @@ + */ class BeforeNodeDeletedListener implements IEventListener { - private $attachmentService; + private AttachmentService $attachmentService; + private DocumentService $documentService; - public function __construct(AttachmentService $attachmentService) { + public function __construct(AttachmentService $attachmentService, + DocumentService $documentService) { $this->attachmentService = $attachmentService; + $this->documentService = $documentService; } public function handle(Event $event): void { @@ -48,6 +52,7 @@ public function handle(Event $event): void { $node = $event->getNode(); if ($node instanceof File && $node->getMimeType() === 'text/markdown') { $this->attachmentService->deleteAttachments($node); + $this->documentService->resetDocument($node->getId(), true); } } } diff --git a/lib/Listeners/BeforeNodeWrittenListener.php b/lib/Listeners/BeforeNodeWrittenListener.php new file mode 100644 index 00000000000..1079d7c1b9c --- /dev/null +++ b/lib/Listeners/BeforeNodeWrittenListener.php @@ -0,0 +1,61 @@ + + * + * @author Jonas + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Text\Listeners; + +use OCA\Text\Exception\DocumentHasUnsavedChangesException; +use OCA\Text\Service\DocumentService; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventListener; +use OCP\Files\Events\Node\BeforeNodeWrittenEvent; +use OCP\Files\File; + +/** + * @template-implements IEventListener + */ +class BeforeNodeWrittenListener implements IEventListener { + private DocumentService $documentService; + + public function __construct(DocumentService $documentService) { + $this->documentService = $documentService; + } + + public function handle(Event $event): void { + if (!$event instanceof BeforeNodeWrittenEvent) { + return; + } + $node = $event->getNode(); + if ($node instanceof File && $node->getMimeType() === 'text/markdown') { + if (!$this->documentService->isSaveFromText()) { + // Reset document session to avoid manual conflict resolution if there's no unsaved steps + try { + $this->documentService->resetDocument($node->getId()); + } catch (DocumentHasUnsavedChangesException) { + // Do not throw during event handling in this is expected to happen + } + } + } + } +} diff --git a/lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php b/lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php new file mode 100644 index 00000000000..d9780b3f194 --- /dev/null +++ b/lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php @@ -0,0 +1,9 @@ +getAttributes(RequireDocumentBaseVersionEtag::class))) { + $this->assertDocumentBaseVersionEtag(); + } + if (!empty($reflectionMethod->getAttributes(RequireDocumentSession::class))) { $this->assertDocumentSession($controller); } } + /** + * @throws InvalidDocumentBaseVersionEtagException + */ + private function assertDocumentBaseVersionEtag(): void { + $documentId = (int)$this->request->getParam('documentId'); + $baseVersionEtag = $this->request->getParam('baseVersionEtag'); + + $document = $this->documentService->getDocument($documentId); + if ($baseVersionEtag && $document?->getBaseVersionEtag() !== $baseVersionEtag) { + throw new InvalidDocumentBaseVersionEtagException(); + } + } + /** * @throws InvalidSessionException */ @@ -118,9 +141,13 @@ private function assertUserOrShareToken(ISessionAwareController $controller): vo $controller->setDocument($document); } - public function afterException($controller, $methodName, \Exception $exception): DataResponse|Response { + public function afterException($controller, $methodName, \Exception $exception): JSONResponse|Response { + if ($exception instanceof InvalidDocumentBaseVersionEtagException) { + return new JSONResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); + } + if ($exception instanceof InvalidSessionException) { - return new DataResponse([], 403); + return new JSONResponse([], 403); } return parent::afterException($controller, $methodName, $exception); diff --git a/lib/Service/ApiService.php b/lib/Service/ApiService.php index bcd89e4ded6..760ee908628 100644 --- a/lib/Service/ApiService.php +++ b/lib/Service/ApiService.php @@ -71,7 +71,7 @@ public function __construct(IRequest $request, $this->l10n = $l10n; } - public function create(?int $fileId = null, ?string $filePath = null, ?string $token = null, ?string $guestName = null): DataResponse { + public function create(?int $fileId = null, ?string $filePath = null, ?string $baseVersionEtag = null, ?string $token = null, ?string $guestName = null): DataResponse { try { if ($token) { $file = $this->documentService->getFileByShareToken($token, $this->request->getParam('filePath')); @@ -85,17 +85,17 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t } catch (NotFoundException $e) { return new DataResponse([], Http::STATUS_NOT_FOUND); } catch (NotPermittedException $e) { - return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 404); + return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 404); } } elseif ($fileId) { try { $file = $this->documentService->getFileById($fileId); } catch (NotFoundException|NotPermittedException $e) { $this->logger->error('No permission to access this file', [ 'exception' => $e ]); - return new DataResponse($this->l10n->t('No permission to access this file.'), Http::STATUS_NOT_FOUND); + return new DataResponse(['error' => $this->l10n->t('No permission to access this file.')], Http::STATUS_NOT_FOUND); } } else { - return new DataResponse('No valid file argument provided', Http::STATUS_PRECONDITION_FAILED); + return new DataResponse(['error' => 'No valid file argument provided'], Http::STATUS_PRECONDITION_FAILED); } $storage = $file->getStorage(); @@ -106,7 +106,7 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t $share = $storage->getShare(); $shareAttribtues = $share->getAttributes(); if ($shareAttribtues !== null && $shareAttribtues->getAttribute('permissions', 'download') === false) { - return new DataResponse($this->l10n->t('This file cannot be displayed as download is disabled by the share'), 403); + return new DataResponse(['error' => $this->l10n->t('This file cannot be displayed as download is disabled by the share')], 403); } } @@ -115,6 +115,9 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t $this->sessionService->removeInactiveSessionsWithoutSteps($file->getId()); $document = $this->documentService->getDocument($file->getId()); $freshSession = $document === null; + if ($baseVersionEtag && $baseVersionEtag !== $document?->getBaseVersionEtag()) { + return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); + } if ($freshSession) { $this->logger->info('Create new document of ' . $file->getId()); @@ -129,32 +132,30 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $t } } catch (Exception $e) { $this->logger->error($e->getMessage(), ['exception' => $e]); - return new DataResponse('Failed to create the document session', 500); + return new DataResponse(['error' => 'Failed to create the document session'], 500); } /** @var Document $document */ $session = $this->sessionService->initSession($document->getId(), $guestName); + $documentState = null; + $content = null; if ($freshSession) { $this->logger->debug('Starting a fresh editing session for ' . $file->getId()); - $documentState = null; $content = $this->loadContent($file); } else { $this->logger->debug('Loading existing session for ' . $file->getId()); - $content = null; try { $stateFile = $this->documentService->getStateFile($document->getId()); $documentState = $stateFile->getContent(); + $this->logger->debug('Existing document, state file loaded ' . $file->getId()); } catch (NotFoundException $e) { - $this->logger->debug('State file not found for ' . $file->getId()); - $documentState = ''; // no state saved yet. - // If there are no steps yet we might still need the content. - $steps = $this->documentService->getSteps($document->getId(), 0); - if (empty($steps)) { - $this->logger->debug('Empty steps, loading content for ' . $file->getId()); - $content = $this->loadContent($file); - } + $this->logger->debug('Existing document, but state file not found for ' . $file->getId()); + + // If we have no state file we need to load the content from the file + // On the client side we use this to initialize a idempotent initial y.js document + $content = $this->loadContent($file); } } @@ -196,7 +197,7 @@ public function push(Session $session, Document $document, int $version, array $ $session = $this->sessionService->updateSessionAwareness($session, $awareness); } catch (DoesNotExistException $e) { // Session was removed in the meantime. #3875 - return new DataResponse([], 403); + return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); } if (empty($steps)) { return new DataResponse([]); @@ -204,10 +205,10 @@ public function push(Session $session, Document $document, int $version, array $ try { $result = $this->documentService->addStep($document, $session, $steps, $version, $token); } catch (InvalidArgumentException $e) { - return new DataResponse($e->getMessage(), 422); + return new DataResponse(['error' => $e->getMessage()], 422); } catch (DoesNotExistException|NotPermittedException) { // Either no write access or session was removed in the meantime (#3875). - return new DataResponse([], 403); + return new DataResponse(['error' => $this->l10n->t('Editing session has expired. Please reload the page.')], Http::STATUS_PRECONDITION_FAILED); } return new DataResponse($result); } diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index 6291cb706a9..38e6c4f2cba 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -74,6 +74,8 @@ class DocumentService { */ public const AUTOSAVE_MINIMUM_DELAY = 10; + private bool $saveFromText = false; + private ?string $userId; private DocumentMapper $documentMapper; private SessionMapper $sessionMapper; @@ -118,6 +120,10 @@ public function getDocument(int $id): ?Document { } } + public function isSaveFromText(): bool { + return $this->saveFromText; + } + /** * @throws NotFoundException * @throws InvalidPathException @@ -150,7 +156,7 @@ public function createDocument(File $file): Document { $document->setLastSavedVersion(0); $document->setLastSavedVersionTime($file->getMTime()); $document->setLastSavedVersionEtag($file->getEtag()); - $document->setBaseVersionEtag($file->getEtag()); + $document->setBaseVersionEtag(uniqid()); try { /** @var Document $document */ $document = $this->documentMapper->insert($document); @@ -389,6 +395,7 @@ public function autosave(Document $document, ?File $file, int $version, ?string ILock::TYPE_APP, Application::APP_NAME ), function () use ($file, $autoSaveDocument, $documentState) { + $this->saveFromText = true; $file->putContent($autoSaveDocument); if ($documentState) { $this->writeDocumentState($file->getId(), $documentState); @@ -425,10 +432,8 @@ public function resetDocument(int $documentId, bool $force = false): void { $this->stepMapper->deleteAll($documentId); $this->sessionMapper->deleteByDocumentId($documentId); $this->documentMapper->delete($document); + $this->getStateFile($documentId)->delete(); - if ($force) { - $this->getStateFile($documentId)->delete(); - } $this->logger->debug('document reset for ' . $documentId); } catch (DoesNotExistException|NotFoundException $e) { // Ignore if document not found or state file not found diff --git a/package-lock.json b/package-lock.json index 90b2e3ecb21..94e9d10a6ee 100644 --- a/package-lock.json +++ b/package-lock.json @@ -82,6 +82,7 @@ "vue-click-outside": "^1.1.0", "vue-material-design-icons": "^5.3.0", "vuex": "^3.6.2", + "y-prosemirror": "^1.0.20", "y-protocols": "^1.0.6", "y-websocket": "^1.5.4", "yjs": "^13.6.14" @@ -30546,7 +30547,6 @@ "version": "1.0.20", "resolved": "https://registry.npmjs.org/y-prosemirror/-/y-prosemirror-1.0.20.tgz", "integrity": "sha512-LVMtu3qWo0emeYiP+0jgNcvZkqhzE/otOoro+87q0iVKxy/sMKuiJZnokfJdR4cn9qKx0Un5fIxXqbAlR2bFkA==", - "peer": true, "dependencies": { "lib0": "^0.2.42" }, @@ -52217,7 +52217,6 @@ "version": "1.0.20", "resolved": "https://registry.npmjs.org/y-prosemirror/-/y-prosemirror-1.0.20.tgz", "integrity": "sha512-LVMtu3qWo0emeYiP+0jgNcvZkqhzE/otOoro+87q0iVKxy/sMKuiJZnokfJdR4cn9qKx0Un5fIxXqbAlR2bFkA==", - "peer": true, "requires": { "lib0": "^0.2.42" } diff --git a/package.json b/package.json index 491cdfef8b1..02d4f7aaf19 100644 --- a/package.json +++ b/package.json @@ -108,6 +108,7 @@ "vue-click-outside": "^1.1.0", "vue-material-design-icons": "^5.3.0", "vuex": "^3.6.2", + "y-prosemirror": "^1.0.20", "y-protocols": "^1.0.6", "y-websocket": "^1.5.4", "yjs": "^13.6.14" diff --git a/src/EditorFactory.js b/src/EditorFactory.js index af57e08f736..319e8f1ec65 100644 --- a/src/EditorFactory.js +++ b/src/EditorFactory.js @@ -49,7 +49,7 @@ const loadSyntaxHighlight = async (language) => { } } -const createEditor = ({ language, onCreate, onUpdate = () => {}, extensions, enableRichEditing, session, relativePath }) => { +const createEditor = ({ language, onCreate = () => {}, onUpdate = () => {}, extensions, enableRichEditing, session, relativePath }) => { let defaultExtensions if (enableRichEditing) { defaultExtensions = [ diff --git a/src/components/CollisionResolveDialog.vue b/src/components/CollisionResolveDialog.vue index 7de994c7d06..304f7724d35 100644 --- a/src/components/CollisionResolveDialog.vue +++ b/src/components/CollisionResolveDialog.vue @@ -71,7 +71,7 @@ export default { const { outsideChange } = this.syncError.data this.clicked = true this.$editor.setEditable(!this.readOnly) - this.setContent(outsideChange, { isRich: this.$isRichEditor }) + this.setContent(outsideChange, { isRichEditor: this.$isRichEditor }) this.$syncService.forceSave().then(() => this.$syncService.syncUp()) }, }, diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 66d565bf4a1..af9c639a699 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -370,6 +370,7 @@ export default { guestName, shareToken: this.shareToken, filePath: this.relativePath, + baseVersionEtag: this.$syncService?.baseVersionEtag, forceRecreate: this.forceRecreate, serialize: this.isRichEditor ? (content) => createMarkdownSerializer(this.$editor.schema).serialize(content ?? this.$editor.state.doc) @@ -493,6 +494,8 @@ export default { logger.debug('onLoaded: Pushing local changes to server') this.$queue.push(updateMessage) } + } else { + this.setInitialYjsState(documentSource, { isRichEditor: this.isRichEditor }) } this.hasConnectionIssue = false @@ -537,12 +540,6 @@ export default { enableRichEditing: this.isRichEditor, }) this.hasEditor = true - if (!documentState && documentSource) { - this.setContent(documentSource, { - isRich: this.isRichEditor, - addToHistory: false, - }) - } this.listenEditorEvents() } else { // $editor already existed. So this is a reconnect. diff --git a/src/components/Editor/DocumentStatus.vue b/src/components/Editor/DocumentStatus.vue index 3782b765197..0061099826c 100644 --- a/src/components/Editor/DocumentStatus.vue +++ b/src/components/Editor/DocumentStatus.vue @@ -22,27 +22,34 @@