Merge pull request #6650 from nextcloud/fix/y-websocket-with-full-updates

Send one full update from y-websocket

Author: mejo-

cypress/component/helpers/yjs.cy.js

@@ -4,10 +4,10 @@
  */
 
 import * as Y from 'yjs'
-import { getDocumentState, getUpdateMessage, applyUpdateMessage } from '../../../src/helpers/yjs.js'
+import { getDocumentState, documentStateToStep, applyStep } from '../../../src/helpers/yjs.js'
 
 describe('Yjs base64 wrapped with our helpers', function() {
-	it('applies step in wrong order', function() {
+	it('applies step generated from document state', function() {
 		const source = new Y.Doc()
 		const target = new Y.Doc()
 		const sourceMap = source.getMap()
@@ -17,44 +17,26 @@ describe('Yjs base64 wrapped with our helpers', function() {
 			// console.log('afterTransaction', tr)
 		})
 
-		const state0 = getDocumentState(source)
-
 		// Add keyA to source and apply to target
 		sourceMap.set('keyA', 'valueA')
 
 		const stateA = getDocumentState(source)
-		const update0A = getUpdateMessage(source, state0)
-		applyUpdateMessage(target, update0A)
+		const step0A = documentStateToStep(stateA)
+		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 updateAB = getUpdateMessage(source, stateA)
+		const step0B = documentStateToStep(stateB)
 
 		// Add keyC to source, apply to target
 		sourceMap.set('keyC', 'valueC')
-		const updateBC = getUpdateMessage(source, stateB)
-		applyUpdateMessage(target, updateBC)
-		expect(targetMap.get('keyB')).to.be.eq(undefined)
-		expect(targetMap.get('keyC')).to.be.eq(undefined)
 
 		// Apply keyB to target
-		applyUpdateMessage(target, updateAB)
+		applyStep(target, step0B)
 		expect(targetMap.get('keyB')).to.be.eq('valueB')
-		expect(targetMap.get('keyC')).to.be.eq('valueC')
-	})
-
-	it('update message is empty if no additional state exists', function() {
-		const source = new Y.Doc()
-		const sourceMap = source.getMap()
-		const state0 = getDocumentState(source)
-		sourceMap.set('keyA', 'valueA')
-		const stateA = getDocumentState(source)
-		const update0A = getUpdateMessage(source, state0)
-		const updateAA = getUpdateMessage(source, stateA)
-		expect(update0A.length).to.be.eq(29)
-		expect(updateAA).to.be.eq(undefined)
+		expect(targetMap.get('keyC')).to.be.eq(undefined)
 	})
 
 })

cypress/e2e/api/SyncServiceProvider.spec.js

@@ -4,6 +4,7 @@
  */
 
 import { randUser } from '../../utils/index.js'
+import SessionApi from '../../../src/services/SessionApi.js'
 import { SyncService } from '../../../src/services/SyncService.js'
 import createSyncServiceProvider from '../../../src/services/SyncServiceProvider.js'
 import { Doc } from 'yjs'
@@ -39,9 +40,11 @@ describe('Sync service provider', function() {
 	 */
 	function createProvider(ydoc) {
 		const queue = []
+		const api = new SessionApi()
 		const syncService = new SyncService({
 			serialize: () => 'Serialized',
 			getDocumentState: () => null,
+			api,
 		})
 		syncService.on('opened', () => syncService.startSync())
 		return createSyncServiceProvider({

lib/Service/DocumentService.php

@@ -208,7 +208,8 @@ public function addStep(Document $document, Session $session, array $steps, int
 		$documentId = $session->getDocumentId();
 		$readOnly = $this->isReadOnly($this->getFileForSession($session, $shareToken), $shareToken);
 		$stepsToInsert = [];
-		$querySteps = [];
+		$stepsIncludeQuery = false;
+		$documentState = null;
 		$newVersion = $version;
 		foreach ($steps as $step) {
 			$message = YjsMessage::fromBase64($step);
@@ -217,7 +218,7 @@ public function addStep(Document $document, Session $session, array $steps, int
 			}
 			// Filter out query steps as they would just trigger clients to send their steps again
 			if ($message->getYjsMessageType() === YjsMessage::YJS_MESSAGE_SYNC && $message->getYjsSyncType() === YjsMessage::YJS_MESSAGE_SYNC_STEP1) {
-				$querySteps[] = $step;
+				$stepsIncludeQuery = true;
 			} else {
 				$stepsToInsert[] = $step;
 			}
@@ -228,8 +229,24 @@ public function addStep(Document $document, Session $session, array $steps, int
 			}
 			$newVersion = $this->insertSteps($document, $session, $stepsToInsert);
 		}
-		// If there were any queries in the steps send the entire history
-		$getStepsSinceVersion = count($querySteps) > 0 ? 0 : $version;
+
+		// By default send all steps the user has not received yet.
+		$getStepsSinceVersion = $version;
+		if ($stepsIncludeQuery) {
+			$this->logger->debug('Loading document state for ' . $documentId);
+			try {
+				$stateFile = $this->getStateFile($documentId);
+				$documentState = $stateFile->getContent();
+				$this->logger->debug('Existing document, state file loaded ' . $documentId);
+				// If there were any queries in the steps send all steps since last save.
+				$getStepsSinceVersion = $document->getLastSavedVersion();
+			} catch (NotFoundException $e) {
+				$this->logger->debug('Existing document, but no state file found for ' . $documentId);
+				// If there is no state file include all the steps.
+				$getStepsSinceVersion = 0;
+			}
+		}
+
 		$allSteps = $this->getSteps($documentId, $getStepsSinceVersion);
 		$stepsToReturn = [];
 		foreach ($allSteps as $step) {
@@ -238,9 +255,11 @@ public function addStep(Document $document, Session $session, array $steps, int
 				$stepsToReturn[] = $step;
 			}
 		}
+
 		return [
 			'steps' => $stepsToReturn,
-			'version' => $newVersion
+			'version' => $newVersion,
+			'documentState' => $documentState
 		];
 	}