From cbbf4ec8ae55653d54b936dafa4c4592963565ce Mon Sep 17 00:00:00 2001 From: Max Date: Wed, 15 Oct 2025 18:57:39 +0200 Subject: [PATCH 1/3] fix(sync): send first update without initial document state Process initial document state as incoming message. This way it will be tracked as received from the remote server by y-websocket and the update send to the server will exclude it. Also use the sync services `opened` event inside WebsocketPolyfill rather than mixing async `syncService.open().then` and event handling. This allows us to emit `opened` and then `sync` from `syncService.open()`. Signed-off-by: Max --- src/components/Editor.vue | 5 +---- src/services/SyncService.ts | 16 ++++++++++++---- src/services/WebSocketPolyfill.ts | 15 +++++++++------ 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 7bc52de0ad1..982257f1a88 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -106,7 +106,6 @@ import { CollaborationCursor } from '../extensions/index.js' import { exposeForDebugging, removeFromDebugging } from '../helpers/debug.js' import { logger } from '../helpers/logger.js' import { setInitialYjsState } from '../helpers/setInitialYjsState.js' -import { applyDocumentState } from '../helpers/yjs.ts' import { ERROR_TYPE, IDLE_TIMEOUT } from '../services/SyncService.ts' import { fetchNode } from '../services/WebdavClient.ts' import { @@ -516,9 +515,7 @@ export default defineComponent({ // Fetch the document state after syntax highlights are loaded this.lowlightLoaded.then(() => { this.syncService.startSync() - if (documentState) { - applyDocumentState(this.ydoc, documentState, this.syncProvider) - } else { + if (!documentState) { setInitialYjsState(this.ydoc, content, { isRichEditor: this.isRichEditor, }) diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 6cb4974aba9..330fc2a6c44 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -177,6 +177,10 @@ class SyncService { this.backend = new PollingBackend(this, this.connection.value, data) // Make sure to only emit this once the backend is in place. this.bus.emit('opened', data) + // Emit sync after opened, so websocket onmessage comes after onopen. + if (data.documentState) { + this._emitDocumentStateStep(data.documentState) + } } startSync() { @@ -198,6 +202,13 @@ class SyncService { } } + _emitDocumentStateStep(documentState: string) { + const documentStateStep = documentStateToStep(documentState) + this.bus.emit('sync', { + steps: [documentStateStep], + }) + } + sendStep(step: Uint8Array) { this.#outbox.storeStep(step) this.sendSteps() @@ -243,10 +254,7 @@ class SyncService { documentState: string } if (documentState) { - const documentStateStep = documentStateToStep(documentState) - this.bus.emit('sync', { - steps: [documentStateStep], - }) + this._emitDocumentStateStep(documentState) } this.pushError = 0 this.#sending = false diff --git a/src/services/WebSocketPolyfill.ts b/src/services/WebSocketPolyfill.ts index 645421db1ee..72b6f8ef813 100644 --- a/src/services/WebSocketPolyfill.ts +++ b/src/services/WebSocketPolyfill.ts @@ -26,6 +26,7 @@ export default function initWebSocketPolyfill( onopen?: () => void #notifyPushBus #onSync + #onOpened #processingVersion = 0 constructor(url: string) { @@ -34,6 +35,13 @@ export default function initWebSocketPolyfill( this.#url = url logger.debug('WebSocketPolyfill#constructor', { url, fileId }) + this.#onOpened = () => { + if (syncService.hasActiveConnection()) { + this.onopen?.() + } + } + syncService.bus.on('opened', this.#onOpened) + this.#onSync = ({ steps }: { steps: Step[] }) => { if (steps) { this.#processSteps(steps) @@ -43,14 +51,9 @@ export default function initWebSocketPolyfill( }) } } - syncService.bus.on('sync', this.#onSync) - syncService.open().then(() => { - if (syncService.hasActiveConnection()) { - this.onopen?.() - } - }) + syncService.open() } /** From 6f8fd00669a1924ac404ba8fa86232e31ae4459a Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 16 Oct 2025 00:09:26 +0200 Subject: [PATCH 2/3] fix(sync): bump version after loading document state * Emit the documents last saved version in the push response if document state is send as well. * Otherwise send the version as 0 so it cannot increase the sync service version. * Use the versions returned by create and push requests alongside the document state to construct a consistent step with the last saved version that matches the document state. This will result in the sync service version getting bumped to the documents last saved version once the coresponding document state has been processed. Signed-off-by: Max --- lib/Service/DocumentService.php | 2 +- src/apis/sync.ts | 2 +- src/helpers/yjs.ts | 5 +++-- src/services/SyncService.ts | 15 +++++++++------ src/tests/helpers/yjs.spec.ts | 4 ++-- 5 files changed, 16 insertions(+), 12 deletions(-) diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index ca5aa551de5..a1b852bb68f 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -265,7 +265,7 @@ public function addStep(Document $document, Session $session, array $steps, int return [ 'steps' => $stepsToReturn, - 'version' => $newVersion, + 'version' => isset($documentState) ? $document->getLastSavedVersion() : 0, 'documentState' => $documentState ]; } diff --git a/src/apis/sync.ts b/src/apis/sync.ts index 00b393eb427..407f4cc6049 100644 --- a/src/apis/sync.ts +++ b/src/apis/sync.ts @@ -20,7 +20,7 @@ interface PushResponse { data: { steps: Step[] documentState: string - awareness: Record + version: number } } diff --git a/src/helpers/yjs.ts b/src/helpers/yjs.ts index d5e8854e793..7e87ca249fb 100644 --- a/src/helpers/yjs.ts +++ b/src/helpers/yjs.ts @@ -43,11 +43,12 @@ export function applyDocumentState( * and encode it and wrap it in a step data structure. * * @param documentState - base64 encoded doc state + * @param version - last saved version for the document state * @return base64 encoded yjs sync protocol update message and version */ -export function documentStateToStep(documentState: string): Step { +export function documentStateToStep(documentState: string, version: number): Step { const message = documentStateToUpdateMessage(documentState) - return { data: [encodeArrayBuffer(message)], sessionId: 0, version: -1 } + return { data: [encodeArrayBuffer(message)], sessionId: 0, version } } /** diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 330fc2a6c44..8d5572a50af 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -173,13 +173,15 @@ class SyncService { console.error('Opened the connection but now it is undefined') return } - this.version = data.document.lastSavedVersion this.backend = new PollingBackend(this, this.connection.value, data) // Make sure to only emit this once the backend is in place. this.bus.emit('opened', data) // Emit sync after opened, so websocket onmessage comes after onopen. if (data.documentState) { - this._emitDocumentStateStep(data.documentState) + this._emitDocumentStateStep( + data.documentState, + data.document.lastSavedVersion, + ) } } @@ -202,8 +204,8 @@ class SyncService { } } - _emitDocumentStateStep(documentState: string) { - const documentStateStep = documentStateToStep(documentState) + _emitDocumentStateStep(documentState: string, version: number) { + const documentStateStep = documentStateToStep(documentState, version) this.bus.emit('sync', { steps: [documentStateStep], }) @@ -249,12 +251,13 @@ class SyncService { }) .then((response) => { this.#outbox.clearSentData(sendable) - const { steps, documentState } = response.data as { + const { steps, documentState, version } = response.data as { steps: Step[] documentState: string + version: number } if (documentState) { - this._emitDocumentStateStep(documentState) + this._emitDocumentStateStep(documentState, version) } this.pushError = 0 this.#sending = false diff --git a/src/tests/helpers/yjs.spec.ts b/src/tests/helpers/yjs.spec.ts index 473d1bc1a82..251e8ed4727 100644 --- a/src/tests/helpers/yjs.spec.ts +++ b/src/tests/helpers/yjs.spec.ts @@ -26,14 +26,14 @@ describe('Yjs base64 wrapped with our helpers', function () { sourceMap.set('keyA', 'valueA') const stateA = getDocumentState(source) - const step0A = documentStateToStep(stateA) + const step0A = documentStateToStep(stateA, 123) 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 step0B = documentStateToStep(stateB) + const step0B = documentStateToStep(stateB, 124) // Add keyC to source, apply to target sourceMap.set('keyC', 'valueC') From 205c7cb903ae81ab81826b45a5dd38d4276c01c3 Mon Sep 17 00:00:00 2001 From: Max Date: Thu, 16 Oct 2025 08:23:38 +0200 Subject: [PATCH 3/3] chore(cleanup): remove unused $newVersion. Signed-off-by: Max --- lib/Service/DocumentService.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/Service/DocumentService.php b/lib/Service/DocumentService.php index a1b852bb68f..6204e0c08ed 100644 --- a/lib/Service/DocumentService.php +++ b/lib/Service/DocumentService.php @@ -211,7 +211,6 @@ public function addStep(Document $document, Session $session, array $steps, int $stepsToInsert = []; $stepsIncludeQuery = false; $documentState = null; - $newVersion = $version; foreach ($steps as $step) { $message = YjsMessage::fromBase64($step); if ($readOnly && $message->isUpdate()) { @@ -228,7 +227,7 @@ public function addStep(Document $document, Session $session, array $steps, int if ($readOnly) { throw new NotPermittedException('Read-only client tries to push steps with changes'); } - $newVersion = $this->insertSteps($document, $session, $stepsToInsert); + $this->insertSteps($document, $session, $stepsToInsert); } // By default, send all steps the user has not received yet. @@ -275,14 +274,12 @@ public function addStep(Document $document, Session $session, array $steps, int * @param Session $session * @param Step[] $steps * - * @return int - * * @throws DoesNotExistException * @throws InvalidArgumentException * * @psalm-param non-empty-list $steps */ - private function insertSteps(Document $document, Session $session, array $steps): int { + private function insertSteps(Document $document, Session $session, array $steps): void { $stepsVersion = null; try { $stepsJson = json_encode($steps, JSON_THROW_ON_ERROR); @@ -298,7 +295,6 @@ private function insertSteps(Document $document, Session $session, array $steps) $this->logger->debug('Adding steps to ' . $document->getId() . ": bumping version from $stepsVersion to $newVersion"); $this->cache->set('document-version-' . $document->getId(), $newVersion); // TODO write steps to cache for quicker reading - return $newVersion; } catch (\Throwable $e) { if ($stepsVersion !== null) { $this->logger->error('This should never happen. An error occurred when storing the version, trying to recover the last stable one', ['exception' => $e]);