From 7d026f96d8540e143710caae96a62d0728636093 Mon Sep 17 00:00:00 2001 From: Maksim Strutovskii Date: Wed, 7 Jun 2023 10:21:46 +0300 Subject: [PATCH] fix: encapsulate translog options into translog in index settings 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 --- .../opensearch/indices/IndexSettings.java | 89 ++++-------- .../opensearch/experiments/ParsingTests.java | 131 +++++++++--------- .../integTest/AbstractRequestIT.java | 13 +- 3 files changed, 99 insertions(+), 134 deletions(-) diff --git a/java-client/src/main/java/org/opensearch/client/opensearch/indices/IndexSettings.java b/java-client/src/main/java/org/opensearch/client/opensearch/indices/IndexSettings.java index d732096e1e..b06d1413f3 100644 --- a/java-client/src/main/java/org/opensearch/client/opensearch/indices/IndexSettings.java +++ b/java-client/src/main/java/org/opensearch/client/opensearch/indices/IndexSettings.java @@ -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 @@ -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; @@ -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; @@ -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; } /** @@ -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"); @@ -1223,15 +1193,6 @@ public static class Builder extends ObjectBuilderBase implements ObjectBuilder 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; } @@ -1924,8 +1893,6 @@ protected static void setupIndexSettingsDeserializer(ObjectDeserializer 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 deserialize(String serializedValue, JsonpDeserializer deserializer) { diff --git a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java index dc644ed972..c52908bbcb 100644 --- a/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java +++ b/java-client/src/test/java/org/opensearch/client/opensearch/integTest/AbstractRequestIT.java @@ -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; @@ -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)) @@ -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)) @@ -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) @@ -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 @@ -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)); @@ -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)