Fix ScriptFilter cache key calculation

Fixes elastic#2651

Author: imotov

src/main/java/org/elasticsearch/index/query/ScriptFilterParser.java

@@ -19,26 +19,26 @@
 
 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;
 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 Filter parse(QueryParseContext parseContext) throws IOException, QueryPar
  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 static class ScriptFilter extends Filter {
 
  private final SearchScript searchScript;
 
- private ScriptFilter(String scriptLang, String script, Map<String, Object> params, ScriptService scriptService) {
+ public ScriptFilter(String scriptLang, String script, Map<String, Object> 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

src/test/java/org/elasticsearch/test/integration/search/scriptfilter/ScriptFilterSearchTests.java

@@ -30,11 +30,12 @@
 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 @@ protected Client getClient() {
 
  @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 void testCustomScriptBoost() throws Exception {
  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));
+ }
 }