Merge pull request #25240 from matthewd/reloadable-channels

Properly support reloading for Action Cable channels

10 files changed, 56 insertions, 84 deletions

diff --git a/actioncable/lib/action_cable/connection/subscriptions.rb b/actioncable/lib/action_cable/connection/subscriptions.rb
index 3742f248d1..6051818bfb 100644
--- a/actioncable/lib/action_cable/connection/subscriptions.rb
+++ b/actioncable/lib/action_cable/connection/subscriptions.rb
@@ -26,12 +26,12 @@ module ActionCable
         id_key = data['identifier']
         id_options = ActiveSupport::JSON.decode(id_key).with_indifferent_access
 
-        subscription_klass = connection.server.channel_classes[id_options[:channel]]
+        subscription_klass = id_options[:channel].safe_constantize
 
-        if subscription_klass
+        if subscription_klass && ActionCable::Channel::Base >= subscription_klass
           subscriptions[id_key] ||= subscription_klass.new(connection, id_key, id_options)
         else
-          logger.error "Subscription class not found (#{data.inspect})"
+          logger.error "Subscription class not found: #{id_options[:channel].inspect}"
         end
       end
 
diff --git a/actioncable/lib/action_cable/engine.rb b/actioncable/lib/action_cable/engine.rb
index 8ce1b24962..96176e0014 100644
--- a/actioncable/lib/action_cable/engine.rb
+++ b/actioncable/lib/action_cable/engine.rb
@@ -31,11 +31,8 @@ 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
+        previous_connection_class = self.connection_class
+        self.connection_class = -> { 'ApplicationCable::Connection'.safe_constantize || previous_connection_class.call }
 
         options.each { |k,v| send("#{k}=", v) }
       end
diff --git a/actioncable/lib/action_cable/server/base.rb b/actioncable/lib/action_cable/server/base.rb
index 717a60fe4f..0ad1e408a9 100644
--- a/actioncable/lib/action_cable/server/base.rb
+++ b/actioncable/lib/action_cable/server/base.rb
@@ -19,13 +19,13 @@ module ActionCable
 
       def initialize
         @mutex = Monitor.new
-        @remote_connections = @event_loop = @worker_pool = @channel_classes = @pubsub = nil
+        @remote_connections = @event_loop = @worker_pool = @pubsub = nil
       end
 
       # Called by Rack to setup the server.
       def call(env)
         setup_heartbeat_timer
-        config.connection_class.new(self, env).process
+        config.connection_class.call.new(self, env).process
       end
 
       # Disconnect all the connections identified by `identifiers` on this server or any others via RemoteConnections.
@@ -67,16 +67,6 @@ module ActionCable
         @worker_pool || @mutex.synchronize { @worker_pool ||= ActionCable::Server::Worker.new(max_size: config.worker_pool_size) }
       end
 
-      # Requires and returns a hash of all of the channel class constants, which are keyed by name.
-      def channel_classes
-        @channel_classes || @mutex.synchronize do
-          @channel_classes ||= begin
-            config.channel_paths.each { |channel_path| require channel_path }
-            config.channel_class_names.each_with_object({}) { |name, hash| hash[name] = name.constantize }
-          end
-        end
-      end
-
       # Adapter used for all streams/broadcasting.
       def pubsub
         @pubsub || @mutex.synchronize { @pubsub ||= config.pubsub_adapter.new(self) }
@@ -84,7 +74,7 @@ module ActionCable
 
       # All of the identifiers applied to the connection class associated with this server.
       def connection_identifiers
-        config.connection_class.identifiers
+        config.connection_class.call.identifiers
       end
     end
 
diff --git a/actioncable/lib/action_cable/server/configuration.rb b/actioncable/lib/action_cable/server/configuration.rb
index 0bb378cf03..ada1ac22cc 100644
--- a/actioncable/lib/action_cable/server/configuration.rb
+++ b/actioncable/lib/action_cable/server/configuration.rb
@@ -8,23 +8,15 @@ module ActionCable
       attr_accessor :disable_request_forgery_protection, :allowed_request_origins
       attr_accessor :cable, :url, :mount_path
 
-      attr_accessor :channel_paths # :nodoc:
-
       def initialize
         @log_tags = []
 
-        @connection_class = ActionCable::Connection::Base
+        @connection_class = -> { ActionCable::Connection::Base }
         @worker_pool_size = 4
 
         @disable_request_forgery_protection = false
       end
 
-      def channel_class_names
-        @channel_class_names ||= channel_paths.collect do |channel_path|
-          Pathname.new(channel_path).basename.to_s.split('.').first.camelize
-        end
-      end
-
       # Returns constant of subscription adapter specified in config/cable.yml.
       # If the adapter cannot be found, this will default to the Redis adapter.
       # Also makes sure proper dependencies are required.
