This is an automated email from the ASF dual-hosted git repository. shuber pushed a commit to branch UNOMI-897-groovy-fixes in repository https://gitbox.apache.org/repos/asf/unomi.git
commit 0076cccb9b281a6c6974b8c08936aabcbc51ec92 Author: Serge Huber <[email protected]> AuthorDate: Sat Jul 12 21:02:42 2025 +0200 UNOMI-897: Fix data corruption and performance issues in GroovyActionDispatcher - Fixed data corruption issue by removing shared GroovyShell instance with separate script instances for separate variables - Improved performance by moving to pre-compilation of scripts to avoid on-the-fly and previously systematic compilation of Groovy scripts - Add ScriptMetadata class for script management --- .../groovy/actions/GroovyActionDispatcher.java | 69 ++-- .../actions/services/GroovyActionsService.java | 95 +++++- .../services/impl/GroovyActionsServiceImpl.java | 355 +++++++++++++++++---- 3 files changed, 420 insertions(+), 99 deletions(-) diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java index 87ed6108b..224e87752 100644 --- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/GroovyActionDispatcher.java @@ -16,33 +16,36 @@ */ package org.apache.unomi.groovy.actions; -import groovy.lang.GroovyCodeSource; -import groovy.lang.GroovyShell; import groovy.lang.Script; import org.apache.unomi.api.Event; import org.apache.unomi.api.actions.Action; import org.apache.unomi.api.actions.ActionDispatcher; +import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.groovy.actions.services.GroovyActionsService; import org.apache.unomi.metrics.MetricAdapter; import org.apache.unomi.metrics.MetricsService; +import org.apache.unomi.services.actions.ActionExecutorDispatcher; import org.osgi.service.component.annotations.Component; import org.osgi.service.component.annotations.Reference; import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * An implementation of an ActionDispatcher for the Groovy language. This dispatcher will load the groovy action script matching to an - * actionName. If a script if found, it will be executed. + * High-performance ActionDispatcher for pre-compiled Groovy scripts. + * Executes scripts without GroovyShell overhead using isolated instances. */ @Component(service = ActionDispatcher.class) public class GroovyActionDispatcher implements ActionDispatcher { private static final Logger LOGGER = LoggerFactory.getLogger(GroovyActionDispatcher.class.getName()); + private static final Logger GROOVY_ACTION_LOGGER = LoggerFactory.getLogger("GroovyAction"); private static final String GROOVY_PREFIX = "groovy"; private MetricsService metricsService; private GroovyActionsService groovyActionsService; + private DefinitionsService definitionsService; + private ActionExecutorDispatcher actionExecutorDispatcher; @Reference public void setMetricsService(MetricsService metricsService) { @@ -54,30 +57,52 @@ public class GroovyActionDispatcher implements ActionDispatcher { this.groovyActionsService = groovyActionsService; } + @Reference + public void setDefinitionsService(DefinitionsService definitionsService) { + this.definitionsService = definitionsService; + } + + @Reference + public void setActionExecutorDispatcher(ActionExecutorDispatcher actionExecutorDispatcher) { + this.actionExecutorDispatcher = actionExecutorDispatcher; + } + public String getPrefix() { return GROOVY_PREFIX; } public Integer execute(Action action, Event event, String actionName) { - GroovyCodeSource groovyCodeSource = groovyActionsService.getGroovyCodeSource(actionName); - if (groovyCodeSource == null) { - LOGGER.warn("Couldn't find a Groovy action with name {}, action will not execute !", actionName); - } else { - GroovyShell groovyShell = groovyActionsService.getGroovyShell(); - groovyShell.setVariable("action", action); - groovyShell.setVariable("event", event); - Script script = groovyShell.parse(groovyCodeSource); - try { - return new MetricAdapter<Integer>(metricsService, this.getClass().getName() + ".action.groovy." + actionName) { - @Override - public Integer execute(Object... args) throws Exception { - return (Integer) script.invokeMethod("execute", null); - } - }.runWithTimer(); - } catch (Exception e) { - LOGGER.error("Error executing Groovy action with key={}", actionName, e); - } + Class<? extends Script> scriptClass = groovyActionsService.getCompiledScript(actionName); + if (scriptClass == null) { + LOGGER.warn("Couldn't find a Groovy action with name {}, action will not execute!", actionName); + return 0; + } + + try { + Script script = scriptClass.newInstance(); + setScriptVariables(script, action, event); + + return new MetricAdapter<Integer>(metricsService, this.getClass().getName() + ".action.groovy." + actionName) { + @Override + public Integer execute(Object... args) throws Exception { + return (Integer) script.invokeMethod("execute", null); + } + }.runWithTimer(); + + } catch (Exception e) { + LOGGER.error("Error executing Groovy action with key={}", actionName, e); } return 0; } + + /** + * Sets required variables on script instance. + */ + private void setScriptVariables(Script script, Action action, Event event) { + script.setProperty("action", action); + script.setProperty("event", event); + script.setProperty("actionExecutorDispatcher", actionExecutorDispatcher); + script.setProperty("definitionsService", definitionsService); + script.setProperty("logger", GROOVY_ACTION_LOGGER); + } } diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java index 4b6d54505..aa6bf8b1b 100644 --- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/GroovyActionsService.java @@ -16,43 +16,106 @@ */ package org.apache.unomi.groovy.actions.services; -import groovy.lang.GroovyCodeSource; -import groovy.lang.GroovyShell; -import groovy.util.GroovyScriptEngine; +import groovy.lang.Script; import org.apache.unomi.groovy.actions.GroovyAction; +import org.apache.unomi.groovy.actions.ScriptMetadata; + /** - * A service to load groovy files and manage {@link GroovyAction} + * Service interface for managing Groovy action scripts. + * <p> + * This service provides functionality to load, compile, cache, and execute + * Groovy scripts as actions within the Apache Unomi framework. It implements + * optimized compilation and caching strategies to achieve high performance. + * </p> + * + * <p> + * Key features: + * <ul> + * <li>Pre-compilation of scripts at startup</li> + * <li>Hash-based change detection for selective recompilation</li> + * <li>Thread-safe compilation and execution</li> + * <li>Unified caching architecture for compiled scripts</li> + * </ul> + * </p> + * + * <p> + * Thread Safety: Implementations must be thread-safe as this service + * is accessed concurrently during script execution. + * </p> + * + * @see GroovyAction + * @see ScriptMetadata + * @since 2.7.0 */ public interface GroovyActionsService { /** - * Save a groovy action from a groovy file + * Saves a Groovy action script with compilation and validation. + * <p> + * This method compiles the script, validates it has the required + * annotations, persists it, and updates the internal cache. + * If the script content hasn't changed, recompilation is skipped. + * </p> * - * @param actionName actionName - * @param groovyScript script to save + * @param actionName the unique identifier for the action + * @param groovyScript the Groovy script source code + * @throws IllegalArgumentException if actionName or groovyScript is null + * @throws RuntimeException if compilation or persistence fails */ void save(String actionName, String groovyScript); /** - * Remove a groovy action + * Removes a Groovy action and all associated metadata. + * <p> + * This method removes the action from both the cache and persistent storage, + * and cleans up any registered action types in the definitions service. + * </p> * - * @param id of the action to remove + * @param id the unique identifier of the action to remove + * @throws IllegalArgumentException if id is null */ void remove(String id); /** - * Get a groovy code source object by an id + * Retrieves a compiled script class, compiling on-demand if not cached. + * <p> + * This method first checks the cache for a compiled version. If not found, + * it loads the script from persistence and compiles it. This method is + * provided for backward compatibility but may have performance implications. + * </p> + * + * @param id the unique identifier of the action + * @return the compiled script class, or {@code null} if not found + * @throws IllegalArgumentException if id is null + */ + Class<?> getOrCompileScript(String id); + + /** + * Retrieves a pre-compiled script class from cache. + * <p> + * This is the preferred method for script execution as it returns + * pre-compiled classes without any compilation overhead. Returns + * {@code null} if the script is not found in the cache. + * </p> * - * @param id of the action to get - * @return Groovy code source + * @param id the unique identifier of the action + * @return the compiled script class, or {@code null} if not found in cache + * @throws IllegalArgumentException if id is null */ - GroovyCodeSource getGroovyCodeSource(String id); + Class<? extends Script> getCompiledScript(String id); /** - * Get an instantiated groovy shell object + * Retrieves script metadata for monitoring and change detection. + * <p> + * The returned metadata includes content hash, compilation timestamp, + * and the compiled class reference. This is useful for monitoring + * tools and debugging. + * </p> * - * @return GroovyShell + * @param actionName the unique identifier of the action + * @return the script metadata, or {@code null} if not found + * @throws IllegalArgumentException if actionName is null */ - GroovyShell getGroovyShell(); + ScriptMetadata getScriptMetadata(String actionName); } diff --git a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java index bee011cd6..25a42ffbb 100644 --- a/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/services/impl/GroovyActionsServiceImpl.java @@ -19,6 +19,7 @@ package org.apache.unomi.groovy.actions.services.impl; import groovy.lang.GroovyClassLoader; import groovy.lang.GroovyCodeSource; import groovy.lang.GroovyShell; +import groovy.lang.Script; import groovy.util.GroovyScriptEngine; import org.apache.commons.io.IOUtils; import org.apache.unomi.api.Metadata; @@ -27,10 +28,10 @@ import org.apache.unomi.api.services.DefinitionsService; import org.apache.unomi.api.services.SchedulerService; import org.apache.unomi.groovy.actions.GroovyAction; import org.apache.unomi.groovy.actions.GroovyBundleResourceConnector; +import org.apache.unomi.groovy.actions.ScriptMetadata; import org.apache.unomi.groovy.actions.annotations.Action; import org.apache.unomi.groovy.actions.services.GroovyActionsService; import org.apache.unomi.persistence.spi.PersistenceService; -import org.apache.unomi.services.actions.ActionExecutorDispatcher; import org.codehaus.groovy.control.CompilerConfiguration; import org.codehaus.groovy.control.customizers.ImportCustomizer; import org.osgi.framework.BundleContext; @@ -43,10 +44,12 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.net.URL; -import java.util.HashMap; +import java.nio.charset.StandardCharsets; import java.util.HashSet; + import java.util.Map; import java.util.TimerTask; +import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.stream.Collectors; @@ -55,7 +58,8 @@ import java.util.stream.Stream; import static java.util.Arrays.asList; /** - * Implementation of the GroovyActionService. Allows to create a groovy action from a groovy file + * High-performance GroovyActionsService implementation with pre-compilation, + * hash-based change detection, and thread-safe execution. */ @Component(service = GroovyActionsService.class, configurationPid = "org.apache.unomi.groovy.actions") @Designate(ocd = GroovyActionsServiceImpl.GroovyActionsServiceConfig.class) @@ -68,9 +72,12 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { private BundleContext bundleContext; private GroovyScriptEngine groovyScriptEngine; - private GroovyShell groovyShell; - private Map<String, GroovyCodeSource> groovyCodeSourceMap; + private CompilerConfiguration compilerConfiguration; private ScheduledFuture<?> scheduledFuture; + + private final Object compilationLock = new Object(); + private GroovyShell compilationShell; + private volatile Map<String, ScriptMetadata> scriptMetadataCache = new ConcurrentHashMap<>(); private static final Logger LOGGER = LoggerFactory.getLogger(GroovyActionsServiceImpl.class.getName()); private static final String BASE_SCRIPT_NAME = "BaseScript"; @@ -78,7 +85,6 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { private DefinitionsService definitionsService; private PersistenceService persistenceService; private SchedulerService schedulerService; - private ActionExecutorDispatcher actionExecutorDispatcher; private GroovyActionsServiceConfig config; @Reference @@ -96,14 +102,7 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { this.schedulerService = schedulerService; } - @Reference - public void setActionExecutorDispatcher(ActionExecutorDispatcher actionExecutorDispatcher) { - this.actionExecutorDispatcher = actionExecutorDispatcher; - } - public GroovyShell getGroovyShell() { - return groovyShell; - } @Activate public void start(GroovyActionsServiceConfig config, BundleContext bundleContext) { @@ -111,20 +110,25 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { this.config = config; this.bundleContext = bundleContext; - this.groovyCodeSourceMap = new HashMap<>(); GroovyBundleResourceConnector bundleResourceConnector = new GroovyBundleResourceConnector(bundleContext); GroovyClassLoader groovyLoader = new GroovyClassLoader(bundleContext.getBundle().adapt(BundleWiring.class).getClassLoader()); this.groovyScriptEngine = new GroovyScriptEngine(bundleResourceConnector, groovyLoader); - initializeGroovyShell(); + // Initialize Groovy compiler and compilation shell + initializeGroovyCompiler(); + try { loadBaseScript(); } catch (IOException e) { LOGGER.error("Failed to load base script", e); } + + // PRE-COMPILE ALL SCRIPTS AT STARTUP (no on-demand compilation) + preloadAllScripts(); + initializeTimers(); - LOGGER.info("Groovy action service initialized."); + LOGGER.info("Groovy action service initialized with {} scripts", scriptMetadataCache.size()); } @Deactivate @@ -136,15 +140,7 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { } /** - * Load the Base script. - * It's a script which provides utility functions that we can use in other groovy script - * The functions added by the base script could be called by the groovy actions executed in - * {@link org.apache.unomi.groovy.actions.GroovyActionDispatcher#execute} - * The base script would be added in the configuration of the {@link GroovyActionsServiceImpl#groovyShell GroovyShell} , so when a - * script will be parsed with the GroovyShell (groovyShell.parse(...)), the action will extends the base script, so the functions - * could be called - * - * @throws IOException + * Loads the base script that provides utility functions for Groovy actions. */ private void loadBaseScript() throws IOException { URL groovyBaseScriptURL = bundleContext.getBundle().getEntry("META-INF/base/BaseScript.groovy"); @@ -152,25 +148,78 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { return; } LOGGER.debug("Found Groovy base script at {}, loading... ", groovyBaseScriptURL.getPath()); - GroovyCodeSource groovyCodeSource = new GroovyCodeSource(IOUtils.toString(groovyBaseScriptURL.openStream()), BASE_SCRIPT_NAME, "/groovy/script"); + GroovyCodeSource groovyCodeSource = new GroovyCodeSource(IOUtils.toString(groovyBaseScriptURL.openStream(), StandardCharsets.UTF_8), BASE_SCRIPT_NAME, "/groovy/script"); groovyScriptEngine.getGroovyClassLoader().parseClass(groovyCodeSource, true); } /** - * Initialize the groovyShell object and define the configuration which contains the name of the base script + * Initializes compiler configuration and shared compilation shell. */ - private void initializeGroovyShell() { - CompilerConfiguration compilerConfiguration = new CompilerConfiguration(); + private void initializeGroovyCompiler() { + // Configure the compiler with imports and base script + compilerConfiguration = new CompilerConfiguration(); compilerConfiguration.addCompilationCustomizers(createImportCustomizer()); - compilerConfiguration.setScriptBaseClass(BASE_SCRIPT_NAME); groovyScriptEngine.setConfig(compilerConfiguration); - groovyShell = new GroovyShell(groovyScriptEngine.getGroovyClassLoader(), compilerConfiguration); - groovyShell.setVariable("actionExecutorDispatcher", actionExecutorDispatcher); - groovyShell.setVariable("definitionsService", definitionsService); - groovyShell.setVariable("logger", LoggerFactory.getLogger("GroovyAction")); + + // Create single shared shell for compilation only + this.compilationShell = new GroovyShell(groovyScriptEngine.getGroovyClassLoader(), compilerConfiguration); + } + + /** + * Pre-compiles all scripts at startup to eliminate runtime compilation overhead. + */ + private void preloadAllScripts() { + long startTime = System.currentTimeMillis(); + LOGGER.info("Pre-compiling all Groovy scripts at startup..."); + + int successCount = 0; + int failureCount = 0; + long totalCompilationTime = 0; + + for (GroovyAction groovyAction : persistenceService.getAllItems(GroovyAction.class)) { + try { + String actionName = groovyAction.getName(); + String scriptContent = groovyAction.getScript(); + + long scriptStartTime = System.currentTimeMillis(); + Class<? extends Script> scriptClass = compileScript(actionName, scriptContent); + long scriptCompilationTime = System.currentTimeMillis() - scriptStartTime; + totalCompilationTime += scriptCompilationTime; + + ScriptMetadata metadata = new ScriptMetadata(actionName, scriptContent, scriptClass); + scriptMetadataCache.put(actionName, metadata); + + successCount++; + LOGGER.debug("Pre-compiled script: {} ({}ms)", actionName, scriptCompilationTime); + + } catch (Exception e) { + failureCount++; + LOGGER.error("Failed to pre-compile script: {}", groovyAction.getName(), e); + } + } + + long totalTime = System.currentTimeMillis() - startTime; + LOGGER.info("Pre-compilation completed: {} scripts successfully compiled, {} failures. Total time: {}ms", + successCount, failureCount, totalTime); + LOGGER.debug("Pre-compilation metrics: Average per script: {}ms, Compilation overhead: {}ms", + successCount > 0 ? totalCompilationTime / successCount : 0, + totalTime - totalCompilationTime); + } + + /** + * Thread-safe script compilation using synchronized shared shell. + */ + private Class<? extends Script> compileScript(String actionName, String scriptContent) { + GroovyCodeSource codeSource = buildClassScript(scriptContent, actionName); + synchronized(compilationLock) { + return compilationShell.parse(codeSource).getClass(); + } } + /** + * Creates import customizer with standard Unomi imports. + */ private ImportCustomizer createImportCustomizer() { ImportCustomizer importCustomizer = new ImportCustomizer(); importCustomizer.addImports("org.apache.unomi.api.services.EventService", "org.apache.unomi.groovy.actions.annotations.Action", @@ -178,25 +227,60 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { return importCustomizer; } + /** + * {@inheritDoc} + * Implementation performs hash-based change detection to skip unnecessary recompilation. + */ @Override public void save(String actionName, String groovyScript) { - GroovyCodeSource groovyCodeSource = buildClassScript(groovyScript, actionName); + if (actionName == null || actionName.trim().isEmpty()) { + throw new IllegalArgumentException("Action name cannot be null or empty"); + } + if (groovyScript == null || groovyScript.trim().isEmpty()) { + throw new IllegalArgumentException("Groovy script cannot be null or empty"); + } + + long startTime = System.currentTimeMillis(); + LOGGER.info("Saving script: {}", actionName); + try { - saveActionType(groovyShell.parse(groovyCodeSource).getClass().getMethod("execute").getAnnotation(Action.class)); + ScriptMetadata existingMetadata = scriptMetadataCache.get(actionName); + if (existingMetadata != null && !existingMetadata.hasChanged(groovyScript)) { + LOGGER.info("Script {} unchanged, skipping recompilation ({}ms)", actionName, + System.currentTimeMillis() - startTime); + return; + } + + long compilationStartTime = System.currentTimeMillis(); + Class<? extends Script> scriptClass = compileScript(actionName, groovyScript); + long compilationTime = System.currentTimeMillis() - compilationStartTime; + + Action actionAnnotation = scriptClass.getMethod("execute").getAnnotation(Action.class); + if (actionAnnotation != null) { + saveActionType(actionAnnotation); + } + saveScript(actionName, groovyScript); - LOGGER.info("The script {} has been loaded.", actionName); - } catch (NoSuchMethodException e) { - LOGGER.error("Failed to save the script {}", actionName, e); + + ScriptMetadata metadata = new ScriptMetadata(actionName, groovyScript, scriptClass); + scriptMetadataCache.put(actionName, metadata); + + long totalTime = System.currentTimeMillis() - startTime; + LOGGER.info("Script {} saved and compiled successfully (total: {}ms, compilation: {}ms)", + actionName, totalTime, compilationTime); + + } catch (Exception e) { + long totalTime = System.currentTimeMillis() - startTime; + LOGGER.error("Failed to save script: {} ({}ms)", actionName, totalTime, e); + throw new RuntimeException("Failed to save script: " + actionName, e); } } /** - * Build an action type from the annotation {@link Action} - * - * @param action Annotation containing the values to save + * Builds and registers ActionType from Action annotation. */ private void saveActionType(Action action) { - Metadata metadata = new Metadata(null, action.id(), action.name().equals("") ? action.id() : action.name(), action.description()); + Metadata metadata = new Metadata(null, action.id(), action.name().isEmpty() ? action.id() : action.name(), action.description()); metadata.setHidden(action.hidden()); metadata.setReadOnly(true); metadata.setSystemTags(new HashSet<>(asList(action.systemTags()))); @@ -209,48 +293,197 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { definitionsService.setActionType(actionType); } + /** + * {@inheritDoc} + */ @Override public void remove(String id) { - if (groovyCodeSourceMap.containsKey(id)) { - try { - definitionsService.removeActionType( - groovyShell.parse(groovyCodeSourceMap.get(id)).getClass().getMethod("execute").getAnnotation(Action.class).id()); - } catch (NoSuchMethodException e) { - LOGGER.error("Failed to delete the action type for the id {}", id, e); + if (id == null || id.trim().isEmpty()) { + throw new IllegalArgumentException("Script ID cannot be null or empty"); + } + + LOGGER.info("Removing script: {}", id); + + ScriptMetadata removedMetadata = scriptMetadataCache.remove(id); + persistenceService.remove(id, GroovyAction.class); + + try { + if (removedMetadata != null) { + Class<? extends Script> cachedClass = removedMetadata.getCompiledClass(); + Action actionAnnotation = cachedClass.getMethod("execute").getAnnotation(Action.class); + if (actionAnnotation != null) { + definitionsService.removeActionType(actionAnnotation.id()); + } } - persistenceService.remove(id, GroovyAction.class); + } catch (Exception e) { + LOGGER.error("Failed to remove action type for script: {}", id, e); } + + LOGGER.info("Script {} removed successfully", id); } + + /** + * {@inheritDoc} + * Performance Warning: Compiles on-demand if not cached. + */ + @Override + public Class<? extends Script> getOrCompileScript(String id) { + if (id == null || id.trim().isEmpty()) { + throw new IllegalArgumentException("Script ID cannot be null or empty"); + } + + ScriptMetadata metadata = scriptMetadataCache.get(id); + if (metadata != null) { + return metadata.getCompiledClass(); + } + + long startTime = System.currentTimeMillis(); + LOGGER.warn("Script {} not found in cache, compiling on-demand (performance warning)", id); + + GroovyAction groovyAction = persistenceService.load(id, GroovyAction.class); + if (groovyAction == null) { + LOGGER.warn("Script {} not found in persistence, returning null ({}ms)", id, + System.currentTimeMillis() - startTime); + return null; + } + + try { + long compilationStartTime = System.currentTimeMillis(); + Class<? extends Script> scriptClass = compileScript(id, groovyAction.getScript()); + long compilationTime = System.currentTimeMillis() - compilationStartTime; + + ScriptMetadata newMetadata = new ScriptMetadata(id, groovyAction.getScript(), scriptClass); + scriptMetadataCache.put(id, newMetadata); + + long totalTime = System.currentTimeMillis() - startTime; + LOGGER.warn("On-demand compilation completed for {} (total: {}ms, compilation: {}ms)", + id, totalTime, compilationTime); + return scriptClass; + } catch (Exception e) { + long totalTime = System.currentTimeMillis() - startTime; + LOGGER.error("Failed to compile script {} on-demand ({}ms)", id, totalTime, e); + return null; + } + } + + /** + * {@inheritDoc} + */ + @Override + public Class<? extends Script> getCompiledScript(String id) { + if (id == null || id.trim().isEmpty()) { + throw new IllegalArgumentException("Script ID cannot be null or empty"); + } + + ScriptMetadata metadata = scriptMetadataCache.get(id); + if (metadata == null) { + LOGGER.warn("Script {} not found in cache", id); + return null; + } + return metadata.getCompiledClass(); + } + + /** + * {@inheritDoc} + */ @Override - public GroovyCodeSource getGroovyCodeSource(String id) { - return groovyCodeSourceMap.get(id); + public ScriptMetadata getScriptMetadata(String actionName) { + if (actionName == null || actionName.trim().isEmpty()) { + throw new IllegalArgumentException("Action name cannot be null or empty"); + } + + return scriptMetadataCache.get(actionName); } /** - * Build a GroovyCodeSource object and add it to the class loader of the groovyScriptEngine - * - * @param groovyScript groovy script as a string - * @param actionName Name of the action - * @return Built GroovyCodeSource + * Creates GroovyCodeSource for compilation. */ private GroovyCodeSource buildClassScript(String groovyScript, String actionName) { return new GroovyCodeSource(groovyScript, actionName, "/groovy/script"); } + /** + * Persists script to storage. + */ private void saveScript(String actionName, String script) { GroovyAction groovyScript = new GroovyAction(actionName, script); persistenceService.save(groovyScript); LOGGER.info("The script {} has been persisted.", actionName); } + /** + * Refreshes scripts from persistence with selective recompilation. + * Uses hash-based change detection and atomic cache updates. + */ private void refreshGroovyActions() { - Map<String, GroovyCodeSource> refreshedGroovyCodeSourceMap = new HashMap<>(); - persistenceService.getAllItems(GroovyAction.class).forEach(groovyAction -> refreshedGroovyCodeSourceMap - .put(groovyAction.getName(), buildClassScript(groovyAction.getScript(), groovyAction.getName()))); - groovyCodeSourceMap = refreshedGroovyCodeSourceMap; + long startTime = System.currentTimeMillis(); + + Map<String, ScriptMetadata> newMetadataCache = new ConcurrentHashMap<>(); + int unchangedCount = 0; + int recompiledCount = 0; + int errorCount = 0; + long totalCompilationTime = 0; + + for (GroovyAction groovyAction : persistenceService.getAllItems(GroovyAction.class)) { + String actionName = groovyAction.getName(); + String scriptContent = groovyAction.getScript(); + + try { + ScriptMetadata existingMetadata = scriptMetadataCache.get(actionName); + if (existingMetadata != null && !existingMetadata.hasChanged(scriptContent)) { + newMetadataCache.put(actionName, existingMetadata); + unchangedCount++; + LOGGER.debug("Script {} unchanged during refresh, keeping cached version", actionName); + } else { + if (recompiledCount == 0) { + LOGGER.info("Refreshing scripts from persistence layer..."); + } + + long compilationStartTime = System.currentTimeMillis(); + Class<? extends Script> scriptClass = compileScript(actionName, scriptContent); + long compilationTime = System.currentTimeMillis() - compilationStartTime; + totalCompilationTime += compilationTime; + + ScriptMetadata metadata = new ScriptMetadata(actionName, scriptContent, scriptClass); + newMetadataCache.put(actionName, metadata); + recompiledCount++; + LOGGER.info("Script {} recompiled during refresh ({}ms)", actionName, compilationTime); + } + + } catch (Exception e) { + if (errorCount == 0 && recompiledCount == 0) { + LOGGER.info("Refreshing scripts from persistence layer..."); + } + + errorCount++; + LOGGER.error("Failed to refresh script: {}", actionName, e); + + ScriptMetadata existingMetadata = scriptMetadataCache.get(actionName); + if (existingMetadata != null) { + newMetadataCache.put(actionName, existingMetadata); + LOGGER.warn("Keeping existing version of script {} due to compilation error", actionName); + } + } + } + + this.scriptMetadataCache = newMetadataCache; + + if (recompiledCount > 0 || errorCount > 0) { + long totalTime = System.currentTimeMillis() - startTime; + LOGGER.info("Script refresh completed: {} unchanged, {} recompiled, {} errors. Total time: {}ms", + unchangedCount, recompiledCount, errorCount, totalTime); + LOGGER.debug("Refresh metrics: Recompilation time: {}ms, Cache update overhead: {}ms", + totalCompilationTime, totalTime - totalCompilationTime); + } else { + LOGGER.debug("Script refresh completed: {} scripts checked, no changes detected ({}ms)", + unchangedCount, System.currentTimeMillis() - startTime); + } } + /** + * Initializes periodic script refresh timer. + */ private void initializeTimers() { TimerTask task = new TimerTask() { @Override
