allow unsubscribe by name or subscription [#4433 state:resolved]

Signed-off-by: Jeremy Kemper <jeremy@bitsweat.net>

3 files changed, 38 insertions, 5 deletions

diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb
index b3758218a2..27939e3434 100644
--- a/actionpack/lib/action_controller/test_case.rb
+++ b/actionpack/lib/action_controller/test_case.rb
@@ -36,6 +36,7 @@ module ActionController
     end
 
     def teardown_subscriptions
+      ActiveSupport::Notifications.unsubscribe("action_view.render_template")
       ActiveSupport::Notifications.unsubscribe("action_view.render_template!")
     end
 
diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb
index a3ddc7705a..300ec842a9 100644
--- a/activesupport/lib/active_support/notifications/fanout.rb
+++ b/activesupport/lib/active_support/notifications/fanout.rb
@@ -19,8 +19,8 @@ module ActiveSupport
       end
 
       def unsubscribe(subscriber)
-        @subscribers.delete(subscriber)
         @listeners_for.clear
+        @subscribers.reject! {|s| s.matches?(subscriber)}
       end
 
       def publish(name, *args)
@@ -60,7 +60,7 @@ module ActiveSupport
         end
 
         def publish(*args)
-          return unless matches?(args.first)
+          return unless subscribed_to?(args.first)
           push(*args)
           true
         end
@@ -69,10 +69,20 @@ module ActiveSupport
           true
         end
 
-        private
-          def matches?(name)
-            !@pattern || @pattern =~ name.to_s
+        def subscribed_to?(name)
+          !@pattern || @pattern =~ name.to_s
+        end
+
+        def matches?(subscriber_or_name)
+          case subscriber_or_name
+          when String
+            @pattern && @pattern =~ subscriber_or_name
+          when self
+            true
           end
+        end
+
+        private
 
           def push(*args)
             @block.call(*args)
diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb
index 92fbe5b92f..c2e1c588f0 100644
--- a/activesupport/test/notifications_test.rb
+++ b/activesupport/test/notifications_test.rb
@@ -6,7 +6,9 @@ module Notifications
       ActiveSupport::Notifications.notifier = nil
       @notifier = ActiveSupport::Notifications.notifier
       @events = []
+      @named_events = []
       @subscription = @notifier.subscribe { |*args| @events << event(*args) }
+      @named_subscription = @notifier.subscribe("named.subscription") { |*args| @named_events << event(*args) }
     end
 
     private
@@ -30,6 +32,26 @@ module Notifications
       assert_equal [[:foo]], @events
     end
 
+    def test_unsubscribing_by_name_removes_a_subscription
+      @notifier.publish "named.subscription", :foo
+      @notifier.wait
+      assert_equal [["named.subscription", :foo]], @named_events
+      @notifier.unsubscribe("named.subscription")
+      @notifier.publish "named.subscription", :foo
+      @notifier.wait
+      assert_equal [["named.subscription", :foo]], @named_events
+    end
+
+    def test_unsubscribing_by_name_leaves_the_other_subscriptions
+      @notifier.publish "named.subscription", :foo
+      @notifier.wait
+      assert_equal [["named.subscription", :foo]], @events
+      @notifier.unsubscribe("named.subscription")
+      @notifier.publish "named.subscription", :foo
+      @notifier.wait
+      assert_equal [["named.subscription", :foo], ["named.subscription", :foo]], @events
+    end
+
   private
     def event(*args)
       args