diff --git a/actioncable/test/channel/stream_test.rb b/actioncable/test/channel/stream_test.rb
index 0b0c72ccf6..38543920d3 100644
--- a/actioncable/test/channel/stream_test.rb
+++ b/actioncable/test/channel/stream_test.rb
@@ -128,10 +128,6 @@ module ActionCable::StreamTests
     setup do
       @server = TestServer.new(subscription_adapter: ActionCable::SubscriptionAdapter::Inline)
       @server.config.allowed_request_origins = %w( http://rubyonrails.com )
-      @server.stubs(:channel_classes).returns(
-        ChatChannel.name => ChatChannel,
-        UserCallbackChannel.name => UserCallbackChannel,
-      )
     end
 
     test 'custom encoder' do
diff --git a/actioncable/test/client/echo_channel.rb b/actioncable/test/client/echo_channel.rb
deleted file mode 100644
index 5a7bac25c5..0000000000
--- a/actioncable/test/client/echo_channel.rb
+++ /dev/null
@@ -1,22 +0,0 @@
-class EchoChannel < ActionCable::Channel::Base
-  def subscribed
-    stream_from "global"
-  end
-
-  def unsubscribed
-    'Goodbye from EchoChannel!'
-  end
-
-  def ding(data)
-    transmit(dong: data['message'])
-  end
-
-  def delay(data)
-    sleep 1
-    transmit(dong: data['message'])
-  end
-
-  def bulk(data)
-    ActionCable.server.broadcast "global", wide: data['message']
-  end
-end
diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb
index fe503fd703..f4b4a53aea 100644
--- a/actioncable/test/client_test.rb
+++ b/actioncable/test/client_test.rb
@@ -11,6 +11,29 @@ class ClientTest < ActionCable::TestCase
   WAIT_WHEN_EXPECTING_EVENT = 8
   WAIT_WHEN_NOT_EXPECTING_EVENT = 0.5
 
+  class EchoChannel < ActionCable::Channel::Base
+    def subscribed
+      stream_from "global"
+    end
+
+    def unsubscribed
+      'Goodbye from EchoChannel!'
+    end
+
+    def ding(data)
+      transmit(dong: data['message'])
+    end
+
+    def delay(data)
+      sleep 1
+      transmit(dong: data['message'])
+    end
+
+    def bulk(data)
+      ActionCable.server.broadcast "global", wide: data['message']
+    end
+  end
+
   def setup
     ActionCable.instance_variable_set(:@server, nil)
     server = ActionCable.server
@@ -21,7 +44,6 @@ class ClientTest < ActionCable::TestCase
 
     # and now the "real" setup for our test:
     server.config.disable_request_forgery_protection = true
-    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?
@@ -148,10 +170,10 @@ class ClientTest < ActionCable::TestCase
     with_puma_server do |port|
       c = faye_client(port)
       assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
-      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'EchoChannel')
-      assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
-      c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
-      assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "message"=>{"dong"=>"hello"}}, c.read_message)
+      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'ClientTest::EchoChannel')
+      assert_equal({"identifier"=>"{\"channel\":\"ClientTest::EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
+      c.send_message command: 'message', identifier: JSON.generate(channel: 'ClientTest::EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
+      assert_equal({"identifier"=>"{\"channel\":\"ClientTest::EchoChannel\"}", "message"=>{"dong"=>"hello"}}, c.read_message)
       c.close
     end
   end
@@ -165,12 +187,12 @@ class ClientTest < ActionCable::TestCase
 
       clients.map {|c| Concurrent::Future.execute {
         assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
-        c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'EchoChannel')
-        assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
-        c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
-        assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message)
+        c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'ClientTest::EchoChannel')
+        assert_equal({"identifier"=>'{"channel":"ClientTest::EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
+        c.send_message command: 'message', identifier: JSON.generate(channel: 'ClientTest::EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
+        assert_equal({"identifier"=>'{"channel":"ClientTest::EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message)
         barrier_1.wait WAIT_WHEN_EXPECTING_EVENT
-        c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'bulk', message: 'hello')
+        c.send_message command: 'message', identifier: JSON.generate(channel: 'ClientTest::EchoChannel'), data: JSON.generate(action: 'bulk', message: 'hello')
         barrier_2.wait WAIT_WHEN_EXPECTING_EVENT
         assert_equal clients.size, c.read_messages(clients.size).size
       } }.each(&:wait!)
@@ -185,10 +207,10 @@ class ClientTest < ActionCable::TestCase
 
       clients.map {|c| Concurrent::Future.execute {
         assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
-        c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'EchoChannel')
-        assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
-        c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
-        assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message)
+        c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'ClientTest::EchoChannel')
+        assert_equal({"identifier"=>'{"channel":"ClientTest::EchoChannel"}', "type"=>"confirm_subscription"}, c.read_message)
+        c.send_message command: 'message', identifier: JSON.generate(channel: 'ClientTest::EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
+        assert_equal({"identifier"=>'{"channel":"ClientTest::EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message)
       } }.each(&:wait!)
 
       clients.map {|c| Concurrent::Future.execute { c.close } }.each(&:wait!)
@@ -199,17 +221,17 @@ class ClientTest < ActionCable::TestCase
     with_puma_server do |port|
       c = faye_client(port)
       assert_equal({"type" => "welcome"}, c.read_message)  # pop the first welcome message off the stack
-      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'EchoChannel')
-      assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
-      c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'delay', message: 'hello')
+      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'ClientTest::EchoChannel')
+      assert_equal({"identifier"=>"{\"channel\":\"ClientTest::EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
+      c.send_message command: 'message', identifier: JSON.generate(channel: 'ClientTest::EchoChannel'), data: JSON.generate(action: 'delay', message: 'hello')
       c.close # disappear before write
 
       c = faye_client(port)
       assert_equal({"type" => "welcome"}, c.read_message) # pop the first welcome message off the stack
-      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'EchoChannel')
-      assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
-      c.send_message command: 'message', identifier: JSON.generate(channel: 'EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
-      assert_equal({"identifier"=>'{"channel":"EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message)
+      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'ClientTest::EchoChannel')
+      assert_equal({"identifier"=>"{\"channel\":\"ClientTest::EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
+      c.send_message command: 'message', identifier: JSON.generate(channel: 'ClientTest::EchoChannel'), data: JSON.generate(action: 'ding', message: 'hello')
+      assert_equal({"identifier"=>'{"channel":"ClientTest::EchoChannel"}', "message"=>{"dong"=>"hello"}}, c.read_message)
       c.close # disappear before read
     end
   end
@@ -217,12 +239,12 @@ class ClientTest < ActionCable::TestCase
   def test_unsubscribe_client
     with_puma_server do |port|
       app = ActionCable.server
-      identifier = JSON.generate(channel: 'EchoChannel')
+      identifier = JSON.generate(channel: 'ClientTest::EchoChannel')
 
       c = faye_client(port)
       assert_equal({"type" => "welcome"}, c.read_message)
       c.send_message command: 'subscribe', identifier: identifier
-      assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
+      assert_equal({"identifier"=>"{\"channel\":\"ClientTest::EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
       assert_equal(1, app.connections.count)
       assert(app.remote_connections.where(identifier: identifier))
 
@@ -242,8 +264,8 @@ class ClientTest < ActionCable::TestCase
     with_puma_server do |port|
       c = faye_client(port)
       assert_equal({"type" => "welcome"}, c.read_message)
-      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'EchoChannel')
-      assert_equal({"identifier"=>"{\"channel\":\"EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
+      c.send_message command: 'subscribe', identifier: JSON.generate(channel: 'ClientTest::EchoChannel')
+      assert_equal({"identifier"=>"{\"channel\":\"ClientTest::EchoChannel\"}", "type"=>"confirm_subscription"}, c.read_message)
 
       ActionCable.server.restart
       c.wait_for_close
diff --git a/actioncable/test/connection/subscriptions_test.rb b/actioncable/test/connection/subscriptions_test.rb
index 53e8547245..a5b1e5dcf3 100644
--- a/actioncable/test/connection/subscriptions_test.rb
+++ b/actioncable/test/connection/subscriptions_test.rb
@@ -24,7 +24,6 @@ class ActionCable::Connection::SubscriptionsTest < ActionCable::TestCase
 
   setup do
     @server = TestServer.new
-    @server.stubs(:channel_classes).returns(ChatChannel.name => ChatChannel)
 
     @chat_identifier = ActiveSupport::JSON.encode(id: 1, channel: 'ActionCable::Connection::SubscriptionsTest::ChatChannel')
   end
diff --git a/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/channel.rb b/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/channel.rb
index d56fa30f4d..d672697283 100644
--- a/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/channel.rb
+++ b/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/channel.rb
@@ -1,4 +1,3 @@
-# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading.
 module ApplicationCable
   class Channel < ActionCable::Channel::Base
   end
diff --git a/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/connection.rb b/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/connection.rb
index b4f41389ad..0ff5442f47 100644
--- a/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/connection.rb
+++ b/railties/lib/rails/generators/rails/app/templates/app/channels/application_cable/connection.rb
@@ -1,4 +1,3 @@
-# Be sure to restart your server when you modify this file. Action Cable runs in a loop that does not support auto reloading.
 module ApplicationCable
   class Connection < ActionCable::Connection::Base
   end