Merge branch 'feature/protocol-2-resume-recover' into feature/protoco…

…l-2-presence

ably.gemspec

@@ -5,7 +5,7 @@ require 'ably/version'
 
 Gem::Specification.new do |spec|
   spec.name          = 'ably'
-  spec.version       = Ably::LIB_VERSION
+  spec.version       = Ably::VERSION
   spec.authors       = ['Lewis Marshall', "Matthew O'Riordan"]
   spec.email         = ['[email protected]', '[email protected]']
   spec.description   = %q{A Ruby client library for ably.io realtime messaging}

lib/ably/agent.rb

@@ -1,3 +1,3 @@
 module Ably
-  AGENT = "ably-ruby/#{Ably::LIB_VERSION} ruby/#{RUBY_VERSION}"
+  AGENT = "ably-ruby/#{Ably::VERSION} ruby/#{RUBY_VERSION}"
 end

lib/ably/models/protocol_message.rb

@@ -274,11 +274,12 @@ def to_s
     end
 
     # True if the ProtocolMessage appears to be invalid, however this is not a guarantee
+    # Used for validating incoming protocol messages, so no need to add unnecessary checks
     # @return [Boolean]
     # @api private
     def invalid?
       action_enum = action rescue nil
-      !action_enum || (ack_required? && !has_message_serial?)
+      !action_enum
     end
 
     # @!attribute [r] logger

lib/ably/modules/http_helpers.rb

@@ -18,7 +18,7 @@ def encode64(text)
     end
 
     def user_agent
-      "Ably Ruby client #{Ably::LIB_VERSION} (https://www.ably.io)"
+      "Ably Ruby client #{Ably::VERSION} (https://www.ably.io)"
     end
 
     def setup_outgoing_middleware(builder)

lib/ably/realtime/channels.rb

@@ -52,7 +52,7 @@ def release(channel)
       # @api private
       def set_channel_serials(serials)
         serials.each do |channel_name, channel_serial|
-          channels[channel_name].properties.channel_serial = channel_serial
+          get(channel_name).properties.channel_serial = channel_serial
         end
       end
 

lib/ably/realtime/client.rb

@@ -1,5 +1,6 @@
 require 'uri'
 require 'ably/realtime/channel/publisher'
+require 'ably/realtime/recovery_key_context'
 
 module Ably
   module Realtime
@@ -11,6 +12,7 @@ class Client
       include Ably::Modules::Conversions
 
       extend Forwardable
+      using Ably::Util::AblyExtensions
 
       DOMAIN = 'realtime.ably.io'
 
@@ -120,17 +122,17 @@ def initialize(options)
           acc[key.to_s] = value.to_s
         end
         @rest_client           = Ably::Rest::Client.new(options.merge(realtime_client: self))
-        @echo_messages         = rest_client.options.fetch(:echo_messages, true)
-        @queue_messages        = rest_client.options.fetch(:queue_messages, true)
+        @echo_messages         = rest_client.options.fetch_with_default(:echo_messages, true)
+        @queue_messages        = rest_client.options.fetch_with_default(:queue_messages, true)
         @custom_realtime_host  = rest_client.options[:realtime_host] || rest_client.options[:ws_host]
-        @auto_connect          = rest_client.options.fetch(:auto_connect, true)
-        @recover               = rest_client.options.fetch(:recover, '')
+        @auto_connect          = rest_client.options.fetch_with_default(:auto_connect, true)
+        @recover               = rest_client.options.fetch_with_default(:recover, '')
 
         @auth       = Ably::Realtime::Auth.new(self)
         @channels   = Ably::Realtime::Channels.new(self)
         @connection = Ably::Realtime::Connection.new(self, options)
 
-        unless @recover.empty?
+        unless @recover.nil_or_empty?
           recovery_context = RecoveryKeyContext.from_json(@recover, logger)
           unless recovery_context.nil?
             @channels.set_channel_serials recovery_context.channel_serials # RTN16j

lib/ably/realtime/client/incoming_message_dispatcher.rb

@@ -38,27 +38,25 @@ def logger
 
       def dispatch_protocol_message(*args)
         protocol_message = args.first
