refactor(sync): cache steps in sync service

Store the steps that need to be send where we also do the debouncing. They will be updated whenever there is a new message from y-websocket. Signed-off-by: Max <max@nextcloud.com>
nextcloud · Nov 27, 2024 · 36048c1 · 36048c1
1 parent f3ca9a9
commit 36048c1
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 188 deletions.
diff --git a/src/components/Editor.vue b/src/components/Editor.vue
@@ -354,7 +354,6 @@ export default {
 	},
 	created() {
 		this.$ydoc = new Doc()
-		this.$queue = []
 		// The following can be useful for debugging ydoc updates
 		// this.$ydoc.on('update', function(update, origin, doc, tr) {
 		//   console.debug('ydoc update', update, origin, doc, tr)
@@ -404,7 +403,6 @@ export default {
 				ydoc: this.$ydoc,
 				syncService: this.$syncService,
 				fileId: this.fileId,
-				queue: this.$queue,
 				initialSession: this.initialSession,
 				disableBC: true,
 			})
@@ -696,8 +694,10 @@ export default {
 		},
 
 		async close() {
-			await this.$syncService.sendRemainingSteps(this.$queue)
+			await this.$syncService.sendRemainingSteps()
+				.catch(err => logger.warn('Failed to send remaining steps', { err }))
 			await this.disconnect()
+				.catch(err => logger.warn('Failed to disconnect', { err }))
 			if (this.$editor) {
 				try {
 					this.unlistenEditorEvents()

diff --git a/src/helpers/yjs.js b/src/helpers/yjs.js
@@ -105,27 +105,6 @@ export function applyStep(ydoc, step, origin = 'origin') {
 	)
 }
 
-/**
- * Get the steps for sending to the server
- *
- * @param {object[]} queue - queue for the outgoing steps
- */
-export function getSteps(queue) {
-	return queue.map(s => encodeArrayBuffer(s))
-		.filter(s => s < 'AQ')
-		.slice(-1)
-}
-
-/**
- * Encode the latest awareness message for sending
- *
- * @param {object[]} queue - queue for the outgoing steps
- */
-export function getAwareness(queue) {
-	return queue.map(s => encodeArrayBuffer(s))
-		.findLast(s => s > 'AQ') || ''
-}
-
 /**
  * Log y.js messages with their type and initiator call stack
  *

diff --git a/src/services/Outbox.js b/src/services/Outbox.js
@@ -0,0 +1,61 @@
+/**
+ * SPDX-FileCopyrightText: 2024 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+import { encodeArrayBuffer } from '../helpers/base64.js'
+import { logger } from '../helpers/logger.js'
+
+export default class Outbox {
+
+	#awarenessUpdate = ''
+	#syncUpdate = ''
+	#syncQuery = ''
+
+	storeStep(step) {
+		const encoded = encodeArrayBuffer(step)
+		if (encoded < 'AAA' || encoded > 'Ag') {
+			logger.warn('Unexpected step type:', { step, encoded })
+			return
+		}
+		if (encoded < 'AAE') {
+			this.#syncQuery = encoded
+			return
+		}
+		if (encoded < 'AQ') {
+			this.#syncUpdate = encoded
+			return
+		}
+		this.#awarenessUpdate = encoded
+	}
+
+	getDataToSend() {
+		return {
+			steps: [this.#syncUpdate, this.#syncQuery].filter(s => s),
+			awareness: this.#awarenessUpdate,
+		}
+	}
+
+	get hasUpdate() {
+		return !!this.#syncUpdate
+	}
+
+	/*
+	 * Clear data that has just been sent.
+	 *
+	 * Only clear data that has not changed in the meantime.
+	 * @param {Sendable} - data that was to the server
+	 */
+	clearSentData({ steps, awareness }) {
+		if (steps.includes(this.#syncUpdate)) {
+			this.#syncUpdate = ''
+		}
+		if (steps.includes(this.#syncQuery)) {
+			this.#syncQuery = ''
+		}
+		if (this.#awarenessUpdate === awareness) {
+			this.#awarenessUpdate = ''
+		}
+	}
+
+}
diff --git a/src/services/SyncService.js b/src/services/SyncService.js
@@ -24,8 +24,9 @@ import mitt from 'mitt'
 import debounce from 'debounce'
 
 import PollingBackend from './PollingBackend.js'
+import Outbox from './Outbox.js'
 import SessionApi, { Connection } from './SessionApi.js'
-import { getSteps, getAwareness, documentStateToStep } from '../helpers/yjs.js'
+import { documentStateToStep } from '../helpers/yjs.js'
 import { logger } from '../helpers/logger.js'
 
 /**
@@ -71,6 +72,7 @@ class SyncService {
 
 	#sendIntervalId
 	#connection
+	#outbox = new Outbox()
 
 	constructor({ baseVersionEtag, serialize, getDocumentState, ...options }) {
 		/** @type {import('mitt').Emitter<import('./SyncService').EventTypes>} _bus */
@@ -167,30 +169,37 @@ class SyncService {
 			})
 	}
 
-	sendSteps(getSendable) {
+	sendStep(step) {
+		this.#outbox.storeStep(step)
+		this.sendSteps()
+	}
+
+	sendSteps() {
 		// If already waiting to send, do nothing.
 		if (this.#sendIntervalId) {
 			return
 		}
-		return new Promise((resolve, reject) => {
-			this.#sendIntervalId = setInterval(() => {
-				if (this.#connection && !this.sending) {
-					this.sendStepsNow(getSendable).then(resolve).catch(reject)
-				}
-			}, 200)
-		})
+		this.#sendIntervalId = setInterval(() => {
+			if (this.#connection && !this.sending) {
+				this.sendStepsNow()
+			}
+		}, 200)
 	}
 
