From 5ddd973eaee74c82e7c9b6be0bedffa9fe6ec0a8 Mon Sep 17 00:00:00 2001
From: Yohan Boniface <yohanboniface@free.fr>
Date: Mon, 24 Feb 2025 18:52:40 +0100
Subject: [PATCH] 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.
---
 umap/static/umap/js/modules/sync/engine.js    | 14 ++++++++------
 umap/static/umap/js/modules/sync/websocket.js |  4 ++++
 umap/tests/integration/test_websocket_sync.py |  5 +++++
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/umap/static/umap/js/modules/sync/engine.js b/umap/static/umap/js/modules/sync/engine.js
index 10314167..08b76cc5 100644
--- a/umap/static/umap/js/modules/sync/engine.js
+++ b/umap/static/umap/js/modules/sync/engine.js
@@ -66,7 +66,12 @@ export class SyncEngine {
     this.peerId = Utils.generateId()
   }
 
+  get isOpen() {
+    return this.transport?.isOpen
+  }
+
   async authenticate() {
+    if (this.isOpen) return
     const websocketTokenURI = this._umap.urls.get('map_websocket_auth_token', {
       map_id: this._umap.id,
     })
@@ -198,6 +203,7 @@ export class SyncEngine {
    */
   onOperationMessage(payload) {
     if (payload.sender === this.peerId) return
+    debug('received operation', payload)
     this._operations.storeRemoteOperations([payload])
     this._applyOperation(payload)
   }
@@ -261,7 +267,7 @@ export class SyncEngine {
    * @param {*} operations The list of (encoded operations)
    */
   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
 
@@ -502,11 +508,7 @@ export class Operations {
    * @return {bool} true if the two operations share the same context.
    */
   static haveSameContext(local, remote) {
-    const shouldCheckKey =
-      local.hasOwnProperty('key') &&
-      remote.hasOwnProperty('key') &&
-      typeof local.key !== 'undefined' &&
-      typeof remote.key !== 'undefined'
+    const shouldCheckKey = local.key !== undefined && remote.key !== undefined
 
     return (
       Utils.deepEqual(local.subject, remote.subject) &&
diff --git a/umap/static/umap/js/modules/sync/websocket.js b/umap/static/umap/js/modules/sync/websocket.js
index ee51019a..8ea16d26 100644
--- a/umap/static/umap/js/modules/sync/websocket.js
+++ b/umap/static/umap/js/modules/sync/websocket.js
@@ -76,4 +76,8 @@ export class WebSocketTransport {
     this.receiver.closeRequested = true
     this.websocket.close()
   }
+
+  get isOpen() {
+    return this.websocket?.readyState === WebSocket.OPEN
+  }
 }
diff --git a/umap/tests/integration/test_websocket_sync.py b/umap/tests/integration/test_websocket_sync.py
index 4f081ed2..41d0de52 100644
--- a/umap/tests/integration/test_websocket_sync.py
+++ b/umap/tests/integration/test_websocket_sync.py
@@ -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(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
     with peerA.expect_response(re.compile("./datalayer/create/.*")):
         peerA.get_by_role("button", name="Save").click()