Forbid trappy methods from java.time (#28476)

ava.time has the functionality needed to deal with timezones with varying 
offsets correctly, but it also has a bunch of methods that silently let you
forget about the hard cases, which raises the risk that we'll quietly do the
wrong thing at some point in the future.

This change adds the trappy methods to the list of forbidden methods to try and
help stop this from happening.

It also fixes the only use of these methods in the codebase so far:
IngestDocument#deepCopy() used ZonedDateTime.of() which may alter the offset of
the given time in cases where the offset is ambiguous.

Author: DaveCTurner

buildSrc/src/main/resources/forbidden/es-server-signatures.txt

@@ -82,3 +82,56 @@ org.joda.time.DateTime#<init>(int, int, int, int, int, int)
 org.joda.time.DateTime#<init>(int, int, int, int, int, int, int)
 org.joda.time.DateTime#now()
 org.joda.time.DateTimeZone#getDefault()
+
+@defaultMessage Local times may be ambiguous or nonexistent in a specific time zones. Use ZoneRules#getValidOffsets() instead.
+java.time.LocalDateTime#atZone(java.time.ZoneId)
+java.time.ZonedDateTime#of(int, int, int, int, int, int, int, java.time.ZoneId)
+java.time.ZonedDateTime#of(java.time.LocalDate, java.time.LocalTime, java.time.ZoneId)
+java.time.ZonedDateTime#of(java.time.LocalDateTime, java.time.ZoneId)
+java.time.ZonedDateTime#truncatedTo(java.time.temporal.TemporalUnit)
+java.time.ZonedDateTime#of(int, int, int, int, int, int, int, java.time.ZoneId)
+java.time.ZonedDateTime#of(java.time.LocalDate, java.time.LocalTime, java.time.ZoneId)
+java.time.ZonedDateTime#of(java.time.LocalDateTime, java.time.ZoneId)
+java.time.ZonedDateTime#ofLocal(java.time.LocalDateTime, java.time.ZoneId, java.time.ZoneOffset)
+java.time.OffsetDateTime#atZoneSimilarLocal(java.time.ZoneId)
+java.time.zone.ZoneRules#getOffset(java.time.LocalDateTime)
+
+@defaultMessage Manipulation of an OffsetDateTime may yield a time that is not valid in the desired time zone. Use ZonedDateTime instead.
+java.time.OffsetDateTime#minus(long, java.time.temporal.TemporalUnit)
+java.time.OffsetDateTime#minus(long, java.time.temporal.TemporalUnit)
+java.time.OffsetDateTime#minus(java.time.temporal.TemporalAmount)
+java.time.OffsetDateTime#minusDays(long)
+java.time.OffsetDateTime#minusHours(long)
+java.time.OffsetDateTime#minusMinutes(long)
+java.time.OffsetDateTime#minusMonths(long)
+java.time.OffsetDateTime#minusNanos(long)
+java.time.OffsetDateTime#minusSeconds(long)
+java.time.OffsetDateTime#minusWeeks(long)
+java.time.OffsetDateTime#minusYears(long)
+java.time.OffsetDateTime#plus(long, java.time.temporal.TemporalUnit)
+java.time.OffsetDateTime#plus(java.time.temporal.TemporalAmount)
+java.time.OffsetDateTime#plusDays(long)
+java.time.OffsetDateTime#plusHours(long)
+java.time.OffsetDateTime#plusMinutes(long)
+java.time.OffsetDateTime#plusMonths(long)
+java.time.OffsetDateTime#plusNanos(long)
+java.time.OffsetDateTime#plusSeconds(long)
+java.time.OffsetDateTime#plusWeeks(long)
+java.time.OffsetDateTime#plusYears(long)
+java.time.OffsetDateTime#with(java.time.temporal.TemporalAdjuster)
+java.time.OffsetDateTime#with(java.time.temporal.TemporalField, long)
+java.time.OffsetDateTime#withDayOfMonth(int)
+java.time.OffsetDateTime#withDayOfYear(int)
+java.time.OffsetDateTime#withHour(int)
+java.time.OffsetDateTime#withMinute(int)
+java.time.OffsetDateTime#withMonth(int)
+java.time.OffsetDateTime#withNano(int)
+java.time.OffsetDateTime#withOffsetSameInstant(java.time.ZoneOffset)
+java.time.OffsetDateTime#withOffsetSameLocal(java.time.ZoneOffset)
+java.time.OffsetDateTime#withSecond(int)
+java.time.OffsetDateTime#withYear(int)
+
+@defaultMessage Daylight saving is not the only reason for a change in timezone offset.
+java.time.zone.ZoneRules#getStandardOffset(java.time.Instant)
+java.time.zone.ZoneRules#getDaylightSavings(java.time.Instant)
+java.time.zone.ZoneRules#isDaylightSavings(java.time.Instant)

server/src/main/java/org/elasticsearch/ingest/IngestDocument.java

@@ -609,13 +609,11 @@ private static Object deepCopy(Object value) {
             return Arrays.copyOf(bytes, bytes.length);
         } else if (value == null || value instanceof String || value instanceof Integer ||
             value instanceof Long || value instanceof Float ||
-            value instanceof Double || value instanceof Boolean) {
+            value instanceof Double || value instanceof Boolean ||
+            value instanceof ZonedDateTime) {
             return value;
         } else if (value instanceof Date) {
             return ((Date) value).clone();
-        } else if (value instanceof ZonedDateTime) {
-            ZonedDateTime zonedDateTime = (ZonedDateTime) value;
-            return ZonedDateTime.of(zonedDateTime.toLocalDate(), zonedDateTime.toLocalTime(), zonedDateTime.getZone());
         } else {
             throw new IllegalArgumentException("unexpected value type [" + value.getClass() + "]");
         }

server/src/test/java/org/elasticsearch/ingest/IngestDocumentTests.java

@@ -22,6 +22,8 @@
 import org.elasticsearch.test.ESTestCase;
 import org.junit.Before;
 
+import java.time.Instant;
+import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
 import java.util.ArrayList;
@@ -986,6 +988,22 @@ public void testCopyConstructor() {
         assertIngestDocument(ingestDocument, copy);
     }
 
+    public void testCopyConstructorWithZonedDateTime() {
+        ZoneId timezone = ZoneId.of("Europe/London");
+
+        Map<String, Object> sourceAndMetadata = new HashMap<>();
+        sourceAndMetadata.put("beforeClockChange", ZonedDateTime.ofInstant(Instant.ofEpochSecond(1509237000), timezone));
+        sourceAndMetadata.put("afterClockChange", ZonedDateTime.ofInstant(Instant.ofEpochSecond(1509240600), timezone));
+
+        IngestDocument original = new IngestDocument(sourceAndMetadata, new HashMap<>());
+        IngestDocument copy = new IngestDocument(original);
+
+        assertThat(copy.getSourceAndMetadata().get("beforeClockChange"),
+            equalTo(original.getSourceAndMetadata().get("beforeClockChange")));
+        assertThat(copy.getSourceAndMetadata().get("afterClockChange"),
+            equalTo(original.getSourceAndMetadata().get("afterClockChange")));
+    }
+
     public void testSetInvalidSourceField() throws Exception {
         Map<String, Object> document = new HashMap<>();
         Object randomObject = randomFrom(new ArrayList<>(), new HashMap<>(), 12, 12.34);