-        # RTL15b
-        unless protocol_message.nil?
-          if protocol_message.has_channel_serial? &&
-                  (
-                    protocol_message.action == :message ||
-                    protocol_message.action == :presence ||
-                    protocol_message.action == :attached
-                  )
-
-            logger.info "Setting channel serial for #{channel.name}"
-            logger.info "Previous serial #{channel.name}, new serial #{protocol_message.channel_serial}"
-            get_channel(protocol_message.channel).tap do |channel|
-              channel.properties.channel_serial = protocol_message.channel_serial
-            end
-          end
-        end
 
         unless protocol_message.kind_of?(Ably::Models::ProtocolMessage)
           raise ArgumentError, "Expected a ProtocolMessage. Received #{protocol_message}"
         end
 
+        # RTL15b
+        if protocol_message.has_channel_serial? &&
+          (
+            protocol_message.action == :message ||
+              protocol_message.action == :presence ||
+              protocol_message.action == :attached
+          )
+          get_channel(protocol_message.channel).tap do |channel|
+            logger.info "Setting channel serial for channel #{channel.name}, " <<
+                          "Previous: #{channel.properties.channel_serial}, New: #{protocol_message.channel_serial}"
+            channel.properties.channel_serial = protocol_message.channel_serial
+          end
+        end
+
         unless protocol_message.action.match_any?(:nack, :error)
           logger.debug { "#{protocol_message.action} received: #{protocol_message}" }
         end

lib/ably/realtime/connection.rb

@@ -9,6 +9,8 @@ class Connection
       include Ably::Modules::Conversions
       include Ably::Modules::SafeYield
       extend Ably::Modules::Enum
+      using Ably::Util::AblyExtensions
+
 
       # The current {Ably::Realtime::Connection::STATE} of the connection.
       # Describes the realtime [Connection]{@link Connection} object states.
@@ -330,7 +332,6 @@ def internet_up?
       #
       # @spec RTN16b, RTN16c
       #
-      # @return [String]
       # @deprecated Use {#create_recovery_key} instead
       #
       def recovery_key
@@ -346,10 +347,10 @@ def recovery_key
       # @return [String] a json string which incorporates the @connectionKey@, the current @msgSerial@ and collection
       # of pairs of channel @name@ and current @channelSerial@ for every currently attached channel
       def create_recovery_key
-        if key.nil? || key.empty? || state == :closing || state == :closed || state == :failed || state == :suspended
-          return "" #RTN16g2
+        if key.nil_or_empty? || state == :closing || state == :closed || state == :failed || state == :suspended
+          return nil #RTN16g2
         end
-        Ably::Modules::RecoveryKeyContext.to_json(key, message_serial, client.channels.get_channel_serials)
+        RecoveryKeyContext.new(key, client_msg_serial, client.channels.get_channel_serials).to_json
       end
 
       # Following a new connection being made, the connection ID, connection key
@@ -472,10 +473,10 @@ def create_websocket_transport
               url_params['clientId'] = client.auth.client_id if client.auth.has_client_id?
               url_params.merge!(client.transport_params)
 
-              if not Ably::Util::String.is_null_or_empty(key)
+              if !key.nil_or_empty? and connection_state_available?
                 url_params.merge! resume: key
                 logger.debug { "Resuming connection with key #{key}" }
-              elsif not Ably::Util::String.is_null_or_empty(client.recover)
+              elsif !client.recover.nil_or_empty?
                 recovery_context = RecoveryKeyContext.from_json(client.recover, logger)
                 unless recovery_context.nil?
                   key = recovery_context.connection_key
@@ -695,3 +696,4 @@ def second_reconnect_attempt_for(state, first_attempt_count)
 require 'ably/realtime/connection/connection_manager'
 require 'ably/realtime/connection/connection_state_machine'
 require 'ably/realtime/connection/websocket_transport'
+require 'ably/realtime/recovery_key_context'

lib/ably/realtime/connection/connection_manager.rb

@@ -14,6 +14,7 @@ class ConnectionManager
       RESOLVABLE_ERROR_CODES = {
         token_expired: Ably::Exceptions::TOKEN_EXPIRED_CODE
       }
