Skip to content

Commit

Permalink
fix: encapsulate translog options into translog in index settings
Browse files Browse the repository at this point in the history 
Now that all the translog options are stored in a separate object, we
should remove the separate fields to avoid confusion.

Public methods (getters, builder setters) should remain intact, mark
them deprecated and point them to the corresponding translog fields.

Deserializer should still recognize string-valued translog attributes, keep
the deprecated setters as field deserializers.

Serializer does not have to produce string-valued translog attributes,
as server parses `translog` attribute. Do not write translog contents to
old `translog.xxx` fields.

Signed-off-by: Maksim Strutovskii <[email protected]>
  • Loading branch information
@Mstrutov
Mstrutov committed Jun 15, 2023
1 parent 85e8e65 commit 7d026f9
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 134 deletions.
89 changes: 28 additions & 61 deletions java-client/src/main/java/org/opensearch/client/opensearch/indices/IndexSettings.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,21 @@

package org.opensearch.client.opensearch.indices;

import org.opensearch.client.opensearch._types.Time;
import jakarta.json.stream.JsonGenerator;
import org.opensearch.client.json.JsonpDeserializable;
import org.opensearch.client.json.JsonpDeserializer;
import org.opensearch.client.json.JsonpMapper;
import org.opensearch.client.json.JsonpSerializable;
import org.opensearch.client.json.ObjectBuilderDeserializer;
import org.opensearch.client.json.ObjectDeserializer;
import org.opensearch.client.opensearch._types.Time;
import org.opensearch.client.util.ApiTypeHelper;
import org.opensearch.client.util.ObjectBuilder;
import org.opensearch.client.util.ObjectBuilderBase;
import jakarta.json.stream.JsonGenerator;

import javax.annotation.Nullable;
import java.util.List;
import java.util.function.Function;
import javax.annotation.Nullable;

// typedef: indices._types.IndexSettings

