[stable32] fix(sync): send first update without initial document state

cypress/e2e/api/SessionApi.spec.js

@@ -82,7 +82,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)
@@ -134,7 +134,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',
@@ -147,7 +147,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',
@@ -208,7 +208,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',
@@ -222,7 +222,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',
@@ -281,7 +281,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.openConnection({ filePath: '', token: shareToken })
 				.then(({ connection: con, data }) => {
 					joining = con
@@ -321,7 +321,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.openConnection({ filePath: '', token: shareToken }).then(
 				({ connection: con }) => {

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.
@@ -265,7 +264,7 @@ public function addStep(Document $document, Session $session, array $steps, int
 
 		return [
 			'steps' => $stepsToReturn,
-			'version' => $newVersion,
+			'version' => isset($documentState) ? $document->getLastSavedVersion() : 0,
 			'documentState' => $documentState
 		];
 	}
@@ -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<mixed> $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]);

src/apis/sync.ts

@@ -20,7 +20,7 @@ interface PushResponse {
 	data: {
 		steps: Step[]
 		documentState: string
-		awareness: Record<string, string>
+		version: number
 	}
 }
 

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,
 					})

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 }
 }
 
 /**

src/services/SyncService.ts

@@ -173,10 +173,16 @@ 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,
+				data.document.lastSavedVersion,
+			)
+		}
 	}
 
 	startSync() {
@@ -198,6 +204,13 @@ class SyncService {
 		}
 	}
 
+	_emitDocumentStateStep(documentState: string, version: number) {
+		const documentStateStep = documentStateToStep(documentState, version)
+		this.bus.emit('sync', {
+			steps: [documentStateStep],
+		})
+	}
+
 	sendStep(step: Uint8Array<ArrayBufferLike>) {
 		this.#outbox.storeStep(step)
 		this.sendSteps()
@@ -238,15 +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) {
-					const documentStateStep = documentStateToStep(documentState)
-					this.bus.emit('sync', {
-						steps: [documentStateStep],
-					})
+					this._emitDocumentStateStep(documentState, version)
 				}
 				this.pushError = 0
 				this.#sending = false

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()
 		}
 
 		/**

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')