From 690641d0c95d32f89712e651118de4deeaf66478 Mon Sep 17 00:00:00 2001 From: uboness Date: Mon, 4 May 2015 20:15:45 +0200 Subject: [PATCH] Cleaned up the `Script` and `Template` constructs Change them to consistently serialize themselves the same way they deserialize themselves. If the script (or template) is read from a `string` value, it will serialize it self to xcontent as a `string` value as well. Otherwise it will serialize as an object, holding only those fields that where configured in the first place. Original commit: elastic/x-pack-elasticsearch@52a82e0bbe055b606d0a3d65d359384981a9920c --- .../elasticsearch/watcher/support/Script.java | 83 ++++++++++--------- .../watcher/support/template/Template.java | 33 +++++--- .../watcher/support/FilterXContentTests.java | 1 - .../script/ScriptTransformTests.java | 2 +- 4 files changed, 66 insertions(+), 53 deletions(-) diff --git a/src/main/java/org/elasticsearch/watcher/support/Script.java b/src/main/java/org/elasticsearch/watcher/support/Script.java index 580f3e21b82..4f542527730 100644 --- a/src/main/java/org/elasticsearch/watcher/support/Script.java +++ b/src/main/java/org/elasticsearch/watcher/support/Script.java @@ -5,7 +5,9 @@ */ package org.elasticsearch.watcher.support; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.ParseField; +import org.elasticsearch.common.collect.ImmutableMap; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentParser; @@ -31,16 +33,16 @@ public class Script implements ToXContent { public static final ParseField PARAMS_FIELD = new ParseField("params"); private final String script; - private final ScriptService.ScriptType type; - private final String lang; - private final Map params; + private final @Nullable ScriptService.ScriptType type; + private final @Nullable String lang; + private final @Nullable Map params; public Script(String script) { - this(script, DEFAULT_TYPE, DEFAULT_LANG, Collections.emptyMap()); + this(script, null, null, null); } public Script(String script, ScriptService.ScriptType type, String lang) { - this(script, type, lang, Collections.emptyMap()); + this(script, type, lang, null); } public Script(String script, ScriptService.ScriptType type, String lang, Map params) { @@ -55,15 +57,15 @@ public class Script implements ToXContent { } public ScriptService.ScriptType type() { - return type; + return type != null ? type : ScriptService.ScriptType.INLINE; } public String lang() { - return lang; + return lang != null ? lang : DEFAULT_LANG; } public Map params() { - return params; + return params != null ? params : ImmutableMap.of(); } @Override @@ -73,50 +75,53 @@ public class Script implements ToXContent { Script script1 = (Script) o; - if (!lang.equals(script1.lang)) return false; - if (!params.equals(script1.params)) return false; if (!script.equals(script1.script)) return false; if (type != script1.type) return false; - - return true; + if (lang != null ? !lang.equals(script1.lang) : script1.lang != null) return false; + return !(params != null ? !params.equals(script1.params) : script1.params != null); } @Override public int hashCode() { int result = script.hashCode(); - result = 31 * result + type.hashCode(); - result = 31 * result + lang.hashCode(); - result = 31 * result + params.hashCode(); + result = 31 * result + (type != null ? type.hashCode() : 0); + result = 31 * result + (lang != null ? lang.hashCode() : 0); + result = 31 * result + (params != null ? params.hashCode() : 0); return result; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - return builder.startObject() - .field(SCRIPT_FIELD.getPreferredName(), script) - .field(TYPE_FIELD.getPreferredName(), type.name().toLowerCase(Locale.ROOT)) - .field(LANG_FIELD.getPreferredName(), lang) - .field(PARAMS_FIELD.getPreferredName(), this.params) - .endObject(); + if (type == null && lang == null && params == null) { + return builder.value(script); + } + builder.startObject(); + builder.field(SCRIPT_FIELD.getPreferredName(), script); + if (type != null) { + builder.field(TYPE_FIELD.getPreferredName(), type.name().toLowerCase(Locale.ROOT)); + } + if (lang != null) { + builder.field(LANG_FIELD.getPreferredName(), lang); + } + if (this.params != null) { + builder.field(PARAMS_FIELD.getPreferredName(), this.params); + } + return builder.endObject(); } public static Script parse(XContentParser parser) throws IOException { - return parse(parser, ScriptService.DEFAULT_LANG); - } - - public static Script parse(XContentParser parser, String defaultLang) throws IOException { XContentParser.Token token = parser.currentToken(); if (token == XContentParser.Token.VALUE_STRING) { return new Script(parser.text()); } if (token != XContentParser.Token.START_OBJECT) { - throw new ParseException("expected a string value or an object, but found [" + token + "] instead"); + throw new ParseException("expected a string value or an object, but found [{}] instead", token); } String script = null; - ScriptService.ScriptType type = ScriptService.ScriptType.INLINE; - String lang = defaultLang; - Map params = Collections.emptyMap(); + ScriptService.ScriptType type = null; + String lang = null; + Map params = null; String currentFieldName = null; while ((token = parser.nextToken()) != XContentParser.Token.END_OBJECT) { @@ -126,7 +131,7 @@ public class Script implements ToXContent { if (token == XContentParser.Token.VALUE_STRING) { script = parser.text(); } else { - throw new ParseException("expected a string value for field [" + currentFieldName + "], but found [" + token + "]"); + throw new ParseException("expected a string value for field [{}], but found [{}]", currentFieldName, token); } } else if (TYPE_FIELD.match(currentFieldName)) { if (token == XContentParser.Token.VALUE_STRING) { @@ -134,39 +139,39 @@ public class Script implements ToXContent { try { type = ScriptService.ScriptType.valueOf(value.toUpperCase(Locale.ROOT)); } catch (IllegalArgumentException iae) { - throw new ParseException("unknown script type [" + value + "]"); + throw new ParseException("unknown script type [{}]", value); } } } else if (LANG_FIELD.match(currentFieldName)) { if (token == XContentParser.Token.VALUE_STRING) { lang = parser.text(); } else { - throw new ParseException("expected a string value for field [" + currentFieldName + "], but found [" + token + "]"); + throw new ParseException("expected a string value for field [{}], but found [{}]", currentFieldName, token); } } else if (PARAMS_FIELD.match(currentFieldName)) { if (token == XContentParser.Token.START_OBJECT) { params = parser.map(); } else { - throw new ParseException("expected an object for field [" + currentFieldName + "], but found [" + token + "]"); + throw new ParseException("expected an object for field [{}], but found [{}]", currentFieldName, token); } } else { - throw new ParseException("unexpected field [" + currentFieldName + "]"); + throw new ParseException("unexpected field [{}]", currentFieldName); } } if (script == null) { - throw new ParseException("missing required string field [" + currentFieldName + "]"); + throw new ParseException("missing required string field [{}]", SCRIPT_FIELD.getPreferredName()); } return new Script(script, type, lang, params); } public static class ParseException extends WatcherException { - public ParseException(String msg) { - super(msg); + public ParseException(String msg, Object... args) { + super(msg, args); } - public ParseException(String msg, Throwable cause) { - super(msg, cause); + public ParseException(String msg, Throwable cause, Object... args) { + super(msg, cause, args); } } } diff --git a/src/main/java/org/elasticsearch/watcher/support/template/Template.java b/src/main/java/org/elasticsearch/watcher/support/template/Template.java index 978504abc97..b51ebcb7d88 100644 --- a/src/main/java/org/elasticsearch/watcher/support/template/Template.java +++ b/src/main/java/org/elasticsearch/watcher/support/template/Template.java @@ -18,7 +18,6 @@ import java.io.IOException; import java.util.HashMap; import java.util.Locale; import java.util.Map; -import java.util.Objects; /** * @@ -30,10 +29,10 @@ public class Template implements ToXContent { private final @Nullable Map params; public Template(String template) { - this(template, ScriptType.INLINE, ImmutableMap.of()); + this(template, null, null); } - public Template(String template, ScriptType type, Map params) { + public Template(String template, @Nullable ScriptType type, @Nullable Map params) { this.template = template; this.type = type; this.params = params; @@ -55,20 +54,25 @@ public class Template implements ToXContent { public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - Template template = (Template) o; - return Objects.equals(this.template, template.template) && - Objects.equals(type, template.type) && - Objects.equals(params, template.params); + + Template template1 = (Template) o; + + if (!template.equals(template1.template)) return false; + if (type != template1.type) return false; + return !(params != null ? !params.equals(template1.params) : template1.params != null); } @Override public int hashCode() { - return Objects.hash(template, type, params); + int result = template.hashCode(); + result = 31 * result + (type != null ? type.hashCode() : 0); + result = 31 * result + (params != null ? params.hashCode() : 0); + return result; } @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { - if (type == null && params == null) { + if (type == null && this.params == null) { return builder.value(template); } builder.startObject(); @@ -88,7 +92,7 @@ public class Template implements ToXContent { return new Template(String.valueOf(parser.objectText())); } if (token != XContentParser.Token.START_OBJECT) { - throw new ParseException("expected a string value or an object, but found [" + token + "] instead"); + throw new ParseException("expected a string value or an object, but found [{}]instead", token); } String template = null; @@ -138,7 +142,7 @@ public class Template implements ToXContent { private final String template; private ScriptType type; - private HashMap params = new HashMap<>(); + private HashMap params; private Builder(String template) { this.template = template; @@ -150,17 +154,22 @@ public class Template implements ToXContent { } public Builder putParams(Map params) { + if (params == null) { + params = new HashMap<>(); + } this.params.putAll(params); return this; } public Builder putParam(String key, Object value) { + if (params == null) { + params = new HashMap<>(); + } params.put(key, value); return this; } public Template build() { - type = type != null ? type : ScriptType.INLINE; return new Template(template, type, params); } } diff --git a/src/test/java/org/elasticsearch/watcher/support/FilterXContentTests.java b/src/test/java/org/elasticsearch/watcher/support/FilterXContentTests.java index 36ace34fbe7..ddc7543bc2b 100644 --- a/src/test/java/org/elasticsearch/watcher/support/FilterXContentTests.java +++ b/src/test/java/org/elasticsearch/watcher/support/FilterXContentTests.java @@ -48,7 +48,6 @@ public class FilterXContentTests extends ElasticsearchTestCase { added = keys.add("key" + randomInt(7)); } while (!added); } - System.out.println(keys); Map filteredData = XContentFilterKeysUtils.filterMapOrdered(keys, parser); assertThat(filteredData.size(), equalTo(numKeys)); diff --git a/src/test/java/org/elasticsearch/watcher/transform/script/ScriptTransformTests.java b/src/test/java/org/elasticsearch/watcher/transform/script/ScriptTransformTests.java index 873c2bf795b..3c4158d8e4d 100644 --- a/src/test/java/org/elasticsearch/watcher/transform/script/ScriptTransformTests.java +++ b/src/test/java/org/elasticsearch/watcher/transform/script/ScriptTransformTests.java @@ -144,7 +144,7 @@ public class ScriptTransformTests extends ElasticsearchTestCase { XContentParser parser = JsonXContent.jsonXContent.createParser(builder.bytes()); parser.nextToken(); ExecutableScriptTransform transform = new ScriptTransformFactory(ImmutableSettings.EMPTY, service).parseExecutable("_id", parser); - assertThat(transform.transform().getScript(), equalTo(new Script("_script", ScriptService.ScriptType.INLINE, ScriptService.DEFAULT_LANG, ImmutableMap.of()))); + assertThat(transform.transform().getScript(), equalTo(new Script("_script"))); }