Skip to content

Commit

Permalink
Fix Joda compatibility in stream protocol (#53823)
Browse files Browse the repository at this point in the history
The JodaCompatibleZonedDateTime is a compatibility object that unions
Joda's DateTime and Java's ZonedDateTime, meant for use in scripts. When
it was added, we serialized the JCZDT as a Joda DateTime so that when
sending to older nodes they could still read the object. However, on
newer nodes, we continued also reading this as a Joda DateTime. This
commit changes the read side to form a JCZDT.

closes #53586
  • Loading branch information
rjernst authored Mar 19, 2020
1 parent b873d8e commit c2ae78d
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,10 @@
import org.elasticsearch.common.geo.GeoPoint;
import org.elasticsearch.common.settings.SecureString;
import org.elasticsearch.common.text.Text;
import org.elasticsearch.common.time.DateUtils;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
import org.joda.time.DateTime;
import org.elasticsearch.script.JodaCompatibleZonedDateTime;
import org.joda.time.DateTimeZone;

import java.io.ByteArrayInputStream;
Expand Down Expand Up @@ -768,9 +769,12 @@ private List<Object> readArrayList() throws IOException {
return list;
}

private DateTime readDateTime() throws IOException {
final String timeZoneId = readString();
return new DateTime(readLong(), DateTimeZone.forID(timeZoneId));
private JodaCompatibleZonedDateTime readDateTime() throws IOException {
// we reuse DateTime to communicate with older nodes that don't know about the joda compat layer, but
// here we are on a new node so we always want a compat datetime
final ZoneId zoneId = DateUtils.dateTimeZoneToZoneId(DateTimeZone.forID(readString()));
long millis = readLong();
return new JodaCompatibleZonedDateTime(Instant.ofEpochMilli(millis), zoneId);
}

private ZonedDateTime readZonedDateTime() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,14 @@
import org.elasticsearch.common.lucene.BytesRefs;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.util.PageCacheRecycler;
import org.elasticsearch.script.JodaCompatibleZonedDateTime;
import org.elasticsearch.test.ESTestCase;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

import java.io.EOFException;
import java.io.IOException;
import java.time.ZoneId;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
Expand All @@ -49,6 +52,7 @@
import static org.hamcrest.Matchers.endsWith;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.nullValue;

Expand Down Expand Up @@ -303,6 +307,7 @@ public void testSimpleStreams() throws Exception {
out.writeTimeZone(DateTimeZone.forID("CET"));
out.writeOptionalTimeZone(DateTimeZone.getDefault());
out.writeOptionalTimeZone(null);
out.writeGenericValue(new DateTime(123456, DateTimeZone.forID("America/Los_Angeles")));
final byte[] bytes = BytesReference.toBytes(out.bytes());
StreamInput in = StreamInput.wrap(BytesReference.toBytes(out.bytes()));
assertEquals(in.available(), bytes.length);
Expand Down Expand Up @@ -335,6 +340,11 @@ public void testSimpleStreams() throws Exception {
assertEquals(DateTimeZone.forID("CET"), in.readTimeZone());
assertEquals(DateTimeZone.getDefault(), in.readOptionalTimeZone());
assertNull(in.readOptionalTimeZone());
Object dt = in.readGenericValue();
assertThat(dt, instanceOf(JodaCompatibleZonedDateTime.class));
JodaCompatibleZonedDateTime jdt = (JodaCompatibleZonedDateTime) dt;
assertThat(jdt.getZonedDateTime().toInstant().toEpochMilli(), equalTo(123456L));
assertThat(jdt.getZonedDateTime().getZone(), equalTo(ZoneId.of("America/Los_Angeles")));
assertEquals(0, in.available());
in.close();
out.close();
Expand Down

0 comments on commit c2ae78d

Please sign in to comment.