more (low-hanging) cleanups for 1.3

src/main/java/org/jruby/rack/AbstractRackDispatcher.java

@@ -9,6 +9,8 @@
 
 import java.io.IOException;
 
+import static org.jruby.rack.RackLogger.Level.*;
+
 /**
  *
  * @author nicksieger
@@ -46,10 +48,10 @@ protected void handleException(
             final RackResponseEnvironment response) throws IOException {
 
         if ( response.isCommitted() ) {
-            context.log(RackLogger.ERROR, "couldn't handle exception (response is committed)", e);
+            context.log(ERROR, "couldn't handle exception (response is committed)", e);
             return;
         }
-        context.log(RackLogger.INFO, "resetting rack response due exception: " + e);
+        context.log(INFO, "resetting rack response due exception: " + e);
         response.reset();
 
         afterException(request, e, response);

src/main/java/org/jruby/rack/DefaultErrorApplication.java

@@ -33,6 +33,8 @@
 
 import org.jruby.Ruby;
 
+import static org.jruby.rack.RackLogger.Level.*;
+
 /**
  * Default error application if the Rack error application can not be setup or
  * "jruby.rack.error" handling is turned off (set to false).
@@ -117,7 +119,7 @@ public String getBody() {
                     body = buildErrorBody();
                 }
                 catch (Exception e) {
-                    log(RackLogger.INFO, "failed building error body", e);
+                    log(INFO, "failed building error body", e);
                     body = getError() == null ? "" : getError().toString();
                 }
             }
@@ -149,11 +151,11 @@ public void respond(RackResponseEnvironment response) {
                 defaultRespond(this, response);
             }
             catch (IOException e) {
-                log(RackLogger.WARN, "could not write response body", e);
+                log(WARN, "could not write response body", e);
             }
         }
 
-        private void log(String level, String message, Throwable e) {
+        private void log(RackLogger.Level level, String message, Throwable e) {
             if ( context != null ) context.log(level, message, e);
         }
 

src/main/java/org/jruby/rack/DefaultRackApplicationFactory.java

@@ -565,7 +565,7 @@ private String resolveRackupScript() throws RackInitializationException {
                     }
                     catch (IOException ex) { /* won't happen */ }
 
-                    rackContext.log(RackLogger.ERROR, "failed to read rackup from '"+ rackup + "' (" + e + ")");
+                    rackContext.log(ERROR, "failed to read rackup from '"+ rackup + "' (" + e + ")");
                     throw new RackInitializationException("failed to read rackup input", e);
                 }
             }

src/main/java/org/jruby/rack/DefaultRackDispatcher.java

@@ -11,6 +11,8 @@
 
 import org.jruby.rack.servlet.ServletRackContext;
 
