From 218f0777d6abf1b2f933e44bd08f75f3d28c75ca Mon Sep 17 00:00:00 2001 From: Guilherme Mansur Date: Tue, 21 May 2019 17:07:27 -0400 Subject: Merge payload for EventObject subscribers When instrumenting a block of code like: ```ruby ActiveSupport::Notifications.instrument("process_action.action_controller", raw_paylaod) do |payload| payload[:view_runtime] = render_view end ``` If we use an evented subscriber like so: ``` ruby ActiveSupport::Notifications.subscribe("process_action.action_controller", raw_payload) do |event| assert event.payload[:view_runtime] end ``` The code breaks because the underlying EventObject's payload does not have the `:view_runtime` key added during instrumentation. This is because the `EventedObject` subscriber calls the `finish` method with the `payload` of the event at the time it was pushed into the stack, before the block executes, but we want to call `finish` with the `payload` after the instrument block executes this way if the `payload` was modified during the block we have access to it. This is consistent with the other types of subscribers who don't have this bug. --- activesupport/test/notifications_test.rb | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) (limited to 'activesupport/test') diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index 3b98749f1b..0d89189803 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -42,6 +42,27 @@ module Notifications assert_operator event.duration, :>, 0 end + def test_subscribe_to_events_where_payload_is_changed_during_instrumentation + @notifier.subscribe do |event| + assert_equal "success!", event.payload[:my_key] + end + + ActiveSupport::Notifications.instrument("foo") do |payload| + payload[:my_key] = "success!" + end + end + + def test_subscribe_to_events_can_handle_nested_hashes_in_the_paylaod + @notifier.subscribe do |event| + assert_equal "success!", event.payload[:some_key][:key_one] + assert_equal "great_success!", event.payload[:some_key][:key_two] + end + + ActiveSupport::Notifications.instrument("foo", some_key: { key_one: "success!" }) do |payload| + payload[:some_key][:key_two] = "great_success!" + end + end + def test_subscribe_via_top_level_api old_notifier = ActiveSupport::Notifications.notifier ActiveSupport::Notifications.notifier = ActiveSupport::Notifications::Fanout.new -- cgit v1.2.3