[AIT-316] refactor: enforce strict Annotation type usage and extend handling

- Refactored to mandate the `Annotation` type across annotation-related methods in `RealtimeAnnotations` and `RestAnnotations`.
- Introduced `_copy_with` in `Annotation` for simplified object cloning with modifications.
- Enhanced data validation in `encode_data` to raise `AblyException` for unsupported payloads.

Author: ttypic

ably/realtime/annotations.py

@@ -5,7 +5,7 @@
 
 from ably.rest.annotations import RestAnnotations, construct_validate_annotation
 from ably.transport.websockettransport import ProtocolMessageAction
-from ably.types.annotation import AnnotationAction
+from ably.types.annotation import Annotation, AnnotationAction
 from ably.types.channelstate import ChannelState
 from ably.types.flags import Flag
 from ably.util.eventemitter import EventEmitter
@@ -40,13 +40,13 @@ def __init__(self, channel: RealtimeChannel, connection_manager: ConnectionManag
         self.__subscriptions = EventEmitter()
         self.__rest_annotations = RestAnnotations(channel)
 
-    async def publish(self, msg_or_serial, annotation: dict, params: dict | None = None):
+    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: Dict containing annotation properties (type, name, data, etc.)
+            annotation: Annotation object
             params: Optional dict of query parameters
 
         Returns:
@@ -87,7 +87,7 @@ async def publish(self, msg_or_serial, annotation: dict, params: dict | None = N
     async def delete(
         self,
         msg_or_serial,
-        annotation: dict,
+        annotation: Annotation,
         params: dict | None = None,
     ):
         """
@@ -98,7 +98,7 @@ async def delete(
 
         Args:
             msg_or_serial: Either a message serial (string) or a Message object
-            annotation: Dict containing annotation properties
+            annotation: Annotation containing annotation properties
             params: Optional dict of query parameters
 
         Returns:
@@ -107,9 +107,11 @@ async def delete(
         Raises:
             AblyException: If the request fails or inputs are invalid
         """
-        annotation_values = annotation.copy()
-        annotation_values['action'] = AnnotationAction.ANNOTATION_DELETE
-        return await self.publish(msg_or_serial, annotation_values, params)
+        return await self.publish(
+            msg_or_serial,
+            annotation._copy_with(action=AnnotationAction.ANNOTATION_DELETE),
+            params,
+        )
 
     async def subscribe(self, *args):
         """
@@ -163,6 +165,10 @@ async def subscribe(self, *args):
         # Check if ANNOTATION_SUBSCRIBE mode is enabled
         if self.__channel.state == ChannelState.ATTACHED:
             if Flag.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 "

ably/rest/annotations.py

@@ -49,13 +49,13 @@ def serial_from_msg_or_serial(msg_or_serial):
     return message_serial
 
 
-def construct_validate_annotation(msg_or_serial, annotation: dict):
+def construct_validate_annotation(msg_or_serial, annotation: Annotation) -> Annotation:
     """
     Construct and validate an Annotation from input values.
 
     Args:
         msg_or_serial: Either a string serial or a Message object
-        annotation: Dict of annotation properties or Annotation object
+        annotation: Annotation object
 
     Returns:
         Annotation: The constructed annotation
@@ -65,18 +65,17 @@ def construct_validate_annotation(msg_or_serial, annotation: dict):
     """
     message_serial = serial_from_msg_or_serial(msg_or_serial)
 
-    if not annotation or (not isinstance(annotation, dict) and not isinstance(annotation, Annotation)):
+    if not annotation or not isinstance(annotation, Annotation):
         raise AblyException(
             message='Second argument of annotations.publish() must be a dict or Annotation '
             '(the intended annotation to publish)',
             status_code=400,
             code=40003,
         )
 
-    annotation_values = annotation.copy()
-    annotation_values['message_serial'] = message_serial
-
-    return Annotation.from_values(annotation_values)
+    return annotation._copy_with(
+        message_serial=message_serial,
+    )
 
 
 class RestAnnotations:
@@ -109,15 +108,15 @@ def __base_path_for_serial(self, serial):
     async def publish(
         self,
         msg_or_serial,
-        annotation: dict | Annotation,
+        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: Dict containing annotation properties (type, name, data, etc.) or Annotation object
+            annotation: Annotation object
             params: Optional dict of query parameters
 
         Returns:
@@ -152,7 +151,7 @@ async def publish(
     async def delete(
         self,
         msg_or_serial,
-        annotation: dict | Annotation,
+        annotation: Annotation,
         params: dict | None = None,
     ):
         """
@@ -163,7 +162,7 @@ async def delete(
 
         Args:
             msg_or_serial: Either a message serial (string) or a Message object
-            annotation: Dict containing annotation properties or Annotation object
+            annotation: Annotation object
             params: Optional dict of query parameters
 
         Returns:
@@ -172,13 +171,11 @@ async def delete(
         Raises:
             AblyException: If the request fails or inputs are invalid
         """
-        # Set action to delete
-        if isinstance(annotation, Annotation):
-            annotation_values = annotation.as_dict()
-        else:
-            annotation_values = annotation.copy()
-        annotation_values['action'] = AnnotationAction.ANNOTATION_DELETE
-        return await self.publish(msg_or_serial, annotation_values, params)
+        return await self.publish(
+            msg_or_serial,
+            annotation._copy_with(action=AnnotationAction.ANNOTATION_DELETE),
+            params,
+        )
 
     async def get(self, msg_or_serial, params: dict | None = None):
         """

ably/rest/auth.py

@@ -89,7 +89,7 @@ def __init__(self, ably: AblyRest | AblyRealtime, options: Options):
 
     async def get_auth_transport_param(self):
         auth_credentials = {}
-        if self.auth_options.client_id:
+        if self.auth_options.client_id and self.auth_options.client_id != '*':
             auth_credentials["clientId"] = self.auth_options.client_id
         if self.__auth_mechanism == Auth.Method.BASIC:
             key_name = self.__auth_options.key_name

ably/types/annotation.py

@@ -8,6 +8,10 @@
 log = logging.getLogger(__name__)
 
 
+# Sentinel value to distinguish between "not provided" and "explicitly None"
+_UNSET = object()
+
+
 class AnnotationAction(IntEnum):
     """Annotation action types"""
     ANNOTATION_CREATE = 0
@@ -59,6 +63,7 @@ def __init__(self,
         self.__client_id = to_text(client_id) if client_id is not None else None
         self.__timestamp = timestamp
         self.__extras = extras
+        self.__encoding = encoding
 
     def __eq__(self, other):
         if isinstance(other, Annotation):
@@ -204,6 +209,62 @@ def __str__(self):
     def __repr__(self):
         return self.__str__()
 
+    def _copy_with(self,
+                  action=_UNSET,
+                  serial=_UNSET,
+                  message_serial=_UNSET,
+                  type=_UNSET,
+                  name=_UNSET,
+                  count=_UNSET,
+                  data=_UNSET,
+                  encoding=_UNSET,
+                  client_id=_UNSET,
+                  timestamp=_UNSET,
+                  extras=_UNSET):
+        """
+        Create a copy of this Annotation with optionally modified fields.
+
+        To explicitly set a field to None, pass None as the value.
+        Fields not provided will retain their original values.
+
+        Args:
+            action: Override the action type (or None to clear it)
+            serial: Override the serial (or None to clear it)
+            message_serial: Override the message serial (or None to clear it)
+            type: Override the type (or None to clear it)
+            name: Override the name (or None to clear it)
+            count: Override the count (or None to clear it)
+            data: Override the data payload (or None to clear it)
+            encoding: Override the encoding format (or None to clear it)
+            client_id: Override the client ID (or None to clear it)
+            timestamp: Override the timestamp (or None to clear it)
+            extras: Override the extras metadata (or None to clear it)
+
+        Returns:
+            A new Annotation instance with the specified fields updated
+
+        Example:
+            # Keep existing name, change type
+            new_ann = annotation.copy_with(type="like")
+
+            # Explicitly set name to None
+            new_ann = annotation.copy_with(name=None)
+        """
+        # Get encoding from the mixin's property
+        return Annotation(
+            action=self.__action if action is _UNSET else action,
+            serial=self.__serial if serial is _UNSET else serial,
+            message_serial=self.__message_serial if message_serial is _UNSET else message_serial,
+            type=self.__type if type is _UNSET else type,
+            name=self.__name if name is _UNSET else name,
+            count=self.__count if count is _UNSET else count,
+            data=self.__data if data is _UNSET else data,
+            encoding=self.__encoding if encoding is _UNSET else encoding,
+            client_id=self.__client_id if client_id is _UNSET else client_id,
+            timestamp=self.__timestamp if timestamp is _UNSET else timestamp,
+            extras=self.__extras if extras is _UNSET else extras,
+        )
+
 
 def make_annotation_response_handler(cipher=None):
     """Create a response handler for annotation API responses"""

ably/util/encoding.py

@@ -3,6 +3,7 @@
 from typing import Any
 
 from ably.util.crypto import CipherData
+from ably.util.exceptions import AblyException
 
 
 def encode_data(data: Any, encoding_array: list, binary: bool = False):
@@ -29,6 +30,9 @@ def encode_data(data: Any, encoding_array: list, binary: bool = False):
 
     result = { 'data': data }
 
+    if not (isinstance(data, (bytes, str, list, dict, bytearray)) or data is None):
+        raise AblyException("Invalid data payload", 400, 40011)
+
     if encoding:
         result['encoding'] = '/'.join(encoding).strip('/')