8264326: Modernize javax.script.ScriptEngineManager and related classes' implementation

Reviewed-by: sundar

Author: szegedi

src/java.scripting/share/classes/javax/script/AbstractScriptEngine.java

@@ -25,8 +25,6 @@
 
 package javax.script;
 import java.io.Reader;
-import java.util.Map;
-import java.util.Iterator;
 
 /**
  * Provides a standard implementation for several of the variants of the <code>eval</code>
@@ -141,9 +139,9 @@ public Bindings getBindings(int scope) {
     public void setBindings(Bindings bindings, int scope) {
 
         if (scope == ScriptContext.GLOBAL_SCOPE) {
-            context.setBindings(bindings, ScriptContext.GLOBAL_SCOPE);;
+            context.setBindings(bindings, ScriptContext.GLOBAL_SCOPE);
         } else if (scope == ScriptContext.ENGINE_SCOPE) {
-            context.setBindings(bindings, ScriptContext.ENGINE_SCOPE);;
+            context.setBindings(bindings, ScriptContext.ENGINE_SCOPE);
         } else {
             throw new IllegalArgumentException("Invalid scope value.");
         }

src/java.scripting/share/classes/javax/script/ScriptEngineManager.java

@@ -28,6 +28,8 @@
 import java.security.*;
 import java.util.ServiceLoader;
 import java.util.ServiceConfigurationError;
+import java.util.function.Function;
+import java.util.stream.Stream;
 
 /**
  * The <code>ScriptEngineManager</code> implements a discovery and instantiation
@@ -58,8 +60,7 @@ public class ScriptEngineManager  {
      * @see java.lang.Thread#getContextClassLoader
      */
     public ScriptEngineManager() {
-        ClassLoader ctxtLoader = Thread.currentThread().getContextClassLoader();
-        init(ctxtLoader);
+        this(Thread.currentThread().getContextClassLoader());
     }
 
     /**
@@ -72,18 +73,6 @@ public ScriptEngineManager() {
      * @param loader ClassLoader used to discover script engine factories.
      */
     public ScriptEngineManager(ClassLoader loader) {
-        init(loader);
-    }
-
-    private void init(final ClassLoader loader) {
-        globalScope = new SimpleBindings();
-        engineSpis = new TreeSet<ScriptEngineFactory>(Comparator.comparing(
-            ScriptEngineFactory::getEngineName,
-            Comparator.nullsLast(Comparator.naturalOrder()))
-        );
-        nameAssociations = new HashMap<String, ScriptEngineFactory>();
-        extensionAssociations = new HashMap<String, ScriptEngineFactory>();
-        mimeTypeAssociations = new HashMap<String, ScriptEngineFactory>();
         initEngines(loader);
     }
 
@@ -96,23 +85,13 @@ private ServiceLoader<ScriptEngineFactory> getServiceLoader(final ClassLoader lo
     }
 
     private void initEngines(final ClassLoader loader) {
-        Iterator<ScriptEngineFactory> itr = null;
+        Iterator<ScriptEngineFactory> itr;
         try {
-            ServiceLoader<ScriptEngineFactory> sl = AccessController.doPrivileged(
-                new PrivilegedAction<ServiceLoader<ScriptEngineFactory>>() {
-                    @Override
-                    public ServiceLoader<ScriptEngineFactory> run() {
-                        return getServiceLoader(loader);
-                    }
-                });
-
+            var sl = AccessController.doPrivileged(
+                (PrivilegedAction<ServiceLoader<ScriptEngineFactory>>)() -> getServiceLoader(loader));
             itr = sl.iterator();
         } catch (ServiceConfigurationError err) {
-            System.err.println("Can't find ScriptEngineFactory providers: " +
-                          err.getMessage());
-            if (DEBUG) {
-                err.printStackTrace();
-            }
+            reportException("Can't find ScriptEngineFactory providers: ", err);
             // do not throw any exception here. user may want to
             // manage his/her own factories using this manager
             // by explicit registratation (by registerXXX) methods.
@@ -125,25 +104,15 @@ public ServiceLoader<ScriptEngineFactory> run() {
                     ScriptEngineFactory fact = itr.next();
                     engineSpis.add(fact);
                 } catch (ServiceConfigurationError err) {
-                    System.err.println("ScriptEngineManager providers.next(): "
-                                 + err.getMessage());
-                    if (DEBUG) {
-                        err.printStackTrace();
-                    }
+                    reportException("ScriptEngineManager providers.next(): ", err);
                     // one factory failed, but check other factories...
-                    continue;
                 }
             }
         } catch (ServiceConfigurationError err) {
-            System.err.println("ScriptEngineManager providers.hasNext(): "
-                            + err.getMessage());
-            if (DEBUG) {
-                err.printStackTrace();
-            }
+            reportException("ScriptEngineManager providers.hasNext(): ", err);
             // do not throw any exception here. user may want to
             // manage his/her own factories using this manager
             // by explicit registratation (by registerXXX) methods.
-            return;
         }
     }
 
