Skip to content

Commit 4a8617d

Browse files
mejo-mejo-
authored
Merge pull request #7790 from nextcloud/backport/7787/stable31
[stable31] fix(sync): send first update without initial document state
2 parents a6f16d4 + d0da538 commit 4a8617d

7 files changed

Lines changed: 47 additions & 33 deletions

File tree

‎cypress/component/helpers/yjs.cy.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,14 +21,14 @@ describe('Yjs base64 wrapped with our helpers', function() {
2121
sourceMap.set('keyA', 'valueA')
2222

2323
const stateA = getDocumentState(source)
24-
const step0A = documentStateToStep(stateA)
24+
const step0A = documentStateToStep(stateA, 123)
2525
applyStep(target, step0A)
2626
expect(targetMap.get('keyA')).to.be.eq('valueA')
2727

2828
// Add keyB to source, don't apply to target yet
2929
sourceMap.set('keyB', 'valueB')
3030
const stateB = getDocumentState(source)
31-
const step0B = documentStateToStep(stateB)
31+
const step0B = documentStateToStep(stateB, 124)
3232

3333
// Add keyC to source, apply to target
3434
sourceMap.set('keyC', 'valueC')

‎cypress/e2e/api/SessionApi.spec.js‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ describe('The session Api', function() {
9393
const version = 0
9494
cy.pushSteps({ connection, steps, version })
9595
.its('version')
96-
.should('be.at.least', 1)
96+
.should('eql', 0)
9797
cy.syncSteps(connection)
9898
.its('steps[0].data')
9999
.should('eql', steps)
@@ -151,7 +151,7 @@ describe('The session Api', function() {
151151
it('saves', function() {
152152
cy.pushSteps({ connection, steps: [messages.update], version })
153153
.its('version')
154-
.should('be.at.least', 1)
154+
.should('eql', 0)
155155
cy.save(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true })
156156
cy.downloadFile(filePath)
157157
.its('data')
@@ -162,7 +162,7 @@ describe('The session Api', function() {
162162
const documentState = 'Base64 encoded string'
163163
cy.pushSteps({ connection, steps: [messages.update], version })
164164
.its('version')
165-
.should('be.at.least', 1)
165+
.should('eql', 0)
166166
cy.save(connection, {
167167
version: 1,
168168
autosaveContent: '# Heading 1',
@@ -224,7 +224,7 @@ describe('The session Api', function() {
224224
it('saves public', function() {
225225
cy.pushSteps({ connection, steps: [messages.update], version })
226226
.its('version')
227-
.should('be.at.least', 1)
227+
.should('eql', 0)
228228
cy.save(connection, { version: 1, autosaveContent: '# Heading 1', manualSave: true })
229229
cy.login(user)
230230
cy.downloadFile('saves.md')
@@ -236,7 +236,7 @@ describe('The session Api', function() {
236236
const documentState = 'Base64 encoded string'
237237
cy.pushSteps({ connection, steps: [messages.update], version })
238238
.its('version')
239-
.should('be.at.least', 1)
239+
.should('eql', 0)
240240
cy.save(connection, {
241241
version: 1,
242242
autosaveContent: '# Heading 1',
@@ -309,7 +309,7 @@ describe('The session Api', function() {
309309
let joining
310310
cy.pushSteps({ connection, steps: [messages.update], version })
311311
.its('version')
312-
.should('be.at.least', 1)
312+
.should('eql', 0)
313313
cy.createTextSession(undefined, { filePath: '', shareToken })
314314
.then(con => {
315315
joining = con
@@ -348,7 +348,7 @@ describe('The session Api', function() {
348348
cy.log('Initial user pushes steps')
349349
cy.pushSteps({ connection, steps: [messages.update], version })
350350
.its('version')
351-
.should('be.at.least', 1)
351+
.should('eql', 0)
352352
cy.log('Other user creates session')
353353
cy.createTextSession(undefined, { filePath: '', shareToken })
354354
.then(con => {

‎lib/Service/DocumentService.php‎

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,6 @@ public function addStep(Document $document, Session $session, array $steps, int
211211
$stepsToInsert = [];
212212
$stepsIncludeQuery = false;
213213
$documentState = null;
214-
$newVersion = $version;
215214
foreach ($steps as $step) {
216215
$message = YjsMessage::fromBase64($step);
217216
if ($readOnly && $message->isUpdate()) {
@@ -228,7 +227,7 @@ public function addStep(Document $document, Session $session, array $steps, int
228227
if ($readOnly) {
229228
throw new NotPermittedException('Read-only client tries to push steps with changes');
230229
}
231-
$newVersion = $this->insertSteps($document, $session, $stepsToInsert);
230+
$this->insertSteps($document, $session, $stepsToInsert);
232231
}
233232

234233
// 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
265264

266265
return [
267266
'steps' => $stepsToReturn,
268-
'version' => $newVersion,
267+
'version' => isset($documentState) ? $document->getLastSavedVersion() : 0,
269268
'documentState' => $documentState
270269
];
271270
}
@@ -275,14 +274,12 @@ public function addStep(Document $document, Session $session, array $steps, int
275274
* @param Session $session
276275
* @param Step[] $steps
277276
*
278-
* @return int
279-
*
280277
* @throws DoesNotExistException
281278
* @throws InvalidArgumentException
282279
*
283280
* @psalm-param non-empty-list<mixed> $steps
284281
*/
285-
private function insertSteps(Document $document, Session $session, array $steps): int {
282+
private function insertSteps(Document $document, Session $session, array $steps): void {
286283
$stepsVersion = null;
287284
try {
288285
$stepsJson = json_encode($steps, JSON_THROW_ON_ERROR);
@@ -298,7 +295,6 @@ private function insertSteps(Document $document, Session $session, array $steps)
298295
$this->logger->debug('Adding steps to ' . $document->getId() . ": bumping version from $stepsVersion to $newVersion");
299296
$this->cache->set('document-version-' . $document->getId(), $newVersion);
300297
// TODO write steps to cache for quicker reading
301-
return $newVersion;
302298
} catch (\Throwable $e) {
303299
if ($stepsVersion !== null) {
304300
$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‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ import {
9696
import ReadonlyBar from './Menu/ReadonlyBar.vue'
9797
9898
import { logger } from '../helpers/logger.js'
99-
import { applyDocumentState, getDocumentState } from '../helpers/yjs.ts'
99+
import { getDocumentState } from '../helpers/yjs.ts'
100100
import { SyncService, ERROR_TYPE, IDLE_TIMEOUT } from './../services/SyncService.ts'
101101
import SessionApi from '../services/SessionApi.js'
102102
import createSyncServiceProvider from './../services/SyncServiceProvider.js'
@@ -556,9 +556,7 @@ export default {
556556
},
557557
558558
onLoaded({ document, documentSource, documentState }) {
559-
if (documentState) {
560-
applyDocumentState(this.$ydoc, documentState, this.$providers[0])
561-
} else {
559+
if (!documentState) {
562560
this.setInitialYjsState(documentSource, { isRichEditor: this.isRichEditor })
563561
}
564562

‎src/helpers/yjs.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,12 @@ export function applyDocumentState(
4343
* and encode it and wrap it in a step data structure.
4444
*
4545
* @param documentState - base64 encoded doc state
46+
* @param version - last saved version for the document state
4647
* @return base64 encoded yjs sync protocol update message and version
4748
*/
48-
export function documentStateToStep(documentState: string): Step {
49+
export function documentStateToStep(documentState: string, version: number): Step {
4950
const message = documentStateToUpdateMessage(documentState)
50-
return { data: [encodeArrayBuffer(message)], sessionId: 0, version: -1 }
51+
return { data: [encodeArrayBuffer(message)], sessionId: 0, version }
5152
}
5253

5354
/**

‎src/services/SyncService.ts‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,13 @@ class SyncService {
199199
}
200200
this.bus.emit('opened', connectionState)
201201
this.bus.emit('loaded', connectionState)
202+
// Emit sync after opened, so websocket onmessage comes after onopen.
203+
if (connectionState.documentState) {
204+
this._emitDocumentStateStep(
205+
connectionState.documentState,
206+
connectionState.document.lastSavedVersion,
207+
)
208+
}
202209
return connectionState
203210
}
204211

@@ -218,6 +225,13 @@ class SyncService {
218225
}
219226
}
220227

228+
_emitDocumentStateStep(documentState: string, version: number) {
229+
const documentStateStep = documentStateToStep(documentState, version)
230+
this.bus.emit('sync', {
231+
steps: [documentStateStep],
232+
})
233+
}
234+
221235
updateSession(guestName: string) {
222236
if (!this.connection?.isPublic) {
223237
return Promise.reject(new Error())
@@ -266,12 +280,13 @@ class SyncService {
266280
return this.connection?.push({ ...sendable, version: this.version })
267281
.then((response) => {
268282
this.#outbox.clearSentData(sendable)
269-
const { steps, documentState } = response.data
283+
const { steps, documentState, version } = response.data as {
284+
steps: Step[]
285+
documentState: string
286+
version: number
287+
}
270288
if (documentState) {
271-
const documentStateStep = documentStateToStep(documentState)
272-
this.bus.emit('sync', {
273-
steps: [documentStateStep],
274-
})
289+
this._emitDocumentStateStep(documentState, version)
275290
}
276291
this.pushError = 0
277292
this.sending = false

‎src/services/WebSocketPolyfill.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,22 @@ export default function initWebSocketPolyfill(syncService: SyncService, fileId:
2525
onopen?: () => void
2626
#notifyPushBus
2727
#onSync
28+
#onOpened
2829
#processingVersion = 0
2930

3031
constructor(url: string) {
3132
this.#notifyPushBus = getNotifyBus()
3233
this.#notifyPushBus?.on('notify_push', this.#onNotifyPush.bind(this))
3334
this.#url = url
3435
logger.debug('WebSocketPolyfill#constructor', { url, fileId, initialSession })
36+
37+
this.#onOpened = () => {
38+
if (syncService.hasActiveConnection()) {
39+
this.onopen?.()
40+
}
41+
}
42+
syncService.bus.on('opened', this.#onOpened)
43+
3544
this.#onSync = ({ steps }: { steps: Step[] }) => {
3645
if (steps) {
3746
this.#processSteps(steps)
@@ -41,14 +50,9 @@ export default function initWebSocketPolyfill(syncService: SyncService, fileId:
4150
})
4251
}
4352
}
44-
4553
syncService.bus.on('sync', this.#onSync)
4654

47-
syncService.open({ fileId, initialSession }).then((_data) => {
48-
if (syncService.hasActiveConnection()) {
49-
this.onopen?.()
50-
}
51-
})
55+
syncService.open({ fileId, initialSession })
5256
}
5357

5458
/**

0 commit comments

Comments
 (0)