Merge pull request #288 from kares/more-cleanup-1.3

small behavioral changes and cleanup

Author: kares

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

@@ -11,8 +11,6 @@
 import java.io.File;
 import java.io.IOException;
 import java.io.InputStream;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
 import java.net.URISyntaxException;
 import java.net.URL;
 import java.util.Map;
@@ -99,11 +97,7 @@ public void init(final RackContext rackContext) {
      */
     @Override
     public RackApplication newApplication() {
-        return createApplication(new ApplicationObjectFactory() {
-            public IRubyObject create(Ruby runtime) {
-                return createApplicationObject(runtime);
-            }
-        });
+        return createApplication(this::createApplicationObject);
     }
 
     /**
@@ -209,13 +203,7 @@ public RackApplication newErrorApplication() {
             return new DefaultErrorApplication(rackContext);
         }
         try {
-            RackApplication app = createErrorApplication(
-                new ApplicationObjectFactory() {
-                    public IRubyObject create(Ruby runtime) {
-                        return createErrorApplicationObject(runtime);
-                    }
-                }
-            );
+            RackApplication app = createErrorApplication(this::createErrorApplicationObject);
             app.init();
             return app;
         }
@@ -250,7 +238,7 @@ protected IRubyObject createRackServletWrapper(Ruby runtime, String rackup, Stri
         );
     }
 
-    static interface ApplicationObjectFactory {
+    interface ApplicationObjectFactory {
         IRubyObject create(Ruby runtime) ;
     }
 
@@ -267,21 +255,7 @@ protected RubyInstanceConfig initRuntimeConfig(final RubyInstanceConfig config)
         // Don't affect the container and sibling web apps when ENV changes are
         // made inside the Ruby app ...
         // There are quite a such things made in a typical Bundler based app.
-        try { // config.setUpdateNativeENVEnabled(false) using reflection :
-            final Method setUpdateNativeENVEnabled =
-                config.getClass().getMethod("setUpdateNativeENVEnabled", Boolean.TYPE);
-            setUpdateNativeENVEnabled.invoke(config, false);
-        }
-        catch (NoSuchMethodException e) { // ignore method has been added in JRuby 1.6.7
-            rackContext.log(DEBUG, "envronment changes made inside one app " +
-            "might affect another, consider updating JRuby if this is an issue");
-        }
-        catch (IllegalAccessException e) {
-            rackContext.log(WARN, "failed to disable updating native environment", e);
-        }
-        catch (InvocationTargetException e) {
-            throw new RackException(e.getTargetException());
-        }
+        config.setUpdateNativeENVEnabled(false);
 
         final Map<String, String> newEnv = rackConfig.getRuntimeEnvironment();
         if ( newEnv != null ) {
@@ -297,7 +271,6 @@ protected RubyInstanceConfig initRuntimeConfig(final RubyInstanceConfig config)
             else {
                 // allow to work (backwards) "compatibly" with previous `ENV.clear`
                 // RUBYOPT was processed since it happens on config.processArguments
-                @SuppressWarnings("unchecked")
                 final Map<String, String> env = config.getEnvironment();
                 if ( env != null && env.containsKey("RUBYOPT") ) {
                     newEnv.put( "RUBYOPT", env.get("RUBYOPT") );
@@ -355,9 +328,7 @@ protected void loadJRubyRack(final Ruby runtime) {
     public void initRuntime(final Ruby runtime) {
         loadJRubyRack(runtime);
         // set $servlet_context :
-        runtime.getGlobalVariables().set(
-            "$servlet_context", JavaUtil.convertJavaToRuby(runtime, rackContext)
-        );
+        runtime.getGlobalVariables().set("$servlet_context", JavaUtil.convertJavaToRuby(runtime, rackContext));
         // load our (servlet) Rack handler :
         runtime.evalScriptlet("require 'rack/handler/servlet'");
 
@@ -411,9 +382,6 @@ public String checkAndSetRackVersion(final Ruby runtime) {
             rackContext.log(DEBUG, "could not read 'rack.version' magic comment from rackup", e);
         }
 
-        if ( rackVersion == null ) {
-            // NOTE: try matching a `require 'bundler/setup'` line ... maybe not ?!
-        }
         if ( rackVersion != null ) {
             runtime.evalScriptlet("require 'rubygems'");
 
@@ -462,10 +430,22 @@ public void destroy() {
             runtime.tearDown(false);
         }
 
+        private void captureMessage(final RaiseException re) {
+            try {
+                IRubyObject rubyException = re.getException();
+                ThreadContext context = rubyException.getRuntime().getCurrentContext();
+                // JRuby-Rack internals (@see jruby/rack/capture.rb) :
+                rubyException.callMethod(context, "capture");
+                rubyException.callMethod(context, "store");
+            }
+            catch (Exception e) {
+                rackContext.log(INFO, "failed to capture exception message", e);
+                // won't be able to capture anything
+            }
+        }
     }
 
     private RackApplication createErrorApplication(final ApplicationObjectFactory appFactory) {
-        // final Ruby runtime = newRuntime();
         return new ErrorApplicationImpl(appFactory);
     }
 
@@ -482,29 +462,13 @@ public void init() {
 
     }
 
-    private void captureMessage(final RaiseException re) {
-        try {
-            IRubyObject rubyException = re.getException();
-            ThreadContext context = rubyException.getRuntime().getCurrentContext();
-            // JRuby-Rack internals (@see jruby/rack/capture.rb) :
-            rubyException.callMethod(context, "capture");
-            rubyException.callMethod(context, "store");
-        }
-        catch (Exception e) {
-            rackContext.log(INFO, "failed to capture exception message", e);
-            // won't be able to capture anything
-        }
-    }
-
     private String findConfigRuPathInSubDirectories(final String path, int level) {
-        @SuppressWarnings("unchecked")
         final Set<String> entries = rackContext.getResourcePaths(path);
         if (entries != null) {
             String config_ru = path + "config.ru";
             if ( entries.contains(config_ru) ) {
                 return config_ru;
             }
-
             if (level > 0) {
                 level--;
                 for ( String subpath : entries ) {
@@ -526,11 +490,9 @@ private static String getContextLoaderScript(final String name, final boolean si
             InputStream is = contextLoader.getResourceAsStream(name);
             return IOHelpers.inputStreamToString(is);
         }
-        catch (IOException e) {
-            if ( silent ) return null; throw e;
-        }
-        catch (RuntimeException e) {
-            if ( silent ) return null; throw e;
+        catch (IOException|RuntimeException e) {
+            if ( silent ) return null;
+            throw e;
         }
     }
 
@@ -565,7 +527,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/DefaultRackConfig.java

@@ -253,18 +253,11 @@ public Map<String, String> getRuntimeEnvironment() {
         if ( env == null ) env = getProperty("jruby.runtime.environment");
         final Object envFlag = toStrictBoolean(env, null);
         if ( envFlag != null ) {
-            boolean keep = ((Boolean) envFlag).booleanValue();
             // jruby.runtime.env = true keep as is (return null)
             // jruby.runtime.env = false clear env (return empty)
-            //return keep ? null : new HashMap<String, String>();
-            if ( keep ) {
-                return new HashMap<String, String>(System.getenv());
-            }
-            else {
-                return new HashMap<String, String>();
-            }
+            return (Boolean) envFlag ? new HashMap<>(System.getenv()) : new HashMap<>();
         }
-        if ( isIgnoreEnvironment() ) return new HashMap<String, String>();
+        if ( isIgnoreEnvironment() ) return new HashMap<>();
         // TODO maybe support custom value 'servlet' to use init params ?
         return toStringMap(env);
     }
@@ -289,14 +282,14 @@ public boolean isThrowInitException() {
 
     static boolean isThrowInitException(RackConfig config) {
         Boolean error = config.getBooleanProperty("jruby.rack.error");
-        if ( error != null && ! error.booleanValue() ) {
-            return true; // jruby.rack.error = false
+        if ( error != null && error.booleanValue() ) {
+            return false; // jruby.rack.error = true
         }
-        error = config.getBooleanProperty("jruby.rack.exception");
-        if ( error != null && ! error.booleanValue() ) {
-            return true; // jruby.rack.exception = false
+        error = config.getBooleanProperty(RackEnvironment.EXCEPTION);
+        if ( error != null && error.booleanValue() ) {
+            return false; // jruby.rack.exception = true
         }
-        return false;
+        return true;
     }
 
     @Override
@@ -421,7 +414,7 @@ private Map<String, String> toStringMap(final String env) {
     }
 
     private static Map<String,String> getLoggerTypes() {
-        final Map<String,String> loggerTypes = new HashMap<String, String>(8);
+        final Map<String,String> loggerTypes = new HashMap<>(8);
         loggerTypes.put("commons_logging", "org.jruby.rack.logging.CommonsLoggingLogger");
         loggerTypes.put("clogging", "org.jruby.rack.logging.CommonsLoggingLogger");
         loggerTypes.put("slf4j", "org.jruby.rack.logging.Slf4jLogger");

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/RackException.java

@@ -13,7 +13,6 @@
  * 
  * @author kares
  */
-@SuppressWarnings("serial")
 public class RackException extends RuntimeException {
 
     public RackException(String message) {
@@ -57,9 +56,7 @@ static String exceptionMessage(final RaiseException e) {
             st.append(b.toString());
             return st.toString();
         }
-        else {
-            return null;
-        }
+        return null;
     }
 
 }

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);
-        }
-
     }
-
 }