Merge pull request #23616 from piotrj/issue_23609_logger_broadcast_silencing

Fix logger silencing for broadcasted loggers

4 files changed, 86 insertions, 21 deletions

diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb
index 7626b28108..de48e717b6 100644
--- a/activesupport/lib/active_support/logger.rb
+++ b/activesupport/lib/active_support/logger.rb
@@ -1,8 +1,10 @@
 require 'active_support/logger_silence'
+require 'active_support/logger_thread_safe_level'
 require 'logger'
 
 module ActiveSupport
   class Logger < ::Logger
+    include ActiveSupport::LoggerThreadSafeLevel
     include LoggerSilence
 
     # Returns true if the logger destination matches one of the sources
@@ -48,6 +50,11 @@ module ActiveSupport
           logger.level = level
           super(level)
         end
+
+        define_method(:local_level=) do |level|
+          logger.local_level = level if logger.respond_to?(:local_level=)
+          super(level) if respond_to?(:local_level=)
+        end
       end
     end
 
diff --git a/activesupport/lib/active_support/logger_silence.rb b/activesupport/lib/active_support/logger_silence.rb
index 125d81d973..3eb8098c77 100644
--- a/activesupport/lib/active_support/logger_silence.rb
+++ b/activesupport/lib/active_support/logger_silence.rb
@@ -7,39 +7,22 @@ module LoggerSilence
 
   included do
     cattr_accessor :silencer
-    attr_reader :local_levels
     self.silencer = true
   end
 
-  def after_initialize
-    @local_levels = Concurrent::Map.new(:initial_capacity => 2)
-  end
-
-  def local_log_id
-    Thread.current.__id__
-  end
-
-  def level
-    local_levels[local_log_id] || super
-  end
-
   # Silences the logger for the duration of the block.
   def silence(temporary_level = Logger::ERROR)
     if silencer
       begin
-        old_local_level            = local_levels[local_log_id]
-        local_levels[local_log_id] = temporary_level
+        old_local_level            = local_level
+        self.local_level           = temporary_level
 
         yield self
       ensure
-        if old_local_level
-          local_levels[local_log_id] = old_local_level
-        else
-          local_levels.delete(local_log_id)
-        end
+        self.local_level = old_local_level
       end
     else
       yield self
     end
   end
-end
\ No newline at end of file
+end
diff --git a/activesupport/lib/active_support/logger_thread_safe_level.rb b/activesupport/lib/active_support/logger_thread_safe_level.rb
new file mode 100644
index 0000000000..0aad93e7f2
--- /dev/null
+++ b/activesupport/lib/active_support/logger_thread_safe_level.rb
@@ -0,0 +1,31 @@
+require 'active_support/concern'
+
+module ActiveSupport
+  module LoggerThreadSafeLevel
+    extend ActiveSupport::Concern
+
+    def after_initialize
+      @local_levels = Concurrent::Map.new(:initial_capacity => 2)
+    end
+
+    def local_log_id
+      Thread.current.__id__
+    end
+
+    def local_level
+      @local_levels[local_log_id]
+    end
+
+    def local_level=(level)
+      if level
+        @local_levels[local_log_id] = level
+      else
+        @local_levels.delete(local_log_id)
+      end
+    end
+
+    def level
+      local_level || super
+    end
+  end
+end
diff --git a/activesupport/test/logger_test.rb b/activesupport/test/logger_test.rb
index 317e09b7f2..5a91420f1e 100644
--- a/activesupport/test/logger_test.rb
+++ b/activesupport/test/logger_test.rb
@@ -141,6 +141,50 @@ class LoggerTest < ActiveSupport::TestCase
     assert @output.string.include?("THIS IS HERE")
   end
 
+  def test_logger_silencing_works_for_broadcast
+    another_output  = StringIO.new
+    another_logger  = Logger.new(another_output)
+
+    @logger.extend Logger.broadcast(another_logger)
+
+    @logger.debug "CORRECT DEBUG"
+    @logger.silence do
+      @logger.debug "FAILURE"
+      @logger.error "CORRECT ERROR"
+    end
+
+    assert @output.string.include?("CORRECT DEBUG")
+    assert @output.string.include?("CORRECT ERROR")
+    assert_not @output.string.include?("FAILURE")
+
+    assert another_output.string.include?("CORRECT DEBUG")
+    assert another_output.string.include?("CORRECT ERROR")
+    assert_not another_output.string.include?("FAILURE")
+  end
+
+  def test_broadcast_silencing_does_not_break_plain_ruby_logger
+    another_output  = StringIO.new
+    another_logger  = ::Logger.new(another_output)
+
+    @logger.extend Logger.broadcast(another_logger)
+
+    @logger.debug "CORRECT DEBUG"
+    @logger.silence do
+      @logger.debug "FAILURE"
+      @logger.error "CORRECT ERROR"
+    end
+
+    assert @output.string.include?("CORRECT DEBUG")
+    assert @output.string.include?("CORRECT ERROR")
+    assert_not @output.string.include?("FAILURE")
+
+    assert another_output.string.include?("CORRECT DEBUG")
+    assert another_output.string.include?("CORRECT ERROR")
+    assert another_output.string.include?("FAILURE")
+    # We can't silence plain ruby Logger cause with thread safety
+    # but at least we don't break it
+  end
+
   def test_logger_level_per_object_thread_safety
     logger1 = Logger.new(StringIO.new)
     logger2 = Logger.new(StringIO.new)