Scripts: Convert template uses to return String instead of BytesReference (elastic/x-pack-elasticsearch#1279)

This is the xpack side of elastic/elasticsearch#24447. The one caveat is
there are a number of places within the xpack api that use
BytesReference to pass down to templates. These are convert to encode
into a BytesArray when necessary, so as not to require changing all of
those apis here at once, but they should all be convert to String as
well.

Original commit: elastic/x-pack-elasticsearch@8399b9d8c3
This commit is contained in:
Ryan Ernst 2017-05-15 22:37:51 -07:00 committed by GitHub
parent f5cd833af4
commit ef3b954f63
7 changed files with 18 additions and 16 deletions

View File

@ -57,7 +57,7 @@ public class TextTemplateEngine extends AbstractComponent {
} }
Script script = new Script(textTemplate.getType(), "mustache", template, options, mergedModel); Script script = new Script(textTemplate.getType(), "mustache", template, options, mergedModel);
CompiledTemplate compiledTemplate = service.compileTemplate(script, Watcher.SCRIPT_CONTEXT); CompiledTemplate compiledTemplate = service.compileTemplate(script, Watcher.SCRIPT_CONTEXT);
return compiledTemplate.run(model).utf8ToString(); return compiledTemplate.run(model);
} }
private String trimContentType(TextTemplate textTemplate) { private String trimContentType(TextTemplate textTemplate) {

View File

@ -134,9 +134,9 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper {
BooleanQuery.Builder filter = new BooleanQuery.Builder(); BooleanQuery.Builder filter = new BooleanQuery.Builder();
for (BytesReference bytesReference : permissions.getQueries()) { for (BytesReference bytesReference : permissions.getQueries()) {
QueryShardContext queryShardContext = queryShardContextProvider.apply(shardId); QueryShardContext queryShardContext = queryShardContextProvider.apply(shardId);
bytesReference = evaluateTemplate(bytesReference); String templateResult = evaluateTemplate(bytesReference.utf8ToString());
try (XContentParser parser = XContentFactory.xContent(bytesReference) try (XContentParser parser = XContentFactory.xContent(templateResult)
.createParser(queryShardContext.getXContentRegistry(), bytesReference)) { .createParser(queryShardContext.getXContentRegistry(), templateResult)) {
QueryBuilder queryBuilder = queryShardContext.newParseContext(parser).parseInnerQueryBuilder(); QueryBuilder queryBuilder = queryShardContext.newParseContext(parser).parseInnerQueryBuilder();
verifyRoleQuery(queryBuilder); verifyRoleQuery(queryBuilder);
failIfQueryUsesClient(scriptService, queryBuilder, queryShardContext); failIfQueryUsesClient(scriptService, queryBuilder, queryShardContext);
@ -244,7 +244,7 @@ public class SecurityIndexSearcherWrapper extends IndexSearcherWrapper {
} }
} }
BytesReference evaluateTemplate(BytesReference querySource) throws IOException { String evaluateTemplate(String querySource) throws IOException {
// EMPTY is safe here because we never use namedObject // EMPTY is safe here because we never use namedObject
try (XContentParser parser = XContentFactory.xContent(querySource).createParser(NamedXContentRegistry.EMPTY, querySource)) { try (XContentParser parser = XContentFactory.xContent(querySource).createParser(NamedXContentRegistry.EMPTY, querySource)) {
XContentParser.Token token = parser.nextToken(); XContentParser.Token token = parser.nextToken();

View File

@ -9,6 +9,7 @@ import org.apache.logging.log4j.Logger;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.NamedXContentRegistry;
@ -53,9 +54,9 @@ public class ExecutableSearchInput extends ExecutableInput<SearchInput, SearchIn
WatcherSearchTemplateRequest request = null; WatcherSearchTemplateRequest request = null;
try { try {
Script template = input.getRequest().getOrCreateTemplate(); Script template = input.getRequest().getOrCreateTemplate();
BytesReference renderedTemplate = searchTemplateService.renderTemplate(template, ctx, payload); String renderedTemplate = searchTemplateService.renderTemplate(template, ctx, payload);
// We need to make a copy, so that we don't modify the original instance that we keep around in a watch: // We need to make a copy, so that we don't modify the original instance that we keep around in a watch:
request = new WatcherSearchTemplateRequest(input.getRequest(), renderedTemplate); request = new WatcherSearchTemplateRequest(input.getRequest(), new BytesArray(renderedTemplate));
return doExecute(ctx, request); return doExecute(ctx, request);
} catch (Exception e) { } catch (Exception e) {
logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage()); logger.error("failed to execute [{}] input for watch [{}], reason [{}]", TYPE, ctx.watch().id(), e.getMessage());

View File

@ -41,7 +41,7 @@ public class WatcherSearchTemplateService extends AbstractComponent {
this.xContentRegistry = xContentRegistry; this.xContentRegistry = xContentRegistry;
} }
public BytesReference renderTemplate(Script source, public String renderTemplate(Script source,
WatchExecutionContext ctx, WatchExecutionContext ctx,
Payload payload) throws IOException { Payload payload) throws IOException {
// Due the inconsistency with templates in ES 1.x, we maintain our own template format. // Due the inconsistency with templates in ES 1.x, we maintain our own template format.

View File

@ -11,6 +11,7 @@ import org.apache.logging.log4j.util.Supplier;
import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.action.search.SearchResponse;
import org.elasticsearch.action.search.SearchType; import org.elasticsearch.action.search.SearchType;
import org.elasticsearch.common.Nullable; import org.elasticsearch.common.Nullable;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.script.Script; import org.elasticsearch.script.Script;
@ -44,9 +45,9 @@ public class ExecutableSearchTransform extends ExecutableTransform<SearchTransfo
WatcherSearchTemplateRequest request = null; WatcherSearchTemplateRequest request = null;
try { try {
Script template = transform.getRequest().getOrCreateTemplate(); Script template = transform.getRequest().getOrCreateTemplate();
BytesReference renderedTemplate = searchTemplateService.renderTemplate(template, ctx, payload); String renderedTemplate = searchTemplateService.renderTemplate(template, ctx, payload);
// We need to make a copy, so that we don't modify the original instance that we keep around in a watch: // We need to make a copy, so that we don't modify the original instance that we keep around in a watch:
request = new WatcherSearchTemplateRequest(transform.getRequest(), renderedTemplate); request = new WatcherSearchTemplateRequest(transform.getRequest(), new BytesArray(renderedTemplate));
SearchResponse resp = client.search(searchTemplateService.toSearchRequest(request), timeout); SearchResponse resp = client.search(searchTemplateService.toSearchRequest(request), timeout);
return new SearchTransform.Result(request, new Payload.XContent(resp)); return new SearchTransform.Result(request, new Payload.XContent(resp));
} catch (Exception e) { } catch (Exception e) {

View File

@ -57,7 +57,7 @@ public class TextTemplateTests extends ESTestCase {
merged = unmodifiableMap(merged); merged = unmodifiableMap(merged);
ScriptType type = randomFrom(ScriptType.values()); ScriptType type = randomFrom(ScriptType.values());
CompiledTemplate compiledTemplate = templateParams -> new BytesArray("rendered_text"); CompiledTemplate compiledTemplate = templateParams -> "rendered_text";
when(service.compileTemplate(new Script(type, lang, templateText, when(service.compileTemplate(new Script(type, lang, templateText,
type == ScriptType.INLINE ? Collections.singletonMap("content_type", "text/plain") : null, type == ScriptType.INLINE ? Collections.singletonMap("content_type", "text/plain") : null,
merged), Watcher.SCRIPT_CONTEXT)).thenReturn(compiledTemplate); merged), Watcher.SCRIPT_CONTEXT)).thenReturn(compiledTemplate);
@ -72,7 +72,7 @@ public class TextTemplateTests extends ESTestCase {
Map<String, Object> model = singletonMap("key", "model_val"); Map<String, Object> model = singletonMap("key", "model_val");
ScriptType type = randomFrom(ScriptType.values()); ScriptType type = randomFrom(ScriptType.values());
CompiledTemplate compiledTemplate = templateParams -> new BytesArray("rendered_text"); CompiledTemplate compiledTemplate = templateParams -> "rendered_text";
when(service.compileTemplate(new Script(type, lang, templateText, when(service.compileTemplate(new Script(type, lang, templateText,
type == ScriptType.INLINE ? Collections.singletonMap("content_type", "text/plain") : null, type == ScriptType.INLINE ? Collections.singletonMap("content_type", "text/plain") : null,
model), Watcher.SCRIPT_CONTEXT)).thenReturn(compiledTemplate); model), Watcher.SCRIPT_CONTEXT)).thenReturn(compiledTemplate);
@ -85,7 +85,7 @@ public class TextTemplateTests extends ESTestCase {
String templateText = "_template"; String templateText = "_template";
Map<String, Object> model = singletonMap("key", "model_val"); Map<String, Object> model = singletonMap("key", "model_val");
CompiledTemplate compiledTemplate = templateParams -> new BytesArray("rendered_text"); CompiledTemplate compiledTemplate = templateParams -> "rendered_text";
when(service.compileTemplate(new Script(ScriptType.INLINE, lang, templateText, when(service.compileTemplate(new Script(ScriptType.INLINE, lang, templateText,
Collections.singletonMap("content_type", "text/plain"), model), Watcher.SCRIPT_CONTEXT)) Collections.singletonMap("content_type", "text/plain"), model), Watcher.SCRIPT_CONTEXT))
.thenReturn(compiledTemplate); .thenReturn(compiledTemplate);

View File

@ -465,7 +465,7 @@ public class SecurityIndexSearcherWrapperUnitTests extends ESTestCase {
Script script = new Script(ScriptType.INLINE, "mustache", query, Collections.singletonMap("custom", "value")); Script script = new Script(ScriptType.INLINE, "mustache", query, Collections.singletonMap("custom", "value"));
builder = jsonBuilder().startObject().field("template"); builder = jsonBuilder().startObject().field("template");
script.toXContent(builder, ToXContent.EMPTY_PARAMS); script.toXContent(builder, ToXContent.EMPTY_PARAMS);
BytesReference querySource = builder.endObject().bytes(); String querySource = builder.endObject().string();
securityIndexSearcherWrapper.evaluateTemplate(querySource); securityIndexSearcherWrapper.evaluateTemplate(querySource);
ArgumentCaptor<Script> argument = ArgumentCaptor.forClass(Script.class); ArgumentCaptor<Script> argument = ArgumentCaptor.forClass(Script.class);
@ -492,8 +492,8 @@ public class SecurityIndexSearcherWrapperUnitTests extends ESTestCase {
securityIndexSearcherWrapper = securityIndexSearcherWrapper =
new SecurityIndexSearcherWrapper(indexSettings, null, null, threadContext, licenseState, scriptService); new SecurityIndexSearcherWrapper(indexSettings, null, null, threadContext, licenseState, scriptService);
XContentBuilder builder = jsonBuilder(); XContentBuilder builder = jsonBuilder();
BytesReference querySource = new TermQueryBuilder("field", "value").toXContent(builder, ToXContent.EMPTY_PARAMS).bytes(); String querySource = new TermQueryBuilder("field", "value").toXContent(builder, ToXContent.EMPTY_PARAMS).string();
BytesReference result = securityIndexSearcherWrapper.evaluateTemplate(querySource); String result = securityIndexSearcherWrapper.evaluateTemplate(querySource);
assertThat(result, sameInstance(querySource)); assertThat(result, sameInstance(querySource));
verifyZeroInteractions(scriptService); verifyZeroInteractions(scriptService);
} }