From 661298176c72eefc6d0f42b58210119e76962364 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Fri, 5 Feb 2016 22:48:48 +0100 Subject: Inject Rails' channel paths in engine. We were explicitly referencing Rails.root in ActionCable::Server::Configuration.initialize, thereby coupling ourselves to Rails. Instead add `app/channels` to Rails' app paths and assign the existent files to `channel_paths`. Users can still append to those load paths with `<<` and `push` in `config/application.rb`. This means we can remove the custom `Dir` lookup in `channel_paths` and the Rails and root definitions in the tests. --- actioncable/lib/action_cable/engine.rb | 2 ++ actioncable/lib/action_cable/server/configuration.rb | 11 ++--------- actioncable/test/client_test.rb | 10 +--------- actioncable/test/subscription_adapter/common.rb | 8 -------- 4 files changed, 5 insertions(+), 26 deletions(-) (limited to 'actioncable') diff --git a/actioncable/lib/action_cable/engine.rb b/actioncable/lib/action_cable/engine.rb index f5e233e091..acca3961d8 100644 --- a/actioncable/lib/action_cable/engine.rb +++ b/actioncable/lib/action_cable/engine.rb @@ -31,6 +31,8 @@ module ActionCable self.cable = Rails.application.config_for(config_path).with_indifferent_access end + self.channel_paths = Rails.application.paths['app/channels'].existent + options.each { |k,v| send("#{k}=", v) } end end diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb index 9a248933c4..019a374734 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -5,27 +5,20 @@ module ActionCable class Configuration attr_accessor :logger, :log_tags attr_accessor :connection_class, :worker_pool_size - attr_accessor :channel_load_paths attr_accessor :disable_request_forgery_protection, :allowed_request_origins attr_accessor :cable, :url + attr_accessor :channel_paths # :nodoc: + def initialize @log_tags = [] @connection_class = ApplicationCable::Connection @worker_pool_size = 100 - @channel_load_paths = [Rails.root.join('app/channels')] - @disable_request_forgery_protection = false end - def channel_paths - @channel_paths ||= channel_load_paths.flat_map do |path| - Dir["#{path}/**/*_channel.rb"] - end - end - def channel_class_names @channel_class_names ||= channel_paths.collect do |channel_path| Pathname.new(channel_path).basename.to_s.split('.').first.camelize diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index 4ade9832e0..f2b090cc52 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -16,12 +16,8 @@ class ClientTest < ActionCable::TestCase ::Object.const_set(:ApplicationCable, Module.new) ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) - ::Object.const_set(:Rails, Module.new) - ::Rails.singleton_class.send(:define_method, :root) { Pathname.new(__dir__) } - ActionCable.instance_variable_set(:@server, nil) server = ActionCable.server - server.config = ActionCable::Server::Configuration.new inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) @@ -29,7 +25,7 @@ class ClientTest < ActionCable::TestCase # and now the "real" setup for our test: server.config.disable_request_forgery_protection = true - server.config.channel_load_paths = [File.expand_path('client', __dir__)] + server.config.channel_paths = [ File.expand_path('client/echo_channel.rb', __dir__) ] Thread.new { EventMachine.run } unless EventMachine.reactor_running? Thread.pass until EventMachine.reactor_running? @@ -45,10 +41,6 @@ class ClientTest < ActionCable::TestCase ::Object.send(:remove_const, :ApplicationCable) rescue NameError end - begin - ::Object.send(:remove_const, :Rails) - rescue NameError - end end def with_puma_server(rack_app = ActionCable.server, port = 3099) diff --git a/actioncable/test/subscription_adapter/common.rb b/actioncable/test/subscription_adapter/common.rb index 361858784e..1b6f8c70bf 100644 --- a/actioncable/test/subscription_adapter/common.rb +++ b/actioncable/test/subscription_adapter/common.rb @@ -13,11 +13,7 @@ module CommonSubscriptionAdapterTest ::Object.const_set(:ApplicationCable, Module.new) ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) - ::Object.const_set(:Rails, Module.new) - ::Rails.singleton_class.send(:define_method, :root) { Pathname.new(__dir__) } - server = ActionCable::Server::Base.new - server.config = ActionCable::Server::Configuration.new inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) @@ -39,10 +35,6 @@ module CommonSubscriptionAdapterTest ::Object.send(:remove_const, :ApplicationCable) rescue NameError end - begin - ::Object.send(:remove_const, :Rails) - rescue NameError - end end -- cgit v1.2.3 From 861ece99bb6d7ebdb27fad7db5ccc05340aaf0e5 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Fri, 5 Feb 2016 22:56:52 +0100 Subject: Default connection class to ActionCable::Connection::Base. Instead of depending on ApplicationCable::Connection being defined at initialize we should inject it in the Railtie. Thus we can kill more setup in the tests too. --- actioncable/lib/action_cable/engine.rb | 4 ++++ actioncable/lib/action_cable/server/configuration.rb | 4 ++-- actioncable/test/client_test.rb | 8 -------- actioncable/test/subscription_adapter/common.rb | 8 -------- 4 files changed, 6 insertions(+), 18 deletions(-) (limited to 'actioncable') diff --git a/actioncable/lib/action_cable/engine.rb b/actioncable/lib/action_cable/engine.rb index acca3961d8..f5f1cb59e0 100644 --- a/actioncable/lib/action_cable/engine.rb +++ b/actioncable/lib/action_cable/engine.rb @@ -31,6 +31,10 @@ module ActionCable self.cable = Rails.application.config_for(config_path).with_indifferent_access end + if 'ApplicationCable::Connection'.safe_constantize + self.connection_class = ApplicationCable::Connection + end + self.channel_paths = Rails.application.paths['app/channels'].existent options.each { |k,v| send("#{k}=", v) } diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb index 019a374734..58bb8ff65a 100644 --- a/actioncable/lib/action_cable/server/configuration.rb +++ b/actioncable/lib/action_cable/server/configuration.rb @@ -13,8 +13,8 @@ module ActionCable def initialize @log_tags = [] - @connection_class = ApplicationCable::Connection - @worker_pool_size = 100 + @connection_class = ActionCable::Connection::Base + @worker_pool_size = 100 @disable_request_forgery_protection = false end diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index f2b090cc52..d294fa7bc6 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -13,9 +13,6 @@ class ClientTest < ActionCable::TestCase def setup # TODO: ActionCable requires a *lot* of setup at the moment... - ::Object.const_set(:ApplicationCable, Module.new) - ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) - ActionCable.instance_variable_set(:@server, nil) server = ActionCable.server inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } @@ -36,11 +33,6 @@ class ClientTest < ActionCable::TestCase def teardown $VERBOSE = @previous_verbose - - begin - ::Object.send(:remove_const, :ApplicationCable) - rescue NameError - end end def with_puma_server(rack_app = ActionCable.server, port = 3099) diff --git a/actioncable/test/subscription_adapter/common.rb b/actioncable/test/subscription_adapter/common.rb index 1b6f8c70bf..b06e3cb761 100644 --- a/actioncable/test/subscription_adapter/common.rb +++ b/actioncable/test/subscription_adapter/common.rb @@ -10,9 +10,6 @@ module CommonSubscriptionAdapterTest def setup # TODO: ActionCable requires a *lot* of setup at the moment... - ::Object.const_set(:ApplicationCable, Module.new) - ::ApplicationCable.const_set(:Connection, Class.new(ActionCable::Connection::Base)) - server = ActionCable::Server::Base.new inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) @@ -30,11 +27,6 @@ module CommonSubscriptionAdapterTest def teardown @tx_adapter.shutdown if @tx_adapter && @tx_adapter != @rx_adapter @rx_adapter.shutdown if @rx_adapter - - begin - ::Object.send(:remove_const, :ApplicationCable) - rescue NameError - end end -- cgit v1.2.3 From 3ae8eb13cb4967f533339ecddf93e19f06caa6d2 Mon Sep 17 00:00:00 2001 From: Kasper Timm Hansen Date: Sat, 6 Feb 2016 20:40:42 +0100 Subject: Don't rely on the global server as a receiver. The `WorkerTest`'s `Receiver` is imporsonating an `ActionCable::Connection::Base`, but just delegates the logger to `ActionCable.logger`. This creates a mismatch as the connection requires the logger to be a `TaggedLoggerProxy`'ied logger, while the server doesn't. Thus to ensure an exception isn't raised when the worker tries to call `tag` other tests have to assign a proxied logger to their test server. Instead of forcing change on other tests, have Receiver adhere to the connection contract and use a `TaggedLoggerProxy`. As a consequence remove more setup from the tests. --- actioncable/test/client_test.rb | 4 +--- actioncable/test/subscription_adapter/common.rb | 6 ------ actioncable/test/worker_test.rb | 4 +++- 3 files changed, 4 insertions(+), 10 deletions(-) (limited to 'actioncable') diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index d294fa7bc6..d30c381131 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -12,11 +12,9 @@ class ClientTest < ActionCable::TestCase WAIT_WHEN_NOT_EXPECTING_EVENT = 0.2 def setup - # TODO: ActionCable requires a *lot* of setup at the moment... ActionCable.instance_variable_set(:@server, nil) server = ActionCable.server - inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } - server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) + server.config.logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } server.config.cable = { adapter: 'async' }.with_indifferent_access diff --git a/actioncable/test/subscription_adapter/common.rb b/actioncable/test/subscription_adapter/common.rb index b06e3cb761..b31c2aa36c 100644 --- a/actioncable/test/subscription_adapter/common.rb +++ b/actioncable/test/subscription_adapter/common.rb @@ -9,13 +9,7 @@ module CommonSubscriptionAdapterTest WAIT_WHEN_NOT_EXPECTING_EVENT = 0.2 def setup - # TODO: ActionCable requires a *lot* of setup at the moment... server = ActionCable::Server::Base.new - inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } - server.config.logger = ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) - - - # and now the "real" setup for our test: server.config.cable = cable_config.with_indifferent_access adapter_klass = server.config.pubsub_adapter diff --git a/actioncable/test/worker_test.rb b/actioncable/test/worker_test.rb index 4a699cde27..654f49821e 100644 --- a/actioncable/test/worker_test.rb +++ b/actioncable/test/worker_test.rb @@ -17,7 +17,9 @@ class WorkerTest < ActiveSupport::TestCase end def logger - ActionCable.server.logger + # Impersonating a connection requires a TaggedLoggerProxy'ied logger. + inner_logger = Logger.new(StringIO.new).tap { |l| l.level = Logger::UNKNOWN } + ActionCable::Connection::TaggedLoggerProxy.new(inner_logger, tags: []) end end -- cgit v1.2.3