-	sendStepsNow(getSendable) {
+	async sendStepsNow() {
 		this.sending = true
 		clearInterval(this.#sendIntervalId)
 		this.#sendIntervalId = null
-		const sendable = getSendable()
+		const sendable = this.#outbox.getDataToSend()
 		if (sendable.steps.length > 0) {
 			this.emit('stateChange', { dirty: true })
 		}
-		return this.#connection.push(sendable)
+		if (!this.hasActiveConnection) {
+			return
+		}
+		return this.#connection.push({ ...sendable, version: this.version })
 			.then((response) => {
+				this.#outbox.clearSentData(sendable)
 				const { steps, documentState } = response.data
 				if (documentState) {
 					const documentStateStep = documentStateToStep(documentState)
@@ -209,6 +218,7 @@ class SyncService {
 				const { response, code } = err
 				this.sending = false
 				this.pushError++
+				logger.error('Failed to push the steps to the server', err)
 				if (!response || code === 'ECONNABORTED') {
 					this.emit('error', { type: ERROR_TYPE.CONNECTION_FAILED, data: {} })
 				}
@@ -313,25 +323,12 @@ class SyncService {
 		})
 	}
 
-	async sendRemainingSteps(queue) {
-		if (queue.length === 0) {
+	async sendRemainingSteps() {
+		if (!this.#outbox.hasUpdate) {
 			return
 		}
-		let outbox = []
-		const steps = getSteps(queue)
-		const awareness = getAwareness(queue)
-		return this.sendStepsNow(() => {
-			const data = { steps, awareness, version: this.version }
-			outbox = [...queue]
-			logger.debug('sending final steps ', data)
-			return data
-		})?.then(() => {
-			// only keep the steps that were not send yet
-			queue.splice(0,
-				queue.length,
-				...queue.filter(s => !outbox.includes(s)),
-			)
-		}, err => logger.error(err))
+		logger.debug('sending final steps')
+		return this.sendStepsNow().catch(err => logger.error(err))
 	}
 
 	async close() {

diff --git a/src/services/WebSocketPolyfill.js b/src/services/WebSocketPolyfill.js
@@ -22,21 +22,17 @@
 
 import { logger } from '../helpers/logger.js'
 import { decodeArrayBuffer } from '../helpers/base64.js'
-import { getSteps, getAwareness } from '../helpers/yjs.js'
 
 /**
  *
  * @param {object} syncService - the sync service to build upon
  * @param {number} fileId - id of the file to open
  * @param {object} initialSession - initial session to open
- * @param {object[]} queue - queue for the outgoing steps
  */
-export default function initWebSocketPolyfill(syncService, fileId, initialSession, queue) {
+export default function initWebSocketPolyfill(syncService, fileId, initialSession) {
 	return class WebSocketPolyfill {
 
 		#url
-		#session
-		#version
 		binaryType
 		onmessage
 		onerror
@@ -48,34 +44,19 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
 			this.url = url
 			logger.debug('WebSocketPolyfill#constructor', { url, fileId, initialSession })
 			this.#registerHandlers({
-				opened: ({ version, session }) => {
-					logger.debug('opened ', { version, session })
-					this.#version = version
-					this.#session = session
-				},
-				loaded: ({ version, session, content }) => {
-					logger.debug('loaded ', { version, session })
-					this.#version = version
-					this.#session = session
-				},
 				sync: ({ steps, version }) => {
-					logger.debug('synced ', { version, steps })
-					this.#version = version
 					if (steps) {
 						steps.forEach(s => {
 							const data = decodeArrayBuffer(s.step)
 							this.onmessage({ data })
 						})
+						logger.debug('synced ', { version, steps })
 					}
 				},
 			})
 
 			syncService.open({ fileId, initialSession }).then((data) => {
 				if (syncService.hasActiveConnection) {
-					const { version, session } = data
-					this.#version = version
-					this.#session = session
-
 					this.onopen?.()
 				}
 			})
@@ -87,32 +68,10 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
 				.forEach(([key, value]) => syncService.on(key, value))
 		}
 
-		send(...data) {
+		send(step) {
 			// Useful for debugging what steps are sent and how they were initiated
-			// data.forEach(logStep)
-
-			queue.push(...data)
-			let outbox = []
-			return syncService.sendSteps(() => {
-				const data = {
-					steps: getSteps(queue),
-					awareness: getAwareness(queue),
-					version: this.#version,
-				}
-				outbox = [...queue]
-				logger.debug('sending steps ', data)
-				return data
-			})?.then(ret => {
-				// only keep the steps that were not send yet
-				queue.splice(0,
-					queue.length,
-					...queue.filter(s => !outbox.includes(s)),
-				)
-				return ret
-			}, err => {
-				logger.error(`Failed to push the queue with ${queue.length} steps to the server`, err)
-				this.onerror?.(err)
-			})
+			// logStep(step)
+			syncService.sendStep(step)
 		}
 
 		async close() {