Skip to content

Commit

Permalink
Merge pull request #5589 from nextcloud/fix/yjs-state-initial
Browse files Browse the repository at this point in the history
Always initialize with the same yjs document if no state is present
  • Loading branch information
juliushaertl committed Apr 2, 2024
2 parents 2604650 + ee94449 commit 09465c9
Show file tree
Hide file tree
Showing 10 changed files with 243 additions and 14 deletions.
4 changes: 2 additions & 2 deletions cypress/e2e/api/SessionApi.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ describe('The session Api', function() {
return con
})
.its('state.documentSource')
.should('eql', '')
.should('eql', '## Hello world\n')
.then(() => joining.close())
.then(() => connection.close())
})
Expand All @@ -339,7 +339,7 @@ describe('The session Api', function() {
return con
})
.its('state.documentSource')
.should('eql', '')
.should('eql', '## Hello world\n')
.then(() => joining.close())
.then(() => connection.close())
})
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/*/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')
})
})
})

})
30 changes: 30 additions & 0 deletions cypress/support/commands.js
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,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
4 changes: 4 additions & 0 deletions lib/Service/ApiService.php
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,10 @@ public function create(?int $fileId = null, ?string $filePath = null, ?string $b
$this->logger->debug('Existing document, state file loaded ' . $file->getId());
} catch (NotFoundException $e) {
$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);
}
}

Expand Down
3 changes: 1 addition & 2 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": "^2.0.1",
"yjs": "^13.6.14"
Expand Down
2 changes: 1 addition & 1 deletion src/EditorFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const loadSyntaxHighlight = async (language) => {
}
}

const createEditor = ({ language, onCreate, onUpdate = () => {}, extensions, enableRichEditing, session, relativePath, isEmbedded = false }) => {
const createEditor = ({ language, onCreate = () => {}, onUpdate = () => {}, extensions, enableRichEditing, session, relativePath, isEmbedded = false }) => {
let defaultExtensions
if (enableRichEditing) {
defaultExtensions = [
Expand Down
2 changes: 1 addition & 1 deletion src/components/CollisionResolveDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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())
},
},
Expand Down
8 changes: 2 additions & 6 deletions src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,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
Expand Down Expand Up @@ -542,12 +544,6 @@ export default {
isEmbedded: this.isEmbedded,
})
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.
Expand Down
43 changes: 41 additions & 2 deletions src/mixins/setContent.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,16 @@

import escapeHtml from 'escape-html'
import markdownit from './../markdownit/index.js'
import { Doc, encodeStateAsUpdate, XmlFragment, applyUpdate } from 'yjs'
import { generateJSON } from '@tiptap/core'
import { prosemirrorToYXmlFragment } from 'y-prosemirror'
import { Node } from '@tiptap/pm/model'
import { createEditor } from '../EditorFactory.js'

export default {
methods: {
setContent(content, { isRich, addToHistory = true } = {}) {
const html = isRich
setContent(content, { isRichEditor, addToHistory = true } = {}) {
const html = isRichEditor
? markdownit.render(content) + '<p/>'
: `<pre>${escapeHtml(content)}</pre>`
this.$editor.chain()
Expand All @@ -38,5 +43,39 @@ export default {
.run()
},

setInitialYjsState(content, { isRichEditor }) {
const html = isRichEditor
? markdownit.render(content) + '<p/>'
: `<pre>${escapeHtml(content)}</pre>`

const editor = createEditor({
enableRichEditing: isRichEditor,
})
const json = generateJSON(html, editor.extensionManager.extensions)

const doc = Node.fromJSON(editor.schema, json)
const getBaseDoc = (doc) => {
const ydoc = new Doc()
// In order to make the initial document state idempotent, we need to reset the clientID
// While this is not recommended, we cannot avoid it here as we lack another mechanism
// to generate the initial state on the server side
// The only other option to avoid this could be to generate the initial state once and push
// it to the server immediately, however this would require read only sessions to be able
// to still push a state
ydoc.clientID = 0
const type = /** @type {XmlFragment} */ (ydoc.get('default', XmlFragment))
if (!type.doc) {
// This should not happen but is aligned with the upstream implementation
// https://github.com/yjs/y-prosemirror/blob/8db24263770c2baaccb08e08ea9ef92dbcf8a9da/src/lib.js#L209
return ydoc
}

prosemirrorToYXmlFragment(doc, type)
return ydoc
}

const baseUpdate = encodeStateAsUpdate(getBaseDoc(doc))
applyUpdate(this.$ydoc, baseUpdate)
},
},
}

0 comments on commit 09465c9

Please sign in to comment.