Merge pull request #22933 from schneems/schneems/fix-broadcast

[close #22917] Don't output to `STDOUT` twice

activerecord/lib/active_record/railtie.rb

@@ -57,8 +57,10 @@ class Railtie < Rails::Railtie # :nodoc:
     console do |app|
       require "active_record/railties/console_sandbox" if app.sandbox?
       require "active_record/base"
-      console = ActiveSupport::Logger.new(STDERR)
-      Rails.logger.extend ActiveSupport::Logger.broadcast console
+      unless ActiveSupport::Logger.logger_outputs_to?(Rails.logger, STDERR, STDOUT)
+        console = ActiveSupport::Logger.new(STDERR)
+        Rails.logger.extend ActiveSupport::Logger.broadcast console
+      end
     end
 
     runner do

activesupport/lib/active_support/logger.rb

@@ -5,26 +5,27 @@ module ActiveSupport
   class Logger < ::Logger
     include LoggerSilence
 
-    # If +true+, will broadcast all messages sent to this logger to any
-    # logger linked to this one via +broadcast+.
+    # Returns true if the logger destination matches one of the sources
     #
-    # If +false+, the logger will still forward calls to +close+, +progname=+,
-    # +formatter=+ and +level+ to any linked loggers, but no calls to +add+ or
-    # +<<+.
-    #
-    # Defaults to +true+.
-    attr_accessor :broadcast_messages # :nodoc:
+    #   logger = Logger.new(STDOUT)
+    #   ActiveSupport::Logger.logger_outputs_to?(logger, STDOUT)
+    #   # => true
+    def self.logger_outputs_to?(logger, *sources)
+      logdev = logger.instance_variable_get("@logdev")
+      logger_source = logdev.dev if logdev.respond_to?(:dev)
+      sources.any? { |source| source == logger_source }
+    end
 
     # Broadcasts logs to multiple loggers.
     def self.broadcast(logger) # :nodoc:
       Module.new do
         define_method(:add) do |*args, &block|
-          logger.add(*args, &block) if broadcast_messages
+          logger.add(*args, &block)
           super(*args, &block)
         end
 
         define_method(:<<) do |x|
-          logger << x if broadcast_messages
+          logger << x
           super(x)
         end
 
@@ -53,7 +54,6 @@ def self.broadcast(logger) # :nodoc:
     def initialize(*args)
       super
       @formatter = SimpleFormatter.new
-      @broadcast_messages = true
       after_initialize if respond_to? :after_initialize
     end
 

activesupport/lib/active_support/logger_silence.rb

@@ -42,4 +42,4 @@ def silence(temporary_level = Logger::ERROR)
       yield self
     end
   end
-end
+end

activesupport/test/broadcast_logger_test.rb

@@ -2,69 +2,56 @@
 
 module ActiveSupport
   class BroadcastLoggerTest < TestCase
-    attr_reader :logger, :receiving_logger
+    attr_reader :logger, :log1, :log2
     def setup
-      @logger = FakeLogger.new
-      @receiving_logger = FakeLogger.new
-      @logger.extend Logger.broadcast @receiving_logger
+      @log1 = FakeLogger.new
+      @log2 = FakeLogger.new
+      @log1.extend Logger.broadcast @log2
+      @logger = @log1
     end
 
     def test_debug
       logger.debug "foo"
-      assert_equal 'foo', logger.adds.first[2]
-      assert_equal 'foo', receiving_logger.adds.first[2]
-    end
-
-    def test_debug_without_message_broadcasts
-      logger.broadcast_messages = false
-      logger.debug "foo"
-      assert_equal 'foo', logger.adds.first[2]
-      assert_equal [], receiving_logger.adds
+      assert_equal 'foo', log1.adds.first[2]
+      assert_equal 'foo', log2.adds.first[2]
     end
 
     def test_close
       logger.close
-      assert logger.closed, 'should be closed'
-      assert receiving_logger.closed, 'should be closed'
+      assert log1.closed, 'should be closed'
+      assert log2.closed, 'should be closed'
     end
 
     def test_chevrons
       logger << "foo"
-      assert_equal %w{ foo }, logger.chevrons
-      assert_equal %w{ foo }, receiving_logger.chevrons
-    end
-
-    def test_chevrons_without_message_broadcasts
-      logger.broadcast_messages = false
-      logger << "foo"
-      assert_equal %w{ foo }, logger.chevrons
-      assert_equal [], receiving_logger.chevrons
+      assert_equal %w{ foo }, log1.chevrons
+      assert_equal %w{ foo }, log2.chevrons
     end
 
     def test_level
       assert_nil logger.level
       logger.level = 10
-      assert_equal 10, logger.level
-      assert_equal 10, receiving_logger.level
+      assert_equal 10, log1.level
+      assert_equal 10, log2.level
     end
 
     def test_progname
       assert_nil logger.progname
       logger.progname = 10
-      assert_equal 10, logger.progname
-      assert_equal 10, receiving_logger.progname
+      assert_equal 10, log1.progname
+      assert_equal 10, log2.progname
     end
 
     def test_formatter
       assert_nil logger.formatter
       logger.formatter = 10
-      assert_equal 10, logger.formatter
-      assert_equal 10, receiving_logger.formatter
+      assert_equal 10, log1.formatter
+      assert_equal 10, log2.formatter
     end
 
     class FakeLogger
       attr_reader :adds, :closed, :chevrons
-      attr_accessor :level, :progname, :formatter, :broadcast_messages
+      attr_accessor :level, :progname, :formatter
 
       def initialize
         @adds      = []
@@ -73,7 +60,6 @@ def initialize
         @level     = nil
         @progname  = nil
         @formatter = nil
-        @broadcast_messages = true
       end
 
       def debug msg, &block

activesupport/test/logger_test.rb

@@ -17,6 +17,14 @@ def setup
     @logger  = Logger.new(@output)
   end
 
+  def test_log_outputs_to
+    assert Logger.logger_outputs_to?(@logger, @output),            "Expected logger_outputs_to? @output to return true but was false"
+    assert Logger.logger_outputs_to?(@logger, @output, STDOUT),    "Expected logger_outputs_to? @output or STDOUT to return true but was false"
+
+    assert_not Logger.logger_outputs_to?(@logger, STDOUT),         "Expected logger_outputs_to? to STDOUT to return false, but was true"
+    assert_not Logger.logger_outputs_to?(@logger, STDOUT, STDERR), "Expected logger_outputs_to? to STDOUT or STDERR to return false, but was true"
+  end
+
   def test_write_binary_data_to_existing_file
     t = Tempfile.new ['development', 'log']
     t.binmode
@@ -65,7 +73,7 @@ def test_should_log_debugging_message_when_debugging
   def test_should_not_log_debug_messages_when_log_level_is_info
     @logger.level = Logger::INFO
     @logger.add(Logger::DEBUG, @message)
-    assert ! @output.string.include?(@message)
+    assert_not @output.string.include?(@message)
   end
 
   def test_should_add_message_passed_as_block_when_using_add
@@ -129,7 +137,7 @@ def test_silencing_everything_but_errors
       @logger.error "THIS IS HERE"
     end
 
-    assert !@output.string.include?("NOT THERE")
+    assert_not @output.string.include?("NOT THERE")
     assert @output.string.include?("THIS IS HERE")
   end
 

railties/lib/rails/commands/server.rb

@@ -133,11 +133,13 @@ def create_tmp_directories
       def log_to_stdout
         wrapped_app # touch the app so the logger is set up
 
-        console = ActiveSupport::Logger.new($stdout)
+        console = ActiveSupport::Logger.new(STDOUT)
         console.formatter = Rails.logger.formatter
         console.level = Rails.logger.level
 
-        Rails.logger.extend(ActiveSupport::Logger.broadcast(console))
+        unless ActiveSupport::Logger.logger_outputs_to?(Rails.logger, STDOUT)
+          Rails.logger.extend(ActiveSupport::Logger.broadcast(console))
+        end
       end
   end
 end