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..093a91d6f 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 void setGroovyActionsService(GroovyActionsService groovyActionsService) { 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(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 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.getDeclaredConstructor().newInstance(); + setScriptVariables(script, action, event); + + return new MetricAdapter(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/ScriptMetadata.java b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/ScriptMetadata.java new file mode 100644 index 000000000..57b44e3ae --- /dev/null +++ b/extensions/groovy-actions/services/src/main/java/org/apache/unomi/groovy/actions/ScriptMetadata.java @@ -0,0 +1,156 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.unomi.groovy.actions; + +import groovy.lang.Script; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.nio.charset.StandardCharsets; +import java.util.Base64; + +/** + * Metadata container for compiled Groovy scripts with hash-based change detection. + *

+ * This class encapsulates all metadata associated with a compiled Groovy script, + * including content hash for efficient change detection and the compiled class + * for direct execution without recompilation. + *

+ * + *

+ * Thread Safety: This class is immutable and thread-safe. All fields are final + * and the class provides no methods to modify its state after construction. + *

+ * + * @since 2.7.0 + */ +public final class ScriptMetadata { + private final String actionName; + private final String scriptContent; + private final String contentHash; + private final long creationTime; + private final Class compiledClass; + + /** + * Constructs a new ScriptMetadata instance. + * + * @param actionName the unique name/identifier of the action + * @param scriptContent the raw Groovy script content + * @param compiledClass the compiled Groovy script class + * @throws IllegalArgumentException if any parameter is null + */ + public ScriptMetadata(String actionName, String scriptContent, Class compiledClass) { + if (actionName == null) { + throw new IllegalArgumentException("Action name cannot be null"); + } + if (scriptContent == null) { + throw new IllegalArgumentException("Script content cannot be null"); + } + if (compiledClass == null) { + throw new IllegalArgumentException("Compiled class cannot be null"); + } + + this.actionName = actionName; + this.scriptContent = scriptContent; + this.contentHash = calculateHash(scriptContent); + this.creationTime = System.currentTimeMillis(); + this.compiledClass = compiledClass; + } + + /** + * Calculates SHA-256 hash of the given content. + * + * @param content the content to hash + * @return Base64 encoded SHA-256 hash + * @throws RuntimeException if SHA-256 algorithm is not available + */ + private String calculateHash(String content) { + try { + MessageDigest digest = MessageDigest.getInstance("SHA-256"); + byte[] hash = digest.digest(content.getBytes(StandardCharsets.UTF_8)); + return Base64.getEncoder().encodeToString(hash); + } catch (NoSuchAlgorithmException e) { + throw new RuntimeException("SHA-256 algorithm not available", e); + } + } + + /** + * Determines if the script content has changed compared to new content. + *

+ * This method uses SHA-256 hash comparison for efficient change detection + * without storing or comparing the full script content. + *

+ * + * @param newContent the new script content to compare against + * @return {@code true} if content has changed, {@code false} if unchanged + * @throws IllegalArgumentException if newContent is null + */ + public boolean hasChanged(String newContent) { + if (newContent == null) { + throw new IllegalArgumentException("New content cannot be null"); + } + return !contentHash.equals(calculateHash(newContent)); + } + + /** + * Returns the action name/identifier. + * + * @return the action name, never null + */ + public String getActionName() { + return actionName; + } + + /** + * Returns the original script content. + * + * @return the script content, never null + */ + public String getScriptContent() { + return scriptContent; + } + + /** + * Returns the SHA-256 hash of the script content. + * + * @return Base64 encoded content hash, never null + */ + public String getContentHash() { + return contentHash; + } + + /** + * Returns the timestamp when this metadata was created. + * + * @return creation timestamp in milliseconds since epoch + */ + public long getCreationTime() { + return creationTime; + } + + /** + * Returns the compiled Groovy script class. + *

+ * This class can be used to create new script instances for execution + * without requiring recompilation. + *

+ * + * @return the compiled script class, never null + */ + public Class getCompiledClass() { + return compiledClass; + } +} \ No newline at end of file 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..1b4bf14be 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,92 @@ */ 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. + *

+ * 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. + *

+ * + *

+ * Key features: + *

+ *

+ * + *

+ * Thread Safety: Implementations must be thread-safe as this service + * is accessed concurrently during script execution. + *

+ * + * @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. + *

+ * 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. + *

* - * @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. + *

+ * This method removes the action from both the cache and persistent storage, + * and cleans up any registered action types in the definitions service. + *

* - * @param id of the action to remove + * @param actionName the unique identifier of the action to remove + * @throws IllegalArgumentException if id is null */ - void remove(String id); + void remove(String actionName); /** - * Get a groovy code source object by an id + * Retrieves a pre-compiled script class from cache. + *

+ * 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. + *

* - * @param id of the action to get - * @return Groovy code source + * @param actionName 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 getCompiledScript(String actionName); /** - * Get an instantiated groovy shell object + * Retrieves script metadata for monitoring and change detection. + *

+ * The returned metadata includes content hash, compilation timestamp, + * and the compiled class reference. This is useful for monitoring + * tools and debugging. + *

* - * @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..3ad70b69b 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 @@ 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.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,13 @@ import java.io.IOException; import java.net.URL; -import java.util.HashMap; +import java.nio.charset.StandardCharsets; import java.util.HashSet; +import java.util.Set; + 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 +59,8 @@ 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,17 +73,21 @@ public class GroovyActionsServiceImpl implements GroovyActionsService { private BundleContext bundleContext; private GroovyScriptEngine groovyScriptEngine; - private GroovyShell groovyShell; - private Map groovyCodeSourceMap; + private CompilerConfiguration compilerConfiguration; private ScheduledFuture scheduledFuture; + private final Object compilationLock = new Object(); + private GroovyShell compilationShell; + private volatile Map scriptMetadataCache = new ConcurrentHashMap<>(); + private final Map> loggedRefreshErrors = new ConcurrentHashMap<>(); + private static final int MAX_LOGGED_ERRORS = 100; // Prevent memory leak + private static final Logger LOGGER = LoggerFactory.getLogger(GroovyActionsServiceImpl.class.getName()); private static final String BASE_SCRIPT_NAME = "BaseScript"; private DefinitionsService definitionsService; private PersistenceService persistenceService; private SchedulerService schedulerService; - private ActionExecutorDispatcher actionExecutorDispatcher; private GroovyActionsServiceConfig config; @Reference @@ -96,14 +105,7 @@ public void setSchedulerService(SchedulerService schedulerService) { 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 +113,25 @@ public void start(GroovyActionsServiceConfig config, BundleContext bundleContext 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 @@ -140,7 +147,7 @@ public void onDestroy() { * 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 + * The base script would be added in the configuration of the {@link GroovyActionsServiceImpl#compilationShell 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 * @@ -152,25 +159,77 @@ private void loadBaseScript() throws IOException { 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(); + ScriptMetadata metadata = compileAndCreateMetadata(actionName, scriptContent); + long scriptCompilationTime = System.currentTimeMillis() - scriptStartTime; + totalCompilationTime += scriptCompilationTime; + + 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 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 +237,88 @@ private ImportCustomizer createImportCustomizer() { return importCustomizer; } + /** + * Validates that a string parameter is not null or empty. + */ + private void validateNotEmpty(String value, String parameterName) { + if (value == null || value.trim().isEmpty()) { + throw new IllegalArgumentException(parameterName + " cannot be null or empty"); + } + } + + /** + * Compiles a script and creates metadata with timing information. + */ + private ScriptMetadata compileAndCreateMetadata(String actionName, String scriptContent) { + long compilationStartTime = System.currentTimeMillis(); + Class scriptClass = compileScript(actionName, scriptContent); + long compilationTime = System.currentTimeMillis() - compilationStartTime; + + LOGGER.debug("Script {} compiled in {}ms", actionName, compilationTime); + return new ScriptMetadata(actionName, scriptContent, scriptClass); + } + + /** + * Extracts Action annotation from script class if present. + */ + private Action getActionAnnotation(Class scriptClass) { + try { + return scriptClass.getMethod("execute").getAnnotation(Action.class); + } catch (Exception e) { + LOGGER.error("Failed to extract action annotation", e); + return null; + } + } + + /** + * {@inheritDoc} + * Implementation performs hash-based change detection to skip unnecessary recompilation. + */ @Override public void save(String actionName, String groovyScript) { - GroovyCodeSource groovyCodeSource = buildClassScript(groovyScript, actionName); + validateNotEmpty(actionName, "Action name"); + validateNotEmpty(groovyScript, "Groovy script"); + + 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(); + ScriptMetadata metadata = compileAndCreateMetadata(actionName, groovyScript); + long compilationTime = System.currentTimeMillis() - compilationStartTime; + + Action actionAnnotation = getActionAnnotation(metadata.getCompiledClass()); + 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); + + 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 +331,170 @@ private void saveActionType(Action action) { 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); + public void remove(String actionName) { + validateNotEmpty(actionName, "Action name"); + + LOGGER.info("Removing script: {}", actionName); + + ScriptMetadata removedMetadata = scriptMetadataCache.remove(actionName); + persistenceService.remove(actionName, GroovyAction.class); + + // Clean up error tracking to prevent memory leak + loggedRefreshErrors.remove(actionName); + + if (removedMetadata != null) { + Action actionAnnotation = getActionAnnotation(removedMetadata.getCompiledClass()); + if (actionAnnotation != null) { + definitionsService.removeActionType(actionAnnotation.id()); } - persistenceService.remove(id, GroovyAction.class); } + + LOGGER.info("Script {} removed successfully", actionName); } + /** + * {@inheritDoc} + */ @Override - public GroovyCodeSource getGroovyCodeSource(String id) { - return groovyCodeSourceMap.get(id); + public Class getCompiledScript(String id) { + validateNotEmpty(id, "Script ID"); + + ScriptMetadata metadata = scriptMetadataCache.get(id); + if (metadata == null) { + LOGGER.warn("Script {} not found in cache", id); + return null; + } + return metadata.getCompiledClass(); } /** - * 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 + * {@inheritDoc} + */ + @Override + public ScriptMetadata getScriptMetadata(String actionName) { + validateNotEmpty(actionName, "Action name"); + + return scriptMetadataCache.get(actionName); + } + + /** + * 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 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 newMetadataCache = new ConcurrentHashMap<>(); + int unchangedCount = 0; + int recompiledCount = 0; + int errorCount = 0; + int newErrorCount = 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(); + ScriptMetadata metadata = compileAndCreateMetadata(actionName, scriptContent); + long compilationTime = System.currentTimeMillis() - compilationStartTime; + totalCompilationTime += compilationTime; + + // Clear error tracking on successful compilation + loggedRefreshErrors.remove(actionName); + + newMetadataCache.put(actionName, metadata); + recompiledCount++; + LOGGER.info("Script {} recompiled during refresh ({}ms)", actionName, compilationTime); + } + + } catch (Exception e) { + if (newErrorCount == 0 && recompiledCount == 0) { + LOGGER.info("Refreshing scripts from persistence layer..."); + } + + errorCount++; + + // Prevent log spam for repeated compilation errors during refresh + String errorMessage = e.getMessage(); + Set scriptErrors = loggedRefreshErrors.get(actionName); + + if (scriptErrors == null || !scriptErrors.contains(errorMessage)) { + newErrorCount++; + LOGGER.error("Failed to refresh script: {}", actionName, e); + + // Prevent memory leak by limiting tracked errors before adding new entries + if (scriptErrors == null && loggedRefreshErrors.size() >= MAX_LOGGED_ERRORS) { + // Remove one random entry to make space (simple eviction) + String firstKey = loggedRefreshErrors.keySet().iterator().next(); + loggedRefreshErrors.remove(firstKey); + } + + // Now safely add the error + if (scriptErrors == null) { + scriptErrors = ConcurrentHashMap.newKeySet(); + loggedRefreshErrors.put(actionName, scriptErrors); + } + scriptErrors.add(errorMessage); + + LOGGER.warn("Keeping existing version of script {} due to compilation error", actionName); + } + + ScriptMetadata existingMetadata = scriptMetadataCache.get(actionName); + if (existingMetadata != null) { + newMetadataCache.put(actionName, existingMetadata); + } + } + } + + this.scriptMetadataCache = newMetadataCache; + + if (recompiledCount > 0 || newErrorCount > 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 diff --git a/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java index 85d7d3385..5af563e3d 100644 --- a/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java +++ b/itests/src/test/java/org/apache/unomi/itests/GroovyActionsServiceIT.java @@ -17,7 +17,7 @@ package org.apache.unomi.itests; -import groovy.lang.GroovyCodeSource; +import groovy.lang.Script; import org.apache.commons.io.IOUtils; import org.apache.unomi.api.Event; import org.apache.unomi.api.Profile; @@ -95,7 +95,7 @@ public void testGroovyActionsService_triggerGroovyAction() throws IOException, I groovyActionsService.save(UPDATE_ADDRESS_ACTION, loadGroovyAction(UPDATE_ADDRESS_ACTION_GROOVY_FILE)); keepTrying("Failed waiting for the creation of the GroovyAction for the trigger action test", - () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + () -> groovyActionsService.getCompiledScript(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); ActionType actionType = keepTrying("Failed waiting for the creation of the GroovyAction for trigger action test", () -> definitionsService.getActionType(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); @@ -114,10 +114,10 @@ public void testGroovyActionsService_saveActionAndTestSavedValues() throws IOExc ActionType actionType = keepTrying("Failed waiting for the creation of the GroovyAction for the save test", () -> definitionsService.getActionType(UPDATE_ADDRESS_GROOVY_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); - GroovyCodeSource groovyCodeSource = keepTrying("Failed waiting for the creation of the GroovyAction for the save test", - () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + Class compiledScript = keepTrying("Failed waiting for the creation of the GroovyAction for the save test", + () -> groovyActionsService.getCompiledScript(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); - Assert.assertEquals(UPDATE_ADDRESS_ACTION, groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION).getName()); + Assert.assertEquals(UPDATE_ADDRESS_ACTION, compiledScript.getSimpleName()); Assert.assertTrue(actionType.getMetadata().getId().contains(UPDATE_ADDRESS_GROOVY_ACTION)); Assert.assertEquals(2, actionType.getMetadata().getSystemTags().size()); @@ -133,14 +133,14 @@ public void testGroovyActionsService_saveActionAndTestSavedValues() throws IOExc public void testGroovyActionsService_removeGroovyAction() throws IOException, InterruptedException { groovyActionsService.save(UPDATE_ADDRESS_ACTION, loadGroovyAction(UPDATE_ADDRESS_ACTION_GROOVY_FILE)); - GroovyCodeSource groovyCodeSource = keepTrying("Failed waiting for the creation of the GroovyAction for the remove test", - () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); + Class compiledScript = keepTrying("Failed waiting for the creation of the GroovyAction for the remove test", + () -> groovyActionsService.getCompiledScript(UPDATE_ADDRESS_ACTION), Objects::nonNull, DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); - Assert.assertNotNull(groovyCodeSource); + Assert.assertNotNull(compiledScript); groovyActionsService.remove(UPDATE_ADDRESS_ACTION); - waitForNullValue("Groovy action is still present", () -> groovyActionsService.getGroovyCodeSource(UPDATE_ADDRESS_ACTION), + waitForNullValue("Groovy action is still present", () -> groovyActionsService.getCompiledScript(UPDATE_ADDRESS_ACTION), DEFAULT_TRYING_TIMEOUT, DEFAULT_TRYING_TRIES); waitForNullValue("Action type is still present", () -> definitionsService.getActionType(UPDATE_ADDRESS_GROOVY_ACTION),