From a3d97462dadeae573d0b5190229b28d4c92368cb Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 24 Jun 2024 16:47:11 +0530 Subject: [PATCH 01/22] Refactored logic for sending ATTACH message 1. Send attach only when connection state is CONNECTED 2. Not queue attach message, instead send is immediately on connected 3. Removed use of emitter pattern while clearing attach/detach timers --- lib/ably/realtime/channel/channel_manager.rb | 33 +++++++++++++++---- .../realtime/channel/channel_state_machine.rb | 1 + lib/ably/realtime/connection.rb | 17 ++++++++-- .../realtime/connection/connection_manager.rb | 2 +- 4 files changed, 43 insertions(+), 10 deletions(-) diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index d10bc66c..bdf439fd 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -53,7 +53,7 @@ def log_channel_error(error) # @option options [Ably::Models::ErrorInfo] :reason def request_reattach(options = {}) reason = options[:reason] - send_attach_protocol_message + send_attach_protocol_message(options[:forced_attach]) logger.debug { "Explicit channel reattach request sent to Ably due to #{reason}" } channel.set_channel_error_reason(reason) if reason channel.transition_state_machine! :attaching, reason: reason unless channel.attaching? @@ -201,8 +201,9 @@ def channel_retry_timeout connection.defaults.fetch(:channel_retry_timeout) end - def send_attach_protocol_message + def send_attach_protocol_message(forced_attach = false) message_options = {} + message_options[:forced_attach] = forced_attach message_options[:params] = channel.options.params if channel.options.params.any? message_options[:flags] = channel.options.modes_to_flags if channel.options.modes if channel.attach_resume @@ -217,6 +218,11 @@ def send_detach_protocol_message(previous_state) send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Detach, previous_state # return to previous state if failed end + def notify_state_change + @pending_state_change_timer.cancel if @pending_state_change_timer + @pending_state_change_timer = nil + end + def send_state_change_protocol_message(new_state, state_if_failed, message_options = {}) state_at_time_of_request = channel.state @pending_state_change_timer = EventMachine::Timer.new(realtime_request_timeout) do @@ -226,11 +232,6 @@ def send_state_change_protocol_message(new_state, state_if_failed, message_optio end end - channel.once_state_changed do - @pending_state_change_timer.cancel if @pending_state_change_timer - @pending_state_change_timer = nil - end - resend_if_disconnected_and_connected = lambda do connection.unsafe_once(:disconnected) do next unless pending_state_change_timer @@ -245,6 +246,24 @@ def send_state_change_protocol_message(new_state, state_if_failed, message_optio end end end + + # Attach is sent on every connected msg received as per RTN15c6, RTN15c7 + # So, no need to introduce logic that sends attach on disconnect and connect + if new_state == Ably::Models::ProtocolMessage::ACTION.Attach + # Sends attach message only if it's forced_attach/connected_msg_received or + # connection state is connected. + if message_options.delete(:forced_attach) || connection.state?(:connected) + # Shouldn't queue attach message as per RTL4i, so message is added top of the queue + # to be sent immediately while processing next message + connection.send_protocol_message_immediately( + action: new_state.to_i, + channel: channel.name, + **message_options.to_h + ) + end + return + end + resend_if_disconnected_and_connected.call connection.send_protocol_message( diff --git a/lib/ably/realtime/channel/channel_state_machine.rb b/lib/ably/realtime/channel/channel_state_machine.rb index a346fec2..a6f67516 100644 --- a/lib/ably/realtime/channel/channel_state_machine.rb +++ b/lib/ably/realtime/channel/channel_state_machine.rb @@ -29,6 +29,7 @@ class ChannelStateMachine transition :from => :failed, :to => [:attaching, :initialized] after_transition do |channel, transition| + channel.manager.send(:notify_state_change) channel.synchronize_state_with_statemachine end diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index aa0f1ada..217462a0 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -439,9 +439,22 @@ def send_protocol_message(protocol_message) end end + def send_protocol_message_immediately(protocol_message) + Ably::Models::ProtocolMessage.new(protocol_message, logger: logger).tap do |message| + add_message_to_outgoing_queue(message, true) + notify_message_dispatcher_of_new_message message + logger.debug { "Connection: Prot msg pushed at the top =>: #{message.action} #{message}" } + end + end + # @api private - def add_message_to_outgoing_queue(protocol_message) - __outgoing_message_queue__ << protocol_message + def add_message_to_outgoing_queue(protocol_message, send_immediately = false) + if send_immediately + # Adding msg at the top of the queue to get processed immediately while connection is CONNECTED + __outgoing_message_queue__.prepend(protocol_message) + else + __outgoing_message_queue__ << protocol_message + end end # @api private diff --git a/lib/ably/realtime/connection/connection_manager.rb b/lib/ably/realtime/connection/connection_manager.rb index cfbfe87d..0743f995 100644 --- a/lib/ably/realtime/connection/connection_manager.rb +++ b/lib/ably/realtime/connection/connection_manager.rb @@ -583,7 +583,7 @@ def force_reattach_on_channels(error) channels.select do |channel| channel.attached? || channel.attaching? || channel.suspended? end.each do |channel| - channel.manager.request_reattach reason: error + channel.manager.request_reattach reason: error, forced_attach: true end end From f4d7f2bfb25aac70715cb8ba7bc024898e856bae Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 15:56:38 +0530 Subject: [PATCH 02/22] [ECO-4845] refactored rest/realtime auth 1. Renamed rest auth extra_auth_headers to client_id_header with both rest/realtime specific impl 2. Added method client_id_header_sync to realtime auth 3. Annotated implementation with right spec --- lib/ably/auth.rb | 31 ++++++++++++++++++------------- lib/ably/realtime/auth.rb | 4 ++++ lib/ably/realtime/connection.rb | 2 +- lib/ably/rest/client.rb | 4 ++-- 4 files changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 1e6e03fa..071d7178 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -411,11 +411,15 @@ def auth_header end # Extra headers that may be used during authentication - # + # spec - RSA7e # @return [Hash] headers - def extra_auth_headers - if client_id && using_basic_auth? - { 'X-Ably-ClientId' => Base64.urlsafe_encode64(client_id) } + def client_id_header(realtime=false) + if options[:client_id] && using_basic_auth? + if realtime + { 'clientId' => options[:client_id] } + else + { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } + end else {} end @@ -482,15 +486,16 @@ def can_assume_client_id?(assumed_client_id) # # @api private def configure_client_id(new_client_id) - # If new client ID from Ably is a wildcard, but preconfigured clientId is set, then keep the existing clientId - if has_client_id? && new_client_id == '*' - @client_id_validated = true - return - end - - # If client_id is defined and not a wildcard, prevent it changing, this is not supported - if client_id && client_id != '*' && new_client_id != client_id - raise Ably::Exceptions::IncompatibleClientId.new("Client ID is immutable once configured for a client. Client ID cannot be changed to '#{new_client_id}'") + if has_client_id? + # If new client ID from Ably is a wildcard, but preconfigured clientId is set, then keep the existing clientId + if new_client_id == "*" + @client_id_validated = true + return + end + # If client_id is defined and not a wildcard, prevent it changing, this is not supported + if new_client_id != client_id + raise Ably::Exceptions::IncompatibleClientId.new("Client ID is immutable once configured for a client. Client ID cannot be changed to '#{new_client_id}'") + end end @client_id_validated = true @client_id = new_client_id diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index 4d12f87a..c73b2635 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,6 +226,10 @@ def auth_header_sync auth_sync.auth_header end + def client_id_header_sync + auth_sync.client_id_header(true) + end + # Auth params used in URI endpoint for Realtime connections # Will reauthorize implicitly if required and capable # diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 008a2116..185d6065 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -470,7 +470,7 @@ def create_websocket_transport 'false' end - url_params['clientId'] = client.auth.client_id if client.auth.has_client_id? + url_params.merge!(client.auth.client_id_header_sync) # RSA7e1 url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? diff --git a/lib/ably/rest/client.rb b/lib/ably/rest/client.rb index 7aa34e3c..14fe66e7 100644 --- a/lib/ably/rest/client.rb +++ b/lib/ably/rest/client.rb @@ -597,8 +597,8 @@ def send_request(method, path, params, options) end unless options[:send_auth_header] == false request.headers[:authorization] = auth.auth_header - - options[:headers].to_h.merge(auth.extra_auth_headers).map do |key, val| + # RSA7e2 + options[:headers].to_h.merge(auth.client_id_header).map do |key, val| request.headers[key] = val end end From bb4b71f0abe7e17d58d1d10562fb7fd70a66f351 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 16:13:01 +0530 Subject: [PATCH 03/22] Fixed naming convention for client_id_header as per review comment --- lib/ably/auth.rb | 4 ++-- lib/ably/realtime/auth.rb | 2 +- lib/ably/realtime/connection.rb | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 071d7178..1fc3f0e3 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -413,9 +413,9 @@ def auth_header # Extra headers that may be used during authentication # spec - RSA7e # @return [Hash] headers - def client_id_header(realtime=false) + def client_id_header(realtime_params=false) if options[:client_id] && using_basic_auth? - if realtime + if realtime_params { 'clientId' => options[:client_id] } else { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index c73b2635..b36ab9c2 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,7 +226,7 @@ def auth_header_sync auth_sync.auth_header end - def client_id_header_sync + def client_id_params_sync auth_sync.client_id_header(true) end diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 185d6065..3d5fc31b 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -470,7 +470,7 @@ def create_websocket_transport 'false' end - url_params.merge!(client.auth.client_id_header_sync) # RSA7e1 + url_params.merge!(client.auth.client_id_params_sync) # RSA7e1 url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? From c10e485344ac4f87e7d83445a6ab3b9b287f1129 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 16:23:22 +0530 Subject: [PATCH 04/22] Removed unncessary use of auth clientId while entering presence local members --- lib/ably/realtime/presence/members_map.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index 49b006f6..de3bcebc 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -228,13 +228,11 @@ def setup_event_handlers def enter_local_members local_members.values.each do |member| - local_client_id = member.client_id || client.auth.client_id - logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{local_client_id} with data: #{member.data}" } - presence.enter_client_with_id(member.id, local_client_id, member.data).tap do |deferrable| + logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{member.client_id} with data: #{member.data}" } + presence.enter_client_with_id(member.id, member.client_id, member.data).tap do |deferrable| deferrable.errback do |error| - presence_message_client_id = member.client_id || client.auth.client_id re_enter_error = Ably::Models::ErrorInfo.new( - message: "unable to automatically re-enter presence channel for client_id '#{presence_message_client_id}'. Source error code #{error.code} and message '#{error.message}'", + message: "unable to automatically re-enter presence channel for client_id '#{member.client_id}'. Source error code #{error.code} and message '#{error.message}'", code: Ably::Exceptions::Codes::UNABLE_TO_AUTOMATICALLY_REENTER_PRESENCE_CHANNEL ) channel.emit :update, Ably::Models::ChannelStateChange.new( From 0f49edecd66af3e27b7fc5efb75c871e0dccf647 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 25 Jun 2024 16:29:04 +0530 Subject: [PATCH 05/22] renamed auth client_id_header to external_client_id --- lib/ably/auth.rb | 4 ++-- lib/ably/realtime/auth.rb | 4 ++-- lib/ably/realtime/connection.rb | 2 +- lib/ably/rest/client.rb | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index 1fc3f0e3..b654ea79 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -413,9 +413,9 @@ def auth_header # Extra headers that may be used during authentication # spec - RSA7e # @return [Hash] headers - def client_id_header(realtime_params=false) + def external_client_id(realtime=false) if options[:client_id] && using_basic_auth? - if realtime_params + if realtime { 'clientId' => options[:client_id] } else { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index b36ab9c2..55b1168f 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,8 +226,8 @@ def auth_header_sync auth_sync.auth_header end - def client_id_params_sync - auth_sync.client_id_header(true) + def external_client_id_sync + auth_sync.external_client_id(true) end # Auth params used in URI endpoint for Realtime connections diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 3d5fc31b..12669e8e 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -470,7 +470,7 @@ def create_websocket_transport 'false' end - url_params.merge!(client.auth.client_id_params_sync) # RSA7e1 + url_params.merge!(client.auth.external_client_id_sync) # RSA7e1 url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? diff --git a/lib/ably/rest/client.rb b/lib/ably/rest/client.rb index 14fe66e7..e1e18aef 100644 --- a/lib/ably/rest/client.rb +++ b/lib/ably/rest/client.rb @@ -598,7 +598,7 @@ def send_request(method, path, params, options) unless options[:send_auth_header] == false request.headers[:authorization] = auth.auth_header # RSA7e2 - options[:headers].to_h.merge(auth.client_id_header).map do |key, val| + options[:headers].to_h.merge(auth.external_client_id).map do |key, val| request.headers[key] = val end end From 5c3ff5b62702792818437746b66c041009372aca Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 1 Jul 2024 13:15:19 +0100 Subject: [PATCH 06/22] docs: GCM->FCM GCM transport is deprecated in favour of FCM --- lib/ably/rest/push/admin.rb | 2 +- spec/acceptance/rest/push_admin_spec.rb | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/lib/ably/rest/push/admin.rb b/lib/ably/rest/push/admin.rb index f6dc7cb6..a46cd2e0 100644 --- a/lib/ably/rest/push/admin.rb +++ b/lib/ably/rest/push/admin.rb @@ -21,7 +21,7 @@ def initialize(push) # Publish a push message directly to a single recipient # - # @param recipient [Hash] A recipient device, client_id or raw APNS/GCM target. Refer to push documentation + # @param recipient [Hash] A recipient device, client_id or raw APNS/FCM target. Refer to push documentation # @param data [Hash] The notification payload data and fields. Refer to push documentation # # @return [void] diff --git a/spec/acceptance/rest/push_admin_spec.rb b/spec/acceptance/rest/push_admin_spec.rb index 1df3c140..210e45de 100644 --- a/spec/acceptance/rest/push_admin_spec.rb +++ b/spec/acceptance/rest/push_admin_spec.rb @@ -181,7 +181,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registration_token: 'secret_token', } } @@ -250,7 +250,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registration_token: 'secret_token', } } @@ -268,7 +268,7 @@ expect(device).to be_a(Ably::Models::DeviceDetails) expect(device.platform).to eql('ios') expect(device.client_id).to eql(client_id) - expect(device.push.recipient.fetch(:transport_type)).to eql('gcm') + expect(device.push.recipient.fetch(:transport_type)).to eql('fcm') end it 'returns a DeviceDetails object if a DeviceDetails object is provided' do @@ -276,7 +276,7 @@ expect(device).to be_a(Ably::Models::DeviceDetails) expect(device.platform).to eql('ios') expect(device.client_id).to eql(client_id) - expect(device.push.recipient.fetch(:transport_type)).to eql('gcm') + expect(device.push.recipient.fetch(:transport_type)).to eql('fcm') end it 'raises a ResourceMissing exception if device ID does not exist' do @@ -350,14 +350,14 @@ expect(device_retrieved.push.recipient['foo_bar']).to eql('string') end - context 'with GCM target' do + context 'with FCM target' do let(:device_token) { random_str } it 'saves the associated DevicePushDetails' do subject.save(device_details.merge( push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registrationToken: device_token } } @@ -365,7 +365,7 @@ device_retrieved = subject.get(device_details.fetch(:id)) - expect(device_retrieved.push.recipient.fetch('transportType')).to eql('gcm') + expect(device_retrieved.push.recipient.fetch('transportType')).to eql('fcm') expect(device_retrieved.push.recipient[:registration_token]).to eql(device_token) end end @@ -462,7 +462,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registrationToken: 'secret_token', } } @@ -476,7 +476,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registration_token: 'secret_token', } } @@ -525,7 +525,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registration_token: 'secret_token', } } @@ -539,7 +539,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registration_token: 'secret_token', } } @@ -580,7 +580,7 @@ client_id: client_id, push: { recipient: { - transport_type: 'gcm', + transport_type: 'fcm', registration_token: 'secret_token', } } From 9a2134ba1b18c2dc273065101bb2a6597d94fd83 Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 1 Jul 2024 13:16:23 +0100 Subject: [PATCH 07/22] docs: add web as a push target --- lib/ably/rest/push/admin.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ably/rest/push/admin.rb b/lib/ably/rest/push/admin.rb index a46cd2e0..fcc177ea 100644 --- a/lib/ably/rest/push/admin.rb +++ b/lib/ably/rest/push/admin.rb @@ -21,7 +21,7 @@ def initialize(push) # Publish a push message directly to a single recipient # - # @param recipient [Hash] A recipient device, client_id or raw APNS/FCM target. Refer to push documentation + # @param recipient [Hash] A recipient device, client_id or raw APNS/FCM/web target. Refer to push documentation # @param data [Hash] The notification payload data and fields. Refer to push documentation # # @return [void] From f207c79280d7dd9f9662701868de348e06408cce Mon Sep 17 00:00:00 2001 From: owenpearson Date: Mon, 1 Jul 2024 13:44:57 +0100 Subject: [PATCH 08/22] test: update web push recipient structure with structured encryptionKey This was changed server-side so that the endpoint rejects web push recipients with a string encryptionKey. --- spec/acceptance/rest/push_admin_spec.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/spec/acceptance/rest/push_admin_spec.rb b/spec/acceptance/rest/push_admin_spec.rb index 210e45de..a5e1433f 100644 --- a/spec/acceptance/rest/push_admin_spec.rb +++ b/spec/acceptance/rest/push_admin_spec.rb @@ -372,7 +372,8 @@ context 'with web target' do let(:target_url) { 'http://foo.com/bar' } - let(:encryption_key) { random_str } + let(:p256dh) { random_str } + let(:auth) { random_str } it 'saves the associated DevicePushDetails' do subject.save(device_details.merge( @@ -380,7 +381,10 @@ recipient: { transport_type: 'web', targetUrl: target_url, - encryptionKey: encryption_key + encryptionKey: { + p256dh: p256dh, + auth: auth + } } } )) @@ -389,7 +393,8 @@ expect(device_retrieved.push.recipient[:transport_type]).to eql('web') expect(device_retrieved.push.recipient['targetUrl']).to eql(target_url) - expect(device_retrieved.push.recipient['encryptionKey']).to eql(encryption_key) + expect(device_retrieved.push.recipient['encryptionKey']['p256dh']).to eql(p256dh) + expect(device_retrieved.push.recipient['encryptionKey']['auth']).to eql(auth) end end From aee4e162631d29aad0bf1e1de7b4037ecaaeaaa2 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 1 Jul 2024 17:37:33 +0530 Subject: [PATCH 09/22] Removed unused/unnecessary realtime push activation methods with tests --- lib/ably/realtime/push.rb | 27 --------------------------- spec/acceptance/realtime/push_spec.rb | 27 --------------------------- 2 files changed, 54 deletions(-) delete mode 100644 spec/acceptance/realtime/push_spec.rb diff --git a/lib/ably/realtime/push.rb b/lib/ably/realtime/push.rb index b3ac400a..04d7e038 100644 --- a/lib/ably/realtime/push.rb +++ b/lib/ably/realtime/push.rb @@ -20,33 +20,6 @@ def initialize(client) def admin @admin ||= Admin.new(self) end - - # Activates the device for push notifications with FCM or APNS, obtaining a unique identifier from them. - # Subsequently registers the device with Ably and stores the deviceIdentityToken in local storage. - # - # @spec RSH2a - # - # @note This is unsupported in the Ruby library - # - def activate(*arg) - raise_unsupported - end - - # Deactivates the device from receiving push notifications with Ably and FCM or APNS. - # - # @spec RSH2b - # - # @note This is unsupported in the Ruby library - # - def deactivate(*arg) - raise_unsupported - end - - private - - def raise_unsupported - raise Ably::Exceptions::PushNotificationsNotSupported, 'This device does not support receiving or subscribing to push notifications. All PushChannel methods are unavailable' - end end end end diff --git a/spec/acceptance/realtime/push_spec.rb b/spec/acceptance/realtime/push_spec.rb deleted file mode 100644 index f5abb418..00000000 --- a/spec/acceptance/realtime/push_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# encoding: utf-8 -require 'spec_helper' - -describe Ably::Realtime::Push, :event_machine do - vary_by_protocol do - let(:default_options) { { key: api_key, environment: environment, protocol: protocol} } - let(:client_options) { default_options } - let(:client) do - Ably::Realtime::Client.new(client_options) - end - subject { client.push } - - describe '#activate' do - it 'raises an unsupported exception' do - expect { subject.activate('foo') }.to raise_error(Ably::Exceptions::PushNotificationsNotSupported) - stop_reactor - end - end - - describe '#deactivate' do - it 'raises an unsupported exception' do - expect { subject.deactivate('foo') }.to raise_error(Ably::Exceptions::PushNotificationsNotSupported) - stop_reactor - end - end - end -end From c6c7422103b806880fadc3ab33fae25cf04c2705 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 1 Jul 2024 18:43:41 +0530 Subject: [PATCH 10/22] Removed unused/unnecessary rest push activation methods with tests --- lib/ably/rest/push.rb | 19 ------------------- spec/acceptance/rest/push_spec.rb | 25 ------------------------- 2 files changed, 44 deletions(-) delete mode 100644 spec/acceptance/rest/push_spec.rb diff --git a/lib/ably/rest/push.rb b/lib/ably/rest/push.rb index a03d0400..7d33bb3a 100644 --- a/lib/ably/rest/push.rb +++ b/lib/ably/rest/push.rb @@ -20,25 +20,6 @@ def initialize(client) def admin @admin ||= Admin.new(self) end - - # Activate this device for push notifications by registering with the push transport such as GCM/APNS - # - # @note This is unsupported in the Ruby library - def activate(*arg) - raise_unsupported - end - - # Deactivate this device for push notifications by removing the registration with the push transport such as GCM/APNS - # - # @note This is unsupported in the Ruby library - def deactivate(*arg) - raise_unsupported - end - - private - def raise_unsupported - raise Ably::Exceptions::PushNotificationsNotSupported, 'This device does not support receiving or subscribing to push notifications. All PushChannel methods are unavailable' - end end end end diff --git a/spec/acceptance/rest/push_spec.rb b/spec/acceptance/rest/push_spec.rb deleted file mode 100644 index cbb9c5e8..00000000 --- a/spec/acceptance/rest/push_spec.rb +++ /dev/null @@ -1,25 +0,0 @@ -# encoding: utf-8 -require 'spec_helper' - -describe Ably::Rest::Push do - vary_by_protocol do - let(:default_options) { { key: api_key, environment: environment, protocol: protocol} } - let(:client_options) { default_options } - let(:client) do - Ably::Rest::Client.new(client_options) - end - subject { client.push } - - describe '#activate' do - it 'raises an unsupported exception' do - expect { subject.activate('foo') }.to raise_error(Ably::Exceptions::PushNotificationsNotSupported) - end - end - - describe '#deactivate' do - it 'raises an unsupported exception' do - expect { subject.deactivate('foo') }.to raise_error(Ably::Exceptions::PushNotificationsNotSupported) - end - end - end -end From b32bb579ba82fc70ad69e0e3a6f37e18c86e7834 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 2 Jul 2024 15:21:52 +0530 Subject: [PATCH 11/22] Fixed test that updated attach_serial everytime new attach msg is received --- spec/acceptance/realtime/channel_history_spec.rb | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/spec/acceptance/realtime/channel_history_spec.rb b/spec/acceptance/realtime/channel_history_spec.rb index a9caf7fc..128e562c 100644 --- a/spec/acceptance/realtime/channel_history_spec.rb +++ b/spec/acceptance/realtime/channel_history_spec.rb @@ -184,8 +184,11 @@ def ensure_message_history_direction_and_paging_is_correct(direction) end context 'when channel receives update event after an attachment' do + attach_serial = "old attach serial" + subsequent_serial = "new attach serial that needs to be updated" before do channel.on(:attached) do + attach_serial = channel.properties.attach_serial channel.publish(event, message_after_attach) do subsequent_serial = channel.properties.attach_serial.dup.tap { |serial| serial[-1] = '1' } attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 0, channel_serial: subsequent_serial) @@ -194,14 +197,13 @@ def ensure_message_history_direction_and_paging_is_correct(direction) end end - xit 'updates attach_serial' do + it 'updates attach_serial' do rest_channel.publish event, message_before_attach - channel.on(:update) do - channel.history(until_attach: true) do |messages| - expect(messages.items.count).to eql(2) - stop_reactor - end + new_attach_serial = channel.properties.attach_serial + expect(new_attach_serial).not_to eq(attach_serial) + expect(new_attach_serial).to eq(subsequent_serial) + stop_reactor end channel.attach From 1324a72a858910813514ae65bd66dd29b13adff7 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 2 Jul 2024 17:03:27 +0530 Subject: [PATCH 12/22] replaced next unless with assertive if condition for op retry on reconnection --- lib/ably/realtime/channel/channel_manager.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index bdf439fd..fbd12c4b 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -234,16 +234,16 @@ def send_state_change_protocol_message(new_state, state_if_failed, message_optio resend_if_disconnected_and_connected = lambda do connection.unsafe_once(:disconnected) do - next unless pending_state_change_timer connection.unsafe_once(:connected) do - next unless pending_state_change_timer - connection.send_protocol_message( - action: new_state.to_i, - channel: channel.name, - **message_options.to_h - ) - resend_if_disconnected_and_connected.call - end + if pending_state_change_timer + connection.send_protocol_message( + action: new_state.to_i, + channel: channel.name, + **message_options.to_h + ) + resend_if_disconnected_and_connected.call + end + end if pending_state_change_timer end end From 8b9b556e878b7871611ee6f621f72e9cbd229707 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Tue, 2 Jul 2024 18:35:59 +0530 Subject: [PATCH 13/22] Refactored implementation for sending ATTACH and DETACH message 1. Moved sending ATTACH msg as a part of separate codebase 2. DETEACH msg will be re-attempted on reconnection if pending timer exist 3. notify_state_change made accessible to channel_state_machine --- lib/ably/realtime/channel/channel_manager.rb | 79 +++++++++---------- .../realtime/channel/channel_state_machine.rb | 2 +- 2 files changed, 39 insertions(+), 42 deletions(-) diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index fbd12c4b..b46234d2 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -211,66 +211,63 @@ def send_attach_protocol_message(forced_attach = false) end message_options[:channelSerial] = channel.properties.channel_serial # RTL4c1 - send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended, message_options - end - - def send_detach_protocol_message(previous_state) - send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Detach, previous_state # return to previous state if failed - end - def notify_state_change - @pending_state_change_timer.cancel if @pending_state_change_timer - @pending_state_change_timer = nil + state_at_time_of_request = channel.state + attach_action = Ably::Models::ProtocolMessage::ACTION.Attach + @pending_state_change_timer = EventMachine::Timer.new(realtime_request_timeout) do + if channel.state == state_at_time_of_request + error = Ably::Models::ErrorInfo.new(code: Ably::Exceptions::Codes::CHANNEL_OPERATION_FAILED_NO_RESPONSE_FROM_SERVER, message: "Channel #{attach_action} operation failed (timed out)") + channel.transition_state_machine :suspended, reason: error # return to suspended state if failed + end + end + # Sends attach message only if it's forced_attach/connected_msg_received or + # connection state is connected. + if message_options.delete(:forced_attach) || connection.state?(:connected) + # Shouldn't queue attach message as per RTL4i, so message is added top of the queue + # to be sent immediately while processing next message + connection.send_protocol_message_immediately( + action: attach_action.to_i, + channel: channel.name, + **message_options.to_h + ) + end end - def send_state_change_protocol_message(new_state, state_if_failed, message_options = {}) + def send_detach_protocol_message(previous_state) state_at_time_of_request = channel.state + detach_action = Ably::Models::ProtocolMessage::ACTION.Detach + @pending_state_change_timer = EventMachine::Timer.new(realtime_request_timeout) do if channel.state == state_at_time_of_request - error = Ably::Models::ErrorInfo.new(code: Ably::Exceptions::Codes::CHANNEL_OPERATION_FAILED_NO_RESPONSE_FROM_SERVER, message: "Channel #{new_state} operation failed (timed out)") - channel.transition_state_machine state_if_failed, reason: error + error = Ably::Models::ErrorInfo.new(code: Ably::Exceptions::Codes::CHANNEL_OPERATION_FAILED_NO_RESPONSE_FROM_SERVER, message: "Channel #{detach_action} operation failed (timed out)") + channel.transition_state_machine previous_state, reason: error # return to previous state if failed end end - resend_if_disconnected_and_connected = lambda do + on_disconnected_and_connected = lambda do connection.unsafe_once(:disconnected) do connection.unsafe_once(:connected) do - if pending_state_change_timer - connection.send_protocol_message( - action: new_state.to_i, - channel: channel.name, - **message_options.to_h - ) - resend_if_disconnected_and_connected.call - end + yield if pending_state_change_timer end if pending_state_change_timer end end - # Attach is sent on every connected msg received as per RTN15c6, RTN15c7 - # So, no need to introduce logic that sends attach on disconnect and connect - if new_state == Ably::Models::ProtocolMessage::ACTION.Attach - # Sends attach message only if it's forced_attach/connected_msg_received or - # connection state is connected. - if message_options.delete(:forced_attach) || connection.state?(:connected) - # Shouldn't queue attach message as per RTL4i, so message is added top of the queue - # to be sent immediately while processing next message - connection.send_protocol_message_immediately( - action: new_state.to_i, - channel: channel.name, - **message_options.to_h - ) + send_detach_message = lambda do + on_disconnected_and_connected.call do + send_detach_message.call end - return + connection.send_protocol_message( + action: detach_action.to_i, + channel: channel.name + ) end - resend_if_disconnected_and_connected.call + send_detach_message.call + end - connection.send_protocol_message( - action: new_state.to_i, - channel: channel.name, - **message_options.to_h - ) + def notify_state_change + @pending_state_change_timer.cancel if @pending_state_change_timer + @pending_state_change_timer = nil end def update_presence_sync_state_following_attached(attached_protocol_message) diff --git a/lib/ably/realtime/channel/channel_state_machine.rb b/lib/ably/realtime/channel/channel_state_machine.rb index a6f67516..ef6889d4 100644 --- a/lib/ably/realtime/channel/channel_state_machine.rb +++ b/lib/ably/realtime/channel/channel_state_machine.rb @@ -29,7 +29,7 @@ class ChannelStateMachine transition :from => :failed, :to => [:attaching, :initialized] after_transition do |channel, transition| - channel.manager.send(:notify_state_change) + channel.manager.notify_state_change channel.synchronize_state_with_statemachine end From 1c8f52fb169f1f0e016142ef8ffa863c33d7f07c Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 16:22:22 +0530 Subject: [PATCH 14/22] refactored updated attach_serial test as per review comment --- spec/acceptance/realtime/channel_history_spec.rb | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/spec/acceptance/realtime/channel_history_spec.rb b/spec/acceptance/realtime/channel_history_spec.rb index 128e562c..c4617384 100644 --- a/spec/acceptance/realtime/channel_history_spec.rb +++ b/spec/acceptance/realtime/channel_history_spec.rb @@ -185,13 +185,12 @@ def ensure_message_history_direction_and_paging_is_correct(direction) context 'when channel receives update event after an attachment' do attach_serial = "old attach serial" - subsequent_serial = "new attach serial that needs to be updated" + new_attach_serial = "xxxx-xxxx-1" before do channel.on(:attached) do attach_serial = channel.properties.attach_serial channel.publish(event, message_after_attach) do - subsequent_serial = channel.properties.attach_serial.dup.tap { |serial| serial[-1] = '1' } - attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 0, channel_serial: subsequent_serial) + attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 0, channel_serial: new_attach_serial) client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message end end @@ -200,9 +199,8 @@ def ensure_message_history_direction_and_paging_is_correct(direction) it 'updates attach_serial' do rest_channel.publish event, message_before_attach channel.on(:update) do - new_attach_serial = channel.properties.attach_serial - expect(new_attach_serial).not_to eq(attach_serial) - expect(new_attach_serial).to eq(subsequent_serial) + expect(channel.properties.attach_serial).not_to eq(attach_serial) + expect(channel.properties.attach_serial).to eq(new_attach_serial) stop_reactor end From 1ff7d6b7877362714542a6fecd70654fa6136941 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 17:35:50 +0530 Subject: [PATCH 15/22] Added separate method client_id_for_request that returns client_id --- lib/ably/auth.rb | 19 +++++++++++-------- lib/ably/realtime/auth.rb | 4 ++-- lib/ably/realtime/connection.rb | 4 ++-- lib/ably/rest/client.rb | 2 +- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/ably/auth.rb b/lib/ably/auth.rb index b654ea79..0befcb27 100644 --- a/lib/ably/auth.rb +++ b/lib/ably/auth.rb @@ -411,20 +411,23 @@ def auth_header end # Extra headers that may be used during authentication - # spec - RSA7e + # # @return [Hash] headers - def external_client_id(realtime=false) - if options[:client_id] && using_basic_auth? - if realtime - { 'clientId' => options[:client_id] } - else - { 'X-Ably-ClientId' => Base64.urlsafe_encode64(options[:client_id]) } - end + def extra_auth_headers + if client_id_for_request + { 'X-Ably-ClientId' => Base64.urlsafe_encode64(client_id_for_request) } else {} end end + # ClientId that needs to be included with every rest/realtime request + # spec - RSA7e + # @return string + def client_id_for_request + options[:client_id] if options[:client_id] && using_basic_auth? + end + # Auth params used in URI endpoint for Realtime connections # Will reauthorize implicitly if required and capable # diff --git a/lib/ably/realtime/auth.rb b/lib/ably/realtime/auth.rb index 55b1168f..c1fc5983 100644 --- a/lib/ably/realtime/auth.rb +++ b/lib/ably/realtime/auth.rb @@ -226,8 +226,8 @@ def auth_header_sync auth_sync.auth_header end - def external_client_id_sync - auth_sync.external_client_id(true) + def client_id_for_request_sync + auth_sync.client_id_for_request end # Auth params used in URI endpoint for Realtime connections diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 12669e8e..c508ce92 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -469,8 +469,8 @@ def create_websocket_transport else 'false' end - - url_params.merge!(client.auth.external_client_id_sync) # RSA7e1 + # RSA7e1 + url_params['clientId'] = client.auth.client_id_for_request_sync if client.auth.client_id_for_request_sync url_params.merge!(client.transport_params) if !key.nil_or_empty? and connection_state_available? diff --git a/lib/ably/rest/client.rb b/lib/ably/rest/client.rb index e1e18aef..3dbda2df 100644 --- a/lib/ably/rest/client.rb +++ b/lib/ably/rest/client.rb @@ -598,7 +598,7 @@ def send_request(method, path, params, options) unless options[:send_auth_header] == false request.headers[:authorization] = auth.auth_header # RSA7e2 - options[:headers].to_h.merge(auth.external_client_id).map do |key, val| + options[:headers].to_h.merge(auth.extra_auth_headers).map do |key, val| request.headers[key] = val end end From 91e5f29b8324196225b113ce81b4fdd441780b67 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 17:37:48 +0530 Subject: [PATCH 16/22] Revert "Removed unncessary use of auth clientId while entering presence local members" This reverts commit c10e485344ac4f87e7d83445a6ab3b9b287f1129. --- lib/ably/realtime/presence/members_map.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index de3bcebc..49b006f6 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -228,11 +228,13 @@ def setup_event_handlers def enter_local_members local_members.values.each do |member| - logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{member.client_id} with data: #{member.data}" } - presence.enter_client_with_id(member.id, member.client_id, member.data).tap do |deferrable| + local_client_id = member.client_id || client.auth.client_id + logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{local_client_id} with data: #{member.data}" } + presence.enter_client_with_id(member.id, local_client_id, member.data).tap do |deferrable| deferrable.errback do |error| + presence_message_client_id = member.client_id || client.auth.client_id re_enter_error = Ably::Models::ErrorInfo.new( - message: "unable to automatically re-enter presence channel for client_id '#{member.client_id}'. Source error code #{error.code} and message '#{error.message}'", + message: "unable to automatically re-enter presence channel for client_id '#{presence_message_client_id}'. Source error code #{error.code} and message '#{error.message}'", code: Ably::Exceptions::Codes::UNABLE_TO_AUTOMATICALLY_REENTER_PRESENCE_CHANNEL ) channel.emit :update, Ably::Models::ChannelStateChange.new( From c8745a457a65b0a9645a7d1bc85c69c73121f476 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 17:42:53 +0530 Subject: [PATCH 17/22] Removed unncessary channel publish from the attach_serial test --- spec/acceptance/realtime/channel_history_spec.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/spec/acceptance/realtime/channel_history_spec.rb b/spec/acceptance/realtime/channel_history_spec.rb index c4617384..56f1b4a0 100644 --- a/spec/acceptance/realtime/channel_history_spec.rb +++ b/spec/acceptance/realtime/channel_history_spec.rb @@ -184,26 +184,22 @@ def ensure_message_history_direction_and_paging_is_correct(direction) end context 'when channel receives update event after an attachment' do - attach_serial = "old attach serial" + old_attach_serial = "" new_attach_serial = "xxxx-xxxx-1" before do channel.on(:attached) do - attach_serial = channel.properties.attach_serial - channel.publish(event, message_after_attach) do - attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 0, channel_serial: new_attach_serial) - client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message - end + old_attach_serial = channel.properties.attach_serial + attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 0, channel_serial: new_attach_serial) + client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message end end it 'updates attach_serial' do - rest_channel.publish event, message_before_attach channel.on(:update) do - expect(channel.properties.attach_serial).not_to eq(attach_serial) + expect(channel.properties.attach_serial).not_to eq(old_attach_serial) expect(channel.properties.attach_serial).to eq(new_attach_serial) stop_reactor end - channel.attach end end From 7be9e8c20c89b6f0ef0719cd5d41dd1fe073bfaa Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 18:43:25 +0530 Subject: [PATCH 18/22] Removed unncessary use of auth clientId while entering presence local members --- lib/ably/realtime/presence/members_map.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index 5d3c46c2..8183db52 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -154,13 +154,11 @@ def local_members def enter_local_members local_members.values.each do |member| - local_client_id = member.client_id || client.auth.client_id - logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{local_client_id} with data: #{member.data}" } - presence.enter_client_with_id(member.id, local_client_id, member.data).tap do |deferrable| + logger.debug { "#{self.class.name}: Manually re-entering local presence member, client ID: #{member.client_id} with data: #{member.data}" } + presence.enter_client_with_id(member.id, member.client_id, member.data).tap do |deferrable| deferrable.errback do |error| - presence_message_client_id = member.client_id || client.auth.client_id re_enter_error = Ably::Models::ErrorInfo.new( - message: "unable to automatically re-enter presence channel for client_id '#{presence_message_client_id}'. Source error code #{error.code} and message '#{error.message}'", + message: "unable to automatically re-enter presence channel for client_id '#{member.client_id}'. Source error code #{error.code} and message '#{error.message}'", code: Ably::Exceptions::Codes::UNABLE_TO_AUTOMATICALLY_REENTER_PRESENCE_CHANNEL ) channel.emit :update, Ably::Models::ChannelStateChange.new( From 2eba50ff6b161d98803fa71646720018e31e0efa Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 20:34:09 +0530 Subject: [PATCH 19/22] Refactored channel attach, removed check for forced attach --- lib/ably/realtime/channel.rb | 2 +- lib/ably/realtime/channel/channel_manager.rb | 33 ++++++++----------- .../realtime/connection/connection_manager.rb | 2 +- 3 files changed, 15 insertions(+), 22 deletions(-) diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index 162b153b..6543567d 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -365,7 +365,7 @@ def __incoming_msgbus__ def set_options(channel_options) @options = Ably::Models::ChannelOptions(channel_options) - manager.request_reattach if need_reattach? + manager.request_reattach if (need_reattach? and connection.state?(:connected)) end alias options= set_options diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index b46234d2..e4f53805 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -18,7 +18,7 @@ def initialize(channel, connection) def attach if can_transition_to?(:attached) connect_if_connection_initialized - send_attach_protocol_message + send_attach_protocol_message if connection.state?(:connected) end end @@ -49,14 +49,12 @@ def log_channel_error(error) end # Request channel to be reattached by sending an attach protocol message - # @param [Hash] options - # @option options [Ably::Models::ErrorInfo] :reason - def request_reattach(options = {}) - reason = options[:reason] - send_attach_protocol_message(options[:forced_attach]) - logger.debug { "Explicit channel reattach request sent to Ably due to #{reason}" } + # @param [Ably::Models::ErrorInfo] reason + def request_reattach(reason = nil) channel.set_channel_error_reason(reason) if reason channel.transition_state_machine! :attaching, reason: reason unless channel.attaching? + send_attach_protocol_message + logger.debug { "Explicit channel reattach request sent to Ably due to #{reason}" } end def duplicate_attached_received(protocol_message) @@ -201,9 +199,8 @@ def channel_retry_timeout connection.defaults.fetch(:channel_retry_timeout) end - def send_attach_protocol_message(forced_attach = false) + def send_attach_protocol_message message_options = {} - message_options[:forced_attach] = forced_attach message_options[:params] = channel.options.params if channel.options.params.any? message_options[:flags] = channel.options.modes_to_flags if channel.options.modes if channel.attach_resume @@ -220,17 +217,13 @@ def send_attach_protocol_message(forced_attach = false) channel.transition_state_machine :suspended, reason: error # return to suspended state if failed end end - # Sends attach message only if it's forced_attach/connected_msg_received or - # connection state is connected. - if message_options.delete(:forced_attach) || connection.state?(:connected) - # Shouldn't queue attach message as per RTL4i, so message is added top of the queue - # to be sent immediately while processing next message - connection.send_protocol_message_immediately( - action: attach_action.to_i, - channel: channel.name, - **message_options.to_h - ) - end + # Shouldn't queue attach message as per RTL4i, so message is added top of the queue + # to be sent immediately while processing next message + connection.send_protocol_message_immediately( + action: attach_action.to_i, + channel: channel.name, + **message_options.to_h + ) end def send_detach_protocol_message(previous_state) diff --git a/lib/ably/realtime/connection/connection_manager.rb b/lib/ably/realtime/connection/connection_manager.rb index 0743f995..b8213e97 100644 --- a/lib/ably/realtime/connection/connection_manager.rb +++ b/lib/ably/realtime/connection/connection_manager.rb @@ -583,7 +583,7 @@ def force_reattach_on_channels(error) channels.select do |channel| channel.attached? || channel.attaching? || channel.suspended? end.each do |channel| - channel.manager.request_reattach reason: error, forced_attach: true + channel.manager.request_reattach error end end From 7a973ae7718d2019544a61ddfe11e115cc0654ac Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 3 Jul 2024 20:52:57 +0530 Subject: [PATCH 20/22] Refactored send_protocol_message method, removed unnecessary usage of tap --- lib/ably/realtime/connection.rb | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/lib/ably/realtime/connection.rb b/lib/ably/realtime/connection.rb index 217462a0..d121b36d 100644 --- a/lib/ably/realtime/connection.rb +++ b/lib/ably/realtime/connection.rb @@ -431,20 +431,16 @@ def logger # @api private def send_protocol_message(protocol_message) add_message_serial_if_ack_required_to(protocol_message) do - Ably::Models::ProtocolMessage.new(protocol_message, logger: logger).tap do |message| - add_message_to_outgoing_queue message - notify_message_dispatcher_of_new_message message - logger.debug { "Connection: Prot msg queued =>: #{message.action} #{message}" } - end + message = Ably::Models::ProtocolMessage.new(protocol_message, logger: logger) + add_message_to_outgoing_queue(message) + notify_message_dispatcher_of_new_message message end end def send_protocol_message_immediately(protocol_message) - Ably::Models::ProtocolMessage.new(protocol_message, logger: logger).tap do |message| - add_message_to_outgoing_queue(message, true) - notify_message_dispatcher_of_new_message message - logger.debug { "Connection: Prot msg pushed at the top =>: #{message.action} #{message}" } - end + message = Ably::Models::ProtocolMessage.new(protocol_message, logger: logger) + add_message_to_outgoing_queue(message, true) + notify_message_dispatcher_of_new_message message end # @api private @@ -452,8 +448,10 @@ def add_message_to_outgoing_queue(protocol_message, send_immediately = false) if send_immediately # Adding msg at the top of the queue to get processed immediately while connection is CONNECTED __outgoing_message_queue__.prepend(protocol_message) + logger.debug { "Connection: protocol msg pushed at the top =>: #{message.action} #{message}" } else __outgoing_message_queue__ << protocol_message + logger.debug { "Connection: protocol msg queued =>: #{message.action} #{message}" } end end From 07aadbddb6a3988477571b404001c16e012fbf28 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Thu, 4 Jul 2024 13:00:41 +0530 Subject: [PATCH 21/22] Updated channel_manager, 1. marked notify_state_change as accessible 2. annotated code with right spec ids --- lib/ably/realtime/channel.rb | 2 +- lib/ably/realtime/channel/channel_manager.rb | 14 ++++++++------ lib/ably/realtime/channel/channel_state_machine.rb | 2 +- 3 files changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index 6543567d..af1772c0 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -364,7 +364,7 @@ def __incoming_msgbus__ # @return [Ably::Models::ChannelOptions] def set_options(channel_options) @options = Ably::Models::ChannelOptions(channel_options) - + # RTL4i manager.request_reattach if (need_reattach? and connection.state?(:connected)) end alias options= set_options diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index 11979735..90f00b9e 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -18,7 +18,7 @@ def initialize(channel, connection) def attach if can_transition_to?(:attached) connect_if_connection_initialized - send_attach_protocol_message if connection.state?(:connected) + send_attach_protocol_message if connection.state?(:connected) # RTL4i end end @@ -167,6 +167,12 @@ def start_attach_from_suspended_timer end end + # RTL13c + def notify_state_change + @pending_state_change_timer.cancel if @pending_state_change_timer + @pending_state_change_timer = nil + end + private attr_reader :pending_state_change_timer @@ -210,6 +216,7 @@ def send_attach_protocol_message state_at_time_of_request = channel.state attach_action = Ably::Models::ProtocolMessage::ACTION.Attach + # RTL4f @pending_state_change_timer = EventMachine::Timer.new(realtime_request_timeout) do if channel.state == state_at_time_of_request error = Ably::Models::ErrorInfo.new(code: Ably::Exceptions::Codes::CHANNEL_OPERATION_FAILED_NO_RESPONSE_FROM_SERVER, message: "Channel #{attach_action} operation failed (timed out)") @@ -257,11 +264,6 @@ def send_detach_protocol_message(previous_state) send_detach_message.call end - def notify_state_change - @pending_state_change_timer.cancel if @pending_state_change_timer - @pending_state_change_timer = nil - end - def logger connection.logger end diff --git a/lib/ably/realtime/channel/channel_state_machine.rb b/lib/ably/realtime/channel/channel_state_machine.rb index ef6889d4..e3cedd18 100644 --- a/lib/ably/realtime/channel/channel_state_machine.rb +++ b/lib/ably/realtime/channel/channel_state_machine.rb @@ -29,7 +29,7 @@ class ChannelStateMachine transition :from => :failed, :to => [:attaching, :initialized] after_transition do |channel, transition| - channel.manager.notify_state_change + channel.manager.notify_state_change # RTL13c channel.synchronize_state_with_statemachine end From 47c55d7f95bd9e437cd68482ad23587b3f71f7d8 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Thu, 4 Jul 2024 17:22:58 +0530 Subject: [PATCH 22/22] Added test to check for duplicate attach message sent or received --- spec/acceptance/realtime/channel_spec.rb | 28 ++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index d2c4250e..5c7bc06e 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -430,9 +430,35 @@ def disconnect_transport end context 'with connection state' do + + sent_attach_messages = [] + received_attached_messages = [] + before(:each) do + sent_attach_messages = [] + received_attached_messages = [] + client.connection.__outgoing_protocol_msgbus__.subscribe do |message| + if message.action == :attach + sent_attach_messages << message + end + end + client.connection.__incoming_protocol_msgbus__.subscribe do |message| + if message.action == :attached + received_attached_messages << message + end + end + end + + # Should send/receive attach/attached message only once + # No duplicates should be sent or received + let(:check_for_attach_messages) do + expect(sent_attach_messages.size).to eq(1) + expect(received_attached_messages.size).to eq(1) + end + it 'is initialized (#RTL4i)' do expect(connection).to be_initialized channel.attach do + check_for_attach_messages stop_reactor end end @@ -440,6 +466,7 @@ def disconnect_transport it 'is connecting (#RTL4i)' do connection.once(:connecting) do channel.attach do + check_for_attach_messages stop_reactor end end @@ -449,6 +476,7 @@ def disconnect_transport connection.once(:connected) do connection.once(:disconnected) do channel.attach do + check_for_attach_messages stop_reactor end end