Simplify XContent output of epoch times (elastic#114491) (elastic#1…

…14736)

Today the overloads of `XContentBuilder#timeField` do two rather
different things: one formats an object as a `String` representation of
a time (where the object is either an unambiguous time object or else a
`long`) and the other formats only a `long` as one or two fields
depending on the `?human` flag.

This is trappy in a number of ways:

- `long` means an absolute (epoch) time, but sometimes folks will
  mistakenly use this for time intervals too.

- `long` means only milliseconds, there is no facility to specify a
  different unit.

- the dependence on the `?human` flag in exactly one of the overloads is
  kinda weird.

This commit removes the confusion by dropping support for considering a
`Long` as a valid representation of a time at all, and instead requiring
callers to either convert it into a proper time object or else call a
method that is explicitly expecting an epoch time in milliseconds.

libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilder.java

@@ -40,6 +40,7 @@
 import java.util.ServiceLoader;
 import java.util.Set;
 import java.util.function.Function;
+import java.util.function.LongFunction;
 
 /**
  * A utility to build XContent (ie json).
@@ -107,13 +108,15 @@ public static XContentBuilder builder(XContentType xContentType, Set<String> inc
     private static final Map<Class<?>, Writer> WRITERS;
     private static final Map<Class<?>, HumanReadableTransformer> HUMAN_READABLE_TRANSFORMERS;
     private static final Map<Class<?>, Function<Object, Object>> DATE_TRANSFORMERS;
+    private static final LongFunction<String> UNIX_EPOCH_MILLIS_FORMATTER;
+
     static {
         Map<Class<?>, Writer> writers = new HashMap<>();
         writers.put(Boolean.class, (b, v) -> b.value((Boolean) v));
         writers.put(boolean[].class, (b, v) -> b.values((boolean[]) v));
         writers.put(Byte.class, (b, v) -> b.value((Byte) v));
         writers.put(byte[].class, (b, v) -> b.value((byte[]) v));
-        writers.put(Date.class, XContentBuilder::timeValue);
+        writers.put(Date.class, XContentBuilder::timestampValue);
         writers.put(Double.class, (b, v) -> b.value((Double) v));
         writers.put(double[].class, (b, v) -> b.values((double[]) v));
         writers.put(Float.class, (b, v) -> b.value((Float) v));
@@ -129,8 +132,8 @@ public static XContentBuilder builder(XContentType xContentType, Set<String> inc
         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()));
-        writers.put(Calendar.class, XContentBuilder::timeValue);
-        writers.put(GregorianCalendar.class, XContentBuilder::timeValue);
+        writers.put(Calendar.class, XContentBuilder::timestampValue);
+        writers.put(GregorianCalendar.class, XContentBuilder::timestampValue);
         writers.put(BigInteger.class, (b, v) -> b.value((BigInteger) v));
         writers.put(BigDecimal.class, (b, v) -> b.value((BigDecimal) v));
 
@@ -140,6 +143,8 @@ public static XContentBuilder builder(XContentType xContentType, Set<String> inc
         // treat strings as already converted
         dateTransformers.put(String.class, Function.identity());
 
+        LongFunction<String> unixEpochMillisFormatter = Long::toString;
+
         // Load pluggable extensions
         for (XContentBuilderExtension service : ServiceLoader.load(XContentBuilderExtension.class)) {
             Map<Class<?>, Writer> addlWriters = service.getXContentWriters();
@@ -157,11 +162,14 @@ public static XContentBuilder builder(XContentType xContentType, Set<String> inc
             writers.putAll(addlWriters);
             humanReadableTransformer.putAll(addlTransformers);
             dateTransformers.putAll(addlDateTransformers);
+
+            unixEpochMillisFormatter = service::formatUnixEpochMillis;
         }
 
         WRITERS = Map.copyOf(writers);
         HUMAN_READABLE_TRANSFORMERS = Map.copyOf(humanReadableTransformer);
         DATE_TRANSFORMERS = Map.copyOf(dateTransformers);
+        UNIX_EPOCH_MILLIS_FORMATTER = unixEpochMillisFormatter;
     }
 
     @FunctionalInterface
@@ -797,52 +805,53 @@ public XContentBuilder utf8Value(byte[] bytes, int offset, int length) throws IO
     }
 
     ////////////////////////////////////////////////////////////////////////////
-    // Date
+    // Timestamps
     //////////////////////////////////
 
     /**
-     * Write a time-based field and value, if the passed timeValue is null a
-     * null value is written, otherwise a date transformers lookup is performed.
-
-     * @throws IllegalArgumentException if there is no transformers for the type of object
+     * Write a field with a timestamp value: if the passed timestamp is null then writes null, otherwise looks up the date transformer
+     * for the type of {@code timestamp} and uses it to format the value.
+     *
+     * @throws IllegalArgumentException if there is no transformer for the given value type
      */
