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

[stable29] Always initialize with the same yjs document if no state is present #5593

Merged
merged 6 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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)
},
},
}
Loading