From 37f16127c56ae0ce007052cfe05b54895b92e33b Mon Sep 17 00:00:00 2001 From: Igor Motov Date: Wed, 13 Feb 2013 21:05:41 -0500 Subject: [PATCH] Fix ScriptFilter cache key calculation Fixes #2651 --- .../index/query/ScriptFilterParser.java | 19 ++--- .../scriptfilter/ScriptFilterSearchTests.java | 71 ++++++++++++++++++- 2 files changed, 76 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/elasticsearch/index/query/ScriptFilterParser.java b/src/main/java/org/elasticsearch/index/query/ScriptFilterParser.java index 72f84d4f308..791c8f23666 100644 --- a/src/main/java/org/elasticsearch/index/query/ScriptFilterParser.java +++ b/src/main/java/org/elasticsearch/index/query/ScriptFilterParser.java @@ -19,14 +19,12 @@ package org.elasticsearch.index.query; -import com.google.common.collect.Maps; import org.apache.lucene.index.AtomicReaderContext; import org.apache.lucene.search.BitsFilteredDocIdSet; import org.apache.lucene.search.DocIdSet; import org.apache.lucene.search.Filter; import org.apache.lucene.util.Bits; import org.elasticsearch.ElasticSearchIllegalArgumentException; -import org.elasticsearch.ElasticSearchIllegalStateException; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.lucene.docset.MatchDocIdSet; @@ -34,11 +32,13 @@ import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.index.cache.filter.support.CacheKeyFilter; import org.elasticsearch.script.ScriptService; import org.elasticsearch.script.SearchScript; -import org.elasticsearch.search.internal.SearchContext; +import org.elasticsearch.search.lookup.SearchLookup; import java.io.IOException; import java.util.Map; +import static com.google.common.collect.Maps.newHashMap; + /** * */ @@ -100,10 +100,10 @@ public class ScriptFilterParser implements FilterParser { throw new QueryParsingException(parseContext.index(), "script must be provided with a [script] filter"); } if (params == null) { - params = Maps.newHashMap(); + params = newHashMap(); } - Filter filter = new ScriptFilter(scriptLang, script, params, parseContext.scriptService()); + Filter filter = new ScriptFilter(scriptLang, script, params, parseContext.scriptService(), parseContext.lookup()); if (cache) { filter = parseContext.cacheFilter(filter, cacheKey); } @@ -121,16 +121,11 @@ public class ScriptFilterParser implements FilterParser { private final SearchScript searchScript; - private ScriptFilter(String scriptLang, String script, Map params, ScriptService scriptService) { + public ScriptFilter(String scriptLang, String script, Map params, ScriptService scriptService, SearchLookup searchLookup) { this.script = script; this.params = params; - SearchContext context = SearchContext.current(); - if (context == null) { - throw new ElasticSearchIllegalStateException("No search context on going..."); - } - - this.searchScript = context.scriptService().search(context.lookup(), scriptLang, script, params); + this.searchScript = scriptService.search(searchLookup, scriptLang, script, newHashMap(params)); } @Override diff --git a/src/test/java/org/elasticsearch/test/integration/search/scriptfilter/ScriptFilterSearchTests.java b/src/test/java/org/elasticsearch/test/integration/search/scriptfilter/ScriptFilterSearchTests.java index 6a5211d2353..a795f95d7a4 100644 --- a/src/test/java/org/elasticsearch/test/integration/search/scriptfilter/ScriptFilterSearchTests.java +++ b/src/test/java/org/elasticsearch/test/integration/search/scriptfilter/ScriptFilterSearchTests.java @@ -30,11 +30,12 @@ import org.testng.annotations.AfterMethod; import org.testng.annotations.BeforeMethod; import org.testng.annotations.Test; +import java.util.concurrent.atomic.AtomicInteger; + import static org.elasticsearch.client.Requests.refreshRequest; import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.index.query.FilterBuilders.scriptFilter; -import static org.elasticsearch.index.query.QueryBuilders.filteredQuery; -import static org.elasticsearch.index.query.QueryBuilders.matchAllQuery; +import static org.elasticsearch.index.query.QueryBuilders.*; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; @@ -68,6 +69,11 @@ public class ScriptFilterSearchTests extends AbstractNodesTests { @Test public void testCustomScriptBoost() throws Exception { + try { + client.admin().indices().prepareDelete("test").execute().actionGet(); + } catch (Exception ex) { + // + } client.admin().indices().prepareCreate("test").execute().actionGet(); client.prepareIndex("test", "type1", "1") .setSource(jsonBuilder().startObject().field("test", "value beck").field("num1", 1.0f).endObject()) @@ -121,4 +127,65 @@ public class ScriptFilterSearchTests extends AbstractNodesTests { assertThat(response.hits().getAt(2).id(), equalTo("3")); assertThat((Double) response.hits().getAt(2).fields().get("sNum1").values().get(0), equalTo(3.0)); } + + private static AtomicInteger scriptCounter = new AtomicInteger(0); + + public static int incrementScriptCounter() { + return scriptCounter.incrementAndGet(); + } + + @Test + public void testCustomScriptCache() throws Exception { + try { + client.admin().indices().prepareDelete("test").execute().actionGet(); + } catch (Exception ex) { + // + } + client.admin().indices().prepareCreate("test").execute().actionGet(); + client.prepareIndex("test", "type1", "1").setSource(jsonBuilder().startObject().field("test", "1").field("num", 1.0f).endObject()).execute().actionGet(); + client.admin().indices().prepareFlush().execute().actionGet(); + client.prepareIndex("test", "type1", "2").setSource(jsonBuilder().startObject().field("test", "2").field("num", 2.0f).endObject()).execute().actionGet(); + client.admin().indices().prepareFlush().execute().actionGet(); + client.prepareIndex("test", "type1", "3").setSource(jsonBuilder().startObject().field("test", "3").field("num", 3.0f).endObject()).execute().actionGet(); + client.admin().indices().prepareFlush().execute().actionGet(); + client.admin().indices().refresh(refreshRequest()).actionGet(); + + String script = "org.elasticsearch.test.integration.search.scriptfilter.ScriptFilterSearchTests.incrementScriptCounter() > 0"; + + scriptCounter.set(0); + logger.info("running script filter the first time"); + SearchResponse response = client.prepareSearch() + .setQuery(filteredQuery(termQuery("test", "1"), scriptFilter(script).cache(true))) + .execute().actionGet(); + + assertThat(response.hits().totalHits(), equalTo(1l)); + assertThat(scriptCounter.get(), equalTo(3)); + + scriptCounter.set(0); + logger.info("running script filter the second time"); + response = client.prepareSearch() + .setQuery(filteredQuery(termQuery("test", "2"), scriptFilter(script).cache(true))) + .execute().actionGet(); + + assertThat(response.hits().totalHits(), equalTo(1l)); + assertThat(scriptCounter.get(), equalTo(0)); + + scriptCounter.set(0); + logger.info("running script filter with new parameters"); + response = client.prepareSearch() + .setQuery(filteredQuery(termQuery("test", "1"), scriptFilter(script).addParam("param1", "1").cache(true))) + .execute().actionGet(); + + assertThat(response.hits().totalHits(), equalTo(1l)); + assertThat(scriptCounter.get(), equalTo(3)); + + scriptCounter.set(0); + logger.info("running script filter with same parameters"); + response = client.prepareSearch() + .setQuery(filteredQuery(matchAllQuery(), scriptFilter(script).addParam("param1", "1").cache(true))) + .execute().actionGet(); + + assertThat(response.hits().totalHits(), equalTo(3l)); + assertThat(scriptCounter.get(), equalTo(0)); + } } \ No newline at end of file