[Refactor] Strings methods other than MediaType (#9126)

This refactors all of the remaining utility methods (except those that convert MediaType and XContentBuilder to String) from the :server to :libs:opensearch-core library. This commit is to keep the Strings refactor surface area lean in preparation for cloud native and serverless refactoring. Signed-off-by: Nicholas Walter Knize <[email protected]>
opensearch-project · Aug 5, 2023 · 1d28fac · 1d28fac
1 parent 5bb7fa3
commit 1d28fac
Show file tree

Hide file tree

Showing 12 changed files with 244 additions and 245 deletions.
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
diff --git a/...main/java/org/opensearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java b/...main/java/org/opensearch/action/admin/cluster/snapshots/create/CreateSnapshotRequest.java
@@ -74,7 +74,7 @@
  * <li>must not contain hash sign ('#')</li>
  * <li>must not start with underscore ('_')</li>
  * <li>must be lowercase</li>
- * <li>must not contain invalid file name characters {@link org.opensearch.common.Strings#INVALID_FILENAME_CHARS} </li>
+ * <li>must not contain invalid file name characters {@link org.opensearch.core.common.Strings#INVALID_FILENAME_CHARS} </li>
  * </ul>
  *
  * @opensearch.internal

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataCreateIndexService.java
@@ -257,11 +257,8 @@ public boolean validateDotIndex(String index, @Nullable Boolean isHidden) {
      * Validate the name for an index or alias against some static rules.
      */
     public static void validateIndexOrAliasName(String index, BiFunction<String, String, ? extends RuntimeException> exceptionCtor) {
-        if (org.opensearch.common.Strings.validFileName(index) == false) {
-            throw exceptionCtor.apply(
-                index,
-                "must not contain the following characters " + org.opensearch.common.Strings.INVALID_FILENAME_CHARS
-            );
+        if (Strings.validFileName(index) == false) {
+            throw exceptionCtor.apply(index, "must not contain the following characters " + Strings.INVALID_FILENAME_CHARS);
         }
         if (index.isEmpty()) {
             throw exceptionCtor.apply(index, "must not be empty");

diff --git a/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java b/server/src/main/java/org/opensearch/cluster/metadata/MetadataIndexTemplateService.java
@@ -1502,12 +1502,9 @@ private void validate(String name, @Nullable Settings settings, List<String> ind
             if (indexPattern.startsWith("_")) {
                 validationErrors.add("index_pattern [" + indexPattern + "] must not start with '_'");
             }
-            if (org.opensearch.common.Strings.validFileNameExcludingAstrix(indexPattern) == false) {
+            if (Strings.validFileNameExcludingAstrix(indexPattern) == false) {
                 validationErrors.add(
-                    "index_pattern ["
-                        + indexPattern
-                        + "] must not contain the following characters "
-                        + org.opensearch.common.Strings.INVALID_FILENAME_CHARS
+                    "index_pattern [" + indexPattern + "] must not contain the following characters " + Strings.INVALID_FILENAME_CHARS
                 );
             }
         }

diff --git a/server/src/main/java/org/opensearch/common/Strings.java b/server/src/main/java/org/opensearch/common/Strings.java
@@ -32,7 +32,6 @@
 
 package org.opensearch.common;
 
-import org.apache.lucene.util.BytesRefBuilder;
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.OpenSearchException;
 import org.opensearch.core.common.bytes.BytesReference;
@@ -41,11 +40,6 @@
 import org.opensearch.core.xcontent.XContentBuilder;
 
 import java.io.IOException;
-import java.util.Arrays;
-import java.util.Set;
-
-import static java.util.Collections.unmodifiableSet;
-import static org.opensearch.common.util.set.Sets.newHashSet;
 
 /**
  * String utility class.
@@ -56,90 +50,8 @@ public class Strings {
 
     public static final String[] EMPTY_ARRAY = org.opensearch.core.common.Strings.EMPTY_ARRAY;
 
-    // ---------------------------------------------------------------------
-    // General convenience methods for working with Strings
-    // ---------------------------------------------------------------------
-
-    /**
-     * Check that the given BytesReference is neither <code>null</code> nor of length 0
-     * Note: Will return <code>true</code> for a BytesReference that purely consists of whitespace.
-     *
-     * @param bytesReference the BytesReference to check (may be <code>null</code>)
-     * @return <code>true</code> if the BytesReference is not null and has length
-     * @see org.opensearch.core.common.Strings#hasLength(CharSequence)
-     */
-    public static boolean hasLength(BytesReference bytesReference) {
-        return (bytesReference != null && bytesReference.length() > 0);
-    }
-
-    /**
-     * Test whether the given string matches the given substring
-     * at the given index.
-     *
-     * @param str       the original string (or StringBuilder)
-     * @param index     the index in the original string to start matching against
-     * @param substring the substring to match at the given index
-     */
-    public static boolean substringMatch(CharSequence str, int index, CharSequence substring) {
-        for (int j = 0; j < substring.length(); j++) {
-            int i = index + j;
-            if (i >= str.length() || str.charAt(i) != substring.charAt(j)) {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    // ---------------------------------------------------------------------
-    // Convenience methods for working with formatted Strings
-    // ---------------------------------------------------------------------
-
-    /**
-     * Quote the given String with single quotes.
-     *
-     * @param str the input String (e.g. "myString")
-     * @return the quoted String (e.g. "'myString'"),
-     *         or <code>null</code> if the input was <code>null</code>
-     */
-    public static String quote(String str) {
-        return (str != null ? "'" + str + "'" : null);
-    }
-
-    public static final Set<Character> INVALID_FILENAME_CHARS = unmodifiableSet(
-        newHashSet('\\', '/', '*', '?', '"', '<', '>', '|', ' ', ',')
-    );
-
-    public static boolean validFileName(String fileName) {
-        for (int i = 0; i < fileName.length(); i++) {
-            char c = fileName.charAt(i);
-            if (INVALID_FILENAME_CHARS.contains(c)) {
-                return false;
-            }
-        }
-        return true;
-    }
-
-    public static boolean validFileNameExcludingAstrix(String fileName) {
-        for (int i = 0; i < fileName.length(); i++) {
-            char c = fileName.charAt(i);
-            if (c != '*' && INVALID_FILENAME_CHARS.contains(c)) {
-                return false;
-            }
-        }
-        return true;
-    }
-
     private Strings() {}
 
-    public static byte[] toUTF8Bytes(CharSequence charSequence) {
-        return toUTF8Bytes(charSequence, new BytesRefBuilder());
-    }
-
-    public static byte[] toUTF8Bytes(CharSequence charSequence, BytesRefBuilder spare) {
-        spare.copyChars(charSequence);
-        return Arrays.copyOf(spare.bytes(), spare.length());
-    }
-
     /**
      * 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

diff --git a/server/src/main/java/org/opensearch/common/settings/PropertyPlaceholder.java b/server/src/main/java/org/opensearch/common/settings/PropertyPlaceholder.java
@@ -32,7 +32,7 @@
 
 package org.opensearch.common.settings;
 
-import org.opensearch.common.Strings;
+import org.opensearch.core.common.Strings;
 
 import java.util.HashSet;
 import java.util.Objects;

diff --git a/...r/src/main/java/org/opensearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java b/...r/src/main/java/org/opensearch/index/snapshots/blobstore/BlobStoreIndexShardSnapshot.java
@@ -36,8 +36,8 @@
 import org.apache.lucene.util.Version;
 import org.opensearch.OpenSearchParseException;
 import org.opensearch.core.ParseField;
-import org.opensearch.common.Strings;
 import org.opensearch.common.lucene.Lucene;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.unit.ByteSizeValue;
 import org.opensearch.core.xcontent.ToXContent;
 import org.opensearch.core.xcontent.ToXContentFragment;

diff --git a/server/src/main/java/org/opensearch/repositories/RepositoriesService.java b/server/src/main/java/org/opensearch/repositories/RepositoriesService.java
@@ -57,7 +57,6 @@
 import org.opensearch.cluster.service.ClusterManagerTaskKeys;
 import org.opensearch.cluster.service.ClusterManagerTaskThrottler;
 import org.opensearch.cluster.service.ClusterService;
-import org.opensearch.common.Strings;
 import org.opensearch.common.lifecycle.AbstractLifecycleComponent;
 import org.opensearch.common.regex.Regex;
 import org.opensearch.common.settings.Setting;
@@ -66,6 +65,7 @@
 import org.opensearch.common.util.FeatureFlags;
 import org.opensearch.common.util.concurrent.ConcurrentCollections;
 import org.opensearch.common.util.io.IOUtils;
+import org.opensearch.core.common.Strings;
 import org.opensearch.repositories.blobstore.MeteredBlobStoreRepository;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportService;
@@ -599,7 +599,7 @@ private Repository createRepository(RepositoryMetadata repositoryMetadata, Map<S
     }
 
     private static void validate(final String repositoryName) {
-        if (org.opensearch.core.common.Strings.hasLength(repositoryName) == false) {
+        if (Strings.hasLength(repositoryName) == false) {
             throw new RepositoryException(repositoryName, "cannot be empty");
         }
         if (repositoryName.contains("#")) {

diff --git a/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java b/server/src/main/java/org/opensearch/repositories/blobstore/BlobStoreRepository.java
@@ -67,14 +67,14 @@
 import org.opensearch.common.Nullable;
 import org.opensearch.common.Numbers;
 import org.opensearch.common.SetOnce;
-import org.opensearch.common.Strings;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.blobstore.BlobContainer;
 import org.opensearch.common.blobstore.BlobMetadata;
 import org.opensearch.common.blobstore.BlobPath;
 import org.opensearch.common.blobstore.BlobStore;
 import org.opensearch.common.blobstore.DeleteResult;
 import org.opensearch.common.blobstore.fs.FsBlobContainer;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.bytes.BytesArray;
 import org.opensearch.core.common.bytes.BytesReference;
 import org.opensearch.common.collect.Tuple;

diff --git a/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java b/server/src/main/java/org/opensearch/snapshots/SnapshotsService.java
@@ -79,7 +79,6 @@
 import org.opensearch.cluster.service.ClusterService;
 import org.opensearch.common.Nullable;
 import org.opensearch.common.Priority;
-import org.opensearch.common.Strings;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.collect.Tuple;
 import org.opensearch.common.lifecycle.AbstractLifecycleComponent;
@@ -88,6 +87,7 @@
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.unit.TimeValue;
 import org.opensearch.common.util.concurrent.AbstractRunnable;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.common.io.stream.StreamInput;
 import org.opensearch.core.index.Index;
 import org.opensearch.core.index.shard.ShardId;
@@ -477,7 +477,7 @@ public ClusterState execute(ClusterState currentState) {
                         "No indices in the source snapshot ["
                             + sourceSnapshotId
                             + "] matched requested pattern ["
-                            + org.opensearch.core.common.Strings.arrayToCommaDelimitedString(request.indices())
+                            + Strings.arrayToCommaDelimitedString(request.indices())
                             + "]"
                     );
                 }
@@ -823,7 +823,7 @@ private static void validate(String repositoryName, String snapshotName, Cluster
     }
 
     private static void validate(final String repositoryName, final String snapshotName) {
-        if (org.opensearch.core.common.Strings.hasLength(snapshotName) == false) {
+        if (Strings.hasLength(snapshotName) == false) {
             throw new InvalidSnapshotNameException(repositoryName, snapshotName, "cannot be empty");
         }
         if (snapshotName.contains(" ")) {
@@ -1806,7 +1806,7 @@ public void deleteSnapshots(final DeleteSnapshotRequest request, final ActionLis
         logger.info(
             () -> new ParameterizedMessage(
                 "deleting snapshots [{}] from repository [{}]",
-                org.opensearch.core.common.Strings.arrayToCommaDelimitedString(snapshotNames),
+                Strings.arrayToCommaDelimitedString(snapshotNames),
                 repoName
             )
         );

diff --git a/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java b/server/src/test/java/org/opensearch/cluster/metadata/MetadataCreateIndexServiceTests.java
@@ -671,7 +671,11 @@ public void testValidateIndexName() throws Exception {
                 false,
                 new AwarenessReplicaBalance(Settings.EMPTY, clusterService.getClusterSettings())
             );
-            validateIndexName(checkerService, "index?name", "must not contain the following characters " + Strings.INVALID_FILENAME_CHARS);
+            validateIndexName(
+                checkerService,
+                "index?name",
+                "must not contain the following characters " + org.opensearch.core.common.Strings.INVALID_FILENAME_CHARS
+            );
 
             validateIndexName(checkerService, "index#name", "must not contain '#'");
 

diff --git a/server/src/test/java/org/opensearch/repositories/RepositoriesServiceTests.java b/server/src/test/java/org/opensearch/repositories/RepositoriesServiceTests.java
@@ -47,13 +47,13 @@
 import org.opensearch.cluster.node.DiscoveryNode;
 import org.opensearch.cluster.service.ClusterApplierService;
 import org.opensearch.cluster.service.ClusterService;
-import org.opensearch.common.Strings;
 import org.opensearch.common.UUIDs;
 import org.opensearch.common.blobstore.BlobPath;
 import org.opensearch.common.blobstore.BlobStore;
 import org.opensearch.common.lifecycle.Lifecycle;
 import org.opensearch.common.lifecycle.LifecycleListener;
 import org.opensearch.common.settings.Settings;
+import org.opensearch.core.common.Strings;
 import org.opensearch.core.xcontent.NamedXContentRegistry;
 import org.opensearch.index.mapper.MapperService;
 import org.opensearch.core.index.shard.ShardId;