Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable27] Refactor document and session handling #5543

Merged
merged 19 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
98d01d5
fix(backend): Reset document session and yjs file when file is deleted
mejo- Mar 13, 2024
f00de1e
fix(backend): Reset document session when updated from outside editor
mejo- Mar 13, 2024
10938f4
fix(backend): Remove yjs file and all steps when resetting document s…
mejo- Mar 13, 2024
9a98ec2
fix: catch expected exception in event handler
juliusknorr Mar 15, 2024
da4bf9c
fix: Clean up logic to return document state file or file content
juliusknorr Mar 13, 2024
d7c173d
fix: Set base version etag to a unique id per document creation
juliusknorr Mar 13, 2024
c26cb39
fix(sync): If `baseVersionEtag` changed, reset frontend
mejo- Mar 15, 2024
3978f12
fix(Middleware): Response with 412 if `baseVersionEtag` doesn't match
mejo- Mar 18, 2024
a9c3b69
fix(DocumentStatus): Refactor and migrate to `NcNoteCard`
mejo- Mar 18, 2024
5c3fe47
test(cypress): Add session API tests with non-matching baseVersionEtag
mejo- Mar 20, 2024
cfa9ec8
text(cypress): Test browser refresh warning after document session cl…
mejo- Mar 20, 2024
4a08af8
fix(response): Make sure JSONResponse returns valid data
mejo- Mar 20, 2024
cdc310f
fix: Create idempotent y.js doc for initial content
juliusknorr Mar 30, 2024
1a46a1b
tests: Add tests for loading documents from different preconditions
juliusknorr Mar 30, 2024
5f8484f
fix: Always return initial content when needed
juliusknorr Mar 30, 2024
16c9afd
tests: Adjust tests covering initial state
juliusknorr Mar 30, 2024
fbfba86
ci: Make cypress test more stable by closing connections
juliusknorr Mar 31, 2024
4dabf75
fix: Adapt to review feedback
juliusknorr Apr 2, 2024
88f49a5
chore(assets): Recompile assets
nextcloud-command Apr 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions composer/composer/autoload_classmap.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,19 +31,22 @@
'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',
'OCA\\Text\\Listeners\\AddMissingIndicesListener' => $baseDir . '/../lib/Listeners/AddMissingIndicesListener.php',
'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',
Expand Down
3 changes: 3 additions & 0 deletions composer/composer/autoload_static.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,19 +46,22 @@ 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',
'OCA\\Text\\Listeners\\AddMissingIndicesListener' => __DIR__ . '/..' . '/../lib/Listeners/AddMissingIndicesListener.php',
'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',
Expand Down
24 changes: 21 additions & 3 deletions cypress/e2e/api/SessionApi.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -311,15 +311,15 @@ 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 => {
joining = con
return con
})
.its('state.documentSource')
.should('not.eql', '')
.should('eql', '## Hello world\n')
.then(() => joining.close())
.then(() => connection.close())
})
Expand All @@ -335,11 +335,29 @@ 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.then(() => connection.close())
})

it('recovers session even if last person leaves right after create', function() {
let joining
cy.log('Initial user pushes steps')
Expand Down
3 changes: 2 additions & 1 deletion cypress/e2e/conflict.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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()
.get('#read-only-editor')
.should('contain', 'Hello world')
Expand Down
160 changes: 160 additions & 0 deletions cypress/e2e/initial.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,160 @@
/**
* @copyright Copyright (c) 2020 Julius Härtl <jus@bitgrid.net>
*
* @author Julius Härtl <jus@bitgrid.net>
*
* @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 <http://www.gnu.org/licenses/>.
*
*/

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/sync' }).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')
})
})
})

})
2 changes: 1 addition & 1 deletion cypress/e2e/share.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__title').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')
})
})
Expand Down
27 changes: 23 additions & 4 deletions cypress/e2e/sync.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,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
})
Expand All @@ -86,7 +86,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}')
Expand All @@ -112,12 +112,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}')
Expand All @@ -129,6 +129,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' })
Expand Down
30 changes: 30 additions & 0 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,36 @@ Cypress.Commands.add('closeFile', (fileName, 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(`.files-fileList tr[data-file="${fileName}"]`)
})
Expand Down
Loading
Loading