Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable28] Keep base version etag during reload #5986

Merged
merged 7 commits into from
Jul 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 28 additions & 16 deletions cypress/e2e/sync.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,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', () => {
Expand All @@ -64,22 +64,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')
Expand All @@ -97,6 +86,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) => {
Expand Down Expand Up @@ -128,7 +140,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')
Expand Down
33 changes: 17 additions & 16 deletions src/components/Editor.vue
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ export default {
const timeout = new Promise((resolve) => setTimeout(resolve, 2000))
await Promise.any([timeout, this.$syncService.save()])
}
this.$providers.forEach(p => p.destroy())
this.close()
},
methods: {
...mapActions('text', [
Expand All @@ -370,7 +370,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)
Expand All @@ -380,8 +380,6 @@ export default {

this.listenSyncServiceEvents()

this.$providers.forEach(p => p?.destroy())
this.$providers = []
const syncServiceProvider = createSyncServiceProvider({
ydoc: this.$ydoc,
syncService: this.$syncService,
Expand Down Expand Up @@ -429,7 +427,7 @@ export default {
reconnect() {
this.contentLoaded = false
this.hasConnectionIssue = false
this.close().then(this.initSession)
this.disconnect().then(this.initSession)
this.idle = false
},

Expand Down Expand Up @@ -485,7 +483,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
Expand All @@ -498,6 +496,7 @@ export default {
this.setInitialYjsState(documentSource, { isRichEditor: this.isRichEditor })
}

this.$baseVersionEtag = document.baseVersionEtag
this.hasConnectionIssue = false
const language = extensionHighlight[this.fileExtension] || this.fileExtension;

Expand Down Expand Up @@ -658,17 +657,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()
Expand Down
4 changes: 2 additions & 2 deletions src/components/Editor/GuestNameDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,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()
Expand Down
20 changes: 20 additions & 0 deletions src/helpers/yjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,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
*
Expand Down
2 changes: 1 addition & 1 deletion src/services/PollingBackend.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,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()
Expand Down
13 changes: 10 additions & 3 deletions src/services/SessionApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,10 @@ export class Connection {
}
}

get isClosed() {
return this.closed
}

get #defaultParams() {
return {
documentId: this.#document.id,
Expand Down Expand Up @@ -178,9 +182,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
Expand Down
Loading
Loading