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

fix(sync): push local changes on reconnect #5279

Merged
merged 6 commits into from
Jan 19, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Next Next commit
fix(sync): push local changes on reconnect
When an editing session is interrupted
steps that were already pushed to the server
may be cleared alongside the sync service session.

Make sure to push all local state
that is not part of the document state
when (re-)connecting.

Tests

Yjs relies on a browser environment.
Therefore we test it in a cypress test.
Move these tests into component tests
to separate them from 'normal' e2e tests.

Signed-off-by: Max <max@nextcloud.com>
  • Loading branch information
max-nextcloud committed Jan 18, 2024
commit 57662dff439d33ca902597f43b8ff28477784bde
15 changes: 6 additions & 9 deletions cypress.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,6 @@ const { defineConfig } = require('cypress')
const cypressSplit = require('cypress-split')
const getCompareSnapshotsPlugin = require('cypress-visual-regression/dist/plugin.js')

module.exports = defineConfig({
e2e: {
setupNodeEvents(on, config) {
// IMPORTANT: return the config object
return config
},
},
})

module.exports = defineConfig({
projectId: 'hx9gqy',
viewportWidth: 1280,
Expand Down Expand Up @@ -43,6 +34,12 @@ module.exports = defineConfig({
baseUrl: 'http://localhost:8081/index.php/',
specPattern: 'cypress/e2e/**/*.{js,jsx,ts,tsx}',
},
component: {
max-nextcloud marked this conversation as resolved.
Show resolved Hide resolved
devServer: {
framework: "vue",
bundler: "webpack",
},
},
retries: {
runMode: 2,
// do not retry in `cypress open`
Expand Down
77 changes: 77 additions & 0 deletions cypress/component/helpers/yjs.cy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
/*
* @copyright Copyright (c) 2023 Jonas <jonas@nextcloud.com>
*
* @author Jonas <jonas@nextcloud.com>
*
* @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 * as Y from 'yjs'
import { getDocumentState, getUpdateMessage, applyUpdateMessage } from '../../../src/helpers/yjs.js'

describe('Yjs base64 wrapped with our helpers', function() {
it('applies step in wrong order', function() {
const source = new Y.Doc()
const target = new Y.Doc()
const sourceMap = source.getMap()
const targetMap = target.getMap()

target.on('afterTransaction', (tr, doc) => {
// console.log('afterTransaction', tr)
})

const state0 = getDocumentState(source)

// Add keyA to source and apply to target
sourceMap.set('keyA', 'valueA')

const stateA = getDocumentState(source)
const update0A = getUpdateMessage(source, state0)
applyUpdateMessage(target, update0A)
expect(targetMap.get('keyA')).to.be.eq('valueA')

// Add keyB to source, don't apply to target yet
sourceMap.set('keyB', 'valueB')
const stateB = getDocumentState(source)
const updateAB = getUpdateMessage(source, stateA)

// Add keyC to source, apply to target
sourceMap.set('keyC', 'valueC')
const updateBC = getUpdateMessage(source, stateB)
applyUpdateMessage(target, updateBC)
expect(targetMap.get('keyB')).to.be.eq(undefined)
expect(targetMap.get('keyC')).to.be.eq(undefined)

// Apply keyB to target
applyUpdateMessage(target, updateAB)
expect(targetMap.get('keyB')).to.be.eq('valueB')
expect(targetMap.get('keyC')).to.be.eq('valueC')
})

it('update message is empty if no additional state exists', function() {
const source = new Y.Doc()
const sourceMap = source.getMap()
const state0 = getDocumentState(source)
sourceMap.set('keyA', 'valueA')
const stateA = getDocumentState(source)
const update0A = getUpdateMessage(source, state0)
const updateAA = getUpdateMessage(source, stateA)
expect(update0A.length).to.be.eq(40)
expect(updateAA).to.be.eq(undefined)
})

})
File renamed without changes.
12 changes: 12 additions & 0 deletions cypress/support/component-index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
<meta name="viewport" content="width=device-width,initial-scale=1.0">
<title>Components App</title>
</head>
<body>
<div data-cy-root></div>
</body>
</html>
27 changes: 27 additions & 0 deletions cypress/support/component.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// ***********************************************************
// This example support/component.js is processed and
// loaded automatically before your test files.
//
// This is a great place to put global configuration and
// behavior that modifies Cypress.
//
// You can change the location of this file or turn off
// automatically serving support files with the
// 'supportFile' configuration option.
//
// You can read more here:
// https://on.cypress.io/configuration
// ***********************************************************

// Import commands.js using ES2015 syntax:
import './commands.js'

// Alternatively you can use CommonJS syntax:
// require('./commands')

import { mount } from 'cypress/vue2'

Cypress.Commands.add('mount', mount)

// Example use:
// cy.mount(MyComponent)
26 changes: 17 additions & 9 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.2.0",
"vuex": "^3.6.2",
"y-protocols": "^1.0.6",
"y-websocket": "^1.5.1",
"yjs": "^13.6.10"
},
Expand Down
7 changes: 6 additions & 1 deletion src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ import {
import ReadonlyBar from './Menu/ReadonlyBar.vue'

import { logger } from '../helpers/logger.js'
import { getDocumentState, applyDocumentState } from '../helpers/yjs.js'
import { getDocumentState, applyDocumentState, getUpdateMessage } from '../helpers/yjs.js'
import { SyncService, ERROR_TYPE, IDLE_TIMEOUT } from './../services/SyncService.js'
import createSyncServiceProvider from './../services/SyncServiceProvider.js'
import AttachmentResolver from './../services/AttachmentResolver.js'
Expand Down Expand Up @@ -487,6 +487,11 @@ export default {
onLoaded({ documentSource, documentState }) {
if (documentState) {
applyDocumentState(this.$ydoc, documentState, this.$providers[0])
// distribute additional state that may exist locally
Copy link
Member

@juliusknorr juliusknorr Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So we always distribute the local (possibly outdated) state? Can we ensure/proof somehow that this does not lead to unexpected reverting of newer changes from the server?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This state is yjs document state. So basically what we are distributing is our local knowledge of the history of the document. yjs will handle sorting those edits and turning them into a current state. In the end it will produce a consistent view for everyone - as long as all are on the same page. We need to share all local history so our future edits will be applied.

Let's say we worked together on a text. I got disconnected but made some changes before the editor noticed. Now I'm reconnecting.

If I distribute my local changes yjs will put all changes in an order and create a current content that may contain my or your changes depending on the order picked by yjs. There's no way of knowing what would be the correct order and we would have to live with that. At least we have a consistent view on the content.

If I keep connect without distributing my local changes I will receive your changes - the doc would probably look the same as above for me. Your view would stay the same as it was when i was disconnected and changes i make later on will not show for you.

If I drop all local changes and basically reload the editor we'd also be on the same page and my changes would be lost.

const updateMessage = getUpdateMessage(this.$ydoc, documentState)
if (updateMessage) {
this.$queue.push(updateMessage)
max-nextcloud marked this conversation as resolved.
Show resolved Hide resolved
}
}

this.hasConnectionIssue = false
Expand Down
73 changes: 68 additions & 5 deletions src/helpers/yjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,27 +21,80 @@
*/

import { encodeArrayBuffer, decodeArrayBuffer } from '../helpers/base64.js'
import { Doc, encodeStateAsUpdate, applyUpdate } from 'yjs'
import * as Y from 'yjs'
import * as decoding from 'lib0/decoding.js'
import * as encoding from 'lib0/encoding.js'
import * as syncProtocol from 'y-protocols/sync'
import { messageSync } from 'y-websocket'

/**
* Get Document state encode as base64.
*
* @param {Doc} ydoc - encode state of this doc
* Used to store yjs state on the server.
* @param {Y.Doc} ydoc - encode state of this doc
*/
export function getDocumentState(ydoc) {
const update = encodeStateAsUpdate(ydoc)
const update = Y.encodeStateAsUpdate(ydoc)
return encodeArrayBuffer(update)
}

/**
*
* @param {Doc} ydoc - apply state to this doc
* @param {Y.Doc} ydoc - apply state to this doc
* @param {string} documentState - base64 encoded doc state
* @param {object} origin - initiator object e.g. WebsocketProvider
*/
export function applyDocumentState(ydoc, documentState, origin) {
const update = decodeArrayBuffer(documentState)
applyUpdate(ydoc, update, origin)
Y.applyUpdate(ydoc, update, origin)
}

/**
* Update message for everything in ydoc that is not in encodedBaseUpdate
*
* @param {Y.Doc} ydoc - encode state of this doc
* @param {string} encodedBaseUpdate - base64 encoded doc update to build upon
max-nextcloud marked this conversation as resolved.
Show resolved Hide resolved
*/
export function getUpdateMessage(ydoc, encodedBaseUpdate) {
const baseUpdate = decodeArrayBuffer(encodedBaseUpdate)
const baseStateVector = Y.encodeStateVectorFromUpdate(baseUpdate)
const docStateVector = Y.encodeStateVector(ydoc)
if (sameState(baseStateVector, docStateVector)) {
// no additional state in the ydoc - early return
return
}
const encoder = encoding.createEncoder()
encoding.writeVarUint(encoder, messageSync)
const update = Y.encodeStateAsUpdate(ydoc, baseStateVector)
syncProtocol.writeUpdate(encoder, update)
const buf = encoding.toUint8Array(encoder)
return encodeArrayBuffer(buf)
}

/**
* Apply an updated message to the ydoc.
*
* Only used in tests right now.
* @param {Y.Doc} ydoc - encode state of this doc
* @param {string} updateMessage - base64 encoded y-websocket sync message with update
* @param {object} origin - initiator object e.g. WebsocketProvider
*/
export function applyUpdateMessage(ydoc, updateMessage, origin = 'origin') {
const updateBuffer = decodeArrayBuffer(updateMessage)
const decoder = decoding.createDecoder(updateBuffer)
const messageType = decoding.readVarUint(decoder)
if (messageType !== messageSync) {
console.error('y.js update message with invalid type', messageType)
return
}
// There are no responses to updates - so this is a dummy.
const encoder = encoding.createEncoder()
syncProtocol.readSyncMessage(
decoder,
encoder,
ydoc,
origin,
)
}

/**
Expand Down Expand Up @@ -71,3 +124,13 @@ export function logStep(step) {
break
}
}

/**
* Helper function to check if two state vectors have the same state
* @param {Array} arr - state vector to compare
* @param {Array} other - state vector to compare against
*/
function sameState(arr, other) {
return arr.length === other.length
&& arr.every((value, index) => other[index] === value)
}