-    public XContentBuilder timeField(String name, Object timeValue) throws IOException {
-        return field(name).timeValue(timeValue);
+    public XContentBuilder timestampField(String name, Object timestamp) throws IOException {
+        return field(name).timestampValue(timestamp);
     }
 
     /**
-     * If the {@code humanReadable} flag is set, writes both a formatted and
-     * unformatted version of the time value using the date transformer for the
-     * {@link Long} class.
+     * Writes a field containing the raw number of milliseconds since the unix epoch, and also if the {@code humanReadable} flag is set,
+     * writes a formatted representation of this value using the UNIX_EPOCH_MILLIS_FORMATTER.
      */
-    public XContentBuilder timeField(String name, String readableName, long value) throws IOException {
-        assert name.equals(readableName) == false : "expected raw and readable field names to differ, but they were both: " + name;
+    public XContentBuilder timestampFieldsFromUnixEpochMillis(String rawFieldName, String humanReadableFieldName, long unixEpochMillis)
+        throws IOException {
+        assert rawFieldName.equals(humanReadableFieldName) == false
+            : "expected raw and readable field names to differ, but they were both: " + rawFieldName;
         if (humanReadable) {
-            Function<Object, Object> longTransformer = DATE_TRANSFORMERS.get(Long.class);
-            if (longTransformer == null) {
-                throw new IllegalArgumentException("cannot write time value xcontent for unknown value of type Long");
-            }
-            field(readableName).value(longTransformer.apply(value));
+            field(humanReadableFieldName, UNIX_EPOCH_MILLIS_FORMATTER.apply(unixEpochMillis));
         }
-        field(name, value);
+        field(rawFieldName, unixEpochMillis);
         return this;
     }
 
     /**
-     * Write a time-based value, if the value is null a null value is written,
-     * otherwise a date transformers lookup is performed.
-
-     * @throws IllegalArgumentException if there is no transformers for the type of object
+     * Write a timestamp value: if the passed timestamp is null then writes null, otherwise looks up the date transformer for the type of
+     * {@code timestamp} and uses it to format the value.
+     *
+     * @throws IllegalArgumentException if there is no transformer for the given value type
      */
-    public XContentBuilder timeValue(Object timeValue) throws IOException {
-        if (timeValue == null) {
+    public XContentBuilder timestampValue(Object timestamp) throws IOException {
+        if (timestamp == null) {
             return nullValue();
         } else {
-            Function<Object, Object> transformer = DATE_TRANSFORMERS.get(timeValue.getClass());
+            Function<Object, Object> transformer = DATE_TRANSFORMERS.get(timestamp.getClass());
             if (transformer == null) {
-                throw new IllegalArgumentException("cannot write time value xcontent for unknown value of type " + timeValue.getClass());
+                final var exception = new IllegalArgumentException(
+                    "cannot write timestamp value xcontent for value of unknown type " + timestamp.getClass()
+                );
+                assert false : exception;
+                throw exception;
             }
-            return value(transformer.apply(timeValue));
+            return value(transformer.apply(timestamp));
         }
     }
 

libs/x-content/src/main/java/org/elasticsearch/xcontent/XContentBuilderExtension.java

@@ -68,4 +68,9 @@ public interface XContentBuilderExtension {
      * </pre>
      */
     Map<Class<?>, Function<Object, Object>> getDateTransformers();
+
+    /**
+     * Used to format a {@code long} representing the number of milliseconds since the Unix Epoch.
+     */
+    String formatUnixEpochMillis(long unixEpochMillis);
 }

modules/aggregations/src/internalClusterTest/java/org/elasticsearch/aggregations/pipeline/DateDerivativeIT.java

@@ -65,17 +65,17 @@ protected Collection<Class<? extends Plugin>> nodePlugins() {
     }
 
     private static IndexRequestBuilder indexDoc(String idx, ZonedDateTime date, int value) throws Exception {
-        return prepareIndex(idx).setSource(jsonBuilder().startObject().timeField("date", date).field("value", value).endObject());
+        return prepareIndex(idx).setSource(jsonBuilder().startObject().timestampField("date", date).field("value", value).endObject());
     }
 
     private IndexRequestBuilder indexDoc(int month, int day, int value) throws Exception {
         return prepareIndex("idx").setSource(
             jsonBuilder().startObject()
                 .field("value", value)
-                .timeField("date", date(month, day))
+                .timestampField("date", date(month, day))
                 .startArray("dates")
-                .timeValue(date(month, day))
-                .timeValue(date(month + 1, day + 1))
+                .timestampValue(date(month, day))
+                .timestampValue(date(month + 1, day + 1))
                 .endArray()
                 .endObject()
         );

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/DatabaseConfigurationMetadata.java

@@ -66,7 +66,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
         // (we'll be a in a json map where the id is the key)
         builder.startObject();
         builder.field(VERSION.getPreferredName(), version);
-        builder.timeField(MODIFIED_DATE_MILLIS.getPreferredName(), MODIFIED_DATE.getPreferredName(), modifiedDate);
+        builder.timestampFieldsFromUnixEpochMillis(MODIFIED_DATE_MILLIS.getPreferredName(), MODIFIED_DATE.getPreferredName(), modifiedDate);
         builder.field(DATABASE.getPreferredName(), database);
         builder.endObject();
         return builder;

modules/ingest-geoip/src/main/java/org/elasticsearch/ingest/geoip/direct/GetDatabaseConfigurationAction.java

@@ -110,7 +110,11 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
                 builder.startObject();
                 builder.field("id", database.id()); // serialize including the id -- this is get response serialization
                 builder.field(VERSION.getPreferredName(), item.version());
-                builder.timeField(MODIFIED_DATE_MILLIS.getPreferredName(), MODIFIED_DATE.getPreferredName(), item.modifiedDate());
+                builder.timestampFieldsFromUnixEpochMillis(
+                    MODIFIED_DATE_MILLIS.getPreferredName(),
+                    MODIFIED_DATE.getPreferredName(),
+                    item.modifiedDate()
+                );
                 builder.field(DATABASE.getPreferredName(), database);
                 builder.endObject();
             }

server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/DateHistogramIT.java

@@ -88,11 +88,11 @@ private static String format(ZonedDateTime date, String pattern) {
     private IndexRequestBuilder indexDoc(String idx, ZonedDateTime date, int value) throws Exception {
         return prepareIndex(idx).setSource(
             jsonBuilder().startObject()
-                .timeField("date", date)
+                .timestampField("date", date)
                 .field("value", value)
                 .startArray("dates")
-                .timeValue(date)
-                .timeValue(date.plusMonths(1).plusDays(1))
+                .timestampValue(date)
+                .timestampValue(date.plusMonths(1).plusDays(1))
                 .endArray()
                 .endObject()
         );
@@ -103,10 +103,10 @@ private IndexRequestBuilder indexDoc(int month, int day, int value) throws Excep
             jsonBuilder().startObject()
                 .field("value", value)
                 .field("constant", 1)
-                .timeField("date", date(month, day))
+                .timestampField("date", date(month, day))
                 .startArray("dates")
-                .timeValue(date(month, day))
-                .timeValue(date(month + 1, day + 1))
+                .timestampValue(date(month, day))
+                .timestampValue(date(month + 1, day + 1))
                 .endArray()
                 .endObject()
         );
@@ -162,53 +162,53 @@ private void getMultiSortDocs(List<IndexRequestBuilder> builders) throws IOExcep
         for (int i = 1; i <= 3; i++) {
             builders.add(
                 prepareIndex("sort_idx").setSource(
-                    jsonBuilder().startObject().timeField("date", date(1, 1)).field("l", 1).field("d", i).endObject()
+                    jsonBuilder().startObject().timestampField("date", date(1, 1)).field("l", 1).field("d", i).endObject()
                 )
             );
             builders.add(
                 prepareIndex("sort_idx").setSource(
-                    jsonBuilder().startObject().timeField("date", date(1, 2)).field("l", 2).field("d", i).endObject()
+                    jsonBuilder().startObject().timestampField("date", date(1, 2)).field("l", 2).field("d", i).endObject()
                 )
             );
         }
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 3)).field("l", 3).field("d", 1).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 3)).field("l", 3).field("d", 1).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 3).plusHours(1)).field("l", 3).field("d", 2).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 3).plusHours(1)).field("l", 3).field("d", 2).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 4)).field("l", 3).field("d", 1).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 4)).field("l", 3).field("d", 1).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 4).plusHours(2)).field("l", 3).field("d", 3).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 4).plusHours(2)).field("l", 3).field("d", 3).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 5)).field("l", 5).field("d", 1).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 5)).field("l", 5).field("d", 1).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 5).plusHours(12)).field("l", 5).field("d", 2).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 5).plusHours(12)).field("l", 5).field("d", 2).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 6)).field("l", 5).field("d", 1).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 6)).field("l", 5).field("d", 1).endObject()
             )
         );
         builders.add(
             prepareIndex("sort_idx").setSource(
-                jsonBuilder().startObject().timeField("date", date(1, 7)).field("l", 5).field("d", 1).endObject()
+                jsonBuilder().startObject().timestampField("date", date(1, 7)).field("l", 5).field("d", 1).endObject()
             )
         );
     }
