Skip to content
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

Core: Move to java.time in Streamable/XContent #28327

Closed
wants to merge 1 commit into from
Closed

Core: Move to java.time in Streamable/XContent #28327

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
18 changes: 8 additions & 10 deletions server/src/main/java/org/elasticsearch/common/io/stream/StreamInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
import org.elasticsearch.common.bytes.BytesReference;
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.text.Text;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

import java.io.ByteArrayInputStream;
import java.io.EOFException;
Expand Down Expand Up @@ -562,9 +560,9 @@ private List readArrayList() throws IOException {
return list;
}

private DateTime readDateTime() throws IOException {
private ZonedDateTime readDateTime() throws IOException {
final String timeZoneId = readString();
return new DateTime(readLong(), DateTimeZone.forID(timeZoneId));
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(readLong()), ZoneId.of(timeZoneId));
Copy link
Member

Choose a reason for hiding this comment

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

Does ZoneId.of work for a larger range of values than DateTimeZone.forID? Or maybe we tell people not to use the ones that used to work and now don't?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, there are such cases, see #27330 (comment)

Copy link
Member

Choose a reason for hiding this comment

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

Got it. So the plan is to add deprecation warnings to older versions of ES and not support them in master?

}

private ZonedDateTime readZonedDateTime() throws IOException {
Expand Down Expand Up @@ -611,18 +609,18 @@ public GeoPoint readGeoPoint() throws IOException {
}

/**
* Read a {@linkplain DateTimeZone}.
* Read a {@linkplain ZoneId}.
*/
public DateTimeZone readTimeZone() throws IOException {
return DateTimeZone.forID(readString());
public ZoneId readTimeZone() throws IOException {
return ZoneId.of(readString());
}

/**
* Read an optional {@linkplain DateTimeZone}.
* Read an optional {@linkplain ZoneId}.
*/
public DateTimeZone readOptionalTimeZone() throws IOException {
public ZoneId readOptionalTimeZone() throws IOException {
if (readBoolean()) {
return DateTimeZone.forID(readString());
return ZoneId.of(readString());
}
return null;
}
Expand Down
15 changes: 8 additions & 7 deletions server/src/main/java/org/elasticsearch/common/io/stream/StreamOutput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.io.stream.Writeable.Writer;
import org.elasticsearch.common.text.Text;
import org.joda.time.DateTimeZone;
import org.joda.time.ReadableInstant;

import java.io.EOFException;
Expand All @@ -50,6 +49,7 @@
import java.nio.file.FileSystemLoopException;
import java.nio.file.NoSuchFileException;
import java.nio.file.NotDirectoryException;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -625,7 +625,6 @@ public final <K, V> void writeMap(final Map<K, V> map, final Writer<K> keyWriter
writers.put(ZonedDateTime.class, (o, v) -> {
o.writeByte((byte) 23);
final ZonedDateTime zonedDateTime = (ZonedDateTime) v;
zonedDateTime.getZone().getId();
o.writeString(zonedDateTime.getZone().getId());
o.writeLong(zonedDateTime.toInstant().toEpochMilli());
});
Expand All @@ -650,6 +649,8 @@ public void writeGenericValue(@Nullable Object value) throws IOException {
type = Object[].class;
} else if (value instanceof Map) {
type = Map.class;
} else if (value instanceof ZonedDateTime) {
type = ZonedDateTime.class;
} else if (value instanceof ReadableInstant) {
type = ReadableInstant.class;
} else if (value instanceof BytesReference) {
Expand Down Expand Up @@ -904,16 +905,16 @@ public void writeGeoPoint(GeoPoint geoPoint) throws IOException {
}

/**
* Write a {@linkplain DateTimeZone} to the stream.
* Write a {@linkplain ZoneId} to the stream.
*/
public void writeTimeZone(DateTimeZone timeZone) throws IOException {
writeString(timeZone.getID());
public void writeTimeZone(ZoneId timeZone) throws IOException {
writeString(timeZone.getId());
}

/**
* Write an optional {@linkplain DateTimeZone} to the stream.
* Write an optional {@linkplain ZoneId} to the stream.
*/
public void writeOptionalTimeZone(@Nullable DateTimeZone timeZone) throws IOException {
public void writeOptionalTimeZone(@Nullable ZoneId timeZone) throws IOException {
if (timeZone == null) {
writeBoolean(false);
} else {
Expand Down
17 changes: 16 additions & 1 deletion server/src/main/java/org/elasticsearch/common/joda/DateMathParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
import org.joda.time.MutableDateTime;
import org.joda.time.format.DateTimeFormatter;

import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.Objects;
import java.util.function.LongSupplier;

Expand All @@ -44,7 +46,20 @@ public DateMathParser(FormatDateTimeFormatter dateTimeFormatter) {
}

public long parse(String text, LongSupplier now) {
return parse(text, now, false, null);
return parse(text, now, false, (DateTimeZone) null);
Copy link
Member

Choose a reason for hiding this comment

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

Maybe it would be good (for ease of removal in the future) to flip around the joda impl below, so that it is the wrapper method, and the core parse method (which handles no time zone) is the same that handles ZoneId? This could be a follow up.

}

public long parse(String text, LongSupplier now, boolean roundUp, ZoneId timeZone) {
if (timeZone != null) {
// special handling of UTC, because the text interpretation of "Z" does not work for joda time
if (timeZone.equals(ZoneOffset.UTC)) {
return parse(text, now, roundUp, DateTimeZone.UTC);
} else {
return parse(text, now, roundUp, DateTimeZone.forID(timeZone.getId()));
}
} else {
return parse(text, now, roundUp, (DateTimeZone) null);
}
}

// Note: we take a callable here for the timestamp in order to be able to figure out
Expand Down
49 changes: 36 additions & 13 deletions server/src/main/java/org/elasticsearch/common/xcontent/XContentBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,18 @@
import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.unit.ByteSizeValue;
import org.elasticsearch.common.unit.TimeValue;
import org.joda.time.DateTimeZone;
import org.joda.time.ReadableInstant;
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.ISODateTimeFormat;

import java.io.Flushable;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.nio.file.Path;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.SignStyle;
import java.time.temporal.ChronoField;
import java.util.Arrays;
import java.util.Calendar;
import java.util.Collections;
Expand Down Expand Up @@ -86,7 +88,28 @@ public static XContentBuilder builder(XContent xContent, Set<String> includes, S
return new XContentBuilder(xContent, new BytesStreamOutput(), includes, excludes);
}

public static final DateTimeFormatter DEFAULT_DATE_PRINTER = ISODateTimeFormat.dateTime().withZone(DateTimeZone.UTC);
// this can be changed to DateTimeFormatter.ISO_OFFSET_DATE_TIME over time, when nanosecond precision is about to be used
// for now this emulates the same behaviour in format like the joda time date printer
public static final DateTimeFormatter DEFAULT_DATE_PRINTER =
new DateTimeFormatterBuilder()
.parseCaseInsensitive()
.appendValue(ChronoField.YEAR, 4, 10, SignStyle.EXCEEDS_PAD)
.appendLiteral('-')
.appendValue(ChronoField.MONTH_OF_YEAR, 2)
.appendLiteral('-')
.appendValue(ChronoField.DAY_OF_MONTH, 2)
.appendLiteral('T')
.appendValue(ChronoField.HOUR_OF_DAY, 2)
.appendLiteral(':')
.appendValue(ChronoField.MINUTE_OF_HOUR, 2)
.appendLiteral(':')
.appendValue(ChronoField.SECOND_OF_MINUTE, 2, 2, SignStyle.EXCEEDS_PAD)
.appendFraction(ChronoField.MILLI_OF_SECOND, 3, 3, true)
.parseLenient()
.appendOffsetId()
.parseStrict()
.toFormatter()
.withZone(ZoneOffset.UTC);

private static final Map<Class<?>, Writer> WRITERS;
static {
Expand Down Expand Up @@ -667,24 +690,24 @@ public XContentBuilder value(Text value) throws IOException {
// Date
//////////////////////////////////

public XContentBuilder field(String name, ReadableInstant value) throws IOException {
public XContentBuilder field(String name, Instant value) throws IOException {
return field(name).value(value);
}

public XContentBuilder field(String name, ReadableInstant value, DateTimeFormatter formatter) throws IOException {
public XContentBuilder field(String name, Instant value, DateTimeFormatter formatter) throws IOException {
return field(name).value(value, formatter);
}

public XContentBuilder value(ReadableInstant value) throws IOException {
public XContentBuilder value(Instant value) throws IOException {
return value(value, DEFAULT_DATE_PRINTER);
}

public XContentBuilder value(ReadableInstant value, DateTimeFormatter formatter) throws IOException {
public XContentBuilder value(Instant value, DateTimeFormatter formatter) throws IOException {
if (value == null) {
return nullValue();
}
ensureFormatterNotNull(formatter);
return value(formatter.print(value));
return value(formatter.format(value));
}

public XContentBuilder field(String name, Date value) throws IOException {
Expand Down Expand Up @@ -723,7 +746,7 @@ XContentBuilder value(Calendar value) throws IOException {

XContentBuilder value(DateTimeFormatter formatter, long value) throws IOException {
ensureFormatterNotNull(formatter);
return value(formatter.print(value));
return value(formatter.format(Instant.ofEpochMilli(value)));
}

////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -808,8 +831,8 @@ private void unknownValue(Object value, boolean ensureNoSelfReferences) throws I
values((Object[]) value, ensureNoSelfReferences);
} else if (value instanceof Calendar) {
value((Calendar) value);
} else if (value instanceof ReadableInstant) {
value((ReadableInstant) value);
} else if (value instanceof Instant) {
value((Instant) value);
} else if (value instanceof BytesReference) {
value((BytesReference) value);
} else if (value instanceof ToXContent) {
Expand Down
7 changes: 4 additions & 3 deletions server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
import org.joda.time.DateTimeZone;

import java.io.IOException;
import java.time.ZoneId;
import java.util.Iterator;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -268,7 +269,7 @@ public Query termQuery(Object value, @Nullable QueryShardContext context) {

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper, ShapeRelation relation,
@Nullable DateTimeZone timeZone, @Nullable DateMathParser forcedDateParser, QueryShardContext context) {
@Nullable ZoneId timeZone, @Nullable DateMathParser forcedDateParser, QueryShardContext context) {
failIfNotIndexed();
if (relation == ShapeRelation.DISJOINT) {
throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() +
Expand Down Expand Up @@ -303,7 +304,7 @@ public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower
}

public long parseToMilliseconds(Object value, boolean roundUp,
@Nullable DateTimeZone zone, @Nullable DateMathParser forcedDateParser, QueryRewriteContext context) {
@Nullable ZoneId zone, @Nullable DateMathParser forcedDateParser, QueryRewriteContext context) {
DateMathParser dateParser = dateMathParser();
if (forcedDateParser != null) {
dateParser = forcedDateParser;
Expand All @@ -321,7 +322,7 @@ public long parseToMilliseconds(Object value, boolean roundUp,
@Override
public Relation isFieldWithinQuery(IndexReader reader,
Object from, Object to, boolean includeLower, boolean includeUpper,
DateTimeZone timeZone, DateMathParser dateParser, QueryRewriteContext context) throws IOException {
ZoneId timeZone, DateMathParser dateParser, QueryRewriteContext context) throws IOException {
if (dateParser == null) {
dateParser = this.dateMathParser;
}
Expand Down
5 changes: 3 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.joda.time.DateTimeZone;

import java.io.IOException;
import java.time.ZoneId;
import java.util.List;
import java.util.Objects;

Expand Down Expand Up @@ -355,7 +356,7 @@ public Query termsQuery(List<?> values, @Nullable QueryShardContext context) {
public Query rangeQuery(
Object lowerTerm, Object upperTerm,
boolean includeLower, boolean includeUpper,
ShapeRelation relation, DateTimeZone timeZone, DateMathParser parser,
ShapeRelation relation, ZoneId timeZone, DateMathParser parser,
QueryShardContext context) {
throw new IllegalArgumentException("Field [" + name + "] of type [" + typeName() + "] does not support range queries");
}
Expand Down Expand Up @@ -399,7 +400,7 @@ public Relation isFieldWithinQuery(
IndexReader reader,
Object from, Object to,
boolean includeLower, boolean includeUpper,
DateTimeZone timeZone, DateMathParser dateMathParser, QueryRewriteContext context) throws IOException {
ZoneId timeZone, DateMathParser dateMathParser, QueryRewriteContext context) throws IOException {
return Relation.INTERSECTS;
}

Expand Down
13 changes: 8 additions & 5 deletions server/src/main/java/org/elasticsearch/index/mapper/RangeFieldMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
import java.io.IOException;
import java.net.InetAddress;
import java.net.UnknownHostException;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
Expand Down Expand Up @@ -313,7 +314,7 @@ public Query termQuery(Object value, QueryShardContext context) {

@Override
public Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
ShapeRelation relation, DateTimeZone timeZone, DateMathParser parser, QueryShardContext context) {
ShapeRelation relation, ZoneId timeZone, DateMathParser parser, QueryShardContext context) {
failIfNotIndexed();
return rangeType.rangeQuery(name(), hasDocValues(), lowerTerm, upperTerm, includeLower, includeUpper, relation,
timeZone, parser, context);
Expand Down Expand Up @@ -611,9 +612,11 @@ public Query dvRangeQuery(String field, QueryType queryType, Object from, Object

@Override
public Query rangeQuery(String field, boolean hasDocValues, Object lowerTerm, Object upperTerm, boolean includeLower,
boolean includeUpper, ShapeRelation relation, @Nullable DateTimeZone timeZone,
boolean includeUpper, ShapeRelation relation, @Nullable ZoneId timeZone,
@Nullable DateMathParser parser, QueryShardContext context) {
DateTimeZone zone = (timeZone == null) ? DateTimeZone.UTC : timeZone;
// special treatment for Z timezone, which is not known by joda, but used by java time
DateTimeZone zone =
(timeZone == null || "Z".equals(timeZone.getId())) ? DateTimeZone.UTC : DateTimeZone.forID(timeZone.getId());
DateMathParser dateMathParser = (parser == null) ?
new DateMathParser(DateFieldMapper.DEFAULT_DATE_TIME_FORMATTER) : parser;
Long low = lowerTerm == null ? Long.MIN_VALUE :
Expand All @@ -623,7 +626,7 @@ public Query rangeQuery(String field, boolean hasDocValues, Object lowerTerm, Ob
dateMathParser.parse(upperTerm instanceof BytesRef ? ((BytesRef) upperTerm).utf8ToString() : upperTerm.toString(),
context::nowInMillis, false, zone);

return super.rangeQuery(field, hasDocValues, low, high, includeLower, includeUpper, relation, zone,
return super.rangeQuery(field, hasDocValues, low, high, includeLower, includeUpper, relation, timeZone,
dateMathParser, context);
}
@Override
Expand Down Expand Up @@ -934,7 +937,7 @@ public Object parse(Object value, boolean coerce) {
return numberType.parse(value, coerce);
}
public Query rangeQuery(String field, boolean hasDocValues, Object from, Object to, boolean includeFrom, boolean includeTo,
ShapeRelation relation, @Nullable DateTimeZone timeZone, @Nullable DateMathParser dateMathParser,
ShapeRelation relation, @Nullable ZoneId timeZone, @Nullable DateMathParser dateMathParser,
QueryShardContext context) {
Object lower = from == null ? minValue() : parse(from, false);
Object upper = to == null ? maxValue() : parse(to, false);
Expand Down
6 changes: 4 additions & 2 deletions server/src/main/java/org/elasticsearch/index/mapper/SimpleMappedFieldType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.elasticsearch.index.query.QueryShardContext;
import org.joda.time.DateTimeZone;

import java.time.ZoneId;

/**
* {@link MappedFieldType} base impl for field types that are neither dates nor ranges.
*/
Expand All @@ -40,7 +42,7 @@ protected SimpleMappedFieldType(MappedFieldType ref) {

@Override
public final Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
ShapeRelation relation, DateTimeZone timeZone, DateMathParser parser, QueryShardContext context) {
ShapeRelation relation, ZoneId timeZone, DateMathParser parser, QueryShardContext context) {
if (relation == ShapeRelation.DISJOINT) {
throw new IllegalArgumentException("Field [" + name() + "] of type [" + typeName() +
"] does not support DISJOINT ranges");
Expand All @@ -52,7 +54,7 @@ public final Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includ
}

/**
* Same as {@link #rangeQuery(Object, Object, boolean, boolean, ShapeRelation, DateTimeZone, DateMathParser, QueryShardContext)}
* Same as {@link #rangeQuery(Object, Object, boolean, boolean, ShapeRelation, ZoneId, DateMathParser, QueryShardContext)}
* but without the trouble of relations or date-specific options.
*/
protected Query rangeQuery(Object lowerTerm, Object upperTerm, boolean includeLower, boolean includeUpper,
Expand Down
Loading