Skip to content

Core: Remove parseDefaulting from DateFormatter #36386

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Core: Remove parseDefaulting from DateFormatter #36386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions server/src/main/java/org/elasticsearch/cluster/metadata/IndexNameExpressionResolver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.time.DateFormatters;
import org.elasticsearch.common.time.DateMathParser;
import org.elasticsearch.common.time.JavaDateMathParser;
import org.elasticsearch.common.util.set.Sets;
import org.elasticsearch.index.Index;
import org.elasticsearch.index.IndexNotFoundException;
Expand Down Expand Up @@ -921,7 +920,7 @@ String resolveExpression(String expression, final Context context) {
dateFormatter = DateFormatters.forPattern(dateFormatterPattern);
}
DateFormatter formatter = dateFormatter.withZone(timeZone);
DateMathParser dateMathParser = new JavaDateMathParser(formatter);
DateMathParser dateMathParser = formatter.toDateMathParser();
long millis = dateMathParser.parse(mathExpression, context::getStartTime, false, timeZone);

String time = formatter.format(Instant.ofEpochMilli(millis));
Expand Down
36 changes: 24 additions & 12 deletions server/src/main/java/org/elasticsearch/common/time/DateFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@

package org.elasticsearch.common.time;

import org.elasticsearch.ElasticsearchParseException;

import java.time.ZoneId;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalField;
import java.util.Arrays;
import java.util.Locale;
import java.util.Map;
import java.util.stream.Collectors;

