Decouple more classes from XContentBuilder and make builder strict

server/src/main/java/org/elasticsearch/Version.java

@@ -25,6 +25,8 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.xcontent.ToXContentFragment;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.monitor.jvm.JvmInfo;
 
 import java.io.IOException;
@@ -34,7 +36,7 @@
 import java.util.Collections;
 import java.util.List;
 
-public class Version implements Comparable<Version> {
+public class Version implements Comparable<Version>, ToXContentFragment {
     /*
      * The logic for ID is: XXYYZZAA, where XX is major version, YY is minor version, ZZ is revision, and AA is alpha/beta/rc indicator AA
      * values below 25 are for alpha builder (since 5.0), and above 25 and below 50 are beta builds, and below 99 are RC builds, with 99
@@ -422,6 +424,11 @@ public int compareTo(Version other) {
         return Integer.compare(this.id, other.id);
     }
 
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        return builder.value(toString());
+    }
+
     /*
      * We need the declared versions when computing the minimum compatibility version. As computing the declared versions uses reflection it
      * is not cheap. Since computing the minimum compatibility version can occur often, we use this holder to compute the declared versions

server/src/main/java/org/elasticsearch/common/bytes/BytesReference.java

@@ -23,6 +23,7 @@
 import org.apache.lucene.util.BytesRefIterator;
 import org.elasticsearch.common.io.stream.BytesStream;
 import org.elasticsearch.common.io.stream.StreamInput;
+import org.elasticsearch.common.xcontent.ToXContentFragment;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.ByteArrayOutputStream;
@@ -37,7 +38,7 @@
 /**
  * A reference to bytes.
  */
-public abstract class BytesReference implements Accountable, Comparable<BytesReference> {
+public abstract class BytesReference implements Accountable, Comparable<BytesReference>, ToXContentFragment {
 
     private Integer hash = null; // we cache the hash of this reference since it can be quite costly to re-calculated it
 
@@ -334,4 +335,10 @@ public long skip(long n) throws IOException {
             return input.skip(n);
         }
     }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        BytesRef bytes = toBytesRef();
+        return builder.value(bytes.bytes, bytes.offset, bytes.length);
+    }
 }

server/src/main/java/org/elasticsearch/common/document/DocumentField.java

@@ -19,6 +19,7 @@
 
 package org.elasticsearch.common.document;
 
+import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
@@ -127,11 +128,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
             // Stored fields values are converted using MappedFieldType#valueForDisplay.
             // As a result they can either be Strings, Numbers, or Booleans, that's
             // all.
-            if (value instanceof BytesReference) {
-                builder.binaryValue(((BytesReference) value).toBytesRef());
-            } else {
-                builder.value(value);
-            }
+            builder.value(value);
         }
         builder.endArray();
         return builder;

server/src/main/java/org/elasticsearch/common/text/Text.java

@@ -18,6 +18,7 @@
  */
 package org.elasticsearch.common.text;
 
+import org.apache.lucene.util.BytesRef;
 import org.elasticsearch.common.bytes.BytesArray;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.ToXContent;
@@ -125,7 +126,8 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         } else {
             // TODO: TextBytesOptimization we can use a buffer here to convert it? maybe add a
             // request to jackson to support InputStream as well?
-            return builder.utf8Value(this.bytes().toBytesRef());
+            BytesRef br = this.bytes().toBytesRef();
+            return builder.utf8Value(br.bytes, br.offset, br.length);
         }
     }
 }

server/src/main/java/org/elasticsearch/common/transport/TransportAddress.java

@@ -23,6 +23,9 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.network.NetworkAddress;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContentFragment;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.net.InetAddress;
@@ -32,7 +35,7 @@
 /**
  * A transport address used for IP socket address (wraps {@link java.net.InetSocketAddress}).
  */
