fix: prevent client to open two websocket connections

To reproduce:
- create a map
- saved it
- change the "syncEnabled" setting to on
- save again
- open another tab with this map
- switch on edit mode

In this case, the second client will try to authenticate:
- once switch on edit mode
- and once receiving the operation message from peer A about the
  syncEnabled (which calls render, which calls initSyncEngine in
  in this case)

I think we want to keep render to call initSyncEngine, so if a map
owner switch off the syncEnabled setting, this will (should) disconnect
the other peers.
This commit is contained in:
Yohan Boniface 2025-02-24 18:52:40 +01:00
parent 6e40388f7d
commit 5ddd973eae
3 changed files with 17 additions and 6 deletions

View file

@ -66,7 +66,12 @@ export class SyncEngine {
this.peerId = Utils.generateId() this.peerId = Utils.generateId()
} }
get isOpen() {
return this.transport?.isOpen
}
async authenticate() { async authenticate() {
if (this.isOpen) return
const websocketTokenURI = this._umap.urls.get('map_websocket_auth_token', { const websocketTokenURI = this._umap.urls.get('map_websocket_auth_token', {
map_id: this._umap.id, map_id: this._umap.id,
}) })
@ -198,6 +203,7 @@ export class SyncEngine {
*/ */
onOperationMessage(payload) { onOperationMessage(payload) {
if (payload.sender === this.peerId) return if (payload.sender === this.peerId) return
debug('received operation', payload)
this._operations.storeRemoteOperations([payload]) this._operations.storeRemoteOperations([payload])
this._applyOperation(payload) this._applyOperation(payload)
} }
@ -261,7 +267,7 @@ export class SyncEngine {
* @param {*} operations The list of (encoded operations) * @param {*} operations The list of (encoded operations)
*/ */
onListOperationsResponse({ sender, message }) { onListOperationsResponse({ sender, message }) {
debug(`received operations from peer ${sender}`, message.operations) debug(`received operations list from peer ${sender}`, message.operations)
if (message.operations.length === 0) return if (message.operations.length === 0) return
@ -502,11 +508,7 @@ export class Operations {
* @return {bool} true if the two operations share the same context. * @return {bool} true if the two operations share the same context.
*/ */
static haveSameContext(local, remote) { static haveSameContext(local, remote) {
const shouldCheckKey = const shouldCheckKey = local.key !== undefined && remote.key !== undefined
local.hasOwnProperty('key') &&
remote.hasOwnProperty('key') &&
typeof local.key !== 'undefined' &&
typeof remote.key !== 'undefined'
return ( return (
Utils.deepEqual(local.subject, remote.subject) && Utils.deepEqual(local.subject, remote.subject) &&

View file

@ -76,4 +76,8 @@ export class WebSocketTransport {
this.receiver.closeRequested = true this.receiver.closeRequested = true
this.websocket.close() this.websocket.close()
} }
get isOpen() {
return this.websocket?.readyState === WebSocket.OPEN
}
} }

View file

@ -535,6 +535,11 @@ def test_create_and_sync_map(new_page, asgi_live_server, tilelayer, login, user)
expect(markersA).to_have_count(1) expect(markersA).to_have_count(1)
expect(markersB).to_have_count(1) expect(markersB).to_have_count(1)
# Make sure only one layer has been created on peer B
peerB.get_by_role("button", name="Open browser").click()
expect(peerB.locator("h5").get_by_text("Layer 1")).to_be_visible()
peerB.get_by_role("button", name="Close").click()
# Save and quit edit mode again # Save and quit edit mode again
with peerA.expect_response(re.compile("./datalayer/create/.*")): with peerA.expect_response(re.compile("./datalayer/create/.*")):
peerA.get_by_role("button", name="Save").click() peerA.get_by_role("button", name="Save").click()