Skip to content

Commit

Permalink
Fix java time epoch date formatters (#37829) (#38163)
Browse files Browse the repository at this point in the history 
The self written epoch date formatters were not properly able to format
an Instant to a string due to a misconfiguration.

This also adds some tests for fractional formatters
  • Loading branch information
@spinscale
spinscale committed Feb 1, 2019
1 parent 9c284e6 commit 10b913a
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 34 deletions.
38 changes: 14 additions & 24 deletions server/src/main/java/org/elasticsearch/common/time/EpochTime.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public TemporalAccessor resolve(Map<TemporalField,Long> fieldValues,
private static final EpochField NANOS_OF_SECOND = new EpochField(ChronoUnit.NANOS, ChronoUnit.SECONDS, ValueRange.of(0, 999_999_999)) {
@Override
public boolean isSupportedBy(TemporalAccessor temporal) {
return temporal.isSupported(ChronoField.NANO_OF_SECOND) && temporal.getLong(ChronoField.NANO_OF_SECOND) != 0;
return temporal.isSupported(ChronoField.NANO_OF_SECOND);
}
@Override
public long getFrom(TemporalAccessor temporal) {
Expand Down Expand Up @@ -111,55 +111,45 @@ public boolean isSupportedBy(TemporalAccessor temporal) {
}
@Override
public long getFrom(TemporalAccessor temporal) {
return temporal.getLong(ChronoField.NANO_OF_SECOND);
return temporal.getLong(ChronoField.NANO_OF_SECOND) % 1_000_000;
}
};

// this supports seconds without any fraction
private static final DateTimeFormatter SECONDS_FORMATTER1 = new DateTimeFormatterBuilder()
.appendValue(SECONDS, 1, 19, SignStyle.NORMAL)
.optionalStart() // optional is used so isSupported will be called when printing
.appendFraction(NANOS_OF_SECOND, 0, 9, true)
.optionalEnd()
.toFormatter(Locale.ROOT);

// this supports seconds ending in dot
private static final DateTimeFormatter SECONDS_FORMATTER2 = new DateTimeFormatterBuilder()
.append(SECONDS_FORMATTER1)
.appendValue(SECONDS, 1, 19, SignStyle.NORMAL)
.appendLiteral('.')
.toFormatter(Locale.ROOT);

// this supports seconds with a fraction and is also used for printing
private static final DateTimeFormatter SECONDS_FORMATTER3 = new DateTimeFormatterBuilder()
.append(SECONDS_FORMATTER1)
.optionalStart() // optional is used so isSupported will be called when printing
.appendFraction(NANOS_OF_SECOND, 1, 9, true)
.optionalEnd()
.toFormatter(Locale.ROOT);

// this supports milliseconds without any fraction
private static final DateTimeFormatter MILLISECONDS_FORMATTER1 = new DateTimeFormatterBuilder()
.appendValue(MILLIS, 1, 19, SignStyle.NORMAL)
.optionalStart()
.appendFraction(NANOS_OF_MILLI, 0, 6, true)
.optionalEnd()
.toFormatter(Locale.ROOT);

// this supports milliseconds ending in dot
private static final DateTimeFormatter MILLISECONDS_FORMATTER2 = new DateTimeFormatterBuilder()
.append(MILLISECONDS_FORMATTER1)
.appendValue(MILLIS, 1, 19, SignStyle.NORMAL)
.appendLiteral('.')
.toFormatter(Locale.ROOT);

// this supports milliseconds with a fraction and is also used for printing
private static final DateTimeFormatter MILLISECONDS_FORMATTER3 = new DateTimeFormatterBuilder()
.append(MILLISECONDS_FORMATTER1)
.optionalStart() // optional is used so isSupported will be called when printing
.appendFraction(NANOS_OF_MILLI, 1, 6, true)
.optionalEnd()
.toFormatter(Locale.ROOT);

static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER3,
static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER1,
builder -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
SECONDS_FORMATTER1, SECONDS_FORMATTER2, SECONDS_FORMATTER3);
SECONDS_FORMATTER1, SECONDS_FORMATTER2);

static final DateFormatter MILLIS_FORMATTER = new JavaDateFormatter("epoch_millis", MILLISECONDS_FORMATTER3,
static final DateFormatter MILLIS_FORMATTER = new JavaDateFormatter("epoch_millis", MILLISECONDS_FORMATTER1,
builder -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
MILLISECONDS_FORMATTER1, MILLISECONDS_FORMATTER2, MILLISECONDS_FORMATTER3);
MILLISECONDS_FORMATTER1, MILLISECONDS_FORMATTER2);

private abstract static class EpochField implements TemporalField {

Expand Down
57 changes: 47 additions & 10 deletions server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.hamcrest.Matchers.sameInstance;
import static org.hamcrest.Matchers.startsWith;

public class DateFormattersTests extends ESTestCase {

Expand All @@ -55,35 +56,50 @@ public void testEpochMillisParser() {
assertThat(instant.getEpochSecond(), is(0L));
assertThat(instant.getNano(), is(0));
}
{
Instant instant = Instant.from(formatter.parse("123.123456"));
assertThat(instant.getEpochSecond(), is(0L));
assertThat(instant.getNano(), is(123123456));
}
}

public void testEpochMilliParser() {
DateFormatter formatter = DateFormatters.forPattern("epoch_millis");
DateFormatter formatter = DateFormatter.forPattern("8epoch_millis");
DateTimeParseException e = expectThrows(DateTimeParseException.class, () -> formatter.parse("invalid"));
assertThat(e.getMessage(), containsString("could not be parsed"));

e = expectThrows(DateTimeParseException.class, () -> formatter.parse("123.1234567"));
assertThat(e.getMessage(), containsString("unparsed text found at index 3"));
assertThat(e.getMessage(), containsString("unparsed text found"));
}

// this is not in the duelling tests, because the epoch second parser in joda time drops the milliseconds after the comma
// but is able to parse the rest
// as this feature is supported it also makes sense to make it exact
public void testEpochSecondParser() {
public void testEpochSecondParserWithFraction() {
DateFormatter formatter = DateFormatters.forPattern("epoch_second");

DateTimeParseException e = expectThrows(DateTimeParseException.class, () -> formatter.parse("1234.1"));
assertThat(e.getMessage(), is("Text '1234.1' could not be parsed, unparsed text found at index 4"));
e = expectThrows(DateTimeParseException.class, () -> formatter.parse("1234."));
assertThat(e.getMessage(), is("Text '1234.' could not be parsed, unparsed text found at index 4"));
e = expectThrows(DateTimeParseException.class, () -> formatter.parse("abc"));
TemporalAccessor accessor = formatter.parse("1234.1");
Instant instant = DateFormatters.toZonedDateTime(accessor).toInstant();
assertThat(instant.getEpochSecond(), is(1234L));
assertThat(DateFormatters.toZonedDateTime(accessor).toInstant().getNano(), is(100_000_000));

accessor = formatter.parse("1234");
instant = DateFormatters.toZonedDateTime(accessor).toInstant();
assertThat(instant.getEpochSecond(), is(1234L));
assertThat(instant.getNano(), is(0));

DateTimeParseException e = expectThrows(DateTimeParseException.class, () -> formatter.parse("abc"));
assertThat(e.getMessage(), is("Text 'abc' could not be parsed, unparsed text found at index 0"));

e = expectThrows(DateTimeParseException.class, () -> formatter.parse("1234.abc"));
assertThat(e.getMessage(), is("Text '1234.abc' could not be parsed, unparsed text found at index 4"));
assertThat(e.getMessage(), is("Text '1234.abc' could not be parsed, unparsed text found at index 5"));

e = expectThrows(DateTimeParseException.class, () -> formatter.parse("1234.1234567890"));
assertThat(e.getMessage(), is("Text '1234.1234567890' could not be parsed, unparsed text found at index 14"));
}

public void testEpochMilliParsersWithDifferentFormatters() {
DateFormatter formatter = DateFormatter.forPattern("strict_date_optional_time||epoch_millis");
DateFormatter formatter = DateFormatter.forPattern("8strict_date_optional_time||epoch_millis");
TemporalAccessor accessor = formatter.parse("123");
assertThat(DateFormatters.toZonedDateTime(accessor).toInstant().toEpochMilli(), is(123L));
assertThat(formatter.pattern(), is("strict_date_optional_time||epoch_millis"));
Expand Down Expand Up @@ -148,6 +164,26 @@ public void testForceJava8() {
assertThat(formatter, instanceOf(JavaDateFormatter.class));
}

public void testEpochFormatting() {
long seconds = randomLongBetween(0, 130L * 365 * 86400); // from 1970 epoch till around 2100
long nanos = randomLongBetween(0, 999_999_999L);
Instant instant = Instant.ofEpochSecond(seconds, nanos);

DateFormatter millisFormatter = DateFormatter.forPattern("8epoch_millis");
String millis = millisFormatter.format(instant);
Instant millisInstant = Instant.from(millisFormatter.parse(millis));
assertThat(millisInstant.toEpochMilli(), is(instant.toEpochMilli()));
assertThat(millisFormatter.format(Instant.ofEpochSecond(42, 0)), is("42000"));
assertThat(millisFormatter.format(Instant.ofEpochSecond(42, 123456789L)), is("42123.456789"));

DateFormatter secondsFormatter = DateFormatter.forPattern("8epoch_second");
String formattedSeconds = secondsFormatter.format(instant);
Instant secondsInstant = Instant.from(secondsFormatter.parse(formattedSeconds));
assertThat(secondsInstant.getEpochSecond(), is(instant.getEpochSecond()));

assertThat(secondsFormatter.format(Instant.ofEpochSecond(42, 0)), is("42"));
}

public void testParsingStrictNanoDates() {
DateFormatter formatter = DateFormatters.forPattern("strict_date_optional_time_nanos");
formatter.format(formatter.parse("2016-01-01T00:00:00.000"));
Expand Down Expand Up @@ -185,6 +221,7 @@ public void testRoundupFormatterWithEpochDates() {
}

private void assertRoundupFormatter(String format, String input, long expectedMilliSeconds) {
assertThat(format, startsWith("8"));
JavaDateFormatter dateFormatter = (JavaDateFormatter) DateFormatter.forPattern(format);
dateFormatter.parse(input);
DateTimeFormatter roundUpFormatter = dateFormatter.getRoundupParser();
Expand Down

0 comments on commit 10b913a

Please sign in to comment.