From f4b1286a97cb67de98b320dae9aea7674d513723 Mon Sep 17 00:00:00 2001 From: maximthomas Date: Thu, 15 Oct 2026 17:23:00 +0300 Subject: [PATCH 1/8] [#165] Compile runScriptOnConnector scripts once per engine, in child loaders --- .../scriptedcommon/ScriptedConfiguration.java | 121 +++++++++- .../ScriptOnConnectorCacheTest.java | 217 ++++++++++++++++++ 2 files changed, 337 insertions(+), 1 deletion(-) create mode 100644 OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index dd7b6a89..daae5771 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -36,6 +36,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.ConcurrentHashMap; @@ -677,6 +678,12 @@ public void release() { publishedCustomizerClass = null; } groovyScriptEngine = null; + final ScriptOnConnectorCache cache = scriptOnConnectorCache; + scriptOnConnectorCache = null; + if (null != cache) { + // Last, so that a failure here cannot leave the engine published. + cache.clear(); + } } propertyBag.clear(); loggerCache.clear(); @@ -780,6 +787,44 @@ Class loadScript(String scriptName) { return null; } + /** + * Returns the class compiled from the text of a runScriptOnConnector request. + *

+ * Each text is compiled once per engine, by its own child loader of the engine's loader. The + * child shares the engine's compiler configuration, and with it the default imports, and + * resolves the classes of the script roots through the engine's loader, but keeps the new + * classes out of the engine's class cache. No lock is held while compiling, so a cached script + * does not wait for another script to compile. + */ + Class getScriptOnConnectorClass(String scriptText) { + final GroovyClassLoader engineLoader; + final ScriptOnConnectorCache cache; + synchronized (this) { + engineLoader = getGroovyScriptEngine().getGroovyClassLoader(); + // Null while the customizer runs: what it compiles here is not cached. + cache = scriptOnConnectorCache; + } + if (null != cache) { + final Class cached = cache.get(scriptText); + if (null != cached) { + return cached; + } + } + final GroovyClassLoader loader = new GroovyClassLoader(engineLoader); + final Class scriptClass = loader.parseClass(new GroovyCodeSource(scriptText, + "Script" + System.currentTimeMillis() + ".groovy", "fix"), false); + return null == cache ? scriptClass : cache.put(scriptText, new CompiledScript(scriptClass, loader)); + } + + /** + * Runs the text of a runScriptOnConnector request with the given binding. + */ + Object evaluateScriptOnConnector(String scriptText, Binding binding) { + // Runs from Java: a Groovy call site would keep the metaclass of the last class it ran, + // and with it the class. + return InvokerHelper.createScript(getScriptOnConnectorClass(scriptText), binding).run(); + } + protected Log getLogger(final Class clazz) { final String key = clazz.getName(); Log logger = loggerCache.get(key); @@ -805,6 +850,9 @@ protected Log getLogger(final Class clazz) { /** The customizer class of the published engine; {@link #release()} unregisters it. Guarded by {@code this}. */ private Class publishedCustomizerClass = null; + /** The runScriptOnConnector classes of the published engine. Guarded by {@code this}. */ + private ScriptOnConnectorCache scriptOnConnectorCache = null; + /** * Synchronised for the whole initialisation, not double-checked: the * customizer script runs against the half-initialised engine (it may call @@ -836,6 +884,7 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { customizingGroovyScriptEngine = null; } publishedCustomizerClass = customizerClass; + scriptOnConnectorCache = new ScriptOnConnectorCache(); groovyScriptEngine = engine; } return groovyScriptEngine; @@ -845,7 +894,8 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { * This must be called once from thread-safe location and inside the * synchronized to avoid deadlock. The customizer runs while this * configuration's monitor is held: it must not wait for another thread - * that calls getGroovyScriptEngine(), evaluate() or loadScript(). + * that calls getGroovyScriptEngine(), evaluate(), loadScript(), + * getScriptOnConnectorClass() or evaluateScriptOnConnector(). * Returns the customizer class, or null if there is none. */ private Class initializeCustomizer() { @@ -936,4 +986,73 @@ protected ImportCustomizer getImportCustomizer(ImportCustomizer parent) { protected String[] getImports() { return new String[] { ConnectorObject.class.getPackage().getName() + ".*" }; } + + /** A compiled runScriptOnConnector script and the loader whose class cache holds its classes. */ + private static final class CompiledScript { + + private final Class scriptClass; + + private final GroovyClassLoader loader; + + private CompiledScript(Class scriptClass, GroovyClassLoader loader) { + this.scriptClass = scriptClass; + this.loader = loader; + } + + /** + * Drops the metaclasses of every class the script compiled, closures and declared classes + * included, so that the classes can be unloaded once nothing else uses them. + */ + private void unregister() { + for (Class c : loader.getLoadedClasses()) { + InvokerHelper.removeClass(c); + } + } + } + + /** The runScriptOnConnector classes compiled for one engine, keyed by the script text. */ + private static final class ScriptOnConnectorCache { + + /** Least recently used first. Guarded by itself. */ + private final LinkedHashMap scripts = + new LinkedHashMap(16, 0.75f, true); + + private Class get(String scriptText) { + synchronized (scripts) { + final CompiledScript script = scripts.get(scriptText); + return null == script ? null : script.scriptClass; + } + } + + /** + * Caches the script unless another thread cached the same text first, and returns the + * class to run. + */ + private Class put(String scriptText, CompiledScript script) { + final CompiledScript cached; + synchronized (scripts) { + cached = scripts.get(scriptText); + if (null == cached) { + scripts.put(scriptText, script); + } + } + if (null == cached) { + return script.scriptClass; + } + // Outside the monitor: removeClass takes Groovy's own locks. + script.unregister(); + return cached.scriptClass; + } + + private void clear() { + final List dropped; + synchronized (scripts) { + dropped = new ArrayList(scripts.values()); + scripts.clear(); + } + for (CompiledScript script : dropped) { + script.unregister(); + } + } + } } diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java new file mode 100644 index 00000000..43310fe0 --- /dev/null +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -0,0 +1,217 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.forgerock.openicf.misc.scriptedcommon; + +import static java.util.concurrent.TimeUnit.SECONDS; +import static org.fest.assertions.api.Assertions.assertThat; + +import java.io.File; +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.atomic.AtomicReference; + +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.classgen.GeneratorContext; +import org.codehaus.groovy.control.SourceUnit; +import org.codehaus.groovy.control.customizers.ImportCustomizer; +import org.codehaus.groovy.reflection.ClassInfo; +import org.identityconnectors.framework.common.objects.ObjectClass; +import org.testng.annotations.AfterMethod; +import org.testng.annotations.BeforeMethod; +import org.testng.annotations.Test; + +import groovy.lang.Binding; +import groovy.lang.ExpandoMetaClass; +import groovy.lang.GroovyClassLoader; +import groovy.lang.GroovySystem; + +/** + * Tests how {@link ScriptedConfiguration} compiles, caches and releases the scripts of + * runScriptOnConnector. + */ +public class ScriptOnConnectorCacheTest { + + /** What the customizer script does; set by the test that writes one. */ + private static volatile Runnable customizer; + + /** Called from the customizer script written by {@link #writeCustomizer()}. */ + public static void customize() { + customizer.run(); + } + + /** While set, every compilation of a {@link #blockingCompilations()} configuration waits for it. */ + private static volatile CountDownLatch compileGate; + + /** Counted down when a compilation starts waiting for {@link #compileGate}. */ + private static volatile CountDownLatch compileWaiting; + + private File scriptRoot; + + private ScriptedConfiguration configuration; + + private ExecutorService executor; + + @BeforeMethod + public void setUp() throws Exception { + scriptRoot = Files.createTempDirectory("script-on-connector").toFile(); + Files.write(new File(scriptRoot, "Helper.groovy").toPath(), + "class Helper { static String greet() { 'hello' } }\n".getBytes(StandardCharsets.UTF_8)); + configuration = withScriptRoot(new ScriptedConfiguration()); + executor = Executors.newCachedThreadPool(); + } + + @AfterMethod + public void tearDown() { + customizer = null; + final CountDownLatch gate = compileGate; + compileGate = null; + if (null != gate) { + gate.countDown(); + } + executor.shutdownNow(); + configuration.release(); + for (File file : scriptRoot.listFiles()) { + file.delete(); + } + scriptRoot.delete(); + } + + @Test + public void testSameTextReusesTheCompiledClass() { + final Class first = configuration.getScriptOnConnectorClass("return 1"); + assertThat(configuration.getScriptOnConnectorClass("return 1")).isSameAs(first); + assertThat(configuration.getScriptOnConnectorClass("return 2")).isNotSameAs(first); + } + + @Test + public void testScriptClassStaysOutOfTheEngineLoader() { + final GroovyClassLoader engineLoader = configuration.getGroovyScriptEngine().getGroovyClassLoader(); + final int loadedBefore = engineLoader.getLoadedClasses().length; + final Class scriptClass = configuration.getScriptOnConnectorClass("return 1"); + assertThat(engineLoader.getLoadedClasses()).hasSize(loadedBefore); + assertThat(compilingLoader(scriptClass).getParent()).isSameAs(engineLoader); + } + + @Test + public void testScriptSeesTheDefaultImports() { + assertThat(configuration.evaluateScriptOnConnector("return ObjectClass.ACCOUNT_NAME", new Binding())) + .isEqualTo(ObjectClass.ACCOUNT_NAME); + } + + @Test + public void testScriptSeesTheClassesInTheScriptRoots() { + assertThat(configuration.evaluateScriptOnConnector("return Helper.greet()", new Binding())) + .isEqualTo("hello"); + } + + @Test + public void testScriptSeesItsBinding() { + final Binding binding = new Binding(); + binding.setVariable("a", 41); + assertThat(configuration.evaluateScriptOnConnector("return a + 1", binding)).isEqualTo(42); + } + + @Test + public void testReleaseUnregistersTheCachedClasses() { + final Class released = configuration.getScriptOnConnectorClass("return 1"); + registerMetaClass(released); + configuration.release(); + assertThat(ClassInfo.getClassInfo(released).getStrongMetaClass()).isNull(); + final Class recompiled = configuration.getScriptOnConnectorClass("return 1"); + assertThat(recompiled).isNotSameAs(released); + assertThat(compilingLoader(recompiled).getParent()) + .isSameAs(configuration.getGroovyScriptEngine().getGroovyClassLoader()); + } + + @Test + public void testCustomizerCanRunAScriptOnConnector() throws Exception { + final AtomicReference result = new AtomicReference(); + customizer = () -> result.set(configuration.evaluateScriptOnConnector("return 41 + 1", new Binding())); + writeCustomizer(); + configuration.getGroovyScriptEngine(); + assertThat(result.get()).isEqualTo(42); + } + + @Test(timeOut = 60000) + public void testCachedScriptDoesNotWaitForAnotherCompilation() throws Exception { + final ScriptedConfiguration blocking = blockingCompilations(); + configuration = blocking; + final Class cached = blocking.getScriptOnConnectorClass("return 1"); + compileWaiting = new CountDownLatch(1); + compileGate = new CountDownLatch(1); + final Future compiling = executor.submit(() -> blocking.getScriptOnConnectorClass("return 2")); + assertThat(compileWaiting.await(10, SECONDS)).isTrue(); + final Future hit = executor.submit(() -> blocking.getScriptOnConnectorClass("return 1")); + assertThat(hit.get(10, SECONDS)).isSameAs(cached); + compileGate.countDown(); + assertThat(compiling.get(10, SECONDS)).isNotSameAs(cached); + } + + private ScriptedConfiguration withScriptRoot(ScriptedConfiguration result) { + result.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); + return result; + } + + /** A configuration whose compilations wait for {@link #compileGate} while it is set. */ + private ScriptedConfiguration blockingCompilations() { + return withScriptRoot(new ScriptedConfiguration() { + @Override + protected ImportCustomizer getImportCustomizer(ImportCustomizer parent) { + return super.getImportCustomizer(new ImportCustomizer() { + @Override + public void call(SourceUnit source, GeneratorContext context, ClassNode classNode) { + final CountDownLatch gate = compileGate; + if (null != gate) { + compileWaiting.countDown(); + try { + gate.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + super.call(source, context, classNode); + } + }); + } + }); + } + + /** Writes a customizer script that calls {@link #customize()}, and configures it. */ + private void writeCustomizer() throws IOException { + Files.write(new File(scriptRoot, "Customizer.groovy").toPath(), + (ScriptOnConnectorCacheTest.class.getName() + ".customize()\n").getBytes(StandardCharsets.UTF_8)); + configuration.setCustomizerScriptFileName("Customizer.groovy"); + } + + /** Puts a strong metaclass on the class, as a script that changes its own metaclass does. */ + private static void registerMetaClass(Class clazz) { + final ExpandoMetaClass metaClass = new ExpandoMetaClass(clazz, false, true); + metaClass.initialize(); + GroovySystem.getMetaClassRegistry().setMetaClass(clazz, metaClass); + assertThat(ClassInfo.getClassInfo(clazz).getStrongMetaClass()).isNotNull(); + } + + /** The loader whose class cache holds a compiled script class: the parent of its InnerLoader. */ + private static ClassLoader compilingLoader(Class scriptClass) { + return scriptClass.getClassLoader().getParent(); + } +} From a40030c24e05a977881c558f2f268bc72fb5bfb1 Mon Sep 17 00:00:00 2001 From: maximthomas Date: Thu, 15 Oct 2026 17:34:51 +0300 Subject: [PATCH 2/8] [#165] Keep at most 64 runScriptOnConnector classes per engine and unregister evicted ones --- .../scriptedcommon/ScriptedConfiguration.java | 77 ++++++++++++++++--- .../ScriptOnConnectorCacheTest.java | 67 ++++++++++++++++ 2 files changed, 132 insertions(+), 12 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index daae5771..da3bec4b 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -36,6 +36,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -95,6 +96,17 @@ public class ScriptedConfiguration extends AbstractConfiguration implements Stat private static final Log logger = Log.getLog(ScriptedConfiguration.class); private static final String DOT_STAR = ".*"; private static final String EMPTY_STRING = ""; + + /** + * System property with the number of runScriptOnConnector classes that each script engine keeps + * compiled, 64 by default. It is read when the engine is created; a value that is not a positive + * integer is ignored with a warning. + */ + public static final String SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY = + "org.identityconnectors.framework.groovy.scriptOnConnectorCacheSize"; + + /** The number of runScriptOnConnector classes kept when the system property is not set. */ + static final int DEFAULT_SCRIPT_ON_CONNECTOR_CACHE_SIZE = 64; private final CompilerConfiguration config; { @@ -825,6 +837,29 @@ Object evaluateScriptOnConnector(String scriptText, Binding binding) { return InvokerHelper.createScript(getScriptOnConnectorClass(scriptText), binding).run(); } + /** + * Returns the number of runScriptOnConnector classes a new engine keeps: the value of + * {@link #SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY}, or the default when it is not set or not a + * positive integer. + */ + static int getScriptOnConnectorCacheSize() { + final String value = System.getProperty(SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY); + if (null == value) { + return DEFAULT_SCRIPT_ON_CONNECTOR_CACHE_SIZE; + } + try { + final int size = Integer.parseInt(value.trim()); + if (size > 0) { + return size; + } + } catch (NumberFormatException e) { + // Warned about below. + } + logger.warn("Ignoring {0}={1}: not a positive integer, keeping {2} scripts", + SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY, value, DEFAULT_SCRIPT_ON_CONNECTOR_CACHE_SIZE); + return DEFAULT_SCRIPT_ON_CONNECTOR_CACHE_SIZE; + } + protected Log getLogger(final Class clazz) { final String key = clazz.getName(); Log logger = loggerCache.get(key); @@ -884,7 +919,7 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { customizingGroovyScriptEngine = null; } publishedCustomizerClass = customizerClass; - scriptOnConnectorCache = new ScriptOnConnectorCache(); + scriptOnConnectorCache = new ScriptOnConnectorCache(getScriptOnConnectorCacheSize()); groovyScriptEngine = engine; } return groovyScriptEngine; @@ -1010,13 +1045,22 @@ private void unregister() { } } - /** The runScriptOnConnector classes compiled for one engine, keyed by the script text. */ + /** + * The runScriptOnConnector classes compiled for one engine, keyed by the script text. Holds at + * most {@code maxSize} scripts and unregisters the least recently used one beyond that. + */ private static final class ScriptOnConnectorCache { + private final int maxSize; + /** Least recently used first. Guarded by itself. */ private final LinkedHashMap scripts = new LinkedHashMap(16, 0.75f, true); + private ScriptOnConnectorCache(int maxSize) { + this.maxSize = maxSize; + } + private Class get(String scriptText) { synchronized (scripts) { final CompiledScript script = scripts.get(scriptText); @@ -1025,23 +1069,32 @@ private Class get(String scriptText) { } /** - * Caches the script unless another thread cached the same text first, and returns the - * class to run. + * Caches the script unless another thread cached the same text first, evicts the least + * recently used scripts beyond the size, and returns the class to run. */ private Class put(String scriptText, CompiledScript script) { - final CompiledScript cached; + final List dropped = new ArrayList(1); + final Class result; synchronized (scripts) { - cached = scripts.get(scriptText); - if (null == cached) { + final CompiledScript cached = scripts.get(scriptText); + if (null != cached) { + dropped.add(script); + result = cached.scriptClass; + } else { scripts.put(scriptText, script); + final Iterator eldest = scripts.values().iterator(); + while (scripts.size() > maxSize) { + dropped.add(eldest.next()); + eldest.remove(); + } + result = script.scriptClass; } } - if (null == cached) { - return script.scriptClass; - } // Outside the monitor: removeClass takes Groovy's own locks. - script.unregister(); - return cached.scriptClass; + for (CompiledScript d : dropped) { + d.unregister(); + } + return result; } private void clear() { diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index 43310fe0..9f713a59 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -21,6 +21,7 @@ import java.io.File; import java.io.IOException; +import java.lang.ref.WeakReference; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.concurrent.CountDownLatch; @@ -81,6 +82,7 @@ public void setUp() throws Exception { @AfterMethod public void tearDown() { + System.clearProperty(ScriptedConfiguration.SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY); customizer = null; final CountDownLatch gate = compileGate; compileGate = null; @@ -166,6 +168,56 @@ public void testCachedScriptDoesNotWaitForAnotherCompilation() throws Exception assertThat(compiling.get(10, SECONDS)).isNotSameAs(cached); } + @Test + public void testCacheSizeComesFromTheSystemProperty() { + final String property = ScriptedConfiguration.SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY; + assertThat(property).isEqualTo("org.identityconnectors.framework.groovy.scriptOnConnectorCacheSize"); + assertThat(ScriptedConfiguration.getScriptOnConnectorCacheSize()).isEqualTo(64); + System.setProperty(property, "5"); + assertThat(ScriptedConfiguration.getScriptOnConnectorCacheSize()).isEqualTo(5); + System.setProperty(property, " 7 "); + assertThat(ScriptedConfiguration.getScriptOnConnectorCacheSize()).isEqualTo(7); + System.setProperty(property, "0"); + assertThat(ScriptedConfiguration.getScriptOnConnectorCacheSize()).isEqualTo(64); + System.setProperty(property, "-3"); + assertThat(ScriptedConfiguration.getScriptOnConnectorCacheSize()).isEqualTo(64); + System.setProperty(property, "many"); + assertThat(ScriptedConfiguration.getScriptOnConnectorCacheSize()).isEqualTo(64); + } + + @Test + public void testLeastRecentlyUsedScriptIsEvicted() { + System.setProperty(ScriptedConfiguration.SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY, "2"); + final Class a = configuration.getScriptOnConnectorClass("return 'a'"); + final Class b = configuration.getScriptOnConnectorClass("return 'b'"); + assertThat(configuration.getScriptOnConnectorClass("return 'a'")).isSameAs(a); + configuration.getScriptOnConnectorClass("return 'c'"); + assertThat(configuration.getScriptOnConnectorClass("return 'a'")).isSameAs(a); + assertThat(configuration.getScriptOnConnectorClass("return 'b'")).isNotSameAs(b); + } + + @Test + public void testEvictedScriptClassesAreUnregistered() { + System.setProperty(ScriptedConfiguration.SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY, "1"); + final String text = "def c = { -> 1 }; return c.getClass()"; + final Class scriptClass = configuration.getScriptOnConnectorClass(text); + final Class closureClass = (Class) configuration.evaluateScriptOnConnector(text, new Binding()); + assertThat(closureClass).isNotSameAs(scriptClass); + registerMetaClass(scriptClass); + registerMetaClass(closureClass); + configuration.getScriptOnConnectorClass("return 2"); + assertThat(ClassInfo.getClassInfo(scriptClass).getStrongMetaClass()).isNull(); + assertThat(ClassInfo.getClassInfo(closureClass).getStrongMetaClass()).isNull(); + } + + @Test + public void testEvictedScriptClassCanBeUnloaded() throws Exception { + System.setProperty(ScriptedConfiguration.SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY, "1"); + final WeakReference evicted = compileAndRun("def c = { it + 1 }; return c(1)"); + configuration.getScriptOnConnectorClass("return 2"); + assertThat(isCollected(evicted)).isTrue(); + } + private ScriptedConfiguration withScriptRoot(ScriptedConfiguration result) { result.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); return result; @@ -214,4 +266,19 @@ private static void registerMetaClass(Class clazz) { private static ClassLoader compilingLoader(Class scriptClass) { return scriptClass.getClassLoader().getParent(); } + + /** Compiles and runs the script, keeping no strong reference to its class. */ + private WeakReference compileAndRun(String scriptText) { + configuration.evaluateScriptOnConnector(scriptText, new Binding()); + return new WeakReference(configuration.getScriptOnConnectorClass(scriptText)); + } + + /** Runs full collections, which unload classes, until the referent is gone. */ + private static boolean isCollected(WeakReference reference) throws InterruptedException { + for (int i = 0; i < 20 && null != reference.get(); i++) { + System.gc(); + Thread.sleep(50); + } + return null == reference.get(); + } } From a872c5bdfc03522f592c04d824877e5d9acbfb44 Mon Sep 17 00:00:00 2001 From: maximthomas Date: Thu, 15 Oct 2026 17:39:44 +0300 Subject: [PATCH 3/8] [#165] Run runScriptOnConnector scripts from the per-engine class cache --- .../ScriptedConnectorBase.groovy | 13 +++--- .../ScriptOnConnectorCacheTest.java | 45 +++++++++++++++++++ 2 files changed, 51 insertions(+), 7 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/groovy/org/forgerock/openicf/misc/scriptedcommon/ScriptedConnectorBase.groovy b/OpenICF-groovy-connector/src/main/groovy/org/forgerock/openicf/misc/scriptedcommon/ScriptedConnectorBase.groovy index d3418dc0..d268d977 100644 --- a/OpenICF-groovy-connector/src/main/groovy/org/forgerock/openicf/misc/scriptedcommon/ScriptedConnectorBase.groovy +++ b/OpenICF-groovy-connector/src/main/groovy/org/forgerock/openicf/misc/scriptedcommon/ScriptedConnectorBase.groovy @@ -20,11 +20,12 @@ * with the fields enclosed by brackets [] replaced by * your own identifying information: * "Portions Copyrighted [year] [name of copyright owner]" + * + * Portions Copyrighted 2026 3A Systems, LLC */ package org.forgerock.openicf.misc.scriptedcommon -import org.codehaus.groovy.runtime.InvokerHelper import org.identityconnectors.common.CollectionUtil import org.identityconnectors.common.StringUtil import org.identityconnectors.common.logging.Log @@ -339,8 +340,6 @@ public class ScriptedConnectorBase implements A if (GROOVY.equalsIgnoreCase(request.getScriptLanguage()) && StringUtil.isNotBlank(request.getScriptText())) { try { - final GroovyCodeSource codeSource = new GroovyCodeSource(request.scriptText, - "Script" + System.currentTimeMillis() + ".groovy", "fix"); Binding binding = new Binding(); for (Map.Entry entry : request.scriptArguments) { binding.setVariable(entry.key, entry.value) @@ -349,10 +348,10 @@ public class ScriptedConnectorBase implements A createBinding(binding, OperationType.RUNSCRIPTONCONNECTOR, null, null, null, options), { String scriptName, Binding arguments -> arguments.setVariable(LOGGER, logger) - return InvokerHelper.createScript(getScriptedConfiguration().getGroovyScriptEngine(). - getGroovyClassLoader().parseClass(codeSource, false), - arguments).run(); - }); + // Compiled once per text and run from Java, see evaluateScriptOnConnector. + return getScriptedConfiguration().evaluateScriptOnConnector(request.scriptText, + arguments) + }); } catch (final RuntimeException e) { throw e; } catch (final Exception e) { diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index 9f713a59..b1df0472 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -24,6 +24,8 @@ import java.lang.ref.WeakReference; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.Arrays; +import java.util.Collections; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -35,7 +37,9 @@ import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.reflection.ClassInfo; +import org.forgerock.openicf.connectors.groovy.ScriptedPoolableConnector; import org.identityconnectors.framework.common.objects.ObjectClass; +import org.identityconnectors.framework.common.objects.ScriptContext; import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; @@ -218,6 +222,32 @@ public void testEvictedScriptClassCanBeUnloaded() throws Exception { assertThat(isCollected(evicted)).isTrue(); } + @Test + public void testConnectorReusesTheCompiledScript() { + final GroovyClassLoader engineLoader = configuration.getGroovyScriptEngine().getGroovyClassLoader(); + final int loadedBefore = engineLoader.getLoadedClasses().length; + final ScriptedPoolableConnector connector = connector(); + final Object first = connector.runScriptOnConnector(groovy("return this.getClass()"), null); + assertThat(connector.runScriptOnConnector(groovy("return this.getClass()"), null)).isSameAs(first); + assertThat(engineLoader.getLoadedClasses()).hasSize(loadedBefore); + } + + @Test + public void testConnectorScriptSeesItsArgumentsAndTheLogger() { + final ScriptContext request = new ScriptContext("Groovy", "return [a + 1, null != log]", + Collections.singletonMap("a", 41)); + assertThat(connector().runScriptOnConnector(request, null)).isEqualTo(Arrays.asList(42, true)); + } + + @Test + public void testScriptRunByTheConnectorCanBeUnloadedOnceEvicted() throws Exception { + System.setProperty(ScriptedConfiguration.SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY, "1"); + final WeakReference evicted = runOnConnectorAndForget("return this.getClass()"); + // Evicted through the configuration, so the connector code runs no other script after it. + configuration.getScriptOnConnectorClass("return 2"); + assertThat(isCollected(evicted)).isTrue(); + } + private ScriptedConfiguration withScriptRoot(ScriptedConfiguration result) { result.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); return result; @@ -281,4 +311,19 @@ private static boolean isCollected(WeakReference reference) throws Interrupte } return null == reference.get(); } + + /** Runs the script through a connector and keeps no strong reference to what it returned. */ + private WeakReference runOnConnectorAndForget(String scriptText) { + return new WeakReference(connector().runScriptOnConnector(groovy(scriptText), null)); + } + + private ScriptedPoolableConnector connector() { + final ScriptedPoolableConnector connector = new ScriptedPoolableConnector(); + connector.init(configuration); + return connector; + } + + private static ScriptContext groovy(String scriptText) { + return new ScriptContext("Groovy", scriptText, Collections.emptyMap()); + } } From ed25f73f46774a84714db75833b4acdc83a81f59 Mon Sep 17 00:00:00 2001 From: maximthomas Date: Thu, 15 Oct 2026 18:09:16 +0300 Subject: [PATCH 4/8] [#165] Report a broken script-root class as a compilation failure, not a GroovyBugError --- .../scriptedcommon/ScriptedConfiguration.java | 16 ++++++++++++++-- .../ScriptOnConnectorCacheTest.java | 18 ++++++++++++++++++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index da3bec4b..7772fd27 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -43,6 +43,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import org.codehaus.groovy.GroovyBugError; +import org.codehaus.groovy.control.CompilationFailedException; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.runtime.InvokerHelper; @@ -823,8 +825,18 @@ Class getScriptOnConnectorClass(String scriptText) { } } final GroovyClassLoader loader = new GroovyClassLoader(engineLoader); - final Class scriptClass = loader.parseClass(new GroovyCodeSource(scriptText, - "Script" + System.currentTimeMillis() + ".groovy", "fix"), false); + final Class scriptClass; + try { + scriptClass = loader.parseClass(new GroovyCodeSource(scriptText, + "Script" + System.currentTimeMillis() + ".groovy", "fix"), false); + } catch (GroovyBugError e) { + // The child's resolver reports a failed compile of a script-root class as a bug; report it as the + // compilation failure it is, as the engine's resolver does. + if (e.getCause() instanceof CompilationFailedException) { + throw (CompilationFailedException) e.getCause(); + } + throw e; + } return null == cache ? scriptClass : cache.put(scriptText, new CompiledScript(scriptClass, loader)); } diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index b1df0472..b4a8c520 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -34,6 +34,7 @@ import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.classgen.GeneratorContext; +import org.codehaus.groovy.control.CompilationFailedException; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.reflection.ClassInfo; @@ -248,6 +249,23 @@ public void testScriptRunByTheConnectorCanBeUnloadedOnceEvicted() throws Excepti assertThat(isCollected(evicted)).isTrue(); } + @Test(expectedExceptions = CompilationFailedException.class) + public void testBrokenScriptRootClassFailsAsACompilationError() throws Exception { + writeBrokenClass(); + configuration.evaluateScriptOnConnector("return Broken.x()", new Binding()); + } + + @Test(expectedExceptions = RuntimeException.class) + public void testConnectorReportsABrokenScriptRootClassAsARuntimeException() throws Exception { + writeBrokenClass(); + connector().runScriptOnConnector(groovy("return Broken.x()"), null); + } + + private void writeBrokenClass() throws IOException { + Files.write(new File(scriptRoot, "Broken.groovy").toPath(), + "class Broken { static int x() { return 1 + } }\n".getBytes(StandardCharsets.UTF_8)); + } + private ScriptedConfiguration withScriptRoot(ScriptedConfiguration result) { result.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); return result; From ce3a86f2db8377ffcc8518877f011278b576a9b5 Mon Sep 17 00:00:00 2001 From: maximthomas Date: Thu, 15 Oct 2026 19:42:07 +0300 Subject: [PATCH 5/8] [#165] Resolve script-root classes through the engine and compile each text with its own import customizer --- .../scriptedcommon/ScriptedConfiguration.java | 129 ++++++++++++---- .../ScriptOnConnectorCacheTest.java | 144 ++++++++++++++++-- .../scriptedcommon/ShadowedByScriptRoot.java | 31 ++++ 3 files changed, 261 insertions(+), 43 deletions(-) create mode 100644 OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ShadowedByScriptRoot.java diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index 7772fd27..4d0296b4 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -33,6 +33,7 @@ import java.net.MalformedURLException; import java.net.URI; import java.net.URL; +import java.security.CodeSource; import java.util.ArrayList; import java.util.Arrays; import java.util.HashSet; @@ -43,8 +44,9 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import org.codehaus.groovy.GroovyBugError; -import org.codehaus.groovy.control.CompilationFailedException; +import org.codehaus.groovy.ast.ClassNode; +import org.codehaus.groovy.control.ClassNodeResolver; +import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.runtime.InvokerHelper; @@ -100,14 +102,15 @@ public class ScriptedConfiguration extends AbstractConfiguration implements Stat private static final String EMPTY_STRING = ""; /** - * System property with the number of runScriptOnConnector classes that each script engine keeps - * compiled, 64 by default. It is read when the engine is created; a value that is not a positive - * integer is ignored with a warning. + * System property with the number of runScriptOnConnector scripts that each script engine keeps + * compiled, 64 by default; a script counts with its closures and the classes it declares. It is + * read when the engine is created; a value that is not a positive integer is ignored with a + * warning. */ public static final String SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY = "org.identityconnectors.framework.groovy.scriptOnConnectorCacheSize"; - /** The number of runScriptOnConnector classes kept when the system property is not set. */ + /** The number of runScriptOnConnector scripts kept when the system property is not set. */ static final int DEFAULT_SCRIPT_ON_CONNECTOR_CACHE_SIZE = 64; private final CompilerConfiguration config; @@ -805,17 +808,18 @@ Class loadScript(String scriptName) { * Returns the class compiled from the text of a runScriptOnConnector request. *

* Each text is compiled once per engine, by its own child loader of the engine's loader. The - * child shares the engine's compiler configuration, and with it the default imports, and - * resolves the classes of the script roots through the engine's loader, but keeps the new - * classes out of the engine's class cache. No lock is held while compiling, so a cached script - * does not wait for another script to compile. + * child compiles with a configuration built as the engine's is, and with it the default + * imports. It resolves the classes of the script roots through the engine, as the engine's own + * compilations do, but keeps the new classes out of the engine's class cache. No lock is held + * while compiling, so a cached script does not wait for another script to compile. */ Class getScriptOnConnectorClass(String scriptText) { - final GroovyClassLoader engineLoader; + final GroovyScriptEngine engine; final ScriptOnConnectorCache cache; synchronized (this) { - engineLoader = getGroovyScriptEngine().getGroovyClassLoader(); - // Null while the customizer runs: what it compiles here is not cached. + engine = getGroovyScriptEngine(); + // Null while the customizer runs, or when a subclass builds the engine without super: + // what it compiles here is not cached. cache = scriptOnConnectorCache; } if (null != cache) { @@ -824,19 +828,10 @@ Class getScriptOnConnectorClass(String scriptText) { return cached; } } - final GroovyClassLoader loader = new GroovyClassLoader(engineLoader); - final Class scriptClass; - try { - scriptClass = loader.parseClass(new GroovyCodeSource(scriptText, - "Script" + System.currentTimeMillis() + ".groovy", "fix"), false); - } catch (GroovyBugError e) { - // The child's resolver reports a failed compile of a script-root class as a bug; report it as the - // compilation failure it is, as the engine's resolver does. - if (e.getCause() instanceof CompilationFailedException) { - throw (CompilationFailedException) e.getCause(); - } - throw e; - } + final GroovyClassLoader loader = + new ScriptOnConnectorLoader(engine, newCompilerConfiguration(), scriptRootLock); + final Class scriptClass = loader.parseClass(new GroovyCodeSource(scriptText, + "Script" + System.currentTimeMillis() + ".groovy", "fix"), false); return null == cache ? scriptClass : cache.put(scriptText, new CompiledScript(scriptClass, loader)); } @@ -850,7 +845,7 @@ Object evaluateScriptOnConnector(String scriptText, Binding binding) { } /** - * Returns the number of runScriptOnConnector classes a new engine keeps: the value of + * Returns the number of runScriptOnConnector scripts a new engine keeps: the value of * {@link #SCRIPT_ON_CONNECTOR_CACHE_SIZE_PROPERTY}, or the default when it is not set or not a * positive integer. */ @@ -897,9 +892,12 @@ protected Log getLogger(final Class clazz) { /** The customizer class of the published engine; {@link #release()} unregisters it. Guarded by {@code this}. */ private Class publishedCustomizerClass = null; - /** The runScriptOnConnector classes of the published engine. Guarded by {@code this}. */ + /** The runScriptOnConnector scripts of the published engine. Guarded by {@code this}. */ private ScriptOnConnectorCache scriptOnConnectorCache = null; + /** Held while a runScriptOnConnector compilation loads a class of the script roots. */ + private final Object scriptRootLock = new Object(); + /** * Synchronised for the whole initialisation, not double-checked: the * customizer script runs against the half-initialised engine (it may call @@ -912,9 +910,7 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { return customizingGroovyScriptEngine; } - final CompilerConfiguration compilerConfiguration = - new CompilerConfiguration(config); - compilerConfiguration.addCompilationCustomizers(getImportCustomizer(null)); + final CompilerConfiguration compilerConfiguration = newCompilerConfiguration(); final GroovyClassLoader loader = new GroovyClassLoader(getParentLoader(), compilerConfiguration, true); @@ -1034,6 +1030,75 @@ protected String[] getImports() { return new String[] { ConnectorObject.class.getPackage().getName() + ".*" }; } + /** + * Returns a new compiler configuration for an engine or for one runScriptOnConnector + * compilation. Each gets its own import customizer: the compiler resolves the class nodes of + * its imports in place, renaming them while it tries inner class names, so two compilations + * running at once must not share them. + */ + private CompilerConfiguration newCompilerConfiguration() { + final CompilerConfiguration compilerConfiguration = new CompilerConfiguration(config); + compilerConfiguration.addCompilationCustomizers(getImportCustomizer(null)); + return compilerConfiguration; + } + + /** + * Compiles one runScriptOnConnector text and holds its classes in its own class cache. A class + * with a source in the script roots resolves to the engine's class, compiled by the engine if + * needed, as in the engine's own compilations; other classes resolve through the engine's + * loader. + */ + private static final class ScriptOnConnectorLoader extends GroovyClassLoader { + + private final GroovyScriptEngine engine; + + private final Object scriptRootLock; + + private ScriptOnConnectorLoader(GroovyScriptEngine engine, CompilerConfiguration configuration, + Object scriptRootLock) { + super(engine.getGroovyClassLoader(), configuration, false); + this.engine = engine; + this.scriptRootLock = scriptRootLock; + } + + @Override + protected CompilationUnit createCompilationUnit(CompilerConfiguration configuration, CodeSource source) { + final CompilationUnit unit = super.createCompilationUnit(configuration, source); + unit.setClassNodeResolver(new ClassNodeResolver() { + @Override + public LookupResult findClassNode(String name, CompilationUnit compilationUnit) { + final Class scriptRootClass = loadScriptRootClass(name, compilationUnit.getConfiguration()); + return null != scriptRootClass + ? new LookupResult(null, new ClassNode(scriptRootClass)) + : super.findClassNode(name, compilationUnit); + } + }); + return unit; + } + + /** + * Returns the engine's class for the source of the given class in the script roots, or null + * if no script root has one. + */ + private Class loadScriptRootClass(String className, CompilerConfiguration configuration) { + final String path = className.replace('.', '/'); + // The engine checks its script cache before it takes its compiler lock: without this + // lock, two texts that load a class first would get one copy of it each. + synchronized (scriptRootLock) { + for (String extension : configuration.getScriptExtensions()) { + try { + return engine.loadScriptByName(path + "." + extension); + } catch (ResourceException e) { + // No such source in the script roots. + } catch (ScriptException e) { + throw ConnectorException.wrap(e); + } + } + } + return null; + } + } + /** A compiled runScriptOnConnector script and the loader whose class cache holds its classes. */ private static final class CompiledScript { @@ -1058,7 +1123,7 @@ private void unregister() { } /** - * The runScriptOnConnector classes compiled for one engine, keyed by the script text. Holds at + * The runScriptOnConnector scripts compiled for one engine, keyed by the script text. Holds at * most {@code maxSize} scripts and unregisters the least recently used one beyond that. */ private static final class ScriptOnConnectorCache { diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index b4a8c520..45e5d2ea 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -24,8 +24,10 @@ import java.lang.ref.WeakReference; import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -79,8 +81,9 @@ public static void customize() { @BeforeMethod public void setUp() throws Exception { scriptRoot = Files.createTempDirectory("script-on-connector").toFile(); - Files.write(new File(scriptRoot, "Helper.groovy").toPath(), - "class Helper { static String greet() { 'hello' } }\n".getBytes(StandardCharsets.UTF_8)); + final File helper = write("Helper.groovy", "class Helper { static String greet() { 'hello' } }\n"); + // In the past, so that an edit is newer whatever the file system's time resolution. + assertThat(helper.setLastModified(System.currentTimeMillis() - 60000L)).isTrue(); configuration = withScriptRoot(new ScriptedConfiguration()); executor = Executors.newCachedThreadPool(); } @@ -96,10 +99,7 @@ public void tearDown() { } executor.shutdownNow(); configuration.release(); - for (File file : scriptRoot.listFiles()) { - file.delete(); - } - scriptRoot.delete(); + delete(scriptRoot); } @Test @@ -130,6 +130,70 @@ public void testScriptSeesTheClassesInTheScriptRoots() { .isEqualTo("hello"); } + @Test + public void testScriptSharesScriptRootClassesWithOperationScripts() throws Exception { + write("op.groovy", "return Helper\n"); + // The text first: the operation script must see the same Helper. + final Object fromText = configuration.evaluateScriptOnConnector("return Helper", new Binding()); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isSameAs(fromText); + } + + @Test(timeOut = 60000) + public void testScriptsCompileTogetherOnANewEngine() throws Exception { + for (int round = 0; round < 5; round++) { + for (Object result : evaluateTogetherOnANewEngine("return 1")) { + assertThat(result).isEqualTo(1); + } + } + } + + @Test(timeOut = 60000) + public void testScriptsCompiledTogetherShareTheScriptRootClasses() throws Exception { + for (int round = 0; round < 5; round++) { + final List helpers = evaluateTogetherOnANewEngine("return Helper"); + for (Object helper : helpers) { + assertThat(helper).isSameAs(helpers.get(0)); + } + } + } + + @Test + public void testScriptRootClassShadowsAClassOfTheConnectorLoader() throws Exception { + final String name = ShadowedByScriptRoot.class.getName(); + write(name.replace('.', '/') + ".groovy", "package " + ShadowedByScriptRoot.class.getPackage().getName() + + "\nclass ShadowedByScriptRoot { static String origin() { 'script root' } }\n"); + write("op.groovy", "return " + name + ".origin()\n"); + assertThat(configuration.evaluateScriptOnConnector("return " + name + ".origin()", new Binding())) + .isEqualTo("script root"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("script root"); + } + + @Test + public void testCachedScriptKeepsTheScriptRootClassItWasCompiledAgainst() throws Exception { + write("op.groovy", "return Helper.greet()\n"); + final Class cached = configuration.getScriptOnConnectorClass("return Helper.greet()"); + assertThat(configuration.evaluateScriptOnConnector("return Helper.greet()", new Binding())).isEqualTo("hello"); + + editHelper("class Helper { static String greet() { 'hi' } }\n"); + + assertThat(configuration.getScriptOnConnectorClass("return Helper.greet()")).isSameAs(cached); + assertThat(configuration.evaluateScriptOnConnector("return Helper.greet()", new Binding())).isEqualTo("hello"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("hi"); + } + + @Test + public void testNewScriptSharesTheEditedScriptRootClassWithOperationScripts() throws Exception { + write("op.groovy", "return Helper\n"); + assertThat(configuration.evaluateScriptOnConnector("return Helper.greet()", new Binding())).isEqualTo("hello"); + + editHelper("class Helper { static String greet() { 'hi' } }\n"); + + // A new text first, before any operation script has seen the edit. + final Class fromText = (Class) configuration.evaluateScriptOnConnector("return Helper", new Binding()); + assertThat(fromText.getMethod("greet").invoke(null)).isEqualTo("hi"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isSameAs(fromText); + } + @Test public void testScriptSeesItsBinding() { final Binding binding = new Binding(); @@ -158,6 +222,16 @@ public void testCustomizerCanRunAScriptOnConnector() throws Exception { assertThat(result.get()).isEqualTo(42); } + @Test + public void testScriptRunByTheCustomizerIsNotCached() throws Exception { + final AtomicReference compiled = new AtomicReference(); + customizer = () -> compiled.set(configuration.getScriptOnConnectorClass("return 1")); + writeCustomizer(); + configuration.getGroovyScriptEngine(); + assertThat(compiled.get()).isNotNull(); + assertThat(configuration.getScriptOnConnectorClass("return 1")).isNotSameAs(compiled.get()); + } + @Test(timeOut = 60000) public void testCachedScriptDoesNotWaitForAnotherCompilation() throws Exception { final ScriptedConfiguration blocking = blockingCompilations(); @@ -255,15 +329,63 @@ public void testBrokenScriptRootClassFailsAsACompilationError() throws Exception configuration.evaluateScriptOnConnector("return Broken.x()", new Binding()); } - @Test(expectedExceptions = RuntimeException.class) - public void testConnectorReportsABrokenScriptRootClassAsARuntimeException() throws Exception { + @Test(expectedExceptions = CompilationFailedException.class) + public void testConnectorReportsABrokenScriptRootClassAsACompilationError() throws Exception { writeBrokenClass(); connector().runScriptOnConnector(groovy("return Broken.x()"), null); } private void writeBrokenClass() throws IOException { - Files.write(new File(scriptRoot, "Broken.groovy").toPath(), - "class Broken { static int x() { return 1 + } }\n".getBytes(StandardCharsets.UTF_8)); + write("Broken.groovy", "class Broken { static int x() { return 1 + } }\n"); + } + + private File write(String fileName, String text) throws IOException { + final File file = new File(scriptRoot, fileName); + file.getParentFile().mkdirs(); + Files.write(file.toPath(), text.getBytes(StandardCharsets.UTF_8)); + return file; + } + + private static void delete(File file) { + final File[] children = file.listFiles(); + if (null != children) { + for (File child : children) { + delete(child); + } + } + file.delete(); + } + + /** + * Releases the configuration, so that the next call creates a new engine, then runs a variant + * of the text on each of four threads at once and returns the results. + */ + private List evaluateTogetherOnANewEngine(String text) throws Exception { + configuration.release(); + final CountDownLatch start = new CountDownLatch(1); + final List> futures = new ArrayList>(); + for (int i = 0; i < 4; i++) { + // A different text per thread, so that each one compiles. + final String variant = text + " // " + i; + futures.add(executor.submit(() -> { + start.await(); + return configuration.evaluateScriptOnConnector(variant, new Binding()); + })); + } + start.countDown(); + final List results = new ArrayList(); + for (Future future : futures) { + results.add(future.get(10, SECONDS)); + } + return results; + } + + /** Rewrites Helper.groovy so that the engine sees it as changed on its next check. */ + private void editHelper(String text) throws Exception { + // Past the minimum recompilation interval, 100 ms by default, before and after the edit. + Thread.sleep(200); + write("Helper.groovy", text); + Thread.sleep(200); } private ScriptedConfiguration withScriptRoot(ScriptedConfiguration result) { @@ -323,7 +445,7 @@ private WeakReference compileAndRun(String scriptText) { /** Runs full collections, which unload classes, until the referent is gone. */ private static boolean isCollected(WeakReference reference) throws InterruptedException { - for (int i = 0; i < 20 && null != reference.get(); i++) { + for (int i = 0; i < 100 && null != reference.get(); i++) { System.gc(); Thread.sleep(50); } diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ShadowedByScriptRoot.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ShadowedByScriptRoot.java new file mode 100644 index 00000000..ca1ccb9a --- /dev/null +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ShadowedByScriptRoot.java @@ -0,0 +1,31 @@ +/* + * The contents of this file are subject to the terms of the Common Development and + * Distribution License (the License). You may not use this file except in compliance with the + * License. + * + * You can obtain a copy of the License at legal/CDDLv1.0.txt. See the License for the + * specific language governing permission and limitations under the License. + * + * When distributing Covered Software, include this CDDL Header Notice in each file and include + * the License file at legal/CDDLv1.0.txt. If applicable, add the following below the CDDL + * Header, with the fields enclosed by brackets [] replaced by your own identifying + * information: "Portions copyright [year] [name of copyright owner]". + * + * Copyright 2026 3A Systems, LLC. + */ + +package org.forgerock.openicf.misc.scriptedcommon; + +/** + * A class of the connector's class loader that a script root of {@link ScriptOnConnectorCacheTest} + * defines again. + */ +public final class ShadowedByScriptRoot { + + private ShadowedByScriptRoot() { + } + + public static String origin() { + return "classpath"; + } +} From 37b50d1a4587ea11ab567dfe5054e646c778a222 Mon Sep 17 00:00:00 2001 From: maximthomas Date: Thu, 15 Oct 2026 21:16:15 +0300 Subject: [PATCH 6/8] [#165] Run a cached text against the classes it was compiled against, and compile as master did without a cache --- .../scriptedcommon/ScriptedConfiguration.java | 67 ++++++++++--- .../ScriptOnConnectorCacheTest.java | 96 ++++++++++++++++++- 2 files changed, 145 insertions(+), 18 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index 4d0296b4..2a4faae4 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -812,27 +812,33 @@ Class loadScript(String scriptName) { * imports. It resolves the classes of the script roots through the engine, as the engine's own * compilations do, but keeps the new classes out of the engine's class cache. No lock is held * while compiling, so a cached script does not wait for another script to compile. + *

+ * While the customizer runs, or when a subclass builds the engine without super, there is no + * cache: the text is compiled by the engine's loader, with the engine's configuration, and is + * not cached. */ Class getScriptOnConnectorClass(String scriptText) { final GroovyScriptEngine engine; final ScriptOnConnectorCache cache; synchronized (this) { engine = getGroovyScriptEngine(); - // Null while the customizer runs, or when a subclass builds the engine without super: - // what it compiles here is not cached. cache = scriptOnConnectorCache; } - if (null != cache) { - final Class cached = cache.get(scriptText); - if (null != cached) { - return cached; - } + if (null == cache) { + return engine.getGroovyClassLoader().parseClass(newCodeSource(scriptText), false); + } + final Class cached = cache.get(scriptText); + if (null != cached) { + return cached; } final GroovyClassLoader loader = new ScriptOnConnectorLoader(engine, newCompilerConfiguration(), scriptRootLock); - final Class scriptClass = loader.parseClass(new GroovyCodeSource(scriptText, - "Script" + System.currentTimeMillis() + ".groovy", "fix"), false); - return null == cache ? scriptClass : cache.put(scriptText, new CompiledScript(scriptClass, loader)); + final Class scriptClass = loader.parseClass(newCodeSource(scriptText), false); + return cache.put(scriptText, new CompiledScript(scriptClass, loader)); + } + + private static GroovyCodeSource newCodeSource(String scriptText) { + return new GroovyCodeSource(scriptText, "Script" + System.currentTimeMillis() + ".groovy", "fix"); } /** @@ -895,7 +901,10 @@ protected Log getLogger(final Class clazz) { /** The runScriptOnConnector scripts of the published engine. Guarded by {@code this}. */ private ScriptOnConnectorCache scriptOnConnectorCache = null; - /** Held while a runScriptOnConnector compilation loads a class of the script roots. */ + /** + * Held while a runScriptOnConnector compilation looks up a class name in the script roots, + * found or not, and while the engine compiles the class it found. + */ private final Object scriptRootLock = new Object(); /** @@ -1017,6 +1026,12 @@ protected URL[] getRoots(CompilerConfiguration compilerConfiguration, GroovyClas return safeRoots.toArray(new URL[safeRoots.size()]); } + /** + * Returns the import customizer of one compilation: the engine's, or one runScriptOnConnector + * text's. Called once per compilation, possibly from several threads at once. Return a new + * customizer, or add to {@code parent}, on every call: the compiler resolves the class nodes of + * its imports in place. + */ protected ImportCustomizer getImportCustomizer(ImportCustomizer parent) { final ImportCustomizer ic = null != parent ? parent : new ImportCustomizer(); for (final String imp : getImports()) { @@ -1026,6 +1041,10 @@ protected ImportCustomizer getImportCustomizer(ImportCustomizer parent) { return ic; } + /** + * Returns the star imports of one compilation. Called once per compilation, possibly from + * several threads at once. + */ protected String[] getImports() { return new String[] { ConnectorObject.class.getPackage().getName() + ".*" }; } @@ -1046,7 +1065,8 @@ private CompilerConfiguration newCompilerConfiguration() { * Compiles one runScriptOnConnector text and holds its classes in its own class cache. A class * with a source in the script roots resolves to the engine's class, compiled by the engine if * needed, as in the engine's own compilations; other classes resolve through the engine's - * loader. + * loader. The text runs against the classes it was compiled against, even when the engine has + * since recompiled a script-root class whose source changed. */ private static final class ScriptOnConnectorLoader extends GroovyClassLoader { @@ -1054,6 +1074,9 @@ private static final class ScriptOnConnectorLoader extends GroovyClassLoader { private final Object scriptRootLock; + /** The classes the compiler resolved for the text, by name. */ + private final Map resolvedClasses = new ConcurrentHashMap(); + private ScriptOnConnectorLoader(GroovyScriptEngine engine, CompilerConfiguration configuration, Object scriptRootLock) { super(engine.getGroovyClassLoader(), configuration, false); @@ -1068,14 +1091,32 @@ protected CompilationUnit createCompilationUnit(CompilerConfiguration configurat @Override public LookupResult findClassNode(String name, CompilationUnit compilationUnit) { final Class scriptRootClass = loadScriptRootClass(name, compilationUnit.getConfiguration()); - return null != scriptRootClass + final LookupResult result = null != scriptRootClass ? new LookupResult(null, new ClassNode(scriptRootClass)) : super.findClassNode(name, compilationUnit); + if (null != result && result.isClassNode() && result.getClassNode().isResolved()) { + final Class resolved = result.getClassNode().getTypeClass(); + resolvedClasses.put(resolved.getName(), resolved); + } + return result; } }); return unit; } + /** + * Returns the class the compiler resolved for the name, if any. The text's code links a class + * by name when it first runs; without this, it would get the engine's current class. + */ + @Override + public Class loadClass(String name, boolean lookupScriptFiles, boolean preferClassOverScript, + boolean resolve) throws ClassNotFoundException { + final Class resolved = resolvedClasses.get(name); + return null != resolved + ? resolved + : super.loadClass(name, lookupScriptFiles, preferClassOverScript, resolve); + } + /** * Returns the engine's class for the source of the given class in the script roots, or null * if no script root has one. diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index 45e5d2ea..e2dbf82e 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -18,10 +18,13 @@ import static java.util.concurrent.TimeUnit.SECONDS; import static org.fest.assertions.api.Assertions.assertThat; +import static org.testng.Assert.fail; import java.io.File; import java.io.IOException; import java.lang.ref.WeakReference; +import java.net.MalformedURLException; +import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; @@ -37,6 +40,7 @@ import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.classgen.GeneratorContext; import org.codehaus.groovy.control.CompilationFailedException; +import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.codehaus.groovy.reflection.ClassInfo; @@ -48,9 +52,11 @@ import org.testng.annotations.Test; import groovy.lang.Binding; +import groovy.lang.Closure; import groovy.lang.ExpandoMetaClass; import groovy.lang.GroovyClassLoader; import groovy.lang.GroovySystem; +import groovy.util.GroovyScriptEngine; /** * Tests how {@link ScriptedConfiguration} compiles, caches and releases the scripts of @@ -81,9 +87,7 @@ public static void customize() { @BeforeMethod public void setUp() throws Exception { scriptRoot = Files.createTempDirectory("script-on-connector").toFile(); - final File helper = write("Helper.groovy", "class Helper { static String greet() { 'hello' } }\n"); - // In the past, so that an edit is newer whatever the file system's time resolution. - assertThat(helper.setLastModified(System.currentTimeMillis() - 60000L)).isTrue(); + writeHelper("class Helper { static String greet() { 'hello' } }\n"); configuration = withScriptRoot(new ScriptedConfiguration()); executor = Executors.newCachedThreadPool(); } @@ -181,6 +185,24 @@ public void testCachedScriptKeepsTheScriptRootClassItWasCompiledAgainst() throws assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("hi"); } + @Test + public void testCachedScriptLinksTheScriptRootClassesItWasCompiledAgainst() throws Exception { + writeHelper("class Helper { static String greet() { 'hello' }\n" + + "static class Greeting { static String text() { 'hello' } } }\n"); + write("op.groovy", "return Helper.greet() + ' ' + Helper.Greeting.text()\n"); + final String text = "return useHelper ? Helper.greet() + ' ' + Helper.Greeting.text() : 'not run'"; + // Compiled and cached, but the code that refers to Helper has not run yet. + assertThat(configuration.evaluateScriptOnConnector(text, useHelper(false))).isEqualTo("not run"); + + editHelper("class Helper { static String greet() { 'hi' }\n" + + "static class Greeting { static String text() { 'hi' } } }\n"); + // The engine recompiles Helper for the operation script. + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("hi hi"); + + // The cached text refers to Helper and to its nested class for the first time. + assertThat(configuration.evaluateScriptOnConnector(text, useHelper(true))).isEqualTo("hello hello"); + } + @Test public void testNewScriptSharesTheEditedScriptRootClassWithOperationScripts() throws Exception { write("op.groovy", "return Helper\n"); @@ -213,6 +235,47 @@ public void testReleaseUnregistersTheCachedClasses() { .isSameAs(configuration.getGroovyScriptEngine().getGroovyClassLoader()); } + @Test + public void testReleaseUnregistersTheCachedClassesWhenTheReleaseClosureThrows() { + final Class released = configuration.getScriptOnConnectorClass("return 1"); + registerMetaClass(released); + configuration.setReleaseClosure(new Closure(this) { + public Void doCall() { + throw new IllegalStateException("release failed"); + } + }); + try { + configuration.release(); + fail("release() did not rethrow the failure of the release closure"); + } catch (IllegalStateException expected) { + // The cache is cleared anyway. + } finally { + configuration.setReleaseClosure(null); + } + assertThat(ClassInfo.getClassInfo(released).getStrongMetaClass()).isNull(); + } + + @Test + public void testEngineBuiltByASubclassCompilesTheScriptWithItsOwnConfiguration() { + configuration = withScriptRoot(new ScriptedConfiguration() { + private GroovyScriptEngine engine; + + @Override + protected synchronized GroovyScriptEngine getGroovyScriptEngine() { + if (null == engine) { + final CompilerConfiguration own = new CompilerConfiguration(); + own.addCompilationCustomizers( + new ImportCustomizer().addImport("EngineList", ArrayList.class.getName())); + engine = new GroovyScriptEngine(new URL[] { toUrl(scriptRoot) }, + new GroovyClassLoader(ScriptOnConnectorCacheTest.class.getClassLoader(), own)); + } + return engine; + } + }); + assertThat(configuration.evaluateScriptOnConnector("return new EngineList()", new Binding())) + .isInstanceOf(ArrayList.class); + } + @Test public void testCustomizerCanRunAScriptOnConnector() throws Exception { final AtomicReference result = new AtomicReference(); @@ -309,9 +372,11 @@ public void testConnectorReusesTheCompiledScript() { @Test public void testConnectorScriptSeesItsArgumentsAndTheLogger() { - final ScriptContext request = new ScriptContext("Groovy", "return [a + 1, null != log]", + // log.@clazz is the class the logger reports for. + final ScriptContext request = new ScriptContext("Groovy", "return [a + 1, log.@clazz]", Collections.singletonMap("a", 41)); - assertThat(connector().runScriptOnConnector(request, null)).isEqualTo(Arrays.asList(42, true)); + assertThat(connector().runScriptOnConnector(request, null)) + .isEqualTo(Arrays.asList(42, ScriptedConnectorBase.class)); } @Test @@ -339,6 +404,14 @@ private void writeBrokenClass() throws IOException { write("Broken.groovy", "class Broken { static int x() { return 1 + } }\n"); } + private static URL toUrl(File file) { + try { + return file.toURI().toURL(); + } catch (MalformedURLException e) { + throw new IllegalStateException(e); + } + } + private File write(String fileName, String text) throws IOException { final File file = new File(scriptRoot, fileName); file.getParentFile().mkdirs(); @@ -380,6 +453,19 @@ private List evaluateTogetherOnANewEngine(String text) throws Exception return results; } + private static Binding useHelper(boolean useHelper) { + final Binding binding = new Binding(); + binding.setVariable("useHelper", useHelper); + return binding; + } + + /** Writes Helper.groovy before the engine first loads it. */ + private void writeHelper(String text) throws IOException { + final File helper = write("Helper.groovy", text); + // In the past, so that an edit is newer whatever the file system's time resolution. + assertThat(helper.setLastModified(System.currentTimeMillis() - 60000L)).isTrue(); + } + /** Rewrites Helper.groovy so that the engine sees it as changed on its next check. */ private void editHelper(String text) throws Exception { // Past the minimum recompilation interval, 100 ms by default, before and after the edit. From 21f99b410d8567286a36b817ef9d7331d6b5ac6c Mon Sep 17 00:00:00 2001 From: maximthomas Date: Tue, 20 Oct 2026 13:34:52 +0300 Subject: [PATCH 7/8] [#165] Compile runScriptOnConnector texts under the engine's compiler lock with its configuration, and link the classes compiled with a script-root class --- .../scriptedcommon/ScriptedConfiguration.java | 180 +++++++++------ .../ScriptOnConnectorCacheTest.java | 207 ++++++++++++++++++ 2 files changed, 322 insertions(+), 65 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index 2a4faae4..27462061 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -30,6 +30,7 @@ import java.io.File; import java.io.IOException; +import java.lang.reflect.Field; import java.net.MalformedURLException; import java.net.URI; import java.net.URL; @@ -43,9 +44,11 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.CopyOnWriteArrayList; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.control.ClassNodeResolver; +import org.codehaus.groovy.control.CompilationFailedException; import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.customizers.ImportCustomizer; @@ -112,6 +115,13 @@ public class ScriptedConfiguration extends AbstractConfiguration implements Stat /** The number of runScriptOnConnector scripts kept when the system property is not set. */ static final int DEFAULT_SCRIPT_ON_CONNECTOR_CACHE_SIZE = 64; + + /** + * {@code GroovyClassLoader.sourceCache}, whose monitor a loader holds while it compiles: the + * engine compiles its scripts, and the script-root classes they use, under its loader's. Null + * if it cannot be read; runScriptOnConnector scripts are then not cached. + */ + private static final Field SOURCE_CACHE_FIELD = getSourceCacheField(); private final CompilerConfiguration config; { @@ -807,15 +817,15 @@ Class loadScript(String scriptName) { /** * Returns the class compiled from the text of a runScriptOnConnector request. *

- * Each text is compiled once per engine, by its own child loader of the engine's loader. The - * child compiles with a configuration built as the engine's is, and with it the default - * imports. It resolves the classes of the script roots through the engine, as the engine's own - * compilations do, but keeps the new classes out of the engine's class cache. No lock is held - * while compiling, so a cached script does not wait for another script to compile. + * Each text is compiled once per engine, by its own child loader of the engine's loader. As in + * the engine's own compilations, the child compiles with the engine's configuration, under the + * engine's compiler lock, and resolves the classes of the script roots through the engine; but + * it keeps the new classes out of the engine's class cache. A cached script does not wait for + * the lock. *

- * While the customizer runs, or when a subclass builds the engine without super, there is no - * cache: the text is compiled by the engine's loader, with the engine's configuration, and is - * not cached. + * While the customizer runs, when a subclass builds the engine without super, or when the + * engine's compiler lock cannot be read, there is no cache: the text is compiled by the + * engine's loader and is not cached. */ Class getScriptOnConnectorClass(String scriptText) { final GroovyScriptEngine engine; @@ -831,9 +841,14 @@ Class getScriptOnConnectorClass(String scriptText) { if (null != cached) { return cached; } - final GroovyClassLoader loader = - new ScriptOnConnectorLoader(engine, newCompilerConfiguration(), scriptRootLock); - final Class scriptClass = loader.parseClass(newCodeSource(scriptText), false); + final GroovyClassLoader loader; + final Class scriptClass; + // Under the engine's compiler lock, as the engine compiles its own scripts: the engine + // cannot recompile a script-root class while the text resolves the classes it uses. + synchronized (getCompilerLock(engine.getGroovyClassLoader())) { + loader = new ScriptOnConnectorLoader(engine, cache.configuration); + scriptClass = loader.parseClass(newCodeSource(scriptText), false); + } return cache.put(scriptText, new CompiledScript(scriptClass, loader)); } @@ -841,6 +856,26 @@ private static GroovyCodeSource newCodeSource(String scriptText) { return new GroovyCodeSource(scriptText, "Script" + System.currentTimeMillis() + ".groovy", "fix"); } + private static Field getSourceCacheField() { + try { + final Field field = GroovyClassLoader.class.getDeclaredField("sourceCache"); + field.setAccessible(true); + return field; + } catch (NoSuchFieldException | RuntimeException e) { + logger.warn(e, "runScriptOnConnector scripts are not cached: cannot read GroovyClassLoader.sourceCache"); + return null; + } + } + + /** Returns the monitor the loader holds while it compiles. */ + private static Object getCompilerLock(GroovyClassLoader loader) { + try { + return SOURCE_CACHE_FIELD.get(loader); + } catch (IllegalAccessException e) { + throw new IllegalStateException(e); + } + } + /** * Runs the text of a runScriptOnConnector request with the given binding. */ @@ -901,12 +936,6 @@ protected Log getLogger(final Class clazz) { /** The runScriptOnConnector scripts of the published engine. Guarded by {@code this}. */ private ScriptOnConnectorCache scriptOnConnectorCache = null; - /** - * Held while a runScriptOnConnector compilation looks up a class name in the script roots, - * found or not, and while the engine compiles the class it found. - */ - private final Object scriptRootLock = new Object(); - /** * Synchronised for the whole initialisation, not double-checked: the * customizer script runs against the half-initialised engine (it may call @@ -919,7 +948,9 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { return customizingGroovyScriptEngine; } - final CompilerConfiguration compilerConfiguration = newCompilerConfiguration(); + final CompilerConfiguration compilerConfiguration = + new CompilerConfiguration(config); + compilerConfiguration.addCompilationCustomizers(getImportCustomizer(null)); final GroovyClassLoader loader = new GroovyClassLoader(getParentLoader(), compilerConfiguration, true); @@ -936,7 +967,9 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { customizingGroovyScriptEngine = null; } publishedCustomizerClass = customizerClass; - scriptOnConnectorCache = new ScriptOnConnectorCache(getScriptOnConnectorCacheSize()); + scriptOnConnectorCache = null != SOURCE_CACHE_FIELD + ? new ScriptOnConnectorCache(getScriptOnConnectorCacheSize(), compilerConfiguration) + : null; groovyScriptEngine = engine; } return groovyScriptEngine; @@ -1026,12 +1059,6 @@ protected URL[] getRoots(CompilerConfiguration compilerConfiguration, GroovyClas return safeRoots.toArray(new URL[safeRoots.size()]); } - /** - * Returns the import customizer of one compilation: the engine's, or one runScriptOnConnector - * text's. Called once per compilation, possibly from several threads at once. Return a new - * customizer, or add to {@code parent}, on every call: the compiler resolves the class nodes of - * its imports in place. - */ protected ImportCustomizer getImportCustomizer(ImportCustomizer parent) { final ImportCustomizer ic = null != parent ? parent : new ImportCustomizer(); for (final String imp : getImports()) { @@ -1041,26 +1068,10 @@ protected ImportCustomizer getImportCustomizer(ImportCustomizer parent) { return ic; } - /** - * Returns the star imports of one compilation. Called once per compilation, possibly from - * several threads at once. - */ protected String[] getImports() { return new String[] { ConnectorObject.class.getPackage().getName() + ".*" }; } - /** - * Returns a new compiler configuration for an engine or for one runScriptOnConnector - * compilation. Each gets its own import customizer: the compiler resolves the class nodes of - * its imports in place, renaming them while it tries inner class names, so two compilations - * running at once must not share them. - */ - private CompilerConfiguration newCompilerConfiguration() { - final CompilerConfiguration compilerConfiguration = new CompilerConfiguration(config); - compilerConfiguration.addCompilationCustomizers(getImportCustomizer(null)); - return compilerConfiguration; - } - /** * Compiles one runScriptOnConnector text and holds its classes in its own class cache. A class * with a source in the script roots resolves to the engine's class, compiled by the engine if @@ -1072,16 +1083,20 @@ private static final class ScriptOnConnectorLoader extends GroovyClassLoader { private final GroovyScriptEngine engine; - private final Object scriptRootLock; - /** The classes the compiler resolved for the text, by name. */ private final Map resolvedClasses = new ConcurrentHashMap(); - private ScriptOnConnectorLoader(GroovyScriptEngine engine, CompilerConfiguration configuration, - Object scriptRootLock) { + /** + * The loaders of the script-root classes the compiler resolved for the text. Each defined + * the classes compiled with its class too: nested classes, closures, the other classes of + * its source. + */ + private final CopyOnWriteArrayList scriptRootLoaders = + new CopyOnWriteArrayList(); + + private ScriptOnConnectorLoader(GroovyScriptEngine engine, CompilerConfiguration configuration) { super(engine.getGroovyClassLoader(), configuration, false); this.engine = engine; - this.scriptRootLock = scriptRootLock; } @Override @@ -1090,7 +1105,10 @@ protected CompilationUnit createCompilationUnit(CompilerConfiguration configurat unit.setClassNodeResolver(new ClassNodeResolver() { @Override public LookupResult findClassNode(String name, CompilationUnit compilationUnit) { - final Class scriptRootClass = loadScriptRootClass(name, compilationUnit.getConfiguration()); + final Class scriptRootClass = loadScriptRootClass(name, compilationUnit); + if (null != scriptRootClass && scriptRootClass.getClassLoader() instanceof GroovyClassLoader) { + scriptRootLoaders.addIfAbsent((GroovyClassLoader) scriptRootClass.getClassLoader()); + } final LookupResult result = null != scriptRootClass ? new LookupResult(null, new ClassNode(scriptRootClass)) : super.findClassNode(name, compilationUnit); @@ -1105,35 +1123,63 @@ public LookupResult findClassNode(String name, CompilationUnit compilationUnit) } /** - * Returns the class the compiler resolved for the name, if any. The text's code links a class - * by name when it first runs; without this, it would get the engine's current class. + * Returns the class the compiler resolved for the name, or the class of that name compiled + * with a script-root class the compiler resolved, if any. The text's code links a class by + * name when it first runs; without this, it would get the engine's current class. Statically + * compiled code also links classes it reached only through the methods of a script-root + * class, such as their return types. */ @Override public Class loadClass(String name, boolean lookupScriptFiles, boolean preferClassOverScript, boolean resolve) throws ClassNotFoundException { final Class resolved = resolvedClasses.get(name); - return null != resolved - ? resolved - : super.loadClass(name, lookupScriptFiles, preferClassOverScript, resolve); + if (null != resolved) { + return resolved; + } + for (GroovyClassLoader scriptRootLoader : scriptRootLoaders) { + final Class compiledWith = findDefinedClass(scriptRootLoader, name); + if (null != compiledWith) { + return compiledWith; + } + } + return super.loadClass(name, lookupScriptFiles, preferClassOverScript, resolve); + } + + /** + * Adds the URL to the engine's loader, which the text's loader delegates to: a URL a text + * adds reaches the operation scripts and the other texts, as when texts compiled there. + * {@code addClasspath} calls this too. + */ + @Override + public void addURL(URL url) { + engine.getGroovyClassLoader().addURL(url); + } + + /** Returns the class of the given name that the loader defined, or null. */ + private static Class findDefinedClass(GroovyClassLoader loader, String name) { + try { + // Without looking up scripts: returns the loader's own class, or else the engine's. + final Class c = loader.loadClass(name, false, true, false); + return c.getClassLoader() == loader ? c : null; + } catch (ClassNotFoundException e) { + return null; + } } /** * Returns the engine's class for the source of the given class in the script roots, or null * if no script root has one. */ - private Class loadScriptRootClass(String className, CompilerConfiguration configuration) { + private Class loadScriptRootClass(String className, CompilationUnit compilationUnit) { final String path = className.replace('.', '/'); - // The engine checks its script cache before it takes its compiler lock: without this - // lock, two texts that load a class first would get one copy of it each. - synchronized (scriptRootLock) { - for (String extension : configuration.getScriptExtensions()) { - try { - return engine.loadScriptByName(path + "." + extension); - } catch (ResourceException e) { - // No such source in the script roots. - } catch (ScriptException e) { - throw ConnectorException.wrap(e); - } + for (String extension : compilationUnit.getConfiguration().getScriptExtensions()) { + try { + return engine.loadScriptByName(path + "." + extension); + } catch (ResourceException e) { + // No such source in the script roots. + } catch (ScriptException e) { + // The compiler reports any other exception from the resolver as a GroovyBugError. + throw new CompilationFailedException(compilationUnit.getPhase(), compilationUnit, e); } } return null; @@ -1171,12 +1217,16 @@ private static final class ScriptOnConnectorCache { private final int maxSize; + /** The configuration of the engine, which its scripts compile with. */ + private final CompilerConfiguration configuration; + /** Least recently used first. Guarded by itself. */ private final LinkedHashMap scripts = new LinkedHashMap(16, 0.75f, true); - private ScriptOnConnectorCache(int maxSize) { + private ScriptOnConnectorCache(int maxSize, CompilerConfiguration configuration) { this.maxSize = maxSize; + this.configuration = configuration; } private Class get(String scriptText) { diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index e2dbf82e..ea433ba3 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -21,10 +21,14 @@ import static org.testng.Assert.fail; import java.io.File; +import java.io.FileNotFoundException; import java.io.IOException; +import java.io.InputStream; import java.lang.ref.WeakReference; import java.net.MalformedURLException; import java.net.URL; +import java.net.URLConnection; +import java.net.URLStreamHandler; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.util.ArrayList; @@ -35,14 +39,17 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.Future; +import java.util.concurrent.FutureTask; import java.util.concurrent.atomic.AtomicReference; import org.codehaus.groovy.ast.ClassNode; import org.codehaus.groovy.classgen.GeneratorContext; import org.codehaus.groovy.control.CompilationFailedException; +import org.codehaus.groovy.control.CompilationUnit; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.SourceUnit; import org.codehaus.groovy.control.customizers.ImportCustomizer; +import org.codehaus.groovy.control.customizers.SecureASTCustomizer; import org.codehaus.groovy.reflection.ClassInfo; import org.forgerock.openicf.connectors.groovy.ScriptedPoolableConnector; import org.identityconnectors.framework.common.objects.ObjectClass; @@ -57,6 +64,7 @@ import groovy.lang.GroovyClassLoader; import groovy.lang.GroovySystem; import groovy.util.GroovyScriptEngine; +import groovy.util.ScriptException; /** * Tests how {@link ScriptedConfiguration} compiles, caches and releases the scripts of @@ -78,6 +86,12 @@ public static void customize() { /** Counted down when a compilation starts waiting for {@link #compileGate}. */ private static volatile CountDownLatch compileWaiting; + /** While set, a lookup of Trigger.groovy in a {@link #blockingRoot()} waits for it. */ + private static volatile CountDownLatch lookupGate; + + /** Counted down when a lookup starts waiting for {@link #lookupGate}. */ + private static volatile CountDownLatch lookupWaiting; + private File scriptRoot; private ScriptedConfiguration configuration; @@ -101,6 +115,11 @@ public void tearDown() { if (null != gate) { gate.countDown(); } + final CountDownLatch lookup = lookupGate; + lookupGate = null; + if (null != lookup) { + lookup.countDown(); + } executor.shutdownNow(); configuration.release(); delete(scriptRoot); @@ -161,6 +180,28 @@ public void testScriptsCompiledTogetherShareTheScriptRootClasses() throws Except } } + @Test(timeOut = 60000) + public void testTextAndOperationScriptThatLoadAScriptRootClassTogetherShareIt() throws Exception { + configuration = withBlockingRoot(); + write("op.groovy", "def h = Helper\nif (false) { Trigger.x() }\nreturn h\n"); + lookupWaiting = new CountDownLatch(1); + lookupGate = new CountDownLatch(1); + final Future operation = + executor.submit(() -> configuration.evaluate("op.groovy", new Binding(), null)); + // The engine compiles Helper together with the operation script, and stops before the end. + assertThat(lookupWaiting.await(10, SECONDS)).isTrue(); + + final FutureTask text = new FutureTask( + () -> configuration.evaluateScriptOnConnector("return Helper", new Binding())); + final Thread textThread = new Thread(text); + textThread.setDaemon(true); + textThread.start(); + awaitBlocked(textThread); + lookupGate.countDown(); + + assertThat(text.get(10, SECONDS)).isSameAs(operation.get(10, SECONDS)); + } + @Test public void testScriptRootClassShadowsAClassOfTheConnectorLoader() throws Exception { final String name = ShadowedByScriptRoot.class.getName(); @@ -203,6 +244,45 @@ public void testCachedScriptLinksTheScriptRootClassesItWasCompiledAgainst() thro assertThat(configuration.evaluateScriptOnConnector(text, useHelper(true))).isEqualTo("hello hello"); } + @Test + public void testCachedScriptLinksTheEngineClassesItWasCompiledAgainst() throws Exception { + writeHelper("class Helper { static String greet() { 'hello' } }\n" + + "class Farewell { static String text() { 'bye' } }\n"); + write("op.groovy", "return Helper.greet() + ' ' + Farewell.text()\n"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("hello bye"); + // Farewell has no source of its own: the text gets it from the engine, without Helper. + final String text = "return useHelper ? Farewell.text() : 'not run'"; + assertThat(configuration.evaluateScriptOnConnector(text, useHelper(false))).isEqualTo("not run"); + + editHelper("class Helper { static String greet() { 'hi' } }\n" + + "class Farewell { static String text() { 'ciao' } }\n"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("hi ciao"); + + assertThat(configuration.evaluateScriptOnConnector(text, useHelper(true))).isEqualTo("bye"); + } + + @Test + public void testCachedStaticScriptLinksTheClassesCompiledWithAScriptRootClass() throws Exception { + writeHelper("class Helper { static Greeting make() { new Greeting() }\n" + + "static Farewell leave() { new Farewell() }\n" + + "static class Greeting { String text() { 'hello' } } }\n" + + "class Farewell { String text() { 'bye' } }\n"); + write("op.groovy", "return Helper.make().text() + ' ' + Helper.leave().text()\n"); + // The text names neither Greeting nor Farewell: the compiler takes them from Helper's methods. + final String text = "@groovy.transform.CompileStatic\n" + + "String use(boolean b) { b ? Helper.make().text() + ' ' + Helper.leave().text() : 'not run' }\n" + + "return use((boolean) useHelper)\n"; + assertThat(configuration.evaluateScriptOnConnector(text, useHelper(false))).isEqualTo("not run"); + + editHelper("class Helper { static Greeting make() { new Greeting() }\n" + + "static Farewell leave() { new Farewell() }\n" + + "static class Greeting { String text() { 'hi' } } }\n" + + "class Farewell { String text() { 'ciao' } }\n"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("hi ciao"); + + assertThat(configuration.evaluateScriptOnConnector(text, useHelper(true))).isEqualTo("hello bye"); + } + @Test public void testNewScriptSharesTheEditedScriptRootClassWithOperationScripts() throws Exception { write("op.groovy", "return Helper\n"); @@ -216,6 +296,25 @@ public void testNewScriptSharesTheEditedScriptRootClassWithOperationScripts() th assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isSameAs(fromText); } + @Test + public void testUrlThatATextAddsToItsLoaderReachesTheOperationScripts() throws Exception { + final File lib = Files.createTempDirectory("script-on-connector-lib").toFile(); + try { + final CompilerConfiguration libConfiguration = new CompilerConfiguration(); + libConfiguration.setTargetDirectory(lib); + final CompilationUnit unit = new CompilationUnit(libConfiguration); + unit.addSource("Driver.groovy", "package drv\nclass Driver { static String id() { 'drv' } }\n"); + unit.compile(); + + configuration.evaluateScriptOnConnector( + "this.class.classLoader.addURL(new URL('" + toUrl(lib) + "'))", new Binding()); + write("op.groovy", "return this.class.classLoader.loadClass('drv.Driver').id()\n"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("drv"); + } finally { + delete(lib); + } + } + @Test public void testScriptSeesItsBinding() { final Binding binding = new Binding(); @@ -276,6 +375,29 @@ protected synchronized GroovyScriptEngine getGroovyScriptEngine() { .isInstanceOf(ArrayList.class); } + @Test + public void testScriptCompilesWithWhatASubclassAddsToTheEngineConfiguration() throws Exception { + configuration = withScriptRoot(new ScriptedConfiguration() { + @Override + protected URL[] getRoots(CompilerConfiguration compilerConfiguration, GroovyClassLoader loader) { + compilerConfiguration.addCompilationCustomizers( + new ImportCustomizer().addImport("RootsList", ArrayList.class.getName())); + final SecureASTCustomizer secure = new SecureASTCustomizer(); + secure.setMethodDefinitionAllowed(false); + compilerConfiguration.addCompilationCustomizers(secure); + return super.getRoots(compilerConfiguration, loader); + } + }); + assertThat(configuration.evaluateScriptOnConnector("return new RootsList()", new Binding())) + .isInstanceOf(ArrayList.class); + try { + configuration.evaluateScriptOnConnector("def f() { 1 }\nreturn f()", new Binding()); + fail("A text defined a method that the engine's SecureASTCustomizer does not allow"); + } catch (CompilationFailedException expected) { + assertThat(expected.getMessage()).contains("Method definitions are not allowed"); + } + } + @Test public void testCustomizerCanRunAScriptOnConnector() throws Exception { final AtomicReference result = new AtomicReference(); @@ -400,6 +522,34 @@ public void testConnectorReportsABrokenScriptRootClassAsACompilationError() thro connector().runScriptOnConnector(groovy("return Broken.x()"), null); } + @Test + public void testScriptExceptionFromTheEngineFailsAsACompilationError() { + configuration = withScriptRoot(new ScriptedConfiguration() { + private GroovyScriptEngine failing; + + @Override + protected synchronized GroovyScriptEngine getGroovyScriptEngine() { + final GroovyScriptEngine engine = super.getGroovyScriptEngine(); + if (null == failing) { + // A GroovyScriptEngine subclass may throw what loadScriptByName declares. + failing = new GroovyScriptEngine(new URL[] { toUrl(scriptRoot) }, engine.getGroovyClassLoader()) { + @Override + public Class loadScriptByName(String scriptName) throws ScriptException { + throw new ScriptException("Cannot load " + scriptName); + } + }; + } + return failing; + } + }); + try { + configuration.evaluateScriptOnConnector("return Helper.greet()", new Binding()); + fail("The text compiled although the engine could not load the script-root classes"); + } catch (CompilationFailedException expected) { + assertThat(expected.getCause()).isInstanceOf(ScriptException.class); + } + } + private void writeBrokenClass() throws IOException { write("Broken.groovy", "class Broken { static int x() { return 1 + } }\n"); } @@ -474,6 +624,63 @@ private void editHelper(String text) throws Exception { Thread.sleep(200); } + /** A configuration with a {@link #blockingRoot()} after the script root. */ + private ScriptedConfiguration withBlockingRoot() { + return withScriptRoot(new ScriptedConfiguration() { + @Override + protected URL[] getRoots(CompilerConfiguration compilerConfiguration, GroovyClassLoader loader) { + final URL[] roots = super.getRoots(compilerConfiguration, loader); + final URL[] result = Arrays.copyOf(roots, roots.length + 1); + result[roots.length] = blockingRoot(); + return result; + } + }); + } + + /** Waits until the thread has been blocked on a monitor for a while, for at most 10 s. */ + private static void awaitBlocked(Thread thread) throws InterruptedException { + for (int i = 0, blocked = 0; i < 1000 && blocked < 5; i++) { + Thread.sleep(10); + blocked = Thread.State.BLOCKED == thread.getState() ? blocked + 1 : 0; + } + } + + /** + * A script root without sources. While {@link #lookupGate} is set, a lookup of Trigger.groovy + * in it waits for the gate. + */ + private static URL blockingRoot() { + try { + return new URL(null, "blocking:/", new URLStreamHandler() { + @Override + protected URLConnection openConnection(final URL url) { + return new URLConnection(url) { + @Override + public void connect() throws IOException { + throw new FileNotFoundException(url.toString()); + } + + @Override + public InputStream getInputStream() throws IOException { + final CountDownLatch gate = lookupGate; + if (null != gate && "/Trigger.groovy".equals(url.getPath())) { + lookupWaiting.countDown(); + try { + gate.await(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + throw new FileNotFoundException(url.toString()); + } + }; + } + }); + } catch (MalformedURLException e) { + throw new IllegalStateException(e); + } + } + private ScriptedConfiguration withScriptRoot(ScriptedConfiguration result) { result.setScriptRoots(new String[] { scriptRoot.getAbsolutePath() }); return result; From 7b06bd48187600a1578540dcb2e9d2d40b65041b Mon Sep 17 00:00:00 2001 From: maximthomas Date: Tue, 20 Oct 2026 14:15:26 +0300 Subject: [PATCH 8/8] [#165] Compile the classes a runScriptOnConnector text parses at run time with the engine's loader A text's InnerLoader delegates parseClass to the loader that compiled the text. ScriptOnConnectorLoader now passes it on to the engine's loader, as master did, so such a class compiles under the engine's compiler lock and reaches the operation scripts. The text itself compiles through compileText(). --- .../scriptedcommon/ScriptedConfiguration.java | 21 +++++++++-- .../ScriptOnConnectorCacheTest.java | 35 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java index 27462061..e0eaefe4 100644 --- a/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java +++ b/OpenICF-groovy-connector/src/main/java/org/forgerock/openicf/misc/scriptedcommon/ScriptedConfiguration.java @@ -841,13 +841,13 @@ Class getScriptOnConnectorClass(String scriptText) { if (null != cached) { return cached; } - final GroovyClassLoader loader; + final ScriptOnConnectorLoader loader; final Class scriptClass; // Under the engine's compiler lock, as the engine compiles its own scripts: the engine // cannot recompile a script-root class while the text resolves the classes it uses. synchronized (getCompilerLock(engine.getGroovyClassLoader())) { loader = new ScriptOnConnectorLoader(engine, cache.configuration); - scriptClass = loader.parseClass(newCodeSource(scriptText), false); + scriptClass = loader.compileText(newCodeSource(scriptText)); } return cache.put(scriptText, new CompiledScript(scriptClass, loader)); } @@ -1155,6 +1155,23 @@ public void addURL(URL url) { engine.getGroovyClassLoader().addURL(url); } + /** Compiles the text into this loader's class cache. */ + private Class compileText(GroovyCodeSource codeSource) { + return super.parseClass(codeSource, false); + } + + /** + * Compiles the source with the engine's loader, which the text's loader delegates to: a + * class a text compiles at run time compiles under the engine's compiler lock and reaches + * the operation scripts and the other texts, as when texts compiled there. The other + * {@code parseClass} methods call this too. + */ + @Override + public Class parseClass(GroovyCodeSource codeSource, boolean shouldCacheSource) + throws CompilationFailedException { + return engine.getGroovyClassLoader().parseClass(codeSource, shouldCacheSource); + } + /** Returns the class of the given name that the loader defined, or null. */ private static Class findDefinedClass(GroovyClassLoader loader, String name) { try { diff --git a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java index ea433ba3..c9ce85aa 100644 --- a/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java +++ b/OpenICF-groovy-connector/src/test/java/org/forgerock/openicf/misc/scriptedcommon/ScriptOnConnectorCacheTest.java @@ -315,6 +315,41 @@ public void testUrlThatATextAddsToItsLoaderReachesTheOperationScripts() throws E } } + @Test + public void testClassThatATextParsesReachesTheOperationScripts() throws Exception { + final GroovyClassLoader textLoader = (GroovyClassLoader) configuration.evaluateScriptOnConnector( + "return this.class.classLoader", new Binding()); + textLoader.parseClass("class Parsed { static String id() { 'parsed' } }"); + write("op.groovy", "return Parsed.id()\n"); + assertThat(configuration.evaluate("op.groovy", new Binding(), null)).isEqualTo("parsed"); + } + + @Test(timeOut = 60000) + public void testClassThatATextParsesWaitsForTheEngineCompilation() throws Exception { + configuration = withBlockingRoot(); + final GroovyClassLoader textLoader = (GroovyClassLoader) configuration.evaluateScriptOnConnector( + "return this.class.classLoader", new Binding()); + write("op.groovy", "if (false) { Trigger.x() }\nreturn 1\n"); + lookupWaiting = new CountDownLatch(1); + lookupGate = new CountDownLatch(1); + final Future operation = + executor.submit(() -> configuration.evaluate("op.groovy", new Binding(), null)); + // The engine compiles the operation script, and stops before the end. + assertThat(lookupWaiting.await(10, SECONDS)).isTrue(); + + final FutureTask parsed = new FutureTask( + () -> textLoader.parseClass("class Parsed { static String id() { 'parsed' } }")); + final Thread parseThread = new Thread(parsed); + parseThread.setDaemon(true); + parseThread.start(); + awaitBlocked(parseThread); + assertThat(parsed.isDone()).isFalse(); + lookupGate.countDown(); + + assertThat(operation.get(10, SECONDS)).isEqualTo(1); + assertThat(parsed.get(10, SECONDS).getName()).isEqualTo("Parsed"); + } + @Test public void testScriptSeesItsBinding() { final Binding binding = new Binding();