Merge branch 'master' into subscription-rejection

4 files changed, 39 insertions, 38 deletions

diff --git a/lib/action_cable/channel/base.rb b/lib/action_cable/channel/base.rb
index 31b8dece4a..67b312e5ea 100644
--- a/lib/action_cable/channel/base.rb
+++ b/lib/action_cable/channel/base.rb
@@ -76,10 +76,7 @@ module ActionCable
       SUBSCRIPTION_CONFIRMATION_INTERNAL_MESSAGE = 'confirm_subscription'.freeze
       SUBSCRIPTION_REJECTION_INTERNAL_MESSAGE = 'reject_subscription'.freeze
 
-      on_subscribe   :subscribed
-      on_unsubscribe :unsubscribed
-
-      attr_reader :params, :connection, :identifier
+      attr_reader :params, :connection, ::identifier
       delegate :logger, to: :connection
 
       class << self
@@ -147,7 +144,9 @@ module ActionCable
       # Called by the cable connection when its cut so the channel has a chance to cleanup with callbacks.
       # This method is not intended to be called directly by the user. Instead, overwrite the #unsubscribed callback.
       def unsubscribe_from_channel
-        run_unsubscribe_callbacks
+        run_callbacks :unsubscribe do
+          unsubscribed
+        end
       end
 
 
@@ -202,7 +201,9 @@ module ActionCable
 
 
         def subscribe_to_channel
-          run_subscribe_callbacks
+          run_callbacks :subscribe do
+            subscribed
+          end
 
           if subscription_rejected?
             reject_subscription
@@ -238,19 +239,10 @@ module ActionCable
           end
         end
 
-        def run_subscribe_callbacks
-          self.class.on_subscribe_callbacks.each { |callback| send(callback) }
-        end
-
-        def run_unsubscribe_callbacks
-          self.class.on_unsubscribe_callbacks.each { |callback| send(callback) }
-        end
-
         def transmit_subscription_confirmation
           unless subscription_confirmation_sent?
             logger.info "#{self.class.name} is transmitting the subscription confirmation"
             connection.transmit ActiveSupport::JSON.encode(identifier: @identifier, type: SUBSCRIPTION_CONFIRMATION_INTERNAL_MESSAGE)
-
             @subscription_confirmation_sent = true
           end
         end
@@ -264,7 +256,6 @@ module ActionCable
           logger.info "#{self.class.name} is transmitting the subscription rejection"
           connection.transmit ActiveSupport::JSON.encode(identifier: @identifier, type: SUBSCRIPTION_REJECTION_INTERNAL_MESSAGE)
         end
-
     end
   end
 end
diff --git a/lib/action_cable/channel/callbacks.rb b/lib/action_cable/channel/callbacks.rb
index dcdd27b9a7..295d750e86 100644
--- a/lib/action_cable/channel/callbacks.rb
+++ b/lib/action_cable/channel/callbacks.rb
@@ -1,28 +1,35 @@
+require 'active_support/callbacks'
+
 module ActionCable
   module Channel
     module Callbacks
-      extend ActiveSupport::Concern
+      extend  ActiveSupport::Concern
+      include ActiveSupport::Callbacks
 
       included do
-        class_attribute :on_subscribe_callbacks, :on_unsubscribe_callbacks, instance_reader: false
-
-        self.on_subscribe_callbacks = []
-        self.on_unsubscribe_callbacks = []
+        define_callbacks :subscribe
+        define_callbacks :unsubscribe
       end
 
-      module ClassMethods
-        # Name methods that should be called when the channel is subscribed to.
-        # (These methods should be private, so they're not callable by the user).
-        def on_subscribe(*methods)
-          self.on_subscribe_callbacks += methods
+      class_methods do
+        def before_subscribe(*methods, &block)
+          set_callback(:subscribe, :before, *methods, &block)
+        end
+
+        def after_subscribe(*methods, &block)
+          set_callback(:subscribe, :after, *methods, &block)
+        end
+        alias_method :on_subscribe, :after_subscribe
+
+        def before_unsubscribe(*methods, &block)
+          set_callback(:unsubscribe, :before, *methods, &block)
         end
 
-        # Name methods that should be called when the channel is unsubscribed from.
-        # (These methods should be private, so they're not callable by the user).
-        def on_unsubscribe(*methods)
-          self.on_unsubscribe_callbacks += methods
+        def after_unsubscribe(*methods, &block)
+          set_callback(:unsubscribe, :after, *methods, &block)
         end
+        alias_method :on_unsubscribe, :after_unsubscribe
       end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/lib/action_cable/channel/periodic_timers.rb b/lib/action_cable/channel/periodic_timers.rb
index 9bdcc87aa5..25fe8e5e54 100644
--- a/lib/action_cable/channel/periodic_timers.rb
+++ b/lib/action_cable/channel/periodic_timers.rb
@@ -7,8 +7,8 @@ module ActionCable
         class_attribute :periodic_timers, instance_reader: false
         self.periodic_timers = []
 
-        on_subscribe   :start_periodic_timers
-        on_unsubscribe :stop_periodic_timers
+        after_subscribe   :start_periodic_timers
+        after_unsubscribe :stop_periodic_timers
       end
 
       module ClassMethods
@@ -38,4 +38,4 @@ module ActionCable
         end
     end
   end
-end
\ No newline at end of file
+end
diff --git a/lib/action_cable/connection/base.rb b/lib/action_cable/connection/base.rb
index b3de4dd4a9..a988060d56 100644
--- a/lib/action_cable/connection/base.rb
+++ b/lib/action_cable/connection/base.rb
@@ -56,7 +56,7 @@ module ActionCable
       def initialize(server, env)
         @server, @env = server, env
 
-        @logger = new_tagged_logger
+        @logger = new_tagged_logger || server.logger
 
         @websocket      = ActionCable::Connection::WebSocket.new(env)
         @subscriptions  = ActionCable::Connection::Subscriptions.new(self)
@@ -151,7 +151,6 @@ module ActionCable
           server.add_connection(self)
         rescue ActionCable::Connection::Authorization::UnauthorizedError
           respond_to_invalid_request
-          close
         end
 
         def on_message(message)
@@ -186,6 +185,8 @@ module ActionCable
         end
 
         def respond_to_invalid_request
+          close if websocket.alive?
+
           logger.info finished_request_message
           [ 404, { 'Content-Type' => 'text/plain' }, [ 'Page not found' ] ]
         end
@@ -193,8 +194,10 @@ module ActionCable
 
         # Tags are declared in the server but computed in the connection. This allows us per-connection tailored tags.
         def new_tagged_logger
-          TaggedLoggerProxy.new server.logger,
-            tags: server.config.log_tags.map { |tag| tag.respond_to?(:call) ? tag.call(request) : tag.to_s.camelize }
+          if server.logger.respond_to?(:tagged)
+            TaggedLoggerProxy.new server.logger,
+              tags: server.config.log_tags.map { |tag| tag.respond_to?(:call) ? tag.call(request) : tag.to_s.camelize }
+          end
         end
 
         def started_request_message