Skip to content

Commit 4c08893

Browse files
authored
Merge pull request #5541 from nextcloud/backport/5481/stable28
[stable28] Refactor document and session handling
2 parents 5b91486 + 499a54f commit 4c08893

31 files changed

+561
-146
lines changed

composer/composer/autoload_classmap.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,19 +31,22 @@
3131
'OCA\\Text\\Event\\LoadEditor' => $baseDir . '/../lib/Event/LoadEditor.php',
3232
'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => $baseDir . '/../lib/Exception/DocumentHasUnsavedChangesException.php',
3333
'OCA\\Text\\Exception\\DocumentSaveConflictException' => $baseDir . '/../lib/Exception/DocumentSaveConflictException.php',
34+
'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => $baseDir . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',
3435
'OCA\\Text\\Exception\\InvalidSessionException' => $baseDir . '/../lib/Exception/InvalidSessionException.php',
3536
'OCA\\Text\\Exception\\UploadException' => $baseDir . '/../lib/Exception/UploadException.php',
3637
'OCA\\Text\\Exception\\VersionMismatchException' => $baseDir . '/../lib/Exception/VersionMismatchException.php',
3738
'OCA\\Text\\Listeners\\AddMissingIndicesListener' => $baseDir . '/../lib/Listeners/AddMissingIndicesListener.php',
3839
'OCA\\Text\\Listeners\\BeforeAssistantNotificationListener' => $baseDir . '/../lib/Listeners/BeforeAssistantNotificationListener.php',
3940
'OCA\\Text\\Listeners\\BeforeNodeDeletedListener' => $baseDir . '/../lib/Listeners/BeforeNodeDeletedListener.php',
4041
'OCA\\Text\\Listeners\\BeforeNodeRenamedListener' => $baseDir . '/../lib/Listeners/BeforeNodeRenamedListener.php',
42+
'OCA\\Text\\Listeners\\BeforeNodeWrittenListener' => $baseDir . '/../lib/Listeners/BeforeNodeWrittenListener.php',
4143
'OCA\\Text\\Listeners\\FilesLoadAdditionalScriptsListener' => $baseDir . '/../lib/Listeners/FilesLoadAdditionalScriptsListener.php',
4244
'OCA\\Text\\Listeners\\FilesSharingLoadAdditionalScriptsListener' => $baseDir . '/../lib/Listeners/FilesSharingLoadAdditionalScriptsListener.php',
4345
'OCA\\Text\\Listeners\\LoadEditorListener' => $baseDir . '/../lib/Listeners/LoadEditorListener.php',
4446
'OCA\\Text\\Listeners\\LoadViewerListener' => $baseDir . '/../lib/Listeners/LoadViewerListener.php',
4547
'OCA\\Text\\Listeners\\NodeCopiedListener' => $baseDir . '/../lib/Listeners/NodeCopiedListener.php',
4648
'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => $baseDir . '/../lib/Listeners/RegisterDirectEditorEventListener.php',
49+
'OCA\\Text\\Middleware\\Attribute\\RequireDocumentBaseVersionEtag' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php',
4750
'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSession.php',
4851
'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => $baseDir . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php',
4952
'OCA\\Text\\Middleware\\SessionMiddleware' => $baseDir . '/../lib/Middleware/SessionMiddleware.php',

