diff options
| -rw-r--r-- | activesupport/CHANGELOG.md | 4 | ||||
| -rw-r--r-- | activesupport/lib/active_support/notifications.rb | 9 | ||||
| -rw-r--r-- | activesupport/lib/active_support/notifications/fanout.rb | 38 | ||||
| -rw-r--r-- | activesupport/test/notifications/evented_notification_test.rb | 33 | ||||
| -rw-r--r-- | activesupport/test/notifications_test.rb | 19 |
5 files changed, 100 insertions, 3 deletions
diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7002474b8d..8bb531f1b8 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,7 @@ +* Revise `ActiveSupport::Notifications.unsubscribe` to correctly handle Regex or other multiple-pattern subscribers. + + *Zach Kemp* + * Add `before_reset` callback to `CurrentAttributes` and define `after_reset` as an alias of `resets` for symmetry. *Rosa Gutierrez* diff --git a/activesupport/lib/active_support/notifications.rb b/activesupport/lib/active_support/notifications.rb index 7ccc333463..d9e93b530c 100644 --- a/activesupport/lib/active_support/notifications.rb +++ b/activesupport/lib/active_support/notifications.rb @@ -153,6 +153,15 @@ module ActiveSupport # # ActiveSupport::Notifications.unsubscribe("render") # + # Subscribers using a regexp or other pattern-matching object will remain subscribed + # to all events that match their original pattern, unless those events match a string + # passed to `unsubscribe`: + # + # subscriber = ActiveSupport::Notifications.subscribe(/render/) { } + # ActiveSupport::Notifications.unsubscribe('render_template.action_view') + # subscriber.matches?('render_template.action_view') # => false + # subscriber.matches?('render_partial.action_view') # => true + # # == Default Queue # # Notifications ships with a queue implementation that consumes and publishes events diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 11721db103..f06504cf2c 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -2,6 +2,7 @@ require "mutex_m" require "concurrent/map" +require "set" module ActiveSupport module Notifications @@ -39,6 +40,7 @@ module ActiveSupport when String @string_subscribers[subscriber_or_name].clear @listeners_for.delete(subscriber_or_name) + @other_subscribers.each { |sub| sub.unsubscribe!(subscriber_or_name) } else pattern = subscriber_or_name.try(:pattern) if String === pattern @@ -113,11 +115,33 @@ module ActiveSupport end end + class Matcher #:nodoc: + attr_reader :pattern, :exclusions + + def self.wrap(pattern) + return pattern if String === pattern + new(pattern) + end + + def initialize(pattern) + @pattern = pattern + @exclusions = Set.new + end + + def unsubscribe!(name) + exclusions << -name if pattern === name + end + + def ===(name) + pattern === name && !exclusions.include?(name) + end + end + class Evented #:nodoc: attr_reader :pattern def initialize(pattern, delegate) - @pattern = pattern + @pattern = Matcher.wrap(pattern) @delegate = delegate @can_publish = delegate.respond_to?(:publish) end @@ -137,11 +161,15 @@ module ActiveSupport end def subscribed_to?(name) - @pattern === name + pattern === name end def matches?(name) - @pattern && @pattern === name + pattern && pattern === name + end + + def unsubscribe!(name) + pattern.unsubscribe!(name) end end @@ -204,6 +232,10 @@ module ActiveSupport true end + def unsubscribe!(*) + false + end + alias :matches? :=== end end diff --git a/activesupport/test/notifications/evented_notification_test.rb b/activesupport/test/notifications/evented_notification_test.rb index 4beb8194b9..ab2a9b8659 100644 --- a/activesupport/test/notifications/evented_notification_test.rb +++ b/activesupport/test/notifications/evented_notification_test.rb @@ -84,6 +84,39 @@ module ActiveSupport [:finish, "hi", 1, {}] ], listener.events end + + def test_listen_to_regexp + notifier = Fanout.new + listener = Listener.new + notifier.subscribe(/[a-z]*.world/, listener) + notifier.start("hi.world", 1, {}) + notifier.finish("hi.world", 2, {}) + notifier.start("hello.world", 1, {}) + notifier.finish("hello.world", 2, {}) + + assert_equal [ + [:start, "hi.world", 1, {}], + [:finish, "hi.world", 2, {}], + [:start, "hello.world", 1, {}], + [:finish, "hello.world", 2, {}] + ], listener.events + end + + def test_listen_to_regexp_with_exclusions + notifier = Fanout.new + listener = Listener.new + notifier.subscribe(/[a-z]*.world/, listener) + notifier.unsubscribe("hi.world") + notifier.start("hi.world", 1, {}) + notifier.finish("hi.world", 2, {}) + notifier.start("hello.world", 1, {}) + notifier.finish("hello.world", 2, {}) + + assert_equal [ + [:start, "hello.world", 1, {}], + [:finish, "hello.world", 2, {}] + ], listener.events + end end end end diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index b5d72d1a42..bb20d26a25 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -128,6 +128,25 @@ module Notifications assert_equal [["named.subscription", :foo], ["named.subscription", :foo]], @events end + def test_unsubscribing_by_name_leaves_regexp_matched_subscriptions + @matched_events = [] + @notifier.subscribe(/subscription/) { |*args| @matched_events << event(*args) } + @notifier.publish("named.subscription", :before) + @notifier.wait + [@events, @named_events, @matched_events].each do |collector| + assert_includes(collector, ["named.subscription", :before]) + end + @notifier.unsubscribe("named.subscription") + @notifier.publish("named.subscription", :after) + @notifier.publish("other.subscription", :after) + @notifier.wait + assert_includes(@events, ["named.subscription", :after]) + assert_includes(@events, ["other.subscription", :after]) + assert_includes(@matched_events, ["other.subscription", :after]) + assert_not_includes(@matched_events, ["named.subscription", :after]) + assert_not_includes(@named_events, ["named.subscription", :after]) + end + private def event(*args) args |