Merge pull request #12285 from dasch/dasch/allow-attaching-up-front

Allow attaching to AS::Notifications namespace up front

3 files changed, 92 insertions, 2 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 9ab81c73c8..08c4573b14 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,30 @@
+*   Allow attaching event subscribers to ActiveSupport::Notifications namespaces
+    before they're defined. Essentially, this means instead of this:
+
+        class JokeSubscriber < ActiveSupport::Subscriber
+          def sql(event)
+            puts "A rabbi and a priest walk into a bar..."
+          end
+
+          # This call needs to happen *after* defining the methods.
+          attach_to "active_record"
+        end
+
+    You can do this:
+
+      class JokeSubscriber < ActiveSupport::Subscriber
+        # This is much easier to read!
+        attach_to "active_record"
+
+        def sql(event)
+          puts "A rabbi and a priest walk into a bar..."
+        end
+      end
+
+    This should make it easier to read and understand these subscribers.
+
+    *Daniel Schierbeck*
+
 *   Add `Date#middle_of_day`, `DateTime#middle_of_day` and `Time#middle_of_day` methods.
 
     Also added `midday`, `noon`, `at_midday`, `at_noon` and `at_middle_of_day` as aliases.
diff --git a/activesupport/lib/active_support/subscriber.rb b/activesupport/lib/active_support/subscriber.rb
index 34c6f900c1..f2966f23fc 100644
--- a/activesupport/lib/active_support/subscriber.rb
+++ b/activesupport/lib/active_support/subscriber.rb
@@ -31,18 +31,41 @@ module ActiveSupport
 
       # Attach the subscriber to a namespace.
       def attach_to(namespace, subscriber=new, notifier=ActiveSupport::Notifications)
+        @namespace  = namespace
+        @subscriber = subscriber
+        @notifier   = notifier
+
         subscribers << subscriber
 
+        # Add event subscribers for all existing methods on the class.
         subscriber.public_methods(false).each do |event|
-          next if %w{ start finish }.include?(event.to_s)
+          add_event_subscriber(event)
+        end
+      end
 
-          notifier.subscribe("#{event}.#{namespace}", subscriber)
+      # Adds event subscribers for all new methods added to the class.
+      def method_added(event)
+        # Only public methods are added as subscribers, and only if a notifier
+        # has been set up. This means that subscribers will only be set up for
+        # classes that call #attach_to.
+        if public_method_defined?(event) && notifier
+          add_event_subscriber(event)
         end
       end
 
       def subscribers
         @@subscribers ||= []
       end
+
+      protected
+
+      attr_reader :subscriber, :notifier, :namespace
+
+      def add_event_subscriber(event)
+        return if %w{ start finish }.include?(event.to_s)
+
+        notifier.subscribe("#{event}.#{namespace}", subscriber)
+      end
     end
 
     def initialize
diff --git a/activesupport/test/subscriber_test.rb b/activesupport/test/subscriber_test.rb
new file mode 100644
index 0000000000..253411aa3d
--- /dev/null
+++ b/activesupport/test/subscriber_test.rb
@@ -0,0 +1,40 @@
+require 'abstract_unit'
+require 'active_support/subscriber'
+
+class TestSubscriber < ActiveSupport::Subscriber
+  attach_to :doodle
+
+  cattr_reader :event
+
+  def self.clear
+    @@event = nil
+  end
+
+  def open_party(event)
+    @@event = event
+  end
+
+  private
+
+  def private_party(event)
+    @@event = event
+  end
+end
+
+class SubscriberTest < ActiveSupport::TestCase
+  def setup
+    TestSubscriber.clear
+  end
+
+  def test_attaches_subscribers
+    ActiveSupport::Notifications.instrument("open_party.doodle")
+
+    assert_equal "open_party.doodle", TestSubscriber.event.name
+  end
+
+  def test_does_not_attach_private_methods
+    ActiveSupport::Notifications.instrument("private_party.doodle")
+
+    assert_nil TestSubscriber.event
+  end
+end