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

[stable28] Refactor document and session handling #5541

Merged
merged 18 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
85795d6
fix(backend): Reset document session and yjs file when file is deleted
mejo- Mar 13, 2024
8e741de
fix(backend): Reset document session when updated from outside editor
mejo- Mar 13, 2024
8e1ad16
fix(backend): Remove yjs file and all steps when resetting document s…
mejo- Mar 13, 2024
cc2e6c3
fix: catch expected exception in event handler
juliusknorr Mar 15, 2024
aa236cf
fix: Clean up logic to return document state file or file content
juliusknorr Mar 13, 2024
c2798f2
fix: Set base version etag to a unique id per document creation
juliusknorr Mar 13, 2024
ae78252
fix(sync): If `baseVersionEtag` changed, reset frontend
mejo- Mar 15, 2024
f72fb5b
fix(Middleware): Response with 412 if `baseVersionEtag` doesn't match
mejo- Mar 18, 2024
af37fe4
fix(DocumentStatus): Refactor and migrate to `NcNoteCard`
mejo- Mar 18, 2024
25ad80e
test(cypress): Add session API tests with non-matching baseVersionEtag
mejo- Mar 20, 2024
628e463
text(cypress): Test browser refresh warning after document session cl…
mejo- Mar 20, 2024
907d2d4
fix(response): Make sure JSONResponse returns valid data
mejo- Mar 20, 2024
261051c
fix: Create idempotent y.js doc for initial content
juliusknorr Mar 30, 2024
134106f
tests: Add tests for loading documents from different preconditions
juliusknorr Mar 30, 2024
64a6252
fix: Always return initial content when needed
juliusknorr Mar 30, 2024
d464338
tests: Adjust tests covering initial state
juliusknorr Mar 30, 2024
ddabaf0
ci: Make cypress test more stable by closing connections
juliusknorr Mar 31, 2024
499a54f
fix: Adapt to review feedback
juliusknorr Apr 2, 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
28 changes: 25 additions & 3 deletions cypress/e2e/api/SessionApi.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -315,15 +315,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 @@ -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')
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()
.find('#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 <[email protected]>
*
* @author Julius Härtl <[email protected]>
*
* @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/*/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')
})
})
})

})
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__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')
})
})
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 @@ -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
})
Expand All @@ -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}')
Expand All @@ -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}')
Expand All @@ -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' })
Expand Down
30 changes: 30 additions & 0 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}"]`)

Expand Down
Loading
Loading