From a4cc0b2c5b8fcd0fe3790ad629ac3e01d97729bd Mon Sep 17 00:00:00 2001
From: Miguel Grinberg <miguel.grinberg@gmail.com>
Date: Sat, 5 Dec 2020 15:38:07 +0000
Subject: [PATCH] v5 protocol: client manager unit tests

---
 socketio/asyncio_manager.py                  |   8 +-
 socketio/asyncio_pubsub_manager.py           |   5 +-
 socketio/asyncio_server.py                   |   2 +-
 socketio/base_manager.py                     |  27 +-
 socketio/pubsub_manager.py                   |   7 +-
 socketio/server.py                           |   8 +-
 tests/asyncio/test_asyncio_manager.py        | 272 ++++++++++---------
 tests/asyncio/test_asyncio_pubsub_manager.py |  14 +-
 tests/common/test_base_manager.py            |  92 ++++---
 tests/common/test_pubsub_manager.py          |  14 +-
 10 files changed, 228 insertions(+), 221 deletions(-)

diff --git a/socketio/asyncio_manager.py b/socketio/asyncio_manager.py
index db9fecf..f89022c 100644
--- a/socketio/asyncio_manager.py
+++ b/socketio/asyncio_manager.py
@@ -23,7 +23,7 @@ class AsyncManager(BaseManager):
         for sid, eio_sid in self.get_participants(namespace, room):
             if sid not in skip_sid:
                 if callback is not None:
