Merge pull request #426 from ably/feature/integration-protocol-2-tests

[ECO-4058][Protocol-2] Feature/integration protocol 2 tests

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,8 @@
             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
+              # 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
                     expect(protocol_message.connection_details.client_id).to eql(client_id)

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

spec/acceptance/realtime/connection_spec.rb

@@ -383,7 +383,8 @@ 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
+              # 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
                   expect(client.client_id).to eql('incompatible')
@@ -1364,7 +1365,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 +1383,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 +1394,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 +1419,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 +1453,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 +1484,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 +1518,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

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