Skip to content

Commit

Permalink
[Remove] Joda from Serialization (opensearch-project#9350)
Browse files Browse the repository at this point in the history
In Legacy 7.0 joda time was removed in favor of java time to support
nanoseconds properly. This commit removes joda from the serialization
including ignoring the joda flag sent over the wire for DocValueFormat.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
Signed-off-by: Shivansh Arora <hishiv@amazon.com>
  • Loading branch information
nknize authored and shiv0408 committed Apr 25, 2024
1 parent cfca246 commit d8ffe60
Show file tree
Hide file tree
Showing 11 changed files with 55 additions and 162 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,8 @@ public Object readGenericValue() throws IOException {
return readByte();
case 12:
return readDate();
case 13:
return readZonedDateTime();
case 14:
return readBytesReference();
case 15:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,9 @@
package org.opensearch.common.io.stream;

import org.opensearch.common.geo.GeoPoint;
import org.opensearch.common.time.DateUtils;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.Writeable.WriteableRegistry;
import org.joda.time.DateTimeZone;
import org.joda.time.ReadableInstant;

import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;

/**
* This utility class registers generic types for streaming over the wire using
Expand Down Expand Up @@ -47,14 +40,6 @@ public static void registerStreamables() {
* Registers writers by class type
*/
private static void registerWriters() {
/** {@link ReadableInstant} */
WriteableRegistry.registerWriter(ReadableInstant.class, (o, v) -> {
o.writeByte((byte) 13);
final ReadableInstant instant = (ReadableInstant) v;
o.writeString(instant.getZone().getID());
o.writeLong(instant.getMillis());
});
WriteableRegistry.registerClassAlias(ReadableInstant.class, ReadableInstant.class);
/** {@link GeoPoint} */
WriteableRegistry.registerWriter(GeoPoint.class, (o, v) -> {
o.writeByte((byte) 22);
Expand All @@ -68,12 +53,6 @@ private static void registerWriters() {
* NOTE: see {@code StreamOutput#WRITERS} for all registered ordinals
*/
private static void registerReaders() {
/** {@link ZonedDateTime} */
WriteableRegistry.registerReader(Byte.valueOf((byte) 13), (i) -> {
final ZoneId zoneId = DateUtils.dateTimeZoneToZoneId(DateTimeZone.forID(i.readString()));
long millis = i.readLong();
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), zoneId);
});
/** {@link GeoPoint} */
WriteableRegistry.registerReader(Byte.valueOf((byte) 22), GeoPoint::new);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,18 +39,10 @@
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentBuilderExtension;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;
import org.joda.time.Instant;
import org.joda.time.MutableDateTime;
import org.joda.time.ReadableInstant;
import org.joda.time.format.DateTimeFormatter;
import org.joda.time.format.ISODateTimeFormat;
import org.joda.time.tz.CachedDateTimeZone;
import org.joda.time.tz.FixedDateTimeZone;

import java.time.DayOfWeek;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
Expand Down Expand Up @@ -79,7 +71,6 @@
*/
public class XContentOpenSearchExtension implements XContentBuilderExtension {

public static final DateTimeFormatter DEFAULT_DATE_PRINTER = ISODateTimeFormat.dateTime().withZone(DateTimeZone.UTC);
public static final DateFormatter DEFAULT_FORMATTER = DateFormatter.forPattern("strict_date_optional_time_nanos");
public static final DateFormatter LOCAL_TIME_FORMATTER = DateFormatter.forPattern("HH:mm:ss.SSS");
public static final DateFormatter OFFSET_TIME_FORMATTER = DateFormatter.forPattern("HH:mm:ss.SSSZZZZZ");
Expand All @@ -90,11 +81,6 @@ public Map<Class<?>, XContentBuilder.Writer> getXContentWriters() {

// Fully-qualified here to reduce ambiguity around our (OpenSearch') Version class
writers.put(org.apache.lucene.util.Version.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(DateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(CachedDateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(FixedDateTimeZone.class, (b, v) -> b.value(Objects.toString(v)));
writers.put(MutableDateTime.class, XContentBuilder::timeValue);
writers.put(DateTime.class, XContentBuilder::timeValue);
writers.put(TimeValue.class, (b, v) -> b.value(v.toString()));
writers.put(ZonedDateTime.class, XContentBuilder::timeValue);
writers.put(OffsetDateTime.class, XContentBuilder::timeValue);
Expand Down Expand Up @@ -141,14 +127,11 @@ public Map<Class<?>, XContentBuilder.HumanReadableTransformer> getXContentHumanR
@Override
public Map<Class<?>, Function<Object, Object>> getDateTransformers() {
Map<Class<?>, Function<Object, Object>> transformers = new HashMap<>();
transformers.put(Date.class, d -> DEFAULT_DATE_PRINTER.print(((Date) d).getTime()));
transformers.put(DateTime.class, d -> DEFAULT_DATE_PRINTER.print((DateTime) d));
transformers.put(MutableDateTime.class, d -> DEFAULT_DATE_PRINTER.print((MutableDateTime) d));
transformers.put(ReadableInstant.class, d -> DEFAULT_DATE_PRINTER.print((ReadableInstant) d));
transformers.put(Long.class, d -> DEFAULT_DATE_PRINTER.print((long) d));
transformers.put(Calendar.class, d -> DEFAULT_DATE_PRINTER.print(((Calendar) d).getTimeInMillis()));
transformers.put(GregorianCalendar.class, d -> DEFAULT_DATE_PRINTER.print(((Calendar) d).getTimeInMillis()));
transformers.put(Instant.class, d -> DEFAULT_DATE_PRINTER.print((Instant) d));
transformers.put(Date.class, d -> DEFAULT_FORMATTER.format(((Date) d).toInstant()));
transformers.put(Long.class, d -> DEFAULT_FORMATTER.format(Instant.ofEpochMilli((long) d)));
transformers.put(Calendar.class, d -> DEFAULT_FORMATTER.format(((Calendar) d).toInstant()));
transformers.put(GregorianCalendar.class, d -> DEFAULT_FORMATTER.format(((Calendar) d).toInstant()));
transformers.put(Instant.class, d -> DEFAULT_FORMATTER.format((Instant) d));
transformers.put(ZonedDateTime.class, d -> DEFAULT_FORMATTER.format((ZonedDateTime) d));
transformers.put(OffsetDateTime.class, d -> DEFAULT_FORMATTER.format((OffsetDateTime) d));
transformers.put(OffsetTime.class, d -> OFFSET_TIME_FORMATTER.format((OffsetTime) d));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.action.RestResponseListener;

import java.time.Instant;
import java.util.Comparator;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -170,9 +171,9 @@ public int compare(RecoveryState o1, RecoveryState o2) {
t.startRow();
t.addCell(index);
t.addCell(state.getShardId().id());
t.addCell(XContentOpenSearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().startTime()));
t.addCell(XContentOpenSearchExtension.DEFAULT_FORMATTER.format(Instant.ofEpochMilli(state.getTimer().startTime())));
t.addCell(state.getTimer().startTime());
t.addCell(XContentOpenSearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().stopTime()));
t.addCell(XContentOpenSearchExtension.DEFAULT_FORMATTER.format(Instant.ofEpochMilli(state.getTimer().stopTime())));
t.addCell(state.getTimer().stopTime());
t.addCell(new TimeValue(state.getTimer().time()));
t.addCell(state.getRecoverySource().getType().toString().toLowerCase(Locale.ROOT));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.opensearch.rest.RestResponse;
import org.opensearch.rest.action.RestResponseListener;

import java.time.Instant;
import java.util.List;
import java.util.Locale;
import java.util.Map;
Expand Down Expand Up @@ -180,8 +181,8 @@ public Table buildSegmentReplicationTable(RestRequest request, SegmentReplicatio
t.addCell(String.format(Locale.ROOT, "%1.1f%%", state.getIndex().recoveredFilesPercent()));
t.addCell(state.getIndex().recoveredBytes());
t.addCell(String.format(Locale.ROOT, "%1.1f%%", state.getIndex().recoveredBytesPercent()));
t.addCell(XContentOpenSearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().startTime()));
t.addCell(XContentOpenSearchExtension.DEFAULT_DATE_PRINTER.print(state.getTimer().stopTime()));
t.addCell(XContentOpenSearchExtension.DEFAULT_FORMATTER.format(Instant.ofEpochMilli(state.getTimer().startTime())));
t.addCell(XContentOpenSearchExtension.DEFAULT_FORMATTER.format(Instant.ofEpochMilli(state.getTimer().stopTime())));
t.addCell(state.getIndex().totalRecoverFiles());
t.addCell(state.getIndex().totalFileCount());
t.addCell(new ByteSizeValue(state.getIndex().totalRecoverBytes()));
Expand Down
17 changes: 9 additions & 8 deletions server/src/main/java/org/opensearch/search/DocValueFormat.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,8 @@

import org.apache.lucene.document.InetAddressPoint;
import org.apache.lucene.util.BytesRef;
import org.opensearch.Version;
import org.opensearch.common.Numbers;
import org.opensearch.common.joda.Joda;
import org.opensearch.common.joda.JodaDateFormatter;
import org.opensearch.common.network.InetAddresses;
import org.opensearch.common.network.NetworkAddress;
import org.opensearch.common.time.DateFormatter;
Expand Down Expand Up @@ -244,13 +243,14 @@ public DateTime(DateFormatter formatter, ZoneId timeZone, DateFieldMapper.Resolu
}

public DateTime(StreamInput in) throws IOException {
String datePattern = in.readString();
this.formatter = DateFormatter.forPattern(in.readString());
this.parser = formatter.toDateMathParser();
String zoneId = in.readString();
this.timeZone = ZoneId.of(zoneId);
this.resolution = DateFieldMapper.Resolution.ofOrdinal(in.readVInt());
final boolean isJoda = in.readBoolean();
this.formatter = isJoda ? Joda.forPattern(datePattern) : DateFormatter.forPattern(datePattern);
this.parser = formatter.toDateMathParser();
if (in.getVersion().before(Version.V_3_0_0)) {
in.readBoolean(); // ignore deprecated joda
}
}

@Override
Expand All @@ -263,8 +263,9 @@ public void writeTo(StreamOutput out) throws IOException {
out.writeString(formatter.pattern());
out.writeString(timeZone.getId());
out.writeVInt(resolution.ordinal());
// in order not to loose information if the formatter is a joda we send a flag
out.writeBoolean(formatter instanceof JodaDateFormatter);// todo pg consider refactor to isJoda method..
if (out.getVersion().before(Version.V_3_0_0)) {
out.writeBoolean(false); // ignore deprecated joda flag
}
}

public DateMathParser getDateMathParser() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.opensearch.common.io.stream.BytesStreamOutput;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentOpenSearchExtension;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.index.Index;
Expand All @@ -47,6 +46,7 @@
import org.opensearch.test.OpenSearchTestCase;

import java.io.IOException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
Expand Down Expand Up @@ -80,7 +80,7 @@ public void testSerialization() throws IOException {

public void testXContent() throws IOException {
final IndexGraveyard graveyard = createRandom();
final XContentBuilder builder = JsonXContent.contentBuilder();
final XContentBuilder builder = MediaTypeRegistry.JSON.contentBuilder();
builder.startObject();
graveyard.toXContent(builder, ToXContent.EMPTY_PARAMS);
builder.endObject();
Expand All @@ -89,11 +89,13 @@ public void testXContent() throws IOException {
assertThat(
Strings.toString(MediaTypeRegistry.JSON, graveyard, false, true),
containsString(
XContentOpenSearchExtension.DEFAULT_DATE_PRINTER.print(graveyard.getTombstones().get(0).getDeleteDateInMillis())
XContentOpenSearchExtension.DEFAULT_FORMATTER.format(
Instant.ofEpochMilli(graveyard.getTombstones().get(0).getDeleteDateInMillis())
)
)
);
}
XContentParser parser = createParser(JsonXContent.jsonXContent, BytesReference.bytes(builder));
XContentParser parser = createParser(MediaTypeRegistry.JSON.xContent(), BytesReference.bytes(builder));
parser.nextToken(); // the beginning of the parser
assertThat(IndexGraveyard.fromXContent(parser), equalTo(graveyard));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@
import org.apache.lucene.util.BytesRef;
import org.apache.lucene.util.Constants;
import org.opensearch.common.geo.GeoPoint;
import org.opensearch.common.joda.Joda;
import org.opensearch.common.lucene.BytesRefs;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.common.util.PageCacheRecycler;
Expand All @@ -49,11 +48,10 @@
import org.opensearch.core.common.io.stream.StreamOutput;
import org.opensearch.core.common.io.stream.Writeable;
import org.opensearch.test.OpenSearchTestCase;
import org.joda.time.DateTime;
import org.joda.time.DateTimeZone;

import java.io.EOFException;
import java.io.IOException;
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.util.ArrayList;
Expand Down Expand Up @@ -327,10 +325,9 @@ public void testSimpleStreams() throws Exception {
out.writeOptionalBytesReference(new BytesArray("test"));
out.writeOptionalDouble(null);
out.writeOptionalDouble(1.2);
Joda.writeTimeZone(out, DateTimeZone.forID("CET"));
Joda.writeOptionalTimeZone(out, DateTimeZone.getDefault());
Joda.writeOptionalTimeZone(out, null);
out.writeGenericValue(new DateTime(123456, DateTimeZone.forID("America/Los_Angeles")));
out.writeZoneId(ZoneId.of("CET"));
out.writeOptionalZoneId(ZoneId.systemDefault());
out.writeGenericValue(ZonedDateTime.ofInstant(Instant.ofEpochMilli(123456), ZoneId.of("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 @@ -360,9 +357,8 @@ public void testSimpleStreams() throws Exception {
assertThat(in.readOptionalBytesReference(), equalTo(new BytesArray("test")));
assertNull(in.readOptionalDouble());
assertThat(in.readOptionalDouble(), closeTo(1.2, 0.0001));
assertEquals(DateTimeZone.forID("CET"), Joda.readTimeZone(in));
assertEquals(DateTimeZone.getDefault(), Joda.readOptionalTimeZone(in));
assertNull(Joda.readOptionalTimeZone(in));
assertEquals(ZoneId.of("CET"), in.readZoneId());
assertEquals(ZoneId.systemDefault(), in.readOptionalZoneId());
Object dt = in.readGenericValue();
assertThat(dt, instanceOf(ZonedDateTime.class));
ZonedDateTime zdt = (ZonedDateTime) dt;
Expand Down
Loading

0 comments on commit d8ffe60

Please sign in to comment.