Skip to content

Commit c02cd3e

Browse files
spinscalespinscale
authored
Fix java time epoch date formatters (#37829)
The self written epoch date formatters were not properly able to format an Instant to a string due to a misconfiguration. This fix also removes a until now existing runtime behaviour under java 8 regarding the names of the aggregation buckets, which are now the same as before and have been under java 11.
1 parent 859e2f5 commit c02cd3e

File tree

3 files changed

+40
-59
lines changed

3 files changed

+40
-59
lines changed

‎server/src/main/java/org/elasticsearch/common/time/EpochTime.java

Lines changed: 14 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@
1919

2020
package org.elasticsearch.common.time;
2121

22-
import org.elasticsearch.bootstrap.JavaVersion;
23-
2422
import java.time.format.DateTimeFormatter;
2523
import java.time.format.DateTimeFormatterBuilder;
2624
import java.time.format.ResolverStyle;
@@ -72,7 +70,7 @@ public TemporalAccessor resolve(Map<TemporalField,Long> fieldValues,
7270
private static final EpochField NANOS_OF_SECOND = new EpochField(ChronoUnit.NANOS, ChronoUnit.SECONDS, ValueRange.of(0, 999_999_999)) {
7371
@Override
7472
public boolean isSupportedBy(TemporalAccessor temporal) {
75-
return temporal.isSupported(ChronoField.NANO_OF_SECOND) && temporal.getLong(ChronoField.NANO_OF_SECOND) != 0;
73+
return temporal.isSupported(ChronoField.NANO_OF_SECOND);
7674
}
7775
@Override
7876
public long getFrom(TemporalAccessor temporal) {
@@ -117,32 +115,30 @@ public boolean isSupportedBy(TemporalAccessor temporal) {
117115
}
118116
@Override
119117
public long getFrom(TemporalAccessor temporal) {
120-
return temporal.getLong(ChronoField.NANO_OF_SECOND);
118+
return temporal.getLong(ChronoField.NANO_OF_SECOND) % 1_000_000;
121119
}
122120
};
123121

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

129130
// this supports seconds ending in dot
130131
private static final DateTimeFormatter SECONDS_FORMATTER2 = new DateTimeFormatterBuilder()
131-
.append(SECONDS_FORMATTER1)
132+
.appendValue(SECONDS, 1, 19, SignStyle.NORMAL)
132133
.appendLiteral('.')
133134
.toFormatter(Locale.ROOT);
134135

135-
// this supports seconds with a fraction and is also used for printing
136-
private static final DateTimeFormatter SECONDS_FORMATTER3 = new DateTimeFormatterBuilder()
137-
.append(SECONDS_FORMATTER1)
138-
.optionalStart() // optional is used so isSupported will be called when printing
139-
.appendFraction(NANOS_OF_SECOND, 1, 9, true)
140-
.optionalEnd()
141-
.toFormatter(Locale.ROOT);
142-
143136
// this supports milliseconds without any fraction
144137
private static final DateTimeFormatter MILLISECONDS_FORMATTER1 = new DateTimeFormatterBuilder()
145138
.appendValue(MILLIS, 1, 19, SignStyle.NORMAL)
139+
.optionalStart()
140+
.appendFraction(NANOS_OF_MILLI, 0, 6, true)
141+
.optionalEnd()
146142
.toFormatter(Locale.ROOT);
147143

148144
// this supports milliseconds ending in dot
@@ -151,32 +147,13 @@ public long getFrom(TemporalAccessor temporal) {
151147
.appendLiteral('.')
152148
.toFormatter(Locale.ROOT);
153149

154-
// this supports milliseconds with a fraction and is also used for printing
155-
private static final DateTimeFormatter MILLISECONDS_FORMATTER3 = new DateTimeFormatterBuilder()
156-
.append(MILLISECONDS_FORMATTER1)
157-
.optionalStart() // optional is used so isSupported will be called when printing
158-
.appendFraction(NANOS_OF_MILLI, 1, 6, true)
159-
.optionalEnd()
160-
.toFormatter(Locale.ROOT);
161-
162-
static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER3,
150+
static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter("epoch_second", SECONDS_FORMATTER1,
163151
builder -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
164-
SECONDS_FORMATTER1, SECONDS_FORMATTER2, SECONDS_FORMATTER3);
165-
166-
static final DateFormatter MILLIS_FORMATTER = getEpochMillisFormatter();
152+
SECONDS_FORMATTER1, SECONDS_FORMATTER2);
167153

168-
private static DateFormatter getEpochMillisFormatter() {
169-
// the third formatter fails under java 8 as a printer, so fall back to this one
170-
final DateTimeFormatter printer;
171-
if (JavaVersion.current().getVersion().get(0) == 8) {
172-
printer = MILLISECONDS_FORMATTER1;
173-
} else {
174-
printer = MILLISECONDS_FORMATTER3;
175-
}
176-
return new JavaDateFormatter("epoch_millis", printer,
177-
builder -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
178-
MILLISECONDS_FORMATTER1, MILLISECONDS_FORMATTER2, MILLISECONDS_FORMATTER3);
179-
}
154+
static final DateFormatter MILLIS_FORMATTER = new JavaDateFormatter("epoch_millis", MILLISECONDS_FORMATTER1,
155+
builder -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
156+
MILLISECONDS_FORMATTER1, MILLISECONDS_FORMATTER2);
180157

181158
private abstract static class EpochField implements TemporalField {
182159

‎server/src/test/java/org/elasticsearch/common/time/DateFormattersTests.java

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,26 @@ public void testSupportBackwardsJava8Format() {
147147
assertThat(formatter, instanceOf(JavaDateFormatter.class));
148148
}
149149

150+
public void testEpochFormatting() {
151+
long seconds = randomLongBetween(0, 130L * 365 * 86400); // from 1970 epoch till around 2100
152+
long nanos = randomLongBetween(0, 999_999_999L);
153+
Instant instant = Instant.ofEpochSecond(seconds, nanos);
154+
155+
DateFormatter millisFormatter = DateFormatter.forPattern("epoch_millis");
156+
String millis = millisFormatter.format(instant);
157+
Instant millisInstant = Instant.from(millisFormatter.parse(millis));
158+
assertThat(millisInstant.toEpochMilli(), is(instant.toEpochMilli()));
159+
assertThat(millisFormatter.format(Instant.ofEpochSecond(42, 0)), is("42000"));
160+
assertThat(millisFormatter.format(Instant.ofEpochSecond(42, 123456789L)), is("42123.456789"));
161+
162+
DateFormatter secondsFormatter = DateFormatter.forPattern("epoch_second");
163+
String formattedSeconds = secondsFormatter.format(instant);
164+
Instant secondsInstant = Instant.from(secondsFormatter.parse(formattedSeconds));
165+
assertThat(secondsInstant.getEpochSecond(), is(instant.getEpochSecond()));
166+
167+
assertThat(secondsFormatter.format(Instant.ofEpochSecond(42, 0)), is("42"));
168+
}
169+
150170
public void testParsingStrictNanoDates() {
151171
DateFormatter formatter = DateFormatters.forPattern("strict_date_optional_time_nanos");
152172
formatter.format(formatter.parse("2016-01-01T00:00:00.000"));

‎server/src/test/java/org/elasticsearch/search/aggregations/bucket/DateRangeIT.java

Lines changed: 6 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@
2222
import org.elasticsearch.action.index.IndexRequestBuilder;
2323
import org.elasticsearch.action.search.SearchPhaseExecutionException;
2424
import org.elasticsearch.action.search.SearchResponse;
25-
import org.elasticsearch.bootstrap.JavaVersion;
2625
import org.elasticsearch.common.settings.Settings;
2726
import org.elasticsearch.plugins.Plugin;
2827
import org.elasticsearch.script.Script;
@@ -996,39 +995,24 @@ public void testRangeWithFormatNumericValue() throws Exception {
996995
.addAggregation(dateRange("date_range").field("date").addRange(1000, 3000).addRange(3000, 4000)).get();
997996
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(3L));
998997
List<Bucket> buckets = checkBuckets(searchResponse.getAggregations().get("date_range"), "date_range", 2);
999-
if (JavaVersion.current().getVersion().get(0) == 8) {
1000-
assertBucket(buckets.get(0), 2L, "1000.0-3000.0", 1000000L, 3000000L);
1001-
assertBucket(buckets.get(1), 1L, "3000.0-4000.0", 3000000L, 4000000L);
1002-
} else {
1003-
assertBucket(buckets.get(0), 2L, "1000-3000", 1000000L, 3000000L);
1004-
assertBucket(buckets.get(1), 1L, "3000-4000", 3000000L, 4000000L);
1005-
}
998+
assertBucket(buckets.get(0), 2L, "1000-3000", 1000000L, 3000000L);
999+
assertBucket(buckets.get(1), 1L, "3000-4000", 3000000L, 4000000L);
10061000

10071001
// using no format should also work when and to/from are string values
10081002
searchResponse = client().prepareSearch(indexName).setSize(0)
10091003
.addAggregation(dateRange("date_range").field("date").addRange("1000", "3000").addRange("3000", "4000")).get();
10101004
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(3L));
10111005
buckets = checkBuckets(searchResponse.getAggregations().get("date_range"), "date_range", 2);
1012-
if (JavaVersion.current().getVersion().get(0) == 8) {
1013-
assertBucket(buckets.get(0), 2L, "1000.0-3000.0", 1000000L, 3000000L);
1014-
assertBucket(buckets.get(1), 1L, "3000.0-4000.0", 3000000L, 4000000L);
1015-
} else {
1016-
assertBucket(buckets.get(0), 2L, "1000-3000", 1000000L, 3000000L);
1017-
assertBucket(buckets.get(1), 1L, "3000-4000", 3000000L, 4000000L);
1018-
}
1006+
assertBucket(buckets.get(0), 2L, "1000-3000", 1000000L, 3000000L);
1007+
assertBucket(buckets.get(1), 1L, "3000-4000", 3000000L, 4000000L);
10191008

10201009
// also e-notation should work, fractional parts should be truncated
10211010
searchResponse = client().prepareSearch(indexName).setSize(0)
10221011
.addAggregation(dateRange("date_range").field("date").addRange(1.0e3, 3000.8123).addRange(3000.8123, 4.0e3)).get();
10231012
assertThat(searchResponse.getHits().getTotalHits().value, equalTo(3L));
10241013
buckets = checkBuckets(searchResponse.getAggregations().get("date_range"), "date_range", 2);
1025-
if (JavaVersion.current().getVersion().get(0) == 8) {
1026-
assertBucket(buckets.get(0), 2L, "1000.0-3000.0", 1000000L, 3000000L);
1027-
assertBucket(buckets.get(1), 1L, "3000.0-4000.0", 3000000L, 4000000L);
1028-
} else {
1029-
assertBucket(buckets.get(0), 2L, "1000-3000", 1000000L, 3000000L);
1030-
assertBucket(buckets.get(1), 1L, "3000-4000", 3000000L, 4000000L);
1031-
}
1014+
assertBucket(buckets.get(0), 2L, "1000-3000", 1000000L, 3000000L);
1015+
assertBucket(buckets.get(1), 1L, "3000-4000", 3000000L, 4000000L);
10321016

10331017
// using different format should work when to/from is compatible with
10341018
// format in aggregation

0 commit comments

Comments
 (0)