Skip to content

Commit 00b9c28

Browse files
pgomulkapgomulka
authored
Extend the date rounding logic to be conditional (#89693) (#89801)
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 #89096 closes #58986 backports #89693
1 parent ff0db15 commit 00b9c28

File tree

4 files changed

+84
-13
lines changed

4 files changed

+84
-13
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: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,44 @@
2222
import java.util.List;
2323
import java.util.Locale;
2424
import java.util.Objects;
25-
import java.util.function.Consumer;
25+
import java.util.function.BiConsumer;
2626
import java.util.function.UnaryOperator;
2727

2828
class JavaDateFormatter implements DateFormatter {
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+
};
2963

3064
private final String format;
3165
private final DateTimeFormatter printer;
@@ -50,12 +84,7 @@ JavaDateFormatter getRoundupParser() {
5084
format,
5185
printer,
5286
// set up base fields which should be used for default parsing, when we round up for date math
53-
builder -> builder.parseDefaulting(ChronoField.MONTH_OF_YEAR, 1L)
54-
.parseDefaulting(ChronoField.DAY_OF_MONTH, 1L)
55-
.parseDefaulting(ChronoField.HOUR_OF_DAY, 23L)
56-
.parseDefaulting(ChronoField.MINUTE_OF_HOUR, 59L)
57-
.parseDefaulting(ChronoField.SECOND_OF_MINUTE, 59L)
58-
.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
87+
DEFAULT_ROUND_UP,
5988
parsers
6089
);
6190
}
@@ -64,7 +93,7 @@ JavaDateFormatter getRoundupParser() {
6493
JavaDateFormatter(
6594
String format,
6695
DateTimeFormatter printer,
67-
Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
96+
BiConsumer<DateTimeFormatterBuilder, DateTimeFormatter> roundupParserConsumer,
6897
DateTimeFormatter... parsers
6998
) {
7099
if (printer == null) {
@@ -105,15 +134,15 @@ private static DateTimeFormatter[] parsersArray(DateTimeFormatter printer, DateT
105134
*/
106135
private static RoundUpFormatter createRoundUpParser(
107136
String format,
108-
Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
137+
BiConsumer<DateTimeFormatterBuilder, DateTimeFormatter> roundupParserConsumer,
109138
Locale locale,
110139
DateTimeFormatter[] parsers
111140
) {
112141
if (format.contains("||") == false) {
113142
return new RoundUpFormatter(format, mapParsers(parser -> {
114143
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
115144
builder.append(parser);
116-
roundupParserConsumer.accept(builder);
145+
roundupParserConsumer.accept(builder, parser);
117146
return builder.toFormatter(locale);
118147
}, parsers));
119148
}
@@ -205,7 +234,7 @@ private TemporalAccessor doParse(String input) {
205234
return this.parsers[0].parse(input);
206235
}
207236

208-
private boolean parsingSucceeded(Object object, String input, ParsePosition pos) {
237+
private static boolean parsingSucceeded(Object object, String input, ParsePosition pos) {
209238
return object != null && pos.getIndex() == input.length();
210239
}
211240

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,41 @@ 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+
/* due to jdk8 returning Sunday,1 for locale.root this testcase cannot reliably pass.
95+
but this is fine, we don't really need to support missing day of week
96+
// defaulting missing day of week
97+
formatter = DateFormatter.forPattern("YYYY'W'ww[e]");// YYYY'W'wwe
98+
// second week of 2022 is starting on Monday 10th Jan
99+
assertDateMathEquals(formatter.toDateMathParser(), "2022W02", "2022-01-10T23:59:59.999Z", 0, true, ZoneOffset.UTC);
100+
*/
101+
}
102+
103+
public void testDayOfYear() {
104+
DateFormatter formatter = DateFormatter.forPattern("yyyy-DDD'T'HH:mm:ss.SSS");
105+
assertDateMathEquals(formatter.toDateMathParser(), "2022-104T14:08:30.293", "2022-04-14T14:08:30.293", 0, true, ZoneOffset.UTC);
106+
}
107+
108+
public void testAMPM() {
109+
DateFormatter formatter = DateFormatter.forPattern("MM/dd/yyyy hh:mm a"); // h clock-hour-of-am-pm (1-12)
110+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020 12:48 AM", "2020-04-30T00:48:59.999Z", 0, true, ZoneOffset.UTC);
111+
112+
formatter = DateFormatter.forPattern("MM/dd/yyyy KK:mm a"); // K hour-of-am-pm (0-11)
113+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020 00:48 AM", "2020-04-30T00:48:59.999Z", 0, true, ZoneOffset.UTC);
114+
}
115+
116+
public void testAMPMWithTimeMissing() {
117+
DateFormatter formatter = DateFormatter.forPattern("MM/dd/yyyy[ hh:mm a]"); // h clock-hour-of-am-pm (1-12)
118+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020", "2020-04-30T23:59:59.999Z", 0, true, ZoneOffset.UTC);
119+
120+
formatter = DateFormatter.forPattern("MM/dd/yyyy[ KK:mm a]"); // K hour-of-am-pm (0-11)
121+
assertDateMathEquals(formatter.toDateMathParser(), "04/30/2020", "2020-04-30T23:59:59.999Z", 0, true, ZoneOffset.UTC);
122+
}
123+
89124
public void testWeekDates() {
90125
assumeFalse(
91126
"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)