From 2e5c6d4291b337742e1de71bfdcf964532444b1f Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 20:38:46 +0000 Subject: [PATCH 1/7] pubsub: make PresenceMessage.timestamp milliseconds since the epoch PresenceMessage.timestamp was decoded into a naive datetime through the deprecated datetime.utcfromtimestamp, while the LEAVE messages the client synthesizes (RTP19, RTP19a) were stamped with an aware datetime.now(timezone.utc), so the two could not be compared, and the synthesized ones bypassed the client's clock. TP3g gives the timestamp as milliseconds since the epoch, which is what Message.timestamp already is. The timestamp is now the int from the wire, sent back unchanged by to_encoded, and a synthesized LEAVE takes its timestamp from the clock's now_ms(). The RTP2b1 newness check compares timestamps numerically, and the realtime channel still fills a missing timestamp from the ProtocolMessage's before decoding (TP3g). Presence history's start and end now also accept an aware datetime, as channel history does, where before an aware one raised TypeError; a naive one is still read as UTC. This is a breaking change, and a silent one: comparing a timestamp with == against a datetime now returns False. Callers that need a datetime convert with datetime.fromtimestamp(timestamp / 1000, tz=timezone.utc). The UTS tests drop their datetime workarounds and assert the specification's millisecond values, which closes the timezone-split deviation. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 21 +++++++++ ably/pubsub/realtime/presence.py | 7 +-- ably/pubsub/types/presence.py | 35 +++++++-------- test/ably/http/httppresence_test.py | 4 +- test/ably/realtime/presencemap_test.py | 15 +++---- test/uts/deviations.md | 44 ++++--------------- test/uts/helpers/presence.py | 7 ++- .../unit/presence/presence_sync_test.py | 12 ++--- .../rest/unit/presence/rest_presence_test.py | 9 ++-- .../unit/types/presence_message_types_test.py | 10 +---- 10 files changed, 73 insertions(+), 91 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 3e9085fd..19b0245d 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -553,6 +553,27 @@ Example 4.0.0 code: pubsub_realtime_client = create_realtime_client(key='xxx') ``` +### `PresenceMessage.timestamp` is milliseconds since the epoch + +**This change is silent: comparing a timestamp with `==` against a `datetime` now returns +`False` rather than raising.** `PresenceMessage.timestamp` is an `int` of milliseconds since +the unix epoch, as `Message.timestamp` already is. 3.x returned a naive UTC `datetime` for a +presence message received from Ably, and an aware one for a LEAVE the library synthesized. +An ordering comparison such as `<` against a `datetime` raises `TypeError`. Convert the +timestamp where you need a `datetime`: + +Example 3.x code: +```python +received_at = member.timestamp +``` + +Example 4.0.0 code: +```python +from datetime import datetime, timezone + +received_at = datetime.fromtimestamp(member.timestamp / 1000, tz=timezone.utc) +``` + ## Version 2.x to 3.0.0 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include: diff --git a/ably/pubsub/realtime/presence.py b/ably/pubsub/realtime/presence.py index 70e21774..2c87f50e 100644 --- a/ably/pubsub/realtime/presence.py +++ b/ably/pubsub/realtime/presence.py @@ -9,13 +9,13 @@ import asyncio import logging -from datetime import datetime, timezone from typing import TYPE_CHECKING, Any from ably.pubsub.realtime.connection import ConnectionState from ably.pubsub.realtime.presencemap import PresenceMap from ably.pubsub.types.channelstate import ChannelState, ChannelStateChange from ably.pubsub.types.presence import PresenceAction, PresenceMessage +from ably.pubsub.util.clock import select_clock from ably.pubsub.util.eventemitter import EventEmitter from ably.pubsub.util.exceptions import AblyException @@ -69,6 +69,7 @@ def __init__(self, channel: RealtimeChannel): super().__init__() self.channel = channel self.sync_complete = False + self._clock = select_clock(channel.ably.options) # RTP2: Main presence map keyed by memberKey (connectionId:clientId) self.members = PresenceMap( @@ -583,7 +584,7 @@ def set_presence( connection_id=member.connection_id, data=member.data, encoding=member.encoding, - timestamp=datetime.now(timezone.utc) + timestamp=self._clock.now_ms() ) broadcast_messages.append(synthesized_leave) @@ -717,7 +718,7 @@ def _synthesize_leaves(self, members: list[PresenceMessage]) -> None: connection_id=member.connection_id, data=member.data, encoding=member.encoding, - timestamp=datetime.now(timezone.utc) + timestamp=self._clock.now_ms() ) self._subscriptions._emit('leave', synthesized_leave) diff --git a/ably/pubsub/types/presence.py b/ably/pubsub/types/presence.py index 66cbdea7..0043f459 100644 --- a/ably/pubsub/types/presence.py +++ b/ably/pubsub/types/presence.py @@ -1,4 +1,4 @@ -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone from urllib import parse from ably.pubsub.request.paginatedresult import PaginatedResult @@ -7,16 +7,20 @@ from ably.pubsub.util.crypto import CipherData from ably.pubsub.util.encoding import encode_data +_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) -def _ms_since_epoch(dt): - epoch = datetime.utcfromtimestamp(0) - delta = dt - epoch - return int(delta.total_seconds() * 1000) +def _ms_since_epoch(t): + """A history bound as milliseconds since the epoch, as channel history accepts one. -def _dt_from_ms_epoch(ms): - epoch = datetime.utcfromtimestamp(0) - return epoch + timedelta(milliseconds=ms) + An int is already milliseconds. A datetime may be aware or naive, and a naive one is + read as UTC. + """ + if isinstance(t, int): + return t + if t.tzinfo is None: + t = t.replace(tzinfo=timezone.utc) + return (t - _EPOCH) // timedelta(milliseconds=1) class PresenceAction: @@ -164,7 +168,7 @@ def to_encoded(self, binary=False): if self.extras: result['extras'] = self.extras if self.timestamp: - result['timestamp'] = _ms_since_epoch(self.timestamp) + result['timestamp'] = self.timestamp return result @@ -180,9 +184,6 @@ def from_encoded(obj, cipher=None, context=None): # member_key = obj.get('memberKey', None) extras = obj.get('extras', None) - if timestamp is not None: - timestamp = _dt_from_ms_epoch(timestamp) - decoded_data = PresenceMessage.decode(data, encoding, cipher) return PresenceMessage( @@ -237,15 +238,9 @@ async def history(self, limit=None, direction=None, start=None, end=None): if direction: qs['direction'] = direction if start: - if isinstance(start, int): - qs['start'] = start - else: - qs['start'] = _ms_since_epoch(start) + qs['start'] = _ms_since_epoch(start) if end: - if isinstance(end, int): - qs['end'] = end - else: - qs['end'] = _ms_since_epoch(end) + qs['end'] = _ms_since_epoch(end) if 'start' in qs and 'end' in qs and qs['start'] > qs['end']: raise ValueError("'end' parameter has to be greater than or equal to 'start'") diff --git a/test/ably/http/httppresence_test.py b/test/ably/http/httppresence_test.py index 1fa52d42..187c3ab4 100644 --- a/test/ably/http/httppresence_test.py +++ b/test/ably/http/httppresence_test.py @@ -60,10 +60,10 @@ async def test_presence_get_encoded(self): assert presence_history.items[-4].data == '{ "test": "This is a JSONObject clientData payload"}' assert presence_history.items[-5].data == {"example": {"json": "Object"}} - async def test_timestamp_is_datetime(self): + async def test_timestamp_is_ms_since_epoch(self): presence_page = await self.channel.presence.get() member = presence_page.items[0] - assert isinstance(member.timestamp, datetime) + assert isinstance(member.timestamp, int) async def test_presence_message_has_correct_member_key(self): presence_page = await self.channel.presence.get() diff --git a/test/ably/realtime/presencemap_test.py b/test/ably/realtime/presencemap_test.py index de6002bf..53600295 100644 --- a/test/ably/realtime/presencemap_test.py +++ b/test/ably/realtime/presencemap_test.py @@ -4,7 +4,6 @@ Tests RTP2 specification requirements for presence map operations. """ -from datetime import datetime import pytest @@ -117,7 +116,7 @@ def test_to_encoded(self): client_id='client1', action=PresenceAction.ENTER, data='test data', - timestamp=datetime(2024, 1, 1, 12, 0, 0) + timestamp=1704110400000 ) encoded = msg.to_encoded() assert encoded['action'] == PresenceAction.ENTER @@ -125,7 +124,7 @@ def test_to_encoded(self): assert encoded['connectionId'] == 'connection123' assert encoded['clientId'] == 'client1' assert encoded['data'] == 'test data' - assert 'timestamp' in encoded + assert encoded['timestamp'] == 1704110400000 def test_to_encoded_with_dict_data(self): """Test converting message with dict data (should be JSON serialized).""" @@ -246,21 +245,21 @@ def test_synthesized_message_newer_by_timestamp(self): connection_id='connection123', client_id='client1', action=PresenceAction.LEAVE, - timestamp=datetime(2024, 1, 1, 12, 0, 0) + timestamp=1704110400000 ) newer = PresenceMessage( id='connection123:5:0', connection_id='connection123', client_id='client1', action=PresenceAction.PRESENT, - timestamp=datetime(2024, 1, 1, 12, 0, 1) + timestamp=1704110401000 ) assert _is_newer(newer, older) assert not _is_newer(older, newer) def test_synthesized_equal_timestamp_incoming_wins(self): """Test RTP2b1a: equal timestamps, incoming is newer.""" - timestamp = datetime(2024, 1, 1, 12, 0, 0) + timestamp = 1704110400000 existing = PresenceMessage( id='different:0:0', connection_id='connection123', @@ -285,14 +284,14 @@ def test_normal_message_newer_by_msg_serial(self): connection_id='connection123', client_id='client1', action=PresenceAction.PRESENT, - timestamp=datetime(2024, 1, 1, 12, 0, 0) + timestamp=1704110400000 ) newer = PresenceMessage( id='connection123:10:0', connection_id='connection123', client_id='client1', action=PresenceAction.PRESENT, - timestamp=datetime(2024, 1, 1, 11, 0, 0) # Earlier timestamp doesn't matter + timestamp=1704106800000 # Earlier timestamp doesn't matter ) assert _is_newer(newer, older) assert not _is_newer(older, newer) diff --git a/test/uts/deviations.md b/test/uts/deviations.md index b42aa6b0..c5143bbd 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -52,13 +52,13 @@ specification: 130 under `helpers/`, for the mocks, and 18 under `objects/helper the objects builders and the synced-channel mock. The 142 gated Test IDs that record SDK non-compliance — 142 tests, 144 cases — reduce to -**60 distinct root causes**, 20 on the REST side and 40 on the realtime side. Two further -defects are recorded below with no test of their own, because the specification's test -cannot discriminate (RTP18a) or has nothing to assert against (the timezone split on -synthesized LEAVE timestamps), so the file carries **62 SDK root causes** in all. The remaining 11 gated Test IDs are -specification faults, and reduce to 8. The objects tier records no SDK non-compliance; -its specification faults are all corrected or worked around in tests that pass, and -reduce to the seven candidate issues O.1–O.7, none of them filed. +**60 distinct root causes**, 20 on the REST side and 40 on the realtime side. One further +defect is recorded below with no test of its own, because the specification's test +cannot discriminate (RTP18a), so the file carries **61 SDK root causes** in all. The +remaining 11 gated Test IDs are specification faults, and reduce to 8. The objects tier +records no SDK non-compliance; its specification faults are all corrected or worked +around in tests that pass, and reduce to the seven candidate issues O.1–O.7, none of +them filed. Entries closed by a fix are removed rather than kept as history; `git log` holds that. @@ -1677,26 +1677,6 @@ correcting the test does not read as a new discovery. **Status:** open bug, untested. -#### A synthesized LEAVE's timestamp is timezone-aware; every other one is naive — no test - -**Spec points:** RTP19, RTP19a, against TP3g. - -`_synthesize_leaves` and `set_presence` build the LEAVE with `datetime.now(timezone.utc)` -(`presence.py:586`, `:720`), while every wire-derived presence message gets a **naive** -`datetime` from `_dt_from_ms_epoch`, which is built on `datetime.utcfromtimestamp(0)` -(`types/presence.py:12-19`, `:182-184`). Comparing the two raises -`TypeError: can't compare offset-naive and offset-aware datetimes`, verified directly. - -Nothing inside the library compares them, because synthesized leaves are emitted rather than -stored, so this bites only application code — but any subscriber that sorts or compares the -timestamps of the messages it receives will hit it. Related to the deprecation work in -PR #656. - -**Tests affected:** none. `test_rtp19_synth_leave_null_id_timestamp` brackets the LEAVE with -two aware `datetime.now(timezone.utc)` readings and passes. - -**Status:** open bug, untested. - ### Auth #### A failed RTN22 reauth leaves no trace on the connection — 1 test @@ -2599,7 +2579,7 @@ is missing accessors over correct behaviour. Within a tier the order is blast ra |---|---|---| | [#706](https://github.com/ably/ably-python/issues/706) | the fabricated `"None:0"` message id | 3.3 | | [#658](https://github.com/ably/ably-python/issues/658) | presence messages sent on reconnection before reattach | adjacent to 3.13, which is the other half of RTP17 automatic re-entry | -| [#656](https://github.com/ably/ably-python/issues/656) | `utcfromtimestamp` deprecation | adjacent to 2.4 | +| [#656](https://github.com/ably/ably-python/issues/656) | `utcfromtimestamp` deprecation | none: `PresenceMessage.timestamp` is now milliseconds since the epoch, the synthesized LEAVE's included, and no `utcfromtimestamp` remains | | [#709](https://github.com/ably/ably-python/issues/709)–[#712](https://github.com/ably/ably-python/issues/712) | REST request timeout, token-request nonce reuse, single-host retry, `dispose()` teardown | adjacent to I.3, which is a second defect on the line #709 is about and is not covered by it; otherwise none, these having been filed from the REST derivation | Every reproduction below is prefixed by: @@ -2705,14 +2685,6 @@ hasattr(client.channels, 'foo') # True len(client.channels) # 1 ``` -**2.4 Synthesized LEAVE timestamps are timezone-aware; every other one is naive.** RTP19, -RTP19a, TP3g. `presence.py:586` and `:720` use `datetime.now(timezone.utc)`, while every -wire-derived `PresenceMessage.timestamp` comes from `_dt_from_ms_epoch`, built on -`datetime.utcfromtimestamp(0)` (`types/presence.py:12-19`). Comparing them raises -`TypeError: can't compare offset-naive and offset-aware datetimes`. Nothing inside the -library compares them, so it bites only application code — but any subscriber that sorts the -timestamps of the messages it receives will hit it. Related to #656. No gated test. - **2.5 `Channels._on_channel_message` subscripts before it checks.** `channel.py:1038-1045` does `channel = self.__all[channel_name]` and only then `if not channel:`, so a message for an unknown channel raises `KeyError` inside the protocol task — swallowed and logged — and diff --git a/test/uts/helpers/presence.py b/test/uts/helpers/presence.py index cec057a9..5218e902 100644 --- a/test/uts/helpers/presence.py +++ b/test/uts/helpers/presence.py @@ -12,6 +12,7 @@ from ably.pubsub.realtime.presencemap import PresenceMap from ably.pubsub.transport.websockettransport import ProtocolMessageAction from ably.pubsub.types.presence import PresenceAction, PresenceMessage +from ably.pubsub.types.testoptions import TestOptions #: The lowercase action names `RealtimePresence` emits its events under, which come #: from `PresenceAction._action_name` and exist only once `presence.py` is imported. @@ -35,16 +36,18 @@ def presence_message(action, client_id, connection_id, id, timestamp, data=None) ) -def subscribed_presence(connection_id='conn-1', name='presence-test'): +def subscribed_presence(connection_id='conn-1', name='presence-test', clock=None): """A `RealtimePresence` over a stub channel, with every presence event recorded. Returns the presence object and the list of `(event_name, message)` pairs its subscribers receive. One listener is registered per event name because - `EventEmitter` keys its wrappers on the listener alone. + `EventEmitter` keys its wrappers on the listener alone. `clock` is the time the + presence object reads when it synthesizes a LEAVE. """ channel = SimpleNamespace( name=name, ably=SimpleNamespace( + options=SimpleNamespace(_test_options=TestOptions(clock=clock)), connection=SimpleNamespace( connection_manager=SimpleNamespace(connection_id=connection_id), ), diff --git a/test/uts/realtime/unit/presence/presence_sync_test.py b/test/uts/realtime/unit/presence/presence_sync_test.py index 0d228862..3f75205d 100644 --- a/test/uts/realtime/unit/presence/presence_sync_test.py +++ b/test/uts/realtime/unit/presence/presence_sync_test.py @@ -10,9 +10,8 @@ messages. See test/uts/deviations.md. """ -from datetime import datetime, timezone - from ably.pubsub.types.presence import PresenceAction +from test.uts.helpers.clock import FakeClock from test.uts.helpers.deviations import deviation from test.uts.helpers.presence import presence_map, presence_message, subscribed_presence @@ -84,19 +83,22 @@ def test_rtp19_stale_members_leave_after_sync(): # UTS: realtime/unit/RTP19/synth-leave-null-id-timestamp-1 def test_rtp19_synth_leave_null_id_timestamp(): - presence, events = subscribed_presence() + # A fake clock holds still between the two readings, so the bounds pin the value the + # LEAVE must take. + clock = FakeClock() + presence, events = subscribed_presence(clock=clock) presence.set_presence([ presence_message(PresenceAction.ENTER, 'bob', 'c2', 'c2:0:0', 100, data='bob-data'), ], is_sync=False) - before_time = datetime.now(timezone.utc) + before_time = clock.now_ms() events.clear() # A sync carrying no members for bob presence.set_presence([], is_sync=True) - after_time = datetime.now(timezone.utc) + after_time = clock.now_ms() leave_events = leaves(events) assert len(leave_events) == 1 diff --git a/test/uts/rest/unit/presence/rest_presence_test.py b/test/uts/rest/unit/presence/rest_presence_test.py index 584949c8..fa97703c 100644 --- a/test/uts/rest/unit/presence/rest_presence_test.py +++ b/test/uts/rest/unit/presence/rest_presence_test.py @@ -6,7 +6,7 @@ import base64 import uuid -from datetime import datetime +from datetime import datetime, timezone import msgpack import pytest @@ -126,9 +126,7 @@ async def test_rsp3b_get_returns_presence_messages(): assert message.client_id == 'user123' assert message.connection_id == 'conn456' assert message.data == 'status data' - # NOTE: the spec asserts the raw millisecond value. PresenceMessage.timestamp is a - # datetime, which is the "Date/Time object where appropriate to the language" form. - assert message.timestamp == datetime.utcfromtimestamp(1234567890000 / 1000) + assert message.timestamp == 1234567890000 # UTS: rest/unit/RSP3c/get-empty-members-0 @@ -319,8 +317,7 @@ async def test_rsp4b1_history_datetime_objects(): captured_requests = [] mock_http = capturing_mock(captured_requests, []) client = rest_client(mock_http) - # ably-python treats a naive datetime as UTC when converting to epoch milliseconds - start_datetime = datetime(2021, 1, 1, 0, 0, 0) + start_datetime = datetime(2021, 1, 1, 0, 0, 0, tzinfo=timezone.utc) await client.channels.get(channel_name).presence.history(start=start_datetime) diff --git a/test/uts/rest/unit/types/presence_message_types_test.py b/test/uts/rest/unit/types/presence_message_types_test.py index 7c6b2b7c..34375ec7 100644 --- a/test/uts/rest/unit/types/presence_message_types_test.py +++ b/test/uts/rest/unit/types/presence_message_types_test.py @@ -3,17 +3,10 @@ Spec points: TP1, TP2, TP3, TP3a, TP3b, TP3c, TP3d, TP3e, TP3f, TP3g, TP3h, TP3i, TP4, TP5 """ -from datetime import datetime, timedelta - from ably.pubsub.types.presence import PresenceAction, PresenceMessage from test.uts.helpers.deviations import deviation, spec_error -def datetime_from_ms(ms): - """The datetime ably-python decodes a wire timestamp into.""" - return datetime.utcfromtimestamp(0) + timedelta(milliseconds=ms) - - def decode_protocol_message_presence(protocol_message): """The `presence` array of a ProtocolMessage, decoded the way the client decodes one. @@ -158,8 +151,7 @@ def test_tp3_presence_from_json(): assert msg.client_id == 'user-1' assert msg.connection_id == 'conn-1' assert msg.data == 'hello' - # NOTE: features.md types TP3g as `Time`, so a datetime is the idiomatic rendering. - assert msg.timestamp == datetime_from_ms(1234567890000) + assert msg.timestamp == 1234567890000 assert msg.extras['headers']['x-key'] == 'x-value' From d8c4fff714cde215de5710b55f785709125b71fd Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 20:43:23 +0000 Subject: [PATCH 2/7] pubsub: decode the TP3 ProtocolMessage tests the way the channel does The TP3a, TP3d and TP3g UTS tests were gated as unimplemented, on the reading that the realtime channel decodes a presence array with no context from its ProtocolMessage. It does not: RealtimeChannel._on_message calls Message.update_inner_message_fields first, which fills each presence entry's missing id, connectionId and timestamp from the ProtocolMessage. The tests' decoding helper skipped that call. The helper now makes it, so the three tests pass and are un-gated. TP3g's asserts the raw millisecond value, which it can only do now that PresenceMessage.timestamp is milliseconds. No SDK code changes, and nothing breaks. deviations.md drops the row, records the refuted claim under Investigated and not defects, and recounts. Co-Authored-By: Claude Opus 5.5 --- test/uts/deviations.md | 26 ++++++++++++------- .../unit/types/presence_message_types_test.py | 11 ++++---- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/test/uts/deviations.md b/test/uts/deviations.md index c5143bbd..006052d9 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -33,8 +33,8 @@ parametrized over a table of fixtures the specification gives inline; and five `objects/unit` tests are parametrized, three over a table and two over the scenarios the specification loops through. That turns 1481 derived tests into 1608 pytest cases. -Of **1472 Test IDs, derived as 1481 tests and run as 1608 pytest cases**: 1292 Test IDs -(1301 tests, 1426 cases) pass, 153 (153 tests, 155 cases) are gated behind +Of **1472 Test IDs, derived as 1481 tests and run as 1608 pytest cases**: 1295 Test IDs +(1304 tests, 1429 cases) pass, 150 (150 tests, 152 cases) are gated behind `RUN_DEVIATIONS`, and 27 (27 tests, 27 cases) cannot be run at all. The three groups are disjoint: two Test IDs have a gated part and a passing part — `TI1`, as two functions, and `TO/endpoint-affects-host`, as one parametrized test — and are counted with the gated, @@ -45,16 +45,16 @@ the Test IDs come from `uts/rest/unit` (503 tests, 536 cases), 482 from `uts/rest/integration` (84, 122), 73 from `uts/realtime/integration` (73, 95) and 17 from `uts/objects/integration` (17, 29); 8 of the REST integration ids (8, 8), 30 of the realtime ones (30, 30) and 5 of the objects ones (5, 5) come from the `proxy` package -within each. Of the gated Test IDs 69 are REST and 84 realtime, which is 70 REST cases +within each. Of the gated Test IDs 66 are REST and 84 realtime, which is 67 REST cases and 85 realtime. None of the 339 objects Test IDs is gated or unrunnable. A further 148 pytest cases cover the harness itself and are not derived from a specification: 130 under `helpers/`, for the mocks, and 18 under `objects/helpers/`, for the objects builders and the synced-channel mock. -The 142 gated Test IDs that record SDK non-compliance — 142 tests, 144 cases — reduce to -**60 distinct root causes**, 20 on the REST side and 40 on the realtime side. One further +The 139 gated Test IDs that record SDK non-compliance — 139 tests, 141 cases — reduce to +**59 distinct root causes**, 19 on the REST side and 40 on the realtime side. One further defect is recorded below with no test of its own, because the specification's test -cannot discriminate (RTP18a), so the file carries **61 SDK root causes** in all. The +cannot discriminate (RTP18a), so the file carries **60 SDK root causes** in all. The remaining 11 gated Test IDs are specification faults, and reduce to 8. The objects tier records no SDK non-compliance; its specification faults are all corrected or worked around in tests that pass, and reduce to the seven candidate issues O.1–O.7, none of @@ -1053,7 +1053,6 @@ option is gone, after RTN16d has done its first phase against the sandbox throug | RTL25, RTL25a, RTL25b | `RealtimeChannel#whenState`. `Connection._when_state` exists (private, awaitable), so this is a gap on the channel rather than a house style; the tests are written against a `channel.when_state(state)` matching the shape the connection already has | 4 | | RSC2, RSC3, RSC4, TO3b, TO3c, TO3c2 | `log_level` and `log_handler` as client options. The library logs through the standard `logging` module under the `ably.pubsub` logger, whose level and handlers are the idiomatic way to do both; `log_level`, which was stored on `Options` and read by nothing, was removed in 4.0.0, so passing either raises `TypeError` | 4 | | RSP3a2, RSP3a3 | `clientId` and `connectionId` filters on `RestPresence#get`. `Presence.get` is `get(self, limit=None)` (`ably/types/presence.py:216`), while `Presence.history` does take its documented params. `presence.get(client_id=...)` raises `TypeError: get() got an unexpected keyword argument 'client_id'`. The absence also forces the RSP5 decoding adaptation below | 4 (5 cases) | -| TP3a, TP3d, TP3g | Presence attributes defaulted from the encapsulating ProtocolMessage. There is no ProtocolMessage type; `ably/realtime/channel.py:751-761` passes the presence array through without context. Matters for synthesized-leave detection and `memberKey` | 3 | | TB4, RTL7h, RTP6e | `attachOnSubscribe`. `ChannelOptions.__init__` (`channeloptions.py:22-26`) takes only `cipher`, `params` and `modes`, and `subscribe()` on the channel, on presence and on annotations all end unconditionally with `await attach()`. This absence also forces the largest single adaptation in the suite, below | 3 | | RSL7 | `RestChannel#setOptions`. The realtime channel implements it; the REST `options` setter expects the kwargs dict `Channels.get` collected, so a `ChannelOptions` raises `TypeError` | 2 | | RTL7f | `echoMessages` in its client-filter form. The `echo_messages` option sends `echo=false` (RTC1a), which the server honours, but the client does not itself drop a message carrying its own `connectionId`, and that is the only form a mock-backed test can observe | 1 | @@ -2392,6 +2391,15 @@ method, or at the calling level". ably-python uses the calling level (`presencemap.py:140-142`), which is what RTP19 needs. The three RTP19 cases the specification distinguishes all come out right. +### A presence message does take its missing fields from the ProtocolMessage + +TP3a, TP3d and TP3g were gated as unimplemented, on the reading that the realtime channel +hands the `presence` array to `from_encoded_array` with no context. It does not: +`RealtimeChannel._on_message` first calls `Message.update_inner_message_fields`, which +fills each presence entry's missing `id`, `connectionId` and `timestamp` from the +ProtocolMessage, and `test/unit/message_test.py` covers it. The derived tests' decoding +helper skipped that call; it now makes it, and the three tests pass. + ### RTL18 delta recovery matches the specification exactly Verified end to end: the recovery ATTACH carries the `channelSerial` of the last batch that @@ -3513,8 +3521,8 @@ The header states how many derived tests there are, how many pass, how many are gated and how many cannot run. Those numbers are the check that the file is still 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 155 failures, 1574 passes and 27 skips with the variable -set, and 182 skips and 1574 passes without it, the 1574 being 1426 derived cases and 148 +it. As of this writing that is 152 failures, 1577 passes and 27 skips with the variable +set, and 179 skips and 1577 passes without it, the 1577 being 1429 derived cases and 148 harness ones — 130 under `helpers/` and 18 under `objects/helpers/`. Those figures need the `submodules` checkout CI makes: without it, three `rest/unit/encoding` tests that read the ably-common fixtures skip as well. The tiers are told apart by the junit report's diff --git a/test/uts/rest/unit/types/presence_message_types_test.py b/test/uts/rest/unit/types/presence_message_types_test.py index 34375ec7..12cd7277 100644 --- a/test/uts/rest/unit/types/presence_message_types_test.py +++ b/test/uts/rest/unit/types/presence_message_types_test.py @@ -3,6 +3,7 @@ Spec points: TP1, TP2, TP3, TP3a, TP3b, TP3c, TP3d, TP3e, TP3f, TP3g, TP3h, TP3i, TP4, TP5 """ +from ably.pubsub.types.message import Message from ably.pubsub.types.presence import PresenceAction, PresenceMessage from test.uts.helpers.deviations import deviation, spec_error @@ -10,9 +11,12 @@ def decode_protocol_message_presence(protocol_message): """The `presence` array of a ProtocolMessage, decoded the way the client decodes one. - ably-python has no ProtocolMessage type; `RealtimeChannel._on_message` hands the raw - `presence` array of an incoming PRESENCE or SYNC message to `from_encoded_array`. + ably-python has no ProtocolMessage type; `RealtimeChannel._on_message` fills each + entry's missing `id`, `connectionId` and `timestamp` from the raw ProtocolMessage + through `Message.update_inner_message_fields`, then hands the `presence` array of an + incoming PRESENCE or SYNC message to `from_encoded_array`. """ + Message.update_inner_message_fields(protocol_message) return PresenceMessage.from_encoded_array(protocol_message['presence']) @@ -77,7 +81,6 @@ def test_tp3h_member_key_combines_ids(): # UTS: rest/unit/TP3d/connectionid-from-protocol-message-0 -@deviation def test_tp3d_connectionid_from_protocol_message(): protocol_msg = { 'action': 14, # PRESENCE @@ -94,7 +97,6 @@ def test_tp3d_connectionid_from_protocol_message(): # UTS: rest/unit/TP3a/id-from-protocol-message-1 -@deviation def test_tp3a_id_from_protocol_message(): protocol_msg = { 'action': 14, # PRESENCE @@ -113,7 +115,6 @@ def test_tp3a_id_from_protocol_message(): # UTS: rest/unit/TP3g/timestamp-from-protocol-message-0 -@deviation def test_tp3g_timestamp_from_protocol_message(): protocol_msg = { 'action': 14, # PRESENCE From 14eaa71fcffd64cd848ac84867e57ec2bce9bb7a Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 20:48:03 +0000 Subject: [PATCH 3/7] pubsub: make Stats.interval_time a timezone-aware UTC datetime Stats.interval_time was parsed from the intervalId with strptime and left naive, although an intervalId is always a UTC time. TS12p asks for a language-idiomatic Time, and for Python that is an aware datetime: a naive one says nothing about its zone and could not be compared with the aware datetimes the rest of an application holds. interval_time is now the same instant with tzinfo set to UTC. This is a breaking change: an ordering comparison against a naive datetime now raises TypeError, and == against one quietly returns False. Callers must compare it with aware datetimes, for example datetime(..., tzinfo= timezone.utc). With PresenceMessage.timestamp no longer decoded through it, nothing under ably/ calls datetime.utcfromtimestamp. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 18 ++++++++++++++++++ ably/pubsub/types/stats.py | 5 +++-- test/ably/http/httpstats_test.py | 4 ++-- test/unit/stats_test.py | 23 +++++++++++++++++++++++ 4 files changed, 46 insertions(+), 4 deletions(-) create mode 100644 test/unit/stats_test.py diff --git a/UPDATING.md b/UPDATING.md index 19b0245d..b2d8aa9a 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -574,6 +574,24 @@ from datetime import datetime, timezone received_at = datetime.fromtimestamp(member.timestamp / 1000, tz=timezone.utc) ``` +### `Stats.interval_time` is timezone-aware + +`Stats.interval_time` is now an aware `datetime` in UTC, where 3.x returned a naive one. An +ordering comparison such as `<` against a naive `datetime` now raises `TypeError`, and `==` +against one quietly returns `False`. Compare it with aware datetimes: + +Example 3.x code: +```python +if stat.interval_time >= datetime(2024, 1, 1): + ... +``` + +Example 4.0.0 code: +```python +if stat.interval_time >= datetime(2024, 1, 1, tzinfo=timezone.utc): + ... +``` + ## Version 2.x to 3.0.0 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include: diff --git a/ably/pubsub/types/stats.py b/ably/pubsub/types/stats.py index ead5e548..ea88f993 100644 --- a/ably/pubsub/types/stats.py +++ b/ably/pubsub/types/stats.py @@ -1,5 +1,5 @@ import logging -from datetime import datetime +from datetime import datetime, timezone log = logging.getLogger(__name__) @@ -64,4 +64,5 @@ def granularity_from_interval_id(interval_id): def interval_from_interval_id(interval_id): granularity = granularity_from_interval_id(interval_id) - return datetime.strptime(interval_id, INTERVALS_FMT[granularity]) + # TS12p: the intervalId is a UTC time + return datetime.strptime(interval_id, INTERVALS_FMT[granularity]).replace(tzinfo=timezone.utc) diff --git a/test/ably/http/httpstats_test.py b/test/ably/http/httpstats_test.py index 5f039058..dedb8149 100644 --- a/test/ably/http/httpstats_test.py +++ b/test/ably/http/httpstats_test.py @@ -1,5 +1,5 @@ import logging -from datetime import datetime, timedelta +from datetime import datetime, timedelta, timezone import pytest @@ -304,4 +304,4 @@ async def test_interval(self): stat = stats[0] assert stat.unit == 'minute' assert stat.interval_id == self.last_interval.strftime('%Y-%m-%d:%H:%M') - assert stat.interval_time == self.last_interval + assert stat.interval_time == self.last_interval.replace(tzinfo=timezone.utc) diff --git a/test/unit/stats_test.py b/test/unit/stats_test.py new file mode 100644 index 00000000..b2e85d1b --- /dev/null +++ b/test/unit/stats_test.py @@ -0,0 +1,23 @@ +"""Unit tests for the Stats type. + +Tests cover: +- TS12p: intervalTime parsed from the intervalId as a UTC time +""" + +from datetime import datetime, timezone + +import pytest + +from ably.pubsub.types.stats import Stats + + +@pytest.mark.parametrize('interval_id, expected', [ + ('2024-01-02:03:04', datetime(2024, 1, 2, 3, 4, tzinfo=timezone.utc)), + ('2024-01-02:03', datetime(2024, 1, 2, 3, tzinfo=timezone.utc)), + ('2024-01-02', datetime(2024, 1, 2, tzinfo=timezone.utc)), + ('2024-01', datetime(2024, 1, 1, tzinfo=timezone.utc)), +]) +def test_interval_time_is_utc(interval_id, expected): + stats = Stats.from_dict({'intervalId': interval_id}) + assert stats.interval_time == expected + assert stats.interval_time.tzinfo is timezone.utc From f0b1f8f22f3ef3255fdc7ee0064eade08dbb22dd Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 20:55:46 +0000 Subject: [PATCH 4/7] pubsub: make HttpPaginatedResponse.error_code an int HttpPaginatedResponse.error_code returned the X-Ably-Errorcode header as the server sent it, a string such as '40101', where HP6 makes errorCode an integer, and every other error code the library exposes, on AblyException and ErrorInfo, is an int. error_code is now the header parsed as an int, and still None when the header is absent. This is a breaking change, and a silent one: a caller comparing error_code with a string, as in == '40101', now gets False. Callers must compare it with an int. The UTS tests drop their string assertions for the specification's integer ones, which closes the HP6 deviation. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 19 +++++++++++++++++++ ably/pubsub/request/paginatedresult.py | 3 ++- test/ably/http/httprequest_test.py | 2 +- test/uts/deviations.md | 1 - test/uts/rest/integration/auth_test.py | 3 +-- test/uts/rest/unit/request_test.py | 7 ++----- 6 files changed, 25 insertions(+), 10 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index b2d8aa9a..54e182c0 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -592,6 +592,25 @@ if stat.interval_time >= datetime(2024, 1, 1, tzinfo=timezone.utc): ... ``` +### `HttpPaginatedResponse.error_code` is an `int` + +**This change is silent: comparing `error_code` with a string, as in `== '40101'`, now +returns `False` rather than raising.** The `error_code` of the response that `request()` +returns is now the `int` parsed from the `X-Ably-Errorcode` header, where 3.x returned the +header's string. It is still `None` when the header is absent. Compare it with an `int`: + +Example 3.x code: +```python +if response.error_code == '40101': + ... +``` + +Example 4.0.0 code: +```python +if response.error_code == 40101: + ... +``` + ## Version 2.x to 3.0.0 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include: diff --git a/ably/pubsub/request/paginatedresult.py b/ably/pubsub/request/paginatedresult.py index d3361ee1..3d20e36a 100644 --- a/ably/pubsub/request/paginatedresult.py +++ b/ably/pubsub/request/paginatedresult.py @@ -124,7 +124,8 @@ def success(self): @property def error_code(self): - return self.response.headers.get('X-Ably-Errorcode') + error_code = self.response.headers.get('X-Ably-Errorcode') + return int(error_code) if error_code is not None else None @property def error_message(self): diff --git a/test/ably/http/httprequest_test.py b/test/ably/http/httprequest_test.py index bc378e92..36d146b2 100644 --- a/test/ably/http/httprequest_test.py +++ b/test/ably/http/httprequest_test.py @@ -85,7 +85,7 @@ async def test_error(self): assert isinstance(result, HttpPaginatedResponse) # RSC19d assert result.status_code == 400 # HP4 assert not result.success - assert result.error_code + assert isinstance(result.error_code, int) # HP6 assert result.error_message async def test_headers(self): diff --git a/test/uts/deviations.md b/test/uts/deviations.md index 006052d9..f4122b07 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -2195,7 +2195,6 @@ pseudocode is not mistaken for non-compliance. | RSC18 | The constructor rejects basic auth over HTTP | Construction succeeds; 40103 is raised from `make_request` when a request needing Basic Auth is attempted, and no request goes out. RSA1/RSC18 say only "any attempt to use" | Compliant; the UTS is stricter than its source | | RSAN1a3 | Code 40003 for a missing `Annotation.type` | 400/40000 | Cosmetic; worth aligning cross-SDK | | RSH1a | Empty `recipient` or `data` rejected with code 40000, the error reaching the caller from the server | `PushAdmin.publish` validates its arguments itself and raises before touching the HTTP layer: an empty or non-dict `recipient` or `data` raises an `AblyException` with 400/40000, the code the server gives, which is also a `ValueError`. So no request is made and there is no server error; the integration tier's `test_rsh1a_push_publish_invalid_recipient` asserts the local error's code | Compliant: RSH1a asks for empty values to be "immediately rejected". The specification's test would need a recipient the SDK will send and the server will reject, an unknown `transportType` say, to exercise the server-side path it describes | -| HP6 | `errorCode` is a number | The raw header string, `'40101'` | Open bug, trivial | | HP8 | `headers` is a map | A list of `(name, value)` pairs, so the lookup the spec describes is impossible without converting, and case-insensitivity is lost | Open bug; changing the return type is breaking | | RSC15a | Six hosts tried | Four: the primary and three fallbacks, because TO3l5's `httpMaxRetryCount`, default 3, caps the fallback hosts tried. The specification's six is the number of responses its setup queues, with a comment counting the primary and all five fallbacks, and no assertion reads it | Compliant; the setup's comment overlooks TO3l5 | | TI | `ErrorInfo` equality by attributes | No `__eq__`, so errors compare by identity. Python exceptions conventionally do, and the requirement appears nowhere in `features.md`. Adding `__eq__` without `__hash__` would make `AblyException` unhashable and break any caller that puts one in a set | Intentional | diff --git a/test/uts/rest/integration/auth_test.py b/test/uts/rest/integration/auth_test.py index 9d7c253d..d83d9511 100644 --- a/test/uts/rest/integration/auth_test.py +++ b/test/uts/rest/integration/auth_test.py @@ -140,8 +140,7 @@ async def test_rsa4_invalid_credentials_rejected(sandbox): result = await channel_status(client, channel_name) assert result.status_code == 401 - # `error_code` is the raw `X-Ably-Errorcode` header, so a string. - assert result.error_code == '40400' + assert result.error_code == 40400 # UTS: rest/integration/RSC10/token-renewal-expired-jwt-0 diff --git a/test/uts/rest/unit/request_test.py b/test/uts/rest/unit/request_test.py index 20132fa7..4cca30d4 100644 --- a/test/uts/rest/unit/request_test.py +++ b/test/uts/rest/unit/request_test.py @@ -313,9 +313,7 @@ async def test_rsc19d_response_error_code_header(): response = await client.request('GET', '/test', version='3') - # DEVIATION: the spec asserts errorCode == 40101, and HP6 types it as a number. - # error_code returns the raw X-Ably-Errorcode header value, so it is a string. - assert response.error_code == '40101' + assert response.error_code == 40101 # UTS: rest/unit/RSC19d/response-error-message-header-3 @@ -470,8 +468,7 @@ async def test_rsc19e_http_error_no_fallback(): # Should return the error response, not retry to fallback assert response.status_code == 400 assert response.success is False - # DEVIATION: the spec asserts errorCode == 40000; error_code returns the raw header - assert response.error_code == '40000' + assert response.error_code == 40000 # Only one request should have been made (no fallback) assert len(mock_http.captured_requests) == 1 From ee1fc93e6b0c63c20c51f0cfa022372beff34fac Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 21:00:23 +0000 Subject: [PATCH 5/7] pubsub: make HttpPaginatedResponse.headers a read-only mapping HttpPaginatedResponse.headers returned a list of (name, value) pairs, so a caller could not look a header up by name without converting the list, and lost the case-insensitivity HTTP header names have. HP8's prose calls headers an array of key value pairs, but the IDL types it Dict, and the UTS looks headers up by name in their original case, as in headers["Content-Type"]. headers is now a ResponseHeaders, a read-only Mapping looked up by name case-insensitively. As before, names are lower-cased and a repeated header's values are joined with ', '. This is a breaking change: iterating the mapping yields only the names, so `for name, value in headers` must become `for name, value in headers.items()`, and code relying on list methods or indexing by position must look headers up by name. The UTS test asserts the specification's lookups directly, which closes the HP8 deviation. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 22 ++++++++++++++ ably/pubsub/request/paginatedresult.py | 28 ++++++++++++++++- test/ably/http/httprequest_test.py | 5 ++- test/unit/paginatedresult_test.py | 42 ++++++++++++++++++++++++++ test/uts/deviations.md | 1 - test/uts/rest/unit/request_test.py | 10 +++--- 6 files changed, 99 insertions(+), 9 deletions(-) create mode 100644 test/unit/paginatedresult_test.py diff --git a/UPDATING.md b/UPDATING.md index 54e182c0..43a6b690 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -611,6 +611,28 @@ if response.error_code == 40101: ... ``` +### `HttpPaginatedResponse.headers` is a read-only mapping + +The `headers` of the response that `request()` returns is now a read-only mapping from header +name to value, looked up case-insensitively, where 3.x returned a list of `(name, value)` +pairs. As before, names are lower-cased and a header sent more than once has its values +joined with `', '`. Iterating over the mapping itself now yields only the names, so look a +header up by name, or iterate over `.items()`: + +Example 3.x code: +```python +for name, value in response.headers: + ... +``` + +Example 4.0.0 code: +```python +request_id = response.headers.get('X-Request-Id') + +for name, value in response.headers.items(): + ... +``` + ## Version 2.x to 3.0.0 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include: diff --git a/ably/pubsub/request/paginatedresult.py b/ably/pubsub/request/paginatedresult.py index 3d20e36a..8be1ef50 100644 --- a/ably/pubsub/request/paginatedresult.py +++ b/ably/pubsub/request/paginatedresult.py @@ -1,5 +1,6 @@ import calendar import logging +from collections.abc import Mapping from urllib.parse import urlencode from ably.pubsub.request.http import Request @@ -42,6 +43,31 @@ def format_params(params=None, direction=None, start=None, end=None, limit=None, return '?' + urlencode(params) if params else '' +class ResponseHeaders(Mapping): + """The headers of a response, read-only and looked up by name case-insensitively (HP8). + + Names iterate lower-cased, and a header sent more than once reads as its values joined + with ', '. + """ + + def __init__(self, headers): + self.__headers = {name.lower(): value for name, value in headers.items()} + + def __getitem__(self, name): + if not isinstance(name, str): + raise KeyError(name) + return self.__headers[name.lower()] + + def __iter__(self): + return iter(self.__headers) + + def __len__(self): + return len(self.__headers) + + def __repr__(self): + return f'{type(self).__name__}({self.__headers!r})' + + class PaginatedResult: def __init__(self, http, items, content_type, rel_first, rel_next, response_processor, response): @@ -133,4 +159,4 @@ def error_message(self): @property def headers(self): - return list(self.response.headers.items()) + return ResponseHeaders(self.response.headers) diff --git a/test/ably/http/httprequest_test.py b/test/ably/http/httprequest_test.py index 36d146b2..9de29f59 100644 --- a/test/ably/http/httprequest_test.py +++ b/test/ably/http/httprequest_test.py @@ -1,3 +1,5 @@ +from collections.abc import Mapping + import httpx import pytest import respx @@ -69,7 +71,8 @@ async def test_get(self): assert result.success is True # HP5 assert result.error_code is None # HP6 assert result.error_message is None # HP7 - assert isinstance(result.headers, list) # HP7 + assert isinstance(result.headers, Mapping) # HP8 + assert result.headers['Content-Type'] == result.headers['content-type'] @dont_vary_protocol async def test_not_found(self): diff --git a/test/unit/paginatedresult_test.py b/test/unit/paginatedresult_test.py new file mode 100644 index 00000000..5308c5fb --- /dev/null +++ b/test/unit/paginatedresult_test.py @@ -0,0 +1,42 @@ +"""Unit tests for the headers of an HttpPaginatedResponse. + +Tests cover: +- HP8: headers as a read-only mapping, looked up by name case-insensitively +""" + +import httpx +import pytest + +from ably.pubsub.request.paginatedresult import ResponseHeaders + + +def test_headers_look_up_case_insensitively(): + headers = ResponseHeaders(httpx.Headers({'Content-Type': 'application/json'})) + assert headers['content-type'] == 'application/json' + assert headers['CONTENT-TYPE'] == 'application/json' + assert headers.get('Content-Type') == 'application/json' + assert 'Content-Type' in headers + assert 'X-Missing' not in headers + assert headers.get('X-Missing') is None + assert 1 not in headers + + +def test_headers_iterate_as_lower_cased_names(): + headers = ResponseHeaders(httpx.Headers([('X-One', '1'), ('X-Two', '2')])) + assert list(headers) == ['x-one', 'x-two'] + assert dict(headers.items()) == {'x-one': '1', 'x-two': '2'} + assert len(headers) == 2 + + +def test_repeated_header_joins_its_values(): + headers = ResponseHeaders(httpx.Headers([('Link', ''), ('Link', '')])) + assert headers['Link'] == ', ' + assert len(headers) == 1 + + +def test_headers_are_read_only(): + headers = ResponseHeaders(httpx.Headers({'X-One': '1'})) + with pytest.raises(TypeError): + headers['X-One'] = '2' + with pytest.raises(TypeError): + del headers['X-One'] diff --git a/test/uts/deviations.md b/test/uts/deviations.md index f4122b07..844e5fff 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -2195,7 +2195,6 @@ pseudocode is not mistaken for non-compliance. | RSC18 | The constructor rejects basic auth over HTTP | Construction succeeds; 40103 is raised from `make_request` when a request needing Basic Auth is attempted, and no request goes out. RSA1/RSC18 say only "any attempt to use" | Compliant; the UTS is stricter than its source | | RSAN1a3 | Code 40003 for a missing `Annotation.type` | 400/40000 | Cosmetic; worth aligning cross-SDK | | RSH1a | Empty `recipient` or `data` rejected with code 40000, the error reaching the caller from the server | `PushAdmin.publish` validates its arguments itself and raises before touching the HTTP layer: an empty or non-dict `recipient` or `data` raises an `AblyException` with 400/40000, the code the server gives, which is also a `ValueError`. So no request is made and there is no server error; the integration tier's `test_rsh1a_push_publish_invalid_recipient` asserts the local error's code | Compliant: RSH1a asks for empty values to be "immediately rejected". The specification's test would need a recipient the SDK will send and the server will reject, an unknown `transportType` say, to exercise the server-side path it describes | -| HP8 | `headers` is a map | A list of `(name, value)` pairs, so the lookup the spec describes is impossible without converting, and case-insensitivity is lost | Open bug; changing the return type is breaking | | RSC15a | Six hosts tried | Four: the primary and three fallbacks, because TO3l5's `httpMaxRetryCount`, default 3, caps the fallback hosts tried. The specification's six is the number of responses its setup queues, with a comment counting the primary and all five fallbacks, and no assertion reads it | Compliant; the setup's comment overlooks TO3l5 | | TI | `ErrorInfo` equality by attributes | No `__eq__`, so errors compare by identity. Python exceptions conventionally do, and the requirement appears nowhere in `features.md`. Adding `__eq__` without `__hash__` would make `AblyException` unhashable and break any caller that puts one in a set | Intentional | | RSA6b, RSA6d | The capability literal as passed | Canonicalised by `Capability.c14n`, which RSA9f requires | Compliant; rendering only | diff --git a/test/uts/rest/unit/request_test.py b/test/uts/rest/unit/request_test.py index 4cca30d4..612dfe8c 100644 --- a/test/uts/rest/unit/request_test.py +++ b/test/uts/rest/unit/request_test.py @@ -349,12 +349,10 @@ async def test_rsc19d_response_headers_accessible(): response = await client.request('GET', '/test', version='3') - # DEVIATION: the spec (and HP8) expects a map keyed by header name. `headers` is a - # list of (name, value) pairs, with the names lower-cased by httpx. - headers = dict(response.headers) - assert headers['content-type'] == 'application/json' - assert headers['x-request-id'] == 'req-123' - assert headers['x-custom-header'] == 'custom-value' + headers = response.headers + assert headers['Content-Type'] == 'application/json' + assert headers['X-Request-Id'] == 'req-123' + assert headers['X-Custom-Header'] == 'custom-value' # UTS: rest/unit/RSC19d/response-items-decoded-5 From 6e344d085e5a83fcb8b19ca8290beb789c0faa11 Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 21:06:17 +0000 Subject: [PATCH 6/7] pubsub: return nothing from push removals and hide the paginated response remove and remove_where on push.admin.device_registrations and push.admin.channel_subscriptions returned the HTTP response of the DELETE, and every PaginatedResult carried the HTTP response it was built from as a public response attribute, so the library's public surface handed out the underlying httpx.Response. RSH1b4, RSH1b5, RSH1c4 and RSH1c5 give these removals no return value, and nothing in TG or HP asks for the response. The four removals now return None, and a failed one still raises AblyException. PaginatedResult no longer keeps the response; HttpPaginatedResponse reads its status code and headers from it once, at construction, so status_code, success, error_code, error_message and headers work as before. This is a breaking change: callers that checked remove()'s status_code must stop, since success is the absence of an exception, and callers that read result.response must read the dedicated HttpPaginatedResponse attributes instead. The UTS tests assert that the removals return None, which closes the push remove return-value note. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 21 +++++++++++++ ably/pubsub/http/push.py | 8 ++--- ably/pubsub/request/paginatedresult.py | 16 +++++++--- test/ably/http/httppush_test.py | 33 +++++++------------- test/ably/http/httprequest_test.py | 16 +++++++--- test/uts/deviations.md | 1 - test/uts/rest/integration/push_admin_test.py | 25 +++++---------- 7 files changed, 67 insertions(+), 53 deletions(-) diff --git a/UPDATING.md b/UPDATING.md index 43a6b690..ae848f2f 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -633,6 +633,27 @@ for name, value in response.headers.items(): ... ``` +### Push removals return nothing, and results no longer expose the HTTP response + +`remove` and `remove_where` on `push.admin.device_registrations` and +`push.admin.channel_subscriptions` now return `None`, where 3.x returned the underlying HTTP +response. A removal that fails raises an `AblyException`, so there is no status code to check: + +Example 3.x code: +```python +response = await rest.push.admin.device_registrations.remove(device_id) +assert response.status_code == 204 +``` + +Example 4.0.0 code: +```python +await pubsub_http_client.push.admin.device_registrations.remove(device_id) +``` + +For the same reason a `PaginatedResult` no longer has a `response` attribute. The +`HttpPaginatedResponse` that `request()` returns keeps `status_code`, `success`, +`error_code`, `error_message` and `headers`, so read those instead of `response`. + ## Version 2.x to 3.0.0 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include: diff --git a/ably/pubsub/http/push.py b/ably/pubsub/http/push.py index 1fc6ea98..d349d72b 100644 --- a/ably/pubsub/http/push.py +++ b/ably/pubsub/http/push.py @@ -115,7 +115,7 @@ async def remove(self, device_id: str): - `device_id`: the id of the device """ path = f'/push/deviceRegistrations/{device_id}' - return await self.ably.http.delete(path) + await self.ably.http.delete(path) async def remove_where(self, **params): """Deletes the registered devices identified by the given parameters. @@ -124,7 +124,7 @@ async def remove_where(self, **params): - `**params`: the parameters that identify the devices to remove """ path = '/push/deviceRegistrations' + format_params(params) - return await self.ably.http.delete(path) + await self.ably.http.delete(path) class PushChannelSubscriptions: @@ -180,7 +180,7 @@ async def remove(self, subscription: dict): """ subscription = PushChannelSubscription.factory(subscription) params = subscription.as_dict() - return await self.remove_where(**params) + await self.remove_where(**params) async def remove_where(self, **params): """Deletes the subscriptions identified by the given parameters. @@ -189,4 +189,4 @@ async def remove_where(self, **params): - `**params`: the parameters that identify the subscriptions to remove """ path = '/push/channelSubscriptions' + format_params(**params) - return await self.ably.http.delete(path) + await self.ably.http.delete(path) diff --git a/ably/pubsub/request/paginatedresult.py b/ably/pubsub/request/paginatedresult.py index 8be1ef50..a94ceeaa 100644 --- a/ably/pubsub/request/paginatedresult.py +++ b/ably/pubsub/request/paginatedresult.py @@ -77,7 +77,6 @@ def __init__(self, http, items, content_type, rel_first, rel_next, self.__rel_first = rel_first self.__rel_next = rel_next self.__response_processor = response_processor - self.response = response @property def items(self): @@ -139,9 +138,16 @@ async def paginated_query_with_request(cls, http, request, response_processor, class HttpPaginatedResponse(PaginatedResult): + def __init__(self, http, items, content_type, rel_first, rel_next, + response_processor, response): + super().__init__(http, items, content_type, rel_first, rel_next, + response_processor, response) + self.__status_code = response.status_code + self.__headers = ResponseHeaders(response.headers) + @property def status_code(self): - return self.response.status_code + return self.__status_code @property def success(self): @@ -150,13 +156,13 @@ def success(self): @property def error_code(self): - error_code = self.response.headers.get('X-Ably-Errorcode') + error_code = self.__headers.get('X-Ably-Errorcode') return int(error_code) if error_code is not None else None @property def error_message(self): - return self.response.headers.get('X-Ably-Errormessage') + return self.__headers.get('X-Ably-Errormessage') @property def headers(self): - return ResponseHeaders(self.response.headers) + return self.__headers diff --git a/test/ably/http/httppush_test.py b/test/ably/http/httppush_test.py index 1e17ab18..2a82300f 100644 --- a/test/ably/http/httppush_test.py +++ b/test/ably/http/httppush_test.py @@ -226,14 +226,12 @@ async def test_admin_device_registrations_remove(self): # Remove get_response = await get(device.id) assert get_response.id == device.id # Exists - remove_device_response = await self.remove_device(device.id) - assert remove_device_response.status_code == 204 + assert await self.remove_device(device.id) is None with pytest.raises(AblyException): # Doesn't exist await get(device.id) # Remove again, it doesn't fail - remove_device_response = await self.remove_device(device.id) - assert remove_device_response.status_code == 204 + assert await self.remove_device(device.id) is None # RSH1b5 async def test_admin_device_registrations_remove_where(self): @@ -243,8 +241,7 @@ async def test_admin_device_registrations_remove_where(self): device = self.get_device() foo_device = await get(device.id) assert foo_device.id == device.id # Exists - remove_foo_device_response = await self.remove_device_where(deviceId=device.id) - assert remove_foo_device_response.status_code == 204 + assert await self.remove_device_where(deviceId=device.id) is None with pytest.raises(AblyException): # Doesn't exist await get(device.id) @@ -252,8 +249,7 @@ async def test_admin_device_registrations_remove_where(self): device = self.get_device() boo_device = await get(device.id) assert boo_device.id == device.id # Exists - remove_boo_device_response = await self.remove_device_where(clientId=device.client_id) - assert remove_boo_device_response.status_code == 204 + assert await self.remove_device_where(clientId=device.client_id) is None # Doesn't exist (Deletion is async: wait up to a few seconds before giving up) with pytest.raises(AblyException): for _ in range(5): @@ -261,8 +257,7 @@ async def test_admin_device_registrations_remove_where(self): await get(device.id) # Remove with no matching params - remove_boo_device_response = await self.remove_device_where(clientId=device.client_id) - assert remove_boo_device_response.status_code == 204 + assert await self.remove_device_where(clientId=device.client_id) is None # # RSH1c1 async def test_admin_channel_subscriptions_list(self): @@ -351,8 +346,7 @@ async def test_admin_channel_subscriptions_remove(self): subscription = await save(PushChannelSubscription(channel, device_id=device.id)) list_response = await list_(channel=channel) assert device.id in (x.device_id for x in list_response.items) - remove_response = await remove(subscription) - assert remove_response.status_code == 204 + assert await remove(subscription) is None list_response = await list_(channel=channel) assert device.id not in (x.device_id for x in list_response.items) @@ -361,14 +355,12 @@ async def test_admin_channel_subscriptions_remove(self): subscription = await save(PushChannelSubscription(channel, client_id=client_id)) list_response = await list_(channel=channel) assert client_id in (x.client_id for x in list_response.items) - remove_response = await remove(subscription) - assert remove_response.status_code == 204 + assert await remove(subscription) is None list_response = await list_(channel=channel) assert client_id not in (x.client_id for x in list_response.items) # Remove again, it doesn't fail - remove_response = await remove(subscription) - assert remove_response.status_code == 204 + assert await remove(subscription) is None # RSH1c5 async def test_admin_channel_subscriptions_remove_where(self): @@ -383,8 +375,7 @@ async def test_admin_channel_subscriptions_remove_where(self): await save(PushChannelSubscription(channel, device_id=device.id)) list_response = await list_(channel=channel) assert device.id in (x.device_id for x in list_response.items) - remove_response = await remove(channel=channel, device_id=device.id) - assert remove_response.status_code == 204 + assert await remove(channel=channel, device_id=device.id) is None list_response = await list_(channel=channel) assert device.id not in (x.device_id for x in list_response.items) @@ -393,11 +384,9 @@ async def test_admin_channel_subscriptions_remove_where(self): await save(PushChannelSubscription(channel, client_id=client_id)) list_response = await list_(channel=channel) assert client_id in (x.client_id for x in list_response.items) - remove_response = await remove(channel=channel, client_id=client_id) - assert remove_response.status_code == 204 + assert await remove(channel=channel, client_id=client_id) is None list_response = await list_(channel=channel) assert client_id not in (x.client_id for x in list_response.items) # Remove again, it doesn't fail - remove_response = await remove(channel=channel, client_id=client_id) - assert remove_response.status_code == 204 + assert await remove(channel=channel, client_id=client_id) is None diff --git a/test/ably/http/httprequest_test.py b/test/ably/http/httprequest_test.py index 9de29f59..107ecae7 100644 --- a/test/ably/http/httprequest_test.py +++ b/test/ably/http/httprequest_test.py @@ -91,11 +91,15 @@ async def test_error(self): assert isinstance(result.error_code, int) # HP6 assert result.error_message + @dont_vary_protocol + @respx.mock async def test_headers(self): key = 'X-Test' value = 'lorem ipsum' - result = await self.ably.request('GET', '/time', headers={key: value}, version=Defaults.protocol_version) - assert result.response.request.headers[key] == value + route = respx.get('https://sandbox.realtime.ably-nonprod.net/time') + route.return_value = httpx.Response(200, headers={'Content-Type': 'application/json'}, text='[123]') + await self.ably.request('GET', '/time', headers={key: value}, version=Defaults.protocol_version) + assert route.calls.last.request.headers[key] == value # RSC19e @dont_vary_protocol @@ -233,7 +237,11 @@ async def test_400_cloudfront_fallback(self): assert result.items[0] == 123 await ably.close() + @dont_vary_protocol + @respx.mock async def test_version(self): version = "150" # chosen arbitrarily - result = await self.ably.request('GET', '/time', "150") - assert result.response.request.headers["X-Ably-Version"] == version + route = respx.get('https://sandbox.realtime.ably-nonprod.net/time') + route.return_value = httpx.Response(200, headers={'Content-Type': 'application/json'}, text='[123]') + await self.ably.request('GET', '/time', "150") + assert route.calls.last.request.headers["X-Ably-Version"] == version diff --git a/test/uts/deviations.md b/test/uts/deviations.md index 844e5fff..f6eaebf3 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -2179,7 +2179,6 @@ pseudocode is not mistaken for non-compliance. | RTP17b's synthesized-LEAVE filter | RTP17b's own implementation note allows the check to live "either inside the presence map's `remove()` method, or at the calling level". ably-python uses the calling level (`presence.py:557-558`). Compliant | | RTP19a's route | the specification models an ATTACHED without HAS_PRESENCE as `startSync()` then `endSync()`. `on_attached(has_presence=False)` calls `_synthesize_leaves(...)` then `clear()` (`presence.py:611-618`), which is the requirement itself rather than the model of it | | `hasNext` as a value | `push_admin.md` RSH1b2 writes `ASSERT result.hasNext == true`. In Python `has_next` is a bound method (`ably/http/paginatedresult.py:63`), truthy whatever the page holds, so the direct translation asserts nothing. `test_rsh1b2_list_devices_pagination` writes `result.has_next() is True`, confirmed to fail when inverted | -| Push `remove` return values | the specification's remove steps assert nothing about a return value ("should not throw"). `PushDeviceRegistrations.remove` / `remove_where` and `PushChannelSubscriptions.remove` / `remove_where` return the `ably.http.http.Response` from the DELETE rather than `None` (`ably/rest/push.py:112-127`, `:176-192`), so the six derived removal tests assert `response.status_code == 204`, which is stronger than the specification asks and matches what `test/ably/rest/restpush_test.py` already asserts | | `rest/proxy/RSC15l/unreachable-endpoint-error-0` | the specification asserts only that the error carries a non-null `status_code` or `code`, leaving the values open, so the derived test asserts exactly that and is the weakest of the eight proxy tests. What this SDK produces against a refused connection is 500 / 80000, the RSC19e wrapping of the `httpx.ConnectError`, which it keeps as the `cause`, and that is recorded here rather than asserted, since pinning it would assert more than the specification does | | `RSL1k4`'s history read | the event log is per session, so any request the client under test makes through it is recorded — including the `history()` the test verifies deduplication with. The log is therefore read **before** the history call, and the POST count assertion made against that snapshot. The history read itself is a `wall_clock_poll_until` rather than one fetch, for the same reason every other integration verification is: a published message does not reach history at once | | RSP4b1's time bounds | the specification records `time_before = now_millis()` before generating the presence events and `time_after = now_millis()` after, then asserts a `history(start=, end=)` over that window returns them. Read from the runner's clock the window is only as good as the skew against the sandbox, which decides the timestamps actually stored, so a runner running a little fast would exclude the very events the test generated. Both bounds come from `await client.time()` instead — the same instant on the clock that stamps the events. `Presence.history` passes an `int` straight through as milliseconds (`ably/types/presence.py:232-241`), which is what `client.time()` returns, so no conversion is involved | diff --git a/test/uts/rest/integration/push_admin_test.py b/test/uts/rest/integration/push_admin_test.py index f997217b..48d54a4e 100644 --- a/test/uts/rest/integration/push_admin_test.py +++ b/test/uts/rest/integration/push_admin_test.py @@ -195,9 +195,7 @@ async def test_rsh1b4_remove_device(sandbox): await registrations.save(apns_device(device_id)) - response = await registrations.remove(device_id) - # `remove` answers with the raw response rather than nothing. - assert response.status_code == 204 + assert await registrations.remove(device_id) is None # Removal is asynchronous, so an immediate `get` can still find the registration. async def removed(): @@ -216,10 +214,8 @@ async def removed(): async def test_rsh1b4_remove_nonexistent_device(sandbox): client = sandbox_rest_client(sandbox.key(0).key_str) - response = await client.push.admin.device_registrations.remove( - f'nonexistent-device-{random_id()}') - - assert response.status_code == 204 + assert await client.push.admin.device_registrations.remove( + f'nonexistent-device-{random_id()}') is None # UTS: rest/integration/RSH1b5/remove-where-clientid-0 @@ -238,8 +234,7 @@ async def test_rsh1b5_remove_where_clientid(sandbox): # A registration under a different clientId, which removeWhere must leave alone. await registrations.save(apns_device(survivor_id, client_id=survivor_client_id)) try: - response = await registrations.remove_where(clientId=client_id) - assert response.status_code == 204 + assert await registrations.remove_where(clientId=client_id) is None # Deletion is asynchronous on the server side. async def removed(): @@ -349,8 +344,7 @@ async def test_rsh1c4_remove_channel_subscription(sandbox): # The filter has to narrow for the assertion below to mean anything. assert len((await subscriptions.list(channel=channel_name, clientId=client_id)).items) == 1 - response = await subscriptions.remove(subscription) - assert response.status_code == 204 + assert await subscriptions.remove(subscription) is None async def removed(): result = await subscriptions.list(channel=channel_name, clientId=client_id) @@ -365,10 +359,8 @@ async def removed(): async def test_rsh1c4_remove_nonexistent_subscription(sandbox): client = sandbox_rest_client(sandbox.key(0).key_str) - response = await client.push.admin.channel_subscriptions.remove(PushChannelSubscription( - f'{PUSH_NAMESPACE}:nonexistent-{random_id()}', client_id='nonexistent-client')) - - assert response.status_code == 204 + assert await client.push.admin.channel_subscriptions.remove(PushChannelSubscription( + f'{PUSH_NAMESPACE}:nonexistent-{random_id()}', client_id='nonexistent-client')) is None # UTS: rest/integration/RSH1c5/remove-where-subscriptions-0 @@ -389,8 +381,7 @@ async def test_rsh1c5_remove_where_subscriptions(sandbox): survivor = PushChannelSubscription(survivor_channel, client_id=survivor_client_id) await subscriptions.save(survivor) try: - response = await subscriptions.remove_where(clientId=client_id) - assert response.status_code == 204 + assert await subscriptions.remove_where(clientId=client_id) is None async def removed(): result = await subscriptions.list(clientId=client_id) From c381f93f644f923f93cd1914dc1344fa849e9f66 Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 26 Oct 2026 21:14:07 +0000 Subject: [PATCH 7/7] pubsub: add DevicePushDetails and ignore unknown push fields DeviceDetails.push was the dict the server sent, where PCD7 types it as a DevicePushDetails with the recipient, state and errorReason of PCP2-PCP4. And DeviceDetails.from_dict and PushChannelSubscription.from_dict passed every key of the decoded object to the constructor with cls(**obj), so a field the server added that the class did not know raised TypeError and failed the whole get, list or save. DeviceDetails.push is now a DevicePushDetails, built from a dict or taken as one, which validates the recipient's transportType as DeviceDetails used to and decodes errorReason into an AblyException. Both from_dict methods, and DevicePushDetails', now ignore fields they do not model. DevicePushDetails lives beside DeviceDetails in ably.pubsub.types.device and is not added to ably.pubsub.server, whose exports are still to be decided. This is a breaking change: device.push['recipient'] now raises TypeError, and callers must read device.push.recipient, .state and .error_reason. A device can still be built or saved with push given as a dict. A misspelled key in a dict passed to save() is now ignored rather than rejected. The UTS tests build devices with DevicePushDetails as the specification does, which closes the DevicePushDetails missing-accessor row. Co-Authored-By: Claude Opus 5.5 --- UPDATING.md | 22 +++++ ably/pubsub/types/channelsubscription.py | 9 +- ably/pubsub/types/device.py | 83 ++++++++++++++++--- test/unit/push_types_test.py | 76 +++++++++++++++++ test/uts/deviations.md | 1 - test/uts/rest/integration/push_admin_test.py | 17 ++-- .../rest/integration/push_channels_test.py | 7 +- .../push/push_device_registrations_test.py | 23 +++-- 8 files changed, 196 insertions(+), 42 deletions(-) create mode 100644 test/unit/push_types_test.py diff --git a/UPDATING.md b/UPDATING.md index ae848f2f..0359db06 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -654,6 +654,28 @@ For the same reason a `PaginatedResult` no longer has a `response` attribute. Th `HttpPaginatedResponse` that `request()` returns keeps `status_code`, `success`, `error_code`, `error_message` and `headers`, so read those instead of `response`. +### `DeviceDetails.push` is a `DevicePushDetails` + +`DeviceDetails.push` is now a `DevicePushDetails`, with `recipient`, `state` and +`error_reason` attributes, where 3.x returned the dict the server sent. `error_reason` is an +`AblyException`. Indexing it, as in `device.push['recipient']`, now raises `TypeError`. +Read the attributes instead. A device can still be built or saved with `push` given as a +dict: + +Example 3.x code: +```python +token = device.push['recipient']['deviceToken'] +``` + +Example 4.0.0 code: +```python +token = device.push.recipient['deviceToken'] +``` + +`DeviceDetails` and `PushChannelSubscription` now also ignore fields they do not know when +decoding a dict, where 3.x raised `TypeError` on any field the server added. A misspelled key +in a dict you pass to `save()` is therefore ignored rather than rejected. + ## Version 2.x to 3.0.0 The 3.0.0 version of ably-python introduces several breaking changes to improve the realtime experience and align the API with the Ably specification. These include: diff --git a/ably/pubsub/types/channelsubscription.py b/ably/pubsub/types/channelsubscription.py index 9cc386c9..2c4ee693 100644 --- a/ably/pubsub/types/channelsubscription.py +++ b/ably/pubsub/types/channelsubscription.py @@ -3,6 +3,8 @@ class PushChannelSubscription: + _fields = ['channel', 'device_id', 'client_id', 'app_id'] + def __init__(self, channel, device_id=None, client_id=None, app_id=None): if not device_id and not client_id: raise ValueError('missing expected device or client id') @@ -32,10 +34,8 @@ def app_id(self): return self.__app_id def as_dict(self): - keys = ['channel', 'device_id', 'client_id', 'app_id'] - obj = {} - for key in keys: + for key in self._fields: value = getattr(self, key) if value is not None: key = case.snake_to_camel(key) @@ -46,7 +46,8 @@ def as_dict(self): @classmethod def from_dict(cls, obj): obj = {case.camel_to_snake(key): value for key, value in obj.items()} - return cls(**obj) + # Fields this version does not model are ignored, so the server can add them + return cls(**{key: value for key, value in obj.items() if key in cls._fields}) @classmethod def from_array(cls, array): diff --git a/ably/pubsub/types/device.py b/ably/pubsub/types/device.py index 3948b751..421fe250 100644 --- a/ably/pubsub/types/device.py +++ b/ably/pubsub/types/device.py @@ -1,22 +1,83 @@ from ably.pubsub.util import case +from ably.pubsub.util.exceptions import AblyException DevicePushTransportType = {'fcm', 'gcm', 'apns', 'web'} DevicePlatform = {'android', 'ios', 'browser'} DeviceFormFactor = {'phone', 'tablet', 'desktop', 'tv', 'watch', 'car', 'embedded', 'other'} +class DevicePushDetails: + """The push registration of a device (PCP1). + + `recipient` is a dict naming the push transport and address, `state` is the state as + the server sends it, such as 'Active', and `error_reason` is an AblyException. + """ + + def __init__(self, recipient=None, state=None, error_reason=None): + if recipient: + transport_type = recipient.get('transportType') + if transport_type is not None and transport_type not in DevicePushTransportType: + raise ValueError(f'unexpected transport type {transport_type}') + + self.__recipient = recipient + self.__state = state + self.__error_reason = error_reason + + @property + def recipient(self): + return self.__recipient + + @property + def state(self): + return self.__state + + @property + def error_reason(self): + return self.__error_reason + + def as_dict(self): + obj = {} + if self.recipient is not None: + obj['recipient'] = self.recipient + if self.state is not None: + obj['state'] = self.state + if self.error_reason is not None: + obj['errorReason'] = { + 'message': self.error_reason.message, + 'statusCode': self.error_reason.status_code, + 'code': self.error_reason.code, + } + + return obj + + @classmethod + def from_dict(cls, obj): + error_reason = obj.get('errorReason') + if isinstance(error_reason, dict): + error_reason = AblyException.from_dict(error_reason) + + # Fields this version does not model are ignored, so the server can add them + return cls(recipient=obj.get('recipient'), state=obj.get('state'), error_reason=error_reason) + + @classmethod + def factory(cls, push): + if isinstance(push, cls): + return push + + return cls.from_dict(push) + + class DeviceDetails: + _fields = ['id', 'client_id', 'form_factor', 'metadata', 'platform', + 'push', 'update_token', 'app_id', 'device_identity_token', 'modified', 'device_secret'] + def __init__(self, id, client_id=None, form_factor=None, metadata=None, platform=None, push=None, update_token=None, app_id=None, device_identity_token=None, modified=None, device_secret=None): - if push: - recipient = push.get('recipient') - if recipient: - transport_type = recipient.get('transportType') - if transport_type is not None and transport_type not in DevicePushTransportType: - raise ValueError(f'unexpected transport type {transport_type}') + if push is not None: + push = DevicePushDetails.factory(push) if platform is not None and platform not in DevicePlatform: raise ValueError(f'unexpected platform {platform}') @@ -81,13 +142,12 @@ def device_secret(self): return self.__device_secret def as_dict(self): - keys = ['id', 'client_id', 'form_factor', 'metadata', 'platform', - 'push', 'update_token', 'app_id', 'device_identity_token', 'modified', 'device_secret'] - obj = {} - for key in keys: + for key in self._fields: value = getattr(self, key) if value is not None: + if key == 'push': + value = value.as_dict() key = case.snake_to_camel(key) obj[key] = value @@ -96,7 +156,8 @@ def as_dict(self): @classmethod def from_dict(cls, obj): obj = {case.camel_to_snake(key): value for key, value in obj.items()} - return cls(**obj) + # Fields this version does not model are ignored, so the server can add them + return cls(**{key: value for key, value in obj.items() if key in cls._fields}) @classmethod def from_array(cls, array): diff --git a/test/unit/push_types_test.py b/test/unit/push_types_test.py new file mode 100644 index 00000000..9208b7c1 --- /dev/null +++ b/test/unit/push_types_test.py @@ -0,0 +1,76 @@ +"""Unit tests for the push admin types. + +Tests cover: +- PCD7, PCP1-PCP4: DeviceDetails.push as a DevicePushDetails +- fields the server adds that the types do not model are ignored when decoding +""" + +import pytest + +from ably.pubsub.types.channelsubscription import PushChannelSubscription +from ably.pubsub.types.device import DeviceDetails, DevicePushDetails +from ably.pubsub.util.exceptions import AblyException + +DEVICE = { + 'id': 'device-001', + 'clientId': 'client-abc', + 'platform': 'ios', + 'formFactor': 'phone', + 'push': { + 'recipient': {'transportType': 'apns', 'deviceToken': 'token-123'}, + 'state': 'Failing', + 'errorReason': {'message': 'Bad token', 'statusCode': 400, 'code': 40000}, + }, +} + + +def test_device_push_is_device_push_details(): + device = DeviceDetails.from_dict(DEVICE) + assert isinstance(device.push, DevicePushDetails) + assert device.push.recipient == {'transportType': 'apns', 'deviceToken': 'token-123'} + assert device.push.state == 'Failing' + assert isinstance(device.push.error_reason, AblyException) + assert device.push.error_reason.code == 40000 + assert device.push.error_reason.status_code == 400 + assert device.push.error_reason.message == 'Bad token' + + +def test_device_as_dict_round_trips(): + assert DeviceDetails.from_dict(DEVICE).as_dict() == DEVICE + + +def test_device_accepts_push_as_dict_or_details(): + recipient = {'transportType': 'apns', 'deviceToken': 'token-123'} + from_dict = DeviceDetails('device-001', push={'recipient': recipient}) + from_details = DeviceDetails('device-001', push=DevicePushDetails(recipient=recipient)) + assert from_dict.push.recipient == recipient + assert from_details.push.recipient == recipient + assert from_dict.as_dict() == from_details.as_dict() == {'id': 'device-001', 'push': {'recipient': recipient}} + + +def test_push_rejects_unknown_transport_type(): + with pytest.raises(ValueError): + DevicePushDetails(recipient={'transportType': 'xyz'}) + with pytest.raises(ValueError): + DeviceDetails('device-001', push={'recipient': {'transportType': 'xyz'}}) + + +def test_device_ignores_unknown_fields(): + device = DeviceDetails.from_dict({ + **DEVICE, + 'someNewField': 'value', + 'push': {**DEVICE['push'], 'someNewPushField': 'value'}, + }) + assert device.id == 'device-001' + assert device.as_dict() == DEVICE + + +def test_channel_subscription_ignores_unknown_fields(): + subscription = PushChannelSubscription.from_dict({ + 'channel': 'pushenabled:test', + 'clientId': 'client-abc', + 'someNewField': 'value', + }) + assert subscription.channel == 'pushenabled:test' + assert subscription.client_id == 'client-abc' + assert subscription.as_dict() == {'channel': 'pushenabled:test', 'clientId': 'client-abc'} diff --git a/test/uts/deviations.md b/test/uts/deviations.md index f6eaebf3..cc20db47 100644 --- a/test/uts/deviations.md +++ b/test/uts/deviations.md @@ -1871,7 +1871,6 @@ through an internal object to get at a value the specification makes public. | 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, RTS4c, RTS4d, RTS4e | `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 | 6 | -| 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 accessors would leave every assertion above unchanged; only the spelling would move. diff --git a/test/uts/rest/integration/push_admin_test.py b/test/uts/rest/integration/push_admin_test.py index 48d54a4e..e6ea2b97 100644 --- a/test/uts/rest/integration/push_admin_test.py +++ b/test/uts/rest/integration/push_admin_test.py @@ -8,6 +8,7 @@ from ably.pubsub.request.paginatedresult import PaginatedResult from ably.pubsub.server import AblyException, DeviceDetails, PushChannelSubscription +from ably.pubsub.types.device import DevicePushDetails from test.uts.helpers.client import sandbox_rest_client, wall_clock_poll_until from test.uts.helpers.sandbox import random_id @@ -23,17 +24,13 @@ def apns_device(device_id, client_id=None, device_token=DEVICE_TOKEN, platform='ios', form_factor='phone'): - """The specifications' `DeviceDetails(... push: DevicePushDetails(recipient: ...))`. - - There is no `DevicePushDetails` type in ably-python; `DeviceDetails.push` is - the plain dict the wire carries, so the recipient goes in directly. - """ + """The specifications' `DeviceDetails(... push: DevicePushDetails(recipient: ...))`.""" return DeviceDetails( id=device_id, client_id=client_id, platform=platform, form_factor=form_factor, - push={'recipient': {'transportType': 'apns', 'deviceToken': device_token}}, + push=DevicePushDetails(recipient={'transportType': 'apns', 'deviceToken': device_token}), ) @@ -90,7 +87,7 @@ async def test_rsh1b3_save_and_get_device(sandbox): assert saved.id == device_id assert saved.platform == 'ios' assert saved.form_factor == 'phone' - assert saved.push['recipient']['transportType'] == 'apns' + assert saved.push.recipient['transportType'] == 'apns' retrieved = await registrations.get(device_id) assert isinstance(retrieved, DeviceDetails) @@ -113,10 +110,10 @@ async def test_rsh1b3_update_device_registration(sandbox): updated = await registrations.save(apns_device(device_id, device_token=token_v2)) assert updated.id == device_id - assert updated.push['recipient']['deviceToken'] == token_v2 + assert updated.push.recipient['deviceToken'] == token_v2 retrieved = await registrations.get(device_id) - assert retrieved.push['recipient']['deviceToken'] == token_v2 + assert retrieved.push.recipient['deviceToken'] == token_v2 finally: await remove_device_quietly(client, device_id) @@ -144,7 +141,7 @@ async def test_rsh1b2_list_devices_filtered(sandbox): id=device_id, platform='android', form_factor='tablet', - push={'recipient': {'transportType': 'gcm', 'registrationToken': f'token-{random_id()}'}}, + push=DevicePushDetails(recipient={'transportType': 'gcm', 'registrationToken': f'token-{random_id()}'}), )) await registrations.save(apns_device(decoy_id)) try: diff --git a/test/uts/rest/integration/push_channels_test.py b/test/uts/rest/integration/push_channels_test.py index 79b60a65..4c031e4f 100644 --- a/test/uts/rest/integration/push_channels_test.py +++ b/test/uts/rest/integration/push_channels_test.py @@ -4,7 +4,7 @@ DEVIATION: ably-python implements neither the PushChannel interface (RSH7, the `push` field on a channel) nor LocalDevice (RSH8). `ably/rest/channel.py` gives a channel no -`push`, `DefaultPubSubHttpClient` no `device`, and `ably/types/device.py` defines only `DeviceDetails`. +`push`, `DefaultPubSubHttpClient` no `device`, and `ably/types/device.py` defines no `LocalDevice`. Both tests here therefore depart from the specification and are gated behind RUN_DEVIATIONS, against the same spelling [test/uts/rest/unit/push/push_channels_test.py](../unit/push/push_channels_test.py) @@ -32,6 +32,7 @@ """ from ably.pubsub.server import AblyException, DeviceDetails +from ably.pubsub.types.device import DevicePushDetails from test.uts.helpers.client import sandbox_rest_client, wall_clock_poll_until from test.uts.helpers.deviations import deviation from test.uts.helpers.sandbox import random_id @@ -93,13 +94,11 @@ async def test_rsh7a_subscribe_unsubscribe_device(sandbox): device_token = 'test-apns-token-' + random_id() # A device has to be registered before a device channel subscription will take. - # There is no `DevicePushDetails` type in ably-python; `DeviceDetails.push` is the - # plain dict the wire carries, so the recipient goes in directly. registration = await client.push.admin.device_registrations.save(DeviceDetails( id=device_id, platform='ios', form_factor='phone', - push={'recipient': {'transportType': 'apns', 'deviceToken': device_token}}, + push=DevicePushDetails(recipient={'transportType': 'apns', 'deviceToken': device_token}), )) try: # SPEC FAULT: the specification's own comment says the deviceIdentityToken is diff --git a/test/uts/rest/unit/push/push_device_registrations_test.py b/test/uts/rest/unit/push/push_device_registrations_test.py index 495ecc64..78dbee34 100644 --- a/test/uts/rest/unit/push/push_device_registrations_test.py +++ b/test/uts/rest/unit/push/push_device_registrations_test.py @@ -9,7 +9,7 @@ import pytest from ably.pubsub.request.paginatedresult import PaginatedResult -from ably.pubsub.types.device import DeviceDetails +from ably.pubsub.types.device import DeviceDetails, DevicePushDetails from ably.pubsub.util.exceptions import AblyException from test.uts.helpers.client import rest_client from test.uts.helpers.deviations import deviation @@ -61,10 +61,9 @@ async def test_rsh1b1_get_device_details(): assert device.form_factor == 'phone' assert device.platform == 'ios' assert device.metadata['model'] == 'iPhone 14' - # NOTE: the spec models push as a DevicePushDetails object. DeviceDetails.push is a - # plain dict here, so the same values are read by key. - assert device.push['recipient']['transportType'] == 'apns' - assert device.push['state'] == 'Active' + assert isinstance(device.push, DevicePushDetails) + assert device.push.recipient['transportType'] == 'apns' + assert device.push.state == 'Active' # UTS: rest/unit/RSH1b1/get-unknown-device-error-1 @@ -205,7 +204,7 @@ async def test_rsh1b3_save_put_device_details(): client_id='client-abc', platform='ios', form_factor='phone', - push={'recipient': {'transportType': 'apns', 'deviceToken': 'token-123'}}, + push=DevicePushDetails(recipient={'transportType': 'apns', 'deviceToken': 'token-123'}), ) result = await client.push.admin.device_registrations.save(device) @@ -225,7 +224,7 @@ async def test_rsh1b3_save_put_device_details(): assert isinstance(result, DeviceDetails) assert result.id == 'device-001' - assert result.push['state'] == 'Active' + assert result.push.state == 'Active' # UTS: rest/unit/RSH1b3/save-updates-existing-1 @@ -263,7 +262,7 @@ def on_request(request): id='device-001', platform='ios', form_factor='phone', - push={'recipient': {'transportType': 'apns', 'deviceToken': 'token-old'}}, + push=DevicePushDetails(recipient={'transportType': 'apns', 'deviceToken': 'token-old'}), ) result1 = await client.push.admin.device_registrations.save(device) @@ -272,13 +271,13 @@ def on_request(request): id='device-001', platform='ios', form_factor='phone', - push={'recipient': {'transportType': 'apns', 'deviceToken': 'token-new'}}, + push=DevicePushDetails(recipient={'transportType': 'apns', 'deviceToken': 'token-new'}), ) result2 = await client.push.admin.device_registrations.save(updated_device) - assert result1.push['recipient']['deviceToken'] == 'token-old' - assert result2.push['recipient']['deviceToken'] == 'token-new' + assert result1.push.recipient['deviceToken'] == 'token-old' + assert result2.push.recipient['deviceToken'] == 'token-new' assert request_count == 2 @@ -297,7 +296,7 @@ async def test_rsh1b3_save_error_propagated(): id='device-001', platform='ios', form_factor='phone', - push={'recipient': {}}, + push=DevicePushDetails(recipient={}), ) with pytest.raises(AblyException) as excinfo: