Repository: ably/ably-python
Pull request #440 (2023-02-01 17:02:11)

Realtime.Channels improvements
Some minor improvements to `Realtime.Channels`, see the commit descriptions for more info


Patch:
diff --git a/ably/realtime/realtime.py b/ably/realtime/realtime.py
index a3987ef..d3112f1 100644
--- a/ably/realtime/realtime.py
+++ b/ably/realtime/realtime.py
@@ -4,6 +4,7 @@ from ably.realtime.connection import Connection, ConnectionState
 from ably.rest.auth import Auth
 from ably.rest.rest import AblyRest
 from ably.types.options import Options
+from ably.rest.channel import Channels as RestChannels
 from ably.realtime.realtime_channel import ChannelState, RealtimeChannel
 
 
@@ -154,7 +155,7 @@ class AblyRealtime(AblyRest):
         return self.__channels
 
 
-class Channels:
+class Channels(RestChannels):
     """Creates and destroys RealtimeChannel objects.
 
     Methods
@@ -165,12 +166,8 @@ class Channels:
         Releases a channel
     """
 
-    def __init__(self, realtime):
-        self.all = {}
-        self.__realtime = realtime
-
     # RTS3
-    def get(self, name):
+    def get(self, name) -> RealtimeChannel:
         """Creates a new RealtimeChannel object, or returns the existing channel object.
 
         Parameters
@@ -179,9 +176,11 @@ class Channels:
         name: str
             Channel name
         """
-        if not self.all.get(name):
-            self.all[name] = RealtimeChannel(self.__realtime, name)
-        return self.all[name]
+        if name not in self.__all:
+            channel = self.__all[name] = RealtimeChannel(self.__ably, name)
+        else:
+            channel = self.__all[name]
+        return channel
 
     # RTS4
     def release(self, name):
@@ -196,9 +195,9 @@ class Channels:
         name: str
             Channel name
         """
-        if not self.all.get(name):
+        if name not in self.__all:
             return
-        del self.all[name]
+        del self.__all[name]
 
     def _on_channel_message(self, msg):
         channel_name = msg.get('channel')
@@ -209,7 +208,7 @@ class Channels:
             )
             return
 
-        channel = self.all.get(msg.get('channel'))
+        channel = self.__all[channel_name]
         if not channel:
             log.warning(
                 'Channels.on_channel_message()',
@@ -234,15 +233,14 @@ class Channels:
             ConnectionState.SUSPENDED: ChannelState.SUSPENDED,
         }
 
-        for name in self.all.keys():
-            channel = self.all[name]
+        for channel_name in self.__all:
+            channel = self.__all[channel_name]
             if channel.state in from_channel_states:
                 channel._notify_state(connection_to_channel_state[state], reason)
 
     def _on_connected(self):
-        for channel_name in self.all.keys():
-            channel = self.all[channel_name]
-
+        for channel_name in self.__all:
+            channel = self.__all[channel_name]
             if channel.state == ChannelState.ATTACHING or channel.state == ChannelState.DETACHING:
                 channel._check_pending_state()
             elif channel.state == ChannelState.SUSPENDED:
diff --git a/ably/rest/channel.py b/ably/rest/channel.py
index be2671d..5ea8efd 100644
--- a/ably/rest/channel.py
+++ b/ably/rest/channel.py
@@ -3,6 +3,7 @@ from collections import OrderedDict
 import logging
 import json
 import os
+from typing import Iterator
 from urllib import parse
 import warnings
 
@@ -184,16 +185,16 @@ class Channel(SingleDispatch):
 class Channels:
     def __init__(self, rest):
         self.__ably = rest
-        self.__attached = OrderedDict()
+        self.__all = OrderedDict()
 
     def get(self, name, **kwargs):
         if isinstance(name, bytes):
             name = name.decode('ascii')
 
-        if name not in self.__attached:
-            result = self.__attached[name] = Channel(self.__ably, name, kwargs)
+        if name not in self.__all:
+            result = self.__all[name] = Channel(self.__ably, name, kwargs)
         else:
-            result = self.__attached[name]
+            result = self.__all[name]
             if len(kwargs) != 0:
                 result.options = kwargs
 
@@ -203,10 +204,7 @@ class Channels:
         return self.get(key)
 
     def __getattr__(self, name):
-        try:
-            return super().__getattr__(name)
-        except AttributeError:
-            return self.get(name)
+        return self.get(name)
 
     def __contains__(self, item):
         if isinstance(item, Channel):
@@ -216,13 +214,13 @@ class Channels:
         else:
             name = item
 
-        return name in self.__attached
+        return name in self.__all
 
-    def __iter__(self):
-        return iter(self.__attached.values())
+    def __iter__(self) -> Iterator[str]:
+        return iter(self.__all.values())
 
     def release(self, key):
-        del self.__attached[key]
+        del self.__all[key]
 
     def __delitem__(self, key):
         return self.release(key)


Test patch:
diff --git a/test/ably/realtime/realtimechannel_test.py b/test/ably/realtime/realtimechannel_test.py
index fe5cc7d..c48ea8a 100644
--- a/test/ably/realtime/realtimechannel_test.py
+++ b/test/ably/realtime/realtimechannel_test.py
@@ -1,6 +1,6 @@
 import asyncio
 import pytest
-from ably.realtime.realtime_channel import ChannelState
+from ably.realtime.realtime_channel import ChannelState, RealtimeChannel
 from ably.transport.websockettransport import ProtocolMessageAction
 from ably.types.message import Message
 from test.ably.testapp import TestApp
@@ -17,14 +17,18 @@ class TestRealtimeChannel(BaseAsyncTestCase):
     async def test_channels_get(self):
         ably = await TestApp.get_ably_realtime()
         channel = ably.channels.get('my_channel')
-        assert channel == ably.channels.all['my_channel']
+        assert channel == ably.channels.get('my_channel')
+        assert isinstance(channel, RealtimeChannel)
         await ably.close()
 
     async def test_channels_release(self):
         ably = await TestApp.get_ably_realtime()
         ably.channels.get('my_channel')
         ably.channels.release('my_channel')
-        assert ably.channels.all.get('my_channel') is None
+
+        for _ in ably.channels:
+            raise AssertionError("Expected no channels to exist")
+
         await ably.close()
 
     async def test_channel_attach(self):