Extract EvaluatebleScriptAdapter

Precursor for InvocableScriptAdapter, which compiles/evaluates a script without affecting the engine's bindings. This allows the same script to be compiled once and then evaluated multiple times (with the same ScriptEngine).

authz/policy/common/src/main/java/org/keycloak/authorization/policy/provider/js/JSPolicyProvider.java

@@ -23,7 +23,7 @@ import org.keycloak.authorization.policy.evaluation.Evaluation;
 import org.keycloak.authorization.policy.provider.PolicyProvider;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.ScriptModel;
-import org.keycloak.scripting.InvocableScriptAdapter;
+import org.keycloak.scripting.EvaluatableScriptAdapter;
 import org.keycloak.scripting.ScriptingProvider;
 
 /**
@@ -35,9 +35,18 @@ public class JSPolicyProvider implements PolicyProvider {
     public void evaluate(Evaluation evaluation) {
         Policy policy = evaluation.getPolicy();
 
+        AuthorizationProvider authorization = evaluation.getAuthorizationProvider();
+        ScriptModel script = getScriptModel(policy, authorization);
+        final EvaluatableScriptAdapter adapter = getScriptingProvider(authorization).prepareEvaluatableScript(script);
+
         try {
-            getInvocableScriptAdapter(policy, evaluation).eval();
+            //how to deal with long running scripts -> timeout?
-        } catch (Exception e) {
+            adapter.eval(bindings -> {
+                bindings.put("script", adapter.getScriptModel());
+                bindings.put("$evaluation", evaluation);
+            });
+        }
+        catch (Exception e) {
             throw new RuntimeException("Error evaluating JS Policy [" + policy.getName() + "].", e);
         }
     }
@@ -47,23 +56,18 @@ public class JSPolicyProvider implements PolicyProvider {
 
     }
 
-    private InvocableScriptAdapter getInvocableScriptAdapter(Policy policy, Evaluation evaluation) {
+    private ScriptModel getScriptModel(final Policy policy, final AuthorizationProvider authorization) {
         String scriptName = policy.getName();
         String scriptCode = policy.getConfig().get("code");
         String scriptDescription = policy.getDescription();
 
-        AuthorizationProvider authorization = evaluation.getAuthorizationProvider();
         RealmModel realm = authorization.getRealm();
 
-        ScriptingProvider scripting = authorization.getKeycloakSession().getProvider(ScriptingProvider.class);
-
         //TODO lookup script by scriptId instead of creating it every time
-        ScriptModel script = scripting.createScript(realm.getId(), ScriptModel.TEXT_JAVASCRIPT, scriptName, scriptCode, scriptDescription);
+        return getScriptingProvider(authorization).createScript(realm.getId(), ScriptModel.TEXT_JAVASCRIPT, scriptName, scriptCode, scriptDescription);
+    }
 
-        //how to deal with long running scripts -> timeout?
+    private ScriptingProvider getScriptingProvider(final AuthorizationProvider authorization) {
-        return scripting.prepareInvocableScript(script, bindings -> {
+        return authorization.getKeycloakSession().getProvider(ScriptingProvider.class);
-            bindings.put("script", script);
-            bindings.put("$evaluation", evaluation);
-        });
     }
 }

server-spi-private/src/main/java/org/keycloak/scripting/EvaluatableScriptAdapter.java

@@ -0,0 +1,14 @@
+package org.keycloak.scripting;
+
+import org.keycloak.models.ScriptModel;
+
+/**
+ * Wraps a {@link ScriptModel} so it can be evaluated with custom bindings.
+ *
+ * @author <a href="mailto:jay@anslow.me.uk">Jay Anslow</a>
+ */
+public interface EvaluatableScriptAdapter {
+    ScriptModel getScriptModel();
+
+    Object eval(ScriptBindingsConfigurer bindingsConfigurer) throws ScriptExecutionException;
+}

server-spi-private/src/main/java/org/keycloak/scripting/InvocableScriptAdapter.java

@@ -56,7 +56,7 @@ public class InvocableScriptAdapter implements Invocable {
         }
 
         this.scriptModel = scriptModel;
-        this.scriptEngine = loadScriptIntoEngine(scriptModel, scriptEngine);
+        this.scriptEngine = scriptEngine;
     }
 
     @Override
@@ -78,14 +78,6 @@ public class InvocableScriptAdapter implements Invocable {
         }
     }
 
