Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-4058] Feature/Integration protocol 2 #423

Merged
merged 62 commits into from
Jul 5, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
301d73d
removed unnecessary connection serial references from the code
sacOO7 Jun 2, 2024
67dee1d
removed connection serial references from connection file
sacOO7 Jun 2, 2024
58fb431
Refactored comments related to connection serial, removed comments me…
sacOO7 Jun 5, 2024
aaa6211
removed unnecessary has_connection_serial, has_serial method and rela…
sacOO7 Jun 5, 2024
832d9c4
removed connection_serial property from protocol_message, removed all
sacOO7 Jun 5, 2024
d4b2e6c
Updated ably protocol version to 2, updated relevant tests
sacOO7 Jun 6, 2024
21279fa
Renamed VERSION constant to LIB_VERSION for readability
sacOO7 Jun 6, 2024
efb7dad
Set idempotent rest publishing to true by default, updated relevant t…
sacOO7 Jun 6, 2024
4b448ff
Added RecoveryKeyContext class and unit tests for the same
sacOO7 Jun 6, 2024
7b8af6b
Fixed typos in ruby docs for internal modules and realtime channel class
sacOO7 Jun 6, 2024
a0114b9
Added channelSerial field to channel properties, implemented following
sacOO7 Jun 6, 2024
4ba3917
Implemented create_recovery_key method on connection. Implemented
sacOO7 Jun 6, 2024
c0760d6
Fix file location: Moved recovery_key_context from test to lib
sacOO7 Jun 6, 2024
9dbe92c
renamed in_sync enum to sync_complete, updated as per spec
sacOO7 Jun 6, 2024
5b0a006
Implemented onAttach channel presence
sacOO7 Jun 6, 2024
b0ad4fb
Revert "Renamed VERSION constant to LIB_VERSION for readability"
sacOO7 Jun 21, 2024
c4ff9e5
Refactored idempotent publishing test name to be enabled by default
sacOO7 Jun 21, 2024
b1789fa
removed nil check for protocol_message while setting channel serial
sacOO7 Jun 21, 2024
54e186d
channel_name accessor made explicit for better readability
sacOO7 Jun 21, 2024
15b21d7
returning nil for create_recovery_key, same as old deprecated recover…
sacOO7 Jun 21, 2024
32e68cf
Added AblyExtensions for empty/nil string check, updated usages for the
sacOO7 Jun 24, 2024
7a04abc
Updated invalid check impl on protocol_message received
sacOO7 Jun 24, 2024
a3d9746
Refactored logic for sending ATTACH message
sacOO7 Jun 24, 2024
4022ab8
Removed unused references to resume_callbacks used for client initiat…
sacOO7 Jun 24, 2024
c328c6c
Fixed impl for creating presence message using id
sacOO7 Jun 24, 2024
e77e6ba
Added missing spec annotations for presence_manager
sacOO7 Jun 24, 2024
82ae801
Merge branch 'feature/protocol-2-resume-recover' into feature/protoco…
sacOO7 Jun 25, 2024
f4d7f2b
[ECO-4845] refactored rest/realtime auth
sacOO7 Jun 25, 2024
bb4b71f
Fixed naming convention for client_id_header as per review comment
sacOO7 Jun 25, 2024
c10e485
Removed unncessary use of auth clientId while entering presence local…
sacOO7 Jun 25, 2024
0f49ede
renamed auth client_id_header to external_client_id
sacOO7 Jun 25, 2024
446a16f
Marked enter_local_members public instead of private
sacOO7 Jun 25, 2024
b32bb57
Fixed test that updated attach_serial everytime new attach msg is rec…
sacOO7 Jul 2, 2024
1324a72
replaced next unless with assertive if condition for op retry on reco…
sacOO7 Jul 2, 2024
8b9b556
Refactored implementation for sending ATTACH and DETACH message
sacOO7 Jul 2, 2024
317572b
Merge pull request #409 from ably/feature/protocol-2-resume-recover
sacOO7 Jul 3, 2024
cbdf545
Merge pull request #410 from ably/feature/protocol-2-presence
sacOO7 Jul 3, 2024
1c8f52f
refactored updated attach_serial test as per review comment
sacOO7 Jul 3, 2024
1ff7d6b
Added separate method client_id_for_request that returns client_id
sacOO7 Jul 3, 2024
91e5f29
Revert "Removed unncessary use of auth clientId while entering presen…
sacOO7 Jul 3, 2024
c8745a4
Removed unncessary channel publish from the attach_serial test
sacOO7 Jul 3, 2024
7be9e8c
Removed unncessary use of auth clientId while entering presence local…
sacOO7 Jul 3, 2024
2eba50f
Refactored channel attach, removed check for forced attach
sacOO7 Jul 3, 2024
7a973ae
Refactored send_protocol_message method, removed unnecessary usage of…
sacOO7 Jul 3, 2024
d37be2f
Synced protocol-2-presence changes
sacOO7 Jul 4, 2024
07aadbd
Updated channel_manager,
sacOO7 Jul 4, 2024
47c55d7
Added test to check for duplicate attach message sent or received
sacOO7 Jul 4, 2024
9981082
Merge pull request #416 from ably/fix/duplicate-attach-msg-send
sacOO7 Jul 4, 2024
af7bde9
Merge pull request #421 from ably/fix/until_attach_serial_test
sacOO7 Jul 4, 2024
08877bf
Merge pull request #418 from ably/fix/wildcard-clientId-request
sacOO7 Jul 4, 2024
e164a90
Merge branch 'main' into feature/integration-protocol-2
sacOO7 Jul 5, 2024
d22b3e7
Fixed logger msg for queuing outgoing msg in realtime#connection
sacOO7 Jul 5, 2024
b8231d8
Removed unnecessary on_resume flag from channels_spec
sacOO7 Jul 5, 2024
8300ecd
[Protocol 2] Fixed tests for realtime/channel_spec
sacOO7 Jul 5, 2024
fc69791
[Protocol 2] Fixed tests for realtime/connection_failures_spec
sacOO7 Jul 5, 2024
771ed34
[Protocol 2] Fixed tests for realtime/connection_spec
sacOO7 Jul 5, 2024
949808e
[Protocol 2] Fixed tests for realtime/message_spec
sacOO7 Jul 5, 2024
3686c27
[Protocol 2] Fixed/added tests for realtime/presence_spec
sacOO7 Jul 5, 2024
73beeec
[Protocol 2] Skipped test because of irregularities with respect to spec
sacOO7 Jul 5, 2024
ef7be90
Fixed typo for unit test, replaced auth_connect with auto_connect
sacOO7 Jul 5, 2024
e3eb351
Added comment on skipped tests describing reason for skipping them
sacOO7 Jul 5, 2024
3e1882e
Merge pull request #426 from ably/feature/integration-protocol-2-tests
sacOO7 Jul 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
750 changes: 449 additions & 301 deletions spec/acceptance/realtime/channel_spec.rb

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions spec/acceptance/realtime/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)) }
Expand All @@ -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)
Expand Down
33 changes: 8 additions & 25 deletions spec/acceptance/realtime/connection_failures_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
57 changes: 25 additions & 32 deletions spec/acceptance/realtime/connection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down
75 changes: 23 additions & 52 deletions spec/acceptance/realtime/message_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -316,6 +314,7 @@ def publish_and_check_extras(extras)
stop_reactor
end
end
no_echo_channel.publish 'test_event', payload
end
end
end
Expand Down Expand Up @@ -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 }
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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

Expand All @@ -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
Expand Down
Loading
Loading