From ce6f6d3835d682dda631f7373e5de2588d8c0512 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Thu, 6 Oct 2016 08:55:31 +0100 Subject: [PATCH] Review comments --- .../org/elasticsearch/index/query/QueryShardContext.java | 6 ++++-- .../main/java/org/elasticsearch/script/ScriptService.java | 6 +++++- .../java/org/elasticsearch/test/AbstractQueryTestCase.java | 6 ++++-- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java index abe573c0767..11af6f919ae 100644 --- a/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java +++ b/core/src/main/java/org/elasticsearch/index/query/QueryShardContext.java @@ -340,7 +340,8 @@ public class QueryShardContext extends QueryRewriteContext { } /** - * Compiles (or retrieves from cache) and executes the provided script + * Compiles (or retrieves from cache) and binds the parameters to the + * provided script */ public SearchScript getSearchScript(Script script, ScriptContext context, Map params) { failIfFrozen(); @@ -358,7 +359,8 @@ public class QueryShardContext extends QueryRewriteContext { } /** - * Compiles (or retrieves from cache) and executes the provided script + * Compiles (or retrieves from cache) and binds the parameters to the + * provided script */ public ExecutableScript getExecutableScript(Script script, ScriptContext context, Map params) { failIfFrozen(); diff --git a/core/src/main/java/org/elasticsearch/script/ScriptService.java b/core/src/main/java/org/elasticsearch/script/ScriptService.java index e63a29aac50..24cb816fb15 100644 --- a/core/src/main/java/org/elasticsearch/script/ScriptService.java +++ b/core/src/main/java/org/elasticsearch/script/ScriptService.java @@ -249,7 +249,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust long timePassed = now - lastInlineCompileTime; lastInlineCompileTime = now; - scriptsPerMinCounter += ((double) timePassed) * compilesAllowedPerNano; + scriptsPerMinCounter += (timePassed) * compilesAllowedPerNano; // It's been over the time limit anyway, readjust the bucket to be level if (scriptsPerMinCounter > totalCompilesPerMinute) { @@ -491,6 +491,10 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust return search(lookup, compiledScript, script.getParams()); } + /** + * Binds provided parameters to a compiled script returning a + * {@link SearchScript} ready for execution + */ public SearchScript search(SearchLookup lookup, CompiledScript compiledScript, Map params) { return getScriptEngineServiceForLang(compiledScript.lang()).search(compiledScript, lookup, params); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 16fd64dc941..711c74375f1 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -591,9 +591,11 @@ public abstract class AbstractQueryTestCase> QueryBuilder rewritten = rewriteQuery(firstQuery, new QueryShardContext(context)); Query firstLuceneQuery = rewritten.toQuery(context); if (isCachable(firstQuery)) { - assert context.isCachable() : firstQuery.toString(); + assertTrue("query was marked as not cacheable in the context but this test indicates it should be cacheable: " + + firstQuery.toString(), context.isCachable()); } else { - assert context.isCachable() == false : firstQuery.toString(); + assertFalse("query was marked as cacheable in the context but this test indicates it should not be cacheable: " + + firstQuery.toString(), context.isCachable()); } assertNotNull("toQuery should not return null", firstLuceneQuery); assertLuceneQuery(firstQuery, firstLuceneQuery, context);