public interface DateFormatter {
Expand Down Expand Up @@ -86,13 +86,9 @@ public interface DateFormatter {
ZoneId getZone();

/**
* Configure a formatter using default fields for a TemporalAccessor that should be used in case
* the supplied date is not having all of those fields
*
* @param fields A <code>Map&lt;TemporalField, Long&gt;</code> of fields to be used as fallbacks
* @return A new date formatter instance, that will use those fields during parsing
* Return a {@link DateMathParser} built from this formatter.
*/
DateFormatter parseDefaulting(Map<TemporalField, Long> fields);
DateMathParser toDateMathParser();

/**
* Merge several date formatters into a single one. Useful if you need to have several formatters with
Expand All @@ -102,18 +98,20 @@ public interface DateFormatter {
* @param formatters The list of date formatters to be merged together
* @return The new date formtter containing the specified date formatters
*/
static DateFormatter merge(DateFormatter ... formatters) {
static DateFormatter merge(DateFormatter... formatters) {
return new MergedDateFormatter(formatters);
}

class MergedDateFormatter implements DateFormatter {

private final String format;
private final DateFormatter[] formatters;
private final DateMathParser[] dateMathParsers;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you please write a brief doc that this is an optimization that means the formatters don't have to create dateMathParsers each time? It was non-obvious to me.


MergedDateFormatter(DateFormatter ... formatters) {
MergedDateFormatter(DateFormatter... formatters) {
this.formatters = formatters;
this.format = Arrays.stream(formatters).map(DateFormatter::pattern).collect(Collectors.joining("||"));
this.dateMathParsers = Arrays.stream(formatters).map(DateFormatter::toDateMathParser).toArray(DateMathParser[]::new);
}

@Override
Expand Down Expand Up @@ -164,8 +162,22 @@ public ZoneId getZone() {
}

@Override
public DateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
return new MergedDateFormatter(Arrays.stream(formatters).map(f -> f.parseDefaulting(fields)).toArray(DateFormatter[]::new));
public DateMathParser toDateMathParser() {
return (text, now, roundUp, tz) -> {
ElasticsearchParseException failure = null;
for (DateMathParser parser : dateMathParsers) {
try {
return parser.parse(text, now, roundUp, tz);
} catch (ElasticsearchParseException e) {
if (failure == null) {
failure = e;
} else {
failure.addSuppressed(e);
}
}
}
throw failure;
};
}
}
}
15 changes: 7 additions & 8 deletions server/src/main/java/org/elasticsearch/common/time/EpochMillisDateFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@
import java.time.ZoneOffset;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalField;
import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;

/**
Expand All @@ -42,7 +40,8 @@
class EpochMillisDateFormatter implements DateFormatter {

private static final Pattern SPLIT_BY_DOT_PATTERN = Pattern.compile("\\.");
static DateFormatter INSTANCE = new EpochMillisDateFormatter();
static final DateFormatter INSTANCE = new EpochMillisDateFormatter();
static final DateMathParser DATE_MATH_INSTANCE = new JavaDateMathParser(INSTANCE, INSTANCE);

private EpochMillisDateFormatter() {
}
Expand Down Expand Up @@ -103,11 +102,6 @@ public String pattern() {
return "epoch_millis";
}

@Override
public DateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
return this;
}

@Override
public Locale getLocale() {
return Locale.ROOT;
Expand All @@ -117,4 +111,9 @@ public Locale getLocale() {
public ZoneId getZone() {
return ZoneOffset.UTC;
}

@Override
public DateMathParser toDateMathParser() {
return DATE_MATH_INSTANCE;
}
}
13 changes: 6 additions & 7 deletions server/src/main/java/org/elasticsearch/common/time/EpochSecondsDateFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,13 @@
import java.time.ZoneOffset;
import java.time.format.DateTimeParseException;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalField;
import java.util.Locale;
import java.util.Map;
import java.util.regex.Pattern;

public class EpochSecondsDateFormatter implements DateFormatter {

public static DateFormatter INSTANCE = new EpochSecondsDateFormatter();
static final DateMathParser DATE_MATH_INSTANCE = new JavaDateMathParser(INSTANCE, INSTANCE);
private static final Pattern SPLIT_BY_DOT_PATTERN = Pattern.compile("\\.");

private EpochSecondsDateFormatter() {}
Expand Down Expand Up @@ -91,6 +90,11 @@ public ZoneId getZone() {
return ZoneOffset.UTC;
}

@Override
public DateMathParser toDateMathParser() {
return DATE_MATH_INSTANCE;
}

@Override
public DateFormatter withZone(ZoneId zoneId) {
if (zoneId.equals(ZoneOffset.UTC) == false) {
Expand All @@ -106,9 +110,4 @@ public DateFormatter withLocale(Locale locale) {
}
return this;
}

@Override
public DateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
return this;
}
}
21 changes: 19 additions & 2 deletions server/src/main/java/org/elasticsearch/common/time/JavaDateFormatter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,28 @@
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.DateTimeParseException;
import java.time.temporal.ChronoField;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalField;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Locale;
import java.util.Map;
import java.util.Objects;

class JavaDateFormatter implements DateFormatter {

// base fields which should be used for default parsing, when we round up for date math
private static final Map<TemporalField, Long> ROUND_UP_BASE_FIELDS = new HashMap<>(6);
{
ROUND_UP_BASE_FIELDS.put(ChronoField.MONTH_OF_YEAR, 1L);
ROUND_UP_BASE_FIELDS.put(ChronoField.DAY_OF_MONTH, 1L);
ROUND_UP_BASE_FIELDS.put(ChronoField.HOUR_OF_DAY, 23L);
ROUND_UP_BASE_FIELDS.put(ChronoField.MINUTE_OF_HOUR, 59L);
ROUND_UP_BASE_FIELDS.put(ChronoField.SECOND_OF_MINUTE, 59L);
ROUND_UP_BASE_FIELDS.put(ChronoField.MILLI_OF_SECOND, 999L);
}

private final String format;
private final DateTimeFormatter printer;
private final DateTimeFormatter[] parsers;
Expand Down Expand Up @@ -116,8 +129,7 @@ public String pattern() {
return format;
}

@Override
public DateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
JavaDateFormatter parseDefaulting(Map<TemporalField, Long> fields) {
final DateTimeFormatterBuilder parseDefaultingBuilder = new DateTimeFormatterBuilder().append(printer);
fields.forEach(parseDefaultingBuilder::parseDefaulting);
if (parsers.length == 1 && parsers[0].equals(printer)) {
Expand All @@ -143,6 +155,11 @@ public ZoneId getZone() {
return this.printer.getZone();
}

@Override
public DateMathParser toDateMathParser() {
return new JavaDateMathParser(this, this.parseDefaulting(ROUND_UP_BASE_FIELDS));
}

@Override
public int hashCode() {
return Objects.hash(getLocale(), printer.getZone(), format);
Expand Down
18 changes: 3 additions & 15 deletions server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,7 @@
import java.time.temporal.ChronoField;
import java.time.temporal.TemporalAccessor;
import java.time.temporal.TemporalAdjusters;
import java.time.temporal.TemporalField;
import java.time.temporal.TemporalQueries;
import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.function.LongSupplier;

Expand All @@ -47,24 +44,15 @@
*/
public class JavaDateMathParser implements DateMathParser {

// base fields which should be used for default parsing, when we round up
private static final Map<TemporalField, Long> ROUND_UP_BASE_FIELDS = new HashMap<>(6);
{
ROUND_UP_BASE_FIELDS.put(ChronoField.MONTH_OF_YEAR, 1L);
ROUND_UP_BASE_FIELDS.put(ChronoField.DAY_OF_MONTH, 1L);
ROUND_UP_BASE_FIELDS.put(ChronoField.HOUR_OF_DAY, 23L);
ROUND_UP_BASE_FIELDS.put(ChronoField.MINUTE_OF_HOUR, 59L);
ROUND_UP_BASE_FIELDS.put(ChronoField.SECOND_OF_MINUTE, 59L);
ROUND_UP_BASE_FIELDS.put(ChronoField.MILLI_OF_SECOND, 999L);
}


private final DateFormatter formatter;
private final DateFormatter roundUpFormatter;

public JavaDateMathParser(DateFormatter formatter) {
public JavaDateMathParser(DateFormatter formatter, DateFormatter roundUpFormatter) {
Objects.requireNonNull(formatter);
this.formatter = formatter;
this.roundUpFormatter = formatter.parseDefaulting(ROUND_UP_BASE_FIELDS);
this.roundUpFormatter = roundUpFormatter;
}

@Override
Expand Down
8 changes: 4 additions & 4 deletions server/src/test/java/org/elasticsearch/common/time/JavaDateMathParserTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
public class JavaDateMathParserTests extends ESTestCase {

private final DateFormatter formatter = DateFormatters.forPattern("dateOptionalTime||epoch_millis");
private final JavaDateMathParser parser = new JavaDateMathParser(formatter);
private final DateMathParser parser = formatter.toDateMathParser();

public void testBasicDates() {
assertDateMathEquals("2014", "2014-01-01T00:00:00.000");
Expand Down Expand Up @@ -139,7 +139,7 @@ public void testNow() {
public void testRoundingPreservesEpochAsBaseDate() {
// If a user only specifies times, then the date needs to always be 1970-01-01 regardless of rounding
DateFormatter formatter = DateFormatters.forPattern("HH:mm:ss");
JavaDateMathParser parser = new JavaDateMathParser(formatter);
DateMathParser parser = formatter.toDateMathParser();
ZonedDateTime zonedDateTime = DateFormatters.toZonedDateTime(formatter.parse("04:52:20"));
assertThat(zonedDateTime.getYear(), is(1970));
long millisStart = zonedDateTime.toInstant().toEpochMilli();
Expand All @@ -165,7 +165,7 @@ public void testImplicitRounding() {

// implicit rounding with explicit timezone in the date format
DateFormatter formatter = DateFormatters.forPattern("yyyy-MM-ddXXX");
JavaDateMathParser parser = new JavaDateMathParser(formatter);
DateMathParser parser = formatter.toDateMathParser();
long time = parser.parse("2011-10-09+01:00", () -> 0, false, (ZoneId) null);
assertEquals(this.parser.parse("2011-10-09T00:00:00.000+01:00", () -> 0), time);
time = parser.parse("2011-10-09+01:00", () -> 0, true, (ZoneId) null);
Expand Down Expand Up @@ -239,7 +239,7 @@ public void testTimestamps() {
assertDateMathEquals("1418248078000||/m", "2014-12-10T21:47:00.000");

// also check other time units
JavaDateMathParser parser = new JavaDateMathParser(DateFormatters.forPattern("epoch_second||dateOptionalTime"));
DateMathParser parser = DateFormatters.forPattern("epoch_second||dateOptionalTime").toDateMathParser();
long datetime = parser.parse("1418248078", () -> 0);
assertDateEquals(datetime, "1418248078", "2014-12-10T21:47:58.000");

Expand Down