Expand Down Expand Up @@ -201,15 +202,6 @@ public class IndexSettings implements JsonpSerializable {
@Nullable
private final Translog translog;

@Nullable
private final String translogDurability;

@Nullable
private final String translogFlushThresholdSize;

@Nullable
private final Time translogSyncInterval;

@Nullable
private final Boolean queryStringLenient;

Expand Down Expand Up @@ -281,9 +273,6 @@ private IndexSettings(Builder builder) {
this.format = builder.format;
this.maxSlicesPerScroll = builder.maxSlicesPerScroll;
this.translog = builder.translog;
this.translogDurability = builder.translogDurability;
this.translogFlushThresholdSize = builder.translogFlushThresholdSize;
this.translogSyncInterval = builder.translogSyncInterval;
this.queryStringLenient = builder.queryStringLenient;
this.priority = builder.priority;
this.topMetricsMaxSize = builder.topMetricsMaxSize;
Expand Down Expand Up @@ -690,26 +679,22 @@ public final Translog translog() {

/**
* API name: {@code translog.durability}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
@Nullable
public final String translogDurability() {
return this.translogDurability;
return this.translog != null ? this.translog.durability() : null;
}

/**
* API name: {@code translog.flush_threshold_size}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
@Nullable
public final String translogFlushThresholdSize() {
return this.translogFlushThresholdSize;
}

/**
* API name: {@code translog.sync_interval}
*/
@Nullable
public final Time translogSyncInterval() {
return this.translogSyncInterval;
return this.translog != null ? this.translog.flushThresholdSize() : null;
}

/**
Expand Down Expand Up @@ -1020,21 +1005,6 @@ protected void serializeInternal(JsonGenerator generator, JsonpMapper mapper) {
generator.writeKey("translog");
this.translog.serialize(generator, mapper);

}
if (this.translogDurability != null) {
generator.writeKey("translog.durability");
generator.write(this.translogDurability);

}
if (this.translogFlushThresholdSize != null) {
generator.writeKey("translog.flush_threshold_size");
generator.write(this.translogFlushThresholdSize);

}
if (this.translogSyncInterval != null) {
generator.writeKey("translog.sync_interval");
this.translogSyncInterval.serialize(generator, mapper);

}
if (this.queryStringLenient != null) {
generator.writeKey("query_string.lenient");
Expand Down Expand Up @@ -1223,15 +1193,6 @@ public static class Builder extends ObjectBuilderBase implements ObjectBuilder<I
@Nullable
private Translog translog;

@Nullable
private String translogDurability;

@Nullable
private String translogFlushThresholdSize;

@Nullable
private Time translogSyncInterval;

@Nullable
private Boolean queryStringLenient;

Expand Down Expand Up @@ -1726,25 +1687,33 @@ public final Builder translog(@Nullable Translog value) {

/**
* API name: {@code translog.durability}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
public final Builder translogDurability(@Nullable String value) {
this.translogDurability = value;
if (translog == null) {
translog = Translog.of(b -> b.durability(value));
} else {
translog = Translog.of(b -> b.durability(value)
.flushThresholdSize(translog.flushThresholdSize())
.syncInterval(translog.syncInterval()));
}
return this;
}

/**
* API name: {@code translog.flush_threshold_size}
* @deprecated use {@link #translog()} instead
*/
@Deprecated
public final Builder translogFlushThresholdSize(@Nullable String value) {
this.translogFlushThresholdSize = value;
return this;
}

/**
* API name: {@code translog.sync_interval}
*/
public final Builder translogSyncInterval(@Nullable Time value) {
this.translogSyncInterval = value;
if (translog == null) {
translog = Translog.of(b -> b.flushThresholdSize(value));
} else {
translog = Translog.of(b -> b.flushThresholdSize(value)
.durability(translog.durability())
.syncInterval(translog.syncInterval()));
}
return this;
}

Expand Down Expand Up @@ -1924,8 +1893,6 @@ protected static void setupIndexSettingsDeserializer(ObjectDeserializer<IndexSet
"index.translog.durability");
op.add(Builder::translogFlushThresholdSize, JsonpDeserializer.stringDeserializer(),
"translog.flush_threshold_size", "index.translog.flush_threshold_size");
op.add(Builder::translogSyncInterval, Time._DESERIALIZER,
"translog.sync_interval", "index.translog.sync_interval");
op.add(Builder::queryStringLenient, JsonpDeserializer.booleanDeserializer(), "query_string.lenient",
"index.query_string.lenient");
op.add(Builder::priority, JsonpDeserializer.stringDeserializer(), "priority", "index.priority");
Expand Down
131 changes: 64 additions & 67 deletions java-client/src/test/java/org/opensearch/client/opensearch/experiments/ParsingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
package org.opensearch.client.opensearch.experiments;

import jakarta.json.spi.JsonProvider;
import jakarta.json.stream.JsonParsingException;
import org.junit.Assert;
import org.junit.Test;
import org.opensearch.client.json.JsonpDeserializer;
Expand All @@ -43,6 +42,11 @@
import org.opensearch.client.opensearch.experiments.api.FooRequest;
import org.opensearch.client.opensearch.indices.IndexSettings;
import org.opensearch.client.opensearch.indices.IndexSettingsMapping;
import org.opensearch.client.opensearch.indices.Translog;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.nio.charset.StandardCharsets;

import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
Expand All @@ -51,80 +55,73 @@
public class ParsingTests extends Assert {

@Test
public void testFoo() throws Exception {

try {

FooRequest foo = FooRequest.builder()
.name("z")
.value(1)
.indices("a", "b", "c")
.bar(b -> b
.name("Raise the bar")
)
.build();

String str = serialize(foo);
assertEquals("{\"name\":\"z\",\"value\":1,\"indices\":[\"a\",\"b\",\"c\"],\"bar\":{\"name\":\"Raise the bar\"}}", str);

FooRequest foo2 = deserialize(str, FooRequest.parser());
assertEquals(foo.name(), foo2.name());
assertEquals(foo.value(), foo2.value());
assertNull(foo2.size());
assertEquals(foo.indices(), foo2.indices());
assertEquals("Raise the bar", foo.bar().name());
} catch (JsonParsingException je) {
throw new JsonParsingException(je.getMessage() + " at " + je.getLocation(), je, je.getLocation());
}
public void testFoo() {

FooRequest foo = FooRequest.builder()
.name("z")
.value(1)
.indices("a", "b", "c")
.bar(b -> b
.name("Raise the bar")
)
.build();

String str = serialize(foo);
assertEquals("{\"name\":\"z\",\"value\":1,\"indices\":[\"a\",\"b\",\"c\"],\"bar\":{\"name\":\"Raise the bar\"}}", str);

FooRequest foo2 = deserialize(str, FooRequest.parser());
assertEquals(foo.name(), foo2.name());
assertEquals(foo.value(), foo2.value());
assertNull(foo2.size());
assertEquals(foo.indices(), foo2.indices());
assertEquals("Raise the bar", foo.bar().name());
}

@Test
public void testIndexSettingsSyncIntervalTimeParsing() {

try {

var indexSettings = IndexSettings.of(_1 -> _1.translogSyncInterval(Time.of(_2 -> _2.time("10s"))));

var str = serialize(indexSettings);
assertEquals("{\"translog.sync_interval\":\"10s\"}", str);

IndexSettings deserialized = deserialize(str, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translogSyncInterval().time(), deserialized.translogSyncInterval().time());

var responseStr = "{\"translog\":{\"sync_interval\":\"10s\"}}";
IndexSettings responseDeserialized = deserialize(responseStr, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translogSyncInterval().time(), responseDeserialized.translog().syncInterval().time());

} catch (JsonParsingException je) {
throw new JsonParsingException(je.getMessage() + " at " + je.getLocation(), je, je.getLocation());
}
public void testIndexSettingsTranslogOptionsParsing() {

var indexSettings = IndexSettings.of(_1 -> _1.translog(Translog.of(tr -> tr
.syncInterval(Time.of(t -> t.time("10s")))
.durability("async")
.flushThresholdSize("256mb"))));

var str = serialize(indexSettings);
assertEquals("{\"translog\":{\"durability\":\"async\",\"flush_threshold_size\":\"256mb\"," +
"\"sync_interval\":\"10s\"}}", str);

IndexSettings deserialized = deserialize(str, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translog().syncInterval().time(), deserialized.translog().syncInterval().time());
assertEquals(indexSettings.translog().durability(), deserialized.translog().durability());
assertEquals(indexSettings.translog().flushThresholdSize(), deserialized.translog().flushThresholdSize());

var deprecatedForm = "{\"translog\":{\"sync_interval\":\"10s\"},\"translog.durability\":\"async\",\"translog" +
".flush_threshold_size\":\"256mb\"}";
IndexSettings deprecatedDeserialized = deserialize(deprecatedForm, IndexSettings._DESERIALIZER);
assertEquals(indexSettings.translog().syncInterval().time(), deprecatedDeserialized.translog().syncInterval().time());
assertEquals(indexSettings.translog().durability(), deprecatedDeserialized.translog().durability());
assertEquals(indexSettings.translog().flushThresholdSize(), deprecatedDeserialized.translog().flushThresholdSize());
}

@Test
public void testIndexSettingsMappingParsing() {

try {

var mapping = IndexSettingsMapping.of(b -> b
.totalFields(d -> d.limit(1L))
.depth(d -> d.limit(2L))
.nestedFields(d -> d.limit(3L))
.nestedObjects(d -> d.limit(4L))
.fieldNameLength(d -> d.limit(5L)));

var str = serialize(mapping);
assertEquals("{\"total_fields\":{\"limit\":1},\"depth\":{\"limit\":2},\"nested_fields\":{\"limit\":3}," +
"\"nested_objects\":{\"limit\":4},\"field_name_length\":{\"limit\":5}}", str);

var deserialized = deserialize(str, IndexSettingsMapping._DESERIALIZER);
assertEquals(mapping.totalFields().limit(), deserialized.totalFields().limit());
assertEquals(mapping.depth().limit(), deserialized.depth().limit());
assertEquals(mapping.nestedFields().limit(), deserialized.nestedFields().limit());
assertEquals(mapping.nestedObjects().limit(), deserialized.nestedObjects().limit());
assertEquals(mapping.fieldNameLength().limit(), deserialized.fieldNameLength().limit());
} catch (JsonParsingException je) {
throw new JsonParsingException(je.getMessage() + " at " + je.getLocation(), je, je.getLocation());
}
var mapping = IndexSettingsMapping.of(b -> b
.totalFields(d -> d.limit(1L))
.depth(d -> d.limit(2L))
.nestedFields(d -> d.limit(3L))
.nestedObjects(d -> d.limit(4L))
.fieldNameLength(d -> d.limit(5L)));

var str = serialize(mapping);
assertEquals("{\"total_fields\":{\"limit\":1},\"depth\":{\"limit\":2},\"nested_fields\":{\"limit\":3}," +
"\"nested_objects\":{\"limit\":4},\"field_name_length\":{\"limit\":5}}", str);

var deserialized = deserialize(str, IndexSettingsMapping._DESERIALIZER);
assertEquals(mapping.totalFields().limit(), deserialized.totalFields().limit());
assertEquals(mapping.depth().limit(), deserialized.depth().limit());
assertEquals(mapping.nestedFields().limit(), deserialized.nestedFields().limit());
assertEquals(mapping.nestedObjects().limit(), deserialized.nestedObjects().limit());
assertEquals(mapping.fieldNameLength().limit(), deserialized.fieldNameLength().limit());
}

private <T extends JsonpSerializable> T deserialize(String serializedValue, JsonpDeserializer<T> deserializer) {
Expand Down
13 changes: 7 additions & 6 deletions java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import org.opensearch.client.opensearch.indices.IndexSettings;
import org.opensearch.client.opensearch.indices.IndexSettingsAnalysis;
import org.opensearch.client.opensearch.indices.IndexState;
import org.opensearch.client.opensearch.indices.Translog;
import org.opensearch.client.opensearch.model.ModelTestCase;
import org.opensearch.client.transport.endpoints.BooleanResponse;

Expand Down Expand Up @@ -130,7 +131,7 @@ public void testIndexSettings() throws Exception {
var createResponse = javaClient().indices()
.create(r -> r.index("test-settings")
.settings(s -> s
.translogSyncInterval(Time.of(t -> t.time("10s")))
.translog(Translog.of(tl -> tl.syncInterval(Time.of(t -> t.time("10s")))))
.mapping(m -> m
.fieldNameLength(f -> f.limit(300L))
.totalFields(f -> f.limit(30L))
Expand Down Expand Up @@ -165,7 +166,7 @@ public void testIndexSettings() throws Exception {
var putSettingsResponse = javaClient().indices().putSettings(r -> r
.index("test-settings")
.settings(s -> s
.translogSyncInterval(Time.of(t -> t.time("5s")))
.translog(Translog.of(tl -> tl.syncInterval(Time.of(t -> t.time("5s")))))
.mapping(m -> m
.fieldNameLength(f -> f.limit(400L))
.totalFields(f -> f.limit(130L))
Expand Down Expand Up @@ -565,7 +566,7 @@ public void testDefaultIndexSettings() throws IOException {
public void testCompletionSuggesterFailure() throws IOException {

String index = "test-completion-suggester-failure";


Property intValueProp = new Property.Builder()
.long_(v -> v)
Expand Down Expand Up @@ -631,7 +632,7 @@ public void testPit() throws IOException {
}
assumeTrue("The PIT is supported in OpenSearch 2.4.0 and later",
Version.fromString(version).onOrAfter(Version.fromString("2.4.0")));

String index = "test-point-in-time";

javaClient().indices().create(c -> c
Expand Down Expand Up @@ -744,7 +745,7 @@ public void testCompletionSuggester() throws IOException {
public void testTermSuggester() throws IOException {

String index = "test-term-suggester";

// term suggester does not require a special mapping
javaClient().indices().create(c -> c
.index(index));
Expand Down Expand Up @@ -826,7 +827,7 @@ public void testPhraseSuggester() throws IOException {
.text(new TextProperty.Builder().analyzer("trigram").build())
.build()).build())
.build()).build();


javaClient().indices().create(c -> c
.index(index)
Expand Down

0 comments on commit 7d026f9

Please sign in to comment.