#93: only initialize Rhino engine if necessary (#94)

Additionally the Regex class was refactored now following the strategy pattern to prevent if / else on every method call. That refactoring also helped solving the issue. Also add coverage to the class as the Rhino engine has not been unit-tested before if the build has been done with Java 8 - 14.
java-json-tools · Sep 21, 2020 · 99a7ad6 · 99a7ad6
1 parent 5d443f0
commit 99a7ad6
Show file tree

Hide file tree

Showing 2 changed files with 158 additions and 77 deletions.
diff --git a/src/main/java/com/github/fge/jsonschema/core/util/RegexECMA262Helper.java b/src/main/java/com/github/fge/jsonschema/core/util/RegexECMA262Helper.java
@@ -50,9 +50,9 @@
  * expressions", just <a href="http://regex.info">buy it</a> :p</p>
  *
  * <p>As script engine is used either Nashorn or Rhino as its fallback.
- * Nashorn is only available on Java 8 runtimes or higher.</p>
+ * Nashorn is only available on Java 8 up to 14.</p>
  *
- * <p>Rhino is only the fallback as it is tremendously slower.</p>
+ * <p>Rhino is the fallback as it is tremendously slower.</p>
  */
 @ThreadSafe
 public final class RegexECMA262Helper
@@ -62,8 +62,8 @@ public final class RegexECMA262Helper
     private static final String REG_MATCH_FUNCTION_NAME = "regMatch";
 
     /**
-     * JavaScript scriptlet defining functions {@link #REGEX_IS_VALID}
-     * and {@link #REG_MATCH}
+     * JavaScript scriptlet defining functions for validating a regular
+     * expression and for matching an input against a regular expression.
      */
     private static final String jsAsString
         = "function " + REGEX_IS_VALID_FUNCTION_NAME + "(re)"
@@ -81,59 +81,20 @@ public final class RegexECMA262Helper
         + "    return new RegExp(re).test(input);"
         + '}';
 
-    /**
-     * Script scope
-     */
-    private static final Scriptable SCOPE;
-
-    /**
-     * Reference to Javascript function for regex validation
-     */
-    private static final Function REGEX_IS_VALID;
-
-    /**
-     * Reference to Javascript function for regex matching
-     */
-    private static final Function REG_MATCH;
-
-    private static final Invocable PRIMARY_SCRIPT_ENGINE;
+    private static final RegexScript REGEX_SCRIPT = determineRegexScript();
 
     private RegexECMA262Helper()
     {
     }
 
