From ebc2336da4aebe2377b950af513709ed0a7880d8 Mon Sep 17 00:00:00 2001 From: Simon Woolf Date: Fri, 9 Oct 2026 04:20:35 +0100 Subject: [PATCH] feat: deprecate releasing a realtime channel that isn't detached Specification 6.3.0 replaces RTS4a with RTS4c-e: release() must raise 90011 for a channel that is not INITIALIZED, DETACHED or FAILED, rather than dropping it while it may still be attached. RTS4b lets an SDK keep its existing release until the next major version provided it logs a deprecation warning, so Channels.release now warns when called on a channel in any other state, and still removes it. The channels collection UTS tests are rederived for RTS4c-e, with the RTS4e test gated as an RTS4b-permitted deviation. Co-Authored-By: Claude Opus 5.5 (1M context) --- ably/realtime/channel.py | 14 ++- test/unit/channels_release_test.py | 69 ++++++++++++++ test/uts/deviations.md | 49 +++++----- .../unit/channels/channels_collection_test.py | 90 ++++++++++++------- 4 files changed, 162 insertions(+), 60 deletions(-) create mode 100644 test/unit/channels_release_test.py diff --git a/ably/realtime/channel.py b/ably/realtime/channel.py index f67a5173..283e1888 100644 --- a/ably/realtime/channel.py +++ b/ably/realtime/channel.py @@ -1014,16 +1014,26 @@ def release(self, name: str) -> None: """Releases a RealtimeChannel object, deleting it, and enabling it to be garbage collected It also removes any listeners associated with the channel. - To release a channel, the channel state must be INITIALIZED, DETACHED, or FAILED. - + A realtime channel should only be released when it is in the INITIALIZED, DETACHED, or FAILED + state; releasing a realtime channel in any other state is deprecated and will raise an + AblyException in the next major version. Parameters ---------- name: str Channel name """ + # RTS4c if name not in self.__all: return + channel = self.__all[name] + # RTS4b + if channel.state not in (ChannelState.INITIALIZED, ChannelState.DETACHED, ChannelState.FAILED): + log.warning( + f'Calling channels.release() on a channel in the {channel.state.value} state is deprecated, ' + 'and will raise an exception in the next major version. Await channel.detach() before ' + 'calling channels.release(name).' + ) del self.__all[name] def _on_channel_message(self, msg: dict) -> None: diff --git a/test/unit/channels_release_test.py b/test/unit/channels_release_test.py new file mode 100644 index 00000000..83abc266 --- /dev/null +++ b/test/unit/channels_release_test.py @@ -0,0 +1,69 @@ +import asyncio +import logging + +import pytest + +from ably.realtime.connection import ConnectionState +from ably.transport.websockettransport import ProtocolMessageAction +from ably.types.channelstate import ChannelState +from test.uts.helpers.client import await_connection_state, close_open_clients, realtime_client +from test.uts.helpers.mock_websocket import CONNECTED_MESSAGE, MockWebSocket, attached_message, detached_message + +OPERATION_TIMEOUT = 1.0 + + +@pytest.fixture(autouse=True) +async def close_clients(): + yield + await close_open_clients() + + +async def attached_channel(channel_name): + mock_ws = MockWebSocket( + on_connection_attempt=lambda conn: conn.respond_with_success(CONNECTED_MESSAGE), + ) + + def on_message_from_client(msg): + if msg.get('action') == ProtocolMessageAction.ATTACH: + mock_ws.send_to_client(attached_message(msg['channel'])) + elif msg.get('action') == ProtocolMessageAction.DETACH: + mock_ws.send_to_client(detached_message(msg['channel'])) + + mock_ws.on_message_from_client = on_message_from_client + client = realtime_client(mock_ws) + client.connect() + await await_connection_state(client, ConnectionState.CONNECTED) + channel = client.channels.get(channel_name) + await asyncio.wait_for(channel.attach(), OPERATION_TIMEOUT) + return client, channel + + +def deprecation_warnings(caplog): + return [record for record in caplog.records + if record.levelno == logging.WARNING and 'deprecated' in record.getMessage()] + + +# RTS4b +async def test_releasing_an_attached_channel_logs_a_deprecation_warning(caplog): + client, channel = await attached_channel('attached') + + with caplog.at_level(logging.WARNING, logger='ably'): + client.channels.release('attached') + + warnings = deprecation_warnings(caplog) + assert len(warnings) == 1 + assert 'attached state' in warnings[0].getMessage() + assert 'attached' not in client.channels + + +# RTS4b +async def test_releasing_a_detached_channel_logs_no_deprecation_warning(caplog): + client, channel = await attached_channel('detached') + await asyncio.wait_for(channel.detach(), OPERATION_TIMEOUT) + assert channel.state == ChannelState.DETACHED + + with caplog.at_level(logging.WARNING, logger='ably'): + client.channels.release('detached') + + assert deprecation_warnings(caplog) == [] + assert 'detached' not in client.channels diff --git a/test/uts/deviations.md b/test/uts/deviations.md index 487759bd..4bed6e5a 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -24,20 +24,20 @@ One Test ID can become more than one derived test: five Test IDs in `rest/unit` `error_types_test.py`, `fallback_test.py`, `rest_client_test.py` (two) and `paginated_result_test.py` — assert several independent things under a single id, and the derivation writes a function for each rather than one function with an unrelated -second half. That turns 1132 Test IDs into 1141 derived tests. Going the other way, one +second half. That turns 1133 Test IDs into 1142 derived tests. Going the other way, one derived test can become more than one case: five of the twelve `rest/integration` specifications and five of the twenty `realtime/integration` ones carry a `## Protocol Variants` section and run every one of their tests twice, once per protocol, and nine `rest/unit` tests are parametrized over a table of fixtures the specification gives -inline. That turns 1141 derived tests into 1234 pytest cases. +inline. That turns 1142 derived tests into 1235 pytest cases. -Of **1132 Test IDs, derived as 1141 tests and run as 1234 pytest cases**: 907 Test IDs -(916 tests, 1004 cases) pass, 210 (210 tests, 215 cases) are gated behind +Of **1133 Test IDs, derived as 1142 tests and run as 1235 pytest cases**: 908 Test IDs +(917 tests, 1005 cases) pass, 210 (210 tests, 215 cases) are gated behind `RUN_DEVIATIONS`, and 15 (15 tests, 15 cases) cannot be run at all. The three groups are disjoint: two Test IDs, and one parametrized test, have a gated part and a passing part, and are counted with the gated. Every gated test has been confirmed to fail when enabled, so none of them passes under both behaviours. 494 of the Test IDs come from -`uts/rest/unit` (503 tests, 536 cases), 481 from `uts/realtime/unit` (481, 481), 84 +`uts/rest/unit` (503 tests, 536 cases), 482 from `uts/realtime/unit` (482, 482), 84 from `uts/rest/integration` (84, 122) and 73 from `uts/realtime/integration` (73, 95); 8 of the REST integration ids (8, 8) and 30 of the realtime ones (30, 30) come from the `proxy` package within each. Of the gated Test IDs 119 are REST and 91 realtime, which is @@ -424,7 +424,7 @@ the site. | RTL4b (`channel_attach.md`) | `channelRetryTimeout: 100` ("short timeout for testing"), then `AWAIT_STATE client.connection.state == suspended` | `channelRetryTimeout` governs channel retries, not the connection's suspend timer, which runs for `connectionStateTtl`. The test does not enable fake timers either, so on real time it would wait out two minutes. Derived with a `FakeClock`, passing the named option through unchanged | | RTP5f, RTL11 (`realtime_presence_channel_state.md`) | `simulate_disconnect()` then `AWAIT_STATE channel.state == suspended` | A transport drop reaches DISCONNECTED. RTL3c propagates SUSPENDED to channels only from a SUSPENDED *connection*, so the awaited state never arrives. RTP5f's own note ("e.g. connection transitions to SUSPENDED") says as much; the steps do not carry it out | | RTP5a (`realtime_presence_channel_state.md`) | detach, then `presence.get(waitForSync: false).length == 0` | RTP11e has `get` run the ensure-active-channel procedure for any state but SUSPENDED, so the read-back re-attaches the channel and the specification's own server then repopulates the very map being checked for emptiness. The derived test reads the two maps directly | -| RTS3c1, RTL16a (`channel_options.md`), RTS4a (`channels_collection.md`) | `autoConnect: false`, no mock installed, `connect()` never called, then `AWAIT channel.attach()` and assert ATTACHED | RTL4b requires `attach()` to fail unless the connection is CONNECTING, CONNECTED or DISCONNECTED, and with no mock nothing would answer the ATTACH in any case. Derived with a mock that connects and answers each ATTACH. Upstream should give these three setups a mock, as the sibling sections of the same files do | +| RTS3c1, RTL16a (`channel_options.md`) | `autoConnect: false`, no mock installed, `connect()` never called, then `AWAIT channel.attach()` and assert ATTACHED | RTL4b requires `attach()` to fail unless the connection is CONNECTING, CONNECTED or DISCONNECTED, and with no mock nothing would answer the ATTACH in any case. Derived with a mock that connects and answers each ATTACH. Upstream should give these setups a mock, as the sibling sections of the same file do | | RTS3c1 `error-reattach-modes-1` (`channel_options.md`) | `# Put channel in attaching state (implementation detail)` | The premise the test turns on is the one step it does not give, and the setup has no mock to reach ATTACHING with | ### Fixtures written against mock methods the contract does not define @@ -1234,21 +1234,19 @@ exactly as one arriving while ATTACHED does. **Status:** open bug. -#### `Channels.release` does not detach the channel — 1 test +#### `Channels.release` removes a channel that is not INITIALIZED, DETACHED or FAILED — 1 test -**Spec point:** RTS4a. +**Spec points:** RTS4b, RTS4e. -Release "detaches the channel and then releases the channel resource". `Channels.release` -(`channel.py:1012-1026`) is `if name not in self.__all: return` followed by -`del self.__all[name]`, and sends nothing. An attached channel is dropped from the -collection while still attached in the Ably service, and the orphaned object stays in -ATTACHED. It overrides the REST implementation, which is correct for REST, without adding -the detach. +RTS4e requires release to raise 90011/400 for a channel in any other state, and take no +other action. `Channels.release` (`channel.py`) logs a deprecation warning and removes the +channel, sending nothing, so an attached channel is dropped from the collection while still +attached in the Ably service. RTS4b permits keeping the pre-6.3.0 release until the next +major version provided that warning is logged. -**Tests affected:** `test_rts4a_release_detaches_attached` — `assert 0 == 1` on the -DETACH-message count. +**Tests affected:** `test_rts4e_release_attached_fails` — `DID NOT RAISE`. -**Status:** open bug. +**Status:** permitted by RTS4b until the next major version, which raises 90011. #### A decode error other than 40018 has no channel-level handling — 2 tests, 3 cases @@ -1822,7 +1820,7 @@ through an internal object to get at a value the specification makes public. | RTP2d1, RTP2h1a, and the `Interface Under Test` blocks of all three presence-map specs | `put(message) -> PresenceMessage?` and `remove(message) -> PresenceMessage?` | both return `bool` (`presencemap.py:111`, `:159`); the message to emit is the caller's own, which `set_presence` appends to `broadcast_messages` when the return is true. `IS NOT null` is read as `is True`. `put` stores a *copy* with the action rewritten to PRESENT and leaves the caller's message untouched, so RTP2d1's "emit the original action" falls out for free | all of `presence_map_test.py` | | RTP19, and the `Interface Under Test` block of `presence_sync.md` | `endSync() -> List`, the synthesized LEAVEs | `end_sync()` returns `(residual, absent)` of the *stored* members; the synthesis lives one level up in `RealtimePresence.set_presence` (`presence.py:575-587`). Tests reading only counts and clientIds concatenate the two lists exactly as `set_presence` does; tests reading the LEAVE itself drive a `RealtimePresence` and assert on what its subscribers receive | 4 in `presence_sync_test.py` | | TB2, RTS3b, RTS3c, RTS3c1, RTL16 | `channel.options` as a `ChannelOptions` | a dict keyed by wire names, because `RealtimeChannel` passes `ChannelOptions.to_dict()` to the REST `Channel` constructor (`channel.py:84`). Assertions read `channel.options['params']['rewind']`. On `ChannelOptions` itself the cipher attribute is spelled `cipher`, not `cipherParams`. `set_options_without_reattach` replaces the stored mapping wholesale rather than merging, which `test_rts3c_options_updated_existing` pins | 5 | -| RTS2, RTS4a | `channels.exists(name)`, `channels.names`, and an awaitable `release()` | `name in client.channels` (`Channels.__contains__`); the collection iterates over its channels rather than their names; `release` is synchronous. Genuinely idiomatic spelling rather than an absence — recorded only because of the `__getattr__` hazard noted below | 4 | +| RTS2, RTS4c, RTS4d | `channels.exists(name)` and `channels.names` | `name in client.channels` (`Channels.__contains__`); the collection iterates over its channels rather than their names. Genuinely idiomatic spelling rather than an absence — recorded only because of the `__getattr__` hazard noted below | 5 | | RSH1b1, RSH1b2, RSH1b3, RSH1b4, RSH1b5, RSH1c3 | `DevicePushDetails`. The specification builds every device as `DeviceDetails(…, push: DevicePushDetails(recipient: {…}))`; ably-python has no such type | `DeviceDetails.__init__` takes `push` as a plain dict and stores it unchanged (`ably/types/device.py:10-40`), and `DeviceDetails.push` hands that dict back, so the tests read `{'recipient': {…}}` directly. The recipient's `transportType` is still validated against `DevicePushTransportType` in the constructor, which is the only part of `DevicePushDetails` carrying behaviour | the 7 in `push_admin_test.py` that register a device, through its `apns_device()` helper | **Status:** open bugs of the missing-API kind, not of the wrong-behaviour kind. Adding the @@ -2663,11 +2661,12 @@ disconnect per RTN7e", which issue 1.2 above proves false. No gated test: the UT this cannot distinguish the behaviours (recorded under UTS Spec Errors), so the finding is the output. -**3.9 `Channels.release` does not detach the channel.** RTS4a. `channel.py:1012-1026` deletes -the entry and sends nothing, so the channel is dropped from the collection while still +**3.9 `Channels.release` drops a channel that is still attached.** RTS4e. `Channels.release` +deletes the entry and sends nothing, so the channel is dropped from the collection while still attached in the Ably service — the application goes on being billed for and delivered to a -channel it believes it released. -`test/uts/realtime/unit/channels/channels_collection_test.py -k rts4a_release_detaches_attached` +channel it believes it released. RTS4b permits this until the next major version, with the +deprecation warning it now logs; the next major version raises 90011 instead. Do not file. +`test/uts/realtime/unit/channels/channels_collection_test.py -k rts4e_release_attached_fails` **3.10 The UPDATE event drops the CONNECTED message's error.** RTN24. `on_connected` (`connectionmanager.py:425-428`) builds the `ConnectionStateChange` without the `reason` it @@ -3235,12 +3234,12 @@ gated and how many cannot run. Those numbers are the check that the file is stil true: in pytest cases, the gated count must equal the number of failures under `RUN_DEVIATIONS=1`, and gated plus unrunnable must equal the number of skips without it. As of this writing that is 215 failures and 15 skips with the variable set, and -230 skips and 1134 passes without it, the 1134 being 1004 derived cases and 130 +230 skips and 1135 passes without it, the 1135 being 1005 derived cases and 130 `helpers/` ones. The other two counts are measured from the source rather than from a run. The number of -**derived tests** is the number of `# UTS:` comments, 1141. The number of **Test IDs** is -the number of *distinct* ids in them, 1132 — not the same figure, because five ids in +**derived tests** is the number of `# UTS:` comments, 1142. The number of **Test IDs** is +the number of *distinct* ids in them, 1133 — not the same figure, because five ids in `rest/unit` are carried by more than one test function. Counting the comments and calling the result Test IDs is the easy mistake here, and it overstates the specification coverage by nine. diff --git a/test/uts/realtime/unit/channels/channels_collection_test.py b/test/uts/realtime/unit/channels/channels_collection_test.py index 3498ee12..68a3e9da 100644 --- a/test/uts/realtime/unit/channels/channels_collection_test.py +++ b/test/uts/realtime/unit/channels/channels_collection_test.py @@ -1,6 +1,6 @@ """Derived from uts/realtime/unit/channels/channels_collection.md in ably/specification. -Spec points: RTS1, RTS2, RTS3a, RTS4a +Spec points: RTS1, RTS2, RTS3a, RTS4c, RTS4d, RTS4e The specification's `channels.exists(name)` and `channels.names` are spelled `name in channels` and iteration over the collection in ably-python, and @@ -11,12 +11,15 @@ import asyncio +import pytest + from ably.realtime.channel import Channels as RealtimeChannels from ably.realtime.channel import RealtimeChannel from ably.realtime.connection import ConnectionState from ably.transport.websockettransport import ProtocolMessageAction from ably.types.channelstate import ChannelState -from test.uts.helpers.client import await_connection_state, poll_until, realtime_client +from ably.util.exceptions import AblyException +from test.uts.helpers.client import await_connection_state, realtime_client from test.uts.helpers.deviations import deviation from test.uts.helpers.mock_websocket import ( CONNECTED_MESSAGE, @@ -119,37 +122,35 @@ async def test_rts3a_subscript_operator_channel(): assert channel1.name == channel_name -# UTS: realtime/unit/RTS4a/release-removes-channel-0 -async def test_rts4a_release_removes_channel(): - channel_name = 'test-RTS4a' +# UTS: realtime/unit/RTS4c/release-nonexistent-noop-0 +async def test_rts4c_release_nonexistent_noop(): + channel_name = 'test-RTS4c-nonexistent' client = realtime_client() - client.channels.get(channel_name) - assert (channel_name in client.channels) is True - - # `release` is synchronous here, so there is nothing to await client.channels.release(channel_name) assert (channel_name in client.channels) is False -# UTS: realtime/unit/RTS4a/release-nonexistent-noop-1 -async def test_rts4a_release_nonexistent_noop(): - channel_name = 'test-RTS4a-nonexistent' +# UTS: realtime/unit/RTS4d/release-removes-channel-0 +async def test_rts4d_release_removes_channel(): + channel_name = 'test-RTS4d' client = realtime_client() + channel = client.channels.get(channel_name) + assert channel.state == ChannelState.INITIALIZED + assert (channel_name in client.channels) is True + client.channels.release(channel_name) assert (channel_name in client.channels) is False -# UTS: realtime/unit/RTS4a/release-detaches-attached-2 -@deviation -async def test_rts4a_release_detaches_attached(): - # DEVIATION: RTS4a has release detach the channel before dropping it. - # `Channels.release` (`ably/realtime/channel.py:1012`) only deletes the entry from - # its dict, so no DETACH is sent and the channel is left attached in the Ably service. - channel_name = 'test-RTS4a-attached' +async def connected_client(answer_detach): + """A connected client whose mock answers each ATTACH, and each DETACH if `answer_detach`. + + Returns the client and the list of messages it sends. + """ messages_from_client = [] mock_ws = MockWebSocket( @@ -159,33 +160,56 @@ async def test_rts4a_release_detaches_attached(): def on_message_from_client(msg): messages_from_client.append(msg) if msg.get('action') == ProtocolMessageAction.ATTACH: - mock_ws.send_to_client(attached_message(channel_name)) - elif msg.get('action') == ProtocolMessageAction.DETACH: - mock_ws.send_to_client(detached_message(channel_name)) + mock_ws.send_to_client(attached_message(msg['channel'])) + elif msg.get('action') == ProtocolMessageAction.DETACH and answer_detach: + mock_ws.send_to_client(detached_message(msg['channel'])) mock_ws.on_message_from_client = on_message_from_client client = realtime_client(mock_ws) client.connect() await await_connection_state(client, ConnectionState.CONNECTED) + return client, messages_from_client + +# UTS: realtime/unit/RTS4d/release-after-detach-1 +async def test_rts4d_release_after_detach(): + channel_name = 'test-RTS4d-detached' + client, _ = await connected_client(answer_detach=True) channel = client.channels.get(channel_name) - await asyncio.wait_for(channel.attach(), OPERATION_TIMEOUT) - assert channel.state == ChannelState.ATTACHED - state_before_release = channel.state + await asyncio.wait_for(channel.attach(), OPERATION_TIMEOUT) + await asyncio.wait_for(channel.detach(), OPERATION_TIMEOUT) + assert channel.state == ChannelState.DETACHED client.channels.release(channel_name) - # A release which detaches sends DETACH on a task, and the channel reaches - # DETACHED only once the mock's answer has been handled, which is several - # yields away rather than one - await poll_until(lambda: channel.state == ChannelState.DETACHED, OPERATION_TIMEOUT, - 'the released channel to detach') - assert state_before_release == ChannelState.ATTACHED assert (channel_name in client.channels) is False + + +# UTS: realtime/unit/RTS4e/release-attached-fails-0 +@deviation +async def test_rts4e_release_attached_fails(): + # DEVIATION: RTS4e has release raise 90011 for a channel that is not INITIALIZED, + # DETACHED or FAILED. RTS4b lets an SDK keep its pre-6.3.0 release until its next + # major version, provided it logs a deprecation warning, so `Channels.release` + # (`ably/realtime/channel.py`) warns and removes the channel. + channel_name = 'test-RTS4e-attached' + client, messages_from_client = await connected_client(answer_detach=False) + channel = client.channels.get(channel_name) + + await asyncio.wait_for(channel.attach(), OPERATION_TIMEOUT) + assert channel.state == ChannelState.ATTACHED + + with pytest.raises(AblyException) as exc_info: + client.channels.release(channel_name) + + assert exc_info.value.code == 90011 + assert exc_info.value.status_code == 400 + assert channel.state == ChannelState.ATTACHED + assert (channel_name in client.channels) is True + assert client.channels.get(channel_name) is channel detach_messages = [m for m in messages_from_client if m.get('action') == ProtocolMessageAction.DETACH] - assert len(detach_messages) == 1 - assert channel.state == ChannelState.DETACHED + assert len(detach_messages) == 0 # UTS: realtime/unit/RTS3a/get-after-release-new-3