-                    id = self._generate_ack_id(sid, namespace, callback)
+                    id = self._generate_ack_id(sid, callback)
                 else:
                     id = None
                 tasks.append(self.server._emit_internal(eio_sid, event, data,
@@ -39,19 +39,19 @@ class AsyncManager(BaseManager):
         """
         return super().close_room(room, namespace)
 
-    async def trigger_callback(self, sid, namespace, id, data):
+    async def trigger_callback(self, sid, id, data):
         """Invoke an application callback.
 
         Note: this method is a coroutine.
         """
         callback = None
         try:
-            callback = self.callbacks[sid][namespace][id]
+            callback = self.callbacks[sid][id]
         except KeyError:
             # if we get an unknown callback we just ignore it
             self._get_logger().warning('Unknown callback received, ignoring.')
         else:
-            del self.callbacks[sid][namespace][id]
+            del self.callbacks[sid][id]
         if callback is not None:
             ret = callback(*data)
             if asyncio.iscoroutine(ret):
diff --git a/socketio/asyncio_pubsub_manager.py b/socketio/asyncio_pubsub_manager.py
index 0e41f25..fc4e205 100644
--- a/socketio/asyncio_pubsub_manager.py
+++ b/socketio/asyncio_pubsub_manager.py
@@ -60,7 +60,7 @@ class AsyncPubSubManager(AsyncManager):
                                    'context of a server.')
             if room is None:
                 raise ValueError('Cannot use callback without a room set.')
-            id = self._generate_ack_id(room, namespace, callback)
+            id = self._generate_ack_id(room, callback)
             callback = (room, namespace, id)
         else:
             callback = None
@@ -122,12 +122,11 @@ class AsyncPubSubManager(AsyncManager):
         if self.host_id == message.get('host_id'):
             try:
                 sid = message['sid']
-                namespace = message['namespace']
                 id = message['id']
                 args = message['args']
             except KeyError:
                 return
-            await self.trigger_callback(sid, namespace, id, args)
+            await self.trigger_callback(sid, id, args)
 
     async def _return_callback(self, host_id, sid, namespace, callback_id,
                                *args):
diff --git a/socketio/asyncio_server.py b/socketio/asyncio_server.py
index 5eaeb3a..d35d355 100644
--- a/socketio/asyncio_server.py
+++ b/socketio/asyncio_server.py
@@ -482,7 +482,7 @@ class AsyncServer(server.Server):
         namespace = namespace or '/'
         sid = self.manager.sid_from_eio_sid(eio_sid, namespace)
         self.logger.info('received ack from %s [%s]', sid, namespace)
-        await self.manager.trigger_callback(sid, namespace, id, data)
+        await self.manager.trigger_callback(sid, id, data)
 
     async def _trigger_event(self, event, namespace, *args):
         """Invoke an application event handler."""
diff --git a/socketio/base_manager.py b/socketio/base_manager.py
index 2053dc1..cec0e91 100644
--- a/socketio/base_manager.py
+++ b/socketio/base_manager.py
@@ -55,7 +55,7 @@ class BaseManager(object):
             # the client is in the process of being disconnected
             return False
         try:
-            return self.rooms[namespace][None][sid]
+            return self.rooms[namespace][None][sid] is not None
         except KeyError:
             pass
 
@@ -87,10 +87,8 @@ class BaseManager(object):
                 rooms.append(room_name)
         for room in rooms:
             self.leave_room(sid, namespace, room)
-        if sid in self.callbacks and namespace in self.callbacks[sid]:
-            del self.callbacks[sid][namespace]
-            if len(self.callbacks[sid]) == 0:
-                del self.callbacks[sid]
+        if sid in self.callbacks:
+            del self.callbacks[sid]
         if namespace in self.pending_disconnect and \
                 sid in self.pending_disconnect[namespace]:
             self.pending_disconnect[namespace].remove(sid)
@@ -148,33 +146,30 @@ class BaseManager(object):
         for sid, eio_sid in self.get_participants(namespace, room):
             if sid not in skip_sid:
                 if callback is not None:
-                    id = self._generate_ack_id(sid, namespace, callback)
+                    id = self._generate_ack_id(sid, callback)
                 else:
                     id = None
                 self.server._emit_internal(eio_sid, event, data, namespace, id)
 
-    def trigger_callback(self, sid, namespace, id, data):
+    def trigger_callback(self, sid, id, data):
         """Invoke an application callback."""
         callback = None
         try:
-            callback = self.callbacks[sid][namespace][id]
+            callback = self.callbacks[sid][id]
         except KeyError:
             # if we get an unknown callback we just ignore it
             self._get_logger().warning('Unknown callback received, ignoring.')
         else:
-            del self.callbacks[sid][namespace][id]
+            del self.callbacks[sid][id]
         if callback is not None:
             callback(*data)
 
-    def _generate_ack_id(self, sid, namespace, callback):
+    def _generate_ack_id(self, sid, callback):
         """Generate a unique identifier for an ACK packet."""
-        namespace = namespace or '/'
         if sid not in self.callbacks:
-            self.callbacks[sid] = {}
-        if namespace not in self.callbacks[sid]:
-            self.callbacks[sid][namespace] = {0: itertools.count(1)}
-        id = six.next(self.callbacks[sid][namespace][0])
-        self.callbacks[sid][namespace][id] = callback
+            self.callbacks[sid] = {0: itertools.count(1)}
+        id = six.next(self.callbacks[sid][0])
+        self.callbacks[sid][id] = callback
         return id
 
     def _get_logger(self):
diff --git a/socketio/pubsub_manager.py b/socketio/pubsub_manager.py
index b043857..dcbef88 100644
--- a/socketio/pubsub_manager.py
+++ b/socketio/pubsub_manager.py
@@ -58,7 +58,7 @@ class PubSubManager(BaseManager):
                                    'context of a server.')
             if room is None:
                 raise ValueError('Cannot use callback without a room set.')
-            id = self._generate_ack_id(room, namespace, callback)
+            id = self._generate_ack_id(room, callback)
             callback = (room, namespace, id)
         else:
             callback = None
@@ -120,16 +120,15 @@ class PubSubManager(BaseManager):
         if self.host_id == message.get('host_id'):
             try:
                 sid = message['sid']
-                namespace = message['namespace']
                 id = message['id']
                 args = message['args']
             except KeyError:
                 return
-            self.trigger_callback(sid, namespace, id, args)
+            self.trigger_callback(sid, id, args)
 
     def _return_callback(self, host_id, sid, namespace, callback_id, *args):
         # When an event callback is received, the callback is returned back
-        # the sender, which is identified by the host_id
+        # to the sender, which is identified by the host_id
         self._publish({'method': 'callback', 'host_id': host_id,
                        'sid': sid, 'namespace': namespace, 'id': callback_id,
                        'args': args})
diff --git a/socketio/server.py b/socketio/server.py
index b842f40..6418e76 100644
--- a/socketio/server.py
+++ b/socketio/server.py
@@ -579,7 +579,7 @@ class Server(object):
         """
         return self.eio.sleep(seconds)
 
-    def _emit_internal(self, sid, event, data, namespace=None, id=None):
+    def _emit_internal(self, eio_sid, event, data, namespace=None, id=None):
         """Send a message to a client."""
         # tuples are expanded to multiple arguments, everything else is sent
         # as a single argument
@@ -589,8 +589,8 @@ class Server(object):
             data = [data]
         else:
             data = []
-        self._send_packet(sid, packet.Packet(packet.EVENT, namespace=namespace,
-                                             data=[event] + data, id=id))
+        self._send_packet(eio_sid, packet.Packet(
+            packet.EVENT, namespace=namespace, data=[event] + data, id=id))
 
     def _send_packet(self, eio_sid, pkt):
         """Send a Socket.IO packet to a client."""
@@ -680,7 +680,7 @@ class Server(object):
         namespace = namespace or '/'
         sid = self.manager.sid_from_eio_sid(eio_sid, namespace)
         self.logger.info('received ack from %s [%s]', sid, namespace)
-        self.manager.trigger_callback(sid, namespace, id, data)
+        self.manager.trigger_callback(sid, id, data)
 
     def _trigger_event(self, event, namespace, *args):
         """Invoke an application event handler."""
diff --git a/tests/asyncio/test_asyncio_manager.py b/tests/asyncio/test_asyncio_manager.py
index ea4abf0..66fe53a 100644
--- a/tests/asyncio/test_asyncio_manager.py
+++ b/tests/asyncio/test_asyncio_manager.py
@@ -31,140 +31,142 @@ def _run(coro):
 @unittest.skipIf(sys.version_info < (3, 5), 'only for Python 3.5+')
 class TestAsyncManager(unittest.TestCase):
     def setUp(self):
+        id = 0
+
+        def generate_id():
+            nonlocal id
+            id += 1
+            return str(id)
+
         mock_server = mock.MagicMock()
         mock_server._emit_internal = AsyncMock()
+        mock_server.eio.generate_id = generate_id
         self.bm = asyncio_manager.AsyncManager()
         self.bm.set_server(mock_server)
         self.bm.initialize()
 
     def test_connect(self):
-        self.bm.connect('123', '/foo')
+        sid = self.bm.connect('123', '/foo')
         assert None in self.bm.rooms['/foo']
-        assert '123' in self.bm.rooms['/foo']
-        assert '123' in self.bm.rooms['/foo'][None]
-        assert '123' in self.bm.rooms['/foo']['123']
-        assert self.bm.rooms['/foo'] == {
-            None: {'123': True},
-            '123': {'123': True},
-        }
+        assert sid in self.bm.rooms['/foo']
+        assert sid in self.bm.rooms['/foo'][None]
+        assert sid in self.bm.rooms['/foo'][sid]
+        assert dict(self.bm.rooms['/foo'][None]) == {sid: '123'}
+        assert dict(self.bm.rooms['/foo'][sid]) == {sid: '123'}
+        assert self.bm.sid_from_eio_sid('123', '/foo') == sid
 
     def test_pre_disconnect(self):
-        self.bm.connect('123', '/foo')
-        self.bm.connect('456', '/foo')
-        self.bm.pre_disconnect('123', '/foo')
-        assert self.bm.pending_disconnect == {'/foo': ['123']}
-        assert not self.bm.is_connected('123', '/foo')
-        self.bm.pre_disconnect('456', '/foo')
-        assert self.bm.pending_disconnect == {'/foo': ['123', '456']}
-        assert not self.bm.is_connected('456', '/foo')
-        self.bm.disconnect('123', '/foo')
-        assert self.bm.pending_disconnect == {'/foo': ['456']}
-        self.bm.disconnect('456', '/foo')
+        sid1 = self.bm.connect('123', '/foo')
+        sid2 = self.bm.connect('456', '/foo')
+        assert self.bm.is_connected(sid1, '/foo')
+        assert self.bm.pre_disconnect(sid1, '/foo') == '123'
+        assert self.bm.pending_disconnect == {'/foo': [sid1]}
+        assert not self.bm.is_connected(sid1, '/foo')
+        assert self.bm.pre_disconnect(sid2, '/foo') == '456'
+        assert self.bm.pending_disconnect == {'/foo': [sid1, sid2]}
+        assert not self.bm.is_connected(sid2, '/foo')
+        self.bm.disconnect(sid1, '/foo')
+        assert self.bm.pending_disconnect == {'/foo': [sid2]}
+        self.bm.disconnect(sid2, '/foo')
         assert self.bm.pending_disconnect == {}
 
     def test_disconnect(self):
-        self.bm.connect('123', '/foo')
-        self.bm.connect('456', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.enter_room('456', '/foo', 'baz')
-        self.bm.disconnect('123', '/foo')
-        assert self.bm.rooms['/foo'] == {
-            None: {'456': True},
-            '456': {'456': True},
-            'baz': {'456': True},
-        }
+        sid1 = self.bm.connect('123', '/foo')
+        sid2 = self.bm.connect('456', '/foo')
+        self.bm.enter_room(sid1, '/foo', 'bar')
+        self.bm.enter_room(sid2, '/foo', 'baz')
+        self.bm.disconnect(sid1, '/foo')
+        assert dict(self.bm.rooms['/foo'][None]) == {sid2: '456'}
+        assert dict(self.bm.rooms['/foo'][sid2]) == {sid2: '456'}
+        assert dict(self.bm.rooms['/foo']['baz']) == {sid2: '456'}
 
     def test_disconnect_default_namespace(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
-        self.bm.connect('456', '/')
-        self.bm.connect('456', '/foo')
-        assert self.bm.is_connected('123', '/')
-        assert self.bm.is_connected('123', '/foo')
-        self.bm.disconnect('123', '/')
-        assert not self.bm.is_connected('123', '/')
-        assert self.bm.is_connected('123', '/foo')
-        self.bm.disconnect('123', '/foo')
-        assert not self.bm.is_connected('123', '/foo')
-        assert self.bm.rooms['/'] == {
-            None: {'456': True},
-            '456': {'456': True},
-        }
-        assert self.bm.rooms['/foo'] == {
-            None: {'456': True},
-            '456': {'456': True},
-        }
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
+        sid3 = self.bm.connect('456', '/')
+        sid4 = self.bm.connect('456', '/foo')
+        assert self.bm.is_connected(sid1, '/')
+        assert self.bm.is_connected(sid2, '/foo')
+        assert not self.bm.is_connected(sid2, '/')
+        assert not self.bm.is_connected(sid1, '/foo')
+        self.bm.disconnect(sid1, '/')
+        assert not self.bm.is_connected(sid1, '/')
+        assert self.bm.is_connected(sid2, '/foo')
+        self.bm.disconnect(sid2, '/foo')
+        assert not self.bm.is_connected(sid2, '/foo')
+        assert dict(self.bm.rooms['/'][None]) == {sid3: '456'}
+        assert dict(self.bm.rooms['/'][sid3]) == {sid3: '456'}
+        assert dict(self.bm.rooms['/foo'][None]) == {sid4: '456'}
+        assert dict(self.bm.rooms['/foo'][sid4]) == {sid4: '456'}
 
     def test_disconnect_twice(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
-        self.bm.connect('456', '/')
-        self.bm.connect('456', '/foo')
-        self.bm.disconnect('123', '/')
-        self.bm.disconnect('123', '/foo')
-        self.bm.disconnect('123', '/')
-        self.bm.disconnect('123', '/foo')
-        assert self.bm.rooms['/'] == {
-            None: {'456': True},
-            '456': {'456': True},
-        }
-        assert self.bm.rooms['/foo'] == {
-            None: {'456': True},
-            '456': {'456': True},
-        }
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
+        sid3 = self.bm.connect('456', '/')
+        sid4 = self.bm.connect('456', '/foo')
+        self.bm.disconnect(sid1, '/')
+        self.bm.disconnect(sid2, '/foo')
+        self.bm.disconnect(sid1, '/')
+        self.bm.disconnect(sid2, '/foo')
+        assert dict(self.bm.rooms['/'][None]) == {sid3: '456'}
+        assert dict(self.bm.rooms['/'][sid3]) == {sid3: '456'}
+        assert dict(self.bm.rooms['/foo'][None]) == {sid4: '456'}
+        assert dict(self.bm.rooms['/foo'][sid4]) == {sid4: '456'}
 
     def test_disconnect_all(self):
-        self.bm.connect('123', '/foo')
-        self.bm.connect('456', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.enter_room('456', '/foo', 'baz')
-        self.bm.disconnect('123', '/foo')
-        self.bm.disconnect('456', '/foo')
+        sid1 = self.bm.connect('123', '/foo')
+        sid2 = self.bm.connect('456', '/foo')
+        self.bm.enter_room(sid1, '/foo', 'bar')
+        self.bm.enter_room(sid2, '/foo', 'baz')
+        self.bm.disconnect(sid1, '/foo')
+        self.bm.disconnect(sid2, '/foo')
         assert self.bm.rooms == {}
 
     def test_disconnect_with_callbacks(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
-        self.bm._generate_ack_id('123', '/', 'f')
-        self.bm._generate_ack_id('123', '/foo', 'g')
-        self.bm.disconnect('123', '/foo')
-        assert '/foo' not in self.bm.callbacks['123']
-        self.bm.disconnect('123', '/')
-        assert '123' not in self.bm.callbacks
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
+        sid3 = self.bm.connect('456', '/foo')
+        self.bm._generate_ack_id(sid1, 'f')
+        self.bm._generate_ack_id(sid2, 'g')
+        self.bm._generate_ack_id(sid3, 'h')
+        self.bm.disconnect(sid2, '/foo')
+        assert sid2 not in self.bm.callbacks
+        self.bm.disconnect(sid1, '/')
+        assert sid1 not in self.bm.callbacks
+        assert sid3 in self.bm.callbacks
 
     def test_trigger_sync_callback(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
         cb = mock.MagicMock()
-        id1 = self.bm._generate_ack_id('123', '/', cb)
-        id2 = self.bm._generate_ack_id('123', '/foo', cb)
-        _run(self.bm.trigger_callback('123', '/', id1, ['foo']))
-        _run(self.bm.trigger_callback('123', '/foo', id2, ['bar', 'baz']))
+        id1 = self.bm._generate_ack_id(sid1, cb)
+        id2 = self.bm._generate_ack_id(sid2, cb)
+        _run(self.bm.trigger_callback(sid1, id1, ['foo']))
+        _run(self.bm.trigger_callback(sid2, id2, ['bar', 'baz']))
         assert cb.call_count == 2
         cb.assert_any_call('foo')
         cb.assert_any_call('bar', 'baz')
 
     def test_trigger_async_callback(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
         cb = AsyncMock()
-        id1 = self.bm._generate_ack_id('123', '/', cb)
-        id2 = self.bm._generate_ack_id('123', '/foo', cb)
-        _run(self.bm.trigger_callback('123', '/', id1, ['foo']))
-        _run(self.bm.trigger_callback('123', '/foo', id2, ['bar', 'baz']))
+        id1 = self.bm._generate_ack_id(sid1, cb)
+        id2 = self.bm._generate_ack_id(sid2, cb)
+        _run(self.bm.trigger_callback(sid1, id1, ['foo']))
+        _run(self.bm.trigger_callback(sid2, id2, ['bar', 'baz']))
         assert cb.mock.call_count == 2
         cb.mock.assert_any_call('foo')
         cb.mock.assert_any_call('bar', 'baz')
 
     def test_invalid_callback(self):
-        self.bm.connect('123', '/')
+        sid = self.bm.connect('123', '/')
         cb = mock.MagicMock()
-        id = self.bm._generate_ack_id('123', '/', cb)
+        id = self.bm._generate_ack_id(sid, cb)
 
         # these should not raise an exception
-        _run(self.bm.trigger_callback('124', '/', id, ['foo']))
-        _run(self.bm.trigger_callback('123', '/foo', id, ['foo']))
-        _run(self.bm.trigger_callback('123', '/', id + 1, ['foo']))
+        _run(self.bm.trigger_callback('xxx', id, ['foo']))
+        _run(self.bm.trigger_callback(sid, id + 1, ['foo']))
         assert cb.mock.call_count == 0
 
     def test_get_namespaces(self):
@@ -177,30 +179,32 @@ class TestAsyncManager(unittest.TestCase):
         assert '/foo' in namespaces
 
     def test_get_participants(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('456', '/')
-        self.bm.connect('789', '/')
-        self.bm.disconnect('789', '/')
-        assert '789' not in self.bm.rooms['/'][None]
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('456', '/')
+        sid3 = self.bm.connect('789', '/')
+        self.bm.disconnect(sid3, '/')
+        assert sid3 not in self.bm.rooms['/'][None]
         participants = list(self.bm.get_participants('/', None))
         assert len(participants) == 2
-        assert '789' not in participants
+        assert (sid1, '123') in participants
+        assert (sid2, '456') in participants
+        assert (sid3, '789') not in participants
 
     def test_leave_invalid_room(self):
-        self.bm.connect('123', '/foo')
-        self.bm.leave_room('123', '/foo', 'baz')
-        self.bm.leave_room('123', '/bar', 'baz')
+        sid = self.bm.connect('123', '/foo')
+        self.bm.leave_room(sid, '/foo', 'baz')
+        self.bm.leave_room(sid, '/bar', 'baz')
 
     def test_no_room(self):
         rooms = self.bm.get_rooms('123', '/foo')
         assert [] == rooms
 
     def test_close_room(self):
-        self.bm.connect('123', '/foo')
+        sid = self.bm.connect('123', '/foo')
         self.bm.connect('456', '/foo')
         self.bm.connect('789', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.enter_room('123', '/foo', 'bar')
+        self.bm.enter_room(sid, '/foo', 'bar')
+        self.bm.enter_room(sid, '/foo', 'bar')
         _run(self.bm.close_room('bar', '/foo'))
         assert 'bar' not in self.bm.rooms['/foo']
 
@@ -208,19 +212,19 @@ class TestAsyncManager(unittest.TestCase):
         self.bm.close_room('bar', '/foo')
 
     def test_rooms(self):
-        self.bm.connect('123', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        r = self.bm.get_rooms('123', '/foo')
+        sid = self.bm.connect('123', '/foo')
+        self.bm.enter_room(sid, '/foo', 'bar')
+        r = self.bm.get_rooms(sid, '/foo')
         assert len(r) == 2
-        assert '123' in r
+        assert sid in r
         assert 'bar' in r
 
     def test_emit_to_sid(self):
-        self.bm.connect('123', '/foo')
+        sid = self.bm.connect('123', '/foo')
         self.bm.connect('456', '/foo')
         _run(
             self.bm.emit(
-                'my event', {'foo': 'bar'}, namespace='/foo', room='123'
+                'my event', {'foo': 'bar'}, namespace='/foo', room=sid
             )
         )
         self.bm.server._emit_internal.mock.assert_called_once_with(
@@ -228,10 +232,10 @@ class TestAsyncManager(unittest.TestCase):
         )
 
     def test_emit_to_room(self):
-        self.bm.connect('123', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.connect('456', '/foo')
-        self.bm.enter_room('456', '/foo', 'bar')
+        sid1 = self.bm.connect('123', '/foo')
+        self.bm.enter_room(sid1, '/foo', 'bar')
+        sid2 = self.bm.connect('456', '/foo')
+        self.bm.enter_room(sid2, '/foo', 'bar')
         self.bm.connect('789', '/foo')
         _run(
             self.bm.emit(
@@ -247,10 +251,10 @@ class TestAsyncManager(unittest.TestCase):
         )
 
     def test_emit_to_all(self):
-        self.bm.connect('123', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.connect('456', '/foo')
-        self.bm.enter_room('456', '/foo', 'bar')
+        sid1 = self.bm.connect('123', '/foo')
+        self.bm.enter_room(sid1, '/foo', 'bar')
+        sid2 = self.bm.connect('456', '/foo')
+        self.bm.enter_room(sid2, '/foo', 'bar')
         self.bm.connect('789', '/foo')
         self.bm.connect('abc', '/bar')
         _run(self.bm.emit('my event', {'foo': 'bar'}, namespace='/foo'))
@@ -266,15 +270,15 @@ class TestAsyncManager(unittest.TestCase):
         )
 
     def test_emit_to_all_skip_one(self):
-        self.bm.connect('123', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.connect('456', '/foo')
-        self.bm.enter_room('456', '/foo', 'bar')
+        sid1 = self.bm.connect('123', '/foo')
+        self.bm.enter_room(sid1, '/foo', 'bar')
+        sid2 = self.bm.connect('456', '/foo')
+        self.bm.enter_room(sid2, '/foo', 'bar')
         self.bm.connect('789', '/foo')
         self.bm.connect('abc', '/bar')
         _run(
             self.bm.emit(
-                'my event', {'foo': 'bar'}, namespace='/foo', skip_sid='456'
+                'my event', {'foo': 'bar'}, namespace='/foo', skip_sid=sid2
             )
         )
         assert self.bm.server._emit_internal.mock.call_count == 2
@@ -286,18 +290,18 @@ class TestAsyncManager(unittest.TestCase):
         )
 
     def test_emit_to_all_skip_two(self):
-        self.bm.connect('123', '/foo')
-        self.bm.enter_room('123', '/foo', 'bar')
-        self.bm.connect('456', '/foo')
-        self.bm.enter_room('456', '/foo', 'bar')
-        self.bm.connect('789', '/foo')
+        sid1 = self.bm.connect('123', '/foo')
+        self.bm.enter_room(sid1, '/foo', 'bar')
+        sid2 = self.bm.connect('456', '/foo')
+        self.bm.enter_room(sid2, '/foo', 'bar')
+        sid3 = self.bm.connect('789', '/foo')
         self.bm.connect('abc', '/bar')
         _run(
             self.bm.emit(
                 'my event',
                 {'foo': 'bar'},
                 namespace='/foo',
-                skip_sid=['123', '789'],
+                skip_sid=[sid1, sid3],
             )
         )
         assert self.bm.server._emit_internal.mock.call_count == 1
@@ -306,7 +310,7 @@ class TestAsyncManager(unittest.TestCase):
         )
 
     def test_emit_with_callback(self):
-        self.bm.connect('123', '/foo')
+        sid = self.bm.connect('123', '/foo')
         self.bm._generate_ack_id = mock.MagicMock()
         self.bm._generate_ack_id.return_value = 11
         _run(
@@ -314,7 +318,7 @@ class TestAsyncManager(unittest.TestCase):
                 'my event', {'foo': 'bar'}, namespace='/foo', callback='cb'
             )
         )
-        self.bm._generate_ack_id.assert_called_once_with('123', '/foo', 'cb')
+        self.bm._generate_ack_id.assert_called_once_with(sid, 'cb')
         self.bm.server._emit_internal.mock.assert_called_once_with(
             '123', 'my event', {'foo': 'bar'}, '/foo', 11
         )
diff --git a/tests/asyncio/test_asyncio_pubsub_manager.py b/tests/asyncio/test_asyncio_pubsub_manager.py
index ca270fb..7ca06b4 100644
--- a/tests/asyncio/test_asyncio_pubsub_manager.py
+++ b/tests/asyncio/test_asyncio_pubsub_manager.py
@@ -154,10 +154,10 @@ class TestAsyncPubSubManager(unittest.TestCase):
                 _run(self.pm.emit('foo', 'bar', callback='cb'))
 
     def test_emit_with_ignore_queue(self):
-        self.pm.connect('123', '/')
+        sid = self.pm.connect('123', '/')
         _run(
             self.pm.emit(
-                'foo', 'bar', room='123', namespace='/', ignore_queue=True
+                'foo', 'bar', room=sid, namespace='/', ignore_queue=True
             )
         )
         self.pm._publish.mock.assert_not_called()
@@ -166,11 +166,11 @@ class TestAsyncPubSubManager(unittest.TestCase):
         )
 
     def test_can_disconnect(self):
-        self.pm.connect('123', '/')
-        assert _run(self.pm.can_disconnect('123', '/')) is True
-        _run(self.pm.can_disconnect('123', '/foo'))
+        sid = self.pm.connect('123', '/')
+        assert _run(self.pm.can_disconnect(sid, '/')) is True
+        _run(self.pm.can_disconnect(sid, '/foo'))
         self.pm._publish.mock.assert_called_once_with(
-            {'method': 'disconnect', 'sid': '123', 'namespace': '/foo'}
+            {'method': 'disconnect', 'sid': sid, 'namespace': '/foo'}
         )
 
     def test_close_room(self):
@@ -310,7 +310,7 @@ class TestAsyncPubSubManager(unittest.TestCase):
                     }
                 )
             )
-            trigger.mock.assert_called_once_with('sid', '/', 123, ('one', 2))
+            trigger.mock.assert_called_once_with('sid', 123, ('one', 2))
 
     def test_handle_callback_bad_host_id(self):
         with mock.patch.object(
diff --git a/tests/common/test_base_manager.py b/tests/common/test_base_manager.py
index 479b80f..0bcba82 100644
--- a/tests/common/test_base_manager.py
+++ b/tests/common/test_base_manager.py
@@ -36,17 +36,18 @@ class TestBaseManager(unittest.TestCase):
         assert self.bm.sid_from_eio_sid('123', '/foo') == sid
 
     def test_pre_disconnect(self):
-        self.bm.connect('123', '/foo')
-        self.bm.connect('456', '/foo')
-        self.bm.pre_disconnect('123', '/foo')
-        assert self.bm.pending_disconnect == {'/foo': ['123']}
-        assert not self.bm.is_connected('123', '/foo')
-        self.bm.pre_disconnect('456', '/foo')
-        assert self.bm.pending_disconnect == {'/foo': ['123', '456']}
-        assert not self.bm.is_connected('456', '/foo')
-        self.bm.disconnect('123', '/foo')
-        assert self.bm.pending_disconnect == {'/foo': ['456']}
-        self.bm.disconnect('456', '/foo')
+        sid1 = self.bm.connect('123', '/foo')
+        sid2 = self.bm.connect('456', '/foo')
+        assert self.bm.is_connected(sid1, '/foo')
+        assert self.bm.pre_disconnect(sid1, '/foo') == '123'
+        assert self.bm.pending_disconnect == {'/foo': [sid1]}
+        assert not self.bm.is_connected(sid1, '/foo')
+        assert self.bm.pre_disconnect(sid2, '/foo') == '456'
+        assert self.bm.pending_disconnect == {'/foo': [sid1, sid2]}
+        assert not self.bm.is_connected(sid2, '/foo')
+        self.bm.disconnect(sid1, '/foo')
+        assert self.bm.pending_disconnect == {'/foo': [sid2]}
+        self.bm.disconnect(sid2, '/foo')
         assert self.bm.pending_disconnect == {}
 
     def test_disconnect(self):
@@ -66,6 +67,8 @@ class TestBaseManager(unittest.TestCase):
         sid4 = self.bm.connect('456', '/foo')
         assert self.bm.is_connected(sid1, '/')
         assert self.bm.is_connected(sid2, '/foo')
+        assert not self.bm.is_connected(sid2, '/')
+        assert not self.bm.is_connected(sid1, '/foo')
         self.bm.disconnect(sid1, '/')
         assert not self.bm.is_connected(sid1, '/')
         assert self.bm.is_connected(sid2, '/foo')
@@ -100,14 +103,17 @@ class TestBaseManager(unittest.TestCase):
         assert self.bm.rooms == {}
 
     def test_disconnect_with_callbacks(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
-        self.bm._generate_ack_id('123', '/', 'f')
-        self.bm._generate_ack_id('123', '/foo', 'g')
-        self.bm.disconnect('123', '/foo')
-        assert '/foo' not in self.bm.callbacks['123']
-        self.bm.disconnect('123', '/')
-        assert '123' not in self.bm.callbacks
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
+        sid3 = self.bm.connect('456', '/foo')
+        self.bm._generate_ack_id(sid1, 'f')
+        self.bm._generate_ack_id(sid2, 'g')
+        self.bm._generate_ack_id(sid3, 'h')
+        self.bm.disconnect(sid2, '/foo')
+        assert sid2 not in self.bm.callbacks
+        self.bm.disconnect(sid1, '/')
+        assert sid1 not in self.bm.callbacks
+        assert sid3 in self.bm.callbacks
 
     def test_disconnect_bad_namespace(self):
         self.bm.connect('123', '/')
@@ -115,26 +121,28 @@ class TestBaseManager(unittest.TestCase):
         self.bm.disconnect('123', '/bar')  # should not assert
 
     def test_trigger_callback(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('123', '/foo')
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('123', '/foo')
         cb = mock.MagicMock()
-        id1 = self.bm._generate_ack_id('123', '/', cb)
-        id2 = self.bm._generate_ack_id('123', '/foo', cb)
-        self.bm.trigger_callback('123', '/', id1, ['foo'])
-        self.bm.trigger_callback('123', '/foo', id2, ['bar', 'baz'])
-        assert cb.call_count == 2
+        id1 = self.bm._generate_ack_id(sid1, cb)
+        id2 = self.bm._generate_ack_id(sid2, cb)
+        id3 = self.bm._generate_ack_id(sid1, cb)
+        self.bm.trigger_callback(sid1, id1, ['foo'])
+        self.bm.trigger_callback(sid1, id3, ['bar'])
+        self.bm.trigger_callback(sid2, id2, ['bar', 'baz'])
+        assert cb.call_count == 3
         cb.assert_any_call('foo')
+        cb.assert_any_call('bar')
         cb.assert_any_call('bar', 'baz')
 
     def test_invalid_callback(self):
-        self.bm.connect('123', '/')
+        sid = self.bm.connect('123', '/')
         cb = mock.MagicMock()
-        id = self.bm._generate_ack_id('123', '/', cb)
+        id = self.bm._generate_ack_id(sid, cb)
 
         # these should not raise an exception
-        self.bm.trigger_callback('124', '/', id, ['foo'])
-        self.bm.trigger_callback('123', '/foo', id, ['foo'])
-        self.bm.trigger_callback('123', '/', id + 1, ['foo'])
+        self.bm.trigger_callback('xxx', id, ['foo'])
+        self.bm.trigger_callback(sid, id + 1, ['foo'])
         assert cb.call_count == 0
 
     def test_get_namespaces(self):
@@ -147,19 +155,21 @@ class TestBaseManager(unittest.TestCase):
         assert '/foo' in namespaces
 
     def test_get_participants(self):
-        self.bm.connect('123', '/')
-        self.bm.connect('456', '/')
-        sid = self.bm.connect('789', '/')
-        self.bm.disconnect(sid, '/')
-        assert sid not in self.bm.rooms['/'][None]
+        sid1 = self.bm.connect('123', '/')
+        sid2 = self.bm.connect('456', '/')
+        sid3 = self.bm.connect('789', '/')
+        self.bm.disconnect(sid3, '/')
+        assert sid3 not in self.bm.rooms['/'][None]
         participants = list(self.bm.get_participants('/', None))
         assert len(participants) == 2
-        assert sid not in participants
+        assert (sid1, '123') in participants
+        assert (sid2, '456') in participants
+        assert (sid3, '789') not in participants
 
     def test_leave_invalid_room(self):
-        self.bm.connect('123', '/foo')
-        self.bm.leave_room('123', '/foo', 'baz')
-        self.bm.leave_room('123', '/bar', 'baz')
+        sid = self.bm.connect('123', '/foo')
+        self.bm.leave_room(sid, '/foo', 'baz')
+        self.bm.leave_room(sid, '/bar', 'baz')
 
     def test_no_room(self):
         rooms = self.bm.get_rooms('123', '/foo')
@@ -270,7 +280,7 @@ class TestBaseManager(unittest.TestCase):
         self.bm.emit(
             'my event', {'foo': 'bar'}, namespace='/foo', callback='cb'
         )
-        self.bm._generate_ack_id.assert_called_once_with(sid, '/foo', 'cb')
+        self.bm._generate_ack_id.assert_called_once_with(sid, 'cb')
         self.bm.server._emit_internal.assert_called_once_with(
             '123', 'my event', {'foo': 'bar'}, '/foo', 11
         )
diff --git a/tests/common/test_pubsub_manager.py b/tests/common/test_pubsub_manager.py
index 7c02b4d..3347e17 100644
--- a/tests/common/test_pubsub_manager.py
+++ b/tests/common/test_pubsub_manager.py
@@ -149,9 +149,9 @@ class TestBaseManager(unittest.TestCase):
                 self.pm.emit('foo', 'bar', callback='cb')
 
     def test_emit_with_ignore_queue(self):
-        self.pm.connect('123', '/')
+        sid = self.pm.connect('123', '/')
         self.pm.emit(
-            'foo', 'bar', room='123', namespace='/', ignore_queue=True
+            'foo', 'bar', room=sid, namespace='/', ignore_queue=True
         )
         self.pm._publish.assert_not_called()
         self.pm.server._emit_internal.assert_called_once_with(
@@ -159,11 +159,11 @@ class TestBaseManager(unittest.TestCase):
         )
 
     def test_can_disconnect(self):
-        self.pm.connect('123', '/')
-        assert self.pm.can_disconnect('123', '/')
-        self.pm.can_disconnect('123', '/foo')
+        sid = self.pm.connect('123', '/')
+        assert self.pm.can_disconnect(sid, '/')
+        self.pm.can_disconnect(sid, '/foo')
         self.pm._publish.assert_called_once_with(
-            {'method': 'disconnect', 'sid': '123', 'namespace': '/foo'}
+            {'method': 'disconnect', 'sid': sid, 'namespace': '/foo'}
         )
 
     def test_close_room(self):
@@ -277,7 +277,7 @@ class TestBaseManager(unittest.TestCase):
                     'args': ('one', 2),
                 }
             )
-            trigger.assert_called_once_with('sid', '/', 123, ('one', 2))
+            trigger.assert_called_once_with('sid', 123, ('one', 2))
 
     def test_handle_callback_bad_host_id(self):
         with mock.patch.object(self.pm, 'trigger_callback') as trigger: