Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,9 @@ public Task createTask(long id, String type, String action, TaskId parentTaskId,
@Override
public String getDescription() {
final StringBuilder stringBuilder = new StringBuilder("repositories[");
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(repositories), ",", "", "", 512, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(repositories), ",", 512, stringBuilder);
stringBuilder.append("], snapshots[");
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(snapshots), ",", "", "", 1024, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(snapshots), ",", 1024, stringBuilder);
stringBuilder.append("]");
return stringBuilder.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ public int hashCode() {
@Override
public String getDescription() {
final StringBuilder stringBuilder = new StringBuilder("shard").append(shardId).append(", repositories[");
Strings.collectionToDelimitedStringWithLimit(repositories, ",", "", "", 1024, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(repositories, ",", 1024, stringBuilder);
stringBuilder.append("]");
return stringBuilder.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public boolean ignoreUnavailable() {
@Override
public String getDescription() {
final StringBuilder stringBuilder = new StringBuilder("repository[").append(repository).append("], snapshots[");
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(snapshots), ",", "", "", 1024, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(snapshots), ",", 1024, stringBuilder);
stringBuilder.append("]");
return stringBuilder.toString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,9 @@ public ClusterState execute(BatchExecutionContext<LazyRolloverTask> batchExecuti
}

if (state != batchExecutionContext.initialState()) {
var reason = new StringBuilder();
Strings.collectionToDelimitedStringWithLimit(results, ",", "lazy bulk rollover [", "]", 1024, reason);
var reason = new StringBuilder("lazy bulk rollover [");
Strings.collectionToDelimitedStringWithLimit(results, ",", 1024, reason);
reason.append(']');
try (var ignored = batchExecutionContext.dropHeadersContext()) {
state = allocationService.reroute(state, reason.toString(), listener.reroute());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,8 +528,9 @@ public ClusterState execute(BatchExecutionContext<RolloverTask> batchExecutionCo
}

if (state != batchExecutionContext.initialState()) {
var reason = new StringBuilder();
Strings.collectionToDelimitedStringWithLimit(results, ",", "bulk rollover [", "]", 1024, reason);
var reason = new StringBuilder("bulk rollover [");
Strings.collectionToDelimitedStringWithLimit(results, ",", 1024, reason);
reason.append(']');
try (var ignored = batchExecutionContext.dropHeadersContext()) {
state = allocationService.reroute(state, reason.toString(), listener.reroute());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ public ActionRequestValidationException validate() {
@Override
public String getDescription() {
final StringBuilder stringBuilder = new StringBuilder("shards[");
Strings.collectionToDelimitedStringWithLimit(shardIds, ",", "", "", 1024, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(shardIds, ",", 1024, stringBuilder);
return completeDescription(stringBuilder, fields, filters, allowedTypes, includeEmptyFields);
}

Expand All @@ -170,11 +170,11 @@ static String completeDescription(
boolean includeEmptyFields
) {
stringBuilder.append("], fields[");
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(fields), ",", "", "", 1024, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(fields), ",", 1024, stringBuilder);
stringBuilder.append("], filters[");
Strings.collectionToDelimitedString(Arrays.asList(filters), ",", "", "", stringBuilder);
Strings.collectionToDelimitedString(Arrays.asList(filters), ",", stringBuilder);
stringBuilder.append("], types[");
Strings.collectionToDelimitedString(Arrays.asList(allowedTypes), ",", "", "", stringBuilder);
Strings.collectionToDelimitedString(Arrays.asList(allowedTypes), ",", stringBuilder);
stringBuilder.append("], includeEmptyFields[");
stringBuilder.append(includeEmptyFields);
stringBuilder.append("]");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ public int hashCode() {
@Override
public String getDescription() {
final StringBuilder stringBuilder = new StringBuilder("indices[");
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(indices), ",", "", "", 1024, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(Arrays.asList(indices), ",", 1024, stringBuilder);
return FieldCapabilitiesNodeRequest.completeDescription(stringBuilder, fields, filters, types, includeEmptyFields);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ default String describeTasks(List<T> tasks) {
Strings.collectionToDelimitedStringWithLimit(
(Iterable<String>) () -> tasks.stream().map(Object::toString).filter(s -> s.isEmpty() == false).iterator(),
", ",
"",
"",
1024,
output
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1194,8 +1194,6 @@ private ClusterState openIndices(final Index[] indices, final ClusterState curre
Strings.collectionToDelimitedStringWithLimit(
indicesToOpen.stream().map(i -> (CharSequence) i.v2().getIndex().toString()).toList(),
",",
"",
"",
512,
indexNames
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1702,7 +1702,7 @@ private String buildTasksDescription(List<ExecutionResult<T>> tasks) {
Strings.collectionToDelimitedStringWithLimit((Iterable<String>) () -> tasksBySource.entrySet().stream().map(entry -> {
var tasksDescription = executor.describeTasks(entry.getValue());
return tasksDescription.isEmpty() ? entry.getKey() : entry.getKey() + "[" + tasksDescription + "]";
}).filter(s -> s.isEmpty() == false).iterator(), ", ", "", "", MAX_TASK_DESCRIPTION_CHARS, output);
}).filter(s -> s.isEmpty() == false).iterator(), ", ", MAX_TASK_DESCRIPTION_CHARS, output);
if (output.length() > MAX_TASK_DESCRIPTION_CHARS) {
output.append(" (").append(tasks.size()).append(" tasks in total)");
}
Expand Down
45 changes: 11 additions & 34 deletions server/src/main/java/org/elasticsearch/common/Strings.java
Original file line number Diff line number Diff line change
Expand Up @@ -546,54 +546,43 @@ public static Set<String> commaDelimitedListToSet(String str) {
* String. E.g. useful for <code>toString()</code> implementations.
*
* @param coll the Collection to display
* @param delim the delimiter to use (probably a ",")
* @param prefix the String to start each element with
* @param suffix the String to end each element with
* @param delimiter the delimiter to use (probably a ",")
* @return the delimited String
*/
public static String collectionToDelimitedString(Iterable<?> coll, String delim, String prefix, String suffix) {
public static String collectionToDelimitedString(Iterable<?> coll, String delimiter) {
StringBuilder sb = new StringBuilder();
collectionToDelimitedString(coll, delim, prefix, suffix, sb);
collectionToDelimitedString(coll, delimiter, sb);
return sb.toString();
}

public static void collectionToDelimitedString(Iterable<?> coll, String delim, String prefix, String suffix, StringBuilder sb) {
public static void collectionToDelimitedString(Iterable<?> coll, String delimiter, StringBuilder sb) {
Iterator<?> it = coll.iterator();
while (it.hasNext()) {
sb.append(prefix).append(it.next()).append(suffix);
sb.append(it.next());
if (it.hasNext()) {
sb.append(delim);
sb.append(delimiter);
}
}
}

/**
* Converts a collection of items to a string like {@link #collectionToDelimitedString(Iterable, String, String, String, StringBuilder)}
* Converts a collection of items to a string like {@link #collectionToDelimitedString(Iterable, String, StringBuilder)}
* except that it stops if the string gets too long and just indicates how many items were omitted.
*
* @param coll the collection of items to display
* @param delim the delimiter to write between the items (usually {@code ","})
* @param prefix a string to write before each item (usually {@code ""} or {@code "["})
* @param suffix a string to write after each item (usually {@code ""} or {@code "]"})
* @param delimiter the delimiter to write between the items (e.g. {@code ","})
* @param appendLimit if this many characters have been appended to the string and there are still items to display then the remaining
* items are omitted
*/
public static void collectionToDelimitedStringWithLimit(
Iterable<?> coll,
String delim,
String prefix,
String suffix,
int appendLimit,
StringBuilder sb
) {
public static void collectionToDelimitedStringWithLimit(Iterable<?> coll, String delimiter, int appendLimit, StringBuilder sb) {
final Iterator<?> it = coll.iterator();
final long lengthLimit = sb.length() + appendLimit; // long to avoid overflow
int count = 0;
while (it.hasNext()) {
sb.append(prefix).append(it.next()).append(suffix);
sb.append(it.next());
count += 1;
if (it.hasNext()) {
sb.append(delim);
sb.append(delimiter);
if (sb.length() > lengthLimit) {
int omitted = 0;
while (it.hasNext()) {
Expand All @@ -606,18 +595,6 @@ public static void collectionToDelimitedStringWithLimit(
}
}

/**
* Convenience method to return a Collection as a delimited (e.g. CSV)
* String. E.g. useful for <code>toString()</code> implementations.
*
* @param coll the Collection to display
* @param delim the delimiter to use (probably a ",")
* @return the delimited String
*/
public static String collectionToDelimitedString(Iterable<?> coll, String delim) {
return collectionToDelimitedString(coll, delim, "", "");
}

/**
* Convenience method to return a Collection as a CSV String.
* E.g. useful for <code>toString()</code> implementations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2510,8 +2510,6 @@ public void onFailure(Exception e) {
Strings.collectionToDelimitedStringWithLimit(
deleteEntry.snapshots().stream().map(SnapshotId::getName).toList(),
",",
"",
"",
1024,
sb
);
Expand Down
32 changes: 13 additions & 19 deletions server/src/test/java/org/elasticsearch/common/StringsTests.java
Original file line number Diff line number Diff line change
Expand Up @@ -228,84 +228,78 @@ public void testDelimitedListToStringArray() {

public void testCollectionToDelimitedStringWithLimitZero() {
final String delimiter = randomFrom("", ",", ", ", "/");
final String prefix = randomFrom("", "[");
final String suffix = randomFrom("", "]");

final int count = between(0, 100);
final List<String> strings = new ArrayList<>(count);
while (strings.size() < count) {
// avoid starting with a sequence of empty appends, it makes the assertions much messier
final int minLength = strings.isEmpty() && delimiter.isEmpty() && prefix.isEmpty() && suffix.isEmpty() ? 1 : 0;
final int minLength = strings.isEmpty() && delimiter.isEmpty() ? 1 : 0;
strings.add(randomAlphaOfLength(between(minLength, 10)));
}

final StringBuilder stringBuilder = new StringBuilder();
collectionToDelimitedStringWithLimit(strings, delimiter, prefix, suffix, 0, stringBuilder);
collectionToDelimitedStringWithLimit(strings, delimiter, 0, stringBuilder);
final String completelyTruncatedDescription = stringBuilder.toString();

if (count == 0) {
assertThat(completelyTruncatedDescription, equalTo(""));
} else if (count == 1) {
assertThat(completelyTruncatedDescription, equalTo(prefix + strings.get(0) + suffix));
assertThat(completelyTruncatedDescription, equalTo(strings.get(0)));
} else {
assertThat(
completelyTruncatedDescription,
equalTo(prefix + strings.get(0) + suffix + delimiter + "... (" + count + " in total, " + (count - 1) + " omitted)")
equalTo(strings.get(0) + delimiter + "... (" + count + " in total, " + (count - 1) + " omitted)")
);
}
}

public void testCollectionToDelimitedStringWithLimitTruncation() {
final String delimiter = randomFrom("", ",", ", ", "/");
final String prefix = randomFrom("", "[");
final String suffix = randomFrom("", "]");

final int count = between(2, 100);
final List<String> strings = new ArrayList<>(count);
while (strings.size() < count) {
// avoid empty appends, it makes the assertions much messier
final int minLength = delimiter.isEmpty() && prefix.isEmpty() && suffix.isEmpty() ? 1 : 0;
final int minLength = delimiter.isEmpty() ? 1 : 0;
strings.add(randomAlphaOfLength(between(minLength, 10)));
}

final int fullDescriptionLength = collectionToDelimitedString(strings, delimiter, prefix, suffix).length();
final int lastItemSize = prefix.length() + strings.get(count - 1).length() + suffix.length();
final int fullDescriptionLength = collectionToDelimitedString(strings, delimiter).length();
final int lastItemSize = strings.get(count - 1).length();
final int truncatedLength = between(0, fullDescriptionLength - lastItemSize - 1);
final StringBuilder stringBuilder = new StringBuilder();
collectionToDelimitedStringWithLimit(strings, delimiter, prefix, suffix, truncatedLength, stringBuilder);
collectionToDelimitedStringWithLimit(strings, delimiter, truncatedLength, stringBuilder);
final String truncatedDescription = stringBuilder.toString();

assertThat(truncatedDescription, allOf(containsString("... (" + count + " in total,"), endsWith(" omitted)")));

assertThat(
truncatedDescription,
truncatedDescription.length(),
lessThanOrEqualTo(truncatedLength + (prefix + "0123456789" + suffix + delimiter + "... (999 in total, 999 omitted)").length())
lessThanOrEqualTo(truncatedLength + ("0123456789" + delimiter + "... (999 in total, 999 omitted)").length())
);
}

public void testCollectionToDelimitedStringWithLimitNoTruncation() {
final String delimiter = randomFrom("", ",", ", ", "/");
final String prefix = randomFrom("", "[");
final String suffix = randomFrom("", "]");

final int count = between(1, 100);
final List<String> strings = new ArrayList<>(count);
while (strings.size() < count) {
strings.add(randomAlphaOfLength(between(0, 10)));
}

final String fullDescription = collectionToDelimitedString(strings, delimiter, prefix, suffix);
final String fullDescription = collectionToDelimitedString(strings, delimiter);
for (String string : strings) {
assertThat(fullDescription, containsString(prefix + string + suffix));
assertThat(fullDescription, containsString(string));
}

final int lastItemSize = prefix.length() + strings.get(count - 1).length() + suffix.length();
final int lastItemSize = strings.get(count - 1).length();
final int minLimit = fullDescription.length() - lastItemSize;
final int limit = randomFrom(between(minLimit, fullDescription.length()), between(minLimit, Integer.MAX_VALUE), Integer.MAX_VALUE);

final StringBuilder stringBuilder = new StringBuilder();
collectionToDelimitedStringWithLimit(strings, delimiter, prefix, suffix, limit, stringBuilder);
collectionToDelimitedStringWithLimit(strings, delimiter, limit, stringBuilder);
assertThat(stringBuilder.toString(), equalTo(fullDescription));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,12 @@ public void testCorsConfigWithBadRegex() {
public void testCorsConfig() {
final Set<String> methods = new HashSet<>(Arrays.asList("get", "options", "post"));
final Set<String> headers = new HashSet<>(Arrays.asList("Content-Type", "Content-Length"));
final String prefix = randomBoolean() ? " " : ""; // sometimes have a leading whitespace between comma delimited elements
final String maybeSpace = randomFrom(" ", ""); // sometimes have a leading whitespace between comma delimited elements
final Settings settings = Settings.builder()
.put(SETTING_CORS_ENABLED.getKey(), true)
.put(SETTING_CORS_ALLOW_ORIGIN.getKey(), "*")
.put(SETTING_CORS_ALLOW_METHODS.getKey(), collectionToDelimitedString(methods, ",", prefix, ""))
.put(SETTING_CORS_ALLOW_HEADERS.getKey(), collectionToDelimitedString(headers, ",", prefix, ""))
.put(SETTING_CORS_ALLOW_METHODS.getKey(), maybeSpace + collectionToDelimitedString(methods, "," + maybeSpace))
.put(SETTING_CORS_ALLOW_HEADERS.getKey(), maybeSpace + collectionToDelimitedString(headers, "," + maybeSpace))
.put(SETTING_CORS_ALLOW_CREDENTIALS.getKey(), true)
.build();
final CorsHandler.Config corsConfig = CorsHandler.buildConfig(settings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ private void auditFilterChanges(String jobId, String filterId, Set<String> added

private static void appendCommaSeparatedSet(Set<String> items, StringBuilder sb) {
sb.append("[");
Strings.collectionToDelimitedString(items, ", ", "'", "'", sb);
Strings.collectionToDelimitedString(items, ", ", sb);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I missed this usage when I looked at your other PR - TBH I missed the whole method without the limit. Here there actually is a non-empty prefix and suffix... I'm inclined to think that it probably wouldn't have a huge effect if we don't wrap the items with single quotes, but technically we're changing something here. How do you feel about it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof that's a subtle one, the dataflow-to-here results pane in IntelliJ uses a proportional font in which "" and "'" are indistinguishable to my eyes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh this is a great workaround!

sb.append("]");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,7 @@ public void calculateCO2AndCosts() {

if (missingStackTraces.isEmpty() == false) {
StringBuilder stringBuilder = new StringBuilder();
Strings.collectionToDelimitedStringWithLimit(missingStackTraces, ",", "", "", 80, stringBuilder);
Strings.collectionToDelimitedStringWithLimit(missingStackTraces, ",", 80, stringBuilder);
log.warn("CO2/cost calculator: missing trace events for StackTraceID [" + stringBuilder + "].");
}
}
Expand Down