From e6f22573d20d524655b51c7745d3728e0d61401b 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 | 6 ++---- src/services/SyncService.ts | 16 ++++++++++++---- src/services/WebSocketPolyfill.ts | 16 ++++++++++------ 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/components/Editor.vue b/src/components/Editor.vue index 543d03c7b5c..ec74abfc249 100644 --- a/src/components/Editor.vue +++ b/src/components/Editor.vue @@ -96,7 +96,7 @@ import { import ReadonlyBar from './Menu/ReadonlyBar.vue' import { logger } from '../helpers/logger.js' -import { applyDocumentState, getDocumentState } from '../helpers/yjs.ts' +import { getDocumentState } from '../helpers/yjs.ts' import { SyncService, ERROR_TYPE, IDLE_TIMEOUT } from './../services/SyncService.ts' import SessionApi from '../services/SessionApi.js' import createSyncServiceProvider from './../services/SyncServiceProvider.js' @@ -556,9 +556,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 }) } diff --git a/src/services/SyncService.ts b/src/services/SyncService.ts index 6061b31af26..a0d8a4e1121 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -199,6 +199,10 @@ class SyncService { } this.bus.emit('opened', connectionState) this.bus.emit('loaded', connectionState) + // Emit sync after opened, so websocket onmessage comes after onopen. + if (connectionState.documentState) { + this._emitDocumentStateStep(connectionState.documentState) + } return connectionState } @@ -218,6 +222,13 @@ class SyncService { } } + _emitDocumentStateStep(documentState: string) { + const documentStateStep = documentStateToStep(documentState) + this.bus.emit('sync', { + steps: [documentStateStep], + }) + } + updateSession(guestName: string) { if (!this.connection?.isPublic) { return Promise.reject(new Error()) @@ -268,10 +279,7 @@ class SyncService { this.#outbox.clearSentData(sendable) const { steps, documentState } = response.data 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 66694c2ac72..882a38c06b4 100644 --- a/src/services/WebSocketPolyfill.ts +++ b/src/services/WebSocketPolyfill.ts @@ -25,6 +25,7 @@ export default function initWebSocketPolyfill(syncService: SyncService, fileId: onopen?: () => void #notifyPushBus #onSync + #onOpened #processingVersion = 0 constructor(url: string) { @@ -32,6 +33,14 @@ export default function initWebSocketPolyfill(syncService: SyncService, fileId: this.#notifyPushBus?.on('notify_push', this.#onNotifyPush.bind(this)) this.#url = url logger.debug('WebSocketPolyfill#constructor', { url, fileId, initialSession }) + + this.#onOpened = () => { + if (syncService.hasActiveConnection()) { + this.onopen?.() + } + } + syncService.bus.on('opened', this.#onOpened) + this.#onSync = ({ steps }: { steps: Step[] }) => { if (steps) { this.#processSteps(steps) @@ -41,14 +50,9 @@ export default function initWebSocketPolyfill(syncService: SyncService, fileId: }) } } - syncService.bus.on('sync', this.#onSync) - syncService.open({ fileId, initialSession }).then((_data) => { - if (syncService.hasActiveConnection()) { - this.onopen?.() - } - }) + syncService.open({ fileId, initialSession }) } /** From 648f03f47736ef5483f5efb456146dd6b8798ea3 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 --- cypress/component/helpers/yjs.cy.js | 4 ++-- cypress/e2e/api/SessionApi.spec.js | 14 +++++++------- lib/Service/DocumentService.php | 2 +- src/helpers/yjs.ts | 5 +++-- src/services/SyncService.ts | 17 ++++++++++++----- 5 files changed, 25 insertions(+), 17 deletions(-) diff --git a/cypress/component/helpers/yjs.cy.js b/cypress/component/helpers/yjs.cy.js index 2d5d3dedf9c..7861ae661bd 100644 --- a/cypress/component/helpers/yjs.cy.js +++ b/cypress/component/helpers/yjs.cy.js @@ -21,14 +21,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') diff --git a/cypress/e2e/api/SessionApi.spec.js b/cypress/e2e/api/SessionApi.spec.js index 942370f2710..64739fd1427 100644 --- a/cypress/e2e/api/SessionApi.spec.js +++ b/cypress/e2e/api/SessionApi.spec.js @@ -93,7 +93,7 @@ describe('The session Api', function() { const version = 0 cy.pushSteps({ connection, steps, version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.syncSteps(connection) .its('steps[0].data') .should('eql', steps) @@ -151,7 +151,7 @@ describe('The session Api', function() { it('saves', function() { cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.save(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true }) cy.downloadFile(filePath) .its('data') @@ -162,7 +162,7 @@ describe('The session Api', function() { const documentState = 'Base64 encoded string' cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.save(connection, { version: 1, autosaveContent: '# Heading 1', @@ -224,7 +224,7 @@ describe('The session Api', function() { it('saves public', function() { cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.save(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true }) cy.login(user) cy.downloadFile('saves.md') @@ -236,7 +236,7 @@ describe('The session Api', function() { const documentState = 'Base64 encoded string' cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.save(connection, { version: 1, autosaveContent: '# Heading 1', @@ -309,7 +309,7 @@ describe('The session Api', function() { let joining cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.createTextSession(undefined, { filePath: '', shareToken }) .then(con => { joining = con @@ -348,7 +348,7 @@ describe('The session Api', function() { cy.log('Initial user pushes steps') cy.pushSteps({ connection, steps: [messages.update], version }) .its('version') - .should('be.at.least', 1) + .should('eql', 0) cy.log('Other user creates session') cy.createTextSession(undefined, { filePath: '', shareToken }) .then(con => { 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/helpers/yjs.ts b/src/helpers/yjs.ts index e9e1567ee48..e9605b9f5ee 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 a0d8a4e1121..660cb20152c 100644 --- a/src/services/SyncService.ts +++ b/src/services/SyncService.ts @@ -201,7 +201,10 @@ class SyncService { this.bus.emit('loaded', connectionState) // Emit sync after opened, so websocket onmessage comes after onopen. if (connectionState.documentState) { - this._emitDocumentStateStep(connectionState.documentState) + this._emitDocumentStateStep( + connectionState.documentState, + connectionState.document.lastSavedVersion, + ) } return connectionState } @@ -222,8 +225,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], }) @@ -277,9 +280,13 @@ class SyncService { return this.connection?.push({ ...sendable, version: this.version }) .then((response) => { this.#outbox.clearSentData(sendable) - const { steps, documentState } = response.data + 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 From d0da538858f095ef994b19aab2c856ffcb040381 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 Signed-off-by: Jonas --- 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]);