1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Currently subscription api on notifications is only supported for String, Regex or nil. If we pass anything different, this causes issues because, it gets added to "other subscribers" silently.

We now raise an argument error instead if something invalid is passed.

Partially accesses original class implementation from 4f2a04cc08 which used the case/when
This commit is contained in:
Vipul A M 2020-06-14 19:58:09 +05:30
parent b1646a48fa
commit bcec581160
3 changed files with 22 additions and 2 deletions

View file

@ -231,6 +231,12 @@ module ActiveSupport
# ActiveSupport::Notifications.subscribe(/render/) do |event|
# @event = event
# end
#
# Raises an error if invalid event name type is passed:
#
# ActiveSupport::Notifications.subscribe(:render) {|*args| ...}
# #=> ArgumentError (pattern must be specified as a String, Regexp or empty)
#
def subscribe(pattern = nil, callback = nil, &block)
notifier.subscribe(pattern, callback, monotonic: false, &block)
end

View file

@ -24,12 +24,15 @@ module ActiveSupport
def subscribe(pattern = nil, callable = nil, monotonic: false, &block)
subscriber = Subscribers.new(pattern, callable || block, monotonic)
synchronize do
if String === pattern
case pattern
when String
@string_subscribers[pattern] << subscriber
@listeners_for.delete(pattern)
else
when NilClass, Regexp
@other_subscribers << subscriber
@listeners_for.clear
else
raise ArgumentError, "pattern must be specified as a String, Regexp or empty"
end
end
subscriber

View file

@ -443,6 +443,17 @@ module Notifications
assert_not not_child.parent_of?(parent)
end
def test_subscribe_raises_error_on_non_supported_arguments
notifier = ActiveSupport::Notifications::Fanout.new
assert_raises ArgumentError do
notifier.subscribe(:symbol) { |*_| }
end
assert_raises ArgumentError do
notifier.subscribe(Object.new) { |*_| }
end
end
private
def random_id
@random_id ||= SecureRandom.hex(10)