Skip to content

Commit

Permalink
[Remove] Joda from Serialization
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>
  • Loading branch information
nknize committed Aug 15, 2023
1 parent 1342578 commit c9fefdf
Show file tree
Hide file tree
Showing 14 changed files with 79 additions and 196 deletions.
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,10 @@ tasks.register("verifyVersions") {
* after the backport of the backcompat code is complete.
*/

boolean bwc_tests_enabled = true
boolean bwc_tests_enabled = false

/* place an issue link here when committing bwc changes */
String bwc_tests_disabled_issue = ""
String bwc_tests_disabled_issue = "https://github.com/opensearch-project/OpenSearch/issues/9349"

/* there's no existing MacOS release, therefore disable bcw tests */
if (Os.isFamily(Os.FAMILY_MAC)) {
Expand Down
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 @@ -41,14 +41,14 @@
import org.opensearch.core.common.bytes.BytesArray;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.common.settings.SecureString;
import org.opensearch.script.JodaCompatibleZonedDateTime;
import org.opensearch.test.OpenSearchTestCase;

import java.io.ByteArrayInputStream;
import java.io.EOFException;
import java.io.IOException;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -400,10 +400,10 @@ public void testOptionalInstantSerialization() throws IOException {
}
}

public void testJodaDateTimeSerialization() throws IOException {
public void testJavaDateTimeSerialization() throws IOException {
final BytesStreamOutput output = new BytesStreamOutput();
long millis = randomIntBetween(0, Integer.MAX_VALUE);
JodaCompatibleZonedDateTime time = new JodaCompatibleZonedDateTime(Instant.ofEpochMilli(millis), ZoneOffset.ofHours(-7));
ZonedDateTime time = ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.ofHours(-7));
output.writeGenericValue(time);

final BytesReference bytesReference = output.bytes();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,10 @@

package org.opensearch.common.io.stream;

import org.joda.time.DateTimeZone;
import org.joda.time.ReadableInstant;
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.opensearch.script.JodaCompatibleZonedDateTime;

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

/**
* This utility class registers generic types for streaming over the wire using
Expand Down Expand Up @@ -47,24 +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 JodaCompatibleZonedDateTime} */
WriteableRegistry.registerWriter(JodaCompatibleZonedDateTime.class, (o, v) -> {
// write the joda compatibility datetime as joda datetime
o.writeByte((byte) 13);
final JodaCompatibleZonedDateTime zonedDateTime = (JodaCompatibleZonedDateTime) v;
String zoneId = zonedDateTime.getZonedDateTime().getZone().getId();
// joda does not understand "Z" for utc, so we must special case
o.writeString(zoneId.equals("Z") ? DateTimeZone.UTC.getID() : zoneId);
o.writeLong(zonedDateTime.toInstant().toEpochMilli());
});
/** {@link GeoPoint} */
WriteableRegistry.registerWriter(GeoPoint.class, (o, v) -> {
o.writeByte((byte) 22);
Expand All @@ -78,12 +53,6 @@ private static void registerWriters() {
* NOTE: see {@code StreamOutput#WRITERS} for all registered ordinals
*/
private static void registerReaders() {
/** {@link JodaCompatibleZonedDateTime */
WriteableRegistry.registerReader(Byte.valueOf((byte) 13), (i) -> {
final ZoneId zoneId = DateUtils.dateTimeZoneToZoneId(DateTimeZone.forID(i.readString()));
long millis = i.readLong();
return new JodaCompatibleZonedDateTime(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 @@ -33,25 +33,17 @@
package org.opensearch.common.xcontent;

import org.apache.lucene.util.BytesRef;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.common.time.DateFormatter;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.common.unit.TimeValue;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentBuilderExtension;
import org.opensearch.script.JodaCompatibleZonedDateTime;
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 @@ -80,7 +72,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 @@ -91,11 +82,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 @@ -143,14 +129,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 @@ -41,12 +41,12 @@
import org.opensearch.common.geo.GeoUtils;
import org.opensearch.common.time.DateUtils;
import org.opensearch.geometry.utils.Geohash;
import org.opensearch.script.JodaCompatibleZonedDateTime;

import java.io.IOException;
import java.math.BigInteger;
import java.time.Instant;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.util.AbstractList;
import java.util.Arrays;
import java.util.Comparator;
Expand Down Expand Up @@ -159,15 +159,15 @@ public int size() {
*
* @opensearch.internal
*/
public static final class Dates extends ScriptDocValues<JodaCompatibleZonedDateTime> {
public static final class Dates extends ScriptDocValues<ZonedDateTime> {

private final SortedNumericDocValues in;
private final boolean isNanos;

/**
* Values wrapped in {@link java.time.ZonedDateTime} objects.
*/
private JodaCompatibleZonedDateTime[] dates;
private ZonedDateTime[] dates;
private int count;

public Dates(SortedNumericDocValues in, boolean isNanos) {
Expand All @@ -179,12 +179,12 @@ public Dates(SortedNumericDocValues in, boolean isNanos) {
* Fetch the first field value or 0 millis after epoch if there are no
* in.
*/
public JodaCompatibleZonedDateTime getValue() {
public ZonedDateTime getValue() {
return get(0);
}

@Override
public JodaCompatibleZonedDateTime get(int index) {
public ZonedDateTime get(int index) {
if (count == 0) {
throw new IllegalStateException(
"A document doesn't have a value for a field! "
Expand Down Expand Up @@ -223,13 +223,13 @@ void refreshArray() throws IOException {
}
if (dates == null || count > dates.length) {
// Happens for the document. We delay allocating dates so we can allocate it with a reasonable size.
dates = new JodaCompatibleZonedDateTime[count];
dates = new ZonedDateTime[count];
}
for (int i = 0; i < count; ++i) {
if (isNanos) {
dates[i] = new JodaCompatibleZonedDateTime(DateUtils.toInstant(in.nextValue()), ZoneOffset.UTC);
dates[i] = ZonedDateTime.ofInstant(DateUtils.toInstant(in.nextValue()), ZoneOffset.UTC);
} else {
dates[i] = new JodaCompatibleZonedDateTime(Instant.ofEpochMilli(in.nextValue()), ZoneOffset.UTC);
dates[i] = ZonedDateTime.ofInstant(Instant.ofEpochMilli(in.nextValue()), ZoneOffset.UTC);
}
}
}
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,12 +34,11 @@

import org.apache.lucene.document.InetAddressPoint;
import org.apache.lucene.util.BytesRef;
import org.opensearch.Version;
import org.opensearch.common.Numbers;
import org.opensearch.core.common.io.stream.NamedWriteable;
import org.opensearch.core.common.io.stream.StreamInput;
import org.opensearch.core.common.io.stream.StreamOutput;
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_2_10_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_2_10_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 @@ -33,20 +33,20 @@
package org.opensearch.cluster.metadata;

import org.opensearch.common.UUIDs;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.common.io.stream.BytesStreamOutput;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.xcontent.XContentOpenSearchExtension;
import org.opensearch.core.common.Strings;
import org.opensearch.core.common.bytes.BytesReference;
import org.opensearch.core.xcontent.MediaTypeRegistry;
import org.opensearch.core.xcontent.ToXContent;
import org.opensearch.core.xcontent.XContentBuilder;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.common.xcontent.json.JsonXContent;
import org.opensearch.core.index.Index;
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
Loading

0 comments on commit c9fefdf

Please sign in to comment.