0.9: Presence compare logic update + missing specs

RTP1, RTP2*, RTP3

lib/ably/models/presence_message.rb

@@ -123,7 +123,6 @@ def to_json(*args)
       end.to_json
     end
 
-
     # Assign this presence message to a ProtocolMessage before delivery to the Ably system
     # @api private
     def assign_to_protocol_message(protocol_message)
@@ -145,6 +144,17 @@ def protocol_message
       @protocol_message
     end
 
+    # Create a static shallow clone of this object with the optional attributes to overide existing values
+    # Shallow clones have no dependency on the originating ProtocolMessage as all field values are stored as opposed to calculated
+    # Clones are useful when the original PresenceMessage needs to be mutated, such as storing in a PresenceMap with action :present
+    def shallow_clone(new_attributes = {})
+      self.class.new(attributes.to_hash.merge(
+        id: id,
+        connection_id: connection_id,
+        timestamp: as_since_epoch(timestamp)
+      ).merge(new_attributes))
+    end
+
     private
     def raw_hash_object
       @raw_hash_object

lib/ably/realtime/presence.rb

@@ -210,7 +210,7 @@ def update_client(client_id, data = nil, &success_block)
       send_presence_action_for_client(Ably::Models::PresenceMessage::ACTION.Update, client_id, data, &success_block)
     end
 
-    # Get the presence state for this Channel.
+    # Get the presence members for this Channel.
     #
     # @param (see Ably::Realtime::Presence::MembersMap#get)
     # @option options (see Ably::Realtime::Presence::MembersMap#get)
@@ -222,9 +222,9 @@ def get(options = {}, &block)
 
       ensure_channel_attached(deferrable) do
         members.get(options).tap do |members_map_deferrable|
-          members_map_deferrable.callback do |*args|
-            safe_yield(block, *args) if block_given?
-            deferrable.succeed(*args)
+          members_map_deferrable.callback do |members|
+            safe_yield(block, members) if block_given?
+            deferrable.succeed(members)
           end
           members_map_deferrable.errback do |*args|
             deferrable.fail(*args)

lib/ably/realtime/presence/members_map.rb

@@ -228,24 +228,52 @@ def ensure_presence_message_is_valid(presence_message)
       end
 
       # If the message received is older than the last known event for presence
-      # then skip.  This can occur during a SYNC operation.  For example:
+      # then skip (return false). This can occur during a SYNC operation.  For example:
       #   - SYNC starts
       #   - LEAVE event received for clientId 5
       #   - SYNC present even received for clientId 5 with a timestamp before LEAVE event because the LEAVE occured before the SYNC operation completed
       #
-      # @return [Boolean]
+      # @return [Boolean] true when +new_message+ is newer than the existing member in the PresenceMap
       #
-      def should_update_member?(presence_message)
-        if members[presence_message.member_key]
-          members[presence_message.member_key].fetch(:message).timestamp < presence_message.timestamp
+      def should_update_member?(new_message)
+        if members[new_message.member_key]
+          existing_message = members[new_message.member_key].fetch(:message)
+
+          # If both are messages published by clients (not fabricated), use the ID to determine newness, see #RTP2b2
+          if new_message.id.start_with?(new_message.connection_id) && existing_message.id.start_with?(existing_message.connection_id)
+            new_message_parts = new_message.id.match(/(\d+):(\d+)$/)
+            existing_message_parts = existing_message.id.match(/(\d+):(\d+)$/)
+
+            if !new_message_parts || !existing_message_parts
+              logger.fatal { "#{self.class.name}: Message IDs for new message #{new_message.id} or old message #{existing_message.id} are invalid. \nNew message: #{new_message.to_json}" }
+              return existing_message.timestamp < new_message.timestamp
+            end
+
+            # ID is in the format "connid:msgSerial:index" such as "aaaaaa:0:0"
+            # if msgSerial is greater then the new_message should update the member
+            # if msgSerial is equal and index is greater, then update the member
+            if new_message_parts[1].to_i > existing_message_parts[1].to_i # msgSerial
+              true
+            elsif new_message_parts[1].to_i == existing_message_parts[1].to_i # msgSerial equal
+              new_message_parts[2].to_i > existing_message_parts[2].to_i # compare index
+            else
+              false
+            end
+          else
+            # This message is fabricated or could not be validated so rely on timestamps, see #RTP2b1
+            new_message.timestamp > existing_message.timestamp
+          end
         else
           true
         end
       end
 
       def add_presence_member(presence_message)
         logger.debug { "#{self.class.name}: Member '#{presence_message.member_key}' for event '#{presence_message.action}' #{members.has_key?(presence_message.member_key) ? 'updated' : 'added'}.\n#{presence_message.to_json}" }
-        members[presence_message.member_key] = { present: true, message: presence_message }
+        present_action =
+        # Mutate the PresenceMessage so that the action is :present, see #RTP2d
+        present_presence_message = presence_message.shallow_clone(action: Ably::Models::PresenceMessage::ACTION.Present)
+        members[presence_message.member_key] = { present: true, message: present_presence_message }
         presence.emit_message presence_message.action, presence_message
       end