From 80e159ddaa2a27b762c77485cf93f255ae58c903 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=C5=9Aliwa?= Date: Tue, 24 Aug 2021 13:33:37 +0200 Subject: [PATCH 01/16] [#278] Added ChannelOptions with params and modes attributes (TB2c,TB2d) --- lib/ably/realtime/channel.rb | 1 + lib/ably/realtime/channel/channel_options.rb | 27 ++++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 lib/ably/realtime/channel/channel_options.rb diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index a7a83ed54..0c579de4d 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -368,3 +368,4 @@ def channel_name require 'ably/realtime/channel/channel_state_machine' require 'ably/realtime/channel/push_channel' require 'ably/realtime/channel/channel_properties' +require 'ably/realtime/channel/channel_options' diff --git a/lib/ably/realtime/channel/channel_options.rb b/lib/ably/realtime/channel/channel_options.rb new file mode 100644 index 000000000..3366f81f8 --- /dev/null +++ b/lib/ably/realtime/channel/channel_options.rb @@ -0,0 +1,27 @@ +module Ably::Realtime + class Channel + # Represents options of a channel + class ChannelOptions + # {Ably::Realtime::Channel} this object associated with + # + # @return [Ably::Realtime::Channel] + attr_reader :channel + + # (TB2c) params (for realtime client libraries only) a Dict of key/value pairs + # + # @return [Hash] + attr_reader :params + + # (TB2d) modes (for realtime client libraries only) an array of ChannelMode + # + # @return [Array] + attr_reader :modes + + def initialize(channel, params = {}, modes = []) + @channel = channel + @params = params + @modes = modes + end + end + end +end From 826eed9d19ca14e19a7fc3fe6539091703dea4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=C5=9Aliwa?= Date: Wed, 15 Sep 2021 01:10:32 +0200 Subject: [PATCH 02/16] [#278] Moved ChannelOptions to the Ably::Models module. Updated failed tests. Implemented channel_options as ChannelOptions object instead of Hash. --- lib/ably/models/channel_options.rb | 56 ++++++++++++++++++++ lib/ably/realtime/channel.rb | 3 +- lib/ably/realtime/channel/channel_options.rb | 27 ---------- lib/ably/realtime/presence.rb | 2 +- lib/ably/realtime/presence/members_map.rb | 2 +- lib/ably/rest/channel.rb | 2 +- lib/ably/rest/presence.rb | 2 +- spec/acceptance/realtime/channels_spec.rb | 8 +-- spec/acceptance/rest/channels_spec.rb | 8 +-- spec/unit/realtime/channels_spec.rb | 8 +-- 10 files changed, 73 insertions(+), 45 deletions(-) create mode 100644 lib/ably/models/channel_options.rb delete mode 100644 lib/ably/realtime/channel/channel_options.rb diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb new file mode 100644 index 000000000..3f5c58f5c --- /dev/null +++ b/lib/ably/models/channel_options.rb @@ -0,0 +1,56 @@ +module Ably::Models + # Represents options of a channel + class ChannelOptions + MODES = [:publish, :subscribe, :presence, :presence_subscribe].freeze + + # {Ably::Realtime::Channel} this object associated with + # + # @return [Ably::Realtime::Channel] + attr_reader :channel + + # (TB2c) params (for realtime client libraries only) a Dict of key/value pairs + # + # @return [Hash] + attr_reader :params + + # (TB2d) modes (for realtime client libraries only) an array of ChannelMode + # + # @return [Array<>] + attr_reader :modes + + def initialize(channel, params = {}, modes = []) + @channel = channel + @params = params + @modes = modes + end + + # Get value of the key from the params + # @return [String] + # + def [](key) + @params[key] + end + + # Delegates fetching a key from the params to the params instance variable + # @return [String] + # + def fetch(*args) + @params.fetch(*args) + end + + # Returns a params hash + # @return [Hash] + # + def to_h + @params.to_h + end + alias to_hash to_h + + # Converts the params to the string + # @return [String] + # + def to_s + @params.to_s + end + end +end diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index 0c579de4d..a7ea75355 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -321,7 +321,7 @@ def clear_error_reason # @api private def update_options(channel_options) - @options = channel_options.clone.freeze + @options = Ably::Models::ChannelOptions.new(self, channel_options.clone).freeze end # Used by {Ably::Modules::StateEmitter} to debug state changes @@ -368,4 +368,3 @@ def channel_name require 'ably/realtime/channel/channel_state_machine' require 'ably/realtime/channel/push_channel' require 'ably/realtime/channel/channel_properties' -require 'ably/realtime/channel/channel_options' diff --git a/lib/ably/realtime/channel/channel_options.rb b/lib/ably/realtime/channel/channel_options.rb deleted file mode 100644 index 3366f81f8..000000000 --- a/lib/ably/realtime/channel/channel_options.rb +++ /dev/null @@ -1,27 +0,0 @@ -module Ably::Realtime - class Channel - # Represents options of a channel - class ChannelOptions - # {Ably::Realtime::Channel} this object associated with - # - # @return [Ably::Realtime::Channel] - attr_reader :channel - - # (TB2c) params (for realtime client libraries only) a Dict of key/value pairs - # - # @return [Hash] - attr_reader :params - - # (TB2d) modes (for realtime client libraries only) an array of ChannelMode - # - # @return [Array] - attr_reader :modes - - def initialize(channel, params = {}, modes = []) - @channel = channel - @params = params - @modes = modes - end - end - end -end diff --git a/lib/ably/realtime/presence.rb b/lib/ably/realtime/presence.rb index 8c8a89fed..b62bf16bb 100644 --- a/lib/ably/realtime/presence.rb +++ b/lib/ably/realtime/presence.rb @@ -338,7 +338,7 @@ def create_presence_message(action, client_id, data) } Ably::Models::PresenceMessage.new(model, logger: logger).tap do |presence_message| - presence_message.encode(client.encoders, channel.options) do |encode_error, error_message| + presence_message.encode(client.encoders, channel.options.params) do |encode_error, error_message| client.logger.error error_message end end diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index ef2d8df32..d7b7e6f6c 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -207,7 +207,7 @@ def connection def setup_event_handlers presence.__incoming_msgbus__.subscribe(:presence, :sync) do |presence_message| - presence_message.decode(client.encoders, channel.options) do |encode_error, error_message| + presence_message.decode(client.encoders, channel.options.params) do |encode_error, error_message| client.logger.error error_message end update_members_and_emit_events presence_message diff --git a/lib/ably/rest/channel.rb b/lib/ably/rest/channel.rb index 91f72832e..b5f08515b 100644 --- a/lib/ably/rest/channel.rb +++ b/lib/ably/rest/channel.rb @@ -166,7 +166,7 @@ def presence # @api private def update_options(channel_options) - @options = channel_options.clone.freeze + @options = Ably::Models::ChannelOptions.new(self, channel_options.clone).freeze end private diff --git a/lib/ably/rest/presence.rb b/lib/ably/rest/presence.rb index a025f5a5b..a6448e80f 100644 --- a/lib/ably/rest/presence.rb +++ b/lib/ably/rest/presence.rb @@ -89,7 +89,7 @@ def base_path end def decode_message(presence_message) - presence_message.decode client.encoders, channel.options + presence_message.decode client.encoders, channel.options.params rescue Ably::Exceptions::CipherError, Ably::Exceptions::EncoderError => e client.logger.error { "Decoding Error on presence channel '#{channel.name}', presence message client_id '#{presence_message.client_id}'. #{e.class.name}: #{e.message}" } end diff --git a/spec/acceptance/realtime/channels_spec.rb b/spec/acceptance/realtime/channels_spec.rb index 992aeecb3..11baf5ef2 100644 --- a/spec/acceptance/realtime/channels_spec.rb +++ b/spec/acceptance/realtime/channels_spec.rb @@ -10,7 +10,7 @@ end it 'returns channel object and passes the provided options' do - expect(channel_with_options.options).to eql(options) + expect(channel_with_options.options.params).to eql(options) stop_reactor end end @@ -39,7 +39,7 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'overrides the existing channel options and returns the channel object' do - expect(original_channel.options).to_not include(:encrypted) + expect(original_channel.options.params).to_not include(:encrypted) new_channel = client.channels.get(channel_name, new_channel_options) expect(new_channel).to be_a(Ably::Realtime::Channel) expect(new_channel.options[:encrypted]).to eql(true) @@ -51,10 +51,10 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'returns the existing channel without modifying the channel options' do - expect(original_channel.options).to eql(options) + expect(original_channel.options.params).to eql(options) new_channel = client.channels.get(channel_name) expect(new_channel).to be_a(Ably::Realtime::Channel) - expect(original_channel.options).to eql(options) + expect(original_channel.options.params).to eql(options) stop_reactor end end diff --git a/spec/acceptance/rest/channels_spec.rb b/spec/acceptance/rest/channels_spec.rb index b34231a7e..6b2741f9d 100644 --- a/spec/acceptance/rest/channels_spec.rb +++ b/spec/acceptance/rest/channels_spec.rb @@ -9,7 +9,7 @@ end it 'returns channel object and passes the provided options' do - expect(channel_with_options.options).to eql(options) + expect(channel_with_options.options.params).to eql(options) end end @@ -37,7 +37,7 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'overrides the existing channel options and returns the channel object' do - expect(original_channel.options).to_not include(:encrypted) + expect(original_channel.options.params).to_not include(:encrypted) new_channel = client.channels.get(channel_name, new_channel_options) expect(new_channel).to be_a(Ably::Rest::Channel) expect(new_channel.options[:encrypted]).to eql(true) @@ -48,10 +48,10 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'returns the existing channel without modifying the channel options' do - expect(original_channel.options).to eql(options) + expect(original_channel.options.params).to eql(options) new_channel = client.channels.get(channel_name) expect(new_channel).to be_a(Ably::Rest::Channel) - expect(original_channel.options).to eql(options) + expect(original_channel.options.params).to eql(options) end end diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index d1b947792..ae5a89a57 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -25,17 +25,17 @@ it 'will update the options on the channel if provided' do channel = subject.get(channel_name, options) - expect(channel.options).to eql(options) - expect(channel.options).to_not include(:encrypted) + expect(channel.options.params).to eql(options) + expect(channel.options.params).to_not include(:encrypted) subject.get(channel_name, encrypted: true) expect(channel.options[:encrypted]).to eql(true) end it 'will leave the options intact on the channel if not provided' do channel = subject.get(channel_name, options) - expect(channel.options).to eql(options) + expect(channel.options.params).to eql(options) subject.get(channel_name) - expect(channel.options).to eql(options) + expect(channel.options.params).to eql(options) end end end From 7d5e86b27454b80a231b9dd95b5498a2d61ca4b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Wed, 17 Nov 2021 06:18:34 +0300 Subject: [PATCH 03/16] Implement RTL16 --- lib/ably/models/channel_options.rb | 82 +++++++++++++---------- lib/ably/realtime/channel.rb | 19 +++--- lib/ably/realtime/presence.rb | 2 +- lib/ably/realtime/presence/members_map.rb | 2 +- lib/ably/rest/channel.rb | 5 +- lib/ably/rest/presence.rb | 2 +- lib/ably/util/crypto.rb | 2 +- spec/acceptance/realtime/channels_spec.rb | 33 +++++++-- spec/acceptance/rest/channels_spec.rb | 26 +++++-- spec/unit/realtime/channels_spec.rb | 14 ++-- 10 files changed, 120 insertions(+), 67 deletions(-) diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index 3f5c58f5c..a07caf559 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -1,56 +1,68 @@ module Ably::Models + # Convert token details argument to a {ChannelOptions} object + # + # @param attributes (see #initialize) + # + # @return [ChannelOptions] + def self.ChannelOptions(attributes) + case attributes + when ChannelOptions + return attributes + else + ChannelOptions.new(attributes) + end + end + # Represents options of a channel class ChannelOptions - MODES = [:publish, :subscribe, :presence, :presence_subscribe].freeze + extend Ably::Modules::Enum + extend Forwardable + include Ably::Modules::ModelCommon - # {Ably::Realtime::Channel} this object associated with - # - # @return [Ably::Realtime::Channel] - attr_reader :channel + MODES = ruby_enum('MODES', + presence: 0, + publish: 1, + subscribe: 2, + presence_subscribe: 3 + ) - # (TB2c) params (for realtime client libraries only) a Dict of key/value pairs - # - # @return [Hash] - attr_reader :params + attr_reader :attributes + + alias_method :to_h, :attributes - # (TB2d) modes (for realtime client libraries only) an array of ChannelMode + def_delegators :attributes, :fetch + # Initialize a new ChannelOptions # - # @return [Array<>] - attr_reader :modes + # @option params [Hash] (TB2c) params (for realtime client libraries only) a of key/value pairs + # @option modes [Hash] modes (for realtime client libraries only) an array of ChannelMode + # @option cipher [Hash,Ably::Models::CipherParams] :cipher A hash of options or a {Ably::Models::CipherParams} to configure the encryption. *:key* is required, all other options are optional. + # + def initialize(attributes) + @attributes = IdiomaticRubyWrapper(attributes.clone) - def initialize(channel, params = {}, modes = []) - @channel = channel - @params = params - @modes = modes + attributes[:cipher] = Ably::Models::CipherParams(cipher) if cipher + attributes.clone end - # Get value of the key from the params - # @return [String] + # @!attribute cipher # - def [](key) - @params[key] + # @return [CipherParams] + def cipher + attributes[:cipher] end - # Delegates fetching a key from the params to the params instance variable - # @return [String] + # @!attribute params # - def fetch(*args) - @params.fetch(*args) - end - - # Returns a params hash # @return [Hash] - # - def to_h - @params.to_h + def params + attributes[:params].to_h end - alias to_hash to_h - # Converts the params to the string - # @return [String] + # @!attribute modes # - def to_s - @params.to_s + # @return [Array<>] + def modes + attributes[:modes].to_a end end end diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index 5aa7937aa..d34baa399 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -96,8 +96,7 @@ class Channel # # @param client [Ably::Rest::Client] # @param name [String] The name of the channel - # @param channel_options [Hash] Channel options, currently reserved for Encryption options - # @option channel_options [Hash,Ably::Models::CipherParams] :cipher A hash of options or a {Ably::Models::CipherParams} to configure the encryption. *:key* is required, all other options are optional. See {Ably::Util::Crypto#initialize} for a list of +:cipher+ options + # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # def initialize(client, name, channel_options = {}) name = ensure_utf_8(:name, name) @@ -309,6 +308,15 @@ def __incoming_msgbus__ ) end + # Sets or updates the stored channel options. (#RSL7) (#RTL16) + # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} + # @return [Ably::Models::ChannelOptions] + def set_options(channel_options) + @options = Ably::Models::ChannelOptions(channel_options) + end + alias update_options set_options + alias options= set_options + # @api private def set_channel_error_reason(error) @error_reason = error @@ -319,13 +327,6 @@ def clear_error_reason @error_reason = nil end - # @api private - def update_options(channel_options) - @options = Ably::Models::ChannelOptions.new(self, channel_options.clone).freeze - end - alias set_options update_options # (RSL7) - alias options= update_options - # Used by {Ably::Modules::StateEmitter} to debug state changes # @api private def logger diff --git a/lib/ably/realtime/presence.rb b/lib/ably/realtime/presence.rb index b62bf16bb..8c8a89fed 100644 --- a/lib/ably/realtime/presence.rb +++ b/lib/ably/realtime/presence.rb @@ -338,7 +338,7 @@ def create_presence_message(action, client_id, data) } Ably::Models::PresenceMessage.new(model, logger: logger).tap do |presence_message| - presence_message.encode(client.encoders, channel.options.params) do |encode_error, error_message| + presence_message.encode(client.encoders, channel.options) do |encode_error, error_message| client.logger.error error_message end end diff --git a/lib/ably/realtime/presence/members_map.rb b/lib/ably/realtime/presence/members_map.rb index d7b7e6f6c..ef2d8df32 100644 --- a/lib/ably/realtime/presence/members_map.rb +++ b/lib/ably/realtime/presence/members_map.rb @@ -207,7 +207,7 @@ def connection def setup_event_handlers presence.__incoming_msgbus__.subscribe(:presence, :sync) do |presence_message| - presence_message.decode(client.encoders, channel.options.params) do |encode_error, error_message| + presence_message.decode(client.encoders, channel.options) do |encode_error, error_message| client.logger.error error_message end update_members_and_emit_events presence_message diff --git a/lib/ably/rest/channel.rb b/lib/ably/rest/channel.rb index 49b298840..0e7181ed2 100644 --- a/lib/ably/rest/channel.rb +++ b/lib/ably/rest/channel.rb @@ -29,8 +29,7 @@ class Channel # # @param client [Ably::Rest::Client] # @param name [String] The name of the channel - # @param channel_options [Hash] Channel options, currently reserved for Encryption options - # @option channel_options [Hash,Ably::Models::CipherParams] :cipher A hash of options or a {Ably::Models::CipherParams} to configure the encryption. *:key* is required, all other options are optional. See {Ably::Util::Crypto#initialize} for a list of +:cipher+ options + # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # def initialize(client, name, channel_options = {}) name = (ensure_utf_8 :name, name) @@ -166,7 +165,7 @@ def presence # @api private def update_options(channel_options) - @options = Ably::Models::ChannelOptions.new(self, channel_options.clone).freeze + @options = Ably::Models::ChannelOptions(channel_options) end alias set_options update_options # (RSL7) alias options= update_options diff --git a/lib/ably/rest/presence.rb b/lib/ably/rest/presence.rb index a6448e80f..a025f5a5b 100644 --- a/lib/ably/rest/presence.rb +++ b/lib/ably/rest/presence.rb @@ -89,7 +89,7 @@ def base_path end def decode_message(presence_message) - presence_message.decode client.encoders, channel.options.params + presence_message.decode client.encoders, channel.options rescue Ably::Exceptions::CipherError, Ably::Exceptions::EncoderError => e client.logger.error { "Decoding Error on presence channel '#{channel.name}', presence message client_id '#{presence_message.client_id}'. #{e.class.name}: #{e.message}" } end diff --git a/lib/ably/util/crypto.rb b/lib/ably/util/crypto.rb index 06fae0d19..d8202812a 100644 --- a/lib/ably/util/crypto.rb +++ b/lib/ably/util/crypto.rb @@ -30,7 +30,7 @@ class Crypto # crypto.decrypt(decrypted) # => 'secret text' # def initialize(params) - @fixed_iv = params.delete(:fixed_iv) if params.kind_of?(Hash) + @fixed_iv = params[:fixed_iv] @cipher_params = Ably::Models::CipherParams(params) end diff --git a/spec/acceptance/realtime/channels_spec.rb b/spec/acceptance/realtime/channels_spec.rb index 11baf5ef2..927c77966 100644 --- a/spec/acceptance/realtime/channels_spec.rb +++ b/spec/acceptance/realtime/channels_spec.rb @@ -10,7 +10,8 @@ end it 'returns channel object and passes the provided options' do - expect(channel_with_options.options.params).to eql(options) + expect(channel_options).to be_a(Ably::Models::ChannelOptions) + expect(channel_options.to_h).to eq(options) stop_reactor end end @@ -20,7 +21,29 @@ auto_close Ably::Realtime::Client.new(key: api_key, environment: environment, protocol: protocol) end let(:channel_name) { random_str } - let(:options) { { key: 'value' } } + let(:options) do + { params: { key: 'value' } } + end + + subject(:channel_options) { channel_with_options.options } + + describe '#set_options (#RTL16)' do + let(:channel) { client.channel(channel_name) } + + it "updates channel's options" do + expect { channel.options = options }.to change { channel.options.to_h }.from({}).to(options) + stop_reactor + end + + context 'when providing Ably::Models::ChannelOptions object' do + let(:options_object) { Ably::Models::ChannelOptions.new(options) } + + it "updates channel's options" do + expect { channel.options = options_object}.to change { channel.options.to_h }.from({}).to(options) + stop_reactor + end + end + end describe 'using shortcut method #channel on the client object' do let(:channel) { client.channel(channel_name) } @@ -39,7 +62,7 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'overrides the existing channel options and returns the channel object' do - expect(original_channel.options.params).to_not include(:encrypted) + expect(original_channel.options.to_h).to_not include(:encrypted) new_channel = client.channels.get(channel_name, new_channel_options) expect(new_channel).to be_a(Ably::Realtime::Channel) expect(new_channel.options[:encrypted]).to eql(true) @@ -51,10 +74,10 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'returns the existing channel without modifying the channel options' do - expect(original_channel.options.params).to eql(options) + expect(original_channel.options.to_h).to eq(options) new_channel = client.channels.get(channel_name) expect(new_channel).to be_a(Ably::Realtime::Channel) - expect(original_channel.options.params).to eql(options) + expect(original_channel.options.to_h).to eq(options) stop_reactor end end diff --git a/spec/acceptance/rest/channels_spec.rb b/spec/acceptance/rest/channels_spec.rb index b07410101..6b6485e6d 100644 --- a/spec/acceptance/rest/channels_spec.rb +++ b/spec/acceptance/rest/channels_spec.rb @@ -5,11 +5,11 @@ shared_examples 'a channel' do it 'returns a channel object' do expect(channel).to be_a Ably::Rest::Channel - expect(channel.name).to eql(channel_name) + expect(channel.name).to eq(channel_name) end it 'returns channel object and passes the provided options' do - expect(channel_with_options.options.params).to eql(options) + expect(channel_with_options.options.to_h).to eq(options) end end @@ -32,12 +32,28 @@ it_behaves_like 'a channel' end + describe '#set_options (#RTL16)' do + let(:channel) { client.channel(channel_name) } + + it "updates channel's options" do + expect { channel.options = options }.to change { channel.options.to_h }.from({}).to(options) + end + + context 'when providing Ably::Models::ChannelOptions object' do + let(:options_object) { Ably::Models::ChannelOptions.new(options) } + + it "updates channel's options" do + expect { channel.options = options_object}.to change { channel.options.to_h }.from({}).to(options) + end + end + end + describe 'accessing an existing channel object with different options' do let(:new_channel_options) { { encrypted: true } } let(:original_channel) { client.channels.get(channel_name, options) } it 'overrides the existing channel options and returns the channel object (RSN3c)' do - expect(original_channel.options.params).to_not include(:encrypted) + expect(original_channel.options.to_h).to_not include(:encrypted) new_channel = client.channels.get(channel_name, new_channel_options) expect(new_channel).to be_a(Ably::Rest::Channel) @@ -49,10 +65,10 @@ let(:original_channel) { client.channels.get(channel_name, options) } it 'returns the existing channel without modifying the channel options' do - expect(original_channel.options.params).to eql(options) + expect(original_channel.options.to_h).to eq(options) new_channel = client.channels.get(channel_name) expect(new_channel).to be_a(Ably::Rest::Channel) - expect(original_channel.options.params).to eql(options) + expect(original_channel.options.to_h).to eq(options) end end diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index 81e8d5768..27556926b 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -5,7 +5,9 @@ let(:connection) { instance_double('Ably::Realtime::Connection', unsafe_on: true, on_resume: true) } let(:client) { instance_double('Ably::Realtime::Client', connection: connection, client_id: 'clientId') } let(:channel_name) { 'unique' } - let(:options) { { 'bizarre' => 'value' } } + let(:options) do + { params: { bizarre: 'value' } } + end subject { Ably::Realtime::Channels.new(client) } @@ -25,17 +27,17 @@ it 'will update the options on the channel if provided (RSN3c)' do channel = subject.get(channel_name, options) - expect(channel.options.params).to eql(options) - expect(channel.options.params).to_not include(:encrypted) + expect(channel.options.to_h).to eq(options) + expect(channel.options.to_h).to_not include(:encrypted) subject.get(channel_name, encrypted: true) - expect(channel.options[:encrypted]).to eql(true) + expect(channel.options[:encrypted]).to eq(true) end it 'will leave the options intact on the channel if not provided' do channel = subject.get(channel_name, options) - expect(channel.options.params).to eql(options) + expect(channel.options.to_h).to eq(options) subject.get(channel_name) - expect(channel.options.params).to eql(options) + expect(channel.options.to_h).to eq(options) end end end From f97bc61614781a49fb97dce2027020eb1102356b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Thu, 18 Nov 2021 06:05:40 +0300 Subject: [PATCH 04/16] Implement RTS3a Previously Channels#get would only accept hash as an argument for options. This commit adds specs for that case and also updates docs --- lib/ably/modules/channels_collection.rb | 2 +- lib/ably/realtime/channels.rb | 2 +- spec/spec_helper.rb | 1 + spec/support/channel_options_helper.rb | 25 +++++++++++++++++++++++++ spec/unit/realtime/channels_spec.rb | 20 +++++++++++++------- 5 files changed, 41 insertions(+), 9 deletions(-) create mode 100644 spec/support/channel_options_helper.rb diff --git a/lib/ably/modules/channels_collection.rb b/lib/ably/modules/channels_collection.rb index 113fdbdb3..b9b93b493 100644 --- a/lib/ably/modules/channels_collection.rb +++ b/lib/ably/modules/channels_collection.rb @@ -13,7 +13,7 @@ def initialize(client, channel_klass) # Return a Channel for the given name # # @param name [String] The name of the channel - # @param channel_options [Hash] Channel options including the encryption options + # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # # @return [Channel] # diff --git a/lib/ably/realtime/channels.rb b/lib/ably/realtime/channels.rb index 9b06dea26..ef52e8d3d 100644 --- a/lib/ably/realtime/channels.rb +++ b/lib/ably/realtime/channels.rb @@ -13,7 +13,7 @@ def initialize(client) # Return a {Ably::Realtime::Channel} for the given name # # @param name [String] The name of the channel - # @param channel_options [Hash] Channel options, currently reserved for Encryption options + # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # @return [Ably::Realtime::Channel} # def get(*args) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e96e9cdf9..e9fc87161 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -27,6 +27,7 @@ def console(message) require 'support/event_emitter_helper' require 'support/private_api_formatter' require 'support/protocol_helper' +require 'support/channel_options_helper' require 'support/random_helper' require 'support/serialization_helper' require 'support/test_logger_helper' diff --git a/spec/support/channel_options_helper.rb b/spec/support/channel_options_helper.rb new file mode 100644 index 000000000..3ce8cff4a --- /dev/null +++ b/spec/support/channel_options_helper.rb @@ -0,0 +1,25 @@ +module RSpec + module ChannelOptionsHelper + def with_different_option_types(var_name, &block) + shared_examples 'a method that accepts different types of channel options' do + describe 'hash' do + let(:channel_options) { public_send(var_name) } + it { expect(channel_options).to be_a(Hash) } + context("when options are Hash", &block) + end + + describe 'object' do + let(:channel_options) { Ably::Models::ChannelOptions.new(public_send(var_name)) } + it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } + context("when options are ChannelOptions", &block) + end + end + + it_behaves_like 'a method that accepts different types of channel options' + end + end +end + +RSpec.configure do |config| + config.extend RSpec::ChannelOptionsHelper +end diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index 27556926b..62fc98eb3 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -13,16 +13,22 @@ context 'creating channels' do context '#get' do - it 'creates a channel if it does not exist (RSN3a)' do - expect(Ably::Realtime::Channel).to receive(:new).with(client, channel_name, options) - subject.get(channel_name, options) + context "when channel doesn't exist" do + with_different_option_types(:options) do + it 'creates a channel (RSN3a)' do + expect(Ably::Realtime::Channel).to receive(:new).with(client, channel_name, channel_options) + subject.get(channel_name, channel_options) + end + end end context 'when an existing channel exists' do - it 'will reuse a channel object if it exists (RSN3a)' do - channel = subject.get(channel_name, options) - expect(channel).to be_a(Ably::Realtime::Channel) - expect(subject.get(channel_name, options).object_id).to eql(channel.object_id) + with_different_option_types(:options) do + it 'will reuse a channel object if it exists (RSN3a)' do + channel = subject.get(channel_name, options) + expect(channel).to be_a(Ably::Realtime::Channel) + expect(subject.get(channel_name, options).object_id).to eql(channel.object_id) + end end it 'will update the options on the channel if provided (RSN3c)' do From 603633d536e11535323e1647fda47d23c2090161 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Thu, 18 Nov 2021 07:11:02 +0300 Subject: [PATCH 05/16] RTL4l --- lib/ably/models/channel_options.rb | 16 +++++++++---- lib/ably/models/protocol_message.rb | 18 +++++++++++---- lib/ably/realtime/channel/channel_manager.rb | 11 +++++---- spec/acceptance/realtime/channel_spec.rb | 24 ++++++++++++++++++++ spec/lib/unit/models/channel_options_spec.rb | 24 ++++++++++++++++++++ 5 files changed, 80 insertions(+), 13 deletions(-) create mode 100644 spec/lib/unit/models/channel_options_spec.rb diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index a07caf559..086d65327 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -37,9 +37,10 @@ class ChannelOptions # @option modes [Hash] modes (for realtime client libraries only) an array of ChannelMode # @option cipher [Hash,Ably::Models::CipherParams] :cipher A hash of options or a {Ably::Models::CipherParams} to configure the encryption. *:key* is required, all other options are optional. # - def initialize(attributes) - @attributes = IdiomaticRubyWrapper(attributes.clone) + def initialize(attrs) + @attributes = IdiomaticRubyWrapper(attrs.clone) + attributes[:modes] = modes.to_a.map { |mode| Ably::Models::ChannelOptions::MODES[mode] } if modes attributes[:cipher] = Ably::Models::CipherParams(cipher) if cipher attributes.clone end @@ -60,9 +61,16 @@ def params # @!attribute modes # - # @return [Array<>] + # @return [Array] def modes - attributes[:modes].to_a + attributes[:modes] + end + + # Converts modes to a bitfield that coresponds to ProtocolMessage#flags + # + # @return [Integer] + def message_flags + modes.map { |mode| Ably::Models::ProtocolMessage::ATTACH_FLAGS_MAPPING[mode.to_sym] }.reduce(:|) end end end diff --git a/lib/ably/models/protocol_message.rb b/lib/ably/models/protocol_message.rb index 1534a135e..10f09189b 100644 --- a/lib/ably/models/protocol_message.rb +++ b/lib/ably/models/protocol_message.rb @@ -66,6 +66,14 @@ class ProtocolMessage auth: 17 ) + ATTACH_FLAGS_MAPPING = { + resume: 32, # 2^5 + presence: 65536, # 2^16 + publish: 131072, # 2^17 + subscribe: 262144, # 2^18 + presence_subscribe: 524288, # 2^19 + } + # Indicates this protocol message action will generate an ACK response such as :message or :presence # @api private def self.ack_required?(for_action) @@ -218,27 +226,27 @@ def has_transient_flag? # @api private def has_attach_resume_flag? - flags & 32 == 32 # 2^5 + flags & ATTACH_FLAGS_MAPPING[:resume] == ATTACH_FLAGS_MAPPING[:resume] # 2^5 end # @api private def has_attach_presence_flag? - flags & 65536 == 65536 # 2^16 + flags & ATTACH_FLAGS_MAPPING[:presence] == ATTACH_FLAGS_MAPPING[:presence] # 2^16 end # @api private def has_attach_publish_flag? - flags & 131072 == 131072 # 2^17 + flags & ATTACH_FLAGS_MAPPING[:publish] == ATTACH_FLAGS_MAPPING[:publish] # 2^17 end # @api private def has_attach_subscribe_flag? - flags & 262144 == 262144 # 2^18 + flags & ATTACH_FLAGS_MAPPING[:subscribe] == ATTACH_FLAGS_MAPPING[:subscribe] # 2^18 end # @api private def has_attach_presence_subscribe_flag? - flags & 524288 == 524288 # 2^19 + flags & ATTACH_FLAGS_MAPPING[:presence_subscribe] == ATTACH_FLAGS_MAPPING[:presence_subscribe] # 2^19 end def connection_details diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index 4e61a19ca..06ebec474 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -199,14 +199,15 @@ def channel_retry_timeout end def send_attach_protocol_message - send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended # move to suspended + message_opptions = { flags: channel.options.message_flags } if channel.options.modes + send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended, message_opptions end def send_detach_protocol_message(previous_state) send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Detach, previous_state # return to previous state if failed end - def send_state_change_protocol_message(new_state, state_if_failed) + def send_state_change_protocol_message(new_state, state_if_failed, message_options = {}) state_at_time_of_request = channel.state @pending_state_change_timer = EventMachine::Timer.new(realtime_request_timeout) do if channel.state == state_at_time_of_request @@ -227,7 +228,8 @@ def send_state_change_protocol_message(new_state, state_if_failed) next unless pending_state_change_timer connection.send_protocol_message( action: new_state.to_i, - channel: channel.name + channel: channel.name, + **message_options.to_h ) resend_if_disconnected_and_connected.call end @@ -237,7 +239,8 @@ def send_state_change_protocol_message(new_state, state_if_failed) connection.send_protocol_message( action: new_state.to_i, - channel: channel.name + channel: channel.name, + **message_options.to_h ) end diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index 8f437aad6..9b7a2dbb9 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -117,6 +117,30 @@ def disconnect_transport end end + context 'context when channel options contain modes' do + before do + channel.options = { modes: %i[publish] } + end + + it 'sends an ATTACH with options as flags (#RTL4l)' do + connection.once(:connected) do + # client.connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| + # return if protocol_message.action != :attached + + # expect(protocol_message.has_attach_publish_flag?).to(eq) + # end + client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| + next if protocol_message.action != :attach + + expect(protocol_message.has_attach_publish_flag?).to eq(true) + stop_reactor + end + + channel.attach + end + end + end + it 'implicitly attaches the channel (#RTL7c)' do expect(channel).to be_initialized channel.subscribe { |message| } diff --git a/spec/lib/unit/models/channel_options_spec.rb b/spec/lib/unit/models/channel_options_spec.rb new file mode 100644 index 000000000..cfd2b4d0d --- /dev/null +++ b/spec/lib/unit/models/channel_options_spec.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ably::Models::ChannelOptions do + let(:options) { described_class.new(modes: modes) } + + describe 'message_flags' do + let(:modes) { %w[publish subscribe presence_subscribe] } + + subject(:protocol_message) do + Ably::Models::ProtocolMessage.new(action: Ably::Models::ProtocolMessage::ACTION.Attach, flags: options.message_flags) + end + + it 'converts modes to ProtocolMessage#flags correctly' do + expect(protocol_message.has_attach_publish_flag?).to eq(true) + expect(protocol_message.has_attach_subscribe_flag?).to eq(true) + expect(protocol_message.has_attach_presence_subscribe_flag?).to eq(true) + + expect(protocol_message.has_attach_resume_flag?).to eq(false) + expect(protocol_message.has_attach_presence_flag?).to eq(false) + end + end +end From 3e1f4d5f68c7363b88c3cecf9ed455ae0f803ac7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Thu, 18 Nov 2021 14:31:30 +0300 Subject: [PATCH 06/16] RTL4m --- lib/ably/models/channel_options.rb | 17 ++++++++++++++++- lib/ably/realtime/channel/channel_manager.rb | 4 +++- spec/acceptance/realtime/channel_spec.rb | 18 +++++++++++++----- spec/lib/unit/models/channel_options_spec.rb | 16 ++++++++++++++-- 4 files changed, 46 insertions(+), 9 deletions(-) diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index 086d65327..dc4792113 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -69,8 +69,23 @@ def modes # Converts modes to a bitfield that coresponds to ProtocolMessage#flags # # @return [Integer] - def message_flags + def modes_to_flags modes.map { |mode| Ably::Models::ProtocolMessage::ATTACH_FLAGS_MAPPING[mode.to_sym] }.reduce(:|) end + + + # Sets modes from ProtocolMessage#flags + # + # @return [Array] + def set_modes_from_flags(flags) + return unless flags + + message_modes = MODES.select do |mode| + flag = Ably::Models::ProtocolMessage::ATTACH_FLAGS_MAPPING[mode.to_sym] + flags & flag == flag + end + + attributes[:modes] = message_modes.map { |mode| Ably::Models::ChannelOptions::MODES[mode] } + end end end diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index 06ebec474..02a867a76 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -38,6 +38,7 @@ def attached(attached_protocol_message) if attached_protocol_message update_presence_sync_state_following_attached attached_protocol_message channel.properties.set_attach_serial(attached_protocol_message.channel_serial) + channel.options.set_modes_from_flags(attached_protocol_message.flags) end end @@ -64,6 +65,7 @@ def duplicate_attached_received(protocol_message) end channel.properties.set_attach_serial(protocol_message.channel_serial) + channel.options.set_modes_from_flags(protocol_message.flags) if protocol_message.has_channel_resumed_flag? logger.debug { "ChannelManager: Additional resumed ATTACHED message received for #{channel.state} channel '#{channel.name}'" } @@ -199,7 +201,7 @@ def channel_retry_timeout end def send_attach_protocol_message - message_opptions = { flags: channel.options.message_flags } if channel.options.modes + message_opptions = { flags: channel.options.modes_to_flags } if channel.options.modes send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended, message_opptions end diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index 9b7a2dbb9..68adc33d2 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -124,11 +124,6 @@ def disconnect_transport it 'sends an ATTACH with options as flags (#RTL4l)' do connection.once(:connected) do - # client.connection.__incoming_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| - # return if protocol_message.action != :attached - - # expect(protocol_message.has_attach_publish_flag?).to(eq) - # end client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| next if protocol_message.action != :attach @@ -141,6 +136,19 @@ def disconnect_transport end end + context 'when received attached' do + it 'decodes flags and sets it as modes on channel options (#RTL4m)'do + channel.on(:attached) do + expect(channel.options.modes.map(&:to_sym)).to eq(%i[subscribe]) + stop_reactor + end + + channel.transition_state_machine(:attaching) + attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 262144) + client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message + end + end + it 'implicitly attaches the channel (#RTL7c)' do expect(channel).to be_initialized channel.subscribe { |message| } diff --git a/spec/lib/unit/models/channel_options_spec.rb b/spec/lib/unit/models/channel_options_spec.rb index cfd2b4d0d..1ee9bd5ca 100644 --- a/spec/lib/unit/models/channel_options_spec.rb +++ b/spec/lib/unit/models/channel_options_spec.rb @@ -3,13 +3,14 @@ require 'spec_helper' RSpec.describe Ably::Models::ChannelOptions do + let(:modes) { nil } let(:options) { described_class.new(modes: modes) } - describe 'message_flags' do + describe '#modes_to_flags' do let(:modes) { %w[publish subscribe presence_subscribe] } subject(:protocol_message) do - Ably::Models::ProtocolMessage.new(action: Ably::Models::ProtocolMessage::ACTION.Attach, flags: options.message_flags) + Ably::Models::ProtocolMessage.new(action: Ably::Models::ProtocolMessage::ACTION.Attach, flags: options.modes_to_flags) end it 'converts modes to ProtocolMessage#flags correctly' do @@ -21,4 +22,15 @@ expect(protocol_message.has_attach_presence_flag?).to eq(false) end end + + describe '#set_modes_from_flags' do + let(:subscribe_flag) { 262144 } + + it 'converts flags to ChannelOptions#modes correctly' do + result = options.set_modes_from_flags(subscribe_flag) + + expect(result).to eq(options.modes) + expect(options.modes.map(&:to_sym)).to eq(%i[subscribe]) + end + end end From c97190b33c20b29a11472b0cdef383c77a88c7a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Thu, 18 Nov 2021 17:00:15 +0300 Subject: [PATCH 07/16] RSN3a, RSN3c --- lib/ably/models/channel_options.rb | 2 +- lib/ably/modules/channels_collection.rb | 2 +- spec/unit/realtime/channels_spec.rb | 8 ++-- spec/unit/rest/channels_spec.rb | 53 +++++++++++++++++++------ 4 files changed, 46 insertions(+), 19 deletions(-) diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index dc4792113..ce81fe0c1 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -30,7 +30,7 @@ class ChannelOptions alias_method :to_h, :attributes - def_delegators :attributes, :fetch + def_delegators :attributes, :fetch, :size # Initialize a new ChannelOptions # # @option params [Hash] (TB2c) params (for realtime client libraries only) a of key/value pairs diff --git a/lib/ably/modules/channels_collection.rb b/lib/ably/modules/channels_collection.rb index b9b93b493..ed82ad56c 100644 --- a/lib/ably/modules/channels_collection.rb +++ b/lib/ably/modules/channels_collection.rb @@ -20,7 +20,7 @@ def initialize(client, channel_klass) def get(name, channel_options = {}) if channels.has_key?(name) channels[name].tap do |channel| - channel.update_options channel_options if channel_options && !channel_options.empty? + channel.update_options channel_options if channel_options && channel_options.size.nonzero? end else channels[name] ||= channel_klass.new(client, name, channel_options) diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index 62fc98eb3..07eb58f6b 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -15,7 +15,7 @@ context '#get' do context "when channel doesn't exist" do with_different_option_types(:options) do - it 'creates a channel (RSN3a)' do + it 'creates a channel (RTS3a)' do expect(Ably::Realtime::Channel).to receive(:new).with(client, channel_name, channel_options) subject.get(channel_name, channel_options) end @@ -24,10 +24,10 @@ context 'when an existing channel exists' do with_different_option_types(:options) do - it 'will reuse a channel object if it exists (RSN3a)' do - channel = subject.get(channel_name, options) + it 'will reuse a channel object if it exists (RTS3a)' do + channel = subject.get(channel_name, channel_options) expect(channel).to be_a(Ably::Realtime::Channel) - expect(subject.get(channel_name, options).object_id).to eql(channel.object_id) + expect(subject.get(channel_name, channel_options).object_id).to eql(channel.object_id) end end diff --git a/spec/unit/rest/channels_spec.rb b/spec/unit/rest/channels_spec.rb index 1a6221ac2..9c9df56de 100644 --- a/spec/unit/rest/channels_spec.rb +++ b/spec/unit/rest/channels_spec.rb @@ -4,28 +4,55 @@ describe Ably::Rest::Channels do let(:client) { instance_double('Ably::Rest::Client') } let(:channel_name) { 'unique'.encode(Encoding::UTF_8) } - let(:options) { { 'bizarre' => 'value' } } + let(:options) do + { params: { 'bizarre' => 'value' } } + end subject { Ably::Rest::Channels.new(client) } - context 'creating channels' do - it '#get creates a channel' do - expect(Ably::Rest::Channel).to receive(:new).with(client, channel_name, options) - subject.get(channel_name, options) + describe '#get' do + context "when channel doesn't exist" do + with_different_option_types(:options) do + it 'creates a channel (RSN3a)' do + expect(Ably::Rest::Channel).to receive(:new).with(client, channel_name, options) + subject.get(channel_name, options) + end + end end - it '#get will reuse the channel object' do - channel = subject.get(channel_name, options) - expect(channel).to be_a(Ably::Rest::Channel) - expect(subject.get(channel_name, options).object_id).to eql(channel.object_id) - end + context 'when an existing channel exists' do + with_different_option_types(:options) do + it 'will reuse a channel object if it exists (RSN3a)' do + channel = subject.get(channel_name, channel_options) + expect(channel).to be_a(Ably::Rest::Channel) + expect(subject.get(channel_name, channel_options).object_id).to eql(channel.object_id) + end + end - it '[] creates a channel' do - expect(Ably::Rest::Channel).to receive(:new).with(client, channel_name, options) - subject.get(channel_name, options) + context 'with new channel_options' do + let(:modes) { %i[subscribe] } + let(:new_options) do + { modes: modes } + end + + with_different_option_types(:new_options) do + it 'will update channel with provided options (RSN3c)' do + channel = subject.get(channel_name, options) + expect(channel.options.modes).to be_nil + + subject.get(channel_name, channel_options) + expect(channel.options.modes).to eq(modes) + end + end + end end end + it '[] creates a channel' do + expect(Ably::Rest::Channel).to receive(:new).with(client, channel_name, options) + subject.get(channel_name, options) + end + context '#fetch' do it 'retrieves a channel if it exists' do channel = subject.get(channel_name, options) From d33d9e399c4f3eac082037c3e8a0dc2fdeb445e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Thu, 18 Nov 2021 17:53:43 +0300 Subject: [PATCH 08/16] RTL16a --- lib/ably/modules/channels_collection.rb | 2 +- lib/ably/realtime/channel.rb | 16 +++++-- lib/ably/rest/channel.rb | 13 ++++-- spec/acceptance/realtime/channel_spec.rb | 58 ++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 7 deletions(-) diff --git a/lib/ably/modules/channels_collection.rb b/lib/ably/modules/channels_collection.rb index ed82ad56c..12d2c6e26 100644 --- a/lib/ably/modules/channels_collection.rb +++ b/lib/ably/modules/channels_collection.rb @@ -20,7 +20,7 @@ def initialize(client, channel_klass) def get(name, channel_options = {}) if channels.has_key?(name) channels[name].tap do |channel| - channel.update_options channel_options if channel_options && channel_options.size.nonzero? + channel.options = channel_options if channel_options && channel_options.size.nonzero? end else channels[name] ||= channel_klass.new(client, name, channel_options) diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index d34baa399..894aaf2c7 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -308,13 +308,14 @@ def __incoming_msgbus__ ) end - # Sets or updates the stored channel options. (#RSL7) (#RTL16) + # Sets or updates the stored channel options. (#RTL16) # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # @return [Ably::Models::ChannelOptions] def set_options(channel_options) - @options = Ably::Models::ChannelOptions(channel_options) + update_options(channel_options) + + manager.request_reattach if need_reattach? end - alias update_options set_options alias options= set_options # @api private @@ -338,6 +339,15 @@ def logger private :change_state private + + def need_reattach? + !!(attaching? || attached?) && !!(options.modes || options.params) + end + + def update_options(channel_options) + @options = Ably::Models::ChannelOptions(channel_options) + end + def setup_event_handlers __incoming_msgbus__.subscribe(:message) do |message| message.decode(client.encoders, options) do |encode_error, error_message| diff --git a/lib/ably/rest/channel.rb b/lib/ably/rest/channel.rb index 0e7181ed2..541c975b3 100644 --- a/lib/ably/rest/channel.rb +++ b/lib/ably/rest/channel.rb @@ -163,14 +163,21 @@ def presence @presence ||= Presence.new(client, self) end + # Sets or updates the stored channel options. (#RSL7) + # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} + # @return [Ably::Models::ChannelOptions] + def set_options(channel_options) + update_options(channel_options) + end + alias options= set_options + + private + # @api private def update_options(channel_options) @options = Ably::Models::ChannelOptions(channel_options) end - alias set_options update_options # (RSL7) - alias options= update_options - private def base_path "/channels/#{URI.encode_www_form_component(name)}" end diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index 68adc33d2..3b8945451 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -1982,6 +1982,64 @@ def fake_error(error) end end + describe '#set_options (#RTL16a)' do + let(:modes) { %i[subscribe] } + let(:channel_options) do + { modes: modes } + end + + shared_examples 'an update that sends ATTACH message' do |state| + it 'sends an ATTACH message on options change' do + attach_sent = 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(262144) + end + end + + channel.once(state) do + channel.options = channel_options + 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) + stop_reactor + end + end + + channel.attach + end + end + + context 'when channel is attaching' do + it_behaves_like 'an update that sends ATTACH message', :attaching + end + + context 'when channel is attaching' do + it_behaves_like 'an update that sends ATTACH message', :attached + end + + context 'when channel is initialized' do + it "doesn't send ATTACH message" do + client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| + raise "Unexpected message" if protocol_message.action == :attach + end + + channel.options = channel_options + expect(channel.options.modes.map(&:to_sym)).to eq(modes) + + EventMachine.next_tick do + stop_reactor + end + end + end + end + context 'channel state change' do it 'emits a ChannelStateChange object' do channel.on(:attached) do |channel_state_change| From b766d98ee06a61e9daa5ce1ee1a3bef1d277fd63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Fri, 19 Nov 2021 05:34:17 +0300 Subject: [PATCH 09/16] RTS3c1 --- lib/ably/modules/channels_collection.rb | 5 ++++- lib/ably/realtime/channel.rb | 21 ++++++++++++++++++++- spec/acceptance/realtime/channels_spec.rb | 19 +++++++------------ 3 files changed, 31 insertions(+), 14 deletions(-) diff --git a/lib/ably/modules/channels_collection.rb b/lib/ably/modules/channels_collection.rb index 12d2c6e26..e4117e1ca 100644 --- a/lib/ably/modules/channels_collection.rb +++ b/lib/ably/modules/channels_collection.rb @@ -20,7 +20,10 @@ def initialize(client, channel_klass) def get(name, channel_options = {}) if channels.has_key?(name) channels[name].tap do |channel| - channel.options = channel_options if channel_options && channel_options.size.nonzero? + if channel_options && channel_options.size.nonzero? + channel.raise_on_reattach! if channel.respond_to?(:raise_on_reattach!) + channel.options = channel_options + end end else channels[name] ||= channel_klass.new(client, name, channel_options) diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index 894aaf2c7..e33ff9c3b 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -92,6 +92,11 @@ class Channel # @api private attr_reader :manager + # Flag that controlls reattaching behavior on options update per RTS3c1 + # @return [Bolean] + # @api private + attr_reader :raise_on_reattach + # Initialize a new Channel object # # @param client [Ably::Rest::Client] @@ -111,6 +116,7 @@ def initialize(client, name, channel_options = {}) @manager = ChannelManager.new(self, client.connection) @push = PushChannel.new(self) @properties = ChannelProperties.new(self) + @raise_on_reattach = false setup_event_handlers setup_presence @@ -314,7 +320,11 @@ def __incoming_msgbus__ def set_options(channel_options) update_options(channel_options) - manager.request_reattach if need_reattach? + if need_reattach? + raise_reattachment_error if raise_on_reattach + + manager.request_reattach + end end alias options= set_options @@ -334,12 +344,21 @@ def logger client.logger end + # @api private + def raise_on_reattach! + @raise_on_reattach = true + end + # As we are using a state machine, do not allow change_state to be used # #transition_state_machine must be used instead private :change_state private + def raise_reattachment_error + raise ArgumentError, "You are trying to indirectly update channel options which will trigger reattachment of the channel. Please, use Channel#set_options directly if you wish to continue" + end + def need_reattach? !!(attaching? || attached?) && !!(options.modes || options.params) end diff --git a/spec/acceptance/realtime/channels_spec.rb b/spec/acceptance/realtime/channels_spec.rb index 927c77966..1419014fe 100644 --- a/spec/acceptance/realtime/channels_spec.rb +++ b/spec/acceptance/realtime/channels_spec.rb @@ -27,21 +27,16 @@ subject(:channel_options) { channel_with_options.options } - describe '#set_options (#RTL16)' do - let(:channel) { client.channel(channel_name) } - - it "updates channel's options" do - expect { channel.options = options }.to change { channel.options.to_h }.from({}).to(options) - stop_reactor - end - - context 'when providing Ably::Models::ChannelOptions object' do - let(:options_object) { Ably::Models::ChannelOptions.new(options) } + context 'when channel supposed to trigger reattachment per RTL16a (#RTS3c1)' do + it 'will raise an error' do + channel = client.channels.get(channel_name, options) - it "updates channel's options" do - expect { channel.options = options_object}.to change { channel.options.to_h }.from({}).to(options) + channel.on(:attached) do + expect { client.channels.get(channel_name, { modes: [] }) }.to raise_error ArgumentError, /use Channel#set_options directly/ stop_reactor end + + channel.attach end end From ca37d60d70dc398f7cc4b1b62d002e8e0f746cfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Mon, 29 Nov 2021 19:05:43 +0300 Subject: [PATCH 10/16] RTL4k --- lib/ably/models/channel_options.rb | 1 - lib/ably/models/protocol_message.rb | 4 ++++ lib/ably/realtime/channel/channel_manager.rb | 5 ++++- spec/acceptance/realtime/channel_spec.rb | 23 ++++++++++++++++++++ 4 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index ce81fe0c1..f3b955718 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -73,7 +73,6 @@ def modes_to_flags modes.map { |mode| Ably::Models::ProtocolMessage::ATTACH_FLAGS_MAPPING[mode.to_sym] }.reduce(:|) end - # Sets modes from ProtocolMessage#flags # # @return [Array] diff --git a/lib/ably/models/protocol_message.rb b/lib/ably/models/protocol_message.rb index 10f09189b..b07e1e64f 100644 --- a/lib/ably/models/protocol_message.rb +++ b/lib/ably/models/protocol_message.rb @@ -193,6 +193,10 @@ def has_correct_message_size? message_size <= connection_details.max_message_size end + def params + @params ||= attributes[:params].to_h + end + def flags Integer(attributes[:flags]) rescue TypeError diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index 02a867a76..9e79db424 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -201,7 +201,10 @@ def channel_retry_timeout end def send_attach_protocol_message - message_opptions = { flags: channel.options.modes_to_flags } if channel.options.modes + message_opptions = {} + message_opptions[:flags] = channel.options.modes_to_flags if channel.options.modes + message_opptions[:params] = channel.options.params if channel.options.params.any? + send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended, message_opptions end diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index 3b8945451..efcbb0ff3 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -136,6 +136,29 @@ def disconnect_transport end end + context 'context when channel options contain params' do + let(:params) do + { foo: 'foo', bar: 'bar'} + end + + before do + channel.options = { params: params } + end + + it 'sends an ATTACH with params (#RTL4k)' do + connection.once(:connected) do + client.connection.__outgoing_protocol_msgbus__.subscribe(:protocol_message) do |protocol_message| + next if protocol_message.action != :attach + + expect(protocol_message.params).to eq(params) + stop_reactor + end + + channel.attach + end + end + end + context 'when received attached' do it 'decodes flags and sets it as modes on channel options (#RTL4m)'do channel.on(:attached) do From b7610045be83b0eff80faa464da7837988897495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C2=A0TheSmartnik?= Date: Mon, 29 Nov 2021 19:42:53 +0300 Subject: [PATCH 11/16] RTL4k1 --- lib/ably/models/channel_options.rb | 9 +++++++++ lib/ably/realtime/channel.rb | 5 +++++ lib/ably/realtime/channel/channel_manager.rb | 1 + spec/acceptance/realtime/channel_spec.rb | 14 ++++++++++++++ spec/unit/models/protocol_message_spec.rb | 18 ++++++++++++++++++ 5 files changed, 47 insertions(+) diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index f3b955718..079afe68f 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -73,9 +73,18 @@ def modes_to_flags modes.map { |mode| Ably::Models::ProtocolMessage::ATTACH_FLAGS_MAPPING[mode.to_sym] }.reduce(:|) end + # @return [Hash] + # @api private + def set_params(hash) + return if hash.empty? + + attributes[:params] = hash + end + # Sets modes from ProtocolMessage#flags # # @return [Array] + # @api private def set_modes_from_flags(flags) return unless flags diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index e33ff9c3b..03163b8a9 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -36,6 +36,7 @@ class Channel include Ably::Modules::MessageEmitter include Ably::Realtime::Channel::Publisher extend Ably::Modules::Enum + extend Forwardable # ChannelState # The permited states for this channel @@ -97,6 +98,10 @@ class Channel # @api private attr_reader :raise_on_reattach + # ChannelOptions params attrribute (#RTL4k) + # return [Hash] + def_delegators :options, :params + # Initialize a new Channel object # # @param client [Ably::Rest::Client] diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index 9e79db424..6cbca04a6 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -39,6 +39,7 @@ def attached(attached_protocol_message) update_presence_sync_state_following_attached attached_protocol_message channel.properties.set_attach_serial(attached_protocol_message.channel_serial) channel.options.set_modes_from_flags(attached_protocol_message.flags) + channel.options.set_params(attached_protocol_message.params) end end diff --git a/spec/acceptance/realtime/channel_spec.rb b/spec/acceptance/realtime/channel_spec.rb index efcbb0ff3..38953e986 100644 --- a/spec/acceptance/realtime/channel_spec.rb +++ b/spec/acceptance/realtime/channel_spec.rb @@ -170,6 +170,20 @@ def disconnect_transport attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, flags: 262144) client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message end + + it 'set params as channel options params (#RTL4k1)' do + params = { param: :something } + + channel.on(:attached) do + expect(channel.params).to eq(channel.options.params) + expect(channel.params).to eq(params) + stop_reactor + end + + channel.transition_state_machine(:attaching) + attached_message = Ably::Models::ProtocolMessage.new(action: 11, channel: channel_name, params: params) + client.connection.__incoming_protocol_msgbus__.publish :protocol_message, attached_message + end end it 'implicitly attaches the channel (#RTL7c)' do diff --git a/spec/unit/models/protocol_message_spec.rb b/spec/unit/models/protocol_message_spec.rb index 53c7a20a6..44d6e2fe2 100644 --- a/spec/unit/models/protocol_message_spec.rb +++ b/spec/unit/models/protocol_message_spec.rb @@ -223,6 +223,24 @@ def new_protocol_message(options) end end + context '#params (#RTL4k1)' do + let(:params) do + { foo: :bar } + end + + context 'when present' do + specify do + expect(new_protocol_message({ params: params }).params).to eq(params) + end + end + + context 'when empty' do + specify do + expect(new_protocol_message({}).params).to eq({}) + end + end + end + context '#has_connection_serial?' do context 'without connection_serial' do let(:protocol_message) { new_protocol_message({}) } From a907de5164f5a99b69b914d8d3abb5ce276b5e4d Mon Sep 17 00:00:00 2001 From: TheSmartnik Date: Mon, 24 Jan 2022 18:18:53 +0300 Subject: [PATCH 12/16] After CR improvements --- lib/ably/models/channel_options.rb | 4 +--- lib/ably/models/idiomatic_ruby_wrapper.rb | 4 ++++ lib/ably/modules/channels_collection.rb | 2 +- lib/ably/realtime/channel.rb | 8 ++------ lib/ably/realtime/channel/channel_manager.rb | 8 ++++---- lib/ably/rest/channel.rb | 9 ++------- spec/lib/unit/models/channel_options_spec.rb | 18 +++++++++++++++++- 7 files changed, 31 insertions(+), 22 deletions(-) diff --git a/lib/ably/models/channel_options.rb b/lib/ably/models/channel_options.rb index 079afe68f..ec23f0be4 100644 --- a/lib/ably/models/channel_options.rb +++ b/lib/ably/models/channel_options.rb @@ -30,7 +30,7 @@ class ChannelOptions alias_method :to_h, :attributes - def_delegators :attributes, :fetch, :size + def_delegators :attributes, :fetch, :size, :empty? # Initialize a new ChannelOptions # # @option params [Hash] (TB2c) params (for realtime client libraries only) a of key/value pairs @@ -76,8 +76,6 @@ def modes_to_flags # @return [Hash] # @api private def set_params(hash) - return if hash.empty? - attributes[:params] = hash end diff --git a/lib/ably/models/idiomatic_ruby_wrapper.rb b/lib/ably/models/idiomatic_ruby_wrapper.rb index ce3e2c3ce..a9e964a1e 100644 --- a/lib/ably/models/idiomatic_ruby_wrapper.rb +++ b/lib/ably/models/idiomatic_ruby_wrapper.rb @@ -94,6 +94,10 @@ def size attributes.size end + def empty? + attributes.empty? + end + def keys map { |key, value| key } end diff --git a/lib/ably/modules/channels_collection.rb b/lib/ably/modules/channels_collection.rb index e4117e1ca..cc26ec047 100644 --- a/lib/ably/modules/channels_collection.rb +++ b/lib/ably/modules/channels_collection.rb @@ -20,7 +20,7 @@ def initialize(client, channel_klass) def get(name, channel_options = {}) if channels.has_key?(name) channels[name].tap do |channel| - if channel_options && channel_options.size.nonzero? + if channel_options && !channel_options.empty? channel.raise_on_reattach! if channel.respond_to?(:raise_on_reattach!) channel.options = channel_options end diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index 03163b8a9..fae389cbf 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -111,7 +111,7 @@ class Channel def initialize(client, name, channel_options = {}) name = ensure_utf_8(:name, name) - update_options channel_options + @options = Ably::Models::ChannelOptions(channel_options) @client = client @name = name @queue = [] @@ -323,7 +323,7 @@ def __incoming_msgbus__ # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # @return [Ably::Models::ChannelOptions] def set_options(channel_options) - update_options(channel_options) + @options = Ably::Models::ChannelOptions(channel_options) if need_reattach? raise_reattachment_error if raise_on_reattach @@ -368,10 +368,6 @@ def need_reattach? !!(attaching? || attached?) && !!(options.modes || options.params) end - def update_options(channel_options) - @options = Ably::Models::ChannelOptions(channel_options) - end - def setup_event_handlers __incoming_msgbus__.subscribe(:message) do |message| message.decode(client.encoders, options) do |encode_error, error_message| diff --git a/lib/ably/realtime/channel/channel_manager.rb b/lib/ably/realtime/channel/channel_manager.rb index 6cbca04a6..5cec145be 100644 --- a/lib/ably/realtime/channel/channel_manager.rb +++ b/lib/ably/realtime/channel/channel_manager.rb @@ -202,11 +202,11 @@ def channel_retry_timeout end def send_attach_protocol_message - message_opptions = {} - message_opptions[:flags] = channel.options.modes_to_flags if channel.options.modes - message_opptions[:params] = channel.options.params if channel.options.params.any? + message_options = {} + message_options[:flags] = channel.options.modes_to_flags if channel.options.modes + message_options[:params] = channel.options.params if channel.options.params.any? - send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended, message_opptions + send_state_change_protocol_message Ably::Models::ProtocolMessage::ACTION.Attach, :suspended, message_options end def send_detach_protocol_message(previous_state) diff --git a/lib/ably/rest/channel.rb b/lib/ably/rest/channel.rb index 541c975b3..972dcb217 100644 --- a/lib/ably/rest/channel.rb +++ b/lib/ably/rest/channel.rb @@ -34,7 +34,7 @@ class Channel def initialize(client, name, channel_options = {}) name = (ensure_utf_8 :name, name) - update_options channel_options + @options = Ably::Models::ChannelOptions(channel_options) @client = client @name = name @push = PushChannel.new(self) @@ -167,17 +167,12 @@ def presence # @param channel_options [Hash, Ably::Models::ChannelOptions] A hash of options or a {Ably::Models::ChannelOptions} # @return [Ably::Models::ChannelOptions] def set_options(channel_options) - update_options(channel_options) + @options = Ably::Models::ChannelOptions(channel_options) end alias options= set_options private - # @api private - def update_options(channel_options) - @options = Ably::Models::ChannelOptions(channel_options) - end - def base_path "/channels/#{URI.encode_www_form_component(name)}" end diff --git a/spec/lib/unit/models/channel_options_spec.rb b/spec/lib/unit/models/channel_options_spec.rb index 1ee9bd5ca..1d9d16a8d 100644 --- a/spec/lib/unit/models/channel_options_spec.rb +++ b/spec/lib/unit/models/channel_options_spec.rb @@ -4,7 +4,8 @@ RSpec.describe Ably::Models::ChannelOptions do let(:modes) { nil } - let(:options) { described_class.new(modes: modes) } + let(:params) { {} } + let(:options) { described_class.new(modes: modes, params: params) } describe '#modes_to_flags' do let(:modes) { %w[publish subscribe presence_subscribe] } @@ -33,4 +34,19 @@ expect(options.modes.map(&:to_sym)).to eq(%i[subscribe]) end end + + describe '#set_params' do + let(:previous_params) { { example_attribute: 1 } } + let(:new_params) { { new_attribute: 1 } } + let(:params) { previous_params } + + it 'should be able to overwrite attributes' do + expect { options.set_params(new_params) }.to \ + change { options.params }.from(previous_params).to(new_params) + end + + it 'should be able to make params empty' do # (1) + expect { options.set_params({}) }.to change { options.params }.from(previous_params).to({}) + end + end end From 81be2438d09fb173a6c2b42a9d39e7ca1be76172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=C5=9Aliwa?= Date: Mon, 28 Feb 2022 19:09:18 +0100 Subject: [PATCH 13/16] Removed with_different_option_types(...) helper. Added shared_examples to rest/channels and realtime/channels. --- spec/support/channel_options_helper.rb | 25 ----------- spec/unit/realtime/channels_spec.rb | 32 ++++++++++++- spec/unit/rest/channels_spec.rb | 62 +++++++++++++++++++++----- 3 files changed, 81 insertions(+), 38 deletions(-) delete mode 100644 spec/support/channel_options_helper.rb diff --git a/spec/support/channel_options_helper.rb b/spec/support/channel_options_helper.rb deleted file mode 100644 index 3ce8cff4a..000000000 --- a/spec/support/channel_options_helper.rb +++ /dev/null @@ -1,25 +0,0 @@ -module RSpec - module ChannelOptionsHelper - def with_different_option_types(var_name, &block) - shared_examples 'a method that accepts different types of channel options' do - describe 'hash' do - let(:channel_options) { public_send(var_name) } - it { expect(channel_options).to be_a(Hash) } - context("when options are Hash", &block) - end - - describe 'object' do - let(:channel_options) { Ably::Models::ChannelOptions.new(public_send(var_name)) } - it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } - context("when options are ChannelOptions", &block) - end - end - - it_behaves_like 'a method that accepts different types of channel options' - end - end -end - -RSpec.configure do |config| - config.extend RSpec::ChannelOptionsHelper -end diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index 07eb58f6b..43e1b67cf 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -14,16 +14,30 @@ context 'creating channels' do context '#get' do context "when channel doesn't exist" do - with_different_option_types(:options) do + shared_examples 'creates a channel' do it 'creates a channel (RTS3a)' do expect(Ably::Realtime::Channel).to receive(:new).with(client, channel_name, channel_options) subject.get(channel_name, channel_options) end end + + describe 'hash' do + let(:channel_options) { options } + it { expect(channel_options).to be_a(Hash) } + + include_examples 'creates a channel' + end + + describe 'ChannelOptions object' do + let(:channel_options) { Ably::Models::ChannelOptions.new(options) } + it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } + + include_examples 'creates a channel' + end end context 'when an existing channel exists' do - with_different_option_types(:options) do + shared_examples 'reuse a channel object if it exists' do it 'will reuse a channel object if it exists (RTS3a)' do channel = subject.get(channel_name, channel_options) expect(channel).to be_a(Ably::Realtime::Channel) @@ -31,6 +45,20 @@ end end + describe 'hash' do + let(:channel_options) { options } + it { expect(channel_options).to be_a(Hash) } + + include_examples 'reuse a channel object if it exists' + end + + describe 'ChannelOptions object' do + let(:channel_options) { Ably::Models::ChannelOptions.new(options) } + it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } + + include_examples 'reuse a channel object if it exists' + end + it 'will update the options on the channel if provided (RSN3c)' do channel = subject.get(channel_name, options) expect(channel.options.to_h).to eq(options) diff --git a/spec/unit/rest/channels_spec.rb b/spec/unit/rest/channels_spec.rb index 9c9df56de..da50c5c4c 100644 --- a/spec/unit/rest/channels_spec.rb +++ b/spec/unit/rest/channels_spec.rb @@ -12,16 +12,30 @@ describe '#get' do context "when channel doesn't exist" do - with_different_option_types(:options) do + shared_examples 'creates a channel' do it 'creates a channel (RSN3a)' do expect(Ably::Rest::Channel).to receive(:new).with(client, channel_name, options) subject.get(channel_name, options) end end + + describe 'hash' do + let(:channel_options) { options } + it { expect(channel_options).to be_a(Hash) } + + include_examples 'creates a channel' + end + + describe 'ChannelOptions object' do + let(:channel_options) { Ably::Models::ChannelOptions.new(options) } + it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } + + include_examples 'creates a channel' + end end context 'when an existing channel exists' do - with_different_option_types(:options) do + shared_examples 'reuse a channel object if it exists' do it 'will reuse a channel object if it exists (RSN3a)' do channel = subject.get(channel_name, channel_options) expect(channel).to be_a(Ably::Rest::Channel) @@ -29,21 +43,47 @@ end end - context 'with new channel_options' do - let(:modes) { %i[subscribe] } - let(:new_options) do - { modes: modes } - end + describe 'hash' do + let(:channel_options) { options } + it { expect(channel_options).to be_a(Hash) } + + include_examples 'reuse a channel object if it exists' + end + + describe 'ChannelOptions object' do + let(:channel_options) { Ably::Models::ChannelOptions.new(options) } + it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } - with_different_option_types(:new_options) do - it 'will update channel with provided options (RSN3c)' do - channel = subject.get(channel_name, options) - expect(channel.options.modes).to be_nil + include_examples 'reuse a channel object if it exists' + end + + context 'with new channel_options modes' do + shared_examples 'update channel with provided options :modes' do + it 'will update channel with provided options modes (RSN3c)' do + channel = subject.get(channel_name, channel_options) + expect(channel.options.modes).to eq(modes) subject.get(channel_name, channel_options) expect(channel.options.modes).to eq(modes) end end + + let(:modes) { %i[subscribe] } + let(:new_options) { { modes: modes } } + + describe 'hash' do + let(:channel_options) { new_options } + it { expect(channel_options).to be_a(Hash) } + + include_examples 'update channel with provided options :modes' + end + + describe 'ChannelOptions object' do + let(:channel_options) { Ably::Models::ChannelOptions.new(new_options) } + it { expect(channel_options).to be_a(Ably::Models::ChannelOptions) } + + include_examples 'update channel with provided options :modes' + end end end end From 070b48de0660ebe790986311f5ba89e0c9139628 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C5=81ukasz=20=C5=9Aliwa?= Date: Mon, 28 Feb 2022 19:12:54 +0100 Subject: [PATCH 14/16] Removed support/channel_options_helper require --- spec/spec_helper.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index e9fc87161..e96e9cdf9 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -27,7 +27,6 @@ def console(message) require 'support/event_emitter_helper' require 'support/private_api_formatter' require 'support/protocol_helper' -require 'support/channel_options_helper' require 'support/random_helper' require 'support/serialization_helper' require 'support/test_logger_helper' From 13c3894c253ed6a319bfdb4d270aea3dcc824241 Mon Sep 17 00:00:00 2001 From: TheSmartnik Date: Mon, 7 Mar 2022 20:02:42 +0400 Subject: [PATCH 15/16] Add extra spec for RTS3c1 --- spec/acceptance/realtime/channels_spec.rb | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/spec/acceptance/realtime/channels_spec.rb b/spec/acceptance/realtime/channels_spec.rb index 1419014fe..2b6656159 100644 --- a/spec/acceptance/realtime/channels_spec.rb +++ b/spec/acceptance/realtime/channels_spec.rb @@ -38,6 +38,24 @@ channel.attach end + + context 'params keys are the same but values are different' do + let(:options) do + { params: { x: '1' } } + end + + it 'will raise an error' do + channel = client.channels.get(channel_name, options) + + channel.on(:attached) do + expect { client.channels.get(channel_name, { params: { x: '2' } }) }.to raise_error ArgumentError, /use Channel#set_options directly/ + + stop_reactor + end + + channel.attach + end + end end describe 'using shortcut method #channel on the client object' do From a41b6eddc918886749d722d8a058ec120e9c81e4 Mon Sep 17 00:00:00 2001 From: TheSmartnik Date: Sun, 13 Mar 2022 22:39:34 +0400 Subject: [PATCH 16/16] Add warning on implicit channel set options --- lib/ably/modules/channels_collection.rb | 21 +++++++++++++++++-- lib/ably/realtime/channel.rb | 25 +++-------------------- spec/acceptance/realtime/channels_spec.rb | 22 +++++++++++++++++--- spec/unit/realtime/channels_spec.rb | 4 +++- spec/unit/rest/channels_spec.rb | 2 +- 5 files changed, 45 insertions(+), 29 deletions(-) diff --git a/lib/ably/modules/channels_collection.rb b/lib/ably/modules/channels_collection.rb index cc26ec047..aa9c14214 100644 --- a/lib/ably/modules/channels_collection.rb +++ b/lib/ably/modules/channels_collection.rb @@ -21,8 +21,12 @@ def get(name, channel_options = {}) if channels.has_key?(name) channels[name].tap do |channel| if channel_options && !channel_options.empty? - channel.raise_on_reattach! if channel.respond_to?(:raise_on_reattach!) - channel.options = channel_options + if channel.respond_to?(:need_reattach?) && channel.need_reattach? + raise_implicit_options_update + else + warn_implicit_options_update + channel.options = channel_options + end end end else @@ -73,6 +77,19 @@ def each(&block) end private + + def raise_implicit_options_update + raise ArgumentError, "You are trying to indirectly update channel options which will trigger reattachment of the channel. Please use Channel#set_options directly if you wish to continue" + end + + def warn_implicit_options_update + logger.warn { "Channels#get: Using this method to update channel options is deprecated and may be removed in a future version of ably-ruby. Please use Channel#setOptions instead" } + end + + def logger + client.logger + end + def client @client end diff --git a/lib/ably/realtime/channel.rb b/lib/ably/realtime/channel.rb index fae389cbf..2b16276c2 100644 --- a/lib/ably/realtime/channel.rb +++ b/lib/ably/realtime/channel.rb @@ -93,11 +93,6 @@ class Channel # @api private attr_reader :manager - # Flag that controlls reattaching behavior on options update per RTS3c1 - # @return [Bolean] - # @api private - attr_reader :raise_on_reattach - # ChannelOptions params attrribute (#RTL4k) # return [Hash] def_delegators :options, :params @@ -121,7 +116,6 @@ def initialize(client, name, channel_options = {}) @manager = ChannelManager.new(self, client.connection) @push = PushChannel.new(self) @properties = ChannelProperties.new(self) - @raise_on_reattach = false setup_event_handlers setup_presence @@ -325,11 +319,7 @@ def __incoming_msgbus__ def set_options(channel_options) @options = Ably::Models::ChannelOptions(channel_options) - if need_reattach? - raise_reattachment_error if raise_on_reattach - - manager.request_reattach - end + manager.request_reattach if need_reattach? end alias options= set_options @@ -349,25 +339,16 @@ def logger client.logger end - # @api private - def raise_on_reattach! - @raise_on_reattach = true - end - # As we are using a state machine, do not allow change_state to be used # #transition_state_machine must be used instead private :change_state - private - - def raise_reattachment_error - raise ArgumentError, "You are trying to indirectly update channel options which will trigger reattachment of the channel. Please, use Channel#set_options directly if you wish to continue" - end - def need_reattach? !!(attaching? || attached?) && !!(options.modes || options.params) end + private + def setup_event_handlers __incoming_msgbus__.subscribe(:message) do |message| message.decode(client.encoders, options) do |encode_error, error_message| diff --git a/spec/acceptance/realtime/channels_spec.rb b/spec/acceptance/realtime/channels_spec.rb index 2b6656159..9d385da7c 100644 --- a/spec/acceptance/realtime/channels_spec.rb +++ b/spec/acceptance/realtime/channels_spec.rb @@ -17,11 +17,14 @@ end vary_by_protocol do + let(:client_options) do + { key: api_key, environment: environment, protocol: protocol } + end let(:client) do - auto_close Ably::Realtime::Client.new(key: api_key, environment: environment, protocol: protocol) + auto_close Ably::Realtime::Client.new(client_options) end let(:channel_name) { random_str } - let(:options) do + let(:options) do { params: { key: 'value' } } end @@ -71,8 +74,10 @@ end describe 'accessing an existing channel object with different options' do + let(:client_options) { super().merge(logger: custom_logger_object) } + let(:custom_logger_object) { TestLogger.new } let(:new_channel_options) { { encrypted: true } } - let(:original_channel) { client.channels.get(channel_name, options) } + let!(:original_channel) { client.channels.get(channel_name, options) } it 'overrides the existing channel options and returns the channel object' do expect(original_channel.options.to_h).to_not include(:encrypted) @@ -81,6 +86,17 @@ expect(new_channel.options[:encrypted]).to eql(true) stop_reactor end + + it 'shows deprecation warning' do + client.channels.get(channel_name, new_channel_options) + + warning = custom_logger_object.logs.find do |severity, message| + message.match(/Using this method to update channel options is deprecated and may be removed/) + end + + expect(warning).to_not be_nil + stop_reactor + end end describe 'accessing an existing channel object without specifying any channel options' do diff --git a/spec/unit/realtime/channels_spec.rb b/spec/unit/realtime/channels_spec.rb index 43e1b67cf..9fbc70b6f 100644 --- a/spec/unit/realtime/channels_spec.rb +++ b/spec/unit/realtime/channels_spec.rb @@ -3,7 +3,9 @@ describe Ably::Realtime::Channels do let(:connection) { instance_double('Ably::Realtime::Connection', unsafe_on: true, on_resume: true) } - let(:client) { instance_double('Ably::Realtime::Client', connection: connection, client_id: 'clientId') } + let(:client) do + instance_double('Ably::Realtime::Client', connection: connection, client_id: 'clientId', logger: double('logger').as_null_object) + end let(:channel_name) { 'unique' } let(:options) do { params: { bizarre: 'value' } } diff --git a/spec/unit/rest/channels_spec.rb b/spec/unit/rest/channels_spec.rb index da50c5c4c..5b4eba0dd 100644 --- a/spec/unit/rest/channels_spec.rb +++ b/spec/unit/rest/channels_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Ably::Rest::Channels do - let(:client) { instance_double('Ably::Rest::Client') } + let(:client) { instance_double('Ably::Rest::Client', logger: double('logger').as_null_object) } let(:channel_name) { 'unique'.encode(Encoding::UTF_8) } let(:options) do { params: { 'bizarre' => 'value' } }