Skip to content

Commit

Permalink
[Refactor] Strings methods other than MediaType (#9126) (#9128)
Browse files Browse the repository at this point in the history
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.


(cherry picked from commit 1d28fac)

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
1 parent e59b21a commit 58ca736
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 245 deletions.
359 changes: 224 additions & 135 deletions libs/core/src/main/java/org/opensearch/core/common/Strings.java

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,11 +258,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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
);
}
}
Expand Down
88 changes: 0 additions & 88 deletions server/src/main/java/org/opensearch/common/Strings.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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("#")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,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;
Expand All @@ -89,6 +88,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;
Expand Down Expand Up @@ -655,7 +655,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())
+ "]"
);
}
Expand Down Expand Up @@ -1001,7 +1001,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(" ")) {
Expand Down Expand Up @@ -2246,7 +2246,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
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,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 '#'");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 58ca736

Please sign in to comment.