From 409072e263bd6720a2da9c0d63ea1af6585fe48c Mon Sep 17 00:00:00 2001 From: Owen Stephens Date: Wed, 24 Jan 2024 23:14:56 +0000 Subject: [PATCH] Unsubscribe all built-in subscribers Fixes #385 Assumes that a built-in subscriber is nested under the component namespace, so under ActionView or ActionController. --- lib/lograge.rb | 9 ++++++++- spec/lograge_spec.rb | 28 ++++++++++++++++++++-------- spec/support/custom_listener.rb | 11 +++++++++++ 3 files changed, 39 insertions(+), 9 deletions(-) create mode 100644 spec/support/custom_listener.rb diff --git a/lib/lograge.rb b/lib/lograge.rb index 7ac247ac..a5010980 100644 --- a/lib/lograge.rb +++ b/lib/lograge.rb @@ -121,11 +121,18 @@ def unsubscribe(component, subscriber) events = subscriber.public_methods(false).reject { |method| method.to_s == 'call' } events.each do |event| Lograge.notification_listeners_for("#{event}.#{component}").each do |listener| - ActiveSupport::Notifications.unsubscribe listener if listener.instance_variable_get('@delegate') == subscriber + ActiveSupport::Notifications.unsubscribe listener if built_in_rails_listener?(listener, component) end end end + def built_in_rails_listener?(listener, component) + delegate = listener.instance_variable_get('@delegate') + + # Assume that anything nested under the component namespace is built in to Rails + delegate.class.name.start_with?("#{component.to_s.classify}::") + end + def setup(app) self.application = app disable_rack_cache_verbose_output diff --git a/spec/lograge_spec.rb b/spec/lograge_spec.rb index 68b1e2e0..85d43b23 100644 --- a/spec/lograge_spec.rb +++ b/spec/lograge_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'support/custom_listener' + require 'active_support' require 'active_support/notifications' require 'active_support/core_ext/string' @@ -21,7 +23,7 @@ Lograge.remove_existing_log_subscriptions end.to change { Lograge.notification_listeners_for('process_action.action_controller') - } + }.to([]) end it 'removes subscribers for all events' do @@ -29,16 +31,26 @@ Lograge.remove_existing_log_subscriptions end.to change { Lograge.notification_listeners_for('render_template.action_view') - } + }.to([]) end - it "does not remove subscribers that aren't from Rails" do - blk = -> {} - ActiveSupport::Notifications.subscribe('process_action.action_controller', &blk) - Lograge.remove_existing_log_subscriptions - listeners = Lograge.notification_listeners_for('process_action.action_controller') - expect(listeners.size).to eq(1) + shared_examples 'preserving non-Rails subscribers' do |event_name| + context "with event_name #{event_name}" do + it "does not remove subscribers that aren't from Rails" do + proc_subscriber = ActiveSupport::Notifications.subscribe(event_name, proc {}) + custom_subscriber = + ActiveSupport::Notifications.subscribe(event_name, CustomListener.new) + + Lograge.remove_existing_log_subscriptions + + listeners = Lograge.notification_listeners_for(event_name) + expect(listeners).to contain_exactly(proc_subscriber, custom_subscriber) + end + end end + + include_examples 'preserving non-Rails subscribers', 'render_template.action_view' + include_examples 'preserving non-Rails subscribers', 'process_action.action_controller' end describe 'keep_original_rails_log option' do diff --git a/spec/support/custom_listener.rb b/spec/support/custom_listener.rb new file mode 100644 index 00000000..d17c122d --- /dev/null +++ b/spec/support/custom_listener.rb @@ -0,0 +1,11 @@ +class CustomListener + attr_reader :events + + def initialize + @events = [] + end + + def call(*args) + @events << args + end +end