You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by sh...@apache.org on 2025/07/13 06:03:17 UTC
(unomi) 01/02: UNOMI-897: Fix data corruption and performance issues in GroovyActionDispatcher
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 <sh...@jahia.com>
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