diff --git a/ably/pubsub/realtime/presencemap.py b/ably/pubsub/realtime/presencemap.py index a5d2c427..654fa77d 100644 --- a/ably/pubsub/realtime/presencemap.py +++ b/ably/pubsub/realtime/presencemap.py @@ -37,8 +37,9 @@ def _is_newer(item: PresenceMessage, existing: PresenceMessage) -> bool: Raises: ValueError: If message ids cannot be parsed for comparison """ - # RTP2b1: if either is synthesized, compare by timestamp - if item.is_synthesized() or existing.is_synthesized(): + # RTP2b1: if either is synthesized, compare by timestamp. A message without an id + # cannot be compared by msgSerial and index, so it takes the same path. + if item.is_synthesized() or existing.is_synthesized() or not item.id or not existing.id: # RTP2b1a: if equal, prefer the newly-arrived one (item) if item.timestamp is None and existing.timestamp is None: return True diff --git a/ably/pubsub/types/message.py b/ably/pubsub/types/message.py index aaf1b4d6..9cc8fcb6 100644 --- a/ably/pubsub/types/message.py +++ b/ably/pubsub/types/message.py @@ -370,7 +370,7 @@ def from_encoded(obj, cipher=None, context=None): @staticmethod def __update_empty_fields(proto_msg: dict, msg: dict, msg_index: int): - if msg.get("id") is None or msg.get("id") == '': + if (msg.get("id") is None or msg.get("id") == '') and proto_msg.get('id'): msg['id'] = f"{proto_msg.get('id')}:{msg_index}" if msg.get("connectionId") is None or msg.get("connectionId") == '': msg['connectionId'] = proto_msg.get('connectionId') diff --git a/test/unit/message_test.py b/test/unit/message_test.py index 22e0b53a..99534713 100644 --- a/test/unit/message_test.py +++ b/test/unit/message_test.py @@ -95,3 +95,29 @@ def test_validate_message_size_raises_40009_above_the_limit(): assert excinfo.value.code == 40009 assert excinfo.value.status_code == 400 + + +# TM2a +def test_update_inner_message_fields_does_not_fabricate_id_without_protocol_id(): + proto_msg: dict = { + 'connectionId': 'custom_connection_id', + 'presence': [{'action': 2, 'clientId': 'c1'}], + } + ably.pubsub.types.message.Message.update_inner_message_fields(proto_msg) + assert proto_msg['presence'][0].get('id') is None + + +# RTP2b +def test_presence_without_any_id_is_compared_by_timestamp(): + from ably.pubsub.realtime.presencemap import _is_newer + from ably.pubsub.types.presence import PresenceMessage + + def build(timestamp): + proto_msg = {'action': 14, 'timestamp': timestamp, + 'presence': [{'action': 2, 'clientId': 'c1'}]} + ably.pubsub.types.message.Message.update_inner_message_fields(proto_msg) + return PresenceMessage.from_encoded(proto_msg['presence'][0]) + + older, newer = build(1000), build(2000) + assert _is_newer(newer, older) is True + assert _is_newer(older, newer) is False