Skip to content

Commit

Permalink
[Refactor] Strings methods other than MediaType
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.

Signed-off-by: Nicholas Walter Knize <nknize@apache.org>
  • Loading branch information
nknize committed Aug 4, 2023
1 parent 5bb7fa3 commit d9deacc
Show file tree
Hide file tree
Showing 12 changed files with 246 additions and 244 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 @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
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 Expand Up @@ -338,7 +338,7 @@ public static FileInfo fromXContent(XContentParser parser) throws IOException {
}

// Verify that file information is complete
if (name == null || Strings.validFileName(name) == false) {
if (name == null || org.opensearch.core.common.Strings.validFileName(name) == false) {
throw new OpenSearchParseException("missing or invalid file name [" + name + "]");
} else if (physicalName == null || Strings.validFileName(physicalName) == false) {
throw new OpenSearchParseException("missing or invalid physical file name [" + physicalName + "]");
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 @@ -606,7 +606,10 @@ private static void validate(final String repositoryName) {
throw new RepositoryException(repositoryName, "must not contain '#'");
}
if (Strings.validFileName(repositoryName) == false) {
throw new RepositoryException(repositoryName, "must not contain the following characters " + Strings.INVALID_FILENAME_CHARS);
throw new RepositoryException(
repositoryName,
"must not contain the following characters " + org.opensearch.core.common.Strings.INVALID_FILENAME_CHARS
);
}
}

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 @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -845,7 +845,7 @@ private static void validate(final String repositoryName, final String snapshotN
throw new InvalidSnapshotNameException(
repositoryName,
snapshotName,
"must not contain the following characters " + Strings.INVALID_FILENAME_CHARS
"must not contain the following characters " + org.opensearch.core.common.Strings.INVALID_FILENAME_CHARS
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 '#'");

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 d9deacc

Please sign in to comment.