-public final class TransportAddress implements Writeable {
+public final class TransportAddress implements Writeable, ToXContentFragment {
 
     /**
      * A <a href="https://en.wikipedia.org/wiki/0.0.0.0">non-routeable v4 meta transport address</a> that can be used for
@@ -128,4 +131,9 @@ public int hashCode() {
     public String toString() {
         return NetworkAddress.format(address);
     }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        return builder.value(toString());
+    }
 }

server/src/main/java/org/elasticsearch/common/unit/ByteSizeValue.java

@@ -27,12 +27,15 @@
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.logging.Loggers;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContentFragment;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.Locale;
 import java.util.Objects;
 
-public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue> {
+public class ByteSizeValue implements Writeable, Comparable<ByteSizeValue>, ToXContentFragment {
     private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ByteSizeValue.class));
 
     private final long size;
@@ -269,4 +272,9 @@ public int compareTo(ByteSizeValue other) {
         long otherValue = other.size * other.unit.toBytes(1);
         return Long.compare(thisValue, otherValue);
     }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        return builder.value(toString());
+    }
 }

server/src/main/java/org/elasticsearch/common/unit/TimeValue.java

@@ -24,6 +24,9 @@
 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.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.ToXContentFragment;
+import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.joda.time.Period;
 import org.joda.time.PeriodType;
 import org.joda.time.format.PeriodFormat;
@@ -40,7 +43,7 @@
 import java.util.Set;
 import java.util.concurrent.TimeUnit;
 
-public class TimeValue implements Writeable, Comparable<TimeValue> {
+public class TimeValue implements Writeable, Comparable<TimeValue>, ToXContentFragment {
 
     /** How many nano-seconds in one milli-second */
     public static final long NSEC_PER_MSEC = TimeUnit.NANOSECONDS.convert(1, TimeUnit.MILLISECONDS);
@@ -398,4 +401,9 @@ public int compareTo(TimeValue timeValue) {
         double otherValue = ((double) timeValue.duration) * timeValue.timeUnit.toNanos(1);
         return Double.compare(thisValue, otherValue);
     }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        return builder.value(toString());
+    }
 }

server/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java

@@ -19,22 +19,21 @@
 
 package org.elasticsearch.common.xcontent;
 
-import org.apache.lucene.util.BytesRef;
-import org.elasticsearch.common.lease.Releasable;
 import org.elasticsearch.common.unit.ByteSizeValue;
-import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.common.util.CollectionUtils;
 import org.joda.time.DateTimeZone;
 import org.joda.time.ReadableInstant;
 import org.joda.time.format.DateTimeFormatter;
 import org.joda.time.format.ISODateTimeFormat;
 
 import java.io.ByteArrayOutputStream;
+import java.io.Closeable;
 import java.io.Flushable;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.nio.file.Path;
+import java.time.ZonedDateTime;
 import java.util.Arrays;
 import java.util.Calendar;
 import java.util.Collections;
@@ -49,7 +48,7 @@
 /**
  * A utility to build XContent (ie json).
  */