@@ -212,44 +181,7 @@ public Object get(String key) {
      * @throws NullPointerException if shortName is null.
      */
     public ScriptEngine getEngineByName(String shortName) {
-        if (shortName == null) throw new NullPointerException();
-        //look for registered name first
-        Object obj;
-        if (null != (obj = nameAssociations.get(shortName))) {
-            ScriptEngineFactory spi = (ScriptEngineFactory)obj;
-            try {
-                ScriptEngine engine = spi.getScriptEngine();
-                engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
-                return engine;
-            } catch (Exception exp) {
-                if (DEBUG) exp.printStackTrace();
-            }
-        }
-
-        for (ScriptEngineFactory spi : engineSpis) {
-            List<String> names = null;
-            try {
-                names = spi.getNames();
-            } catch (Exception exp) {
-                if (DEBUG) exp.printStackTrace();
-            }
-
-            if (names != null) {
-                for (String name : names) {
-                    if (shortName.equals(name)) {
-                        try {
-                            ScriptEngine engine = spi.getScriptEngine();
-                            engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
-                            return engine;
-                        } catch (Exception exp) {
-                            if (DEBUG) exp.printStackTrace();
-                        }
-                    }
-                }
-            }
-        }
-
-        return null;
+        return getEngineBy(shortName, nameAssociations, ScriptEngineFactory::getNames);
     }
 
     /**
@@ -263,41 +195,7 @@ public ScriptEngine getEngineByName(String shortName) {
      * @throws NullPointerException if extension is null.
      */
     public ScriptEngine getEngineByExtension(String extension) {
-        if (extension == null) throw new NullPointerException();
-        //look for registered extension first
-        Object obj;
-        if (null != (obj = extensionAssociations.get(extension))) {
-            ScriptEngineFactory spi = (ScriptEngineFactory)obj;
-            try {
-                ScriptEngine engine = spi.getScriptEngine();
-                engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
-                return engine;
-            } catch (Exception exp) {
-                if (DEBUG) exp.printStackTrace();
-            }
-        }
-
-        for (ScriptEngineFactory spi : engineSpis) {
-            List<String> exts = null;
-            try {
-                exts = spi.getExtensions();
-            } catch (Exception exp) {
-                if (DEBUG) exp.printStackTrace();
-            }
-            if (exts == null) continue;
-            for (String ext : exts) {
-                if (extension.equals(ext)) {
-                    try {
-                        ScriptEngine engine = spi.getScriptEngine();
-                        engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
-                        return engine;
-                    } catch (Exception exp) {
-                        if (DEBUG) exp.printStackTrace();
-                    }
-                }
-            }
-        }
-        return null;
+        return getEngineBy(extension, extensionAssociations, ScriptEngineFactory::getExtensions);
     }
 
     /**
@@ -311,41 +209,52 @@ public ScriptEngine getEngineByExtension(String extension) {
      * @throws NullPointerException if mimeType is null.
      */
     public ScriptEngine getEngineByMimeType(String mimeType) {
-        if (mimeType == null) throw new NullPointerException();
-        //look for registered types first
-        Object obj;
-        if (null != (obj = mimeTypeAssociations.get(mimeType))) {
-            ScriptEngineFactory spi = (ScriptEngineFactory)obj;
-            try {
-                ScriptEngine engine = spi.getScriptEngine();
-                engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
-                return engine;
-            } catch (Exception exp) {
-                if (DEBUG) exp.printStackTrace();
-            }
-        }
+        return getEngineBy(mimeType, mimeTypeAssociations, ScriptEngineFactory::getMimeTypes);
+    }
 
-        for (ScriptEngineFactory spi : engineSpis) {
-            List<String> types = null;
-            try {
-                types = spi.getMimeTypes();
-            } catch (Exception exp) {
-                if (DEBUG) exp.printStackTrace();
-            }
-            if (types == null) continue;
-            for (String type : types) {
-                if (mimeType.equals(type)) {
-                    try {
-                        ScriptEngine engine = spi.getScriptEngine();
-                        engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
-                        return engine;
-                    } catch (Exception exp) {
-                        if (DEBUG) exp.printStackTrace();
-                    }
+    private ScriptEngine getEngineBy(String selector, Map<String, ScriptEngineFactory> associations,
+        Function<ScriptEngineFactory, List<String>> valuesFn)
+    {
+        Objects.requireNonNull(selector);
+        Stream<ScriptEngineFactory> spis = Stream.concat(
+            //look for registered types first
+            Stream.ofNullable(associations.get(selector)),
+
+            engineSpis.stream().filter(spi -> {
+                try {
+                    List<String> matches = valuesFn.apply(spi);
+                    return matches != null && matches.contains(selector);
+                } catch (Exception exp) {
+                    debugPrint(exp);
+                    return false;
                 }
-            }
+            })
+        );
+        return spis
+            .map(spi -> {
+                try {
+                    ScriptEngine engine = spi.getScriptEngine();
+                    engine.setBindings(getBindings(), ScriptContext.GLOBAL_SCOPE);
+                    return engine;
+                } catch (Exception exp) {
+                    debugPrint(exp);
+                    return null;
+                }
+            })
+            .filter(Objects::nonNull)
+            .findFirst()
+            .orElse(null);
+    }
+
+    private static void reportException(String msg, Throwable exp) {
+        System.err.println(msg + exp.getMessage());
+        debugPrint(exp);
+    }
+
+    private static void debugPrint(Throwable exp) {
+        if (DEBUG) {
+            exp.printStackTrace();
         }
-        return null;
     }
 
     /**
@@ -354,11 +263,7 @@ public ScriptEngine getEngineByMimeType(String mimeType) {
      * @return List of all discovered <code>ScriptEngineFactory</code>s.
      */
     public List<ScriptEngineFactory> getEngineFactories() {
-        List<ScriptEngineFactory> res = new ArrayList<ScriptEngineFactory>(engineSpis.size());
-        for (ScriptEngineFactory spi : engineSpis) {
-            res.add(spi);
-        }
-        return Collections.unmodifiableList(res);
+        return List.copyOf(engineSpis);
     }
 
     /**
@@ -369,8 +274,7 @@ public List<ScriptEngineFactory> getEngineFactories() {
      * @throws NullPointerException if any of the parameters is null.
      */
     public void registerEngineName(String name, ScriptEngineFactory factory) {
-        if (name == null || factory == null) throw new NullPointerException();
-        nameAssociations.put(name, factory);
+        associateFactory(nameAssociations, name, factory);
     }
 
     /**
@@ -384,8 +288,7 @@ public void registerEngineName(String name, ScriptEngineFactory factory) {
      * @throws NullPointerException if any of the parameters is null.
      */
     public void registerEngineMimeType(String type, ScriptEngineFactory factory) {
-        if (type == null || factory == null) throw new NullPointerException();
-        mimeTypeAssociations.put(type, factory);
+        associateFactory(mimeTypeAssociations, type, factory);
     }
 
     /**
@@ -398,22 +301,33 @@ public void registerEngineMimeType(String type, ScriptEngineFactory factory) {
      * @throws NullPointerException if any of the parameters is null.
      */
     public void registerEngineExtension(String extension, ScriptEngineFactory factory) {
-        if (extension == null || factory == null) throw new NullPointerException();
-        extensionAssociations.put(extension, factory);
+        associateFactory(extensionAssociations, extension, factory);
     }
 
+    private static void associateFactory(Map<String, ScriptEngineFactory> associations, String association,
+        ScriptEngineFactory factory)
+    {
+        if (association == null || factory == null) throw new NullPointerException();
+        associations.put(association, factory);
+    }
+
+    private static final Comparator<ScriptEngineFactory> COMPARATOR = Comparator.comparing(
+        ScriptEngineFactory::getEngineName,
+        Comparator.nullsLast(Comparator.naturalOrder())
+    );
+
     /** Set of script engine factories discovered. */
-    private TreeSet<ScriptEngineFactory> engineSpis;
+    private final TreeSet<ScriptEngineFactory> engineSpis = new TreeSet<>(COMPARATOR);
 
     /** Map of engine name to script engine factory. */
-    private HashMap<String, ScriptEngineFactory> nameAssociations;
+    private final HashMap<String, ScriptEngineFactory> nameAssociations = new HashMap<>();
 
     /** Map of script file extension to script engine factory. */
-    private HashMap<String, ScriptEngineFactory> extensionAssociations;
+    private final HashMap<String, ScriptEngineFactory> extensionAssociations = new HashMap<>();
 
     /** Map of script MIME type to script engine factory. */
-    private HashMap<String, ScriptEngineFactory> mimeTypeAssociations;
+    private final HashMap<String, ScriptEngineFactory> mimeTypeAssociations = new HashMap<>();
 
     /** Global bindings associated with script engines created by this manager. */
-    private Bindings globalScope;
+    private Bindings globalScope = new SimpleBindings();
 }

src/java.scripting/share/classes/javax/script/ScriptException.java

@@ -38,9 +38,9 @@ public class ScriptException extends Exception {
 
     private static final long serialVersionUID = 8265071037049225001L;
 
-    private String fileName;
-    private int lineNumber;
-    private int columnNumber;
+    private final String fileName;
+    private final int lineNumber;
+    private final int columnNumber;
 
     /**
      * Creates a <code>ScriptException</code> with a String to be used in its message.