+      using Ably::Util::AblyExtensions
 
       def initialize(connection)
         @connection     = connection
@@ -112,8 +113,7 @@ def connected(protocol_message)
         # Update the connection details and any associated defaults
         connection.set_connection_details protocol_message.connection_details
 
-        is_connection_resume_or_recover_attempt = !Ably::Util::String.is_null_or_empty(connection.key) ||
-          !Ably::Util::String.is_null_or_empty(client.recover)
+        is_connection_resume_or_recover_attempt = !connection.key.nil_or_empty? || !client.recover.nil_or_empty?
 
         # RTN15c7, RTN16d
         failed_resume_or_recover = !protocol_message.connection_id == connection.id && !protocol_message.error.nil?

lib/ably/rest/client.rb

@@ -16,6 +16,7 @@ class Client
       include Ably::Modules::Conversions
       include Ably::Modules::HttpHelpers
       extend Forwardable
+      using Ably::Util::AblyExtensions
 
       # Default Ably domain for REST
       DOMAIN = 'rest.ably.io'
@@ -186,7 +187,7 @@ def initialize(options)
 
         @agent               = options.delete(:agent) || Ably::AGENT
         @realtime_client     = options.delete(:realtime_client)
-        @tls                 = options.fetch(:tls, true); options.delete(:tls)
+        @tls                 = options.delete_with_default(:tls, true)
         @environment         = options.delete(:environment) # nil is production
         @environment         = nil if [:production, 'production'].include?(@environment)
         @protocol            = options.delete(:protocol) || :msgpack
@@ -200,7 +201,7 @@ def initialize(options)
         @log_retries_as_info = options.delete(:log_retries_as_info)
         @max_message_size    = options.delete(:max_message_size) || MAX_MESSAGE_SIZE
         @max_frame_size      = options.delete(:max_frame_size) || MAX_FRAME_SIZE
-        @idempotent_rest_publishing = options.fetch(:idempotent_rest_publishing, true); options.delete(:idempotent_rest_publishing)
+        @idempotent_rest_publishing = options.delete_with_default(:idempotent_rest_publishing, true)
 
         if options[:fallback_hosts_use_default] && options[:fallback_hosts]
           raise ArgumentError, "fallback_hosts_use_default cannot be set to try when fallback_hosts is also provided"

lib/ably/util/ably_extensions.rb

@@ -0,0 +1,29 @@
+# frozen_string_literal: true
+
+module Ably::Util
+  module AblyExtensions
+    refine Object do
+      def nil_or_empty?
+        self.nil? || self.empty?
+      end
+    end
+
+    refine Hash do
+      def fetch_with_default(key, default)
+        value = self.fetch(key, default)
+        if value.nil?
+          return default
+        end
+        return value
+      end
+
+      def delete_with_default(key, default)
+        value = self.delete(key)
+        if value.nil?
+          return default
+        end
+        return value
+      end
+    end
+  end
+end

lib/ably/version.rb

@@ -1,5 +1,5 @@
 module Ably
-  LIB_VERSION = '1.2.5'
+  VERSION = '1.2.5'
   # The level of compatibility with the Ably service that this SDK supports.
   # Also referred to as the 'wire protocol version'.
   # spec : CSV2

spec/acceptance/rest/message_spec.rb

@@ -214,7 +214,7 @@
         expect(client.idempotent_rest_publishing).to be_truthy
       end
 
-      specify 'idempotent publishing is enabled by default with >= 1.2 (#TO3n)' do
+      specify 'idempotent publishing is enabled by default (#TO3n)' do
         client = Ably::Rest::Client.new(key: api_key, protocol: protocol)
         expect(client.idempotent_rest_publishing).to be_truthy
       end

spec/support/markdown_spec_formatter.rb

@@ -28,7 +28,7 @@ def start(notification)
         else
           'REST'
         end
-        output.write "# Ably #{scope} Client Library #{Ably::LIB_VERSION} Specification\n"
+        output.write "# Ably #{scope} Client Library #{Ably::VERSION} Specification\n"
       end
 
       def close(notification)