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@52a82e0bbe

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 @Nullable ScriptService.ScriptType type;
-    private final String lang;
+    private final @Nullable String lang;
-    private final Map<String, Object> params;
+    private final @Nullable Map<String, Object> params;
 
     public Script(String script) {
-        this(script, DEFAULT_TYPE, DEFAULT_LANG, Collections.<String, Object>emptyMap());
+        this(script, null, null, null);
     }
 
     public Script(String script, ScriptService.ScriptType type, String lang) {
-        this(script, type, lang, Collections.<String, Object>emptyMap());
+        this(script, type, lang, null);
     }
 
     public Script(String script, ScriptService.ScriptType type, String lang, Map<String, Object> 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<String, Object> params() {
-        return params;
+        return params != null ? params : ImmutableMap.<String, Object>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;
-
+        if (lang != null ? !lang.equals(script1.lang) : script1.lang != null) return false;
-        return true;
+        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 + (type != null ? type.hashCode() : 0);
-        result = 31 * result + lang.hashCode();
+        result = 31 * result + (lang != null ? lang.hashCode() : 0);
-        result = 31 * result + params.hashCode();
+        result = 31 * result + (params != null ? params.hashCode() : 0);
         return result;
     }
 
     @Override
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
-        return builder.startObject()
+        if (type == null && lang == null && params == null) {
-                .field(SCRIPT_FIELD.getPreferredName(), script)
+            return builder.value(script);
-                .field(TYPE_FIELD.getPreferredName(), type.name().toLowerCase(Locale.ROOT))
+        }
-                .field(LANG_FIELD.getPreferredName(), lang)
+        builder.startObject();
-                .field(PARAMS_FIELD.getPreferredName(), this.params)
+        builder.field(SCRIPT_FIELD.getPreferredName(), script);
-                .endObject();
+        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;
+        ScriptService.ScriptType type = null;
-        String lang = defaultLang;
+        String lang = null;
-        Map<String, Object> params = Collections.emptyMap();
+        Map<String, Object> 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) {
+        public ParseException(String msg, Object... args) {
-            super(msg);
+            super(msg, args);
         }
 
-        public ParseException(String msg, Throwable cause) {
+        public ParseException(String msg, Throwable cause, Object... args) {
-            super(msg, cause);
+            super(msg, cause, args);
         }
     }
 }

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<String, Object> params;
 
     public Template(String template) {
-        this(template, ScriptType.INLINE, ImmutableMap.<String, Object>of());
+        this(template, null, null);
     }
 
-    public Template(String template, ScriptType type, Map<String, Object> params) {
+    public Template(String template, @Nullable ScriptType type, @Nullable Map<String, Object> 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) &&
+        Template template1 = (Template) o;
-                Objects.equals(type, template.type) &&
+
-                Objects.equals(params, template.params);
+        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<String, Object> params = new HashMap<>();
+        private HashMap<String, Object> params;
 
         private Builder(String template) {
             this.template = template;
@@ -150,17 +154,22 @@ public class Template implements ToXContent {
         }
 
         public Builder putParams(Map<String, Object> 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);
         }
     }

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<String, Object> filteredData = XContentFilterKeysUtils.filterMapOrdered(keys, parser);
         assertThat(filteredData.size(), equalTo(numKeys));

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.<String, Object>of())));
+        assertThat(transform.transform().getScript(), equalTo(new Script("_script")));
     }