Merge pull request #5941 from nextcloud/fix/keep-base-version-etag-du…

…ring-reload

Keep base version etag during reload

cypress/e2e/sync.spec.js

@@ -19,10 +19,10 @@ describe('Sync', () => {
 		cy.intercept({ method: 'POST', url: '**/apps/text/session/*/sync' }).as('sync')
 		cy.intercept({ method: 'POST', url: '**/apps/text/session/*/save' }).as('save')
 		cy.openTestFile()
-		cy.wait('@sync')
+		cy.wait('@sync', { timeout: 10000 })
 		cy.getContent().find('h2').should('contain', 'Hello world')
 		cy.getContent().type('{moveToEnd}* Saving the doc saves the doc state{enter}')
-		cy.wait('@sync')
+		cy.wait('@sync', { timeout: 10000 })
 	})
 
 	it('saves the actual file and document state', () => {
@@ -47,22 +47,11 @@ describe('Sync', () => {
 	})
 
 	it('recovers from a short lost connection', () => {
-		let reconnect = false
-		cy.intercept('**/apps/text/session/*/*', (req) => {
-			if (reconnect) {
-				req.continue()
-				req.alias = 'alive'
-			} else {
-				req.destroy()
-				req.alias = 'dead'
-			}
-		}).as('sessionRequests')
+		cy.intercept('**/apps/text/session/*/*', req => req.destroy()).as('dead')
 		cy.wait('@dead', { timeout: 30000 })
 		cy.get('#editor-container .document-status', { timeout: 30000 })
 			.should('contain', 'Document could not be loaded.')
-			.then(() => {
-				reconnect = true
-			})
+		cy.intercept('**/apps/text/session/*/*', req => req.continue()).as('alive')
 		cy.wait('@alive', { timeout: 30000 })
 		cy.intercept({ method: 'POST', url: '**/apps/text/session/*/sync' })
 			.as('syncAfterRecovery')
@@ -80,6 +69,29 @@ describe('Sync', () => {
 			.should('include', 'after the lost connection')
 	})
 
+	it('reconnects via button after a short lost connection', () => {
+		cy.intercept('**/apps/text/session/*/*', req => req.destroy()).as('dead')
+		cy.wait('@dead', { timeout: 30000 })
+		cy.get('#editor-container .document-status', { timeout: 30000 })
+			.should('contain', 'Document could not be loaded.')
+		cy.get('#editor-container .document-status')
+			.find('.button.primary').click()
+		cy.get('.toastify').should('contain', 'Connection failed.')
+		cy.get('.toastify', { timeout: 30000 }).should('not.exist')
+		cy.get('#editor-container .document-status', { timeout: 30000 })
+			.should('contain', 'Document could not be loaded.')
+		// bring back the network connection
+		cy.intercept('**/apps/text/session/*/*', req => { req.continue() }).as('alive')
+		cy.intercept('**/apps/text/session/*/create').as('create')
+		cy.get('#editor-container .document-status')
+			.find('.button.primary').click()
+		cy.wait('@alive', { timeout: 30000 })
+		cy.wait('@create', { timeout: 10000 })
+			.its('request.body')
+			.should('have.property', 'baseVersionEtag')
+			.should('not.be.empty')
+	})
+
 	it('recovers from a lost and closed connection', () => {
 		let reconnect = false
 		cy.intercept('**/apps/text/session/*/*', (req) => {
@@ -111,7 +123,7 @@ describe('Sync', () => {
 			.should('include', 'after the lost connection')
 	})
 
-	it('shows warning when document session got cleaned up', () => {
+	it('asks to reload page when document session got cleaned up', () => {
 		cy.get('.save-status button')
 			.click()
 		cy.wait('@save')

src/components/Editor.vue

@@ -354,12 +354,12 @@ export default {
 		}
 		unsubscribe('text:image-node:add', this.onAddImageNode)
 		unsubscribe('text:image-node:delete', this.onDeleteImageNode)
+		unsubscribe('text:translate-modal:show', this.showTranslateModal)
 		if (this.dirty) {
 			const timeout = new Promise((resolve) => setTimeout(resolve, 2000))
 			await Promise.any([timeout, this.$syncService.save()])
 		}
-		this.$providers.forEach(p => p.destroy())
-		unsubscribe('text:translate-modal:show', this.showTranslateModal)
+		this.close()
 	},
 	methods: {
 		initSession() {
@@ -373,7 +373,7 @@ export default {
 				guestName,
 				shareToken: this.shareToken,
 				filePath: this.relativePath,
-				baseVersionEtag: this.$syncService?.baseVersionEtag,
+				baseVersionEtag: this.$baseVersionEtag,
 				forceRecreate: this.forceRecreate,
 				serialize: this.isRichEditor
 					? (content) => createMarkdownSerializer(this.$editor.schema).serialize(content ?? this.$editor.state.doc)
@@ -383,8 +383,6 @@ export default {
 
 			this.listenSyncServiceEvents()
 
-			this.$providers.forEach(p => p?.destroy())
-			this.$providers = []
 			const syncServiceProvider = createSyncServiceProvider({
 				ydoc: this.$ydoc,
 				syncService: this.$syncService,
@@ -432,7 +430,7 @@ export default {
 		reconnect() {
 			this.contentLoaded = false
 			this.hasConnectionIssue = false
-			this.close().then(this.initSession)
+			this.disconnect().then(this.initSession)
 			this.idle = false
 		},
 
@@ -487,7 +485,7 @@ export default {
 			})
 		},
 
-		onLoaded({ documentSource, documentState }) {
+		onLoaded({ document, documentSource, documentState }) {
 			if (documentState) {
 				applyDocumentState(this.$ydoc, documentState, this.$providers[0])
 				// distribute additional state that may exist locally
@@ -500,6 +498,7 @@ export default {
 				this.setInitialYjsState(documentSource, { isRichEditor: this.isRichEditor })
 			}
 
+			this.$baseVersionEtag = document.baseVersionEtag
 			this.hasConnectionIssue = false
 			const language = extensionHighlight[this.fileExtension] || this.fileExtension;
 
@@ -667,17 +666,19 @@ export default {
 			await this.$syncService.save()
 		},
 
+		async disconnect() {
+			await this.$syncService.close()
+			this.unlistenSyncServiceEvents()
+			this.$providers.forEach(p => p?.destroy())
+			this.$providers = []
+			this.$syncService = null
+			// disallow editing while still showing the content
+			this.readOnly = true
+		},
+
 		async close() {
-			if (this.currentSession && this.$syncService) {
-				try {
-					await this.$syncService.close()
-					this.unlistenSyncServiceEvents()
-					this.currentSession = null
-					this.$syncService = null
-				} catch (e) {
-					// Ignore issues closing the session since those might happen due to network issues
-				}
-			}
+			await this.$syncService.sendRemainingSteps(this.$queue)
+			await this.disconnect()
 			if (this.$editor) {
 				try {
 					this.unlistenEditorEvents()

src/components/Editor/GuestNameDialog.vue

@@ -55,12 +55,12 @@ export default {
 		},
 	},
 	beforeMount() {
-		this.guestName = this.$syncService.connection.session.guestName
+		this.guestName = this.$syncService.guestName
 		this.updateBufferedGuestName()
 	},
 	methods: {
 		setGuestName() {
-			const previousGuestName = this.$syncService.connection.session.guestName
+			const previousGuestName = this.$syncService.guestName
 			this.$syncService.updateSession(this.guestName).then(() => {
 				localStorage.setItem('nick', this.guestName)
 				this.updateBufferedGuestName()

src/helpers/yjs.js

@@ -80,6 +80,26 @@ export function applyUpdateMessage(ydoc, updateMessage, origin = 'origin') {
 	)
 }
 
+/**
+ * Get the steps for sending to the server
+ *
+ * @param {object[]} queue - queue for the outgoing steps
+ */
+export function getSteps(queue) {
+	return queue.map(s => encodeArrayBuffer(s))
+		.filter(s => s < 'AQ')
+}
+
+/**
+ * Encode the latest awareness message for sending
+ *
+ * @param {object[]} queue - queue for the outgoing steps
+ */
+export function getAwareness(queue) {
+	return queue.map(s => encodeArrayBuffer(s))
+		.findLast(s => s > 'AQ') || ''
+}
+
 /**
  * Log y.js messages with their type and initiator call stack
  *

src/services/PollingBackend.js

@@ -126,7 +126,7 @@ class PollingBackend {
 			}
 			const disconnect = Date.now() - COLLABORATOR_DISCONNECT_TIME
 			const alive = sessions.filter((s) => s.lastContact * 1000 > disconnect)
-			if (this.#syncService.connection.state.document.readOnly) {
+			if (this.#syncService.isReadOnly) {
 				this.maximumReadOnlyTimer()
 			} else if (alive.length < 2) {
 				this.maximumRefetchTimer()

src/services/SessionApi.js

@@ -84,6 +84,10 @@ export class Connection {
 		}
 	}
 
+	get isClosed() {
+		return this.closed
+	}
+
 	get #defaultParams() {
 		return {
 			documentId: this.#document.id,
@@ -161,9 +165,12 @@ export class Connection {
 	}
 
 	close() {
-		const promise = this.#post(this.#url(`session/${this.#document.id}/close`), this.#defaultParams)
-		this.closed = true
-		return promise
+		return this.#post(
+			this.#url(`session/${this.#document.id}/close`),
+			this.#defaultParams,
+		).then(() => {
+			this.closed = true
+		})
 	}
 
 	// To be used in Cypress tests only