Skip to content

Commit b957386

Browse files
committed
Extend the date rounding logic to be conditional (elastic#89693)
Date rounding logic should take into account the fields that will be parsed be a parser. If a parser has a DayOfYear field, the rounding logic should not try to default DayOfMonth as it will conflict with DayOfYear However the DateTimeFormatter does not have a public method to return information of fields that will be parsed. The hacky workaround is to rely on toString() implementation that will return a field info when it was defined with textual pattern. This commits introduced conditional logic for DayOfYear, ClockHourOfAMPM and HourOfAmPM closes elastic#89096 closes elastic#58986
1 parent 822b0da commit b957386

File tree

4 files changed

+153
-80
lines changed

4 files changed

+153
-80
lines changed

docs/changelog/89693.yaml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
pr: 89693
2+
summary: Extend the date rounding logic to be conditional
3+
area: Infra/Core
4+
type: bug
5+
issues:
6+
- 89096
7+
- 58986

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,15 +149,15 @@ public long getFrom(TemporalAccessor temporal) {
149149
static final DateFormatter SECONDS_FORMATTER = new JavaDateFormatter(
150150
"epoch_second",
151151
SECONDS_FORMATTER1,
152-
builder -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
152+
(builder, parser) -> builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
153153
SECONDS_FORMATTER1,
154154
SECONDS_FORMATTER2
155155
);
156156

157157
static final DateFormatter MILLIS_FORMATTER = new JavaDateFormatter(
158158
"epoch_millis",
159159
MILLISECONDS_FORMATTER1,
160-
builder -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
160+
(builder, parser) -> builder.parseDefaulting(EpochTime.NANOS_OF_MILLI, 999_999L),
161161
MILLISECONDS_FORMATTER1,
162162
MILLISECONDS_FORMATTER2
163163
);

server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java

Lines changed: 112 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -17,46 +17,59 @@
1717
import java.time.format.DateTimeParseException;
1818
import java.time.temporal.ChronoField;
1919
import java.time.temporal.TemporalAccessor;
20-
import java.time.temporal.TemporalField;
2120
import java.util.ArrayList;
22-
import java.util.Arrays;
23-
import java.util.Collection;
2421
import java.util.Collections;
25-
import java.util.HashMap;
2622
import java.util.List;
2723
import java.util.Locale;
28-
import java.util.Map;
2924
import java.util.Objects;
30-
import java.util.function.Consumer;
31-
import java.util.stream.Collectors;
25+
import java.util.function.BiConsumer;
26+
import java.util.function.UnaryOperator;
3227

3328
class JavaDateFormatter implements DateFormatter {
34-
35-
// base fields which should be used for default parsing, when we round up for date math
36-
private static final Map<TemporalField, Long> ROUND_UP_BASE_FIELDS = new HashMap<>(6);
37-
38-
{
39-
ROUND_UP_BASE_FIELDS.put(ChronoField.MONTH_OF_YEAR, 1L);
40-
ROUND_UP_BASE_FIELDS.put(ChronoField.DAY_OF_MONTH, 1L);
41-
ROUND_UP_BASE_FIELDS.put(ChronoField.HOUR_OF_DAY, 23L);
42-
ROUND_UP_BASE_FIELDS.put(ChronoField.MINUTE_OF_HOUR, 59L);
43-
ROUND_UP_BASE_FIELDS.put(ChronoField.SECOND_OF_MINUTE, 59L);
44-
ROUND_UP_BASE_FIELDS.put(ChronoField.NANO_OF_SECOND, 999_999_999L);
45-
}
29+
/**
30+
* A default consumer that allows to round up fields (used for range searches, optional fields missing)
31+
* it relies on toString implementation of DateTimeFormatter and ChronoField.
32+
* For instance for pattern
33+
* the parser would have a toString()
34+
* <code>
35+
* Value(MonthOfYear,2)'/'Value(DayOfMonth,2)'/'Value(YearOfEra,4,19,EXCEEDS_PAD)'
36+
* 'Value(ClockHourOfAmPm,2)':'Value(MinuteOfHour,2)' 'Text(AmPmOfDay,SHORT)
37+
* </code>
38+
* and ChronoField.CLOCK_HOUR_OF_AMPM would have toString() ClockHourOfAmPm
39+
* this allows the rounding logic to default CLOCK_HOUR_OF_AMPM field instead of HOUR_OF_DAY
40+
* without this logic, the rounding would result in a conflict as HOUR_OF_DAY would be missing, but CLOCK_HOUR_OF_AMPM would be provided
41+
*/
42+
private static final BiConsumer<DateTimeFormatterBuilder, DateTimeFormatter> DEFAULT_ROUND_UP = (builder, parser) -> {
43+
String parserAsString = parser.toString();
44+
if (parserAsString.contains(ChronoField.MONTH_OF_YEAR.toString())) {
45+
builder.parseDefaulting(ChronoField.MONTH_OF_YEAR, 1L);
46+
}
47+
if (parserAsString.contains(ChronoField.DAY_OF_MONTH.toString())) {
48+
builder.parseDefaulting(ChronoField.DAY_OF_MONTH, 1L);
49+
}
50+
if (parserAsString.contains(ChronoField.CLOCK_HOUR_OF_AMPM.toString())) {
51+
builder.parseDefaulting(ChronoField.CLOCK_HOUR_OF_AMPM, 11L);
52+
builder.parseDefaulting(ChronoField.AMPM_OF_DAY, 1L);
53+
} else if (parserAsString.contains(ChronoField.HOUR_OF_AMPM.toString())) {
54+
builder.parseDefaulting(ChronoField.HOUR_OF_AMPM, 11L);
55+
builder.parseDefaulting(ChronoField.AMPM_OF_DAY, 1L);
56+
} else {
57+
builder.parseDefaulting(ChronoField.HOUR_OF_DAY, 23L);
58+
}
59+
builder.parseDefaulting(ChronoField.MINUTE_OF_HOUR, 59L);
60+
builder.parseDefaulting(ChronoField.SECOND_OF_MINUTE, 59L);
61+
builder.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L);
62+
};
4663

4764
private final String format;
4865
private final DateTimeFormatter printer;
49-
private final List<DateTimeFormatter> parsers;
50-
private final JavaDateFormatter roundupParser;
66+
private final DateTimeFormatter[] parsers;
67+
private final RoundUpFormatter roundupParser;
5168

52-
static class RoundUpFormatter extends JavaDateFormatter {
69+
private static final class RoundUpFormatter extends JavaDateFormatter {
5370

54-
RoundUpFormatter(String format, List<DateTimeFormatter> roundUpParsers) {
55-
super(format, firstFrom(roundUpParsers), null, roundUpParsers);
56-
}
57-
58-
private static DateTimeFormatter firstFrom(List<DateTimeFormatter> roundUpParsers) {
59-
return roundUpParsers.get(0);
71+
RoundUpFormatter(String format, DateTimeFormatter[] roundUpParsers) {
72+
super(format, roundUpParsers[0], (RoundUpFormatter) null, roundUpParsers);
6073
}
6174

6275
@Override
@@ -67,37 +80,47 @@ JavaDateFormatter getRoundupParser() {
6780

6881
// named formatters use default roundUpParser
6982
JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter... parsers) {
70-
this(format, printer, builder -> ROUND_UP_BASE_FIELDS.forEach(builder::parseDefaulting), parsers);
83+
this(
84+
format,
85+
printer,
86+
// set up base fields which should be used for default parsing, when we round up for date math
87+
DEFAULT_ROUND_UP,
88+
parsers
89+
);
7190
}
7291

7392
// subclasses override roundUpParser
7493
JavaDateFormatter(
7594
String format,
7695
DateTimeFormatter printer,
77-
Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
96+
BiConsumer<DateTimeFormatterBuilder, DateTimeFormatter> roundupParserConsumer,
7897
DateTimeFormatter... parsers
7998
) {
8099
if (printer == null) {
81100
throw new IllegalArgumentException("printer may not be null");
82101
}
83-
long distinctZones = Arrays.stream(parsers).map(DateTimeFormatter::getZone).distinct().count();
84-
if (distinctZones > 1) {
85-
throw new IllegalArgumentException("formatters must have the same time zone");
86-
}
87-
long distinctLocales = Arrays.stream(parsers).map(DateTimeFormatter::getLocale).distinct().count();
88-
if (distinctLocales > 1) {
89-
throw new IllegalArgumentException("formatters must have the same locale");
90-
}
91102
this.printer = printer;
92103
this.format = format;
104+
this.parsers = parsersArray(printer, parsers);
105+
this.roundupParser = createRoundUpParser(format, roundupParserConsumer, locale(), this.parsers);
106+
}
93107

108+
private static DateTimeFormatter[] parsersArray(DateTimeFormatter printer, DateTimeFormatter... parsers) {
94109
if (parsers.length == 0) {
95-
this.parsers = Collections.singletonList(printer);
96-
} else {
97-
this.parsers = Arrays.asList(parsers);
110+
return new DateTimeFormatter[] { printer };
111+
}
112+
final ZoneId zoneId = parsers[0].getZone();
113+
final Locale locale = parsers[0].getLocale();
114+
for (int i = 1; i < parsers.length; i++) {
115+
final DateTimeFormatter parser = parsers[i];
116+
if (Objects.equals(parser.getZone(), zoneId) == false) {
117+
throw new IllegalArgumentException("formatters must have the same time zone");
118+
}
119+
if (Objects.equals(parser.getLocale(), locale) == false) {
120+
throw new IllegalArgumentException("formatters must have the same locale");
121+
}
98122
}
99-
List<DateTimeFormatter> roundUp = createRoundUpParser(format, roundupParserConsumer);
100-
this.roundupParser = new RoundUpFormatter(format, roundUp);
123+
return parsers;
101124
}
102125

103126
/**
@@ -109,16 +132,19 @@ JavaDateFormatter getRoundupParser() {
109132
* <code>DateFormatters</code>.
110133
* This means that we need to also have multiple RoundUp parsers.
111134
*/
112-
private List<DateTimeFormatter> createRoundUpParser(String format, Consumer<DateTimeFormatterBuilder> roundupParserConsumer) {
135+
private static RoundUpFormatter createRoundUpParser(
136+
String format,
137+
BiConsumer<DateTimeFormatterBuilder, DateTimeFormatter> roundupParserConsumer,
138+
Locale locale,
139+
DateTimeFormatter[] parsers
140+
) {
113141
if (format.contains("||") == false) {
114-
List<DateTimeFormatter> roundUpParsers = new ArrayList<>();
115-
for (DateTimeFormatter parser : this.parsers) {
142+
return new RoundUpFormatter(format, mapParsers(parser -> {
116143
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
117144
builder.append(parser);
118-
roundupParserConsumer.accept(builder);
119-
roundUpParsers.add(builder.toFormatter(locale()));
120-
}
121-
return roundUpParsers;
145+
roundupParserConsumer.accept(builder, parser);
146+
return builder.toFormatter(locale);
147+
}, parsers));
122148
}
123149
return null;
124150
}
@@ -136,22 +162,26 @@ public static DateFormatter combined(String input, List<DateFormatter> formatter
136162
if (printer == null) {
137163
printer = javaDateFormatter.getPrinter();
138164
}
139-
parsers.addAll(javaDateFormatter.getParsers());
140-
roundUpParsers.addAll(javaDateFormatter.getRoundupParser().getParsers());
165+
Collections.addAll(parsers, javaDateFormatter.parsers);
166+
Collections.addAll(roundUpParsers, javaDateFormatter.getRoundupParser().parsers);
141167
}
142168

143-
return new JavaDateFormatter(input, printer, roundUpParsers, parsers);
169+
return new JavaDateFormatter(
170+
input,
171+
printer,
172+
roundUpParsers.toArray(new DateTimeFormatter[0]),
173+
parsers.toArray(new DateTimeFormatter[0])
174+
);
144175
}
145176

146-
private JavaDateFormatter(
147-
String format,
148-
DateTimeFormatter printer,
149-
List<DateTimeFormatter> roundUpParsers,
150-
List<DateTimeFormatter> parsers
151-
) {
177+
private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter[] roundUpParsers, DateTimeFormatter[] parsers) {
178+
this(format, printer, new RoundUpFormatter(format, roundUpParsers), parsers);
179+
}
180+
181+
private JavaDateFormatter(String format, DateTimeFormatter printer, RoundUpFormatter roundupParser, DateTimeFormatter[] parsers) {
152182
this.format = format;
153183
this.printer = printer;
154-
this.roundupParser = roundUpParsers != null ? new RoundUpFormatter(format, roundUpParsers) : null;
184+
this.roundupParser = roundupParser;
155185
this.parsers = parsers;
156186
}
157187

@@ -191,7 +221,7 @@ public TemporalAccessor parse(String input) {
191221
* @throws DateTimeParseException when unable to parse with any parsers
192222
*/
193223
private TemporalAccessor doParse(String input) {
194-
if (parsers.size() > 1) {
224+
if (parsers.length > 1) {
195225
for (DateTimeFormatter formatter : parsers) {
196226
ParsePosition pos = new ParsePosition(0);
197227
Object object = formatter.toFormat().parseObject(input, pos);
@@ -201,10 +231,10 @@ private TemporalAccessor doParse(String input) {
201231
}
202232
throw new DateTimeParseException("Failed to parse with all enclosed parsers", input, 0);
203233
}
204-
return this.parsers.get(0).parse(input);
234+
return this.parsers[0].parse(input);
205235
}
206236

207-
private boolean parsingSucceeded(Object object, String input, ParsePosition pos) {
237+
private static boolean parsingSucceeded(Object object, String input, ParsePosition pos) {
208238
return object != null && pos.getIndex() == input.length();
209239
}
210240

@@ -214,12 +244,7 @@ public DateFormatter withZone(ZoneId zoneId) {
214244
if (zoneId.equals(zone())) {
215245
return this;
216246
}
217-
List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withZone(zoneId)).collect(Collectors.toList());
218-
List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers()
219-
.stream()
220-
.map(p -> p.withZone(zoneId))
221-
.collect(Collectors.toList());
222-
return new JavaDateFormatter(format, printer.withZone(zoneId), roundUpParsers, parsers);
247+
return mapParsers(p -> p.withZone(zoneId));
223248
}
224249

225250
@Override
@@ -228,12 +253,24 @@ public DateFormatter withLocale(Locale locale) {
228253
if (locale.equals(locale())) {
229254
return this;
230255
}
231-
List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withLocale(locale)).collect(Collectors.toList());
232-
List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers()
233-
.stream()
234-
.map(p -> p.withLocale(locale))
235-
.collect(Collectors.toList());
236-
return new JavaDateFormatter(format, printer.withLocale(locale), roundUpParsers, parsers);
256+
return mapParsers(p -> p.withLocale(locale));
257+
}
258+
259+
private JavaDateFormatter mapParsers(UnaryOperator<DateTimeFormatter> mapping) {
260+
return new JavaDateFormatter(
261+
format,
262+
mapping.apply(printer),
263+
mapParsers(mapping, ((JavaDateFormatter) this.roundupParser).parsers),
264+
mapParsers(mapping, this.parsers)
265+
);
266+
}
267+
268+
private static DateTimeFormatter[] mapParsers(UnaryOperator<DateTimeFormatter> mapping, DateTimeFormatter[] parsers) {
269+
DateTimeFormatter[] res = new DateTimeFormatter[parsers.length];
270+
for (int i = 0; i < parsers.length; i++) {
271+
res[i] = mapping.apply(parsers[i]);
272+
}
273+
return res;
237274
}
238275

239276
@Override
@@ -283,7 +320,4 @@ public String toString() {
283320
return String.format(Locale.ROOT, "format[%s] locale[%s]", format, locale());
284321
}
285322

286-
Collection<DateTimeFormatter> getParsers() {
287-
return parsers;
288-
}
289323
}

server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,38 @@ public void testOverridingLocaleOrZoneAndCompositeRoundUpParser() {
8686
assertDateEquals(gotMillis, "297276785531", "297276785531");
8787
}
8888

89+
public void testWeekBasedDate() {
90+
DateFormatter formatter = DateFormatter.forPattern("strict_basic_week_date");// YYYY'W'wwe
91+
// first week of 2022 is starting on Monday 3rd Jan
92+
assertDateMathEquals(formatter.toDateMathParser(), "2022W0101", "2022-01-03T23:59:59.999Z", 0, true, ZoneOffset.UTC);
93+
94+
// defaulting missing day of week
95+
formatter = DateFormatter.forPattern("YYYY'W'ww[e]");// YYYY'W'wwe
96+
// second week of 2022 is starting on Monday 10th Jan
97+
assertDateMathEquals(formatter.toDateMathParser(), "2022W02", "2022-01-10T23:59:59.999Z", 0, true, ZoneOffset.UTC);
98+
}
99+
100+
public void testDayOfYear() {
101+
DateFormatter formatter = DateFormatter.forPattern("yyyy-DDD'T'HH:mm:ss.SSS");
102+
assertDateMathEquals(formatter.toDateMathParser(), "2022-104T14:08:30.293", "2022-04-14T14:08:30.293", 0, true, ZoneOffset.UTC);
103+
}
104+
105+
public void testAMPM() {
106+
DateFormatter formatter = DateFormatter.forPattern("MM/dd/yyyy hh:mm a"); // h clock-hour-of-am-pm (1-12)
107+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020 12:48 AM", "2020-04-30T00:48:59.999Z", 0, true, ZoneOffset.UTC);
108+
109+
formatter = DateFormatter.forPattern("MM/dd/yyyy KK:mm a"); // K hour-of-am-pm (0-11)
110+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020 00:48 AM", "2020-04-30T00:48:59.999Z", 0, true, ZoneOffset.UTC);
111+
}
112+
113+
public void testAMPMWithTimeMissing() {
114+
DateFormatter formatter = DateFormatter.forPattern("MM/dd/yyyy[ hh:mm a]"); // h clock-hour-of-am-pm (1-12)
115+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020", "2020-04-30T23:59:59.999Z", 0, true, ZoneOffset.UTC);
116+
117+
formatter = DateFormatter.forPattern("MM/dd/yyyy[ KK:mm a]"); // K hour-of-am-pm (0-11)
118+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020", "2020-04-30T23:59:59.999Z", 0, true, ZoneOffset.UTC);
119+
}
120+
89121
public void testWeekDates() {
90122
assumeFalse(
91123
"won't work in jdk8 " + "because SPI mechanism is not looking at classpath - needs ISOCalendarDataProvider in jre's ext/libs",

0 commit comments

Comments
 (0)