Fixed duplicate subscribers in ActiveSupport::Subscriber

ActiveSupport::Subscriber no longer creates multiple subscribers when
you redefine a method.

3 files changed, 34 insertions, 7 deletions

diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md
index 8e63273271..7e9c8cf54b 100644
--- a/activesupport/CHANGELOG.md
+++ b/activesupport/CHANGELOG.md
@@ -1,3 +1,8 @@
+*   Fixed `ActiveSupport::Subscriber` so that no duplicate subscriber is created
+    when a subscriber method is redefined.
+
+    *Dennis Schön*
+
 *   `humanize` strips leading underscores, if any.
 
     Before:
diff --git a/activesupport/lib/active_support/subscriber.rb b/activesupport/lib/active_support/subscriber.rb
index 4b9b48539f..4306c2ac8d 100644
--- a/activesupport/lib/active_support/subscriber.rb
+++ b/activesupport/lib/active_support/subscriber.rb
@@ -64,12 +64,21 @@ module ActiveSupport
       def add_event_subscriber(event)
         return if %w{ start finish }.include?(event.to_s)
 
-        notifier.subscribe("#{event}.#{namespace}", subscriber)
+        pattern = "#{event}.#{namespace}"
+
+        # don't add multiple subscribers (eg. if methods are redefined)
+        return if subscriber.patterns.include?(pattern)
+
+        subscriber.patterns << pattern
+        notifier.subscribe(pattern, subscriber)
       end
     end
 
+    attr_reader :patterns
+
     def initialize
       @queue_key = [self.class.name, object_id].join "-"
+      @patterns  = []
       super
     end
 
diff --git a/activesupport/test/subscriber_test.rb b/activesupport/test/subscriber_test.rb
index 253411aa3d..8be8c51f07 100644
--- a/activesupport/test/subscriber_test.rb
+++ b/activesupport/test/subscriber_test.rb
@@ -4,20 +4,27 @@ require 'active_support/subscriber'
 class TestSubscriber < ActiveSupport::Subscriber
   attach_to :doodle
 
-  cattr_reader :event
+  cattr_reader :events
 
   def self.clear
-    @@event = nil
+    @@events = []
   end
 
   def open_party(event)
-    @@event = event
+    events << event
   end
 
   private
 
   def private_party(event)
-    @@event = event
+    events << event
+  end
+end
+
+# Monkey patch subscriber to test that only one subscriber per method is added.
+class TestSubscriber
+  def open_party(event)
+    events << event
   end
 end
 
@@ -29,12 +36,18 @@ class SubscriberTest < ActiveSupport::TestCase
   def test_attaches_subscribers
     ActiveSupport::Notifications.instrument("open_party.doodle")
 
-    assert_equal "open_party.doodle", TestSubscriber.event.name
+    assert_equal "open_party.doodle", TestSubscriber.events.first.name
+  end
+
+  def test_attaches_only_one_subscriber
+    ActiveSupport::Notifications.instrument("open_party.doodle")
+
+    assert_equal 1, TestSubscriber.events.size
   end
 
   def test_does_not_attach_private_methods
     ActiveSupport::Notifications.instrument("private_party.doodle")
 
-    assert_nil TestSubscriber.event
+    assert_equal TestSubscriber.events, []
   end
 end