From 146a6d3730f387106ad08066761d398eb4911335 Mon Sep 17 00:00:00 2001
From: Shay Banon <kimchy@gmail.com>
Date: Sat, 30 Jul 2011 15:13:02 +0300
Subject: [PATCH] Minor(?) scripting bug(?): (caching-related?) odd behavior
 when changing languages for the same script code, closes #1150.

---
 .../elasticsearch/script/ScriptService.java   | 47 ++++++++++++-------
 1 file changed, 31 insertions(+), 16 deletions(-)

diff --git a/modules/elasticsearch/src/main/java/org/elasticsearch/script/ScriptService.java b/modules/elasticsearch/src/main/java/org/elasticsearch/script/ScriptService.java
index 5400dc1e6cb..15a994561c7 100644
--- a/modules/elasticsearch/src/main/java/org/elasticsearch/script/ScriptService.java
+++ b/modules/elasticsearch/src/main/java/org/elasticsearch/script/ScriptService.java
@@ -54,7 +54,7 @@ public class ScriptService extends AbstractComponent {
 
     private final ConcurrentMap<String, CompiledScript> staticCache = ConcurrentCollections.newConcurrentMap();
 
-    private final ConcurrentMap<String, CompiledScript> cache = new MapMaker().softValues().makeMap();
+    private final ConcurrentMap<CacheKey, CompiledScript> cache = new MapMaker().softValues().makeMap();
 
     public ScriptService(Settings settings) {
         this(settings, new Environment(), ImmutableSet.<ScriptEngineService>builder()
@@ -136,25 +136,21 @@ public class ScriptService extends AbstractComponent {
         if (compiled != null) {
             return compiled;
         }
-        compiled = cache.get(script);
-        if (compiled != null) {
-            return compiled;
-        }
         if (lang == null) {
             lang = defaultLang;
         }
-        synchronized (cache) {
-            compiled = cache.get(script);
-            if (compiled != null) {
-                return compiled;
-            }
-            ScriptEngineService service = scriptEngines.get(lang);
-            if (service == null) {
-                throw new ElasticSearchIllegalArgumentException("script_lang not supported [" + lang + "]");
-            }
-            compiled = new CompiledScript(lang, service.compile(script));
-            cache.put(script, compiled);
+        CacheKey cacheKey = new CacheKey(lang, script);
+        compiled = cache.get(cacheKey);
+        if (compiled != null) {
+            return compiled;
         }
+        // not the end of the world if we compile it twice...
+        ScriptEngineService service = scriptEngines.get(lang);
+        if (service == null) {
+            throw new ElasticSearchIllegalArgumentException("script_lang not supported [" + lang + "]");
+        }
+        compiled = new CompiledScript(lang, service.compile(script));
+        cache.put(cacheKey, compiled);
         return compiled;
     }
 
@@ -186,6 +182,25 @@ public class ScriptService extends AbstractComponent {
         cache.clear();
     }
 
+    public static class CacheKey {
+        public final String lang;
+        public final String script;
+
+        public CacheKey(String lang, String script) {
+            this.lang = lang;
+            this.script = script;
+        }
+
+        @Override public boolean equals(Object o) {
+            CacheKey other = (CacheKey) o;
+            return lang.equals(other.lang) && script.equals(other.script);
+        }
+
+        @Override public int hashCode() {
+            return lang.hashCode() + 31 * script.hashCode();
+        }
+    }
+
     public static class DocScoreNativeScriptFactory implements NativeScriptFactory {
         @Override public ExecutableScript newScript(@Nullable Map<String, Object> params) {
             return new DocScoreSearchScript();