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..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. @@ -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 $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]); 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/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 6061b31af26..660cb20152c 100644 --- a/src/services/SyncService.ts +++ b/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 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 }) } /**