diff --git a/UPDATING.md b/UPDATING.md index 0359db06..9595b77f 100644 --- a/UPDATING.md +++ b/UPDATING.md @@ -676,6 +676,112 @@ token = device.push.recipient['deviceToken'] 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. +### Setting channel options replaces them, cipher included + +**`set_options(ChannelOptions(params=...))` on an encrypted channel now turns encryption off.** +`channel.set_options(options)`, and `channels.get(name, options)` on a channel that already +exists, replace the channel's options whole rather than merging them, so options without a +`cipher` leave the channel unencrypted. 3.x kept the old cipher whenever the new options left +it out, and on a realtime channel even when they gave `cipher=None`. Pass the cipher again +whenever you set the options of an encrypted channel: + +Example 3.x code: +```python +channel = realtime.channels.get('my-channel', ChannelOptions(cipher=cipher_params)) +await channel.set_options(ChannelOptions(params={'rewind': '1'})) +``` + +Example 4.0.0 code: +```python +channel = pubsub_realtime_client.channels.get('my-channel', ChannelOptions(cipher=cipher_params)) +await channel.set_options(ChannelOptions(cipher=cipher_params, params={'rewind': '1'})) +``` + +`channel.options` is now the `ChannelOptions` that was last set, rather than a dict, so read +its attributes rather than its keys: + +Example 3.x code: +```python +rewind = channel.options['params']['rewind'] +``` + +Example 4.0.0 code: +```python +rewind = channel.options.params['rewind'] +``` + +HTTP channels now take a `ChannelOptions` too, through `channels.get(name, options)` or +`await channel.set_options(options)`. `channels.get` raises when given options that would make +an attaching or attached realtime channel reattach, which only a change to `params` or `modes` +does; a change to the cipher alone no longer raises. + +### Channels are no longer attributes of the channels collection + +`client.channels.my_channel` used to get or create the channel called `my_channel`, because the +collection answered any attribute it did not define with a channel of that name. That made a +typo, or even `hasattr(client.channels, ...)`, create a channel. In 4.0.0 an unknown attribute +raises `AttributeError`, so get the channel by name instead: + +Example 3.x code: +```python +channel = rest.channels.my_channel +``` + +Example 4.0.0 code: +```python +channel = pubsub_http_client.channels.get('my_channel') +``` + +### A space in a channel name reaches the channel it names over HTTP + +3.x wrote a space in a channel name as `+` in HTTP request paths, which the Ably service reads +as a literal plus, so HTTP publishes, history, presence, annotations, message reads and +`status()` for `my channel` all went to a different channel, `my+channel`, from the one +realtime clients attach to. 4.0.0 writes the space as `%20`, so they reach `my channel`. +Messages that 3.x stored over HTTP for such a name are in the `+` channel, and stay reachable +by that name: + +Example 4.0.0 code: +```python +stored_by_3x = pubsub_http_client.channels.get('my+channel') +history = await stored_by_3x.history() +``` + +`status()` now escapes the channel name too, so a name containing `/`, `?` or `#` no longer +addresses the wrong resource. + +### `attach()` raises when the channel does not attach + +`await channel.attach()` raised only when the channel became `SUSPENDED` or `FAILED`. When the +connection closed while the channel was attaching, the channel became `DETACHED` and `attach()` +returned as if it had succeeded; so did an `attach()` overtaken by a `detach()`. In 4.0.0 it +raises an `AblyException` whenever the channel moves to any state but `ATTACHED`, carrying the +reason for that state, or code 90001 when there is none. `subscribe()`, which attaches, raises +in the same cases. Code that closes the client, or detaches, while an attach may be pending +should expect the exception: + +Example 4.0.0 code: +```python +try: + await channel.attach() +except AblyException: + # the channel did not attach, for instance because the client was closed meanwhile + ... +``` + +### `connection.ping()` waits for a disconnected connection + +`await connection.ping()` raised an `AblyException` with code 40000 at once when the connection +was `DISCONNECTED`. In 4.0.0 it waits for the connection to reconnect, as it already did while +`CONNECTING`, and then times out `realtime_request_timeout` after its heartbeat goes out. It +still fails if the connection becomes `SUSPENDED`, `CLOSED` or `FAILED` first, which can be up to +two minutes later. Bound the wait yourself if you need an answer sooner: + +Example 4.0.0 code: +```python +response_time_ms = await asyncio.wait_for(pubsub_realtime_client.connection.ping(), timeout=5) +``` + ## 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/annotations.py b/ably/pubsub/http/annotations.py index 88332352..051fa488 100644 --- a/ably/pubsub/http/annotations.py +++ b/ably/pubsub/http/annotations.py @@ -116,8 +116,7 @@ def __base_path_for_serial(self, serial): Returns: str: The API path """ - channel_path = '/channels/{}/'.format(parse.quote_plus(self.__channel.name, safe=':')) - return channel_path + 'messages/' + parse.quote_plus(serial, safe=':') + '/annotations' + return self.__channel.base_path + 'messages/' + parse.quote(serial, safe=':') + '/annotations' async def __send_annotation(self, annotation: Annotation, params: dict | None = None): """ diff --git a/ably/pubsub/http/channel.py b/ably/pubsub/http/channel.py index 134de023..3fa079c2 100644 --- a/ably/pubsub/http/channel.py +++ b/ably/pubsub/http/channel.py @@ -9,6 +9,7 @@ from ably.pubsub.http.annotations import HttpAnnotations from ably.pubsub.request.paginatedresult import PaginatedResult, format_params from ably.pubsub.types.channeldetails import ChannelDetails +from ably.pubsub.types.channeloptions import ChannelOptions from ably.pubsub.types.message import ( Message, MessageAction, @@ -33,12 +34,12 @@ class Channel: __annotations: HttpAnnotations - def __init__(self, ably, name, options): + def __init__(self, ably, name, options: Optional[ChannelOptions] = None): self.__ably = ably self.__name = name - self.__base_path = '/channels/{}/'.format(parse.quote_plus(name, safe=':')) - self.__cipher = None - self.options = options + # RSL2: quote_plus is form encoding, and its `+` for a space is a literal plus in a path + self.__base_path = '/channels/{}/'.format(parse.quote(name, safe=':')) + self._set_options(options or ChannelOptions()) self.__presence = Presence(self) self.__annotations = HttpAnnotations(self) @@ -48,7 +49,7 @@ async def history(self, direction=None, limit: int = None, start=None, end=None) params = format_params({}, direction=direction, start=start, end=end, limit=limit) path = self.__base_path + 'messages' + params - message_handler = make_message_response_handler(self.__cipher) + message_handler = make_message_response_handler(self.cipher) return await PaginatedResult.paginated_query( self.ably.http, url=path, response_processor=message_handler) @@ -73,7 +74,7 @@ def __publish_request_body(self, messages): 400, 40012) if self.cipher: - m.encrypt(self.__cipher) + m.encrypt(self.cipher) request_body_list.append(m) @@ -156,7 +157,8 @@ async def publish(self, *args, **kwargs): async def status(self): """Retrieves current channel active status with no. of publishers, subscribers, presence_members etc""" - path = f'/channels/{self.name}' + # RSL8 + path = '/channels/{}'.format(parse.quote(self.name, safe=':')) response = await self.ably.http.get(path) obj = response.to_native() return ChannelDetails.from_dict(obj) @@ -199,7 +201,7 @@ async def _send_update( # Encrypt if needed if self.cipher: - update_message.encrypt(self.__cipher) + update_message.encrypt(self.cipher) # Serialize the message request_body = update_message.as_dict(binary=self.ably.options.use_binary_protocol) @@ -210,7 +212,7 @@ async def _send_update( request_body = msgpack.packb(request_body, use_bin_type=True) # Build path with params - path = self.__base_path + 'messages/{}'.format(parse.quote_plus(message.serial, safe=':')) + path = self.__base_path + 'messages/{}'.format(parse.quote(message.serial, safe=':')) if params: params = {k: str(v).lower() if isinstance(v, bool) else v for k, v in params.items()} path += '?' + parse.urlencode(params) @@ -292,13 +294,13 @@ async def get_message(self, serial_or_message): ) # Build the path - path = self.__base_path + 'messages/' + parse.quote_plus(serial, safe=':') + path = self.__base_path + 'messages/' + parse.quote(serial, safe=':') # Make the request response = await self.ably.http.get(path) # Create Message from the response - message_handler = make_single_message_response_handler(self.__cipher) + message_handler = make_single_message_response_handler(self.cipher) return message_handler(response) async def get_message_versions(self, serial_or_message, params=None): @@ -332,10 +334,10 @@ async def get_message_versions(self, serial_or_message, params=None): # Build the path params_str = format_params({}, **params) if params else '' - path = self.__base_path + 'messages/' + parse.quote_plus(serial, safe=':') + '/versions' + params_str + path = self.__base_path + 'messages/' + parse.quote(serial, safe=':') + '/versions' + params_str # Create message handler for decoding - message_handler = make_message_response_handler(self.__cipher) + message_handler = make_message_response_handler(self.cipher) # Return paginated result return await PaginatedResult.paginated_query( @@ -358,10 +360,13 @@ def base_path(self): @property def cipher(self): + """The cipher built from `options.cipher`, or None when the channel is not encrypted""" return self.__cipher + # TB2 @property - def options(self): + def options(self) -> ChannelOptions: + """The channel's options, as last set""" return self.__options @property @@ -372,15 +377,20 @@ def presence(self): def annotations(self) -> HttpAnnotations: return self.__annotations - @options.setter - def options(self, options): - self.__options = options + # RSL7 + async def set_options(self, options: ChannelOptions) -> None: + """Replaces the channel's options. + + The options are replaced whole rather than merged, so options without a cipher turn + encryption off. + """ + self._set_options(options) - if options and 'cipher' in options: - cipher = options.get('cipher') - if cipher is not None: - cipher = get_cipher(cipher) - self.__cipher = cipher + def _set_options(self, options: ChannelOptions) -> None: + self.__options = options + # The cipher carries the CBC chaining state from one message to the next, so it is built + # once per set of options rather than on every read + self.__cipher = get_cipher(options.cipher) if options.cipher is not None else None class Channels: @@ -388,25 +398,38 @@ def __init__(self, rest): self.__ably = rest self.__all: dict = OrderedDict() - def get(self, name, **kwargs): + @staticmethod + def _channel_options(options, kwargs) -> Optional[ChannelOptions]: + """What `get` was given - a ChannelOptions, a dict of its fields or keyword arguments - as a + ChannelOptions, or None if it was given no options""" + if options is None and kwargs: + return ChannelOptions(**kwargs) + if isinstance(options, dict): + return ChannelOptions.from_dict(options) + return options + + # RSN3 + def get(self, name, options: Optional[ChannelOptions] = None, **kwargs): + """Returns the channel called `name`, creating it if need be. + + Options given for an existing channel replace its options (RSN3c). + """ if isinstance(name, bytes): name = name.decode('ascii') + options = self._channel_options(options, kwargs) if name not in self.__all: - result = self.__all[name] = Channel(self.__ably, name, kwargs) + result = self.__all[name] = Channel(self.__ably, name, options) else: result = self.__all[name] - if len(kwargs) != 0: - result.options = kwargs + if options is not None: + result._set_options(options) return result def __getitem__(self, key): return self.get(key) - def __getattr__(self, name): - return self.get(name) - def __contains__(self, item): if isinstance(item, Channel): name = item.name diff --git a/ably/pubsub/liveobjects/realtimeobject.py b/ably/pubsub/liveobjects/realtimeobject.py index 62c4792d..da28c893 100644 --- a/ably/pubsub/liveobjects/realtimeobject.py +++ b/ably/pubsub/liveobjects/realtimeobject.py @@ -570,7 +570,7 @@ def _throw_if_missing_channel_mode(self, mode: ChannelMode) -> None: modes = self._channel.modes if self._channel.state == ChannelState.ATTACHED else None # RTO2a if not modes: - modes = (self._channel.options or {}).get('modes') or [] # RTO2b + modes = self._channel.options.modes or [] # RTO2b if mode not in modes: # RTO2a2, RTO2b2 raise AblyException(f'"{mode.name}" channel mode must be set for this operation', 400, 40024) diff --git a/ably/pubsub/realtime/channel.py b/ably/pubsub/realtime/channel.py index c4421fbe..1fd9e29c 100644 --- a/ably/pubsub/realtime/channel.py +++ b/ably/pubsub/realtime/channel.py @@ -71,7 +71,6 @@ def __init__(self, realtime: DefaultPubSubRealtimeClient, name: str, self.__channel_serial: str | None = None self.__retry_timer: Timer | None = None self.__error_reason: AblyException | None = None - self.__channel_options = channel_options or ChannelOptions() self.__params: dict[str, str] | None = None self.__modes: list[ChannelMode] = [] # Channel mode flags from ATTACHED message @@ -84,8 +83,7 @@ def __init__(self, realtime: DefaultPubSubRealtimeClient, name: str, # will be disrupted if the user called .off() to remove all listeners self.__internal_state_emitter = EventEmitter() - # Pass channel options as dictionary to parent Channel class - Channel.__init__(self, realtime, name, self.__channel_options.to_dict()) + Channel.__init__(self, realtime, name, channel_options) # Initialize presence for this channel @@ -97,22 +95,21 @@ def __init__(self, realtime: DefaultPubSubRealtimeClient, name: str, # RTL27: the LiveObjects entry point for this channel self.__object = RealtimeObject(self) + # RTL16 async def set_options(self, channel_options: ChannelOptions) -> None: - """Set channel options""" + """Replaces the channel's options, reattaching if their params or modes change. + + The options are replaced whole rather than merged, so options without a cipher turn + encryption off. + """ should_reattach = self.should_reattach_to_set_options(channel_options) - self.set_options_without_reattach(channel_options) + self._set_options(channel_options) if should_reattach: - self._attach_impl() + # RTL16a: through ATTACHING, so that the ATTACHED answering the ATTACH ends the wait + self._request_state(ChannelState.ATTACHING) state_change = await self.__internal_state_emitter.once_async() - if state_change.current in (ChannelState.SUSPENDED, ChannelState.FAILED): - raise state_change.reason - - def set_options_without_reattach(self, channel_options: ChannelOptions) -> None: - """Internal method""" - self.__channel_options = channel_options - # Update parent class options - self.options = channel_options.to_dict() + self.__raise_unless_attached(state_change) # RTL4 async def attach(self) -> None: @@ -124,23 +121,15 @@ async def attach(self) -> None: Raises ------ AblyException - If unable to attach channel - """ - await self._attach() - - async def _attach(self) -> ChannelStateChange | None: - """The attach procedure `attach` runs, returning the state change it ended with, or None if - the channel was already ATTACHED. - - Raises where `attach` does. An attach that ends in a state other than ATTACHED, - SUSPENDED or FAILED, such as DETACHED when the connection closes first, returns that - state change. + If unable to attach channel, including when the channel moves to DETACHED instead, as + it does when the connection closes first (RTL4d). It carries the reason for the state + the channel moved to, else code 90001 """ log.info(f'RealtimeChannel.attach() called, channel = {self.name}') # RTL4a - if channel is attached do nothing if self.state == ChannelState.ATTACHED: - return None + return self.__error_reason = None @@ -161,10 +150,23 @@ async def _attach(self) -> ChannelStateChange | None: self._request_state(ChannelState.ATTACHING) state_change = await self.__internal_state_emitter.once_async() + self.__raise_unless_attached(state_change) - if state_change.current in (ChannelState.SUSPENDED, ChannelState.FAILED): - raise state_change.reason - return state_change + def __raise_unless_attached(self, state_change: ChannelStateChange) -> None: + # RTL4d: any state but ATTACHED fails the attach, DETACHED included (RTL3b) + if state_change.current != ChannelState.ATTACHED: + raise state_change.reason or AblyException( + f'Unable to attach channel; channel state = {state_change.current.value}, ' + f'connection state = {self.__realtime.connection.state.value}', 400, 90001) + + def _attach_implicitly(self) -> None: + """Starts an attach nobody awaits, logging its failure rather than leaving it unretrieved.""" + def on_done(task: asyncio.Task) -> None: + if not task.cancelled() and task.exception() is not None: + log.warning(f'RealtimeChannel implicit attach failed; channel = {self.name}, ' + f'error = {task.exception()}') + + asyncio.create_task(self.attach()).add_done_callback(on_done) def _attach_impl(self): log.debug("RealtimeChannel.attach_impl(): sending ATTACH protocol message") @@ -172,7 +174,7 @@ def _attach_impl(self): # RTL4c attach_msg = { "action": ProtocolMessageAction.ATTACH, - "params": self.__channel_options.params, + "params": self.options.params, "channel": self.name, } @@ -203,14 +205,9 @@ async def _ensure_active(self) -> None: if self.state == ChannelState.FAILED: raise AblyException(f"Channel operation failed as channel state is {self.state.value}", 400, 90001) - # RTL33b - state_change = await self._attach() - if state_change is not None and state_change.current != ChannelState.ATTACHED: - # RTL33b1, RTL4d: the attach ended without the channel attaching, as when the connection - # closes or `detach` is called first - raise state_change.reason or AblyException( - f'Unable to attach channel; channel state = {state_change.current.value}, ' - f'connection state = {self.__realtime.connection.state.value}', 400, 90001) + # RTL33b, RTL33b1: `attach` raises unless the channel attaches, as when the connection closes + # or `detach` is called first + await self.attach() # RTL5 async def detach(self) -> None: @@ -228,29 +225,25 @@ async def detach(self) -> None: log.info(f'RealtimeChannel.detach() called, channel = {self.name}') - # RTL5g, RTL5b - raise exception if state invalid - if self.__realtime.connection.state in [ConnectionState.CLOSING, ConnectionState.FAILED]: - raise AblyException( - message=f"Unable to detach; channel state = {self.state}", - code=90001, - status_code=400 - ) - # RTL5a - if channel already detached do nothing if self.state in [ChannelState.INITIALIZED, ChannelState.DETACHED]: return + # RTL5b + if self.state == ChannelState.FAILED: + raise AblyException("Unable to detach; channel state = failed", 400, 90001) + + # RTL5j if self.state == ChannelState.SUSPENDED: self._notify_state(ChannelState.DETACHED) return - elif self.state == ChannelState.FAILED: - raise AblyException("Unable to detach; channel state = failed", 400, 90001) - else: - self._request_state(ChannelState.DETACHING) - # RTL5h - wait for pending connection - if self.__realtime.connection.state == ConnectionState.CONNECTING: - self.__realtime.connect() + # RTL5l: with no connection to send a DETACH on, the channel detaches at once + if self.__realtime.connection.state != ConnectionState.CONNECTED: + self._notify_state(ChannelState.DETACHED) + return + + self._request_state(ChannelState.DETACHING) state_change = await self.__internal_state_emitter.once_async() new_state = state_change.current @@ -946,9 +939,13 @@ def __on_retry_timer_expire(self) -> None: def should_reattach_to_set_options(self, new_options: ChannelOptions) -> bool: """Internal method""" + # RTL16a: only params or modes that differ from the current ones call for a reattach, and + # only while attaching or attached; the cipher never reaches the server if self.state != ChannelState.ATTACHING and self.state != ChannelState.ATTACHED: return False - return self.__channel_options != new_options + if new_options.params is not None and new_options.params != (self.options.params or {}): + return True + return new_options.modes is not None and set(new_options.modes) != set(self.options.modes or []) # RTL23 @property @@ -1016,7 +1013,7 @@ def _start_decode_failure_recovery(self, error: AblyException) -> None: self._check_pending_state() def _encode_flags(self) -> int | None: - if not self.__channel_options.modes and not self.__attach_resume: + if not self.options.modes and not self.__attach_resume: return None flags = 0 @@ -1024,8 +1021,8 @@ def _encode_flags(self) -> int | None: if self.__attach_resume: flags |= Flag.ATTACH_RESUME - if self.__channel_options.modes: - flags |= encode_channel_mode(self.__channel_options.modes) + if self.options.modes: + flags |= encode_channel_mode(self.options.modes) return flags @@ -1051,30 +1048,33 @@ def get(self, name: str, options: ChannelOptions | None = None, **kwargs) -> Rea name: str Channel name options: ChannelOptions or dict, optional - Channel options for the channel + Channel options for the channel. For an existing channel they replace its options (RTS3c) **kwargs: Additional keyword arguments to create ChannelOptions (e.g., cipher, params) + + Raises + ------ + AblyException + With code 40000 if the options for an existing channel would make it reattach (RTS3c1). + Use RealtimeChannel.set_options() for those. """ - # Convert kwargs to ChannelOptions if provided - if kwargs and not options: - options = ChannelOptions(**kwargs) - elif options and isinstance(options, dict): - options = ChannelOptions.from_dict(options) + options = self._channel_options(options, kwargs) if name not in self.__all: channel = self.__all[name] = RealtimeChannel(self.__ably, name, options) else: channel = self.__all[name] - # Update options if channel is not attached or currently attaching - if options and channel.should_reattach_to_set_options(options): - raise AblyException( - 'Channels.get() cannot be used to set channel options that would cause the channel to ' - 'reattach. Please, use RealtimeChannel.setOptions() instead.', - 400, - 40000 - ) - elif options: - channel.set_options_without_reattach(options) + if options is not None: + # RTS3c1 + if channel.should_reattach_to_set_options(options): + raise AblyException( + 'Channels.get() cannot be used to set channel options that would cause the channel to ' + 'reattach. Please, use RealtimeChannel.set_options() instead.', + 400, + 40000 + ) + # RTS3c + channel._set_options(options) return channel # RTS4 @@ -1160,7 +1160,7 @@ def _on_connected(self) -> None: if channel.state == ChannelState.ATTACHING or channel.state == ChannelState.DETACHING: channel._check_pending_state() elif channel.state == ChannelState.SUSPENDED: - asyncio.create_task(channel.attach()) + channel._attach_implicitly() elif channel.state == ChannelState.ATTACHED: channel._request_state(ChannelState.ATTACHING) diff --git a/ably/pubsub/realtime/connectionmanager.py b/ably/pubsub/realtime/connectionmanager.py index bd9a7dcd..cf72e389 100644 --- a/ably/pubsub/realtime/connectionmanager.py +++ b/ably/pubsub/realtime/connectionmanager.py @@ -127,6 +127,8 @@ def __init__(self, id: str, clock: Clock): self.id = id self.clock = clock self.future: asyncio.Future[None] = asyncio.get_running_loop().create_future() + # done once the heartbeat has gone out, which is when the ping's timeout starts (RTN13c) + self.sent: asyncio.Future[None] = asyncio.get_running_loop().create_future() self.start_time: float = clock.monotonic_ms() @property @@ -364,17 +366,25 @@ def fail_queued_messages(self, err) -> None: async def ping(self) -> float: # RTN13b - if self.__state not in (ConnectionState.CONNECTED, ConnectionState.CONNECTING): + if self.__state not in ( + ConnectionState.CONNECTED, + ConnectionState.CONNECTING, + ConnectionState.DISCONNECTED, + ): raise AblyException("Cannot send ping request. Calling ping in invalid state", 400, 40000) # RTN13e: the id tells this ping's echo apart from server heartbeats and other pings pending_ping = PendingPing(get_random_id(), self.clock) self.__pending_pings[pending_ping.id] = pending_ping try: - # RTN13d: while connecting, the heartbeat goes out once the connection is established if self.__state == ConnectionState.CONNECTED: await self.__send_heartbeat(pending_ping) - # RTN13c + else: + # RTN13d: the heartbeat goes out once the connection is established, unless a state + # that fails the ping (RTN13b) comes first + await asyncio.wait( + (pending_ping.sent, pending_ping.future), return_when=asyncio.FIRST_COMPLETED) + # RTN13c: counted from the heartbeat going out, not from the call await asyncio.wait_for(pending_ping.future, self.__timeout_in_secs) except asyncio.TimeoutError: raise AblyException("Timeout waiting for ping response", 504, 50003) from None @@ -384,6 +394,8 @@ async def ping(self) -> float: async def __send_heartbeat(self, pending_ping: PendingPing) -> None: pending_ping.start_time = self.clock.monotonic_ms() + if not pending_ping.sent.done(): + pending_ping.sent.set_result(None) try: await self.send_protocol_message(pending_ping.message) except Exception as error: diff --git a/ably/pubsub/realtime/presence.py b/ably/pubsub/realtime/presence.py index 2c87f50e..73edfa4f 100644 --- a/ably/pubsub/realtime/presence.py +++ b/ably/pubsub/realtime/presence.py @@ -260,7 +260,7 @@ async def _enter_or_update_client( return await self._send_presence([wire_msg]) elif channel.state in [ChannelState.INITIALIZED, ChannelState.DETACHED]: # RTP8d: Implicitly attach - asyncio.create_task(channel.attach()) + channel._attach_implicitly() # Queue the message return await self._queue_presence(wire_msg) elif channel.state == ChannelState.ATTACHING: diff --git a/ably/pubsub/types/presence.py b/ably/pubsub/types/presence.py index 0043f459..5084ff0e 100644 --- a/ably/pubsub/types/presence.py +++ b/ably/pubsub/types/presence.py @@ -206,10 +206,10 @@ def from_encoded_array(encoded_array, cipher=None, context=None): class Presence: def __init__(self, channel): - self.__base_path = f'/channels/{parse.quote_plus(channel.name)}/' + self.__base_path = channel.base_path self.__binary = channel.ably.options.use_binary_protocol self.__http = channel.ably.http - self.__cipher = channel.cipher + self.__channel = channel def _path_with_qs(self, rel_path, qs=None): path = rel_path @@ -225,7 +225,7 @@ async def get(self, limit=None): qs['limit'] = limit path = self._path_with_qs(self.__base_path + 'presence', qs) - presence_handler = make_presence_response_handler(self.__cipher) + presence_handler = make_presence_response_handler(self.__channel.cipher) return await PaginatedResult.paginated_query( self.__http, url=path, response_processor=presence_handler) @@ -247,7 +247,7 @@ async def history(self, limit=None, direction=None, start=None, end=None): path = self._path_with_qs(self.__base_path + 'presence/history', qs) - presence_handler = make_presence_response_handler(self.__cipher) + presence_handler = make_presence_response_handler(self.__channel.cipher) return await PaginatedResult.paginated_query( self.__http, url=path, response_processor=presence_handler) diff --git a/test/ably/http/httpauth_test.py b/test/ably/http/httpauth_test.py index 3f793565..5857536b 100644 --- a/test/ably/http/httpauth_test.py +++ b/test/ably/http/httpauth_test.py @@ -186,11 +186,11 @@ async def test_if_authorize_changes_auth_mechanism_to_token(self): @dont_vary_protocol async def test_authorize_always_creates_new_token(self): await self.ably.auth.authorize({'capability': {'test': ['publish']}}) - await self.ably.channels.test.publish('event', 'data') + await self.ably.channels.get('test').publish('event', 'data') await self.ably.auth.authorize({'capability': {'test': ['subscribe']}}) with pytest.raises(AblyAuthException): - await self.ably.channels.test.publish('event', 'data') + await self.ably.channels.get('test').publish('event', 'data') async def test_authorize_create_new_token_if_expired(self): token = await self.ably.auth.authorize() @@ -223,7 +223,7 @@ async def test_with_token_str_https(self): token = token.token ably = await TestApp.get_ably_rest(key=None, token=token, tls=True, use_binary_protocol=self.use_binary_protocol) - await ably.channels.test_auth_with_token_str.publish('event', 'foo_bar') + await ably.channels.get('test_auth_with_token_str').publish('event', 'foo_bar') await ably.close() async def test_with_token_str_http(self): @@ -231,7 +231,7 @@ async def test_with_token_str_http(self): token = token.token ably = await TestApp.get_ably_rest(key=None, token=token, tls=False, use_binary_protocol=self.use_binary_protocol) - await ably.channels.test_auth_with_token_str.publish('event', 'foo_bar') + await ably.channels.get('test_auth_with_token_str').publish('event', 'foo_bar') await ably.close() async def test_if_default_client_id_is_used(self): diff --git a/test/ably/http/httppresence_test.py b/test/ably/http/httppresence_test.py index 187c3ab4..4ba6cf75 100644 --- a/test/ably/http/httppresence_test.py +++ b/test/ably/http/httppresence_test.py @@ -81,7 +81,7 @@ def presence_mock_url(self): kwargs['port_sufix'] = '' else: kwargs['port_sufix'] = ':' + str(port) - url = '{scheme}://{host}{port_sufix}/channels/persisted%3Apresence_fixtures/presence' + url = '{scheme}://{host}{port_sufix}/channels/persisted:presence_fixtures/presence' return url.format(**kwargs) def history_mock_url(self): @@ -94,7 +94,7 @@ def history_mock_url(self): kwargs['port_sufix'] = '' else: kwargs['port_sufix'] = ':' + str(port) - url = '{scheme}://{host}{port_sufix}/channels/persisted%3Apresence_fixtures/presence/history' + url = '{scheme}://{host}{port_sufix}/channels/persisted:presence_fixtures/presence/history' return url.format(**kwargs) @dont_vary_protocol diff --git a/test/unit/channel_options_test.py b/test/unit/channel_options_test.py new file mode 100644 index 00000000..097ea5ea --- /dev/null +++ b/test/unit/channel_options_test.py @@ -0,0 +1,88 @@ +from ably.pubsub.server import ( + ChannelMode, + ChannelOptions, + create_http_client, + create_realtime_client, + generate_random_key, + get_default_params, +) +from ably.pubsub.types.message import Message + + +def cipher_options(**kwargs): + return ChannelOptions(cipher=get_default_params({'key': generate_random_key()}), **kwargs) + + +def is_encrypted(channel): + message = Message('name', 'data') + if channel.cipher: + message.encrypt(channel.cipher) + return 'cipher' in message.as_dict().get('encoding', '') + + +async def test_http_set_options_without_a_cipher_turns_encryption_off(): + ably = create_http_client(token='foo') + channel = ably.channels.get('test', cipher_options()) + assert is_encrypted(channel) + + await channel.set_options(ChannelOptions()) + + assert channel.options.cipher is None + assert channel.cipher is None + assert not is_encrypted(channel) + await ably.close() + + +async def test_http_get_with_options_replaces_the_cipher(): + ably = create_http_client(token='foo') + channel = ably.channels.get('test', cipher_options()) + + assert ably.channels.get('test', ChannelOptions()) is channel + + assert channel.cipher is None + await ably.close() + + +async def test_http_get_without_options_keeps_the_cipher(): + ably = create_http_client(token='foo') + options = cipher_options() + channel = ably.channels.get('test', options) + + assert ably.channels.get('test') is channel + + assert channel.options is options + assert is_encrypted(channel) + await ably.close() + + +async def test_realtime_set_options_without_a_cipher_turns_encryption_off(): + ably = create_realtime_client(key='fake.key:secret', auto_connect=False) + channel = ably.channels.get('test', cipher_options(params={'rewind': '1'})) + + await channel.set_options(ChannelOptions(params={'rewind': '1'})) + + assert channel.options.params == {'rewind': '1'} + assert channel.options.cipher is None + assert not is_encrypted(channel) + await ably.close() + + +async def test_realtime_set_options_replaces_rather_than_merges(): + ably = create_realtime_client(key='fake.key:secret', auto_connect=False) + channel = ably.channels.get('test', ChannelOptions(params={'rewind': '1'}, modes=[ChannelMode.SUBSCRIBE])) + + await channel.set_options(ChannelOptions(params={'delta': 'vcdiff'})) + + assert channel.options.params == {'delta': 'vcdiff'} + assert channel.options.modes is None + await ably.close() + + +async def test_realtime_get_with_options_replaces_the_cipher(): + ably = create_realtime_client(key='fake.key:secret', auto_connect=False) + channel = ably.channels.get('test', cipher_options()) + + assert ably.channels.get('test', ChannelOptions()) is channel + + assert not is_encrypted(channel) + await ably.close() diff --git a/test/uts/deviations.md b/test/uts/deviations.md index cc20db47..936d3b54 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**: 1295 Test IDs -(1304 tests, 1429 cases) pass, 150 (150 tests, 152 cases) are gated behind +Of **1472 Test IDs, derived as 1481 tests and run as 1608 pytest cases**: 1303 Test IDs +(1312 tests, 1437 cases) pass, 142 (142 tests, 144 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 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. +within each. Of the gated Test IDs 64 are REST and 78 realtime, which is 65 REST cases +and 79 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 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 +The 131 gated Test IDs that record SDK non-compliance — 131 tests, 133 cases — reduce to +**55 distinct root causes**, 18 on the REST side and 37 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 **60 SDK root causes** in all. The +cannot discriminate (RTP18a), so the file carries **56 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 @@ -1054,7 +1054,6 @@ option is gone, after RTN16d has done its first phase against the sandbox throug | 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) | | 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 | | RTP12, RTP12a, RTP12c | `RealtimePresence#history`. The realtime *channel* does delegate `history` to the REST implementation; only the presence object is missing it | 2 | | RTN23c1, RTN23c2 | PING/PONG. `ProtocolMessageAction` stops at `ANNOTATION` (21), so PING (22) and PONG (23) are not modelled and action 22 matches no branch of `on_protocol_message` (`websockettransport.py:37-59`, `:143-199`). The message is counted as activity and discarded | 2 | @@ -1254,83 +1253,9 @@ that the first merely permitted. the adapted RTN25 unit test round to the specification's option A, so that test's adaptation goes with the fix. -#### `ping()` rejects DISCONNECTED instead of deferring, and charges the wait to the caller — 3 tests - -**Spec points:** RTN13b, RTN13c, RTN13d. - -Two defects in the same method. - -- RTN13b errors only for INITIALIZED, SUSPENDED, CLOSING, CLOSED and FAILED, and RTN13d - defers a ping requested while CONNECTING *or DISCONNECTED* until the connection is - CONNECTED. `ConnectionManager.ping` (`connectionmanager.py:362`) admits only CONNECTED - and CONNECTING and raises `AblyException("Cannot send ping request. Calling ping in - invalid state", 400, 40000)` for DISCONNECTED. Note that - `deferred-ping-error-suspended-5` would otherwise pass for the wrong reason: its only - stated assertion is that an error arrives, and one does — just immediately, rather than - when the connection suspends. -- RTN13c with RTN13d requires a deferred ping's timeout to run "from when the HEARTBEAT is - actually sent, not when `ping()` is called". `ping()` enters - `asyncio.wait_for(pending_ping.future, self.__timeout_in_secs)` (`:375`) as soon as it is - called, so the whole CONNECTING period is charged against it and a ping requested while - connecting can expire before its HEARTBEAT has gone out. - -**Tests affected:** `test_rtn13d_ping_deferred_disconnected` and -`test_rtn13b_deferred_ping_error_suspended` fail with "the ping errored instead of waiting -for the connection"; `test_rtn13c_deferred_ping_timeout` fails with -`assert 0.096… >= (0.4 * 0.9)` — the error arrived 96 ms after CONNECTED where a 400 ms -`realtimeRequestTimeout` should have run from that point. - -**Status:** open bug. - ### Channels -#### `detach()` never returns when the connection is not CONNECTED — 2 tests - -**Spec point:** RTL5l. - -When the connection is in any state other than CONNECTED and no earlier channel-state -condition applies, the channel must transition immediately to DETACHED. Instead `detach()` -requests DETACHING, `_check_pending_state()` returns without sending anything because the -connection is not CONNECTED, and `detach()` then awaits the internal state emitter -(`channel.py:212`) for a transition nothing will produce. **The coroutine never returns** -and the channel is left in DETACHING. - -**Tests affected:** `test_rtl5l_detach_not_connected_immediate` and -`test_rtl5l_detach_attached_when_disconnected`, each with a one-second `asyncio.wait_for` -so the hang is reported as a failure rather than a stuck run. Both fail with -`asyncio.exceptions.TimeoutError`. In the second, the assertions that set the scene — the -connection settling in DISCONNECTED with the channel still ATTACHED — pass first, so the -failure is unambiguously the detach. - -**Status:** open bug. A hang, not a wrong value. - -#### `set_options()` never returns for an already-ATTACHED channel — 1 test - -**Spec point:** RTL16a. - -When `params` or `modes` are supplied to `setOptions` on an attached channel, the channel -must reattach, pass through ATTACHING, return to ATTACHED, and `setOptions` must resolve. -Instead it hangs, and the root cause is a single missing call. - -`set_options` (`channel.py:93-102`) calls `_attach_impl()` and then awaits -`self.__internal_state_emitter.once_async()`. `_attach_impl()` sends the ATTACH **without -going through `_request_state(ChannelState.ATTACHING)`**, so the channel is still ATTACHED -when the server's ATTACHED arrives. `_on_message` therefore takes the RTL12 branch -(`:722-726`), which emits `update` on the *public* emitter and returns. The internal state -emitter is written only by `_notify_state` (`:821`), which that branch never reaches, so -the await has nothing to wake it. Both halves of the defect — no ATTACHING transition, and -no internal event — follow from the one missing `_request_state`. - -Measured: the second ATTACH is sent, the server's ATTACHED is received, the options *are* -stored, and `asyncio.wait_for(channel.set_options(...), 1.0)` raises `TimeoutError`. - -**Tests affected:** `test_rtl16a_triggers_reattach`. It also constrains -`test_rtl4c1_includes_channel_serial` and `test_rtl4j_attach_resume_flag_not_set`, which run -`set_options` as a task and cancel it — recorded under Adapted Tests. - -**Status:** open bug. A hang, not a wrong value. - -#### A server-initiated DETACHED discards its error, and the pending call then raises `TypeError` — 3 tests +#### A server-initiated DETACHED discards its error, which `detach` then raises as `TypeError` — 3 tests **Spec points:** RTL24, RTL4c, with RTL13a and RTL13b. @@ -1341,23 +1266,23 @@ An attach rejected by a DETACHED carrying an `ErrorInfo` must fail with that err `self._notify_state(ChannelState.SUSPENDED)` (`channel.py:735`), passing **no reason**, so the message's error is dropped. The same shape applies to RTL13a's `_request_state(ATTACHING)` and RTL13b's `_notify_state(SUSPENDED)`. -2. `attach()` then reaches `raise state_change.reason` (`channel.py:150`) with `reason` - `None`, which Python reports as `TypeError: exceptions must derive from BaseException` - rather than as an `AblyException`. The same unguarded `raise` is at `:102` in - `set_options` and `:219` in `detach`. +2. `attach()` and `set_options` then raise their 90001 fallback, having no reason to + raise, but `detach` reaches `raise state_change.reason` with `reason` `None`, which + Python reports as `TypeError: exceptions must derive from BaseException` rather than as + an `AblyException`. **Tests affected:** `test_rtl24_error_reason_attach_failure` and -`test_rtl4c_error_cleared_on_attach`, both `TypeError: exceptions must derive from -BaseException` at `channel.py:150`, and `test_rtl13b_attaching_detached_to_suspended`, +`test_rtl4c_error_cleared_on_attach`, both failing on `error_reason` being `None` once the +attach has raised, and `test_rtl13b_attaching_detached_to_suspended`, which reads the dropped error off the SUSPENDED state change and so falls one step earlier. The clearing half of RTL4c is still covered by `test_rtl4c_error_cleared_preserved_detach`, which sets the error with an ERROR message instead and passes. One adapted test — `test_rtl13a_attached_reattach_triggered` — asserts `reason is None`, and will need revisiting once the reason is carried through. -**Status:** open bug, in two parts. The `raise None` is worth fixing on its own even before -the reason is plumbed through: a missing reason should give an `AblyException`, not a -`TypeError`. +**Status:** open bug, in two parts. The `raise None` is worth fixing in `detach` on its +own, as it is in `attach` and `set_options`, even before the reason is plumbed through: a +missing reason should give an `AblyException`, not a `TypeError`. #### An ATTACHED received while DETACHING or DETACHED is ignored — 2 tests @@ -1869,8 +1794,6 @@ through an internal object to get at a value the specification makes public. | RTP17, RTP17h | a distinct `LocalPresenceMap` type keyed by `clientId` | `RealtimePresence._my_members`, the same `PresenceMap` class built with `member_key_fn=lambda msg: msg.client_id` (`presence.py:79-81`). The keying requirement is met; the newness check is not, and is gated above | all of `local_presence_map_test.py` | | 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, 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 | **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. @@ -2072,24 +1995,6 @@ POSTed to `/keys/{keyName}/revokeTokens` directly, reproduced three times. RSA4a does not say which state a token error observed mid-connection should be reported in, and other SDKs may report DISCONNECTED first and fail afterwards. -### A pending `attach()` resolves, rather than failing, when the connection closes - -**Spec points:** RTL3b, RTL4d. - -RTL3b moves an ATTACHING channel to DETACHED when the connection closes, and RTL4d has the -attach's callback invoked with an `ErrorInfo` "in all other cases" than ATTACHED. -`channel_connection_state.md` spells it `AWAIT attach_future FAILS WITH error`. The RTL3b -transition is correct; the pending `attach()` then returns `None`, because `attach()` ends -with `if state_change.current in (SUSPENDED, FAILED): raise state_change.reason` -(`channel.py:148-150`) and DETACHED is in neither, so the coroutine falls through as a -success. - -**Tests affected:** `test_rtl3b_closed_attaching_to_detached` asserts `await attach_future is -None`, and makes every other assertion the specification does. It fails if the SDK starts -raising, so it does guard the behaviour. - -**Status:** open bug. - ### An attach requested while DETACHING pre-empts the detach **Spec point:** RTL4h. @@ -2154,7 +2059,7 @@ adapted. | Spec points | Specification | ably-python | Tests | |---|---|---|---| | RTL13a | the ATTACHING state change triggered by a server-initiated DETACHED carries that message's `error` as its `reason` | `_on_message` discards the error and calls `_request_state(ATTACHING)` with no reason, so `reason` is null. Gated in its own right under RTL24/RTL4c above; this test asserts `reason is None` | `test_rtl13a_attached_reattach_triggered` | -| RTP16c | answering an ATTACH with a DETACHED puts the channel in DETACHED, and a presence operation from there errors | the channel lands in SUSPENDED (`channel.py:735`) and `attach()` raises `TypeError` on the null reason. RTP16c is about the presence error, which does arrive with 90001, so the test accepts either channel state and either attach error and turns on the presence operation alone | `test_rtp16c_presence_errors_other_states` | +| RTP16c | answering an ATTACH with a DETACHED puts the channel in DETACHED, and a presence operation from there errors | the channel lands in SUSPENDED (`channel.py:735`) and `attach()` raises its 90001 fallback for want of a reason. RTP16c is about the presence error, which does arrive with 90001, so the test accepts either channel state and either attach error and turns on the presence operation alone | `test_rtp16c_presence_errors_other_states` | | RTL7g | `channel.subscribe(listener)` registers the listener even when the implicit attach is rejected | the listener *is* registered (`channel.py:279-282`), and then `subscribe` awaits `attach()`, which re-raises the failure (`:150`). So `await channel.subscribe(...)` raises where the specification's fire-and-forget call returns. The RTL7g requirement itself holds | `test_rtl7g_listener_registered_attach_fails`, `test_rtl7g_no_attach_when_attaching` | | RTL10b | an `AblyException` when `untilAttach` is used on an unattached channel | an `AblyException`, but for the wrong reason: `history` takes no `until_attach`, and `catch_all` wraps the `TypeError` as `50000 500 Unexpected exception` whatever the channel's state. The test also asserts that no HTTP request was made | `test_rtl10b_errors_when_not_attached` | | RTL19b, RTL19c, RTL20, RTL21, PC3 | a vcdiff-decoded payload equals a string literal | a `bytearray`. `EncodeDataMixin.decode` (`mixins.py:106`) leaves the vcdiff result binary because no further encoding step turns it back into text, which is correct — the specification's own `RTL19b/json-wire-form-base-1`, which does carry `utf-8/vcdiff`, receives a string. Five tests assert `b'second message'` where the specification writes the string | 5 in `channel_delta_decoding_test.py` | @@ -2169,7 +2074,7 @@ pseudocode is not mistaken for non-compliance. | Subject | Note | |---|---| | DISCONNECTED as a restable state | RTN15a retries a drop from CONNECTED through `loop.call_soon` (`connectionmanager.py:668`) with no time passing, so DISCONNECTED is left within the same turn of the event loop and a listener registered afterwards never sees it. The heartbeat specification says so itself, under "Verifying Transient States"; tests record the whole sequence with `connection.on(...)` and assert `CONTAINS_IN_ORDER`. Where a test needs the connection to *rest* in DISCONNECTED it fails the immediate retry and raises `disconnected_retry_timeout`. Correct RTN15a behaviour | -| `Channels.__getattr__` | `ably/rest/channel.py:408` answers **any** unknown attribute with `self.get(name)`, so reading an attribute that does not exist creates a channel named after it and never raises — `hasattr(channels, 'get_derived')` is `True`, and mutates the collection. Consequences: a typo becomes a phantom channel, and `client.channels.get_derived(...)` fails with `TypeError: 'RealtimeChannel' object is not callable` rather than `AttributeError`. No test loses coverage to it, because no derived test names an attribute the collection does not define; it is recorded because it silently changes what a probing test measures. `Channels.__iter__` is annotated `Iterator[str]` but yields `Channel` objects, which is a second, smaller instance | +| `Channels.__iter__` | annotated `Iterator[str]`, but yields the `Channel` objects rather than their names | | `RealtimeChannel.publish()` | positional-only (`*args`, `channel.py:342`). The keyword form the specifications write raises `ValueError`, although `RestChannel.publish()` accepts it. Every test uses the positional form | | ACK `res` | every ACK in the specifications is written `res: { "serials": [...] }`, a single object. `WebSocketTransport` (`websockettransport.py:191-193`) reads `res` as a list, one entry per acknowledged ProtocolMessage, which matches the protocol definition. The specification's single object is shorthand for the one-message case | | `subscribe` before `connect` | all eight `message_field_population.md` tests subscribe in their setup, before `client.connect()`. `subscribe` awaits `attach()`, which raises 90001 unless the connection is CONNECTING, CONNECTED or DISCONNECTED, so the derived tests connect first. Nothing they assert depends on the order | @@ -2186,9 +2091,7 @@ pseudocode is not mistaken for non-compliance. | Spec points | Specification | ably-python | Status | |---|---|---|---| -| RSL2 | A space in a channel name is `%20` | `+`, from `parse.quote_plus` in `Channel.__init__`. `quote_plus` is form encoding, and a `+` in a URL *path* is a literal plus, so the name reaching the server is altered | Open bug, and a genuine correctness issue | -| RSL8 | `Channel#status` URI-encodes the channel id | `status()` interpolates the name with no escaping at all. `a/b` addresses the wrong resource, `a?b` truncates the name into a query string, `a#b` becomes a fragment | Open bug | -| RSL2, RSL11b, RSL15b | `:` is `%3A` | Left literal, from `safe=':'`. RFC 3986 allows `:` in a path segment and Ably uses it for namespaces, so the server receives the same value | Intentional | +| RSL2, RSL8, RSL11b, RSL15b | `:` is `%3A` | Left literal, from `safe=':'`. RFC 3986 allows `:` in a path segment and Ably uses it for namespaces, so the server receives the same value | Intentional | | RTC12 / RSC1, RSC1a, RSC1c | A string constructor argument is an API key when it contains `:` and a token when it does not | `AblyRest.__init__` treats its first positional argument as a key unconditionally and hands it to `AuthOptions.set_key`, which requires exactly two colon-separated parts, so a token string raises 40101/401 "key of not len 2 parameters". A token is supplied through the separate `token` or `token_details` arguments. The empty-string case is compliant | Intentional / SDK-wide: the constructor takes credentials as distinct named arguments and has no string-sniffing path to restore | | 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 | @@ -2622,22 +2525,6 @@ connection fails. `test/uts/realtime/unit/channels/channel_publish_pending_test.py -k rtn7e` `test/uts/realtime/integration/proxy/connection_resume_test.py -k rtn15j` -**1.3 `detach()` never returns when the connection is not CONNECTED.** RTL5l. -RTL5l requires an immediate transition to DETACHED. `detach()` requests DETACHING, -`_check_pending_state()` sends nothing because the connection is not CONNECTED, and -`detach()` then awaits the internal state emitter (`channel.py:212`) for a transition -nothing will produce. The coroutine never returns and the channel is stuck in DETACHING. -`test/uts/realtime/unit/channels/channel_detach_test.py -k rtl5l` - -**1.4 `set_options()` never returns for an already-ATTACHED channel.** RTL16a. -`_attach_impl()` (`channel.py:99`) sends the ATTACH without going through -`_request_state(ATTACHING)`, so the channel is still ATTACHED when the server's ATTACHED -arrives; `_on_message` takes the RTL12 branch (`:722-726`), which emits `update` on the -*public* emitter, while `set_options` awaits the *internal* one, written only by -`_notify_state`. The options are stored; the call never resolves. Adding the -`_request_state` fixes both halves. -`test/uts/realtime/unit/channels/channel_options_test.py -k rtl16a` - **1.5 A presence action on a DETACHED channel re-attaches and then hangs.** RTL11, RTP8g. Both require an immediate error with nothing sent. `_enter_or_update_client` groups DETACHED with INITIALIZED (`presence.py:258-264`), starts an implicit attach and queues the message, @@ -2648,8 +2535,8 @@ have the grouping, so the SDK is inconsistent with itself. ### Tier 2 — the wrong error, or an error where there should be none **2.1 `raise state_change.reason` raises `TypeError` when the reason is `None`.** RTL24, -RTL4c, RTL13b, RTP16c. Three sites — `channel.py:102` (`set_options`), `:150` (`attach`), -`:219` (`detach`) — raise whatever the state change carries, and the state change often +RTL4c, RTL13b, RTP16c. `detach` raises whatever the state change carries, where `attach` +and `set_options` fall back to a 90001, and the state change often carries nothing, because a server-initiated DETACHED's error is discarded at `:735`, `_request_state(ATTACHING)` and `_notify_state(SUSPENDED)` being called with no reason. The caller gets `TypeError: exceptions must derive from BaseException` instead of an @@ -2674,21 +2561,6 @@ The registry needs an `(event, listener)` key holding a list per key. `test/uts/realtime/unit/channels/channel_subscribe_test.py -k rtl8b` `test/uts/realtime/unit/presence/realtime_presence_subscribe_test.py -k rtp7b` -**2.3 `Channels.__getattr__` answers any unknown attribute by creating a channel.** -`ably/rest/channel.py:408` is `return self.get(name)`, so `hasattr(client.channels, 'x')` -is always `True` **and creates a channel called `x`**. A typo becomes a phantom channel, -`client.channels.get_derived(...)` raises `TypeError: 'RealtimeChannel' object is not -callable` rather than `AttributeError`, and any code that probes the collection mutates it. -No derived test loses coverage to it, because none names an attribute the collection does -not define — that constraint is itself the evidence. `Channels.__iter__` is annotated -`Iterator[str]` and yields `Channel` objects, which is the same carelessness one size down. -No gated test; reproduce with: -```python -len(client.channels) # 0 -hasattr(client.channels, 'foo') # True -len(client.channels) # 1 -``` - **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 @@ -2830,13 +2702,6 @@ NACK as `reason`, where the spec requires `resumed` true and a 91004 naming the the NACK as `cause`. Local to one method. `test/uts/realtime/unit/presence/realtime_presence_reentry_test.py -k rtp17e` -**3.14 `ping()` rejects DISCONNECTED, and charges the connect wait to the caller's timeout.** -RTN13b, RTN13c, RTN13d. `ConnectionManager.ping` (`:362`) admits only CONNECTED and -CONNECTING where RTN13d requires a ping from DISCONNECTED to be deferred; and `:375` enters -`asyncio.wait_for` as soon as `ping()` is called, so a ping requested while CONNECTING can -expire before its HEARTBEAT has gone out. -`test/uts/realtime/unit/connection/connection_ping_test.py -k "rtn13b_deferred or rtn13c or rtn13d"` - **3.15 A failed RTN22 reauth leaves no trace on the connection.** RSA4c1, RSA4c3. `websockettransport.py:170-175` awaits `auth.authorize()` inside a bare `except Exception` that only logs, so nothing reaches `on_error_from_authorize` and `errorReason` stays as it @@ -3517,8 +3382,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 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 +it. As of this writing that is 144 failures, 1585 passes and 27 skips with the variable +set, and 171 skips and 1585 passes without it, the 1585 being 1437 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/realtime/unit/channels/channel_attach_test.py b/test/uts/realtime/unit/channels/channel_attach_test.py index d59519d9..4401c58d 100644 --- a/test/uts/realtime/unit/channels/channel_attach_test.py +++ b/test/uts/realtime/unit/channels/channel_attach_test.py @@ -427,7 +427,7 @@ def on_message_from_client(msg): assert len(attach_messages) == 2 assert attach_messages[0].get('channelSerial') is None assert attach_messages[1].get('channelSerial') == 'serial-from-server-1' - set_options.cancel() + await asyncio.wait_for(set_options, OPERATION_TIMEOUT) # UTS: realtime/unit/RTL4f/timeout-to-suspended-0 @@ -561,4 +561,4 @@ def on_message_from_client(msg): assert len(attach_messages) == 2 assert not attach_messages[0].get('flags', 0) & Flag.ATTACH_RESUME assert not attach_messages[1].get('flags', 0) & Flag.ATTACH_RESUME - set_options.cancel() + await asyncio.wait_for(set_options, OPERATION_TIMEOUT) diff --git a/test/uts/realtime/unit/channels/channel_attributes_test.py b/test/uts/realtime/unit/channels/channel_attributes_test.py index aeeeaff2..30be9ac8 100644 --- a/test/uts/realtime/unit/channels/channel_attributes_test.py +++ b/test/uts/realtime/unit/channels/channel_attributes_test.py @@ -95,9 +95,7 @@ async def test_rtl24_error_reason_attach_failure(): # DEVIATION: an attach rejected with a DETACHED carrying an error leaves `error_reason` # unset. `_on_message` (`ably/realtime/channel.py:735`) answers a DETACHED received while # ATTACHING with `_notify_state(ChannelState.SUSPENDED)` and no reason, discarding the - # error. `attach()` then reaches `raise state_change.reason` (`:150`) with `reason` None, - # which raises `TypeError: exceptions must derive from BaseException` rather than the - # AblyException RTL24 describes. + # error, so `attach()` raises its 90001 fallback rather than the error RTL24 describes. channel_name = 'test-RTL24-attach-fail' mock_ws = MockWebSocket( @@ -125,8 +123,7 @@ def on_message_from_client(msg): @deviation async def test_rtl4c_error_cleared_on_attach(): # DEVIATION: as above, the error on the DETACHED which rejects the first attach is - # discarded, so `error_reason` is never set and the attach raises TypeError rather than - # an AblyException. The clearing half of RTL4c is covered by + # discarded, so `error_reason` is never set. The clearing half of RTL4c is covered by # `test_rtl4c_error_cleared_preserved_detach`, which sets the error with an ERROR message. channel_name = 'test-RTL24-clear-attach' attach_count = 0 diff --git a/test/uts/realtime/unit/channels/channel_connection_state_test.py b/test/uts/realtime/unit/channels/channel_connection_state_test.py index d1b2a947..d700fce5 100644 --- a/test/uts/realtime/unit/channels/channel_connection_state_test.py +++ b/test/uts/realtime/unit/channels/channel_connection_state_test.py @@ -309,10 +309,8 @@ async def test_rtl3b_closed_attaching_to_detached(): await asyncio.wait_for(client.close(), OPERATION_TIMEOUT) assert client.connection.state == ConnectionState.CLOSED - # The specification has the pending attach fail. ably-python's `attach()` - # raises only for SUSPENDED and FAILED, so the DETACHED which RTL3b brings - # about resolves it instead. See deviations.md - assert await asyncio.wait_for(attach_future, OPERATION_TIMEOUT) is None + with pytest.raises(AblyException): + await asyncio.wait_for(attach_future, OPERATION_TIMEOUT) assert channel.state == ChannelState.DETACHED diff --git a/test/uts/realtime/unit/channels/channel_detach_test.py b/test/uts/realtime/unit/channels/channel_detach_test.py index f113bfb6..fe176595 100644 --- a/test/uts/realtime/unit/channels/channel_detach_test.py +++ b/test/uts/realtime/unit/channels/channel_detach_test.py @@ -171,8 +171,9 @@ def on_message_from_client(msg): mock_ws.send_to_client(attached_message(channel_name)) # The specification allows the superseded attach either to resolve or to be - # rejected; ably-python resolves it on the DETACHING state change - await asyncio.wait_for(attach_future, OPERATION_TIMEOUT) + # rejected; ably-python rejects it on the DETACHING state change + with pytest.raises(AblyException): + await asyncio.wait_for(attach_future, OPERATION_TIMEOUT) await asyncio.wait_for(detach_future, OPERATION_TIMEOUT) assert channel.state == ChannelState.DETACHED @@ -250,7 +251,6 @@ def on_message_from_client(msg): # UTS: realtime/unit/RTL5l/detach-not-connected-immediate-0 -@deviation async def test_rtl5l_detach_not_connected_immediate(): channel_name = 'test-RTL5l' detach_messages = [] @@ -277,11 +277,13 @@ def on_message_from_client(msg): assert channel.state == ChannelState.DETACHED assert detach_messages == [] - attach_future.cancel() + # The detach leaves the pending attach to fail (RTL4d), which is retrieved so it is not left + # unhandled + with pytest.raises(AblyException): + await asyncio.wait_for(attach_future, OPERATION_TIMEOUT) # UTS: realtime/unit/RTL5l/detach-attached-when-disconnected-1 -@deviation async def test_rtl5l_detach_attached_when_disconnected(): channel_name = 'test-channel' messages_sent = [] diff --git a/test/uts/realtime/unit/channels/channel_options_test.py b/test/uts/realtime/unit/channels/channel_options_test.py index 3da5dd78..88a72b33 100644 --- a/test/uts/realtime/unit/channels/channel_options_test.py +++ b/test/uts/realtime/unit/channels/channel_options_test.py @@ -3,10 +3,7 @@ Spec points: DO2a, RTL16, RTL16a, RTS3b, RTS3c, RTS3c1, RTS5, RTS5a, RTS5a1, RTS5a2, TB2, TB2c, TB2d, TB3, TB4 -`ChannelOptions.cipher` is the specification's `cipherParams`, and the options a -channel carries are the mapping `ChannelOptions.to_dict()` produces rather than -a `ChannelOptions` object, so `channel.options['params']` stands in for the -specification's `channel.options.params`. +`ChannelOptions.cipher` is the specification's `cipherParams`. """ import asyncio @@ -138,8 +135,8 @@ async def test_rts3b_options_set_on_new(): channel = client.channels.get(channel_name, channel_options) - assert channel.options['params']['rewind'] == '1' - assert ChannelMode.SUBSCRIBE in channel.options['modes'] + assert channel.options.params['rewind'] == '1' + assert ChannelMode.SUBSCRIBE in channel.options.modes # UTS: realtime/unit/RTS3c/options-updated-existing-0 @@ -157,8 +154,8 @@ async def test_rts3c_options_updated_existing(): same_channel = client.channels.get(channel_name, new_options) assert same_channel is channel - assert channel.options['cipher'] is not None - assert 'modes' not in channel.options + assert channel.options.cipher is not None + assert channel.options.modes is None # UTS: realtime/unit/RTS3c1/error-reattach-params-0 @@ -179,7 +176,7 @@ async def test_rts3c1_error_reattach_params(): client.channels.get(channel_name, new_options) assert error.value.code == 40000 - assert channel.options.get('params') is None + assert channel.options.params is None # UTS: realtime/unit/RTS3c1/error-reattach-modes-1 @@ -213,20 +210,11 @@ async def test_rtl16_set_options_updates(): new_options = ChannelOptions(params={'delta': 'vcdiff'}) await asyncio.wait_for(channel.set_options(new_options), OPERATION_TIMEOUT) - assert channel.options['params']['delta'] == 'vcdiff' + assert channel.options.params['delta'] == 'vcdiff' # UTS: realtime/unit/RTL16a/triggers-reattach-0 -@deviation async def test_rtl16a_triggers_reattach(): - # DEVIATION: `set_options` never returns for an attached channel. It calls - # `_attach_impl()` directly and then awaits the internal state emitter - # (`ably/realtime/channel.py:99-102`), but the server's ATTACHED arrives while the - # channel is still ATTACHED, so `_on_message` takes the RTL12 branch (`:722-726`) and - # emits `update` on the public emitter alone. Nothing ever reaches the internal - # emitter, so the await hangs. Because `_attach_impl()` is called in place of - # `_request_state(ATTACHING)`, no ATTACHING state change is emitted either. The - # options themselves are stored, before the reattach is requested. channel_name = 'test-RTL16a' attach_messages = [] @@ -245,22 +233,18 @@ def record(change): channel.on(record) new_options = ChannelOptions(params={'rewind': '1'}) - # The specification awaits this without a deadline; a deadline is what turns the - # hang into a failure rather than a stuck test run await asyncio.wait_for(channel.set_options(new_options), OPERATION_TIMEOUT) assert any(change.current == ChannelState.ATTACHING for change in state_changes) assert channel.state == ChannelState.ATTACHED - assert channel.options['params']['rewind'] == '1' + assert channel.options.params['rewind'] == '1' # UTS: realtime/unit/RTS5a/creates-derived-channel-0 @deviation async def test_rts5a_creates_derived_channel(): # DEVIATION: `Channels.get_derived` is absent (`ably/realtime/channel.py:964`), as is - # `DeriveOptions`. `Channels.__getattr__` (`ably/rest/channel.py:408`) answers any - # unknown attribute with a channel of that name, so the call raises - # `TypeError: 'RealtimeChannel' object is not callable` rather than AttributeError. + # `DeriveOptions`, so the import fails before the call would raise AttributeError. from ably.pubsub.server import DeriveOptions base_channel_name = 'test-RTS5a' @@ -333,8 +317,8 @@ async def test_rts5_get_derived_with_options(): channel = client.channels.get_derived(base_channel_name, derive_options, channel_options) - assert ChannelMode.SUBSCRIBE in channel.options['modes'] - assert channel.options['attachOnSubscribe'] is False + assert ChannelMode.SUBSCRIBE in channel.options.modes + assert channel.options.attach_on_subscribe is False # UTS: realtime/unit/DO2a/filter-attribute-0 diff --git a/test/uts/realtime/unit/channels/channel_server_initiated_detach_test.py b/test/uts/realtime/unit/channels/channel_server_initiated_detach_test.py index fc681546..5b7e10c5 100644 --- a/test/uts/realtime/unit/channels/channel_server_initiated_detach_test.py +++ b/test/uts/realtime/unit/channels/channel_server_initiated_detach_test.py @@ -215,8 +215,7 @@ async def test_rtl13b_attaching_detached_to_suspended(): # DEVIATION: RTL13b carries the DETACHED message's error onto the SUSPENDED state # change. `RealtimeChannel._on_message` answers a DETACHED received while ATTACHING # with `_notify_state(ChannelState.SUSPENDED)` and no reason, so the error is - # discarded, and the pending `attach()` reaches `raise state_change.reason` with - # `reason` None, raising `TypeError` rather than an AblyException. The state cycle + # discarded, and the pending `attach()` raises its 90001 fallback instead. The state cycle # this drives is covered ungated by # `test_rtl13b_failed_reattach_suspended_retry` above. channel_name = 'test-RTL13b-attaching' diff --git a/test/uts/realtime/unit/channels/channels_collection_test.py b/test/uts/realtime/unit/channels/channels_collection_test.py index 8d85a2df..ef64317a 100644 --- a/test/uts/realtime/unit/channels/channels_collection_test.py +++ b/test/uts/realtime/unit/channels/channels_collection_test.py @@ -4,9 +4,7 @@ The specification's `channels.exists(name)` and `channels.names` are spelled `name in channels` and iteration over the collection in ably-python, and -`channels.release(name)` is synchronous. Reading an attribute the collection -does not define creates a channel of that name (`Channels.__getattr__` in -`ably/rest/channel.py`), so these tests never name one that is not there. +`channels.release(name)` is synchronous. """ import asyncio diff --git a/test/uts/realtime/unit/connection/connection_ping_test.py b/test/uts/realtime/unit/connection/connection_ping_test.py index dc3abd62..e7a503cd 100644 --- a/test/uts/realtime/unit/connection/connection_ping_test.py +++ b/test/uts/realtime/unit/connection/connection_ping_test.py @@ -20,7 +20,6 @@ realtime_client, ) from test.uts.helpers.clock import FakeClock, settle -from test.uts.helpers.deviations import deviation from test.uts.helpers.mock_websocket import ( ERROR_MESSAGE, MockWebSocket, @@ -284,7 +283,6 @@ async def test_rtn13d_ping_deferred_connecting(): # UTS: realtime/unit/RTN13d/ping-deferred-disconnected-1 -@deviation async def test_rtn13d_ping_deferred_disconnected(): clock = FakeClock() attempts = [] @@ -310,8 +308,7 @@ def on_connection_attempt(conn): ping_task = asyncio.ensure_future(client.connection.ping()) await settle() - # RTN13d: a ping requested while DISCONNECTED waits for the connection. - # ably-python admits only CONNECTED and CONNECTING and errors at once + # RTN13d: a ping requested while DISCONNECTED waits for the connection deferred = not ping_task.done() await clock.advance(600) @@ -348,7 +345,6 @@ async def test_rtn13b_deferred_ping_error_failed(): # UTS: realtime/unit/RTN13b/deferred-ping-error-suspended-5 -@deviation async def test_rtn13b_deferred_ping_error_suspended(): clock = FakeClock() mock_ws = MockWebSocket(on_connection_attempt=lambda conn: conn.respond_with_dns_error()) @@ -362,8 +358,7 @@ async def test_rtn13b_deferred_ping_error_suspended(): ping_task = asyncio.ensure_future(client.connection.ping()) await settle() - # RTN13d: the ping waits for the connection rather than failing where the - # specification expects it to be deferred + # RTN13d: the ping waits for the connection rather than failing at once deferred = not ping_task.done() await clock.advance(CONNECTION_STATE_TTL + 1000) @@ -376,7 +371,6 @@ async def test_rtn13b_deferred_ping_error_suspended(): # UTS: realtime/unit/RTN13c/deferred-ping-timeout-1 -@deviation async def test_rtn13c_deferred_ping_timeout(): clock = FakeClock() attempts = [] @@ -404,6 +398,5 @@ async def test_rtn13c_deferred_ping_timeout(): assert error is not None assert 'timeout' in str(error).lower() # RTN13c with RTN13d: a deferred ping's timeout runs from the HEARTBEAT going - # out, not from the call. ably-python starts `wait_for` when `ping()` is - # called, so the ping expires while the connection is still establishing + # out, not from the call assert failed_after >= timeout_in_secs * 0.9 diff --git a/test/uts/realtime/unit/presence/realtime_presence_enter_test.py b/test/uts/realtime/unit/presence/realtime_presence_enter_test.py index 9168d22a..d917f69e 100644 --- a/test/uts/realtime/unit/presence/realtime_presence_enter_test.py +++ b/test/uts/realtime/unit/presence/realtime_presence_enter_test.py @@ -502,9 +502,9 @@ def on_message_from_client(msg): # RTP16c is about the presence error below, so the attach is only asked to leave the # channel somewhere other than ATTACHED. A DETACHED received while ATTACHING moves # the channel to SUSPENDED rather than the DETACHED the specification expects, with - # no reason attached, so `attach()` raises `None` as a TypeError. Both are recorded + # no reason attached, so `attach()` raises its 90001 fallback. Both are recorded # against the channel specifications; see test/uts/deviations.md. - with pytest.raises((AblyException, TypeError)): + with pytest.raises(AblyException): await channel.attach() assert channel.state in (ChannelState.DETACHED, ChannelState.SUSPENDED) diff --git a/test/uts/rest/unit/channel/get_message_test.py b/test/uts/rest/unit/channel/get_message_test.py index 8f3f1571..11fb2c1d 100644 --- a/test/uts/rest/unit/channel/get_message_test.py +++ b/test/uts/rest/unit/channel/get_message_test.py @@ -125,7 +125,7 @@ def on_request(request): request = captured_requests[0] # DEVIATION RSL11b: the spec asserts the serial is encoded as encodeURIComponent # would, giving "serial%2Fwith%3Aspecial%2Bchars". ably-python encodes the serial - # with `quote_plus(serial, safe=':')`, so the colon travels literally. A colon is a + # with `quote(serial, safe=':')`, so the colon travels literally. A colon is a # legal path character, so the server still receives the same serial; the slash and # the plus, which would otherwise change the serial, are both encoded. assert wire_path(request) == ( diff --git a/test/uts/rest/unit/channel/history_test.py b/test/uts/rest/unit/channel/history_test.py index cfae1d4b..c7d43679 100644 --- a/test/uts/rest/unit/channel/history_test.py +++ b/test/uts/rest/unit/channel/history_test.py @@ -177,14 +177,11 @@ def on_request(request): (simple, f'/channels/{simple}/messages'), # DEVIATION RSL2: the spec asserts the colon is percent-encoded, as # encodeURIComponent would ("test-RSL2-with%3Acolon-..."). ably-python builds the - # path with `quote_plus(name, safe=':')`, so a colon travels literally. It is a + # path with `quote(name, safe=':')`, so a colon travels literally. It is a # legal path character, so the server still receives the same channel name. (with_colon, f'/channels/{with_colon}/messages'), (with_slash, f"/channels/{with_slash.replace('/', '%2F')}/messages"), - # DEVIATION RSL2: the spec asserts a space is percent-encoded as "%20". - # `quote_plus` renders it as "+", which in a URL *path* is a literal plus rather - # than a space, so the channel name arrives at the server altered. - (with_space, f"/channels/{with_space.replace(' ', '+')}/messages"), + (with_space, f"/channels/{with_space.replace(' ', '%20')}/messages"), ] for channel_name, expected_path in test_cases: diff --git a/test/uts/rest/unit/channel/rest_channel_attributes_test.py b/test/uts/rest/unit/channel/rest_channel_attributes_test.py index 479ebd90..f585da01 100644 --- a/test/uts/rest/unit/channel/rest_channel_attributes_test.py +++ b/test/uts/rest/unit/channel/rest_channel_attributes_test.py @@ -7,7 +7,6 @@ from ably.pubsub.types.channeldetails import ChannelDetails from ably.pubsub.types.channeloptions import ChannelOptions from test.uts.helpers.client import rest_client -from test.uts.helpers.deviations import deviation from test.uts.helpers.mock_http import MockHttpClient @@ -58,12 +57,7 @@ async def test_rsl9_channel_name_attribute(): # UTS: rest/unit/RSL7/setoptions-updates-options-0 -@deviation async def test_rsl7_setoptions_updates_options(): - # DEVIATION: the REST channel has no `set_options`. `ably/rest/channel.py` exposes only an - # `options` property whose setter expects the mapping of keyword arguments that - # `Channels.get` collected, so a `ChannelOptions` raises `TypeError` from - # `'cipher' in options`. `ably/realtime/channel.py` does implement `set_options`. mock_http = MockHttpClient( on_connection_attempt=lambda conn: conn.respond_with_success(), on_request=lambda request: request.respond_with(200, []), @@ -78,9 +72,7 @@ async def test_rsl7_setoptions_updates_options(): # UTS: rest/unit/RSL7/setoptions-stores-options-1 -@deviation async def test_rsl7_setoptions_stores_options(): - # DEVIATION: as above, `set_options` is absent from the REST channel. mock_http = MockHttpClient( on_connection_attempt=lambda conn: conn.respond_with_success(), on_request=lambda request: request.respond_with(200, []), @@ -91,9 +83,10 @@ async def test_rsl7_setoptions_stores_options(): # Set options - the effect of channel options is primarily on encryption (RSL5), # which is not exercised here; the call completing is what the spec checks. - await channel.set_options(ChannelOptions()) + options = ChannelOptions() + await channel.set_options(options) - assert channel.options is not None + assert channel.options is options # UTS: rest/unit/RSL8/status-get-correct-endpoint-0 @@ -135,12 +128,8 @@ async def test_rsl8_status_special_chars_encoded(): assert request.method == 'GET' # DEVIATION: the spec asserts the path ends with "/channels/" + encode_uri_component(name). - # `Channel.status` in `ably/rest/channel.py` interpolates the raw name into - # `f'/channels/{self.name}'` and encodes nothing; the escaping visible below is httpx - # normalising the URL on the way out. For this name the wire form still reaches the right - # channel, differing from encodeURIComponent only in leaving ":" unescaped, which RFC 3986 - # permits in a path segment. Names containing "/", "?" or "#" are genuinely mis-routed -- - # see the report accompanying this suite. + # `Channel.status` leaves ":" unescaped, as every REST path does, which RFC 3986 permits in + # a path segment, so the name still reaches the right channel. assert str(request.url).endswith('/channels/namespace:my%20channel') # request.url.path is the percent-decoded path, so it carries the name verbatim assert request.url.path == f'/channels/{channel_name}' diff --git a/test/uts/rest/unit/channel/update_delete_message_test.py b/test/uts/rest/unit/channel/update_delete_message_test.py index cb4de9c7..c01474f3 100644 --- a/test/uts/rest/unit/channel/update_delete_message_test.py +++ b/test/uts/rest/unit/channel/update_delete_message_test.py @@ -305,7 +305,7 @@ async def test_rsl15b_serial_url_encoded_path(): request = captured_requests[0] # DEVIATION: the spec asserts encode_uri_component(serial), which escapes ':' as %3A. - # Channel._send_update escapes the serial with `parse.quote_plus(serial, safe=':')`, so + # Channel._send_update escapes the serial with `parse.quote(serial, safe=':')`, so # '/' becomes %2F but ':' is left as-is. ':' is a legal path character and Ably serials # carry it, so both forms name the same serial once the server decodes them. assert raw_path(request) == f'/channels/{channel_name}/messages/serial%2Fspecial:chars'