-    static {
-        PRIMARY_SCRIPT_ENGINE = tryResolvePrimaryEngine();
-
-        final Context ctx = Context.enter();
+    private static RegexScript determineRegexScript()
+    {
         try {
-            SCOPE = ctx.initStandardObjects(null, false);
-            try {
-                ctx.evaluateString(SCOPE, jsAsString, "re", 1, null);
-            } catch(UnsupportedOperationException e) {
-                // See: http://stackoverflow.com/questions/3859305/problems-using-rhino-on-android
-                ctx.setOptimizationLevel(-1);
-                ctx.evaluateString(SCOPE, jsAsString, "re", 1, null);
-            }
-            REGEX_IS_VALID = (Function) SCOPE.get(REGEX_IS_VALID_FUNCTION_NAME, SCOPE);
-            REG_MATCH = (Function) SCOPE.get(REG_MATCH_FUNCTION_NAME, SCOPE);
-        } finally {
-            Context.exit();
-        }
-    }
-
-    private static Invocable tryResolvePrimaryEngine() {
-        final ScriptEngine engine = new ScriptEngineManager()
-                .getEngineByName("nashorn");
-        if(engine != null) {
-            try {
-                engine.eval(jsAsString);
-                return (Invocable) engine;
-            } catch(final ScriptException e) {
-                // the script can't be parsed - the script engine can't be used
-            }
+            return new NashornScript();
+        } catch(final ScriptException e) {
+            // either Nashorn is not available or the JavaScript can't be parsed
         }
-        return null;
+        return new RhinoScript();
     }
 
     /**
@@ -144,11 +105,7 @@ private static Invocable tryResolvePrimaryEngine() {
      */
     public static boolean regexIsValid(final String regex)
     {
-        if(PRIMARY_SCRIPT_ENGINE != null)
-        {
-            return invokeScriptEngine(REGEX_IS_VALID_FUNCTION_NAME, regex);
-        }
-        return invokeFallbackEngine(REGEX_IS_VALID, regex);
+        return REGEX_SCRIPT.regexIsValid(regex);
     }
 
     /**
@@ -167,36 +124,121 @@ public static boolean regexIsValid(final String regex)
      */
     public static boolean regMatch(final String regex, final String input)
     {
-        if(PRIMARY_SCRIPT_ENGINE != null)
+        return REGEX_SCRIPT.regMatch(regex, input);
+    }
+
+    private interface RegexScript
+    {
+        boolean regexIsValid(String regex);
+
+        boolean regMatch(String regex, String input);
+    }
+
+    private static class NashornScript implements RegexScript
+    {
+        /**
+         * Script engine
+         */
+        private final Invocable scriptEngine;
+
+        private NashornScript() throws ScriptException
+        {
+            final ScriptEngine engine = new ScriptEngineManager()
+                    .getEngineByName("nashorn");
+            if (engine == null) {
+                throw new ScriptException("ScriptEngine 'nashorn' not found.");
+            }
+            engine.eval(jsAsString);
+            this.scriptEngine = (Invocable) engine;
+        }
+
+        private boolean invokeScriptEngine(final String function,
+                                           final Object... values)
+        {
+            try {
+                return (Boolean) scriptEngine.invokeFunction(function,
+                        values);
+            } catch(final ScriptException e) {
+                throw new IllegalStateException(
+                        "Unexpected error on invoking Script.", e);
+            } catch(final NoSuchMethodException e) {
+                throw new IllegalStateException(
+                        "Unexpected error on invoking Script.", e);
+            }
+        }
+
+        @Override
+        public boolean regexIsValid(final String regex)
+        {
+            return invokeScriptEngine(REGEX_IS_VALID_FUNCTION_NAME, regex);
+        }
+
+        @Override
+        public boolean regMatch(final String regex, final String input)
         {
             return invokeScriptEngine(REG_MATCH_FUNCTION_NAME, regex, input);
         }
-        return invokeFallbackEngine(REG_MATCH, regex, input);
     }
 
-    private static boolean invokeScriptEngine(final String function,
-                                              final Object... values)
+    private static class RhinoScript implements RegexScript
     {
-        try {
-            return (Boolean) PRIMARY_SCRIPT_ENGINE.invokeFunction(function,
-                    values);
-        } catch(final ScriptException e) {
-            throw new IllegalStateException(
-                    "Unexpected error on invoking Script.", e);
-        } catch(final NoSuchMethodException e) {
-            throw new IllegalStateException(
-                    "Unexpected error on invoking Script.", e);
+        /**
+         * Script scope
+         */
+        private final Scriptable scope;
+
+        /**
+         * Reference to Javascript function for regex validation
+         */
+        private final Function regexIsValid;
+
+        /**
+         * Reference to Javascript function for regex matching
+         */
+        private final Function regMatch;
+
+        private RhinoScript()
+        {
+            final Context ctx = Context.enter();
+            try {
+                this.scope = ctx.initStandardObjects(null, false);
+                try {
+                    ctx.evaluateString(scope, jsAsString, "re", 1, null);
+                } catch(final UnsupportedOperationException e) {
+                    // See: http://stackoverflow.com/questions/3859305/problems-using-rhino-on-android
+                    ctx.setOptimizationLevel(-1);
+                    ctx.evaluateString(scope, jsAsString, "re", 1, null);
+                }
+                this.regexIsValid = (Function)
+                        scope.get(REGEX_IS_VALID_FUNCTION_NAME, scope);
+                this.regMatch = (Function)
+                        scope.get(REG_MATCH_FUNCTION_NAME, scope);
+            } finally {
+                Context.exit();
+            }
         }
-    }
 
-    private static boolean invokeFallbackEngine(final Function function,
-                                                final Object... values)
-    {
-        final Context context = Context.enter();
-        try {
-            return (Boolean) function.call(context, SCOPE, SCOPE, values);
-        } finally {
-            Context.exit();
+        private boolean invokeScriptEngine(final Function function,
+                                           final Object... values)
+        {
+            final Context context = Context.enter();
+            try {
+                return (Boolean) function.call(context, scope, scope, values);
+            } finally {
+                Context.exit();
+            }
+        }
+
+        @Override
+        public boolean regexIsValid(final String regex)
+        {
+            return invokeScriptEngine(regexIsValid, regex);
+        }
+
+        @Override
+        public boolean regMatch(final String regex, final String input)
+        {
+            return invokeScriptEngine(regMatch, regex, input);
         }
     }
 }
diff --git a/src/test/java/com/github/fge/jsonschema/core/util/RegexECMA262HelperTest.java b/src/test/java/com/github/fge/jsonschema/core/util/RegexECMA262HelperTest.java
@@ -23,12 +23,49 @@
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Method;
 import java.util.Iterator;
 
 import static org.testng.Assert.*;
 
 public final class RegexECMA262HelperTest
 {
+    private static Object rhinoScriptInstance;
+    private static Method regexIsValid;
+    private static Method regMatch;
+
+    /**
+     * Depending on the JDK version the Rhino fallback engine will
+     * not be tested. To ensure functionality for Rhino, too, prepare
+     * tests for it via reflection.
+     */
+    static
+    {
+        try {
+            final Class<?> rhinoScriptClass = Class.forName(
+                    RegexECMA262Helper.class.getName() + "$RhinoScript");
+            final Constructor<?> constructor = rhinoScriptClass.getDeclaredConstructor();
+            constructor.setAccessible(true);
+            rhinoScriptInstance = constructor.newInstance();
+            regexIsValid = rhinoScriptInstance.getClass()
+                    .getDeclaredMethod("regexIsValid", String.class);
+            regMatch = rhinoScriptInstance.getClass()
+                    .getDeclaredMethod("regMatch", String.class, String.class);
+        } catch (final Exception e) {
+            throw new IllegalStateException("Can't initialize RhinoScript.", e);
+        }
+    }
+
+    private static boolean rhinoScript(final Method method, final Object... args)
+    {
+        try {
+            return (boolean) method.invoke(rhinoScriptInstance, args);
+        } catch (final Exception e) {
+            throw new IllegalStateException("Can't invoke method on RhinoScript.");
+        }
+    }
+
     @DataProvider
     public Iterator<Object[]> ecma262regexes()
     {
@@ -51,6 +88,7 @@ public void regexesAreCorrectlyAnalyzed(final String regex,
     {
         assertEquals(RegexECMA262Helper.regexIsValid(regex), valid);
         assertEquals(RhinoHelper.regexIsValid(regex), valid);
+        assertEquals(rhinoScript(regexIsValid, regex), valid);
     }
 
     @DataProvider
@@ -77,5 +115,6 @@ public void regexMatchingIsDoneCorrectly(final String regex,
     {
         assertEquals(RegexECMA262Helper.regMatch(regex, input), valid);
         assertEquals(RhinoHelper.regMatch(regex, input), valid);
+        assertEquals(rhinoScript(regMatch, regex, input), valid);
     }
 }