Skip to content

Commit

Permalink
fix(sync): Do not resend document state in first push.
Browse files Browse the repository at this point in the history
Apply document state as a step.
Process it like other steps received from the remote.
In particular include it in the tracking of steps already applied
and set the version accordingly.

Signed-off-by: Max <max@nextcloud.com>
  • Loading branch information
max-nextcloud committed Nov 25, 2024
1 parent 5fa9517 commit ec059cc
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 55 deletions.
32 changes: 7 additions & 25 deletions cypress/component/helpers/yjs.cy.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
*/

import * as Y from 'yjs'
import { getDocumentState, getUpdateMessage, applyUpdateMessage } from '../../../src/helpers/yjs.js'
import { getDocumentState, documentStateToStep, applyStep } from '../../../src/helpers/yjs.js'

describe('Yjs base64 wrapped with our helpers', function() {
it('applies step in wrong order', function() {
it('applies step generated from document state', function() {
const source = new Y.Doc()
const target = new Y.Doc()
const sourceMap = source.getMap()
Expand All @@ -17,44 +17,26 @@ describe('Yjs base64 wrapped with our helpers', function() {
// 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)
const step0A = documentStateToStep(stateA)
applyStep(target, step0A)
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)
const step0B = documentStateToStep(stateB)

// 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)
applyStep(target, step0B)
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(29)
expect(updateAA).to.be.eq(undefined)
expect(targetMap.get('keyC')).to.be.eq(undefined)
})

})
4 changes: 1 addition & 3 deletions src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -503,9 +503,7 @@ export default {
},

onLoaded({ document, documentSource, documentState }) {
if (documentState) {
applyDocumentState(this.$ydoc, documentState, this.$providers[0])
} else {
if (!documentState) {
this.setInitialYjsState(documentSource, { isRichEditor: this.isRichEditor })
}

Expand Down
50 changes: 24 additions & 26 deletions src/helpers/yjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,36 +34,44 @@ export function applyDocumentState(ydoc, documentState, origin) {
}

/**
* Update message for everything in ydoc that is not in encodedBaseUpdate
* Create a step from a document state
* i.e. create a sync protocol update message from it
* and encode it and wrap it in a step data structure.
*
* @param {Y.Doc} ydoc - encode state of this doc
* @param {string} encodedBaseUpdate - base64 encoded doc update to build upon
* @return {Uint8Array|undefined}
* @param {string} documentState - base64 encoded doc state
* @return {string} base64 encoded yjs sync protocol update message
*/
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
}
export function documentStateToStep(documentState) {
const message = documentStateToUpdateMessage(documentState)
return { step: encodeArrayBuffer(message) }
}

/**
* Create an update message from a document state
* i.e. decode the base64 encoded yjs update
* and create a sync protocol update message from it
*
* @param {string} documentState - base64 encoded doc state
* @return {Uint8Array}
*/
function documentStateToUpdateMessage(documentState) {
const update = decodeArrayBuffer(documentState)
const encoder = encoding.createEncoder()
encoding.writeVarUint(encoder, messageSync)
const update = Y.encodeStateAsUpdate(ydoc, baseStateVector)
syncProtocol.writeUpdate(encoder, update)
return encoding.toUint8Array(encoder)
}

/**
* Apply an updated message to the ydoc.
* Apply a step to the ydoc.
*
* Only used in tests right now.
* @param {Y.Doc} ydoc - encode state of this doc
* @param {Uint8Array} updateMessage - y-websocket sync message with update
* @param {string} step - base64 encoded yjs sync update message
* @param {object} origin - initiator object e.g. WebsocketProvider
*/
export function applyUpdateMessage(ydoc, updateMessage, origin = 'origin') {
export function applyStep(ydoc, step, origin = 'origin') {
const updateMessage = decodeArrayBuffer(step.step)
const decoder = decoding.createDecoder(updateMessage)
const messageType = decoding.readVarUint(decoder)
if (messageType !== messageSync) {
Expand Down Expand Up @@ -128,13 +136,3 @@ 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)
}
11 changes: 10 additions & 1 deletion src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import debounce from 'debounce'

import PollingBackend from './PollingBackend.js'
import SessionApi, { Connection } from './SessionApi.js'
import { getSteps, getAwareness } from '../helpers/yjs.js'
import { getSteps, getAwareness, documentStateToStep } from '../helpers/yjs.js'
import { logger } from '../helpers/logger.js'

/**
Expand Down Expand Up @@ -122,6 +122,15 @@ class SyncService {
this.baseVersionEtag = this.#connection.document.baseVersionEtag
this.emit('opened', this.connectionState)
this.emit('loaded', this.connectionState)
const documentState = this.connectionState.documentState
if (documentState) {
const initialStep = documentStateToStep(documentState)
this.emit('sync', {
version: this.version,
steps: [initialStep],
document: this.#connection.document,
})
}

return this.connectionState
}
Expand Down

0 comments on commit ec059cc

Please sign in to comment.