From b8231d8ae32ed5c0859e39d071a5ec65146c5c9c Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:25:21 +0530 Subject: [PATCH 1/9] Removed unnecessary on_resume flag from channels_spec --- spec/unit/realtime/channels_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index 9fbc70b6f..a415a7144 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ably::Realtime::Channels do - let(:connection) { instance_double('Ably::Realtime::Connection', unsafe_on: true, on_resume: true) } + let(:connection) { instance_double('Ably::Realtime::Connection', unsafe_on: true) } let(:client) do instance_double('Ably::Realtime::Client', connection: connection, client_id: 'clientId', logger: double('logger').as_null_object) end From 8300ecde1b7ef1ed65cfcd615d717dd5fdb884b2 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:26:29 +0530 Subject: [PATCH 2/9] [Protocol 2] Fixed tests for realtime/channel_spec Made sure client is connected before doing further operations --- spec/acceptance/realtime/channel_spec.rb | 750 ++++++++++++++--------- 1 file changed, 449 insertions(+), 301 deletions(-) diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index 5c7bc06e6..4286c6a91 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -342,8 +342,10 @@ def disconnect_transport channel.attach end - channel.attach do - channel.detach + client.connection.once :connected do + channel.attach do + channel.detach + end end end end @@ -495,7 +497,9 @@ def disconnect_transport stop_reactor end - channel.attach + client.connection.once :connected do + channel.attach + end end end @@ -516,7 +520,9 @@ def disconnect_transport channel.detach end - channel.attach + client.connection.once :connected do + channel.attach + end end end end @@ -525,23 +531,29 @@ def disconnect_transport describe '#detach' do context 'when state is :attached' do it 'it detaches from a channel (#RTL5d)' do - channel.attach do + channel.once :attached do channel.detach channel.on(:detached) do expect(channel.state).to eq(:detached) stop_reactor end end + connection.once :connected do + channel.attach + end end it 'detaches from a channel and calls the provided block (#RTL5d, #RTL5e)' do - channel.attach do + channel.once :attached do expect(channel.state).to eq(:attached) channel.detach do expect(channel.state).to eq(:detached) stop_reactor end end + connection.once :connected do + channel.attach + end end it 'emits :detaching then :detached events' do @@ -551,26 +563,34 @@ def disconnect_transport end end - channel.attach do - channel.detach + connection.once :connected do + channel.attach do + channel.detach + end end end it 'returns a SafeDeferrable that catches exceptions in callbacks and logs them' do - channel.attach do + channel.once :attached do expect(channel.detach).to be_a(Ably::Util::SafeDeferrable) stop_reactor end + connection.once :connected do + channel.attach + end end it 'calls the Deferrable callback on success' do - channel.attach do + channel.once :attached do channel.detach.callback do expect(channel).to be_a(Ably::Realtime::Channel) expect(channel.state).to eq(:detached) stop_reactor end end + connection.once :connected do + channel.attach + end end context 'and DETACHED message is not received within realtime request timeout' do @@ -579,7 +599,6 @@ def disconnect_transport it 'fails the deferrable and returns to the previous state (#RTL5f, #RTL5e)' do channel.attach do - # don't process any incoming ProtocolMessages so the channel never becomes detached connection.__incoming_protocol_msgbus__.unsubscribe detached_requested_at = Time.now.to_i channel.detach do @@ -594,6 +613,7 @@ def disconnect_transport end end + context 'when state is :failed' do let(:client_options) { default_options.merge(log_level: :fatal) } @@ -637,15 +657,17 @@ def disconnect_transport channel.detach end - channel.attach do - channel.detach + connection.once :connected do + channel.attach do + channel.detach + end end end end context 'when state is :suspended' do it 'moves the channel state immediately to DETACHED state (#RTL5j)' do - channel.attach do + channel.once :attached do channel.once(:suspended) do channel.on do |channel_state_change| expect(channel_state_change.current).to eq(:detached) @@ -660,6 +682,9 @@ def disconnect_transport end channel.transition_state_machine :suspended end + connection.once :connected do + channel.attach + end end end @@ -683,7 +708,7 @@ def disconnect_transport context 'when state is :detached' do it 'does nothing as the channel is detached (#RTL5a)' do - channel.attach do + channel.once :attached do channel.detach do expect(channel).to be_detached channel.on do @@ -694,6 +719,9 @@ def disconnect_transport end end end + connection.once :connected do + channel.attach + end end end @@ -763,8 +791,10 @@ def disconnect_transport context 'initialized' do it 'does the detach operation once the connection state is connected (#RTL5h)' do expect(connection).to be_initialized + channel.on :attaching do + channel.detach + end channel.attach - channel.detach connection.once(:connected) do channel.once(:attached) do channel.once(:detached) do @@ -778,8 +808,10 @@ def disconnect_transport context 'connecting' do it 'does the detach operation once the connection state is connected (#RTL5h)' do connection.once(:connecting) do + channel.on :attaching do + channel.detach + end channel.attach - channel.detach connection.once(:connected) do channel.once(:attached) do channel.once(:detached) do @@ -798,8 +830,10 @@ def disconnect_transport it 'does the detach operation once the connection state is connected (#RTL5h)' do connection.once(:connected) do connection.once(:disconnected) do + channel.on :attaching do + channel.detach + end channel.attach - channel.detach connection.once(:connected) do channel.once(:attached) do channel.once(:detached) do @@ -905,84 +939,92 @@ def disconnect_transport describe '#(RTL17)' do context 'when channel is initialized' do it 'sends messages only on attach' do - expect(channel).to be_initialized - channel.publish('event', payload) + connection.once :connected do + expect(channel).to be_initialized + channel.publish('event', payload) - channel.subscribe do |message| - stop_reactor if message.data == payload && channel.attached? - end + channel.subscribe do |message| + stop_reactor if message.data == payload && channel.attached? + end - channel.attach + channel.attach + end end end context 'when channel is attaching' do it 'sends messages only on attach' do - channel.publish('event', payload) + connection.once :connected do + channel.publish('event', payload) - sent_message = nil - channel.subscribe do |message| - return if message.data != payload - sent_message = message + sent_message = nil + channel.subscribe do |message| + return if message.data != payload + sent_message = message - stop_reactor if channel.attached? - end + stop_reactor if channel.attached? + end - channel.on(:attaching) do - expect(channel).to be_attaching - expect(sent_message).to be_nil - end + channel.on(:attaching) do + expect(channel).to be_attaching + expect(sent_message).to be_nil + end - channel.attach + channel.attach + end end end context 'when channel is detaching' do it 'stops sending message' do - sent_message = nil - event_published = false - channel.subscribe do |message| - sent_message = message if message.data == payload - end + connection.once :connected do + sent_message = nil + event_published = false + channel.subscribe do |message| + sent_message = message if message.data == payload + end - channel.on(:detaching) do - channel.publish('event', payload) - event_published = true - end + channel.on(:detaching) do + channel.publish('event', payload) + event_published = true + end - channel.on(:detaching) do - EventMachine.next_tick do - expect(sent_message).to be_nil - stop_reactor if event_published + channel.on(:detaching) do + EventMachine.next_tick do + expect(sent_message).to be_nil + stop_reactor if event_published + end end - end - channel.attach do - channel.detach + channel.attach do + channel.detach + end end end end context 'when channel is detached' do it 'stops sending message' do - sent_message = nil - event_published = false - channel.subscribe do |message| - sent_message = message if message.data == payload - end + connection.once :connected do + sent_message = nil + event_published = false + channel.subscribe do |message| + sent_message = message if message.data == payload + end - channel.on(:detaching) do - channel.publish('event', payload) - event_published = true - end + channel.on(:detaching) do + channel.publish('event', payload) + event_published = true + end - channel.on(:detached) do - expect(sent_message).to be_nil - stop_reactor if event_published - end + channel.on(:detached) do + expect(sent_message).to be_nil + stop_reactor if event_published + end - channel.attach do - channel.detach + channel.attach do + channel.detach + end end end end @@ -996,8 +1038,10 @@ def disconnect_transport end end - channel.attach do - channel.transition_state_machine(:failed) + connection.once :connected do + channel.attach do + channel.transition_state_machine(:failed) + end end end end @@ -1047,7 +1091,7 @@ def disconnect_transport context 'when channel is Detaching (#RTL6c1)' do it 'publishes messages immediately (#RTL6c1)' do - sub_channel.attach do + sub_channel.once :attached do channel.attach do channel.once(:detaching) do outgoing_message_count = 0 @@ -1069,32 +1113,37 @@ def disconnect_transport channel.detach end end + connection.once :connected do + sub_channel.attach + end end end context 'when channel is Detached (#RTL6c1)' do it 'publishes messages immediately (#RTL6c1)' do - sub_channel.attach do - channel.attach - channel.once(:attached) do - channel.once(:detached) do - outgoing_message_count = 0 - client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| - if protocol_message.action == :message - raise "Expected channel state to be attaching when publishing messages, not #{channel.state}" unless channel.detached? - outgoing_message_count += protocol_message.messages.count + connection.once :connected do + sub_channel.attach do + channel.attach + channel.once(:attached) do + channel.once(:detached) do + outgoing_message_count = 0 + client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| + if protocol_message.action == :message + raise "Expected channel state to be attaching when publishing messages, not #{channel.state}" unless channel.detached? + outgoing_message_count += protocol_message.messages.count + end end - end - sub_channel.subscribe do |message| - messages << message if message.name == 'event' - if messages.count == 3 - expect(outgoing_message_count).to eql(3) - stop_reactor + sub_channel.subscribe do |message| + messages << message if message.name == 'event' + if messages.count == 3 + expect(outgoing_message_count).to eql(3) + stop_reactor + end end + 3.times { channel.publish('event', random_str) } end - 3.times { channel.publish('event', random_str) } + channel.detach end - channel.detach end end end @@ -1457,12 +1506,14 @@ def disconnect_transport context 'with a valid client_id in the message' do it 'succeeds' do - channel.publish([name: 'event', client_id: 'validClient']).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to eql('validClient') - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: 'validClient']).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to eql('validClient') + EM.add_timer(0.5) { stop_reactor } + end end end end @@ -1483,12 +1534,14 @@ def disconnect_transport context 'with an empty client_id in the message' do it 'succeeds and publishes without a client_id' do - channel.publish([name: 'event', client_id: nil]).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to be_nil - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: nil]).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to be_nil + EM.add_timer(0.5) { stop_reactor } + end end end end @@ -1503,20 +1556,25 @@ def disconnect_transport context 'before the client is CONNECTED and the client\'s identity has been obtained' do context 'with a valid client_id in the message' do it 'succeeds' do - channel.publish([name: 'event', client_id: 'valid']).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to eql('valid') - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: 'valid']).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to eql('valid') + EM.add_timer(0.5) { stop_reactor } + end end end end context 'with an invalid client_id in the message' do let(:client_options) { default_options.merge(key: nil, token: token, log_level: :error) } - it 'succeeds in the client library but then fails when delivered to Ably' do + it 'succeeds in the client library ( while connecting ) but then fails when delivered to Ably' do channel.publish([name: 'event', client_id: 'invalid']).tap do |deferrable| + deferrable.errback do |err| + expect(err).to be_truthy + end EM.add_timer(0.5) { stop_reactor } end channel.subscribe('event') do |message| @@ -1527,12 +1585,14 @@ def disconnect_transport context 'with an empty client_id in the message' do it 'succeeds and publishes with an implicit client_id' do - channel.publish([name: 'event', client_id: nil]).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to eql('valid') - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: nil]).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to eql('valid') + EM.add_timer(0.5) { stop_reactor } + end end end end @@ -1586,12 +1646,14 @@ def disconnect_transport context 'with a valid client_id' do it 'succeeds' do - channel.publish([name: 'event', client_id: 'valid']).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to eql('valid') - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: 'valid']).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to eql('valid') + EM.add_timer(0.5) { stop_reactor } + end end end end @@ -1612,12 +1674,14 @@ def disconnect_transport context 'with an empty client_id in the message' do it 'succeeds and publishes with an implicit client_id' do - channel.publish([name: 'event', client_id: nil]).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to eql('valid') - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: nil]).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to eql('valid') + EM.add_timer(0.5) { stop_reactor } + end end end end @@ -1645,12 +1709,14 @@ def disconnect_transport context 'with an empty client_id in the message' do it 'succeeds and publishes with an implicit client_id' do - channel.publish([name: 'event', client_id: nil]).tap do |deferrable| - deferrable.errback { raise 'Should have succeeded' } - end - channel.subscribe('event') do |message| - expect(message.client_id).to be_nil - EM.add_timer(0.5) { stop_reactor } + connection.once :connected do + channel.publish([name: 'event', client_id: nil]).tap do |deferrable| + deferrable.errback { raise 'Should have succeeded' } + end + channel.subscribe('event') do |message| + expect(message.client_id).to be_nil + EM.add_timer(0.5) { stop_reactor } + end end end end @@ -1794,18 +1860,20 @@ def disconnect_transport let(:exception) { StandardError.new("Intentional error") } it 'logs the error and continues' do - emitted_exception = false - expect(client.logger).to receive(:error) do |*args, &block| - expect(args.concat([block ? block.call : nil]).join(',')).to match(/#{exception.message}/) - end - channel.subscribe('click') do |message| - emitted_exception = true - raise exception - end - channel.publish('click', 'data') do - EventMachine.add_timer(1) do - expect(emitted_exception).to eql(true) - stop_reactor + connection.once :connected do + emitted_exception = false + expect(client.logger).to receive(:error) do |*args, &block| + expect(args.concat([block ? block.call : nil]).join(',')).to match(/#{exception.message}/) + end + channel.subscribe('click') do |message| + emitted_exception = true + raise exception + end + channel.publish('click', 'data') do + EventMachine.add_timer(1) do + expect(emitted_exception).to eql(true) + stop_reactor + end end end end @@ -1813,19 +1881,21 @@ def disconnect_transport context 'many times with different event names' do it 'filters events accordingly to each callback' do - click_callback = lambda { |message| messages << message } + connection.once :connected do + click_callback = lambda { |message| messages << message } - channel.subscribe('click', &click_callback) - channel.subscribe('move', &click_callback) - channel.subscribe('press', &click_callback) + channel.subscribe('click', &click_callback) + channel.subscribe('move', &click_callback) + channel.subscribe('press', &click_callback) - channel.attach do - channel.publish('click', 'data') - channel.publish('move', 'data') - channel.publish('press', 'data') do - EventMachine.add_timer(2) do - expect(messages.count).to eql(3) - stop_reactor + channel.attach do + channel.publish('click', 'data') + channel.publish('move', 'data') + channel.publish('press', 'data') do + EventMachine.add_timer(2) do + expect(messages.count).to eql(3) + stop_reactor + end end end end @@ -1836,12 +1906,14 @@ def disconnect_transport describe '#unsubscribe' do context 'with an event argument' do it 'unsubscribes for a single event' do - channel.subscribe('click') { raise 'Should not have been called' } - channel.unsubscribe('click') + connection.once :connected do + channel.subscribe('click') { raise 'Should not have been called' } + channel.unsubscribe('click') - channel.publish('click', 'data') do - EventMachine.add_timer(1) do - stop_reactor + channel.publish('click', 'data') do + EventMachine.add_timer(1) do + stop_reactor + end end end end @@ -1849,12 +1921,14 @@ def disconnect_transport context 'with no event argument' do it 'unsubscribes for a single event' do - channel.subscribe { raise 'Should not have been called' } - channel.unsubscribe + connection.once :connected do + channel.subscribe { raise 'Should not have been called' } + channel.unsubscribe - channel.publish('click', 'data') do - EventMachine.add_timer(1) do - stop_reactor + channel.publish('click', 'data') do + EventMachine.add_timer(1) do + stop_reactor + end end end end @@ -1889,33 +1963,37 @@ def fake_error(error) context 'an :attached channel' do it 'transitions state to :failed (#RTL3a)' do - channel.attach do - channel.on(:failed) do |connection_state_change| - error = connection_state_change.reason - expect(error).to be_a(Ably::Exceptions::ConnectionFailed) - expect(error.code).to eql(50000) - stop_reactor + connection.once :connected do + channel.attach do + channel.on(:failed) do |connection_state_change| + error = connection_state_change.reason + expect(error).to be_a(Ably::Exceptions::ConnectionFailed) + expect(error.code).to eql(50000) + stop_reactor + end + fake_error connection_error end - fake_error connection_error end end it 'updates the channel error_reason (#RTL3a)' do - channel.attach do - channel.on(:failed) do |connection_state_change| - error = connection_state_change.reason - expect(error).to be_a(Ably::Exceptions::ConnectionFailed) - expect(error.code).to eql(50000) - stop_reactor + connection.once :connected do + channel.attach do + channel.on(:failed) do |connection_state_change| + error = connection_state_change.reason + expect(error).to be_a(Ably::Exceptions::ConnectionFailed) + expect(error.code).to eql(50000) + stop_reactor + end + fake_error connection_error end - fake_error connection_error end end end context 'a :detached channel' do it 'remains in the :detached state (#RTL3a)' do - channel.attach do + channel.once :attached do channel.on(:failed) { raise 'Failed state should not have been reached' } channel.detach do @@ -1927,6 +2005,10 @@ def fake_error(error) fake_error connection_error end end + + connection.once :connected do + channel.attach + end end end @@ -1934,20 +2016,22 @@ def fake_error(error) let(:original_error) { RuntimeError.new } it 'remains in the :failed state and ignores the failure error (#RTL3a)' do - channel.attach do - channel.on(:failed) do - channel.on(:failed) { raise 'Failed state should not have been reached' } + connection.once :connected do + channel.attach do + channel.on(:failed) do + channel.on(:failed) { raise 'Failed state should not have been reached' } - EventMachine.add_timer(1) do - expect(channel).to be_failed - expect(channel.error_reason).to eql(original_error) - stop_reactor + EventMachine.add_timer(1) do + expect(channel).to be_failed + expect(channel.error_reason).to eql(original_error) + stop_reactor + end + + fake_error connection_error end - fake_error connection_error + channel.transition_state_machine :failed, reason: original_error end - - channel.transition_state_machine :failed, reason: original_error end end end @@ -1970,11 +2054,13 @@ def fake_error(error) context ':closed' do context 'an :attached channel' do it 'transitions state to :detached (#RTL3b)' do - channel.attach do - channel.on(:detached) do - stop_reactor + connection.once :connected do + channel.attach do + channel.on(:detached) do + stop_reactor + end + client.connection.close end - client.connection.close end end end @@ -1990,13 +2076,15 @@ def fake_error(error) closed_message = Ably::Models::ProtocolMessage.new(action: 8) # CLOSED client.connection.__incoming_protocol_msgbus__.publish :protocol_message, closed_message end - channel.attach + connection.once :connected do + channel.attach + end end end context 'a :detached channel' do it 'remains in the :detached state (#RTL3b)' do - channel.attach do + channel.once :attached do channel.detach do channel.on(:detached) { raise 'Detached state should not have been reached' } @@ -2008,6 +2096,9 @@ def fake_error(error) client.connection.close end end + connection.once :connected do + channel.attach + end end end @@ -2016,20 +2107,22 @@ def fake_error(error) let(:original_error) { Ably::Models::ErrorInfo.new(message: 'Error') } it 'remains in the :failed state and retains the error_reason (#RTL3b)' do - channel.attach do - channel.once(:failed) do - channel.on(:detached) { raise 'Detached state should not have been reached' } + connection.on :connected do + channel.attach do + channel.once(:failed) do + channel.on(:detached) { raise 'Detached state should not have been reached' } - EventMachine.add_timer(1) do - expect(channel).to be_failed - expect(channel.error_reason).to eql(original_error) - stop_reactor + EventMachine.add_timer(1) do + expect(channel).to be_failed + expect(channel.error_reason).to eql(original_error) + stop_reactor + end + + client.connection.close end - client.connection.close + channel.transition_state_machine :failed, reason: original_error end - - channel.transition_state_machine :failed, reason: original_error end end end @@ -2074,23 +2167,26 @@ def fake_error(error) client.connection.transition_state_machine :suspended end end - channel.attach + channel.attach end end context 'an :attached channel' do it 'transitions state to :suspended (#RTL3c)' do - channel.attach do + channel.once :attached do channel.on(:suspended) do stop_reactor end client.connection.transition_state_machine :suspended end + connection.once :connected do + channel.attach + end end describe 'reattaching (#RTN15c3)' do it 'transitions state automatically to :attaching once the connection is re-established ' do - channel.attach do + channel.once :attached do channel.on(:suspended) do client.connection.connect channel.once(:attached) do @@ -2099,10 +2195,13 @@ def fake_error(error) end client.connection.transition_state_machine :suspended end + connection.once :connected do + channel.attach + end end it 'sends ATTACH_RESUME flag when reattaching (RTL4j)' do - channel.attach do + channel.once :attached do channel.on(:suspended) do client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| next if protocol_message.action != :attach @@ -2115,13 +2214,16 @@ def fake_error(error) end client.connection.transition_state_machine :suspended end + connection.once :connected do + channel.attach + end end end end context 'a :detached channel' do it 'remains in the :detached state (#RTL3c)' do - channel.attach do + channel.once :attached do channel.detach do channel.on(:detached) { raise 'Detached state should not have been reached' } @@ -2133,6 +2235,9 @@ def fake_error(error) client.connection.transition_state_machine :suspended end end + connection.once :connected do + channel.attach + end end end @@ -2141,20 +2246,22 @@ def fake_error(error) let(:client_options) { default_options.merge(log_level: :fatal) } it 'remains in the :failed state and retains the error_reason (#RTL3c)' do - channel.attach do - channel.once(:failed) do - channel.on(:detached) { raise 'Detached state should not have been reached' } + connection.once :connected do + channel.attach do + channel.once(:failed) do + channel.on(:detached) { raise 'Detached state should not have been reached' } - EventMachine.add_timer(1) do - expect(channel).to be_failed - expect(channel.error_reason).to eql(original_error) - stop_reactor + EventMachine.add_timer(1) do + expect(channel).to be_failed + expect(channel.error_reason).to eql(original_error) + stop_reactor + end + + client.connection.transition_state_machine :suspended end - client.connection.transition_state_machine :suspended + channel.transition_state_machine :failed, reason: original_error end - - channel.transition_state_machine :failed, reason: original_error end end end @@ -2179,14 +2286,16 @@ def fake_error(error) context ':connected' do context 'a :suspended channel' do it 'is automatically reattached (#RTL3d)' do - channel.attach do - channel.once(:suspended) do - client.connection.connect - channel.once(:attached) do - stop_reactor + connection.once :connected do + channel.attach do + channel.once(:suspended) do + client.connection.connect + channel.once(:attached) do + stop_reactor + end end + client.connection.transition_state_machine :suspended end - client.connection.transition_state_machine :suspended end end @@ -2196,29 +2305,32 @@ def fake_error(error) end it 'returns to a suspended state (#RTL3d)' do - channel.attach do - channel.once(:attached) do - fail "Channel should not have become attached" - end + connection.once :connected do + channel.attach do + channel.once(:attached) do + fail "Channel should not have become attached" + end - channel.once(:suspended) do - client.connection.connect - channel.once(:attaching) do - # don't process any incoming ProtocolMessages so the connection never opens - client.connection.__incoming_protocol_msgbus__.unsubscribe - channel.once(:suspended) do |state_change| - expect(state_change.reason.code).to eql(90007) - stop_reactor + channel.once(:suspended) do + client.connection.connect + channel.once(:attaching) do + # don't process any incoming ProtocolMessages so the connection never opens + client.connection.__incoming_protocol_msgbus__.unsubscribe + channel.once(:suspended) do |state_change| + expect(state_change.reason.code).to eql(90007) + stop_reactor + end end end + client.connection.transition_state_machine :suspended end - client.connection.transition_state_machine :suspended end end end end end + context ':disconnected' do context 'with an initialized channel' do it 'has no effect on the channel states (#RTL3e)' do @@ -2250,25 +2362,31 @@ def fake_error(error) context 'with an attached channel' do it 'has no effect on the channel states (#RTL3e)' do - channel.attach do + channel.once :attached do connection.once(:disconnected) do expect(channel).to be_attached stop_reactor end disconnect_transport end + + connection.once :connected do + channel.attach + end end end context 'with a detached channel' do it 'has no effect on the channel states (#RTL3e)' do - channel.attach do - channel.detach do - connection.once(:disconnected) do - expect(channel).to be_detached - stop_reactor + connection.once :connected do + channel.attach do + channel.detach do + connection.once(:disconnected) do + expect(channel).to be_detached + stop_reactor + end + disconnect_transport end - disconnect_transport end end end @@ -2316,12 +2434,12 @@ def self.build_flags(flags) shared_examples 'an update that sends ATTACH message' do |state, flags| it 'sends an ATTACH message on options change' do - attach_sent = nil + attach_sent_with_flags_set_via_channel_options = nil client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| if protocol_message.action == :attach && protocol_message.flags.nonzero? - attach_sent = true expect(protocol_message.flags).to eq(flags) + attach_sent_with_flags_set_via_channel_options = true end end @@ -2330,10 +2448,7 @@ def self.build_flags(flags) end channel.on(:attached) do - client.connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| - next if protocol_message.action != :attached - - expect(attach_sent).to eq(true) + wait_until(lambda { attach_sent_with_flags_set_via_channel_options }) do stop_reactor end end @@ -2346,7 +2461,7 @@ def self.build_flags(flags) it_behaves_like 'an update that sends ATTACH message', :attaching, build_flags(%i[subscribe]) end - context 'when channel is attaching' do + context 'when channel is attached' do it_behaves_like 'an update that sends ATTACH message', :attached, build_flags(%i[resume subscribe]) end @@ -2417,8 +2532,10 @@ def self.build_flags(flags) expect(channel_state_change.reason).to be_nil stop_reactor end - channel.attach do - channel.detach + connection.once :connected do + channel.attach do + channel.detach + end end end @@ -2454,7 +2571,7 @@ def self.build_flags(flags) connection_id = client.connection.id expect(channel_state_change.resumed).to be_falsey - recover_client = auto_close Ably::Realtime::Client.new(client_options.merge(recover: client.connection.recovery_key)) + recover_client = auto_close Ably::Realtime::Client.new(client_options.merge(recover: client.connection.create_recovery_key)) recover_client.connection.once(:connected) do expect(recover_client.connection.id).to eql(connection_id) recover_channel = recover_client.channels.get(channel_name) @@ -2469,7 +2586,7 @@ def self.build_flags(flags) it 'is false when a connection fails to recover and the channel is attached' do client.connection.once(:connected) do - recovery_key = client.connection.recovery_key + recovery_key = client.connection.create_recovery_key client.connection.once(:closed) do recover_client = auto_close Ably::Realtime::Client.new(client_options.merge(recover: recovery_key, log_level: :error)) recover_client.connection.once(:connected) do @@ -2486,11 +2603,11 @@ def self.build_flags(flags) end end - context 'when a resume fails' do + context 'when a connection resume fails' do let(:client_options) { default_options.merge(log_level: :error) } - it 'is false when a resume fails to recover and the channel is automatically re-attached' do - channel.attach do + it 'is false when channel_serial goes nil (RTP5a1) and the channel is automatically re-attached' do + channel.once :attached do connection_id = client.connection.id channel.once(:attached) do |channel_state_change| expect(client.connection.id).to_not eql(connection_id) @@ -2498,7 +2615,29 @@ def self.build_flags(flags) stop_reactor end client.connection.transport.close_connection_after_writing - client.connection.configure_new '0123456789abcdef', 'wVIsgTHAB1UvXh7z-1991d8586', -1 # force the resume connection key to be invalid + channel.properties.channel_serial = nil + client.connection.configure_new '0123456789abcdef', 'wVIsgTHAB1UvXh7z-1991d8586' # force the resume connection key to be invalid + end + + connection.once :connected do + channel.attach + end + end + + it 'is true when channel_serial is intact and the channel is automatically re-attached' do + channel.once :attached do + connection_id = client.connection.id + channel.once(:attached) do |channel_state_change| + expect(client.connection.id).to_not eql(connection_id) + expect(channel_state_change.resumed).to be_truthy + stop_reactor + end + client.connection.transport.close_connection_after_writing + client.connection.configure_new '0123456789abcdef', 'wVIsgTHAB1UvXh7z-1991d8586' # force the resume connection key to be invalid + end + + connection.once :connected do + channel.attach end end end @@ -2629,7 +2768,9 @@ def self.build_flags(flags) client.connection.__incoming_protocol_msgbus__.publish :protocol_message, detach_message end - channel.attach + connection.once :connected do + channel.attach + end end end @@ -2660,26 +2801,30 @@ def self.build_flags(flags) client.connection.__incoming_protocol_msgbus__.publish :protocol_message, detach_message end - channel.attach + connection.once :connected do + channel.attach + end end context 'when connection is no longer connected' do it 'will not attempt to reattach (#RTL13c)' do - channel.attach do - connection.once(:closing) do - channel.once(:attaching) do |state_change| - raise 'Channel should not attempt to reattach' + connection.once :connected do + channel.attach do + connection.once(:closing) do + channel.once(:attaching) do |state_change| + raise 'Channel should not attempt to reattach' + end + + channel.transition_state_machine! :suspended end - channel.transition_state_machine! :suspended - end + connection.once(:closed) do + expect(channel).to be_suspended + stop_reactor + end - connection.once(:closed) do - expect(channel).to be_suspended - stop_reactor + connection.close end - - connection.close end end end @@ -2700,35 +2845,36 @@ def self.build_flags(flags) end end prevent_protocol_messages_proc.call - end - channel.once(:attaching) do - attaching_at = Time.now - # First attaching fails during server-initiated ATTACHED received - channel.once(:suspended) do |state_change| - expect(Time.now.to_i - attaching_at.to_i).to be_within(1).of(1) - - suspended_at = Time.now - # Automatic attach happens at channel_retry_timeout - channel.once(:attaching) do - expect(Time.now.to_i - attaching_at.to_i).to be_within(1).of(2) - channel.once(:suspended) do - channel.once(:attaching) do - channel.once(:attached) do - stop_reactor + channel.once(:attaching) do + attaching_at = Time.now + # First attaching fails during server-initiated ATTACHED received + channel.once(:suspended) do |state_change| + expect(Time.now.to_i - attaching_at.to_i).to be_within(1).of(1) + + suspended_at = Time.now + # Automatic attach happens at channel_retry_timeout + channel.once(:attaching) do + expect(Time.now.to_i - attaching_at.to_i).to be_within(1).of(2) + channel.once(:suspended) do + channel.once(:attaching) do + channel.once(:attached) do + stop_reactor + end + # Simulate ATTACHED from Ably + attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name) # ATTACHED + client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message end - # Simulate ATTACHED from Ably - attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name) # ATTACHED - client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message end end end + + detach_message = Ably::Models::ProtocolMessage.new(action: detached_action, channel: channel_name) + client.connection.__incoming_protocol_msgbus__.publish :protocol_message, detach_message end - detach_message = Ably::Models::ProtocolMessage.new(action: detached_action, channel: channel_name) - client.connection.__incoming_protocol_msgbus__.publish :protocol_message, detach_message + channel.attach end - channel.attach end end end @@ -2737,14 +2883,16 @@ def self.build_flags(flags) let(:client_options) { default_options.merge(log_level: :fatal) } it 'should transition to the failed state and the error_reason should be set (#RTL14)' do - channel.attach do - channel.once(:failed) do |state_change| - expect(state_change.reason.code).to eql(50505) - expect(channel.error_reason.code).to eql(50505) - stop_reactor + connection.once :connected do + channel.attach do + channel.once(:failed) do |state_change| + expect(state_change.reason.code).to eql(50505) + expect(channel.error_reason.code).to eql(50505) + stop_reactor + end + error_message = Ably::Models::ProtocolMessage.new(action: 9, channel: channel_name, error: { code: 50505 }) # ProtocolMessage ERROR type + client.connection.__incoming_protocol_msgbus__.publish :protocol_message, error_message end - error_message = Ably::Models::ProtocolMessage.new(action: 9, channel: channel_name, error: { code: 50505 }) # ProtocolMessage ERROR type - client.connection.__incoming_protocol_msgbus__.publish :protocol_message, error_message end end end From fc69791fbd06462c66d69ae45e677ab799d1c8fb Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:26:59 +0530 Subject: [PATCH 3/9] [Protocol 2] Fixed tests for realtime/connection_failures_spec --- .../realtime/connection_failures_spec.rb | 33 +++++-------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/spec/acceptance/realtime/connection_failures_spec.rb b/spec/acceptance/realtime/connection_failures_spec.rb index 21f74b230..6afb7616a 100644 --- a/spec/acceptance/realtime/connection_failures_spec.rb +++ b/spec/acceptance/realtime/connection_failures_spec.rb @@ -747,8 +747,6 @@ def send_disconnect_message resumed_connection = false connection.once(:disconnected) do - disconnected_at = Time.now - allow(connection).to receive(:time_since_connection_confirmed_alive?).and_return(connection.connection_state_ttl + 1) # Make sure the next connect does not have the resume param @@ -781,8 +779,6 @@ def send_disconnect_message resumed_with_clean_connection = false connection.once(:disconnected) do - disconnected_at = Time.now - pseudo_time_passed = connection.connection_state_ttl + connection.details.max_idle_interval + 1 allow(connection).to receive(:time_since_connection_confirmed_alive?).and_return(pseudo_time_passed) @@ -815,14 +811,11 @@ def send_disconnect_message channel_emitted_an_attached = false channel.attach do - channel.once(:attached) do |channel_state_change| - expect(channel_state_change.resumed).to be_falsey + channel.once(:attached) do channel_emitted_an_attached = true end connection.once(:disconnected) do - disconnected_at = Time.now - pseudo_time_passed = connection.connection_state_ttl + connection.details.max_idle_interval + 1 allow(connection).to receive(:time_since_connection_confirmed_alive?).and_return(pseudo_time_passed) @@ -955,7 +948,7 @@ def send_disconnect_message previous_connection_id = connection.id connection.transport.close_connection_after_writing - expect(connection).to receive(:configure_new).with(previous_connection_id, anything, anything).and_call_original + expect(connection).to receive(:configure_new).with(previous_connection_id, anything).and_call_original connection.once(:connected) do expect(connection.key).to_not be_nil @@ -1008,16 +1001,6 @@ def send_disconnect_message end end - it 'executes the resume callback', api_private: true do - channel.attach do - connection.transport.close_connection_after_writing - connection.on_resume do - expect(connection).to be_connected - stop_reactor - end - end - end - context 'when messages were published whilst the client was disconnected' do it 'receives the messages published whilst offline' do messages_received = false @@ -1089,7 +1072,7 @@ def send_disconnect_message def kill_connection_transport_and_prevent_valid_resume connection.transport.close_connection_after_writing - connection.configure_new '0123456789abcdef', 'wVIsgTHAB1UvXh7z-1991d8586', -1 # force the resume connection key to be invalid + connection.configure_new '0123456789abcdef', '0123456789abcdef-99' # force the resume connection key to be invalid end it 'updates the connection_id and connection_key' do @@ -1122,7 +1105,7 @@ def kill_connection_transport_and_prevent_valid_resume end channel.on(:attaching) do |channel_state_change| error = channel_state_change.reason - expect(error.message).to match(/Unable to recover connection/i) + expect(error.message).to match(/Invalid connection key/i) reattaching_channels << channel end channel.on(:attached) do @@ -1222,9 +1205,9 @@ def kill_connection_transport_and_prevent_valid_resume it 'sets the error reason on each channel' do channel.attach do channel.on(:attaching) do |state_change| - expect(state_change.reason.message).to match(/Unable to recover connection/i) - expect(state_change.reason.code).to eql(80008) - expect(channel.error_reason.code).to eql(80008) + expect(state_change.reason.message).to match(/Invalid connection key/i) + expect(state_change.reason.code).to eql(80018) + expect(channel.error_reason.code).to eql(80018) channel.on(:attached) do |state_change| stop_reactor @@ -1375,7 +1358,7 @@ def kill_connection_transport_and_prevent_valid_resume end) end - xit 'triggers a re-authentication and then resumes the connection' do + it 'triggers a re-authentication and then resumes the connection' do # [PENDING] After sandbox env update connection isn't found and a new connection is created. Spec fails # connection.once(:connected) do From 771ed3417a241e84ef766198d8eff4280183e7d7 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:28:05 +0530 Subject: [PATCH 4/9] [Protocol 2] Fixed tests for realtime/connection_spec added recovery_key specific tests --- spec/acceptance/realtime/connection_spec.rb | 56 +++++++++------------ 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/spec/acceptance/realtime/connection_spec.rb b/spec/acceptance/realtime/connection_spec.rb index edfb8f702..455246363 100644 --- a/spec/acceptance/realtime/connection_spec.rb +++ b/spec/acceptance/realtime/connection_spec.rb @@ -383,7 +383,7 @@ def publish_and_check_disconnect(options = {}) let(:client_id) { random_str } let(:client_options) { default_options.merge(client_id: 'incompatible', token: token_string, key: nil, log_level: :none) } - it 'fails the connection' do + xit 'fails the connection' do expect(client.client_id).to eql('incompatible') client.connection.once(:failed) do expect(client.client_id).to eql('incompatible') @@ -1364,7 +1364,7 @@ def self.available_states available_states.each do |state| connection.on(state) do - states[state.to_sym] = true if connection.recovery_key + states[state.to_sym] = true if connection.create_recovery_key end end @@ -1382,7 +1382,7 @@ def self.available_states it 'is nil when connection is explicitly CLOSED' do connection.once(:connected) do connection.close do - expect(connection.recovery_key).to be_nil + expect(connection.create_recovery_key).to be_nil stop_reactor end end @@ -1393,36 +1393,22 @@ def self.available_states context 'connection#id after recovery' do it 'remains the same' do previous_connection_id = nil + recovery_key = nil connection.once(:connected) do previous_connection_id = connection.id + recovery_key = client.connection.create_recovery_key connection.transition_state_machine! :failed end connection.once(:failed) do - recover_client = auto_close Ably::Realtime::Client.new(default_options.merge(recover: client.connection.recovery_key)) + recover_client = auto_close Ably::Realtime::Client.new(default_options.merge(recover: recovery_key)) recover_client.connection.on(:connected) do expect(recover_client.connection.id).to eql(previous_connection_id) stop_reactor end end end - - it 'does not call a resume callback', api_private: true do - connection.once(:connected) do - connection.transition_state_machine! :failed - end - - connection.once(:failed) do - recover_client = auto_close Ably::Realtime::Client.new(default_options.merge(recover: client.connection.recovery_key)) - recover_client.connection.on_resume do - raise 'Should not call the resume callback' - end - recover_client.connection.on(:connected) do - EventMachine.add_timer(0.5) { stop_reactor } - end - end - end end context 'when messages have been sent whilst the old connection is disconnected' do @@ -1432,7 +1418,7 @@ def self.available_states channel.attach do connection_id = client.connection.id - recovery_key = client.connection.recovery_key + recovery_key = client.connection.create_recovery_key connection.transport.__incoming_protocol_msgbus__ publishing_client_channel.publish('event', 'message') do connection.transition_state_machine! :failed @@ -1466,7 +1452,7 @@ def self.available_states channel.publish('event', 'message') do msg_serial = connection.send(:client_msg_serial) expect(msg_serial).to eql(0) - recovery_key = client.connection.recovery_key + recovery_key = client.connection.create_recovery_key connection.transition_state_machine! :failed end end @@ -1497,7 +1483,7 @@ def self.available_states expect(message.data).to eql('message-1') msg_serial = connection.send(:client_msg_serial) expect(msg_serial).to eql(0) - recovery_key = client.connection.recovery_key + recovery_key = client.connection.create_recovery_key connection.transition_state_machine! :failed end channel.publish('event', 'message-1') @@ -1531,23 +1517,29 @@ def self.available_states context 'with :recover option' do context 'with invalid syntax' do - let(:invaid_client_options) { default_options.merge(recover: 'invalid') } + let(:client_options) { default_options.merge(recover: 'invalid') } - it 'raises an exception' do - expect { Ably::Realtime::Client.new(invaid_client_options) }.to raise_error ArgumentError, /Recover/ - stop_reactor + it 'logs recovery decode error as a warning and connects successfully' do + connection.once(:connected) do + EventMachine.add_timer(1) { stop_reactor } + end + expect(client.logger).to receive(:warn).at_least(:once) do |*args, &block| + expect(args.concat([block ? block.call : nil]).join(',')).to match(/unable to decode recovery key/) + end end end - context 'with expired (missing) value sent to server' do - let(:client_options) { default_options.merge(recover: 'wVIsgTHAB1UvXh7z-1991d8586:0:0', log_level: :fatal) } + context 'with invalid connection key' do + recovery_key = "{\"connection_key\":\"0123456789abcdef-99\",\"msg_serial\":2," << + "\"channel_serials\":{\"channel1\":\"serial1\",\"channel2\":\"serial2\"}}" + let(:client_options) { default_options.merge(recover: recovery_key, log_level: :fatal) } it 'connects but sets the error reason and includes the reason in the state change' do connection.once(:connected) do |state_change| expect(connection.state).to eq(:connected) - expect(state_change.reason.message).to match(/Unable to recover connection/i) - expect(connection.error_reason.message).to match(/Unable to recover connection/i) - expect(connection.error_reason.code).to eql(80008) + expect(state_change.reason.message).to match(/Invalid connection key/i) + expect(connection.error_reason.message).to match(/Invalid connection key/i) + expect(connection.error_reason.code).to eql(80018) expect(connection.error_reason).to eql(state_change.reason) stop_reactor end From 949808e040da18dfb47c4d06205dd292dfa40e32 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:29:00 +0530 Subject: [PATCH 5/9] [Protocol 2] Fixed tests for realtime/message_spec removed unncessary test that checks for invalid server message using connection serial --- spec/acceptance/realtime/message_spec.rb | 75 ++++++++---------------- 1 file changed, 23 insertions(+), 52 deletions(-) diff --git a/spec/acceptance/realtime/message_spec.rb b/spec/acceptance/realtime/message_spec.rb index 53bd5be3a..e5eec8e77 100644 --- a/spec/acceptance/realtime/message_spec.rb +++ b/spec/acceptance/realtime/message_spec.rb @@ -304,8 +304,6 @@ def publish_and_check_extras(extras) it 'will not echo messages to the client but will still broadcast messages to other connected clients', em_timeout: 10 do channel.attach do |echo_channel| no_echo_channel.attach do - no_echo_channel.publish 'test_event', payload - no_echo_channel.subscribe('test_event') do |message| fail "Message should not have been echoed back" end @@ -316,6 +314,7 @@ def publish_and_check_extras(extras) stop_reactor end end + no_echo_channel.publish 'test_event', payload end end end @@ -418,41 +417,6 @@ def publish_and_check_extras(extras) end end - context 'server incorrectly resends a message that was already received by the client library' do - let(:messages_received) { [] } - let(:connection) { client.connection } - let(:client_options) { default_options.merge(log_level: :fatal) } - - it 'discards the message and logs it as an error to the channel' do - first_message_protocol_message = nil - connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| - first_message_protocol_message ||= protocol_message unless protocol_message.messages.empty? - end - - channel.attach do - channel.subscribe do |message| - messages_received << message - if messages_received.count == 2 - # simulate a duplicate protocol message being received - EventMachine.next_tick do - connection.__incoming_protocol_msgbus__.publish :protocol_message, first_message_protocol_message - end - end - end - 2.times { |i| EventMachine.add_timer(i.to_f / 5) { channel.publish('event', 'data') } } - - expect(client.logger).to receive(:error) do |*args, &block| - expect(args.concat([block ? block.call : nil]).join(',')).to match(/duplicate/) - - EventMachine.add_timer(0.5) do - expect(messages_received.count).to eql(2) - stop_reactor - end - end - end - end - end - context 'encoding and decoding encrypted messages' do shared_examples 'an Ably encrypter and decrypter' do |item, data| let(:algorithm) { data['algorithm'].upcase } @@ -622,11 +586,13 @@ def publish_and_check_extras(extras) let(:payload) { MessagePack.pack({ 'key' => random_str }) } it 'does not attempt to decrypt the message' do - unencrypted_channel_client1.publish 'example', payload - encrypted_channel_client2.subscribe do |message| - expect(message.data).to eql(payload) - expect(message.encoding).to be_nil - stop_reactor + wait_until(lambda { client.connection.state == :connected and other_client.connection.state == :connected }) do + encrypted_channel_client2.subscribe do |message| + expect(message.data).to eql(payload) + expect(message.encoding).to be_nil + stop_reactor + end + unencrypted_channel_client1.publish 'example', payload end end end @@ -671,11 +637,13 @@ def publish_and_check_extras(extras) let(:payload) { MessagePack.pack({ 'key' => random_str }) } it 'delivers the message but still encrypted with the cipher detials in the #encoding attribute (#RTL7e)' do - encrypted_channel_client1.publish 'example', payload - encrypted_channel_client2.subscribe do |message| - expect(message.data).to_not eql(payload) - expect(message.encoding).to match(/^cipher\+aes-256-cbc/) - stop_reactor + encrypted_channel_client2.attach do + encrypted_channel_client2.subscribe do |message| + expect(message.data).to_not eql(payload) + expect(message.encoding).to match(/^cipher\+aes-256-cbc/) + stop_reactor + end + encrypted_channel_client1.publish 'example', payload end end @@ -751,10 +719,13 @@ def publish_and_check_extras(extras) end end - channel.publish(event_name).tap do |deferrable| - deferrable.callback { message_state << :delivered } - deferrable.errback do - raise 'Message delivery should not fail' + # Attaching channel first before publishing message in order to get channel serial set on channel + channel.attach do + channel.publish(event_name).tap do |deferrable| + deferrable.callback { message_state << :delivered } + deferrable.errback do + raise 'Message delivery should not fail' + end end end @@ -777,7 +748,7 @@ def publish_and_check_extras(extras) if protocol_message.messages.find { |message| message.name == event_name } EventMachine.add_timer(0.0001) do connection.transport.unbind # trigger failure - connection.configure_new '0123456789abcdef', 'wVIsgTHAB1UvXh7z-1991d8586', -1 # force the resume connection key to be invalid + connection.configure_new '0123456789abcdef', 'wVIsgTHAB1UvXh7z-1991d8586' # force the resume connection key to be invalid end end end From 3686c27cc86105fdc11d66e227737e7f8071acf8 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:29:38 +0530 Subject: [PATCH 6/9] [Protocol 2] Fixed/added tests for realtime/presence_spec --- spec/acceptance/realtime/presence_spec.rb | 194 +++++++++++++--------- 1 file changed, 116 insertions(+), 78 deletions(-) diff --git a/spec/acceptance/realtime/presence_spec.rb b/spec/acceptance/realtime/presence_spec.rb index ac87036e5..c24f5cee0 100644 --- a/spec/acceptance/realtime/presence_spec.rb +++ b/spec/acceptance/realtime/presence_spec.rb @@ -60,13 +60,13 @@ def setup_test(method_name, args, options) end unless expected_state == :left - it 'raise an exception if the channel is detached' do + it "presence #{method_name} : raise an exception if the channel is detached" do setup_test(method_name, args, options) do channel_client_one.attach do channel_client_one.transition_state_machine :detaching channel_client_one.once(:detached) do presence_client_one.public_send(method_name, args).tap do |deferrable| - deferrable.callback { raise 'Get should not succeed' } + deferrable.callback { raise "presence #{method_name} should not succeed" } deferrable.errback do |error| expect(error).to be_a(Ably::Exceptions::InvalidState) expect(error.message).to match(/Operation is not allowed when channel is in STATE.Detached/) @@ -78,12 +78,12 @@ def setup_test(method_name, args, options) end end - it 'raise an exception if the channel becomes detached' do + it "presence #{method_name} : raise an exception if the channel becomes detached" do setup_test(method_name, args, options) do channel_client_one.attach do channel_client_one.transition_state_machine :detaching presence_client_one.public_send(method_name, args).tap do |deferrable| - deferrable.callback { raise 'Get should not succeed' } + deferrable.callback { raise "presence #{method_name} should not succeed" } deferrable.errback do |error| expect(error).to be_a(Ably::Exceptions::InvalidState) expect(error.message).to match(/Operation failed as channel transitioned to STATE.Detached/) @@ -94,13 +94,13 @@ def setup_test(method_name, args, options) end end - it 'raise an exception if the channel is failed' do + it "presence #{method_name} : raise an exception if the channel is failed" do setup_test(method_name, args, options) do channel_client_one.attach do channel_client_one.transition_state_machine :failed expect(channel_client_one.state).to eq(:failed) presence_client_one.public_send(method_name, args).tap do |deferrable| - deferrable.callback { raise 'Get should not succeed' } + deferrable.callback { raise "presence #{method_name} : Get should not succeed" } deferrable.errback do |error| expect(error).to be_a(Ably::Exceptions::InvalidState) expect(error.message).to match(/Operation is not allowed when channel is in STATE.Failed/) @@ -111,11 +111,11 @@ def setup_test(method_name, args, options) end end - it 'raise an exception if the channel becomes failed' do + it "presence #{method_name} : raise an exception if the channel becomes failed" do setup_test(method_name, args, options) do channel_client_one.attach do presence_client_one.public_send(method_name, args).tap do |deferrable| - deferrable.callback { raise 'Get should not succeed' } + deferrable.callback { raise "presence #{method_name} : Get should not succeed" } deferrable.errback do |error| expect(error).to be_a(Ably::Exceptions::MessageDeliveryFailed) stop_reactor @@ -704,21 +704,29 @@ def allow_sync_fabricate_data_final_sync_and_assert_members context '#sync_complete? and SYNC flags (#RTP1)' do context 'when attaching to a channel without any members present' do - xit 'sync_complete? is true, there is no presence flag, and the presence channel is considered synced immediately (#RTP1)' do - flag_checked = false + it 'sync_complete? is true, no members are received and the presence channel is synced (#RTP1)' do + sync_info_received = false anonymous_client.connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| if protocol_message.action == :attached - flag_checked = true - expect(protocol_message.has_presence_flag?).to eql(false) + if protocol_message.has_presence_flag? + sync_info_received = false + else + sync_info_received = true + end + end + if protocol_message.action == Ably::Models::ProtocolMessage::ACTION.Sync + expect(protocol_message.presence).to be_empty + sync_info_received = true end end channel_anonymous_client.attach do - expect(channel_anonymous_client.presence).to be_sync_complete - EventMachine.next_tick do - expect(flag_checked).to eql(true) - stop_reactor + wait_until(lambda { channel_anonymous_client.presence.sync_complete? and sync_info_received}) do + channel_anonymous_client.presence.get do |members| + expect(members).to be_empty + stop_reactor + end end end end @@ -1589,14 +1597,16 @@ def setup_members_on(presence) end it 'fails if the connection is DETACHED (#RTP11b)' do - channel_client_one.attach do - channel_client_one.detach do - presence_client_one.get.tap do |deferrable| - deferrable.callback { raise 'Get should not succeed' } - deferrable.errback do |error| - expect(error).to be_a(Ably::Exceptions::InvalidState) - expect(error.message).to match(/Operation is not allowed when channel is in STATE.Detached/) - stop_reactor + client_one.connection.once :connected do + channel_client_one.attach do + channel_client_one.detach do + presence_client_one.get.tap do |deferrable| + deferrable.callback { raise 'Get should not succeed' } + deferrable.errback do |error| + expect(error).to be_a(Ably::Exceptions::InvalidState) + expect(error.message).to match(/Operation is not allowed when channel is in STATE.Detached/) + stop_reactor + end end end end @@ -1812,29 +1822,31 @@ def connect_members_deferrables let(:total_members) { members_per_client * 2 } it 'returns a complete list of members on all clients' do - members_per_client.times do |indx| - presence_client_one.enter_client("client_1:#{indx}") - presence_client_two.enter_client("client_2:#{indx}") - end + wait_until(lambda { client_one.connection.state == :connected and client_two.connection.state == :connected }) do + presence_client_one.subscribe(:enter) do + clients_entered[:client_one] += 1 + end - presence_client_one.subscribe(:enter) do - clients_entered[:client_one] += 1 - end + presence_client_two.subscribe(:enter) do + clients_entered[:client_two] += 1 + end - presence_client_two.subscribe(:enter) do - clients_entered[:client_two] += 1 - end + members_per_client.times do |indx| + presence_client_one.enter_client("client_1:#{indx}") + presence_client_two.enter_client("client_2:#{indx}") + end - wait_until(lambda { clients_entered[:client_one] + clients_entered[:client_two] == total_members * 2 }) do - presence_anonymous_client.get(wait_for_sync: true) do |anonymous_members| - expect(anonymous_members.count).to eq(total_members) - expect(anonymous_members.map(&:client_id).uniq.count).to eq(total_members) + wait_until(lambda { clients_entered[:client_one] + clients_entered[:client_two] == total_members * 2 }) do + presence_anonymous_client.get(wait_for_sync: true) do |anonymous_members| + expect(anonymous_members.count).to eq(total_members) + expect(anonymous_members.map(&:client_id).uniq.count).to eq(total_members) - presence_client_one.get(wait_for_sync: true) do |client_one_members| - presence_client_two.get(wait_for_sync: true) do |client_two_members| - expect(client_one_members.count).to eq(total_members) - expect(client_one_members.count).to eq(client_two_members.count) - stop_reactor + presence_client_one.get(wait_for_sync: true) do |client_one_members| + presence_client_two.get(wait_for_sync: true) do |client_two_members| + expect(client_one_members.count).to eq(total_members) + expect(client_one_members.count).to eq(client_two_members.count) + stop_reactor + end end end end @@ -2460,7 +2472,7 @@ def connect_members_deferrables end leave_message = Ably::Models::PresenceMessage.new( - 'id' => "#{client_two.connection.id}:#{presence_client_two.client_id}:1", + 'id' => "#{client_two.connection.id}:#{client_two.connection.send(:client_msg_serial)}:1", 'clientId' => presence_client_two.client_id, 'connectionId' => client_two.connection.id, 'timestamp' => as_since_epoch(Time.now), @@ -2525,37 +2537,59 @@ def cripple_websocket_transport let(:member_data) { random_str } it 'immediately resends all local presence members (#RTP5c2, #RTP19a)' do - sync_complete_confirmed_no_local_members = false - local_member_leave_event_fired = false + member_leave_event_fired = false + local_members_sent = false + + presence_client_one.subscribe(:enter) do |entered_member| + expect(entered_member.action).to eq(Ably::Models::PresenceMessage::ACTION.Enter) + expect(entered_member.data).to eq(member_data) + expect(entered_member.client_id).to eq(client_one.auth.client_id) + expect(entered_member.id).to be_truthy + entered_member_id = entered_member.id - presence_client_one.enter(member_data) - presence_client_one.subscribe(:enter) do presence_client_one.unsubscribe :enter - presence_client_one.subscribe(:leave) do |message| - # The local member will leave the PresenceMap due to the ATTACHED without Presence - local_member_leave_event_fired = true + expect(presence_client_one.members.length).to eql(1) + expect(presence_client_one.members.local_members.length).to eql(1) + + # subscribe to outgoing messages to check for entered local members with id + client_one.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| + if protocol_message.action == :presence + protocol_message.presence.each do |local_member| + expect(local_member.id).to eq(entered_member_id) + expect(local_member.action).to eq(Ably::Models::PresenceMessage::ACTION.Enter) + expect(local_member.data).to eq(member_data) + expect(local_member.client_id).to eq(client_one.auth.client_id) + local_members_sent = true + end + end end - # Local members re-entered automatically appear as updates due to the - # fabricated ATTACHED message sent and the members already being present - presence_client_one.subscribe(:update) do |message| - expect(local_member_leave_event_fired).to be_truthy + presence_client_one.subscribe(:leave) do |message| + # Member will leave the PresenceMap due to the ATTACHED without Presence expect(message.data).to eq(member_data) expect(message.client_id).to eq(client_one.auth.client_id) - EventMachine.next_tick do - expect(presence_client_one.members.length).to eql(1) - expect(presence_client_one.members.local_members.length).to eql(1) - expect(sync_complete_confirmed_no_local_members).to be_truthy - stop_reactor - end + member_leave_event_fired = true + end + + # Shouldn't receive enter/update message when local_members are entered + # This is due to the fact that, when we enter local member we also send + # member id, and server automatically checks for duplicate id and doesn't + # send presenceEnter or presenceUpdate if id is found. + presence_client_one.subscribe(:enter, :update) do |message| + raise { "client shouldn't receive update event for entered local members" } end presence_client_one.members.once(:sync_complete) do - # Immediately after SYNC (no sync actually occurred, but this event fires immediately after a channel SYNCs or is not expecting to SYNC) expect(presence_client_one.members.length).to eql(0) - expect(presence_client_one.members.local_members.length).to eql(0) - sync_complete_confirmed_no_local_members = true + + # Since, this is a client sent event, local_members are not cleared + # local_members acts a source of truth for server and not vice versa + expect(presence_client_one.members.local_members.length).to eql(1) + + wait_until(lambda { member_leave_event_fired and local_members_sent}) do + stop_reactor + end end # ATTACHED ProtocolMessage with no presence flag will clear the presence set immediately, #RTP19a @@ -2565,6 +2599,8 @@ def cripple_websocket_transport flags: 0 # no resume or presence flag ) end + + presence_client_one.enter(member_data) end end end @@ -2677,23 +2713,25 @@ def cripple_websocket_transport context 'channel transitions to the DETACHED state' do it 'clears the PresenceMap and local member map copy and does not emit any presence events (#RTP5a)' do - presence_client_one.enter - presence_client_one.subscribe(:enter) do - presence_client_one.unsubscribe :enter + wait_until(lambda { client_one.connection.state == :connected and anonymous_client.connection.state == :connected }) do + presence_client_one.enter + presence_client_one.subscribe(:enter) do + presence_client_one.unsubscribe :enter - channel_anonymous_client.attach do - presence_anonymous_client.get do |members| - expect(members.count).to eq(1) + channel_anonymous_client.attach do + presence_anonymous_client.get do |members| + expect(members.count).to eq(1) - presence_anonymous_client.subscribe { raise 'No presence events should be emitted' } - channel_anonymous_client.detach do - expect(presence_anonymous_client.members.length).to eq(0) - expect(channel_anonymous_client).to be_detached + presence_anonymous_client.subscribe { raise 'No presence events should be emitted' } + channel_anonymous_client.detach do + expect(presence_anonymous_client.members.length).to eq(0) + expect(channel_anonymous_client).to be_detached - expect(presence_client_one.members.local_members.count).to eq(1) - channel_client_one.detach do - expect(presence_client_one.members.local_members.count).to eq(0) - stop_reactor + expect(presence_client_one.members.local_members.count).to eq(1) + channel_client_one.detach do + expect(presence_client_one.members.local_members.count).to eq(0) + stop_reactor + end end end end From 73beeec59a0adfa2421a36df80e7036bbb0a465d Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:30:34 +0530 Subject: [PATCH 7/9] [Protocol 2] Skipped test because of irregularities with respect to spec --- spec/acceptance/realtime/client_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/acceptance/realtime/client_spec.rb b/spec/acceptance/realtime/client_spec.rb index 28b7a1dea..dd0a7248a 100644 --- a/spec/acceptance/realtime/client_spec.rb +++ b/spec/acceptance/realtime/client_spec.rb @@ -133,7 +133,7 @@ end end - context 'with a wildcard client_id token' do + context 'with a wildcard client_id token ' do subject { auto_close Ably::Realtime::Client.new(client_options) } let(:client_options) { default_options.merge(auth_callback: lambda { |token_params| auth_token_object }, client_id: client_id) } let(:rest_auth_client) { Ably::Rest::Client.new(default_options.merge(key: api_key)) } @@ -142,7 +142,7 @@ context 'and an explicit client_id in ClientOptions' do let(:client_id) { random_str } - it 'allows uses the explicit client_id in the connection' do + xit 'allows uses the explicit client_id in the connection' do connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| if protocol_message.action == :connected expect(protocol_message.connection_details.client_id).to eql(client_id) From ef7be90fc34dd3a6a90a1fbce8b98c583df8a1ba Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 17:31:35 +0530 Subject: [PATCH 8/9] Fixed typo for unit test, replaced auth_connect with auto_connect --- spec/shared/client_initializer_behaviour.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/shared/client_initializer_behaviour.rb b/spec/shared/client_initializer_behaviour.rb index 5ccfbeb22..84ff2df4c 100644 --- a/spec/shared/client_initializer_behaviour.rb +++ b/spec/shared/client_initializer_behaviour.rb @@ -130,7 +130,7 @@ def rest? end context 'with token' do - let(:client_options) { { token: 'token', auth_connect: false } } + let(:client_options) { { token: 'token', auto_connect: false } } it 'sets the token' do expect(subject.auth.current_token_details.token).to eql('token') From e3eb35171b16face9298ad572cc07e028bb4b129 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Fri, 5 Jul 2024 18:21:22 +0530 Subject: [PATCH 9/9] Added comment on skipped tests describing reason for skipping them --- spec/acceptance/realtime/client_spec.rb | 1 + spec/acceptance/realtime/connection_spec.rb | 1 + 2 files changed, 2 insertions(+) diff --git a/spec/acceptance/realtime/client_spec.rb b/spec/acceptance/realtime/client_spec.rb index dd0a7248a..a4f50ae99 100644 --- a/spec/acceptance/realtime/client_spec.rb +++ b/spec/acceptance/realtime/client_spec.rb @@ -142,6 +142,7 @@ context 'and an explicit client_id in ClientOptions' do let(:client_id) { random_str } + # Skipped because more clarification needed on RSA7e, see https://github.com/ably/ably-ruby/issues/425 xit 'allows uses the explicit client_id in the connection' do connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| if protocol_message.action == :connected diff --git a/spec/acceptance/realtime/connection_spec.rb b/spec/acceptance/realtime/connection_spec.rb index 455246363..8827257a9 100644 --- a/spec/acceptance/realtime/connection_spec.rb +++ b/spec/acceptance/realtime/connection_spec.rb @@ -383,6 +383,7 @@ def publish_and_check_disconnect(options = {}) let(:client_id) { random_str } let(:client_options) { default_options.merge(client_id: 'incompatible', token: token_string, key: nil, log_level: :none) } + # Skipped because more clarification needed on RSA7e, see https://github.com/ably/ably-ruby/issues/425 xit 'fails the connection' do expect(client.client_id).to eql('incompatible') client.connection.once(:failed) do