Skip to content

Commit

Permalink
Deprecate Empty Templates (#30194)
Browse files Browse the repository at this point in the history 
Deprecate the use of empty templates.  Bug fix allows empty
templates/scripts to be loaded on start up for upgrades/restarts, 
but empty templates can no longer be created.
  • Loading branch information
@jdconrad
jdconrad authored May 14, 2018
1 parent 7928270 commit 1b0e6ee
Show file tree
Hide file tree
Showing 5 changed files with 148 additions and 14 deletions.
21 changes: 18 additions & 3 deletions server/src/main/java/org/elasticsearch/script/ScriptMetaData.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.ToXContentFragment;
import org.elasticsearch.common.xcontent.XContentBuilder;
import org.elasticsearch.common.xcontent.XContentParser;
Expand All @@ -46,6 +48,11 @@
*/
public final class ScriptMetaData implements MetaData.Custom, Writeable, ToXContentFragment {

/**
* Standard deprecation logger for used to deprecate allowance of empty templates.
*/
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ScriptMetaData.class));

/**
* A builder used to modify the currently stored scripts data held within
* the {@link ClusterState}. Scripts can be added or deleted, then built
Expand Down Expand Up @@ -161,8 +168,8 @@ static ScriptMetaData deleteStoredScript(ScriptMetaData previous, String id) {
*
* {@code
* {
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>",
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser)}>",
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser, boolean)}>",
* "<id>" : "<{@link StoredScriptSource#fromXContent(XContentParser, boolean)}>",
* ...
* }
* }
Expand Down Expand Up @@ -209,6 +216,14 @@ public static ScriptMetaData fromXContent(XContentParser parser) throws IOExcept
lang = id.substring(0, split);
id = id.substring(split + 1);
source = new StoredScriptSource(lang, parser.text(), Collections.emptyMap());

if (source.getSource().isEmpty()) {
if (source.getLang().equals(Script.DEFAULT_TEMPLATE_LANG)) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
} else {
DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
}
}
}

exists = scripts.get(id);
Expand All @@ -231,7 +246,7 @@ public static ScriptMetaData fromXContent(XContentParser parser) throws IOExcept
}

exists = scripts.get(id);
source = StoredScriptSource.fromXContent(parser);
source = StoredScriptSource.fromXContent(parser, true);

if (exists == null) {
scripts.put(id, source);
Expand Down
62 changes: 53 additions & 9 deletions server/src/main/java/org/elasticsearch/script/StoredScriptSource.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.LoggingDeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.ObjectParser;
Expand All @@ -57,6 +59,11 @@
*/
public class StoredScriptSource extends AbstractDiffable<StoredScriptSource> implements Writeable, ToXContentObject {

/**
* Standard deprecation logger for used to deprecate allowance of empty templates.
*/
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(StoredScriptSource.class));

/**
* Standard {@link ParseField} for outer level of stored script source.
*/
Expand Down Expand Up @@ -109,7 +116,7 @@ private void setLang(String lang) {
private void setSource(XContentParser parser) {
try {
if (parser.currentToken() == Token.START_OBJECT) {
//this is really for search templates, that need to be converted to json format
// this is really for search templates, that need to be converted to json format
XContentBuilder builder = XContentFactory.jsonBuilder();
source = Strings.toString(builder.copyCurrentStructure(parser));
options.put(Script.CONTENT_TYPE_OPTION, XContentType.JSON.mediaType());
Expand All @@ -131,18 +138,38 @@ private void setOptions(Map<String, String> options) {

/**
* Validates the parameters and creates an {@link StoredScriptSource}.
*
* @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check.
* This allow empty templates to be loaded for backwards compatibility.
* This allow empty templates to be loaded for backwards compatibility.
*/
private StoredScriptSource build() {
private StoredScriptSource build(boolean ignoreEmpty) {
if (lang == null) {
throw new IllegalArgumentException("must specify lang for stored script");
} else if (lang.isEmpty()) {
throw new IllegalArgumentException("lang cannot be empty");
}

if (source == null) {
throw new IllegalArgumentException("must specify source for stored script");
if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
} else {
DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
}
} else {
throw new IllegalArgumentException("must specify source for stored script");
}
} else if (source.isEmpty()) {
throw new IllegalArgumentException("source cannot be empty");
if (ignoreEmpty || Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
if (Script.DEFAULT_TEMPLATE_LANG.equals(lang)) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
} else {
DEPRECATION_LOGGER.deprecated("empty scripts should no longer be used");
}
} else {
throw new IllegalArgumentException("source cannot be empty");
}
}

if (options.size() > 1 || options.size() == 1 && options.get(Script.CONTENT_TYPE_OPTION) == null) {
Expand Down Expand Up @@ -257,6 +284,8 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
token = parser.nextToken();

if (token == Token.END_OBJECT) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");

return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());
}

Expand All @@ -271,7 +300,7 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
token = parser.nextToken();

if (token == Token.START_OBJECT) {
return PARSER.apply(parser, null).build();
return PARSER.apply(parser, null).build(false);
} else {
throw new ParsingException(parser.getTokenLocation(), "unexpected token [" + token + "], expected [{, <source>]");
}
Expand All @@ -280,7 +309,13 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
token = parser.nextToken();

if (token == Token.VALUE_STRING) {
return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, parser.text(), Collections.emptyMap());
String source = parser.text();

if (source == null || source.isEmpty()) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
}

return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
}
}