composer/composer/autoload_static.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,19 +46,22 @@ class ComposerStaticInitText
4646
'OCA\\Text\\Event\\LoadEditor' => __DIR__ . '/..' . '/../lib/Event/LoadEditor.php',
4747
'OCA\\Text\\Exception\\DocumentHasUnsavedChangesException' => __DIR__ . '/..' . '/../lib/Exception/DocumentHasUnsavedChangesException.php',
4848
'OCA\\Text\\Exception\\DocumentSaveConflictException' => __DIR__ . '/..' . '/../lib/Exception/DocumentSaveConflictException.php',
49+
'OCA\\Text\\Exception\\InvalidDocumentBaseVersionEtagException' => __DIR__ . '/..' . '/../lib/Exception/InvalidDocumentBaseVersionEtagException.php',
4950
'OCA\\Text\\Exception\\InvalidSessionException' => __DIR__ . '/..' . '/../lib/Exception/InvalidSessionException.php',
5051
'OCA\\Text\\Exception\\UploadException' => __DIR__ . '/..' . '/../lib/Exception/UploadException.php',
5152
'OCA\\Text\\Exception\\VersionMismatchException' => __DIR__ . '/..' . '/../lib/Exception/VersionMismatchException.php',
5253
'OCA\\Text\\Listeners\\AddMissingIndicesListener' => __DIR__ . '/..' . '/../lib/Listeners/AddMissingIndicesListener.php',
5354
'OCA\\Text\\Listeners\\BeforeAssistantNotificationListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeAssistantNotificationListener.php',
5455
'OCA\\Text\\Listeners\\BeforeNodeDeletedListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeNodeDeletedListener.php',
5556
'OCA\\Text\\Listeners\\BeforeNodeRenamedListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeNodeRenamedListener.php',
57+
'OCA\\Text\\Listeners\\BeforeNodeWrittenListener' => __DIR__ . '/..' . '/../lib/Listeners/BeforeNodeWrittenListener.php',
5658
'OCA\\Text\\Listeners\\FilesLoadAdditionalScriptsListener' => __DIR__ . '/..' . '/../lib/Listeners/FilesLoadAdditionalScriptsListener.php',
5759
'OCA\\Text\\Listeners\\FilesSharingLoadAdditionalScriptsListener' => __DIR__ . '/..' . '/../lib/Listeners/FilesSharingLoadAdditionalScriptsListener.php',
5860
'OCA\\Text\\Listeners\\LoadEditorListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadEditorListener.php',
5961
'OCA\\Text\\Listeners\\LoadViewerListener' => __DIR__ . '/..' . '/../lib/Listeners/LoadViewerListener.php',
6062
'OCA\\Text\\Listeners\\NodeCopiedListener' => __DIR__ . '/..' . '/../lib/Listeners/NodeCopiedListener.php',
6163
'OCA\\Text\\Listeners\\RegisterDirectEditorEventListener' => __DIR__ . '/..' . '/../lib/Listeners/RegisterDirectEditorEventListener.php',
64+
'OCA\\Text\\Middleware\\Attribute\\RequireDocumentBaseVersionEtag' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentBaseVersionEtag.php',
6265
'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSession' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSession.php',
6366
'OCA\\Text\\Middleware\\Attribute\\RequireDocumentSessionOrUserOrShareToken' => __DIR__ . '/..' . '/../lib/Middleware/Attribute/RequireDocumentSessionOrUserOrShareToken.php',
6467
'OCA\\Text\\Middleware\\SessionMiddleware' => __DIR__ . '/..' . '/../lib/Middleware/SessionMiddleware.php',

cypress/e2e/api/SessionApi.spec.js

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -315,15 +315,15 @@ describe('The session Api', function() {
315315
})
316316
})
317317

318-
it('sends initial content if other session is alive but did not push any steps', function() {
318+
it('does not send initial content if other session is alive but did not push any steps', function() {
319319
let joining
320320
cy.createTextSession(undefined, { filePath: '', shareToken })
321321
.then(con => {
322322
joining = con
323323
return con
324324
})
325325
.its('state.documentSource')
326-
.should('not.eql', '')
326+
.should('eql', '## Hello world\n')
327327
.then(() => joining.close())
328328
.then(() => connection.close())
329329
})
@@ -339,11 +339,33 @@ describe('The session Api', function() {
339339
return con
340340
})
341341
.its('state.documentSource')
342-
.should('eql', '')
342+
.should('eql', '## Hello world\n')
343343
.then(() => joining.close())
344344
.then(() => connection.close())
345345
})
346346