-public final class XContentBuilder implements Releasable, Flushable {
+public final class XContentBuilder implements Closeable, Flushable {
 
     /**
      * Create a new {@link XContentBuilder} using the given {@link XContent} content.
@@ -91,7 +90,6 @@ public static XContentBuilder builder(XContent xContent, Set<String> includes, S
         writers.put(Boolean.class, (b, v) -> b.value((Boolean) v));
         writers.put(Byte.class, (b, v) -> b.value((Byte) v));
         writers.put(byte[].class, (b, v) -> b.value((byte[]) v));
-        writers.put(BytesRef.class, (b, v) -> b.binaryValue((BytesRef) v));
         writers.put(Date.class, (b, v) -> b.value((Date) v));
         writers.put(Double.class, (b, v) -> b.value((Double) v));
         writers.put(double[].class, (b, v) -> b.values((double[]) v));
@@ -105,12 +103,12 @@ public static XContentBuilder builder(XContent xContent, Set<String> includes, S
         writers.put(short[].class, (b, v) -> b.values((short[]) v));
         writers.put(String.class, (b, v) -> b.value((String) v));
         writers.put(String[].class, (b, v) -> b.values((String[]) v));
+        writers.put(Locale.class, (b, v) -> b.value(v.toString()));
+        writers.put(Class.class, (b, v) -> b.value(v.toString()));
+        writers.put(ZonedDateTime.class, (b, v) -> b.value(v.toString()));
 
 
         Map<Class<?>, HumanReadableTransformer> humanReadableTransformer = new HashMap<>();
-        // These will be moved to a different class at a later time to decouple them from XContentBuilder
-        humanReadableTransformer.put(TimeValue.class, v -> ((TimeValue) v).millis());
-        humanReadableTransformer.put(ByteSizeValue.class, v -> ((ByteSizeValue) v).getBytes());
 
         // Load pluggable extensions
         for (XContentBuilderExtension service : ServiceLoader.load(XContentBuilderExtension.class)) {
@@ -613,49 +611,25 @@ public XContentBuilder value(byte[] value, int offset, int length) throws IOExce
     }
 
     /**
-     * Writes the binary content of the given {@link BytesRef}.
-     *
-     * Use {@link org.elasticsearch.common.xcontent.XContentParser#binaryValue()} to read the value back
-     */
-    public XContentBuilder field(String name, BytesRef value) throws IOException {
-        return field(name).binaryValue(value);
-    }
-
-    /**
-     * Writes the binary content of the given {@link BytesRef} as UTF-8 bytes.
+     * Writes the binary content of the given byte array as UTF-8 bytes.
      *
      * Use {@link XContentParser#charBuffer()} to read the value back
      */
-    public XContentBuilder utf8Field(String name, BytesRef value) throws IOException {
-        return field(name).utf8Value(value);
-    }
-
-    /**
-     * Writes the binary content of the given {@link BytesRef}.
-     *
-     * Use {@link org.elasticsearch.common.xcontent.XContentParser#binaryValue()} to read the value back
-     */
-    public XContentBuilder binaryValue(BytesRef value) throws IOException {
-        if (value == null) {
-            return nullValue();
-        }
-        value(value.bytes, value.offset, value.length);
-        return this;
+    public XContentBuilder utf8Field(String name, byte[] bytes, int offset, int length) throws IOException {
+        return field(name).utf8Value(bytes, offset, length);
     }
 
     /**
-     * Writes the binary content of the given {@link BytesRef} as UTF-8 bytes.
+     * Writes the binary content of the given byte array as UTF-8 bytes.
      *
      * Use {@link XContentParser#charBuffer()} to read the value back
      */
-    public XContentBuilder utf8Value(BytesRef value) throws IOException {
-        if (value == null) {
-            return nullValue();
-        }
-        generator.writeUTF8String(value.bytes, value.offset, value.length);
+    public XContentBuilder utf8Value(byte[] bytes, int offset, int length) throws IOException {
+        generator.writeUTF8String(bytes, offset, length);
         return this;
     }
 
+
     ////////////////////////////////////////////////////////////////////////////
     // Date
     //////////////////////////////////
@@ -793,10 +767,11 @@ private void unknownValue(Object value, boolean ensureNoSelfReferences) throws I
             value((ReadableInstant) value);
         } else if (value instanceof ToXContent) {
             value((ToXContent) value);
-        } else {
-            // This is a "value" object (like enum, DistanceUnit, etc) just toString() it
-            // (yes, it can be misleading when toString a Java class, but really, jackson should be used in that case)
+        } else if (value instanceof Enum<?>) {
+            // Write out the Enum toString
             value(Objects.toString(value));
+        } else {
+            throw new IllegalArgumentException("cannot write xcontent for unknown value of type " + value.getClass());
         }
     }
 

server/src/main/java/org/elasticsearch/common/xcontent/XContentElasticsearchExtension.java

@@ -0,0 +1,78 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.common.xcontent;
+
+import org.apache.lucene.util.BytesRef;
+import org.elasticsearch.common.bytes.BytesReference;
+import org.elasticsearch.common.unit.ByteSizeValue;
+import org.elasticsearch.common.unit.TimeValue;
+import org.joda.time.DateTimeZone;
+import org.joda.time.tz.CachedDateTimeZone;
+import org.joda.time.tz.FixedDateTimeZone;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Objects;
+
+/**
+ * SPI extensions for Elasticsearch-specific classes (like the Lucene or Joda
+ * dependency classes) that need to be encoded by {@link XContentBuilder} in a
+ * specific way.
+ */
+public class XContentElasticsearchExtension implements XContentBuilderExtension {
+
+    @Override
+    public Map<Class<?>, XContentBuilder.Writer> getXContentWriters() {
+        Map<Class<?>, XContentBuilder.Writer> writers = new HashMap<>();
+
+        // Fully-qualified here to reduce ambiguity around our (ES') Version class
+        writers.put(org.apache.lucene.util.Version.class, (b, v) -> b.value(Objects.toString(v)));
+        writers.put(DateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
+        writers.put(CachedDateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
+        writers.put(FixedDateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
+
+        writers.put(BytesReference.class, (b, v) -> {
+            if (v == null) {
+                b.nullValue();
+            } else {
+                BytesRef bytes = ((BytesReference) v).toBytesRef();
+                b.value(bytes.bytes, bytes.offset, bytes.length);
+            }
+        });
+
+        writers.put(BytesRef.class, (b, v) -> {
+            if (v == null) {
+                b.nullValue();
+            } else {
+                BytesRef bytes = (BytesRef) v;
+                b.value(bytes.bytes, bytes.offset, bytes.length);
+            }
+        });
+        return writers;
+    }
+
+    @Override
+    public Map<Class<?>, XContentBuilder.HumanReadableTransformer> getXContentHumanReadableTransformers() {
+        Map<Class<?>, XContentBuilder.HumanReadableTransformer> transformers = new HashMap<>();
+        transformers.put(TimeValue.class, v -> ((TimeValue) v).millis());
+        transformers.put(ByteSizeValue.class, v -> ((ByteSizeValue) v).getBytes());
+        return transformers;
+    }
+}