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.

Serializer should still produce string-valued translog attributes, make it
write translog contents to both `translog` and `translog.<option>` attributes.

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

Signed-off-by: Maksim Strutovskii <strutovsky.m.a@gmail.com>
  • Loading branch information
Mstrutov committed Jun 7, 2023
1 parent 6ba68d8 commit f849a4e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 78 deletions.
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 java.util.List;
import java.util.function.Function;
import javax.annotation.Nullable;

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 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,16 @@ 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.translog.durability() != null) {
generator.writeKey("translog.durability");
generator.write(this.translog.durability());

}
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.translog.flushThresholdSize() != null) {
generator.writeKey("translog.flush_threshold_size");
generator.write(this.translog.flushThresholdSize());

}
}
if (this.queryStringLenient != null) {
generator.writeKey("query_string.lenient");
Expand Down Expand Up @@ -1223,15 +1203,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 +1697,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 +1903,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
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
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;

public class ParsingTests extends Assert {

Expand Down Expand Up @@ -83,17 +84,13 @@ public void testIndexSettingsSyncIntervalTimeParsing() {

try {

var indexSettings = IndexSettings.of(_1 -> _1.translogSyncInterval(Time.of(_2 -> _2.time("10s"))));
var indexSettings = IndexSettings.of(_1 -> _1.translog(Translog.of(tr -> tr.syncInterval(Time.of(t -> t.time("10s"))))));

var str = serialize(indexSettings);
assertEquals("{\"translog.sync_interval\":\"10s\"}", str);
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());
assertEquals(indexSettings.translog().syncInterval().time(), deserialized.translog().syncInterval().time());

} catch (JsonParsingException je) {
throw new JsonParsingException(je.getMessage() + " at " + je.getLocation(), je, je.getLocation());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@

package org.opensearch.client.opensearch.integTest;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

import org.junit.Test;
import org.opensearch.Version;
import org.opensearch.client.opensearch.OpenSearchAsyncClient;
Expand Down Expand Up @@ -71,17 +79,10 @@
import org.opensearch.client.opensearch.indices.GetIndicesSettingsResponse;
import org.opensearch.client.opensearch.indices.GetMappingResponse;
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;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;

public abstract class AbstractRequestIT extends OpenSearchJavaClientTestCase {

@Test
Expand Down Expand Up @@ -118,7 +119,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 @@ -153,7 +154,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

0 comments on commit f849a4e

Please sign in to comment.