Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsubscribe all Rails built-in subscribers #386

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion lib/lograge.rb
Expand Up @@ -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}::")
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is great, but I can't see a better way to check if the delegate is subscriber or (for Rails 7.1+) an instance of ActionView::LogSubscriber::Start for ActionView or subscriber for ActionController

end

def setup(app)
self.application = app
disable_rack_cache_verbose_output
Expand Down
28 changes: 20 additions & 8 deletions 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'
Expand All @@ -21,24 +23,34 @@
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
expect do
Lograge.remove_existing_log_subscriptions
end.to change {
Lograge.notification_listeners_for('render_template.action_view')
}
}.to([])
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This spec failed after this change on Rails 7.1

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)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also check a custom class, in case there was some special handling of Proc instances


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
Expand Down
11 changes: 11 additions & 0 deletions 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