+import static org.jruby.rack.RackLogger.Level.*;
+
 /**
  * Dispatcher suited for use in a servlet container
  * @author nick
@@ -50,10 +52,10 @@ protected void afterException(
         }
         catch (final RuntimeException re) {
             // allow the error app to re-throw Ruby/JRuby-Rack exceptions :
-            if (re instanceof RackException) throw (RackException) re;
+            if (re instanceof RackException) throw re;
             //if (e instanceof RaiseException) throw (RaiseException) e;
             // TODO seems redundant maybe we should let the container decide ?!
-            context.log(RackLogger.ERROR, "error app failed to handle exception: " + e, re);
+            context.log(ERROR, "error app failed to handle exception: " + e, re);
             Integer errorCode = getErrorApplicationFailureStatusCode();
             if ( errorCode != null && errorCode.intValue() > 0 ) {
                 response.sendError(errorCode);

src/main/java/org/jruby/rack/RackLogger.java

@@ -12,10 +12,6 @@
  * @author nicksieger
  */
 public interface RackLogger {
-
-    void log(String message) ;
-    void log(String message, Throwable ex) ;
-
     //void debug(String message) ;
     //void debug(String message, Throwable e) ;
 
@@ -37,44 +33,27 @@ enum Level {
     void log(Level level, String message) ;
     void log(Level level, String message, Throwable ex) ;
 
-    @Deprecated final String DEBUG = Level.DEBUG.name();
-    @Deprecated final String INFO = Level.INFO.name();
-    @Deprecated final String WARN = Level.WARN.name();
-    @Deprecated final String ERROR = Level.ERROR.name();
+    default void log(String message) {
+        log(Level.INFO, message);
+    }
 
-    void log(String level, String message) ;
-    void log(String level, String message, Throwable ex) ;
+    default void log(String message, Throwable ex) {
+        log(Level.ERROR, message, ex);
+    }
 
-    abstract class Base implements RackLogger {
+    default void log(String level, String message) {
+        log(Level.valueOf(level), message);
+    }
 
-        public abstract Level getLevel() ;
+    default void log(String level, String message, Throwable ex) {
+        log(Level.valueOf(level), message, ex);
+    }
 
+    abstract class Base implements RackLogger {
+        public abstract Level getLevel() ;
         public void setLevel(Level level) { /* noop */ }
 
         public boolean isFormatting() { return false; }
-
         public void setFormatting(boolean flag) { /* noop */ }
-
-        @Override
-        public void log(String message) {
-            log(Level.INFO, message);
-        }
-
-        @Override
-        public void log(String message, Throwable ex) {
-            log(Level.ERROR, message, ex);
-        }
-
-        @Override
-        public void log(String level, String message) {
-            log(Level.valueOf(level), message);
-        }
-
-        @Override
-        public void log(String level, String message, Throwable ex) {
-            log(Level.valueOf(level), message, ex);
-        }
-
     }
-
 }

src/main/java/org/jruby/rack/RackServletContextListener.java

@@ -16,6 +16,7 @@
 import org.jruby.rack.servlet.ServletRackContext;
 
 import static org.jruby.rack.DefaultRackConfig.isThrowInitException;
+import static org.jruby.rack.RackLogger.Level.*;
 
 /**
  * Web application lifecycle listener.
@@ -94,7 +95,7 @@ protected void handleInitializationException(
             throw RackInitializationException.wrap(e);
         }
         // NOTE: factory should have already logged the error ...
-        rackContext.log(RackLogger.ERROR, "initialization failed", e);
+        rackContext.log(ERROR, "initialization failed", e);
     }
 
 }

src/main/java/org/jruby/rack/servlet/DefaultServletRackContext.java

@@ -35,7 +35,6 @@
  *
  * @author nicksieger
  */
-@SuppressWarnings("rawtypes")
 public class DefaultServletRackContext implements ServletRackContext {
 
     private final RackConfig config;

src/main/java/org/jruby/rack/servlet/ServletRackContext.java

@@ -21,6 +21,13 @@ public interface ServletRackContext extends RackContext, ServletContext {
 
   RackApplicationFactory getRackFactory();
 
-  // ServletContext getRealContext(); // TODO support this in 1.2
+  @Override
+  default void log(String message) {
+    RackContext.super.log(message);
+  }
 
+  @Override
+  default void log(String message, Throwable ex) {
+    RackContext.super.log(message, ex);
+  }
 }

src/main/ruby/jruby/rack/booter.rb

@@ -85,10 +85,6 @@ def gem_path=(path); layout.gem_path = path end
     def public_path; layout.public_path end
     def public_path=(path); layout.public_path = path end
 
-    # @deprecated use {JRuby::Rack#logger} instead
-    # @return [Logger]
-    def logger; JRuby::Rack.logger; end
-
     # Boot-up this booter, preparing the environment for the application.
     def boot!
       adjust_gem_path
@@ -157,7 +153,7 @@ def env_gem_path
 
     # @note called during {#boot!}
     def export_global_settings
-      JRuby::Rack.send(:instance_variable_set, :@booter, self) # TODO
+      JRuby::Rack.instance_variable_set(:@booter, self) # NOTE: unused for now
       JRuby::Rack.app_path = layout.app_path
       JRuby::Rack.public_path = layout.public_path
     end
@@ -183,7 +179,7 @@ def load_settings_from_init_rb
             stream = url.openStream
             stream.to_io.read
           rescue Exception => e
-            logger.info "failed to read from '#{url.toString}' (#{e.message})"
+            JRuby::Rack.logger.info "failed to read from '#{url.toString}' (#{e.message})"
             next
           ensure
             stream.close rescue nil

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

@@ -14,47 +14,30 @@ module JRuby::Rack
   class Railtie < ::Rails::Railtie
 
     config.before_configuration do |app|
-      paths = app.config.paths; public = JRuby::Rack.public_path
-      if paths.respond_to?(:'[]') && paths.respond_to?(:keys)
-        # Rails 3.1/3.2/4.x: paths["app/controllers"] style
+      public = JRuby::Rack.public_path
+      if public # nil if /public does not exist
+        paths = app.config.paths
         old_public  = Pathname.new(paths['public'].to_a.first)
         javascripts = Pathname.new(paths['public/javascripts'].to_a.first)
         stylesheets = Pathname.new(paths['public/stylesheets'].to_a.first)
         paths['public'] = public.to_s; public = Pathname.new(public)
         paths['public/javascripts'] = public.join(javascripts.relative_path_from(old_public)).to_s
         paths['public/stylesheets'] = public.join(stylesheets.relative_path_from(old_public)).to_s
-      else
-        # Rails 3.0: old paths.app.controllers style
-        old_public  = Pathname.new(paths.public.to_a.first)
-        javascripts = Pathname.new(paths.public.javascripts.to_a.first)
-        stylesheets = Pathname.new(paths.public.stylesheets.to_a.first)
-        paths.public = public.to_s; public = Pathname.new(public)
-        paths.public.javascripts = public.join(javascripts.relative_path_from(old_public)).to_s
-        paths.public.stylesheets = public.join(stylesheets.relative_path_from(old_public)).to_s
-      end if public # nil if /public does not exist
+      end
     end
 
     # TODO prefix initializers with 'jruby_rack.' !?
 
     initializer 'set_servlet_logger', :before => :initialize_logger do |app|
       app.config.logger ||= begin
-        config = app.config; logger = JRuby::Rack.logger
+        logger = JRuby::Rack.logger
+        config = app.config
         log_level = config.log_level || :info
         logger.level = logger.class.const_get(log_level.to_s.upcase)
-        log_formatter = config.log_formatter if config.respond_to?(:log_formatter) # >= 4.0
+        log_formatter = config.log_formatter if config.respond_to?(:log_formatter)
         logger.formatter = log_formatter if log_formatter && logger.respond_to?(:formatter=)
-        if defined?(ActiveSupport::TaggedLogging)
-          if ActiveSupport::TaggedLogging.is_a?(Class) # Rails 3.2
-            logger = ActiveSupport::TaggedLogging.new(logger)
-          else # Rails 4.0
-            # extends the logger as well as it's logger.formatter instance :
-            # NOTE: good idea to keep or should we use a clone as Rails.logger ?
-            #dup_logger = logger.dup
-            #dup_logger.formatter = logger.formatter.dup
-            logger = ActiveSupport::TaggedLogging.new(logger)
-          end
-        end
-        logger
+        require 'active_support/tagged_logging' unless defined?(ActiveSupport::TaggedLogging)
+        ActiveSupport::TaggedLogging.new(logger) # returns a logger.clone
       end
     end
 

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

@@ -10,11 +10,14 @@
 
 describe JRuby::Rack::Booter do
 
-  let(:booter) do
-    JRuby::Rack::Booter.new JRuby::Rack.context = @rack_context
-  end
+  let(:context) { JRuby::Rack.context = @rack_context }
+
+  let(:booter) { JRuby::Rack::Booter.new(context) }
 
-  after(:all) { JRuby::Rack.context = nil }
+  after(:all) do
+    JRuby::Rack.context = nil
+    JRuby::Rack.instance_variable_set(:@booter, nil) if JRuby::Rack.instance_variable_get(:@booter)
+  end
 
   before do
     @rack_env = ENV['RACK_ENV']
@@ -155,14 +158,6 @@
     ENV['GEM_PATH'].should == "/blah/gems"
   end
 
-  it "creates a logger that writes messages to the servlet context (by default)" do
-    booter.boot!
-    @rack_context.stub(:isEnabled).and_return true
-    level = org.jruby.rack.RackLogger::Level::DEBUG
-    @rack_context.should_receive(:log).with(level, 'Hello-JRuby!')
-    booter.logger.debug 'Hello-JRuby!'
-  end
-
   before { $loaded_init_rb = nil }
 
   it "loads and executes ruby code in META-INF/init.rb if it exists" do

src/spec/ruby/rack/embed/context_spec.rb

@@ -17,7 +17,7 @@
   end
 
   it "outputs log messages with level and new line to stdout" do
-    info = org.jruby.rack.embed.Context::INFO
+    info = org.jruby.rack.RackLogger::Level::INFO
     context.log info, "this is logging at its best"
     captured.should == "INFO: this is logging at its best\n"
   end

src/spec/ruby/spec_helper.rb

@@ -21,8 +21,8 @@
 
 require 'rspec'
 
-require 'jruby'; ext_class = org.jruby.rack.ext.RackLibrary
-JRuby.runtime.loadExtension 'JRuby::Rack', ext_class.new, true
+require 'jruby' # we rely on JRuby.runtime in a few places
+JRuby::Util.load_ext('org.jruby.rack.ext.RackLibrary')
 
 module SharedHelpers
 
@@ -170,7 +170,7 @@ def should_not_eval_as_nil(code, runtime = @runtime) # alias
 rescue LoadError
 end
 
-# current 'library' environment (based on appraisals) e.g. :rails32
+# current 'library' environment (based on appraisals) e.g. :rails72
 CURRENT_LIB = defined?(Rails::VERSION) ?
   :"rails#{Rails::VERSION::MAJOR}#{Rails::VERSION::MINOR}" : :stub