-    public Object eval() throws ScriptExecutionException {
-        try {
-            return scriptEngine.eval(scriptModel.getCode());
-        } catch (ScriptException e) {
-            throw new ScriptExecutionException(scriptModel, e);
-        }
-    }
-
     @Override
     public <T> T getInterface(Class<T> clazz) {
         return getInvocableEngine().getInterface(clazz);
@@ -109,17 +101,6 @@ public class InvocableScriptAdapter implements Invocable {
         return candidate != null;
     }
 
-    private ScriptEngine loadScriptIntoEngine(ScriptModel script, ScriptEngine engine) {
-
-        try {
-            engine.eval(script.getCode());
-        } catch (ScriptException se) {
-            throw new ScriptExecutionException(script, se);
-        }
-
-        return engine;
-    }
-
     private Invocable getInvocableEngine() {
         return (Invocable) scriptEngine;
     }

server-spi-private/src/main/java/org/keycloak/scripting/ScriptingProvider.java

@@ -38,6 +38,14 @@ public interface ScriptingProvider extends Provider {
      */
     InvocableScriptAdapter prepareInvocableScript(ScriptModel scriptModel, ScriptBindingsConfigurer bindingsConfigurer);
 
+    /**
+     * Returns an {@link EvaluatableScriptAdapter} based on the given {@link ScriptModel}.
+     * <p>The {@code EvaluatableScriptAdapter} wraps a dedicated {@link ScriptEngine} that was populated with empty bindings.</p>
+     *
+     * @param scriptModel the scriptModel to wrap
+     */
+    EvaluatableScriptAdapter prepareEvaluatableScript(ScriptModel scriptModel);
+
     /**
      * Creates a new {@link ScriptModel} instance.
      *

services/src/main/java/org/keycloak/scripting/AbstractEvaluatableScriptAdapter.java

@@ -0,0 +1,76 @@
+package org.keycloak.scripting;
+
+import javax.script.Bindings;
+import javax.script.ScriptContext;
+import javax.script.ScriptEngine;
+import javax.script.ScriptException;
+
+import org.keycloak.models.ScriptModel;
+
+/**
+ * Abstract class for wrapping a {@link ScriptModel} to make it evaluatable.
+ *
+ * @author <a href="mailto:jay@anslow.me.uk">Jay Anslow</a>
+ */
+abstract class AbstractEvaluatableScriptAdapter implements EvaluatableScriptAdapter {
+    /**
+     * Holds the {@link ScriptModel}.
+     */
+    private final ScriptModel scriptModel;
+
+    AbstractEvaluatableScriptAdapter(final ScriptModel scriptModel) {
+        if (scriptModel == null) {
+            throw new IllegalArgumentException("scriptModel must not be null");
+        }
+        this.scriptModel = scriptModel;
+    }
+
+    @Override
+    public Object eval(final ScriptBindingsConfigurer bindingsConfigurer) throws ScriptExecutionException {
+        return evalUnchecked(createBindings(bindingsConfigurer));
+    }
+
+    @Override
+    public ScriptModel getScriptModel() {
+        return scriptModel;
+    }
+
+    /**
+     * Note, calling this method modifies the underlying {@link ScriptEngine},
+     * preventing concurrent use of the ScriptEngine (Nashorn's {@link ScriptEngine} and
+     * {@link javax.script.CompiledScript} is thread-safe, but {@link Bindings} isn't).
+     */
+    InvocableScriptAdapter prepareInvokableScript(final ScriptBindingsConfigurer bindingsConfigurer) {
+        final Bindings bindings = createBindings(bindingsConfigurer);
+        evalUnchecked(bindings);
+        final ScriptEngine engine = getEngine();
+        engine.setBindings(bindings, ScriptContext.ENGINE_SCOPE);
+        return new InvocableScriptAdapter(scriptModel, engine);
+    }
+
+    protected String getCode() {
+        return scriptModel.getCode();
+    }
+
+    protected abstract ScriptEngine getEngine();
+
+    protected abstract Object eval(Bindings bindings) throws ScriptException;
+
+    private Object evalUnchecked(final Bindings bindings) {
+        try {
+            return eval(bindings);
+        }
+        catch (ScriptException e) {
+            throw new ScriptExecutionException(scriptModel, e);
+        }
+    }
+
+    private Bindings createBindings(final ScriptBindingsConfigurer bindingsConfigurer) {
+        if (bindingsConfigurer == null) {
+            throw new IllegalArgumentException("bindingsConfigurer must not be null");
+        }
+        final Bindings bindings = getEngine().createBindings();
+        bindingsConfigurer.configureBindings(bindings);
+        return bindings;
+    }
+}

services/src/main/java/org/keycloak/scripting/CompiledEvaluatableScriptAdapter.java

@@ -0,0 +1,40 @@
+package org.keycloak.scripting;
+
+import javax.script.Bindings;
+import javax.script.CompiledScript;
+import javax.script.ScriptEngine;
+import javax.script.ScriptException;
+
+import org.keycloak.models.ScriptModel;
+
+/**
+ * Wraps a compiled {@link ScriptModel} so it can be evaluated.
+ *
+ * @author <a href="mailto:jay@anslow.me.uk">Jay Anslow</a>
+ */
+class CompiledEvaluatableScriptAdapter extends AbstractEvaluatableScriptAdapter {
+    /**
+     * Holds the {@link CompiledScript} for the {@link ScriptModel}.
+     */
+    private final CompiledScript compiledScript;
+
+    CompiledEvaluatableScriptAdapter(final ScriptModel scriptModel, final CompiledScript compiledScript) {
+        super(scriptModel);
+
+        if (compiledScript == null) {
+            throw new IllegalArgumentException("compiledScript must not be null");
+        }
+
+        this.compiledScript = compiledScript;
+    }
+
+    @Override
+    protected ScriptEngine getEngine() {
+        return compiledScript.getEngine();
+    }
+
+    @Override
+    protected Object eval(final Bindings bindings) throws ScriptException {
+        return compiledScript.eval(bindings);
+    }
+}

services/src/main/java/org/keycloak/scripting/DefaultScriptingProvider.java

@@ -16,12 +16,14 @@
  */
 package org.keycloak.scripting;
 
-import org.keycloak.models.ScriptModel;
-
 import javax.script.Bindings;
-import javax.script.ScriptContext;
+import javax.script.Compilable;
+import javax.script.CompiledScript;
 import javax.script.ScriptEngine;
 import javax.script.ScriptEngineManager;
+import javax.script.ScriptException;
+
+import org.keycloak.models.ScriptModel;
 
 /**
  * A {@link ScriptingProvider} that uses a {@link ScriptEngineManager} to evaluate scripts with a {@link ScriptEngine}.
@@ -32,8 +34,7 @@ public class DefaultScriptingProvider implements ScriptingProvider {
 
     private final ScriptEngineManager scriptEngineManager;
 
-    public DefaultScriptingProvider(ScriptEngineManager scriptEngineManager) {
+    DefaultScriptingProvider(ScriptEngineManager scriptEngineManager) {
-
         if (scriptEngineManager == null) {
             throw new IllegalStateException("scriptEngineManager must not be null!");
         }
@@ -44,13 +45,22 @@ public class DefaultScriptingProvider implements ScriptingProvider {
     /**
      * Wraps the provided {@link ScriptModel} in a {@link javax.script.Invocable} instance with bindings configured through the {@link ScriptBindingsConfigurer}.
      *
-     * @param scriptModel  must not be {@literal null}
+     * @param scriptModel        must not be {@literal null}
      * @param bindingsConfigurer must not be {@literal null}
-     * @return
      */
     @Override
     public InvocableScriptAdapter prepareInvocableScript(ScriptModel scriptModel, ScriptBindingsConfigurer bindingsConfigurer) {
+        final AbstractEvaluatableScriptAdapter evaluatable = prepareEvaluatableScript(scriptModel);
+        return evaluatable.prepareInvokableScript(bindingsConfigurer);
+    }
 
+    /**
+     * Wraps the provided {@link ScriptModel} in a {@link javax.script.Invocable} instance with bindings configured through the {@link ScriptBindingsConfigurer}.
+     *
+     * @param scriptModel must not be {@literal null}
+     */
+    @Override
+    public AbstractEvaluatableScriptAdapter prepareEvaluatableScript(ScriptModel scriptModel) {
         if (scriptModel == null) {
             throw new IllegalArgumentException("script must not be null");
         }
@@ -59,13 +69,18 @@ public class DefaultScriptingProvider implements ScriptingProvider {
             throw new IllegalArgumentException("script must not be null or empty");
         }
 
-        if (bindingsConfigurer == null) {
+        ScriptEngine engine = createPreparedScriptEngine(scriptModel);
-            throw new IllegalArgumentException("bindingsConfigurer must not be null");
+
+        if (engine instanceof Compilable) {
+            try {
+                final CompiledScript compiledScript = ((Compilable) engine).compile(scriptModel.getCode());
+                return new CompiledEvaluatableScriptAdapter(scriptModel, compiledScript);
+            }
+            catch (ScriptException e) {
+                throw new ScriptExecutionException(scriptModel, e);
+            }
         }
-
+        return new UncompiledEvaluatableScriptAdapter(scriptModel, engine);
-        ScriptEngine engine = createPreparedScriptEngine(scriptModel, bindingsConfigurer);
-
-        return new InvocableScriptAdapter(scriptModel, engine);
     }
 
     //TODO allow scripts to be maintained independently of other components, e.g. with dedicated persistence
@@ -74,38 +89,27 @@ public class DefaultScriptingProvider implements ScriptingProvider {
 
     @Override
     public ScriptModel createScript(String realmId, String mimeType, String scriptName, String scriptCode, String scriptDescription) {
+        return new Script(null /* scriptId */, realmId, scriptName, mimeType, scriptCode, scriptDescription);
+    }
 
-        ScriptModel script = new Script(null /* scriptId */, realmId, scriptName, mimeType, scriptCode, scriptDescription);
+    @Override
-        return script;
+    public void close() {
+        //NOOP
     }
 
     /**
      * Looks-up a {@link ScriptEngine} with prepared {@link Bindings} for the given {@link ScriptModel Script}.
-     *
-     * @param script
-     * @param bindingsConfigurer
-     * @return
      */
-    private ScriptEngine createPreparedScriptEngine(ScriptModel script, ScriptBindingsConfigurer bindingsConfigurer) {
+    private ScriptEngine createPreparedScriptEngine(ScriptModel script) {
-
         ScriptEngine scriptEngine = lookupScriptEngineFor(script);
 
         if (scriptEngine == null) {
             throw new IllegalStateException("Could not find ScriptEngine for script: " + script);
         }
 
-        configureBindings(bindingsConfigurer, scriptEngine);
-
         return scriptEngine;
     }
 
-    private void configureBindings(ScriptBindingsConfigurer bindingsConfigurer, ScriptEngine engine) {
-
-        Bindings bindings = engine.createBindings();
-        bindingsConfigurer.configureBindings(bindings);
-        engine.setBindings(bindings, ScriptContext.ENGINE_SCOPE);
-    }
-
     /**
      * Looks-up a {@link ScriptEngine} based on the MIME-type provided by the given {@link Script}.
      */
@@ -114,13 +118,9 @@ public class DefaultScriptingProvider implements ScriptingProvider {
         try {
             Thread.currentThread().setContextClassLoader(DefaultScriptingProvider.class.getClassLoader());
             return scriptEngineManager.getEngineByMimeType(script.getMimeType());
-        } finally {
+        }
+        finally {
             Thread.currentThread().setContextClassLoader(cl);
         }
     }
-
-    @Override
-    public void close() {
-        //NOOP
-    }
 }

services/src/main/java/org/keycloak/scripting/UncompiledEvaluatableScriptAdapter.java

@@ -0,0 +1,39 @@
+package org.keycloak.scripting;
+
+import javax.script.Bindings;
+import javax.script.ScriptEngine;
+import javax.script.ScriptException;
+
+import org.keycloak.models.ScriptModel;
+
+/**
+ * Wraps an uncompiled {@link ScriptModel} so it can be evaluated.
+ *
+ * @author <a href="mailto:jay@anslow.me.uk">Jay Anslow</a>
+ */
+class UncompiledEvaluatableScriptAdapter extends AbstractEvaluatableScriptAdapter {
+    /**
+     * Holds the {@link ScriptEngine} instance.
+     */
+    private final ScriptEngine scriptEngine;
+
+    UncompiledEvaluatableScriptAdapter(final ScriptModel scriptModel, final ScriptEngine scriptEngine) {
+        super(scriptModel);
+        if (scriptEngine == null) {
+            throw new IllegalArgumentException("scriptEngine must not be null");
+        }
+
+        this.scriptEngine = scriptEngine;
+    }
+
+    @Override
+    protected ScriptEngine getEngine() {
+        return scriptEngine;
+    }
+
+    @Override
+    protected Object eval(final Bindings bindings) throws ScriptException {
+        return getEngine().eval(getCode(), bindings);
+    }
+
+}