Scripts: Convert template script engines to return String instead of BytesReference (#24447)

Template script engines (mustache, the only one) currently return a
BytesReference that users must know is utf8 encoded. This commit
modifies all callers and mustache to have the template engine return
String. This is much simpler, and does not require decoding in order to
use (for example, in ingest).
This commit is contained in:
Ryan Ernst 2017-05-15 22:37:31 -07:00 committed by GitHub
parent 548a5c1386
commit 6ce597a378
13 changed files with 36 additions and 78 deletions

View File

@ -105,7 +105,7 @@ public class QueryRewriteContext {
return nowInMillis.getAsLong();
}
public BytesReference getTemplateBytes(Script template) {
public String getTemplateBytes(Script template) {
CompiledTemplate compiledTemplate = scriptService.compileTemplate(template, ScriptContext.Standard.SEARCH);
return compiledTemplate.run(template.getParams());
}

View File

@ -392,7 +392,7 @@ public class QueryShardContext extends QueryRewriteContext {
}
@Override
public final BytesReference getTemplateBytes(Script template) {
public final String getTemplateBytes(Script template) {
failIfFrozen();
return super.getTemplateBytes(template);
}

View File

@ -48,7 +48,7 @@ public class InternalTemplateService implements TemplateService {
return new Template() {
@Override
public String execute(Map<String, Object> model) {
return compiledTemplate.run(model).utf8ToString();
return compiledTemplate.run(model);
}
@Override

View File

@ -325,7 +325,7 @@ public class ScriptService extends AbstractComponent implements Closeable, Clust
/** Compiles a template. Note this will be moved to a separate TemplateService in the future. */
public CompiledTemplate compileTemplate(Script script, ScriptContext scriptContext) {
CompiledScript compiledScript = compile(script, scriptContext);
return params -> (BytesReference)executable(compiledScript, params).run();
return params -> (String)executable(compiledScript, params).run();
}
/**

View File

@ -116,7 +116,7 @@ public final class PhraseSuggester extends Suggester<PhraseSuggestionContext> {
vars.put(SUGGESTION_TEMPLATE_VAR_NAME, spare.toString());
QueryShardContext shardContext = suggestion.getShardContext();
final ExecutableScript executable = collateScript.apply(vars);
final BytesReference querySource = (BytesReference) executable.run();
final String querySource = (String) executable.run();
try (XContentParser parser = XContentFactory.xContent(querySource).createParser(shardContext.getXContentRegistry(),
querySource)) {
QueryBuilder innerQueryBuilder = shardContext.newParseContext(parser).parseInnerQueryBuilder();

View File

@ -31,5 +31,5 @@ import org.elasticsearch.script.ScriptType;
public interface CompiledTemplate {
/** Run a template and return the resulting string, encoded in utf8 bytes. */
BytesReference run(Map<String, Object> params);
String run(Map<String, Object> params);
}

View File

@ -1156,7 +1156,7 @@ public class SuggestSearchIT extends ESIntegTestCase {
@Override
public Object run() {
return new BytesArray(result);
return result;
}
};
}

View File

@ -39,6 +39,7 @@ import org.elasticsearch.script.SearchScript;
import org.elasticsearch.search.lookup.SearchLookup;
import java.io.Reader;
import java.io.StringWriter;
import java.lang.ref.SoftReference;
import java.security.AccessController;
import java.security.PrivilegedAction;
@ -58,21 +59,6 @@ public final class MustacheScriptEngine implements ScriptEngine {
public static final String NAME = "mustache";
/** Thread local UTF8StreamWriter to store template execution results in, thread local to save object creation.*/
private static ThreadLocal<SoftReference<UTF8StreamWriter>> utf8StreamWriter = new ThreadLocal<>();
/** If exists, reset and return, otherwise create, reset and return a writer.*/
private static UTF8StreamWriter utf8StreamWriter() {
SoftReference<UTF8StreamWriter> ref = utf8StreamWriter.get();
UTF8StreamWriter writer = (ref == null) ? null : ref.get();
if (writer == null) {
writer = new UTF8StreamWriter(1024 * 4);
utf8StreamWriter.set(new SoftReference<>(writer));
}
writer.reset();
return writer;
}
/**
* Compile a template string to (in this case) a Mustache object than can
* later be re-used for execution to fill in missing parameter values.
@ -146,8 +132,8 @@ public final class MustacheScriptEngine implements ScriptEngine {
@Override
public Object run() {
final BytesStreamOutput result = new BytesStreamOutput();
try (UTF8StreamWriter writer = utf8StreamWriter().setOutput(result)) {
final StringWriter writer = new StringWriter();
try {
// crazy reflection here
SpecialPermission.check();
AccessController.doPrivileged((PrivilegedAction<Void>) () -> {
@ -158,7 +144,7 @@ public final class MustacheScriptEngine implements ScriptEngine {
logger.error((Supplier<?>) () -> new ParameterizedMessage("Error running {}", template), e);
throw new GeneralScriptException("Error running " + template, e);
}
return result.bytes();
return writer.toString();
}
}

View File

@ -26,6 +26,7 @@ import org.elasticsearch.action.search.TransportSearchAction;
import org.elasticsearch.action.support.ActionFilters;
import org.elasticsearch.action.support.HandledTransportAction;
import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.inject.Inject;
import org.elasticsearch.common.settings.Settings;
@ -102,11 +103,10 @@ public class TransportSearchTemplateAction extends HandledTransportAction<Search
Script script = new Script(searchTemplateRequest.getScriptType(), TEMPLATE_LANG, searchTemplateRequest.getScript(),
searchTemplateRequest.getScriptParams() == null ? Collections.emptyMap() : searchTemplateRequest.getScriptParams());
CompiledTemplate compiledScript = scriptService.compileTemplate(script, SEARCH);
BytesReference source = compiledScript.run(script.getParams());
response.setSource(source);
String source = compiledScript.run(script.getParams());
response.setSource(new BytesArray(source));
SearchRequest searchRequest = searchTemplateRequest.getRequest();
response.setSource(source);
if (searchTemplateRequest.isSimulate()) {
return null;
}

View File

@ -68,8 +68,7 @@ public class CustomMustacheFactoryTests extends ESTestCase {
CompiledScript compiled = new CompiledScript(INLINE, null, MustacheScriptEngine.NAME, script);
ExecutableScript executable = engine.executable(compiled, singletonMap("value", "a \"value\""));
BytesReference result = (BytesReference) executable.run();
assertThat(result.utf8ToString(), equalTo("{\"field\": \"a \\\"value\\\"\"}"));
assertThat(executable.run(), equalTo("{\"field\": \"a \\\"value\\\"\"}"));
}
public void testDefaultEncoder() {
@ -80,8 +79,7 @@ public class CustomMustacheFactoryTests extends ESTestCase {
CompiledScript compiled = new CompiledScript(INLINE, null, MustacheScriptEngine.NAME, script);
ExecutableScript executable = engine.executable(compiled, singletonMap("value", "a \"value\""));
BytesReference result = (BytesReference) executable.run();
assertThat(result.utf8ToString(), equalTo("{\"field\": \"a \"value\"\"}"));
assertThat(executable.run(), equalTo("{\"field\": \"a \"value\"\"}"));
}
public void testUrlEncoder() {
@ -92,7 +90,6 @@ public class CustomMustacheFactoryTests extends ESTestCase {
CompiledScript compiled = new CompiledScript(INLINE, null, MustacheScriptEngine.NAME, script);
ExecutableScript executable = engine.executable(compiled, singletonMap("value", "tilde~ AND date:[2016 FROM*]"));
BytesReference result = (BytesReference) executable.run();
assertThat(result.utf8ToString(), equalTo("{\"field\": \"tilde%7E+AND+date%3A%5B2016+FROM*%5D\"}"));
assertThat(executable.run(), equalTo("{\"field\": \"tilde%7E+AND+date%3A%5B2016+FROM*%5D\"}"));
}
}

View File

@ -58,11 +58,11 @@ public class MustacheScriptEngineTests extends ESTestCase {
+ "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}" + "}}, \"negative_boost\": {{boost_val}} } }}";
Map<String, Object> vars = new HashMap<>();
vars.put("boost_val", "0.3");
BytesReference o = (BytesReference) qe.executable(new CompiledScript(ScriptType.INLINE, "", "mustache",
String o = (String) qe.executable(new CompiledScript(ScriptType.INLINE, "", "mustache",
qe.compile(null, template, compileParams)), vars).run();
assertEquals("GET _search {\"query\": {\"boosting\": {\"positive\": {\"match\": {\"body\": \"gift\"}},"
+ "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}}}, \"negative_boost\": 0.3 } }}",
o.utf8ToString());
o);
}
{
String template = "GET _search {\"query\": " + "{\"boosting\": {" + "\"positive\": {\"match\": {\"body\": \"gift\"}},"
@ -70,11 +70,11 @@ public class MustacheScriptEngineTests extends ESTestCase {
Map<String, Object> vars = new HashMap<>();
vars.put("boost_val", "0.3");
vars.put("body_val", "\"quick brown\"");
BytesReference o = (BytesReference) qe.executable(new CompiledScript(ScriptType.INLINE, "", "mustache",
String o = (String) qe.executable(new CompiledScript(ScriptType.INLINE, "", "mustache",
qe.compile(null, template, compileParams)), vars).run();
assertEquals("GET _search {\"query\": {\"boosting\": {\"positive\": {\"match\": {\"body\": \"gift\"}},"
+ "\"negative\": {\"term\": {\"body\": {\"value\": \"\\\"quick brown\\\"\"}}}, \"negative_boost\": 0.3 } }}",
o.utf8ToString());
o);
}
}
@ -89,7 +89,7 @@ public class MustacheScriptEngineTests extends ESTestCase {
CompiledScript compiledScript = new CompiledScript(ScriptType.INLINE, null, "mustache",
qe.compile(null, script.getIdOrCode(), Collections.emptyMap()));
ExecutableScript executableScript = qe.executable(compiledScript, script.getParams());
assertThat(((BytesReference) executableScript.run()).utf8ToString(), equalTo("{\"match_all\":{}}"));
assertThat(executableScript.run(), equalTo("{\"match_all\":{}}"));
}
public void testParseTemplateAsSingleStringWithConditionalClause() throws IOException {
@ -105,7 +105,7 @@ public class MustacheScriptEngineTests extends ESTestCase {
CompiledScript compiledScript = new CompiledScript(ScriptType.INLINE, null, "mustache",
qe.compile(null, script.getIdOrCode(), Collections.emptyMap()));
ExecutableScript executableScript = qe.executable(compiledScript, script.getParams());
assertThat(((BytesReference) executableScript.run()).utf8ToString(), equalTo("{ \"match_all\":{} }"));
assertThat(executableScript.run(), equalTo("{ \"match_all\":{} }"));
}
public void testEscapeJson() throws IOException {

View File

@ -71,7 +71,7 @@ public class MustacheTests extends ESTestCase {
"Mustache templating broken",
"GET _search {\"query\": {\"boosting\": {\"positive\": {\"match\": {\"body\": \"gift\"}},"
+ "\"negative\": {\"term\": {\"body\": {\"value\": \"solr\"}}}, \"negative_boost\": 0.2 } }}",
((BytesReference) result.run()).utf8ToString()
result.run()
);
}
@ -83,22 +83,16 @@ public class MustacheTests extends ESTestCase {
new String[] { "foo", "bar" },
Arrays.asList("foo", "bar"));
vars.put("data", data);
Object output = engine.executable(mustache, vars).run();
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
BytesReference bytes = (BytesReference) output;
assertThat(bytes.utf8ToString(), equalTo("foo bar"));
assertThat(engine.executable(mustache, vars).run(), equalTo("foo bar"));
// Sets can come out in any order
Set<String> setData = new HashSet<>();
setData.add("foo");
setData.add("bar");
vars.put("data", setData);
output = engine.executable(mustache, vars).run();
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
bytes = (BytesReference) output;
assertThat(bytes.utf8ToString(), both(containsString("foo")).and(containsString("bar")));
Object output = engine.executable(mustache, vars).run();
assertThat(output, instanceOf(String.class));
assertThat((String)output, both(containsString("foo")).and(containsString("bar")));
}
public void testArrayInArrayAccess() throws Exception {
@ -111,11 +105,7 @@ public class MustacheTests extends ESTestCase {
singleton(new String[] { "foo", "bar" })
);
vars.put("data", data);
Object output = engine.executable(mustache, vars).run();
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
BytesReference bytes = (BytesReference) output;
assertThat(bytes.utf8ToString(), equalTo("foo bar"));
assertThat(engine.executable(mustache, vars).run(), equalTo("foo bar"));
}
public void testMapInArrayAccess() throws Exception {
@ -126,22 +116,16 @@ public class MustacheTests extends ESTestCase {
new Object[] { singletonMap("key", "foo"), singletonMap("key", "bar") },
Arrays.asList(singletonMap("key", "foo"), singletonMap("key", "bar")));
vars.put("data", data);
Object output = engine.executable(mustache, vars).run();
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
BytesReference bytes = (BytesReference) output;
assertThat(bytes.utf8ToString(), equalTo("foo bar"));
assertThat(engine.executable(mustache, vars).run(), equalTo("foo bar"));
// HashSet iteration order isn't fixed
Set<Object> setData = new HashSet<>();
setData.add(singletonMap("key", "foo"));
setData.add(singletonMap("key", "bar"));
vars.put("data", setData);
output = engine.executable(mustache, vars).run();
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
bytes = (BytesReference) output;
assertThat(bytes.utf8ToString(), both(containsString("foo")).and(containsString("bar")));
Object output = engine.executable(mustache, vars).run();
assertThat(output, instanceOf(String.class));
assertThat((String)output, both(containsString("foo")).and(containsString("bar")));
}
@ -156,14 +140,8 @@ public class MustacheTests extends ESTestCase {
data.put("list", randomList);
Map<String, Object> vars = new HashMap<>();
vars.put("data", data);
Object output = engine.executable(mustache, vars).run();
assertThat(output, notNullValue());
assertThat(output, instanceOf(BytesReference.class));
BytesReference bytes = (BytesReference) output;
String expectedString = String.format(Locale.ROOT, "%s %s", randomArrayValues.length, randomList.size());
assertThat(bytes.utf8ToString(), equalTo(expectedString));
assertThat(engine.executable(mustache, vars).run(), equalTo(expectedString));
}
public void testPrimitiveToJSON() throws Exception {
@ -399,9 +377,7 @@ public class MustacheTests extends ESTestCase {
private void assertScript(String script, Map<String, Object> vars, Matcher<Object> matcher) {
Object result = engine.executable(new CompiledScript(INLINE, "inline", "mustache", compile(script)), vars).run();
assertThat(result, notNullValue());
assertThat(result, instanceOf(BytesReference.class));
assertThat(((BytesReference) result).utf8ToString(), matcher);
assertThat(result, matcher);
}
private Object compile(String script) {

View File

@ -22,7 +22,6 @@ package org.elasticsearch.script;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.search.Scorer;
import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.search.lookup.LeafSearchLookup;
import org.elasticsearch.search.lookup.SearchLookup;
@ -132,7 +131,7 @@ public class MockScriptEngine implements ScriptEngine {
if (vars != null) {
context.putAll(vars);
}
return new MockExecutableScript(context, script != null ? script : ctx -> new BytesArray(source));
return new MockExecutableScript(context, script != null ? script : ctx -> source);
}
public SearchScript createSearchScript(Map<String, Object> vars, SearchLookup lookup) {