347+
it('refuses create,push,sync,save with non-matching baseVersionEtag', function() {
348+
cy.failToCreateTextSession(undefined, 'wrongBaseVersionEtag', { filePath: '', shareToken })
349+
.its('status')
350+
.should('eql', 412)
351+
352+
connection.setBaseVersionEtag('wrongBaseVersionEtag')
353+
354+
cy.failToPushSteps({ connection, steps: [messages.update], version })
355+
.its('status')
356+
.should('equal', 412)
357+
358+
cy.failToSyncSteps(connection, { version: 0 })
359+
.its('status')
360+
.should('equal', 412)
361+
362+
cy.failToSave(connection)
363+
.its('status')
364+
.should('equal', 412)
365+
366+
cy.then(() => connection.close())
367+
})
368+
347369
it('recovers session even if last person leaves right after create', function() {
348370
let joining
349371
cy.log('Initial user pushes steps')

cypress/e2e/conflict.spec.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ variants.forEach(function({ fixture, mime }) {
5454
cy.get('#viewer .modal-header button.header-close').click()
5555
cy.get('#viewer').should('not.exist')
5656
cy.openFile(fileName)
57-
cy.get('.text-editor .document-status .icon-error')
57+
cy.get('.text-editor .document-status')
58+
.should('contain', 'Document has been changed outside of the editor.')
5859
getWrapper()
5960
.find('#read-only-editor')
6061
.should('contain', 'Hello world')

cypress/e2e/initial.spec.js

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
/**
2+
* @copyright Copyright (c) 2020 Julius Härtl <jus@bitgrid.net>
3+
*
4+
* @author Julius Härtl <jus@bitgrid.net>
5+
*
6+
* @license AGPL-3.0-or-later
7+
*
8+
* This program is free software: you can redistribute it and/or modify
9+
* it under the terms of the GNU Affero General Public License as
10+
* published by the Free Software Foundation, either version 3 of the
11+
* License, or (at your option) any later version.
12+
*
13+
* This program is distributed in the hope that it will be useful,
14+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
15+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16+
* GNU Affero General Public License for more details.
17+
*
18+
* You should have received a copy of the GNU Affero General Public License
19+
* along with this program. If not, see <http://www.gnu.org/licenses/>.
20+
*
21+
*/
22+
23+
import { randUser } from '../utils/index.js'
24+
25+
const user = randUser()
26+
27+
describe('Test state loading of documents', function() {
28+
before(function() {
29+
// Init user
30+
cy.createUser(user)
31+
cy.login(user)
32+
cy.uploadFile('test.md', 'text/markdown')
33+
cy.uploadFile('test.md', 'text/markdown', 'test2.md')
34+
cy.uploadFile('test.md', 'text/markdown', 'test3.md')
35+
})
36+
beforeEach(function() {
37+
cy.login(user)
38+
})
39+
40+
it('Initial content can not be undone', function() {
41+
cy.shareFile('/test.md', { edit: true })
42+
.then((token) => {
43+
cy.visit(`/s/${token}`)
44+
})
45+
.then(() => {
46+
cy.getEditor().should('be.visible')
47+
cy.getContent()
48+
.should('contain', 'Hello world')
49+
.find('h2').should('contain', 'Hello world')
50+
51+
cy.getMenu().should('be.visible')
52+
cy.getActionEntry('undo').should('be.visible').click()
53+
cy.getContent()
54+
.should('contain', 'Hello world')
55+
.find('h2').should('contain', 'Hello world')
56+
})
57+
})
58+
59+
it('Consecutive sessions work properly', function() {
60+
let readToken = null
61+
let writeToken = null
62+
cy.interceptCreate()
63+
cy.shareFile('/test2.md')
64+
.then((token) => {
65+
readToken = token
66+
cy.logout()
67+
cy.visit(`/s/${readToken}`)
68+
cy.wait('@create')
69+
})
70+
.then(() => {
71+
// Open read only for the first time
72+
cy.getEditor().should('be.visible')
73+
cy.getContent()
74+
.should('contain', 'Hello world')
75+
.find('h2').should('contain', 'Hello world')
76+
cy.closeInterceptedSession(readToken)
77+
78+
// Open read only for the second time
79+
cy.reload()
80+
cy.getEditor().should('be.visible')
81+
cy.getContent()
82+
.should('contain', 'Hello world')
83+
.find('h2').should('contain', 'Hello world')
84+
cy.closeInterceptedSession(readToken)
85+
86+
cy.login(user)
87+
cy.shareFile('/test2.md', { edit: true })
88+
.then((token) => {
89+
writeToken = token
90+
// Open write link and edit something
91+
cy.visit(`/s/${writeToken}`)
92+
cy.getEditor().should('be.visible')
93+
cy.getContent()
94+
.should('contain', 'Hello world')
95+
.find('h2').should('contain', 'Hello world')
96+
cy.getContent()
97+
.type('Something new {end}')
98+
cy.intercept({ method: 'POST', url: '**/session/*/push' }).as('push')
99+
cy.intercept({ method: 'POST', url: '**/session/*/sync' }).as('sync')
100+
cy.wait('@push')
101+
cy.wait('@sync')
102+
cy.closeInterceptedSession(writeToken)
103+
104+
// Reopen read only link and check if changes are there
105+
cy.visit(`/s/${readToken}`)
106+
cy.getEditor().should('be.visible')
107+
cy.getContent()
108+
.find('h2').should('contain', 'Something new Hello world')
109+
})
110+
})
111+
})
112+
113+
it('Load after state has been saved', function() {
114+
let readToken = null
115+
let writeToken = null
116+
cy.interceptCreate()
117+
cy.shareFile('/test3.md', { edit: true })
118+
.then((token) => {
119+
writeToken = token
120+
cy.logout()
121+
cy.visit(`/s/${writeToken}`)
122+
})
123+
.then(() => {
124+
// Open a file, write and save
125+
cy.getEditor().should('be.visible')
126+
cy.getContent()
127+
.should('contain', 'Hello world')
128+
.find('h2').should('contain', 'Hello world')
129+
cy.getContent()
130+
.type('Something new {end}')
131+
cy.intercept({ method: 'POST', url: '**/session/*/save' }).as('save')
132+
cy.get('.save-status button').click()
133+
cy.wait('@save', { timeout: 10000 })
134+
cy.closeInterceptedSession(writeToken)
135+
136+
// Open writable file again and assert the content
137+
cy.reload()
138+
cy.getEditor().should('be.visible')
139+
cy.getContent()
140+
.should('contain', 'Hello world')
141+
.find('h2').should('contain', 'Something new Hello world')
142+
143+
cy.login(user)
144+
cy.shareFile('/test3.md')
145+
.then((token) => {
146+
readToken = token
147+
cy.logout()
148+
cy.visit(`/s/${readToken}`)
149+
})
150+
.then(() => {
151+
// Open read only file again and assert the content
152+
cy.getEditor().should('be.visible')
153+
cy.getContent()
154+
.should('contain', 'Hello world')
155+
.find('h2').should('contain', 'Something new Hello world')
156+
})
157+
})
158+
})
159+
160+
})

cypress/e2e/share.spec.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ describe('Open test.md in viewer', function() {
151151
cy.login(recipient)
152152
cy.visit('/apps/files')
153153
cy.openFile('test.md')
154-
cy.getModal().find('.empty-content__name').should('contain', 'Failed to load file')
154+
cy.getModal().find('.document-status').should('contain', 'This file cannot be displayed as download is disabled by the share')
155155
cy.getModal().getContent().should('not.exist')
156156
})
157157
})

cypress/e2e/sync.spec.js

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ describe('Sync', () => {
7474
}).as('sessionRequests')
7575
cy.wait('@dead', { timeout: 30000 })
7676
cy.get('#editor-container .document-status', { timeout: 30000 })
77-
.should('contain', 'File could not be loaded')
77+
.should('contain', 'Document could not be loaded.')
7878
.then(() => {
7979
reconnect = true
8080
})
@@ -83,7 +83,7 @@ describe('Sync', () => {
8383
.as('syncAfterRecovery')
8484
cy.wait('@syncAfterRecovery', { timeout: 30000 })
8585
cy.get('#editor-container .document-status', { timeout: 30000 })
86-
.should('not.contain', 'File could not be loaded')
86+
.should('not.contain', 'Document could not be loaded.')
8787
// FIXME: There seems to be a bug where typed words maybe lost if not waiting for the new session
8888
cy.wait('@syncAfterRecovery', { timeout: 10000 })
8989
cy.getContent().type('* more content added after the lost connection{enter}')
@@ -109,12 +109,12 @@ describe('Sync', () => {
109109

110110
cy.wait('@sessionRequests', { timeout: 30000 })
111111
cy.get('#editor-container .document-status', { timeout: 30000 })
112-
.should('contain', 'File could not be loaded')
112+
.should('contain', 'Document could not be loaded.')
113113

114114
cy.wait('@syncAfterRecovery', { timeout: 60000 })
115115

116116
cy.get('#editor-container .document-status', { timeout: 30000 })
117-
.should('not.contain', 'File could not be loaded')
117+
.should('not.contain', 'Document could not be loaded.')
118118
// FIXME: There seems to be a bug where typed words maybe lost if not waiting for the new session
119119
cy.wait('@syncAfterRecovery', { timeout: 10000 })
120120
cy.getContent().type('* more content added after the lost connection{enter}')
@@ -126,6 +126,25 @@ describe('Sync', () => {
126126
.should('include', 'after the lost connection')
127127
})
128128

129+
it('shows warning when document session got cleaned up', () => {
130+
cy.get('.save-status button')
131+
.click()
132+
cy.wait('@save')
133+
cy.uploadTestFile('test.md')
134+
135+
cy.get('#editor-container .document-status', { timeout: 30000 })
136+
.should('contain', 'Editing session has expired.')
137+
138+
// Reload button works
139+
cy.get('#editor-container .document-status a.button')
140+
.contains('Reload')
141+
.click()
142+
143+
cy.getContent()
144+
cy.get('#editor-container .document-status .notecard')
145+
.should('not.exist')
146+
})
147+
129148
it('passes the doc content from one session to the next', () => {
130149
cy.closeFile()
131150
cy.intercept({ method: 'PUT', url: '**/apps/text/session/*/create' })

cypress/support/commands.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,36 @@ Cypress.Commands.add('closeFile', (params = {}) => {
391391
cy.wait('@close', { timeout: 7000 })
392392
})
393393

394+
let closeData = null
395+
Cypress.Commands.add('interceptCreate', () => {
396+
return cy.intercept({ method: 'PUT', url: '**/session/*/create' }, (req) => {
397+
closeData = {
398+
url: ('' + req.url).replace('create', 'close'),
399+
}
400+
req.continue((res) => {
401+
closeData = {
402+
...closeData,
403+
...res.body,
404+
}
405+
})
406+
}).as('create')
407+
})
408+
409+
Cypress.Commands.add('closeInterceptedSession', (shareToken = undefined) => {
410+
return cy.window().then(win => {
411+
return axios.post(
412+
closeData.url,
413+
{
414+
documentId: closeData.session.documentId,
415+
sessionId: closeData.session.id,
416+
sessionToken: closeData.session.token,
417+
token: shareToken,
418+
},
419+
{ headers: { requesttoken: win.OC.requestToken } },
420+
)
421+
})
422+
})
423+
394424
Cypress.Commands.add('getFile', fileName => {
395425
return cy.get(`[data-cy-files-list] tr[data-cy-files-list-row-name="${fileName}"]`)
396426

0 commit comments

Comments
 (0)