From a9d540a859c76a10d514255bf4569e18c0227b3a Mon Sep 17 00:00:00 2001 From: Isabel Drost-Fromm Date: Fri, 10 Apr 2015 09:33:30 +0200 Subject: [PATCH] Fix updating templates. Closes #10397 When putting new templates to an index they are added to the cache of compiled templates as a side effect of the validate method. When updating templates they are also validated but the scripts that are already in the cache never get updated. As per comments on PR #10526 adding more tests around updating scripts and templates. --- .../elasticsearch/script/ScriptService.java | 17 ++++--- .../index/query/TemplateQueryTest.java | 44 +++++++++++++++++++ .../script/IndexedScriptTests.java | 25 +++++++++++ 3 files changed, 80 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/elasticsearch/script/ScriptService.java b/src/main/java/org/elasticsearch/script/ScriptService.java index d2c848c4009..3320dea795d 100644 --- a/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/src/main/java/org/elasticsearch/script/ScriptService.java @@ -25,6 +25,7 @@ import com.google.common.cache.CacheBuilder; import com.google.common.cache.RemovalListener; import com.google.common.cache.RemovalNotification; import com.google.common.collect.ImmutableMap; + import org.apache.lucene.util.IOUtils; import org.elasticsearch.ElasticsearchIllegalArgumentException; import org.elasticsearch.ElasticsearchIllegalStateException; @@ -34,6 +35,7 @@ import org.elasticsearch.action.delete.DeleteResponse; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.index.IndexResponse; import org.elasticsearch.action.indexedscripts.delete.DeleteIndexedScriptRequest; import org.elasticsearch.action.indexedscripts.get.GetIndexedScriptRequest; @@ -72,6 +74,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Locale; import java.util.Map; +import java.util.Map.Entry; import java.util.Set; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.TimeUnit; @@ -236,30 +239,32 @@ public class ScriptService extends AbstractComponent implements Closeable { /** * Compiles a script straight-away, or returns the previously compiled and cached script, without checking if it can be executed based on settings. */ - public CompiledScript compileInternal(String lang, String script, ScriptType scriptType) { - assert script != null; + public CompiledScript compileInternal(String lang, final String scriptOrId, final ScriptType scriptType) { + assert scriptOrId != null; assert scriptType != null; if (lang == null) { lang = defaultLang; } if (logger.isTraceEnabled()) { - logger.trace("Compiling lang: [{}] type: [{}] script: {}", lang, scriptType, script); + logger.trace("Compiling lang: [{}] type: [{}] script: {}", lang, scriptType, scriptOrId); } ScriptEngineService scriptEngineService = getScriptEngineServiceForLang(lang); - CacheKey cacheKey = newCacheKey(scriptEngineService, script); + CacheKey cacheKey = newCacheKey(scriptEngineService, scriptOrId); if (scriptType == ScriptType.FILE) { CompiledScript compiled = staticCache.get(cacheKey); //On disk scripts will be loaded into the staticCache by the listener if (compiled == null) { - throw new ElasticsearchIllegalArgumentException("Unable to find on disk script " + script); + throw new ElasticsearchIllegalArgumentException("Unable to find on disk script " + scriptOrId); } return compiled; } + String script = scriptOrId; if (scriptType == ScriptType.INDEXED) { - final IndexedScript indexedScript = new IndexedScript(lang, script); + final IndexedScript indexedScript = new IndexedScript(lang, scriptOrId); script = getScriptFromIndex(indexedScript.lang, indexedScript.id); + cacheKey = newCacheKey(scriptEngineService, script); } CompiledScript compiled = cache.getIfPresent(cacheKey); diff --git a/src/test/java/org/elasticsearch/index/query/TemplateQueryTest.java b/src/test/java/org/elasticsearch/index/query/TemplateQueryTest.java index dbc3f300b32..99af5670960 100644 --- a/src/test/java/org/elasticsearch/index/query/TemplateQueryTest.java +++ b/src/test/java/org/elasticsearch/index/query/TemplateQueryTest.java @@ -20,9 +20,11 @@ package org.elasticsearch.index.query; import com.google.common.collect.Maps; +import org.elasticsearch.action.index.IndexRequest.OpType; import org.elasticsearch.action.index.IndexRequestBuilder; import org.elasticsearch.action.indexedscripts.delete.DeleteIndexedScriptResponse; import org.elasticsearch.action.indexedscripts.get.GetIndexedScriptResponse; +import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptRequestBuilder; import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptResponse; import org.elasticsearch.action.search.SearchPhaseExecutionException; import org.elasticsearch.action.search.SearchRequest; @@ -372,6 +374,48 @@ public class TemplateQueryTest extends ElasticsearchIntegrationTest { assertHitCount(sr, 4); } + // Relates to #10397 + @Test + public void testIndexedTemplateOverwrite() throws Exception { + createIndex("testindex"); + ensureGreen("testindex"); + + index("testindex", "test", "1", jsonBuilder().startObject().field("searchtext", "dev1").endObject()); + refresh(); + + int iterations = randomIntBetween(2, 11); + for (int i = 1; i < iterations; i++) { + PutIndexedScriptResponse scriptResponse = client().preparePutIndexedScript(MustacheScriptEngineService.NAME, "git01", + "{\"query\": {\"match\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\",\"type\": \"ooophrase_prefix\"}}}}").get(); + assertEquals(i * 2 - 1, scriptResponse.getVersion()); + + GetIndexedScriptResponse getResponse = client().prepareGetIndexedScript(MustacheScriptEngineService.NAME, "git01").get(); + assertTrue(getResponse.isExists()); + + Map templateParams = Maps.newHashMap(); + templateParams.put("P_Keyword1", "dev"); + + try { + client().prepareSearch("testindex").setTypes("test"). + setTemplateName("git01").setTemplateType(ScriptService.ScriptType.INDEXED).setTemplateParams(templateParams).get(); + fail("Broken test template is parsing w/o error."); + } catch (SearchPhaseExecutionException e) { + // the above is expected to fail + } + + PutIndexedScriptRequestBuilder builder = client() + .preparePutIndexedScript(MustacheScriptEngineService.NAME, "git01", + "{\"query\": {\"match\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\",\"type\": \"phrase_prefix\"}}}}") + .setOpType(OpType.INDEX); + scriptResponse = builder.get(); + assertEquals(i * 2, scriptResponse.getVersion()); + SearchResponse searchResponse = client().prepareSearch("testindex").setTypes("test"). + setTemplateName("git01").setTemplateType(ScriptService.ScriptType.INDEXED).setTemplateParams(templateParams).get(); + assertHitCount(searchResponse, 1); + } + } + + @Test public void testIndexedTemplateWithArray() throws Exception { createIndex(ScriptService.SCRIPT_INDEX); diff --git a/src/test/java/org/elasticsearch/script/IndexedScriptTests.java b/src/test/java/org/elasticsearch/script/IndexedScriptTests.java index a8a91d14013..ac44e4d6dbc 100644 --- a/src/test/java/org/elasticsearch/script/IndexedScriptTests.java +++ b/src/test/java/org/elasticsearch/script/IndexedScriptTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.script; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.index.IndexRequestBuilder; +import org.elasticsearch.action.indexedscripts.put.PutIndexedScriptResponse; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.common.settings.ImmutableSettings; import org.elasticsearch.common.settings.Settings; @@ -87,6 +88,30 @@ public class IndexedScriptTests extends ElasticsearchIntegrationTest { assertThat((Integer)sh.field("test2").getValue(), equalTo(6)); } + // Relates to #10397 + @Test + public void testUpdateScripts() { + createIndex("test_index"); + ensureGreen("test_index"); + client().prepareIndex("test_index", "test_type", "1").setSource("{\"foo\":\"bar\"}").get(); + flush("test_index"); + + int iterations = randomIntBetween(2, 11); + for (int i = 1; i < iterations; i++) { + PutIndexedScriptResponse response = + client().preparePutIndexedScript(GroovyScriptEngineService.NAME, "script1", "{\"script\":\"" + i + "\"}").get(); + assertEquals(i, response.getVersion()); + + String query = "{" + + " \"query\" : { \"match_all\": {}}, " + + " \"script_fields\" : { \"test_field\" : { \"script_id\" : \"script1\", \"lang\":\"groovy\" } } }"; + SearchResponse searchResponse = client().prepareSearch().setSource(query).setIndices("test_index").setTypes("test_type").get(); + assertHitCount(searchResponse, 1); + SearchHit sh = searchResponse.getHits().getAt(0); + assertThat((Integer)sh.field("test_field").getValue(), equalTo(i)); + } + } + @Test public void testDisabledUpdateIndexedScriptsOnly() { if (randomBoolean()) {