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

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

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 => {

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

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

@@ -199,6 +199,13 @@ 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,
+				connectionState.document.lastSavedVersion,
+			)
+		}
 		return connectionState
 	}
 
@@ -218,6 +225,13 @@ class SyncService {
 		}
 	}
 
+	_emitDocumentStateStep(documentState: string, version: number) {
+		const documentStateStep = documentStateToStep(documentState, version)
+		this.bus.emit('sync', {
+			steps: [documentStateStep],
+		})
+	}
+
 	updateSession(guestName: string) {
 		if (!this.connection?.isPublic) {
 			return Promise.reject(new Error())
@@ -266,12 +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) {
-					const documentStateStep = documentStateToStep(documentState)
-					this.bus.emit('sync', {
-						steps: [documentStateStep],
-					})
+					this._emitDocumentStateStep(documentState, version)
 				}
 				this.pushError = 0
 				this.sending = false

src/services/WebSocketPolyfill.ts

@@ -25,13 +25,22 @@ export default function initWebSocketPolyfill(syncService: SyncService, fileId:
 		onopen?: () => void
 		#notifyPushBus
 		#onSync
+		#onOpened
 		#processingVersion = 0
 
 		constructor(url: string) {
 			this.#notifyPushBus = getNotifyBus()
 			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 })
 		}
 
 		/**