Improve channel actions dispatcher to allow inheritance/mixins

Fixes #14

2 files changed, 75 insertions, 4 deletions

diff --git a/lib/action_cable/channel/base.rb b/lib/action_cable/channel/base.rb
index 87ae3a1211..d6bd98d180 100644
--- a/lib/action_cable/channel/base.rb
+++ b/lib/action_cable/channel/base.rb
@@ -75,6 +75,42 @@ module ActionCable
       attr_reader :params, :connection
       delegate :logger, to: :connection
 
+      class << self
+        # A list of method names that should be considered actions. This
+        # includes all public instance methods on a channel, less
+        # any internal methods (defined on Base), adding back in
+        # any methods that are internal, but still exist on the class
+        # itself.
+        #
+        # ==== Returns
+        # * <tt>Set</tt> - A set of all methods that should be considered actions.
+        def action_methods
+          @action_methods ||= begin
+            # All public instance methods of this class, including ancestors
+            methods = (public_instance_methods(true) -
+              # Except for public instance methods of Base and its ancestors
+              ActionCable::Channel::Base.public_instance_methods(true) +
+              # Be sure to include shadowed public instance methods of this class
+              public_instance_methods(false)).uniq.map(&:to_s)
+            methods.to_set
+          end
+        end
+
+        protected
+          # action_methods are cached and there is sometimes need to refresh
+          # them. ::clear_action_methods! allows you to do that, so next time
+          # you run action_methods, they will be recalculated
+          def clear_action_methods!
+            @action_methods = nil
+          end
+
+          # Refresh the cached action_methods when a new action_method is added.
+          def method_added(name)
+            super
+            clear_action_methods!
+          end
+      end
+
       def initialize(connection, identifier, params = {})
         @connection = connection
         @identifier = identifier
@@ -147,7 +183,7 @@ module ActionCable
         end
 
         def processable_action?(action)
-          self.class.instance_methods(false).include?(action)
+          self.class.action_methods.include?(action.to_s)
         end
 
         def dispatch_action(action, data)
@@ -168,7 +204,6 @@ module ActionCable
           end
         end
 
-
         def run_subscribe_callbacks
           self.class.on_subscribe_callbacks.each { |callback| send(callback) }
         end
diff --git a/test/channel/base_test.rb b/test/channel/base_test.rb
index aa31d23297..e7944ff06b 100644
--- a/test/channel/base_test.rb
+++ b/test/channel/base_test.rb
@@ -3,7 +3,22 @@ require 'stubs/test_connection'
 require 'stubs/room'
 
 class ActionCable::Channel::BaseTest < ActiveSupport::TestCase
-  class ChatChannel < ActionCable::Channel::Base
+  class ActionCable::Channel::Base
+    def kick
+      @last_action = [ :kick ]
+    end
+
+    def topic
+    end
+  end
+
+  class BasicChannel < ActionCable::Channel::Base
+    def chatters
+      @last_action = [ :chatters ]
+    end
+  end
+
+  class ChatChannel < BasicChannel
     attr_reader :room, :last_action
     on_subscribe :toggle_subscribed
     on_unsubscribe :toggle_subscribed
@@ -29,6 +44,10 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase
       @last_action = [ :speak, data ]
     end
 
+    def topic(data)
+      @last_action = [ :topic, data ]
+    end
+
     def subscribed?
       @subscribed
     end
@@ -87,11 +106,28 @@ class ActionCable::Channel::BaseTest < ActiveSupport::TestCase
     assert_equal [ :speak, data ], @channel.last_action
   end
 
-  test "try calling a private method" do
+  test "should not dispatch a private method" do
     @channel.perform_action 'action' => :rm_rf
     assert_nil @channel.last_action
   end
 
+  test "should not dispatch a public method defined on Base" do
+    @channel.perform_action 'action' => :kick
+    assert_nil @channel.last_action
+  end
+
+  test "should dispatch a public method defined on Base and redefined on channel" do
+    data = { 'action' => :topic, 'content' => "This is Sparta!" }
+
+    @channel.perform_action data
+    assert_equal [ :topic, data ], @channel.last_action
+  end
+
+  test "should dispatch calling a public method defined in an ancestor" do
+    @channel.perform_action 'action' => :chatters
+    assert_equal [ :chatters ], @channel.last_action
+  end
+
   test "transmitting data" do
     @channel.perform_action 'action' => :get_latest