Expand All @@ -293,7 +328,13 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
builder.copyCurrentStructure(parser);
}

return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, Strings.toString(builder), Collections.emptyMap());
String source = Strings.toString(builder);

if (source == null || source.isEmpty()) {
DEPRECATION_LOGGER.deprecated("empty templates should no longer be used");
}

return new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, source, Collections.emptyMap());
}
}
} catch (IOException ioe) {
Expand All @@ -320,9 +361,12 @@ public static StoredScriptSource parse(BytesReference content, XContentType xCon
*
* Note that the "source" parameter can also handle template parsing including from
* a complex JSON object.
*
* @param ignoreEmpty Specify as {@code true} to ignoreEmpty the empty source check.
* This allows empty templates to be loaded for backwards compatibility.
*/
public static StoredScriptSource fromXContent(XContentParser parser) {
return PARSER.apply(parser, null).build();
public static StoredScriptSource fromXContent(XContentParser parser, boolean ignoreEmpty) {
return PARSER.apply(parser, null).build(ignoreEmpty);
}

/**
Expand Down
41 changes: 41 additions & 0 deletions server/src/test/java/org/elasticsearch/script/ScriptMetaDataTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.io.stream.Writeable;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.common.xcontent.DeprecationHandler;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentBuilder;
Expand Down Expand Up @@ -130,6 +132,45 @@ public void testBuilder() {
assertEquals("1 + 1", result.getStoredScript("_id").getSource());
}

public void testLoadEmptyScripts() throws IOException {
XContentBuilder builder = XContentFactory.jsonBuilder();
builder.startObject().field("mustache#empty", "").endObject();
XContentParser parser = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
BytesReference.bytes(builder).streamInput());
ScriptMetaData.fromXContent(parser);
assertWarnings("empty templates should no longer be used");

builder = XContentFactory.jsonBuilder();
builder.startObject().field("lang#empty", "").endObject();
parser = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
BytesReference.bytes(builder).streamInput());
ScriptMetaData.fromXContent(parser);
assertWarnings("empty scripts should no longer be used");

builder = XContentFactory.jsonBuilder();
builder.startObject().startObject("script").field("lang", "lang").field("source", "").endObject().endObject();
parser = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
BytesReference.bytes(builder).streamInput());
ScriptMetaData.fromXContent(parser);
assertWarnings("empty scripts should no longer be used");

builder = XContentFactory.jsonBuilder();
builder.startObject().startObject("script").field("lang", "mustache").field("source", "").endObject().endObject();
parser = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION,
BytesReference.bytes(builder).streamInput());
ScriptMetaData.fromXContent(parser);
assertWarnings("empty templates should no longer be used");
}

@Override
protected boolean enableWarningsCheck() {
return true;
}

private ScriptMetaData randomScriptMetaData(XContentType sourceContentType, int minNumberScripts) throws IOException {
ScriptMetaData.Builder builder = new ScriptMetaData.Builder(null);
int numScripts = scaledRandomIntBetween(minNumberScripts, 32);
Expand Down
2 changes: 1 addition & 1 deletion server/src/test/java/org/elasticsearch/script/StoredScriptSourceTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ protected StoredScriptSource createTestInstance() {

@Override
protected StoredScriptSource doParseInstance(XContentParser parser) {
return StoredScriptSource.fromXContent(parser);
return StoredScriptSource.fromXContent(parser, false);
}

@Override
Expand Down
36 changes: 35 additions & 1 deletion server/src/test/java/org/elasticsearch/script/StoredScriptTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import org.elasticsearch.common.xcontent.XContentType;
import org.elasticsearch.test.AbstractSerializingTestCase;

import java.io.IOException;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -204,6 +205,39 @@ public void testSourceParsingErrors() throws Exception {
}
}

public void testEmptyTemplateDeprecations() throws IOException {
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
builder.startObject().endObject();

StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());

assertThat(parsed, equalTo(source));
assertWarnings("empty templates should no longer be used");
}

try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
builder.startObject().field("template", "").endObject();

StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());

assertThat(parsed, equalTo(source));
assertWarnings("empty templates should no longer be used");
}

try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {
builder.startObject().field("script").startObject().field("lang", "mustache")
.field("source", "").endObject().endObject();

StoredScriptSource parsed = StoredScriptSource.parse(BytesReference.bytes(builder), XContentType.JSON);
StoredScriptSource source = new StoredScriptSource(Script.DEFAULT_TEMPLATE_LANG, "", Collections.emptyMap());

assertThat(parsed, equalTo(source));
assertWarnings("empty templates should no longer be used");
}
}

@Override
protected StoredScriptSource createTestInstance() {
return new StoredScriptSource(
Expand All @@ -219,7 +253,7 @@ protected Writeable.Reader<StoredScriptSource> instanceReader() {

@Override
protected StoredScriptSource doParseInstance(XContentParser parser) {
return StoredScriptSource.fromXContent(parser);
return StoredScriptSource.fromXContent(parser, false);
}

@Override
Expand Down

0 comments on commit 1b0e6ee

Please sign in to comment.