@@ -997,7 +997,7 @@ public void testSingleValueWithTimeZone() throws Exception {
         IndexRequestBuilder[] reqs = new IndexRequestBuilder[5];
         ZonedDateTime date = date("2014-03-11T00:00:00+00:00");
         for (int i = 0; i < reqs.length; i++) {
-            reqs[i] = prepareIndex("idx2").setId("" + i).setSource(jsonBuilder().startObject().timeField("date", date).endObject());
+            reqs[i] = prepareIndex("idx2").setId("" + i).setSource(jsonBuilder().startObject().timestampField("date", date).endObject());
             date = date.plusHours(1);
         }
         indexRandom(true, reqs);

server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/DateHistogramOffsetIT.java

@@ -63,7 +63,7 @@ private void prepareIndex(ZonedDateTime date, int numHours, int stepSizeHours, i
         IndexRequestBuilder[] reqs = new IndexRequestBuilder[numHours];
         for (int i = idxIdStart; i < idxIdStart + reqs.length; i++) {
             reqs[i - idxIdStart] = prepareIndex("idx2").setId("" + i)
-                .setSource(jsonBuilder().startObject().timeField("date", date).endObject());
+                .setSource(jsonBuilder().startObject().timestampField("date", date).endObject());
             date = date.plusHours(stepSizeHours);
         }
         indexRandom(true, reqs);

server/src/internalClusterTest/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java

@@ -58,10 +58,10 @@ private static IndexRequestBuilder indexDoc(int month, int day, int value) throw
         return prepareIndex("idx").setSource(
             jsonBuilder().startObject()
                 .field("value", value)
-                .timeField("date", date(month, day))
+                .timestampField("date", date(month, day))
                 .startArray("dates")
-                .timeValue(date(month, day))
-                .timeValue(date(month + 1, day + 1))
+                .timestampValue(date(month, day))
+                .timestampValue(date(month + 1, day + 1))
                 .endArray()
                 .endObject()
         );
@@ -620,8 +620,8 @@ public void testScriptCaching() throws Exception {
         );
         indexRandom(
             true,
-            prepareIndex("cache_test_idx").setId("1").setSource(jsonBuilder().startObject().timeField("date", date(1, 1)).endObject()),
-            prepareIndex("cache_test_idx").setId("2").setSource(jsonBuilder().startObject().timeField("date", date(2, 1)).endObject())
+            prepareIndex("cache_test_idx").setId("1").setSource(jsonBuilder().startObject().timestampField("date", date(1, 1)).endObject()),
+            prepareIndex("cache_test_idx").setId("2").setSource(jsonBuilder().startObject().timestampField("date", date(2, 1)).endObject())
         );
 
         // Make sure we are starting with a clear cache

server/src/main/java/org/elasticsearch/action/admin/indices/dangling/list/ListDanglingIndicesResponse.java

@@ -79,7 +79,7 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
 
             builder.field("index_name", info.indexName);
             builder.field("index_uuid", info.indexUUID);
-            builder.timeField("creation_date_millis", "creation_date", info.creationDateMillis);
+            builder.timestampFieldsFromUnixEpochMillis("creation_date_millis", "creation_date", info.creationDateMillis);
 
             builder.array("node_ids", info.nodeIds.toArray(new String[0]));
 

server/src/main/java/org/elasticsearch/action/admin/indices/stats/FieldUsageShardResponse.java

@@ -69,7 +69,7 @@ public FieldUsageStats getStats() {
     public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
         builder.startObject();
         builder.field(Fields.TRACKING_ID, trackingId);
-        builder.timeField(Fields.TRACKING_STARTED_AT_MILLIS, Fields.TRACKING_STARTED_AT, trackingStartTime);
+        builder.timestampFieldsFromUnixEpochMillis(Fields.TRACKING_STARTED_AT_MILLIS, Fields.TRACKING_STARTED_AT, trackingStartTime);
         builder.startObject(Fields.ROUTING)
             .field(Fields.STATE, shardRouting.state())
             .field(Fields.PRIMARY, shardRouting.primary())

server/src/main/java/org/elasticsearch/action/bulk/FailureStoreDocumentConverter.java

@@ -18,12 +18,14 @@
 import org.elasticsearch.xcontent.json.JsonXContent;
 
 import java.io.IOException;
+import java.time.Instant;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
 import java.util.Set;
 import java.util.function.Supplier;
 
+import static org.elasticsearch.common.xcontent.XContentElasticsearchExtension.DEFAULT_FORMATTER;
 import static org.elasticsearch.ingest.CompoundProcessor.PIPELINE_ORIGIN_EXCEPTION_HEADER;
 import static org.elasticsearch.ingest.CompoundProcessor.PROCESSOR_TAG_EXCEPTION_HEADER;
 import static org.elasticsearch.ingest.CompoundProcessor.PROCESSOR_TYPE_EXCEPTION_HEADER;
@@ -84,7 +86,7 @@ private static XContentBuilder createSource(IndexRequest source, Exception excep
         XContentBuilder builder = JsonXContent.contentBuilder();
         builder.startObject();
         {
-            builder.timeField("@timestamp", timeSupplier.get());
+            builder.field("@timestamp", DEFAULT_FORMATTER.format(Instant.ofEpochMilli(timeSupplier.get())));
             builder.startObject("document");
             {
                 if (source.id() != null) {