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

[stable26] Fix 403 on close #4969

Merged
merged 3 commits into from
Nov 9, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions cypress/e2e/SessionApi.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,19 @@ describe('The session Api', function() {
})
})

it('signals closing connection', function() {
cy.then(() => {
return new Promise((resolve, reject) => {
connection.close()
connection.push({ steps: [messages.update], version, awareness: '' })
.then(
() => reject(new Error('Push should have thrown ConnectionClosed()')),
resolve,
)
})
})
})

it('sends initial content if other session is alive but did not push any steps', function() {
let joining
cy.createTextSession(undefined, { filePath: '', shareToken })
Expand Down
4 changes: 2 additions & 2 deletions js/editor.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/editor.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-editors.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-editors.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-files.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-files.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-public.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-public.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-text.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-text.js.map

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions js/text-viewer.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion js/text-viewer.js.map

Large diffs are not rendered by default.

31 changes: 25 additions & 6 deletions src/services/SessionApi.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,14 @@
import axios from '@nextcloud/axios'
import { generateUrl } from '@nextcloud/router'

export class ConnectionClosedError extends Error {

constructor(message = 'Close has already been called on the connection', ...rest) {
super(message, ...rest)
}

}

class SessionApi {

#options
Expand Down Expand Up @@ -50,6 +58,7 @@ class SessionApi {
export class Connection {

#content
#closed
#documentState
#document
#session
Expand All @@ -66,6 +75,7 @@ export class Connection {
this.#content = content
this.#documentState = documentState
this.#options = options
this.closed = false
}

get document() {
Expand Down Expand Up @@ -95,7 +105,7 @@ export class Connection {
}

sync({ version, autosaveContent, documentState, force, manualSave }) {
return axios.post(this.#url('session/sync'), {
return this.#post(this.#url('session/sync'), {
...this.#defaultParams,
filePath: this.#options.filePath,
version,
Expand All @@ -107,7 +117,7 @@ export class Connection {
}

push({ steps, version, awareness }) {
return axios.post(this.#url('session/push'), {
return this.#post(this.#url('session/push'), {
...this.#defaultParams,
filePath: this.#options.filePath,
steps,
Expand All @@ -118,7 +128,7 @@ export class Connection {

// TODO: maybe return a new connection here so connections have immutable state
update(guestName) {
return axios.post(this.#url('session'), {
return this.#post(this.#url('session'), {
...this.#defaultParams,
guestName,
}).then(({ data }) => {
Expand All @@ -134,15 +144,15 @@ export class Connection {
+ '&sessionId=' + encodeURIComponent(this.#session.id)
+ '&sessionToken=' + encodeURIComponent(this.#session.token)
+ '&shareToken=' + encodeURIComponent(this.#options.shareToken || '')
return axios.post(url, formData, {
return this.#post(url, formData, {
headers: {
'Content-Type': 'multipart/form-data',
},
})
}

insertAttachmentFile(filePath) {
return axios.post(_endpointUrl('attachment/filepath'), {
return this.#post(_endpointUrl('attachment/filepath'), {
documentId: this.#document.id,
sessionId: this.#session.id,
sessionToken: this.#session.token,
Expand All @@ -151,7 +161,16 @@ export class Connection {
}

close() {
return axios.post(this.#url('session/close'), this.#defaultParams)
const promise = this.#post(this.#url('session/close'), this.#defaultParams)
this.closed = true
return promise
}

#post(...args) {
if (this.closed) {
return Promise.reject(new ConnectionClosedError())
}
return axios.post(...args)
}

#url(endpoint) {
Expand Down
8 changes: 4 additions & 4 deletions src/services/SyncService.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,16 +152,16 @@ class SyncService {
return new Promise((resolve, reject) => {
this.#sendIntervalId = setInterval(() => {
if (this.connection && !this.sending) {
clearInterval(this.#sendIntervalId)
this.#sendIntervalId = null
this._sendSteps(getSendable).then(resolve).catch(reject)
this.sendStepsNow(getSendable).then(resolve).catch(reject)
}
}, 200)
})
}

_sendSteps(getSendable) {
sendStepsNow(getSendable) {
this.sending = true
clearInterval(this.#sendIntervalId)
this.#sendIntervalId = null
const data = getSendable()
if (data.steps.length > 0) {
this.emit('stateChange', { dirty: true })
Expand Down
22 changes: 20 additions & 2 deletions src/services/WebSocketPolyfill.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
send(...data) {
this.#queue.push(...data)
let outbox = []
syncService.sendSteps(() => {
return syncService.sendSteps(() => {
outbox = [...this.#queue]
const data = {
steps: this.#steps,
Expand All @@ -109,7 +109,8 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
.findLast(s => s > 'AQ') || ''
}

close() {
async close() {
await this.#sendRemainingSteps()
Object.entries(this.#handlers)
.forEach(([key, value]) => syncService.off(key, value))
this.#handlers = []
Expand All @@ -119,5 +120,22 @@ export default function initWebSocketPolyfill(syncService, fileId, initialSessio
logger.debug('Websocket closed')
}

#sendRemainingSteps() {
if (this.#queue.length) {
return syncService.sendStepsNow(() => {
const data = {
steps: this.#steps,
awareness: this.#awareness,
version: this.#version,
}
this.#queue = []
logger.debug('sending final steps ', data)
return data
})?.catch(err => {
logger.error(err)
})
}
}

}
}