[Refactor] Remaining Strings utility methods to core library (#9103)

This commit refactors the remaining Strings# utility methods from the duplicate :server:Strings class to the :libs:opensearch-core:Strings class. The CollectionUtils class (used by the Strings utility) is also refactored to the :libs:opensearch-common library and the unnecessary Strings.toString(XContentBuilder... static method is finally refactored as an instance method in XContentBuilder. The Strings class is also cleaned up for clarity. Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
opensearch-project · Aug 5, 2023 · 4c59810 · 4c59810
1 parent bb78930
commit 4c59810
Show file tree

Hide file tree

Showing 350 changed files with 4,002 additions and 4,410 deletions.
diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/indices/CreateIndexRequest.java b/client/rest-high-level/src/main/java/org/opensearch/client/indices/CreateIndexRequest.java
@@ -38,7 +38,6 @@
 import org.opensearch.action.support.ActiveShardCount;
 import org.opensearch.client.TimedRequest;
 import org.opensearch.client.Validatable;
-import org.opensearch.common.Strings;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.common.settings.Settings;
@@ -135,7 +134,7 @@ public CreateIndexRequest settings(String source, MediaType mediaType) {
      * Allows to set the settings using a json builder.
      */
     public CreateIndexRequest settings(XContentBuilder builder) {
-        settings(Strings.toString(builder), builder.contentType());
+        settings(builder.toString(), builder.contentType());
         return this;
     }
 

diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/slm/SnapshotLifecyclePolicy.java b/client/rest-high-level/src/main/java/org/opensearch/client/slm/SnapshotLifecyclePolicy.java
@@ -33,9 +33,9 @@
 package org.opensearch.client.slm;
 
 import org.opensearch.common.Nullable;
-import org.opensearch.common.Strings;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.ParseField;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ConstructingObjectParser;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;

diff --git a/...t-high-level/src/main/java/org/opensearch/client/slm/SnapshotLifecyclePolicyMetadata.java b/...t-high-level/src/main/java/org/opensearch/client/slm/SnapshotLifecyclePolicyMetadata.java
@@ -33,9 +33,9 @@
 package org.opensearch.client.slm;
 
 import org.opensearch.common.Nullable;
-import org.opensearch.common.Strings;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.ParseField;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ConstructingObjectParser;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;

diff --git a/client/rest-high-level/src/main/java/org/opensearch/client/slm/SnapshotLifecycleStats.java b/client/rest-high-level/src/main/java/org/opensearch/client/slm/SnapshotLifecycleStats.java
@@ -32,10 +32,10 @@
 
 package org.opensearch.client.slm;
 
-import org.opensearch.common.Strings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.ParseField;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ConstructingObjectParser;
 import org.opensearch.core.xcontent.ToXContentFragment;
 import org.opensearch.core.xcontent.ToXContentObject;

diff --git a/...st-high-level/src/main/java/org/opensearch/client/slm/SnapshotRetentionConfiguration.java b/...st-high-level/src/main/java/org/opensearch/client/slm/SnapshotRetentionConfiguration.java
@@ -33,10 +33,10 @@
 package org.opensearch.client.slm;
 
 import org.opensearch.common.Nullable;
-import org.opensearch.common.Strings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.xcontent.XContentType;
 import org.opensearch.core.ParseField;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ConstructingObjectParser;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;

diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/BulkProcessorIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/BulkProcessorIT.java
@@ -42,7 +42,6 @@
 import org.opensearch.action.get.MultiGetResponse;
 import org.opensearch.action.index.IndexRequest;
 import org.opensearch.action.search.SearchRequest;
-import org.opensearch.common.Strings;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.unit.ByteSizeUnit;
 import org.opensearch.core.common.unit.ByteSizeValue;
@@ -423,7 +422,7 @@ private static BytesArray bytesBulkRequest(String localIndex, int id) throws IOE
 
         XContentBuilder source = jsonBuilder().startObject().field("field", randomRealisticUnicodeOfLengthBetween(1, 30)).endObject();
 
-        String request = Strings.toString(action) + "\n" + Strings.toString(source) + "\n";
+        String request = action + "\n" + source + "\n";
         return new BytesArray(request);
     }
 

diff --git a/client/rest-high-level/src/test/java/org/opensearch/client/SearchIT.java b/client/rest-high-level/src/test/java/org/opensearch/client/SearchIT.java
@@ -52,7 +52,6 @@
 import org.opensearch.action.search.SearchScrollRequest;
 import org.opensearch.client.core.CountRequest;
 import org.opensearch.client.core.CountResponse;
-import org.opensearch.common.Strings;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.core.xcontent.XContentBuilder;
@@ -769,7 +768,7 @@ public void testSearchScroll() throws Exception {
         for (int i = 0; i < 100; i++) {
             XContentBuilder builder = jsonBuilder().startObject().field("field", i).endObject();
             Request doc = new Request(HttpPut.METHOD_NAME, "/test/_doc/" + Integer.toString(i));
-            doc.setJsonEntity(Strings.toString(builder));
+            doc.setJsonEntity(builder.toString());
             client().performRequest(doc);
         }
         client().performRequest(new Request(HttpPost.METHOD_NAME, "/test/_refresh"));
@@ -837,7 +836,7 @@ public void testSearchWithPit() throws Exception {
         for (int i = 0; i < 100; i++) {
             XContentBuilder builder = jsonBuilder().startObject().field("field", i).endObject();
             Request doc = new Request(HttpPut.METHOD_NAME, "/test/_doc/" + Integer.toString(i));
-            doc.setJsonEntity(Strings.toString(builder));
+            doc.setJsonEntity(builder.toString());
             client().performRequest(doc);
         }
         client().performRequest(new Request(HttpPost.METHOD_NAME, "/test/_refresh"));

diff --git a/...est-high-level/src/test/java/org/opensearch/client/documentation/CRUDDocumentationIT.java b/...est-high-level/src/test/java/org/opensearch/client/documentation/CRUDDocumentationIT.java
@@ -74,7 +74,7 @@
 import org.opensearch.client.core.TermVectorsResponse;
 import org.opensearch.client.indices.CreateIndexRequest;
 import org.opensearch.client.indices.CreateIndexResponse;
-import org.opensearch.common.Strings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.core.common.unit.ByteSizeUnit;
@@ -298,15 +298,14 @@ public void testUpdate() throws Exception {
 
             Request request = new Request("POST", "/_scripts/increment-field");
             request.setJsonEntity(
-                Strings.toString(
-                    JsonXContent.contentBuilder()
-                        .startObject()
-                        .startObject("script")
-                        .field("lang", "painless")
-                        .field("source", "ctx._source.field += params.count")
-                        .endObject()
-                        .endObject()
-                )
+                JsonXContent.contentBuilder()
+                    .startObject()
+                    .startObject("script")
+                    .field("lang", "painless")
+                    .field("source", "ctx._source.field += params.count")
+                    .endObject()
+                    .endObject()
+                    .toString()
             );
             Response response = client().performRequest(request);
             assertEquals(RestStatus.OK.getStatus(), response.getStatusLine().getStatusCode());

diff --git a/...ution/archives/integ-test-zip/src/test/java/org/opensearch/test/rest/NodeRestUsageIT.java b/...ution/archives/integ-test-zip/src/test/java/org/opensearch/test/rest/NodeRestUsageIT.java
@@ -35,8 +35,8 @@
 import org.opensearch.client.Request;
 import org.opensearch.client.Response;
 import org.opensearch.client.ResponseException;
-import org.opensearch.common.Strings;
 import org.opensearch.common.xcontent.XContentType;
+import org.opensearch.core.common.Strings;
 import org.opensearch.search.aggregations.AggregationBuilders;
 import org.opensearch.search.builder.SearchSourceBuilder;
 

diff --git a/libs/core/src/main/java/org/opensearch/core/common/Strings.java b/libs/core/src/main/java/org/opensearch/core/common/Strings.java
@@ -9,11 +9,17 @@
 package org.opensearch.core.common;
 
 import org.apache.lucene.util.BytesRefBuilder;
+import org.opensearch.ExceptionsHelper;
+import org.opensearch.OpenSearchException;
 import org.opensearch.common.Nullable;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.core.common.util.CollectionUtils;
+import org.opensearch.core.xcontent.MediaType;
+import org.opensearch.core.xcontent.ToXContent;
+import org.opensearch.core.xcontent.XContentBuilder;
 
 import java.io.BufferedReader;
+import java.io.IOException;
 import java.io.StringReader;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -680,6 +686,77 @@ public static boolean isAllOrWildcard(String data) {
         return "_all".equals(data) || "*".equals(data);
     }
 
+    /**
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
+     * Wraps the output into an anonymous object if needed. The content is not pretty-printed
+     * nor human readable.
+     */
+    public static String toString(MediaType mediaType, ToXContent toXContent) {
+        return toString(mediaType, toXContent, false, false);
+    }
+
+    /**
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
+     * Wraps the output into an anonymous object if needed.
+     * Allows to configure the params.
+     * The content is not pretty-printed nor human readable.
+     */
+    public static String toString(MediaType mediaType, ToXContent toXContent, ToXContent.Params params) {
+        return toString(mediaType, toXContent, params, false, false);
+    }
+
+    /**
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
+     * Wraps the output into an anonymous object if needed. Allows to control whether the outputted
+     * json needs to be pretty printed and human readable.
+     *
+     */
+    public static String toString(MediaType mediaType, ToXContent toXContent, boolean pretty, boolean human) {
+        return toString(mediaType, toXContent, ToXContent.EMPTY_PARAMS, pretty, human);
+    }
+
+    /**
+     * Return a {@link String} that is the json representation of the provided {@link ToXContent}.
+     * Wraps the output into an anonymous object if needed.
+     * Allows to configure the params.
+     * Allows to control whether the outputted json needs to be pretty printed and human readable.
+     */
+    private static String toString(MediaType mediaType, ToXContent toXContent, ToXContent.Params params, boolean pretty, boolean human) {
+        try {
+            XContentBuilder builder = createBuilder(mediaType, pretty, human);
+            if (toXContent.isFragment()) {
+                builder.startObject();
+            }
+            toXContent.toXContent(builder, params);
+            if (toXContent.isFragment()) {
+                builder.endObject();
+            }
+            return builder.toString();
+        } catch (IOException e) {
+            try {
+                XContentBuilder builder = createBuilder(mediaType, pretty, human);
+                builder.startObject();
+                builder.field("error", "error building toString out of XContent: " + e.getMessage());
+                builder.field("stack_trace", ExceptionsHelper.stackTrace(e));
+                builder.endObject();
+                return builder.toString();
+            } catch (IOException e2) {
+                throw new OpenSearchException("cannot generate error message for deserialization", e);
+            }
+        }
+    }
+
+    private static XContentBuilder createBuilder(MediaType mediaType, boolean pretty, boolean human) throws IOException {
+        XContentBuilder builder = XContentBuilder.builder(mediaType.xContent());
+        if (pretty) {
+            builder.prettyPrint();
+        }
+        if (human) {
+            builder.humanReadable(true);
+        }
+        return builder;
+    }
+
     /**
      * Truncates string to a length less than length. Backtracks to throw out
      * high surrogates.

diff --git a/libs/core/src/main/java/org/opensearch/core/xcontent/XContentBuilder.java b/libs/core/src/main/java/org/opensearch/core/xcontent/XContentBuilder.java
@@ -32,6 +32,8 @@
 
 package org.opensearch.core.xcontent;
 
+import org.opensearch.core.common.bytes.BytesReference;
+
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.Flushable;
@@ -151,6 +153,14 @@ public static XContentBuilder builder(XContent xContent, Set<String> includes, S
         DATE_TRANSFORMERS = Collections.unmodifiableMap(dateTransformers);
     }
 
+    /**
+     * Returns a string representation of the builder (only applicable for text based xcontent).
+     */
+    @Override
+    public String toString() {
+        return BytesReference.bytes(this).utf8ToString();
+    }
+
     /**
      * The writer interface for the serializable content builder
      *

diff --git a/...st/java/org/opensearch/core/action/support/DefaultShardOperationFailedExceptionTests.java b/...st/java/org/opensearch/core/action/support/DefaultShardOperationFailedExceptionTests.java
@@ -39,7 +39,7 @@
 import org.apache.lucene.store.LockObtainFailedException;
 import org.opensearch.OpenSearchException;
 import org.opensearch.action.support.broadcast.BroadcastShardOperationFailedException;
-import org.opensearch.common.Strings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.common.io.stream.BytesStreamOutput;
 import org.opensearch.core.common.io.stream.StreamInput;

diff --git a/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java b/libs/core/src/test/java/org/opensearch/core/common/StringsTests.java
@@ -9,8 +9,14 @@
 package org.opensearch.core.common;
 
 import org.opensearch.common.util.set.Sets;
+import org.opensearch.core.xcontent.MediaTypeRegistry;
+import org.opensearch.core.xcontent.ToXContent;
+import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.test.OpenSearchTestCase;
 
+import java.util.Collections;
+
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 
 /** tests for Strings utility class */
@@ -63,4 +69,49 @@ public void testSplitStringToSet() {
         assertEquals(Strings.tokenizeByCommaToSet("   aa   "), Sets.newHashSet("aa"));
         assertEquals(Strings.tokenizeByCommaToSet("   "), Sets.newHashSet());
     }
+
+    public void testToStringToXContent() {
+        final ToXContent toXContent;
+        final boolean error;
+        if (randomBoolean()) {
+            if (randomBoolean()) {
+                error = false;
+                toXContent = (builder, params) -> builder.field("ok", "here").field("catastrophe", "");
+            } else {
+                error = true;
+                toXContent = (builder, params) -> builder.startObject().field("ok", "here").field("catastrophe", "").endObject();
+            }
+        } else {
+            if (randomBoolean()) {
+                error = false;
+                toXContent = (ToXContentObject) (builder, params) -> builder.startObject()
+                    .field("ok", "here")
+                    .field("catastrophe", "")
+                    .endObject();
+            } else {
+                error = true;
+                toXContent = (ToXContentObject) (builder, params) -> builder.field("ok", "here").field("catastrophe", "");
+            }
+        }
+
+        String toString = Strings.toString(MediaTypeRegistry.JSON, toXContent);
+        if (error) {
+            assertThat(toString, containsString("\"error\":\"error building toString out of XContent:"));
+            assertThat(toString, containsString("\"stack_trace\":"));
+        } else {
+            assertThat(toString, containsString("\"ok\":\"here\""));
+            assertThat(toString, containsString("\"catastrophe\":\"\""));
+        }
+    }
+
+    public void testToStringToXContentWithOrWithoutParams() {
+        ToXContent toXContent = (builder, params) -> builder.field("color_from_param", params.param("color", "red"));
+        // Rely on the default value of "color" param when params are not passed
+        assertThat(Strings.toString(MediaTypeRegistry.JSON, toXContent), containsString("\"color_from_param\":\"red\""));
+        // Pass "color" param explicitly
+        assertThat(
+            Strings.toString(MediaTypeRegistry.JSON, toXContent, new ToXContent.MapParams(Collections.singletonMap("color", "blue"))),
+            containsString("\"color_from_param\":\"blue\"")
+        );
+    }
 }
diff --git a/libs/x-content/src/test/java/org/opensearch/common/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/opensearch/common/xcontent/ObjectParserTests.java
@@ -33,7 +33,6 @@
 
 import org.opensearch.common.CheckedFunction;
 import org.opensearch.core.ParseField;
-import org.opensearch.common.Strings;
 import org.opensearch.core.xcontent.XContentParserUtils;
 import org.opensearch.core.xcontent.ObjectParser;
 import org.opensearch.core.xcontent.ObjectParser.NamedObjectParser;
@@ -449,7 +448,7 @@ public void testAllVariants() throws IOException {
         }
         builder.field("string_or_null", nullValue ? null : "5");
         builder.endObject();
-        XContentParser parser = createParser(JsonXContent.jsonXContent, Strings.toString(builder));
+        XContentParser parser = createParser(JsonXContent.jsonXContent, builder.toString());
         class TestStruct {
             int int_field;
             int nullableIntField;

diff --git a/libs/x-content/src/test/java/org/opensearch/common/xcontent/SimpleStruct.java b/libs/x-content/src/test/java/org/opensearch/common/xcontent/SimpleStruct.java
@@ -33,7 +33,7 @@
 package org.opensearch.common.xcontent;
 
 import org.opensearch.core.ParseField;
-import org.opensearch.common.Strings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.ConstructingObjectParser;
 import org.opensearch.core.xcontent.ToXContentObject;
 import org.opensearch.core.xcontent.XContentBuilder;