refactor(script): add compiled script caching to GroovyEngine and improve DocBoostMatcher error handling

src/main/java/org/codelibs/fess/indexer/DocBoostMatcher.java

@@ -17,6 +17,8 @@
 
 import java.util.Map;
 
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.codelibs.fess.Constants;
 import org.codelibs.fess.opensearch.config.exentity.BoostDocumentRule;
 import org.codelibs.fess.util.ComponentUtil;
@@ -29,6 +31,7 @@
  *
  */
 public class DocBoostMatcher {
+    private static final Logger logger = LogManager.getLogger(DocBoostMatcher.class);
 
     /** The expression used to calculate the boost value (defaults to "0") */
     private String boostExpression = "0";
@@ -105,7 +108,12 @@ public float getValue(final Map<String, Object> map) {
             return ((Double) value).floatValue();
         }
         if (value != null) {
-            return Float.parseFloat(value.toString());
+            try {
+                return Float.parseFloat(value.toString());
+            } catch (final NumberFormatException e) {
+                logger.warn("Failed to parse boost value: expression={}, value={}", boostExpression, value, e);
+                return 0.0f;
+            }
         }
 
         return 0.0f;

src/main/java/org/codelibs/fess/script/groovy/GroovyEngine.java

@@ -15,9 +15,11 @@
  */
 package org.codelibs.fess.script.groovy;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
@@ -30,37 +32,100 @@
 import org.lastaflute.di.core.factory.SingletonLaContainerFactory;
 import org.lastaflute.job.LaJobRuntime;
 
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.RemovalNotification;
+
 import groovy.lang.Binding;
 import groovy.lang.GroovyClassLoader;
-import groovy.lang.GroovyShell;
+import groovy.lang.Script;
+import jakarta.annotation.PostConstruct;
+import jakarta.annotation.PreDestroy;
 
 /**
  * Groovy script engine implementation that extends AbstractScriptEngine.
  * This class provides support for executing Groovy scripts with parameter binding
  * and DI container integration.
  *
- * <p>Thread Safety: This class is thread-safe. Each evaluate() call creates
- * a new GroovyShell instance to ensure thread isolation.</p>
+ * <p>Thread Safety: This class is thread-safe. Each cached entry holds its own
+ * GroovyClassLoader. The cache uses Guava Cache with segment-based locking for
+ * lock-free concurrent reads. Each evaluate() call creates a new Script instance
+ * to ensure thread isolation of bindings.</p>
+ *
+ * <p>Note on class-level isolation: Compiled Script classes are cached and reused.
+ * Class-level state (static fields, metaclass mutations) persists across evaluations
+ * of the same script. In Fess, scripts are short expressions configured by
+ * administrators (e.g., "data1 &gt; 10", "10 * boost1 + boost2") and do not use
+ * static state, so this is acceptable.</p>
  *
- * <p>Resource Management: GroovyClassLoader instances are properly managed
- * and cleaned up after script evaluation to prevent memory leaks.</p>
+ * <p>Resource Management: Each cached entry's GroovyClassLoader is closed on
+ * eviction via RemovalListener. All remaining entries are cleaned up via close() (@PreDestroy).</p>
  */
 public class GroovyEngine extends AbstractScriptEngine {
     private static final Logger logger = LogManager.getLogger(GroovyEngine.class);
 
+    /** Maximum number of compiled scripts to cache. Configurable via DI. */
+    protected int scriptCacheSize = 1000;
+
+    /** Maximum length of script text included in warning log messages. Configurable via DI. */
+    protected int maxScriptLogLength = 200;
+
+    private Cache<String, CachedScript> scriptCache;
+
     /**
      * Default constructor for GroovyEngine.
      */
     public GroovyEngine() {
         super();
+        buildScriptCache();
+    }
+
+    /**
+     * Rebuilds the script cache after DI injection.
+     * Called by the DI container after property injection.
+     */
+    @PostConstruct
+    public void init() {
+        buildScriptCache();
+    }
+
+    private void buildScriptCache() {
+        final Cache<String, CachedScript> oldCache = scriptCache;
+        scriptCache = CacheBuilder.newBuilder()
+                .maximumSize(scriptCacheSize)
+                .removalListener((final RemovalNotification<String, CachedScript> notification) -> {
+                    notification.getValue().close();
+                })
+                .build();
+        if (oldCache != null) {
+            oldCache.invalidateAll();
+        }
+    }
+
+    /**
+     * Sets the maximum number of compiled scripts to cache.
+     *
+     * @param scriptCacheSize the cache size
+     */
+    public void setScriptCacheSize(final int scriptCacheSize) {
+        this.scriptCacheSize = scriptCacheSize;
+    }
+
+    /**
+     * Sets the maximum length of script text included in warning log messages.
+     *
+     * @param maxScriptLogLength the max length
+     */
+    public void setMaxScriptLogLength(final int maxScriptLogLength) {
+        this.maxScriptLogLength = maxScriptLogLength;
     }
 
     /**
      * Evaluates a Groovy script template with the provided parameters.
      *
-     * <p>This method creates a new GroovyShell instance for each evaluation to ensure
-     * thread safety. The DI container is automatically injected into the binding map
-     * as "container" for script access.</p>
+     * <p>This method caches compiled Script classes per script text.
+     * Each evaluation creates a new Script instance to ensure thread-safe binding isolation.
+     * The DI container is automatically injected into the binding map as "container".</p>
      *
      * @param template the Groovy script to evaluate (null-safe, returns null if empty)
      * @param paramMap the parameters to bind to the script (null-safe, treated as empty map if null)
@@ -70,66 +135,83 @@ public GroovyEngine() {
      */
     @Override
     public Object evaluate(final String template, final Map<String, Object> paramMap) {
-        // Null-safety: return null for blank templates
-        // Early return is safe here as no resources have been allocated yet
         if (StringUtil.isBlank(template)) {
             if (logger.isDebugEnabled()) {
                 logger.debug("Template is blank, returning null");
             }
             return null;
         }
 
-        // Null-safety: use empty map if paramMap is null
         final Map<String, Object> safeParamMap = paramMap != null ? paramMap : Collections.emptyMap();
 
-        // Prepare binding map with parameters and DI container
         final Map<String, Object> bindingMap = new HashMap<>(safeParamMap);
         bindingMap.put("container", SingletonLaContainerFactory.getContainer());
 
-        // Create GroovyShell with custom class loader for proper resource management
-        GroovyClassLoader classLoader = null;
         try {
-            // Get parent class loader with fallback to ensure robustness across threading contexts
-            ClassLoader parentClassLoader = Thread.currentThread().getContextClassLoader();
-            if (parentClassLoader == null) {
-                parentClassLoader = GroovyEngine.class.getClassLoader();
-            }
-            classLoader = new GroovyClassLoader(parentClassLoader);
-            final GroovyShell groovyShell = new GroovyShell(classLoader, new Binding(bindingMap));
+            final CachedScript cached = getOrCompile(template);
+            final Script script = cached.scriptClass.getDeclaredConstructor().newInstance();
+            script.setBinding(new Binding(bindingMap));
 
             if (logger.isDebugEnabled()) {
                 logger.debug("Evaluating Groovy script: template={}", template);
             }
 
-            final Object result = groovyShell.evaluate(template);
+            final Object result = script.run();
             logScriptExecution(template, "success");
             return result;
         } catch (final JobProcessingException e) {
-            // Rethrow JobProcessingException to allow scripts to signal job-specific errors
-            // that should be handled by the job framework
             if (logger.isDebugEnabled()) {
                 logger.debug("Script raised JobProcessingException", e);
             }
             logScriptExecution(template, "failure:" + e.getClass().getSimpleName());
             throw e;
         } catch (final Exception e) {
-            // Log and return null for other exceptions to maintain backward compatibility
-            logger.warn("Failed to evaluate Groovy script: template={}, parameters={}", template, safeParamMap, e);
+            final String truncatedScript =
+                    template.length() > maxScriptLogLength ? template.substring(0, maxScriptLogLength) + "..." : template;
+            logger.warn("Failed to evaluate Groovy script: script(length={})={}, parameterKeys={}", template.length(), truncatedScript,
+                    safeParamMap.keySet(), e);
             logScriptExecution(template, "failure:" + e.getClass().getSimpleName());
             return null;
-        } finally {
-            // Properly clean up GroovyClassLoader resources
-            if (classLoader != null) {
+        }
+    }
+
+    @SuppressWarnings("unchecked")
+    private CachedScript getOrCompile(final String template) {
+        try {
+            return scriptCache.get(template, () -> {
+                ClassLoader parentClassLoader = Thread.currentThread().getContextClassLoader();
+                if (parentClassLoader == null) {
+                    parentClassLoader = GroovyEngine.class.getClassLoader();
+                }
+                final GroovyClassLoader classLoader = new GroovyClassLoader(parentClassLoader);
                 try {
-                    classLoader.clearCache();
-                    classLoader.close();
+                    final Class<? extends Script> scriptClass = (Class<? extends Script>) classLoader.parseClass(template);
+                    return new CachedScript(scriptClass, classLoader);
                 } catch (final Exception e) {
-                    logger.warn("Failed to close GroovyClassLoader", e);
+                    try {
+                        classLoader.clearCache();
+                        classLoader.close();
+                    } catch (final IOException closeEx) {
+                        logger.warn("Failed to close GroovyClassLoader after compilation failure", closeEx);
+                    }
+                    throw e;
                 }
-            }
+            });
+        } catch (final ExecutionException e) {
+            throw (RuntimeException) e.getCause();
         }
     }
 
+    /**
+     * Closes all cached GroovyClassLoaders and clears the script cache.
+     * Called by the DI container on shutdown.
+     */
+    @PreDestroy
+    public void close() {
+        scriptCache.invalidateAll();
+        scriptCache.cleanUp();
+    }
+
     /**
      * Returns the name identifier for this script engine.
      *
@@ -195,4 +277,27 @@ protected void logScriptExecution(final String script, final String result) {
         }
     }
 
+    /**
+     * Holds a compiled Script class and its associated GroovyClassLoader.
+     * When evicted from the cache, close() releases the class loader resources.
+     */
+    private static class CachedScript {
+        final Class<? extends Script> scriptClass;
+        private final GroovyClassLoader classLoader;
+
+        CachedScript(final Class<? extends Script> scriptClass, final GroovyClassLoader classLoader) {
+            this.scriptClass = scriptClass;
+            this.classLoader = classLoader;
+        }
+
+        void close() {
+            try {
+                classLoader.clearCache();
+                classLoader.close();
+            } catch (final IOException e) {
+                LogManager.getLogger(GroovyEngine.class).warn("Failed to close GroovyClassLoader", e);
+            }
+        }
+    }
+
 }

src/main/resources/fess_se++.xml

@@ -4,6 +4,11 @@
 <components>
 	<component name="groovyEngine"
 		class="org.codelibs.fess.script.groovy.GroovyEngine">
+		<!--
+		<property name="scriptCacheSize">1000</property>
+		<property name="maxScriptLogLength">200</property>
+		-->
+		<postConstruct name="init"></postConstruct>
 		<postConstruct name="register"></postConstruct>
 	</component>
 </components>