Disable Watcher script optimization for stored scripts (#53497)

The watcher TextTemplateEngine uses a fast path mechanism where it
checks for the existence of `{{` to decide if a mustache script
required compilation. This does not work for stored script, as the field
that is checked contains the id of the script, which means, the name of
the script is returned as its value.

This commit checks for the script type and does not involve this fast
path check if a stored script is used.

Closes #40212
This commit is contained in:
Alexander Reelsen 2020-03-16 18:07:14 +01:00
parent 91ca9c5c33
commit 7571ca437a
4 changed files with 40 additions and 8 deletions

View File

@ -29,12 +29,12 @@ public class TextTemplate implements ToXContent {
private final Script script;
private final String inlineTemplate;
private final boolean isUsingMustache;
private final boolean mayRequireCompilation;
public TextTemplate(String template) {
this.script = null;
this.inlineTemplate = template;
this.isUsingMustache = template.contains("{{");
this.mayRequireCompilation = template.contains("{{");
}
public TextTemplate(String template, @Nullable XContentType contentType, ScriptType type,
@ -50,14 +50,14 @@ public class TextTemplate implements ToXContent {
params = new HashMap<>();
}
this.script = new Script(type, type == ScriptType.STORED ? null : Script.DEFAULT_TEMPLATE_LANG, template, options, params);
this.isUsingMustache = template.contains("{{");
this.inlineTemplate = null;
this.mayRequireCompilation = script.getType() == ScriptType.STORED || script.getIdOrCode().contains("{{");
}
public TextTemplate(Script script) {
this.script = script;
this.inlineTemplate = null;
this.isUsingMustache = script.getIdOrCode().contains("{{");
this.mayRequireCompilation = script.getType() == ScriptType.STORED || script.getIdOrCode().contains("{{");
}
public Script getScript() {
@ -68,8 +68,14 @@ public class TextTemplate implements ToXContent {
return script != null ? script.getIdOrCode() : inlineTemplate;
}
public boolean isUsingMustache() {
return isUsingMustache;
/**
* Check if compilation may be required.
* If a stored script is used, we cannot tell at this stage, so we always assume
* that stored scripts require compilation.
* If an inline script is used, we checked for the mustache opening brackets
*/
public boolean mayRequireCompilation() {
return mayRequireCompilation;
}
public XContentType getContentType() {

View File

@ -32,7 +32,7 @@ public class TextTemplateEngine {
String mediaType = compileParams(detectContentType(template));
template = trimContentType(textTemplate);
if (textTemplate.isUsingMustache() == false) {
if (textTemplate.mayRequireCompilation() == false) {
return template;
}

View File

@ -455,7 +455,7 @@ public class EmailTemplate implements ToXContentObject {
static void validateEmailAddresses(TextTemplate ... emails) {
for (TextTemplate emailTemplate : emails) {
// no mustache, do validation
if (emailTemplate.isUsingMustache() == false) {
if (emailTemplate.mayRequireCompilation() == false) {
String email = emailTemplate.getTemplate();
try {
for (Email.Address address : Email.AddressList.parse(email)) {

View File

@ -225,6 +225,32 @@ public class TextTemplateTests extends ESTestCase {
assertEquals("[1:2] [script] unknown field [type]", ex.getMessage());
}
public void testMustacheTemplateRequiresCompilation() {
final TextTemplate inlineTemplateRequiresCompilation = createTextTemplate(ScriptType.INLINE, "{{foo.bar}}");
assertThat(inlineTemplateRequiresCompilation.mayRequireCompilation(), is(true));
final TextTemplate inlineTemplateNoRequiresCompilation = createTextTemplate(ScriptType.INLINE, "script without mustache");
assertThat(inlineTemplateNoRequiresCompilation.mayRequireCompilation(), is(false));
final TextTemplate storedScriptTemplate = createTextTemplate(ScriptType.STORED, "stored_script_id");
assertThat(storedScriptTemplate.mayRequireCompilation(), is(true));
}
private TextTemplate createTextTemplate(ScriptType type, String idOrCode) {
final TextTemplate template;
if (randomBoolean()) {
if (type == ScriptType.STORED) {
template = new TextTemplate(new Script(type, null, idOrCode, null, Collections.emptyMap()));
} else {
template = new TextTemplate(new Script(type, lang, idOrCode, Collections.emptyMap(), Collections.emptyMap()));
}
} else {
template = new TextTemplate(idOrCode, null, type, null);
}
return template;
}
public void testNullObject() throws Exception {
assertThat(engine.render(null ,new HashMap<>()), is(nullValue()));
}