Merge pull request #309 from kares/more-logger-silence

cleanup Rails LoggerSilence support

Author: kares

src/main/java/org/jruby/rack/ext/Logger.java

@@ -338,8 +338,7 @@ public IRubyObject unknown(final ThreadContext context, final Block block) {
 
     // def add(severity, message = nil, progname = nil, &block)
     @JRubyMethod(name = "add", required = 1, optional = 2)
-    public IRubyObject add(final ThreadContext context,
-        final IRubyObject[] args, final Block block) {
+    public IRubyObject add(final ThreadContext context, final IRubyObject[] args, final Block block) {
         int severity = UNKNOWN;
         final IRubyObject sev = args[0];
         if ( ! sev.isNil() ) {
@@ -471,42 +470,6 @@ private void doLog(RubyString message) {
         logger.log( message );
     }
 
-    // LoggerSilence API :
-
-    private static boolean silencer = false; // we're NOT true by default!
-
-    @JRubyMethod(name= "silencer", meta = true)
-    public static IRubyObject get_silencer(final ThreadContext context, final IRubyObject self) {
-        return context.runtime.newBoolean(silencer);
-    }
-
-    @JRubyMethod(name = "silencer=", meta = true)
-    public static IRubyObject set_silencer(final ThreadContext context, final IRubyObject self,
-        final IRubyObject value) {
-        return context.runtime.newBoolean(silencer = value.isTrue());
-    }
-
-    @JRubyMethod(name = "silence")
-    public IRubyObject silence(final ThreadContext context, final Block block) {
-        return doSilence(ERROR, context, block); // temp_level = Logger::ERROR
-    }
-
-    @JRubyMethod(name = "silence", required = 1)
-    public IRubyObject silence(final ThreadContext context, final IRubyObject temp_level, final Block block) {
-        final int tempLevel = (int) temp_level.convertToInteger("to_i").getLongValue();
-        return doSilence(tempLevel, context, block);
-    }
-
-    private IRubyObject doSilence(final int tempLevel, final ThreadContext context, final Block block) {
-        if ( silencer ) {
-            // not implemented - on purpose!
-            return block.yield(context, this);
-        }
-        else {
-            return block.yield(context, this);
-        }
-    }
-
     // (old) BufferedLogger API compatibility :
 
     @JRubyMethod(name = "flush", alias = { "auto_flushing", "auto_flushing=" })

src/main/ruby/jruby/rack/rails/railtie.rb

@@ -38,9 +38,7 @@ class Railtie < ::Rails::Railtie
         logger.formatter = log_formatter if log_formatter && logger.respond_to?(:formatter=)
         require 'active_support/tagged_logging' unless defined?(ActiveSupport::TaggedLogging)
         logger = ActiveSupport::TaggedLogging.new(logger) # returns a logger.clone
-        logger.singleton_class.instance_eval do
-          include ActiveSupport::LoggerSilence if defined?(ActiveSupport::LoggerSilence)
-        end
+        logger.singleton_class.send(:include, ActiveSupport::LoggerSilence) if defined?(ActiveSupport::LoggerSilence)
         logger
       end
     end

src/spec/ruby/jruby/rack/logger_spec.rb

@@ -102,19 +102,6 @@
     expect( logger.class::FATAL ).to eql 4
   end
 
-  it 'is not silencable (by default)' do
-    expect( JRuby::Rack::Logger.silencer ).to be false
-  end
-
-  it 'supports silence with block' do
-    called = nil
-    logger.silence do |logger|
-      called = true
-      expect( logger ).to be logger
-    end
-    expect( called ).to be true
-  end
-
   describe JRuby::Rack::ServletLog do
     let(:servlet_context_logger) do
       org.jruby.rack.logging.ServletContextLogger.new(servlet_context)

src/spec/ruby/jruby/rack/rails_booter_spec.rb

@@ -10,23 +10,23 @@
 
 describe JRuby::Rack::RailsBooter do
 
-  let(:booter) do
-    real_logger = org.jruby.rack.logging.BufferLogger.new
-    JRuby::Rack.logger = JRuby::Rack::Logger.new real_logger
-    JRuby::Rack::RailsBooter.new JRuby::Rack.context = @rack_context
+  let(:real_logger) do
+    org.jruby.rack.logging.BufferLogger.new
   end
 
-  let(:rails_booter) do
-    rails_booter = booter; def rails_booter.rails2?; nil end; rails_booter
+  let(:booter) do
+    JRuby::Rack.logger = JRuby::Rack::Logger.new(real_logger)
+    JRuby::Rack.context = @rack_context
+    JRuby::Rack::RailsBooter.new @rack_context
   end
 
   after { JRuby::Rack.context = nil; JRuby::Rack.logger = nil }
 
   it "should determine RAILS_ROOT from the 'rails.root' init parameter" do
     @rack_context.should_receive(:getInitParameter).with("rails.root").and_return "/WEB-INF"
     @rack_context.should_receive(:getRealPath).with("/WEB-INF").and_return "./WEB-INF"
-    rails_booter.boot!
-    rails_booter.app_path.should == "./WEB-INF"
+    booter.boot!
+    booter.app_path.should == "./WEB-INF"
   end
 
   before do
@@ -41,62 +41,62 @@
 
   it "should default rails path to /WEB-INF" do
     @rack_context.should_receive(:getRealPath).with("/WEB-INF").and_return "/usr/apps/WEB-INF"
-    rails_booter.boot!
-    rails_booter.app_path.should == "/usr/apps/WEB-INF"
+    booter.boot!
+    booter.app_path.should == "/usr/apps/WEB-INF"
   end
 
   it "leaves ENV['RAILS_ENV'] as is if it was already set" do
     ENV['RAILS_ENV'] = 'staging'
-    rails_booter.boot!
+    booter.boot!
     ENV['RAILS_ENV'].should == 'staging'
-    rails_booter.rails_env.should == "staging"
+    booter.rails_env.should == "staging"
   end
 
   it "determines RAILS_ENV from the 'rails.env' init parameter" do
     ENV['RAILS_ENV'] = nil
     @rack_context.should_receive(:getInitParameter).with("rails.env").and_return "test"
-    rails_booter.boot!
-    rails_booter.rails_env.should == "test"
+    booter.boot!
+    booter.rails_env.should == "test"
   end
 
   it "gets rails environment from rack environmnent" do
     ENV.delete('RAILS_ENV')
     ENV['RACK_ENV'] = 'development'
     @rack_context.stub(:getInitParameter)
-    rails_booter.boot!
-    rails_booter.rails_env.should == 'development'
+    booter.boot!
+    booter.rails_env.should == 'development'
   end
 
   it "default RAILS_ENV to 'production'" do
     ENV.delete('RAILS_ENV'); ENV.delete('RACK_ENV')
-    rails_booter.boot!
-    rails_booter.rails_env.should == "production"
+    booter.boot!
+    booter.rails_env.should == "production"
   end
 
   it "should set RAILS_RELATIVE_URL_ROOT based on the servlet context path" do
     @rack_context.should_receive(:getContextPath).and_return '/myapp'
-    rails_booter.boot!
+    booter.boot!
     ENV['RAILS_RELATIVE_URL_ROOT'].should == '/myapp'
   end
 
   it "should append to RAILS_RELATIVE_URL_ROOT if 'rails.relative_url_append' is set" do
     @rack_context.should_receive(:getContextPath).and_return '/myapp'
     @rack_context.should_receive(:getInitParameter).with("rails.relative_url_append").and_return "/blah"
-    rails_booter.boot!
+    booter.boot!
     ENV['RAILS_RELATIVE_URL_ROOT'].should == '/myapp/blah'
   end
 
   it "should determine the public html root from the 'public.root' init parameter" do
     @rack_context.should_receive(:getInitParameter).with("public.root").and_return "/blah"
     @rack_context.should_receive(:getRealPath).with("/blah").and_return "."
-    rails_booter.boot!
-    rails_booter.public_path.should == "."
+    booter.boot!
+    booter.public_path.should == "."
   end
 
   it "should default public root to '/'" do
     @rack_context.should_receive(:getRealPath).with("/").and_return "."
-    rails_booter.boot!
-    rails_booter.public_path.should == "."
+    booter.boot!
+    booter.public_path.should == "."
   end
 
   RAILS_ROOT_DIR = File.expand_path("../../../rails", __FILE__)
@@ -211,6 +211,7 @@ def logger=(logger); @logger = logger; end
         expect(rails_logger).to be_a(ActiveSupport::TaggedLogging)
         if defined? ActiveSupport::LoggerSilence
           expect(rails_logger).to be_a(ActiveSupport::LoggerSilence)
+          expect(rails_logger.silencer).to be true
           # sanity check silence works:
           value_returned = rails_logger.silence(Logger::WARN) { |logger| logger.class.name }
           expect(value_returned).to eql('JRuby::Rack::Logger')

src/spec/ruby/rails/stub/active_support/logger_silence.rb

@@ -12,16 +12,18 @@ def self.included(base)
       base.class_eval do
         # cattr_accessor :silencer, default: true
         @@silencer = true
-        def self.silencer; @@silencer end
-        def silencer; self.class.silencer end
+        module_eval do
+          def self.silencer; @@silencer end
+          def silencer; @@silencer end
+        end
 
         include ActiveSupport::LoggerThreadSafeLevel
       end
     end
 
     # Silences the logger for the duration of the block.
     def silence(severity = Logger::ERROR)
-      silencer ? log_at(severity) { yield self } : yield(self)
+      silencer ? log_at(severity) { yield(self) } : yield(self)
     end
   end
 end