Annotation review fixes: spec compliance and code cleanup

- Refactor publish/delete to use shared __send_annotation() with explicit
  action setting per RSAN1c1/RSAN2a/RTAN1a/RTAN2a
- RTAN4e: Change subscribe mode check from exception to warning per spec;
  guard against empty modes when server doesn't send flags
- RTAN4c/RTAN5a: Support array of types in subscribe/unsubscribe
- RSAN1c4: Fix idempotent ID generation to use base64(9 random bytes):0
- Export Annotation, AnnotationAction, ChannelMode, ChannelOptions from ably
- Use isinstance() consistently for bool checks across channel modules

Author: sacOO7

ably/__init__.py

@@ -4,7 +4,10 @@
 from ably.rest.auth import Auth
 from ably.rest.push import Push
 from ably.rest.rest import AblyRest
+from ably.types.annotation import Annotation, AnnotationAction
 from ably.types.capability import Capability
+from ably.types.channelmode import ChannelMode
+from ably.types.channeloptions import ChannelOptions
 from ably.types.channelsubscription import PushChannelSubscription
 from ably.types.device import DeviceDetails
 from ably.types.message import MessageAction, MessageVersion

ably/realtime/annotations.py

@@ -40,30 +40,21 @@ def __init__(self, channel: RealtimeChannel, connection_manager: ConnectionManag
         self.__subscriptions = EventEmitter()
         self.__rest_annotations = RestAnnotations(channel)
 
-    async def publish(self, msg_or_serial, annotation: Annotation, params: dict | None = None):
+    async def __send_annotation(self, annotation: Annotation, params: dict | None = None):
         """
-        Publish an annotation on a message via the realtime connection.
+        Internal method to send an annotation via the realtime connection.
 
         Args:
-            msg_or_serial: Either a message serial (string) or a Message object
-            annotation: Annotation object
+            annotation: Validated Annotation object with action and message_serial set
             params: Optional dict of query parameters
-
-        Returns:
-            None
-
-        Raises:
-            AblyException: If the request fails, inputs are invalid, or channel is in unpublishable state
         """
-        annotation = construct_validate_annotation(msg_or_serial, annotation)
-
         # Check if channel and connection are in publishable state
         self.__channel._throw_if_unpublishable_state()
 
         log.info(
-            f'RealtimeAnnotations.publish(), channelName = {self.__channel.name}, '
-            f'sending annotation with messageSerial = {annotation.message_serial}, '
-            f'type = {annotation.type}'
+            f'RealtimeAnnotations: sending annotation, channelName = {self.__channel.name}, '
+            f'messageSerial = {annotation.message_serial}, '
+            f'type = {annotation.type}, action = {annotation.action}'
         )
 
         # Convert to wire format (array of annotations)
@@ -84,6 +75,28 @@ async def publish(self, msg_or_serial, annotation: Annotation, params: dict | No
         # Send via WebSocket
         await self.__connection_manager.send_protocol_message(protocol_message)
 
+    async def publish(self, msg_or_serial, annotation: Annotation, params: dict | None = None):
+        """
+        Publish an annotation on a message via the realtime connection.
+
+        Args:
+            msg_or_serial: Either a message serial (string) or a Message object
+            annotation: Annotation object
+            params: Optional dict of query parameters
+
+        Returns:
+            None
+
+        Raises:
+            AblyException: If the request fails, inputs are invalid, or channel is in unpublishable state
+        """
+        annotation = construct_validate_annotation(msg_or_serial, annotation)
+
+        # RSAN1c1/RTAN1a: Explicitly set action to ANNOTATION_CREATE
+        annotation = annotation._copy_with(action=AnnotationAction.ANNOTATION_CREATE)
+
+        await self.__send_annotation(annotation, params)
+
     async def delete(
         self,
         msg_or_serial,
@@ -93,9 +106,6 @@ async def delete(
         """
         Delete an annotation on a message.
 
-        This is a convenience method that sets the action to 'annotation.delete'
-        and calls publish().
-
         Args:
             msg_or_serial: Either a message serial (string) or a Message object
             annotation: Annotation containing annotation properties
@@ -107,23 +117,24 @@ async def delete(
         Raises:
             AblyException: If the request fails or inputs are invalid
         """
-        return await self.publish(
-            msg_or_serial,
-            annotation._copy_with(action=AnnotationAction.ANNOTATION_DELETE),
-            params,
-        )
+        annotation = construct_validate_annotation(msg_or_serial, annotation)
+
+        # RSAN2a/RTAN2a: Explicitly set action to ANNOTATION_DELETE
+        annotation = annotation._copy_with(action=AnnotationAction.ANNOTATION_DELETE)
+
+        await self.__send_annotation(annotation, params)
 
     async def subscribe(self, *args):
         """
         Subscribe to annotation events on this channel.
 
         Parameters
         ----------
-        *args: type, listener
-            Subscribe type and listener
+        *args: type_or_types, listener
+            Subscribe type(s) and listener
 
-            arg1(type): str, optional
-                Subscribe to annotations of the given type
+            arg1(type_or_types): str or list[str], optional
+                Subscribe to annotations of the given type or types (RTAN4c)
 
             arg2(listener): callable
                 Subscribe to all annotations on the channel
@@ -132,62 +143,63 @@ async def subscribe(self, *args):
 
         Raises
         ------
-        AblyException
-            If unable to subscribe due to invalid channel state or missing ANNOTATION_SUBSCRIBE mode
         ValueError
             If no valid subscribe arguments are passed
         """
         # Parse arguments similar to channel.subscribe
         if len(args) == 0:
             raise ValueError("annotations.subscribe called without arguments")
 
-        if len(args) >= 2 and isinstance(args[0], str):
-            annotation_type = args[0]
+        annotation_types = None
+
+        # RTAN4c: Support string or list of strings as first argument
+        if len(args) >= 2 and isinstance(args[0], (str, list)):
+            if isinstance(args[0], list):
+                annotation_types = args[0]
+            else:
+                annotation_types = [args[0]]
             if not args[1]:
                 raise ValueError("annotations.subscribe called without listener")
             if not is_callable_or_coroutine(args[1]):
                 raise ValueError("subscribe listener must be function or coroutine function")
             listener = args[1]
         elif is_callable_or_coroutine(args[0]):
             listener = args[0]
-            annotation_type = None
         else:
             raise ValueError('invalid subscribe arguments')
 
-        # Register subscription
-        if annotation_type is not None:
-            self.__subscriptions.on(annotation_type, listener)
-        else:
-            self.__subscriptions.on(listener)
-
+        # RTAN4d: Implicitly attach channel on subscribe
         await self.__channel.attach()
 
-        # Check if ANNOTATION_SUBSCRIBE mode is enabled
-        if self.__channel.state == ChannelState.ATTACHED:
+        # RTAN4e: Check if ANNOTATION_SUBSCRIBE mode is enabled (log warning per spec),
+        # only when server explicitly sent modes (non-empty list)
+        if self.__channel.state == ChannelState.ATTACHED and self.__channel.modes:
             if ChannelMode.ANNOTATION_SUBSCRIBE not in self.__channel.modes:
-                if annotation_type is not None:
-                    self.__subscriptions.off(annotation_type, listener)
-                else:
-                    self.__subscriptions.off(listener)
-                raise AblyException(
-                    message="You are trying to add an annotation listener, but you haven't requested the "
-                    "annotation_subscribe channel mode in ChannelOptions, so this won't do anything "
-                    "(we only deliver annotations to clients who have explicitly requested them)",
-                    code=93001,
-                    status_code=400,
+                log.warning(
+                    "You are trying to add an annotation listener, but the "
+                    "ANNOTATION_SUBSCRIBE channel mode was not included in the ATTACHED flags. "
+                    "This subscription may not receive annotations. Ensure you request the "
+                    "annotation_subscribe channel mode in ChannelOptions."
                 )
 
+        # Register subscription after successful attach
+        if annotation_types is not None:
+            for t in annotation_types:
+                self.__subscriptions.on(t, listener)
+        else:
+            self.__subscriptions.on(listener)
+
     def unsubscribe(self, *args):
         """
         Unsubscribe from annotation events on this channel.
 
         Parameters
         ----------
-        *args: type, listener
-            Unsubscribe type and listener
+        *args: type_or_types, listener
+            Unsubscribe type(s) and listener
 
-            arg1(type): str, optional
-                Unsubscribe from annotations of the given type
+            arg1(type_or_types): str or list[str], optional
+                Unsubscribe from annotations of the given type or types
 
             arg2(listener): callable
                 Unsubscribe from all annotations on the channel
@@ -203,10 +215,15 @@ def unsubscribe(self, *args):
         # RTAN5: Support no arguments to unsubscribe all annotation listeners
         if len(args) == 0:
             self.__subscriptions.off()
-        elif len(args) >= 2 and isinstance(args[0], str):
-            annotation_type = args[0]
+        elif len(args) >= 2 and isinstance(args[0], (str, list)):
+            # RTAN5a: Support string or list of strings for type(s)
+            if isinstance(args[0], list):
+                annotation_types = args[0]
+            else:
+                annotation_types = [args[0]]
             listener = args[1]
-            self.__subscriptions.off(annotation_type, listener)
+            for t in annotation_types:
+                self.__subscriptions.off(t, listener)
         elif is_callable_or_coroutine(args[0]):
             listener = args[0]
             self.__subscriptions.off(listener)

ably/realtime/channel.py

@@ -540,7 +540,7 @@ async def _send_update(
             f'channel = {self.name}, state = {self.state}, serial = {message.serial}'
         )
 
-        stringified_params = {k: str(v).lower() if type(v) is bool else v for k, v in params.items()} \
+        stringified_params = {k: str(v).lower() if isinstance(v, bool) else v for k, v in params.items()} \
             if params else None
 
         # Send protocol message

ably/rest/annotations.py

@@ -1,8 +1,9 @@
 from __future__ import annotations
 
+import base64
 import json
 import logging
-import uuid
+import os
 from urllib import parse
 
 import msgpack
@@ -118,31 +119,19 @@ def __base_path_for_serial(self, serial):
         channel_path = '/channels/{}/'.format(parse.quote_plus(self.__channel.name, safe=':'))
         return channel_path + 'messages/' + parse.quote_plus(serial, safe=':') + '/annotations'
 
-    async def publish(
-        self,
-        msg_or_serial,
-        annotation: Annotation,
-        params: dict | None = None,
-    ):
+    async def __send_annotation(self, annotation: Annotation, params: dict | None = None):
         """
-        Publish an annotation on a message.
+        Internal method to send an annotation to the API.
 
         Args:
-            msg_or_serial: Either a message serial (string) or a Message object
-            annotation: Annotation object
+            annotation: Validated Annotation object with action and message_serial set
             params: Optional dict of query parameters
-
-        Returns:
-            None
-
-        Raises:
-            AblyException: If the request fails or inputs are invalid
         """
-        annotation = construct_validate_annotation(msg_or_serial, annotation)
-
         # RSAN1c4: Generate random ID if not provided (for idempotent publishing)
+        # Spec: base64-encode at least 9 random bytes, append ':0'
         if not annotation.id and self.__client_options.idempotent_rest_publishing:
-            annotation = annotation._copy_with(id=str(uuid.uuid4()))
+            random_id = base64.b64encode(os.urandom(9)).decode('ascii') + ':0'
+            annotation = annotation._copy_with(id=random_id)
 
         # Convert to wire format
         request_body = annotation.as_dict(binary=self.__channel.ably.options.use_binary_protocol)
@@ -165,6 +154,33 @@ async def publish(
         # Send request
         await self.__channel.ably.http.post(path, body=request_body)
 
+    async def publish(
+        self,
+        msg_or_serial,
+        annotation: Annotation,
+        params: dict | None = None,
+    ):
+        """
+        Publish an annotation on a message.
+
+        Args:
+            msg_or_serial: Either a message serial (string) or a Message object
+            annotation: Annotation object
+            params: Optional dict of query parameters
+
+        Returns:
+            None
+
+        Raises:
+            AblyException: If the request fails or inputs are invalid
+        """
+        annotation = construct_validate_annotation(msg_or_serial, annotation)
+
+        # RSAN1c1: Explicitly set action to ANNOTATION_CREATE
+        annotation = annotation._copy_with(action=AnnotationAction.ANNOTATION_CREATE)
+
+        await self.__send_annotation(annotation, params)
+
     async def delete(
         self,
         msg_or_serial,
@@ -188,11 +204,12 @@ async def delete(
         Raises:
             AblyException: If the request fails or inputs are invalid
         """
-        return await self.publish(
-            msg_or_serial,
-            annotation._copy_with(action=AnnotationAction.ANNOTATION_DELETE),
-            params,
-        )
+        annotation = construct_validate_annotation(msg_or_serial, annotation)
+
+        # RSAN2a: Explicitly set action to ANNOTATION_DELETE
+        annotation = annotation._copy_with(action=AnnotationAction.ANNOTATION_DELETE)
+
+        return await self.__send_annotation(annotation, params)
 
     async def get(self, msg_or_serial, params: dict | None = None):
         """

ably/rest/channel.py

@@ -112,7 +112,7 @@ async def publish_messages(self, messages, params=None, timeout=None):
 
         path = self.__base_path + 'messages'
         if params:
-            params = {k: str(v).lower() if type(v) is bool else v for k, v in params.items()}
+            params = {k: str(v).lower() if isinstance(v, bool) else v for k, v in params.items()}
             path += '?' + parse.urlencode(params)
         response = await self.ably.http.post(path, body=request_body, timeout=timeout)
 
@@ -211,7 +211,7 @@ async def _send_update(
         # Build path with params
         path = self.__base_path + 'messages/{}'.format(parse.quote_plus(message.serial, safe=':'))
         if params:
-            params = {k: str(v).lower() if type(v) is bool else v for k, v in params.items()}
+            params = {k: str(v).lower() if isinstance(v, bool) else v for k, v in params.items()}
             path += '?' + parse.urlencode(params)
 
         # Send request

ably/types/annotation.py

@@ -187,8 +187,8 @@ def from_encoded(obj, cipher=None, context=None):
         timestamp = obj.get('timestamp')
         extras = obj.get('extras', None)
 
-        # Decode data if present
-        decoded_data = Annotation.decode(data, encoding, cipher, context) if data is not None else {}
+        # Decode data if present, passing data=None explicitly when absent
+        decoded_data = Annotation.decode(data, encoding, cipher, context) if data is not None else {'data': None}
 
         # Convert action from int to enum
         if action is not None:
@@ -245,10 +245,8 @@ def update_inner_annotation_fields(proto_msg: dict):
         """
         annotations: list[dict] = proto_msg.get('annotations')
         if annotations is not None:
-            annotation_index = 0
-            for annotation in annotations:
+            for annotation_index, annotation in enumerate(annotations):
                 Annotation.__update_empty_fields(proto_msg, annotation, annotation_index)
-                annotation_index = annotation_index + 1
 
     def __str__(self):
         return (