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.
This commit is contained in:
David Turner 2018-02-02 18:24:02 +00:00 committed by GitHub
parent 3ddea8d8d2
commit ab8f5ea54c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 73 additions and 4 deletions

View File

@ -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#<init>(int, int, int, int, int, int, int)
org.joda.time.DateTime#now() org.joda.time.DateTime#now()
org.joda.time.DateTimeZone#getDefault() 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)

View File

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

View File

@ -22,6 +22,8 @@ package org.elasticsearch.ingest;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.junit.Before; import org.junit.Before;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset; import java.time.ZoneOffset;
import java.time.ZonedDateTime; import java.time.ZonedDateTime;
import java.util.ArrayList; import java.util.ArrayList;
@ -986,6 +988,22 @@ public class IngestDocumentTests extends ESTestCase {
assertIngestDocument(ingestDocument, copy); 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 { public void testSetInvalidSourceField() throws Exception {
Map<String, Object> document = new HashMap<>(); Map<String, Object> document = new HashMap<>();
Object randomObject = randomFrom(new ArrayList<>(), new HashMap<>(), 12, 12.34); Object randomObject = randomFrom(new ArrayList<>(), new HashMap<>(), 12, 12.34);