This is an automated email from the ASF dual-hosted git repository.
epugh pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/main by this push:
new 29b483ada4d replacing StringBuilder with StringJoiner in order to
simplify logic (#4628)
29b483ada4d is described below
commit 29b483ada4df3831b14fb1df8d5e08728be5354c
Author: Renato Haeberli <[email protected]>
AuthorDate: Mon Aug 17 21:20:23 2026 +0200
replacing StringBuilder with StringJoiner in order to simplify logic (#4628)
Co-authored-by: Chan Chan <[email protected]>
---
.../solr/jersey/PostRequestLoggingFilter.java | 11 +++---
.../org/apache/solr/common/params/SolrParams.java | 28 ++++++---------
.../java/org/apache/solr/common/util/StrUtils.java | 22 ++++++------
.../apache/solr/common/params/SolrParamTest.java | 41 ++++++++++++++++++++++
.../java/org/apache/solr/util/RestTestBase.java | 8 ++---
5 files changed, 73 insertions(+), 37 deletions(-)
diff --git
a/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
b/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
index 7c3b6450e4a..b9bed2f8176 100644
--- a/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
+++ b/solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java
@@ -37,6 +37,7 @@ import java.util.HashSet;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
+import java.util.StringJoiner;
import java.util.stream.Collectors;
import org.apache.solr.client.api.model.SolrJerseyResponse;
import org.apache.solr.common.util.CollectionUtil;
@@ -165,7 +166,7 @@ public class PostRequestLoggingFilter implements
ContainerResponseFilter {
public static String filterAndStringifyQueryParameters(
MultivaluedMap<String, String> unfilteredParams) {
final var paramNamesToLog = getParamNamesToLog(unfilteredParams);
- final StringBuilder sb = new StringBuilder(128);
+ var output = new StringJoiner("&");
unfilteredParams.entrySet().stream()
.sorted(Map.Entry.comparingByKey())
.forEachOrdered(
@@ -174,13 +175,11 @@ public class PostRequestLoggingFilter implements
ContainerResponseFilter {
if (!paramNamesToLog.contains(name)) return;
for (String val : entry.getValue()) {
- if (!sb.isEmpty()) sb.append('&');
- StrUtils.partialURLEncodeVal(sb, name);
- sb.append('=');
- StrUtils.partialURLEncodeVal(sb, val);
+ output.add(
+ StrUtils.partialURLEncodeVal(name) + "=" +
StrUtils.partialURLEncodeVal(val));
}
});
- return sb.toString();
+ return output.toString();
}
private static Set<String> getParamNamesToLog(MultivaluedMap<String, String>
queryParameters) {
diff --git a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java
b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java
index 0f0e0f59676..9243350766e 100644
--- a/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java
+++ b/solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java
@@ -25,6 +25,7 @@ import java.util.Arrays;
import java.util.Iterator;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.StringJoiner;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.apache.solr.client.solrj.util.ClientUtils;
@@ -439,19 +440,17 @@ public abstract class SolrParams
*/
public String toQueryString() {
final Charset charset = StandardCharsets.UTF_8;
- final StringBuilder sb = new StringBuilder(128);
- boolean first = true;
+
+ var output = new StringJoiner("&", "?", "");
+ output.setEmptyValue("");
for (final Iterator<String> it = getParameterNamesIterator();
it.hasNext(); ) {
- final String name = it.next(), nameEnc = URLEncoder.encode(name,
charset);
+ final String name = it.next();
+ final String nameEnc = URLEncoder.encode(name, charset);
for (String val : getParams(name)) {
- sb.append(first ? '?' : '&')
- .append(nameEnc)
- .append('=')
- .append(URLEncoder.encode(val, charset));
- first = false;
+ output.add(nameEnc + "=" + URLEncoder.encode(val, charset));
}
}
- return sb.toString();
+ return output.toString();
}
/**
@@ -490,19 +489,14 @@ public abstract class SolrParams
*/
@Override
public String toString() {
- final StringBuilder sb = new StringBuilder(128);
- boolean first = true;
+ StringJoiner query = new StringJoiner("&");
for (final Iterator<String> it = getParameterNamesIterator();
it.hasNext(); ) {
final String name = it.next();
for (String val : getParams(name)) {
- if (!first) sb.append('&');
- first = false;
- StrUtils.partialURLEncodeVal(sb, name);
- sb.append('=');
- StrUtils.partialURLEncodeVal(sb, val);
+ query.add(StrUtils.partialURLEncodeVal(name) + "=" +
StrUtils.partialURLEncodeVal(val));
}
}
- return sb.toString();
+ return query.toString();
}
/**
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
index f2aaea28671..95e97502752 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java
@@ -302,36 +302,38 @@ public class StrUtils {
*
* <p>Characters with a numeric value less than 32 are encoded.
&,=,%,+,space are encoded.
*/
- public static void partialURLEncodeVal(StringBuilder dest, String val) {
+ public static String partialURLEncodeVal(String val) {
+ var output = new StringBuilder();
for (int i = 0; i < val.length(); i++) {
char ch = val.charAt(i);
if (ch < 32) {
- dest.append('%');
- if (ch < 0x10) dest.append('0');
- dest.append(Integer.toHexString(ch));
+ output.append('%');
+ if (ch < 0x10) output.append('0');
+ output.append(Integer.toHexString(ch));
} else {
switch (ch) {
case ' ':
- dest.append('+');
+ output.append('+');
break;
case '&':
- dest.append("%26");
+ output.append("%26");
break;
case '%':
- dest.append("%25");
+ output.append("%25");
break;
case '=':
- dest.append("%3D");
+ output.append("%3D");
break;
case '+':
- dest.append("%2B");
+ output.append("%2B");
break;
default:
- dest.append(ch);
+ output.append(ch);
break;
}
}
}
+ return output.toString();
}
/**
diff --git
a/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java
b/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java
index 0b60871deff..3a11fbb5f3f 100644
--- a/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java
+++ b/solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java
@@ -27,6 +27,8 @@ import java.util.Map;
import org.apache.solr.SolrTestCase;
import org.apache.solr.common.SolrException;
import org.apache.solr.search.QueryParsing;
+import org.apache.solr.servlet.SolrRequestParsers;
+import org.junit.Test;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -352,6 +354,45 @@ public class SolrParamTest extends SolrTestCase {
assertNull(defaults.get("asagdsaga"));
}
+ @Test
+ public void testFirstParamHasQuestionMark() {
+
+ final ModifiableSolrParams in = getDummySolrParams();
+ String queryString = in.toQueryString();
+ assertEquals('?', queryString.charAt(0));
+
+ MultiMapSolrParams out =
SolrRequestParsers.parseQueryString(queryString.substring(1));
+ assertEquals(in, out);
+ }
+
+ @Test
+ public void testIfToStringCanBeParsed() {
+
+ final ModifiableSolrParams in = getDummySolrParams();
+ String queryString = in.toString();
+
+ assertEquals("first=1st&second=2nd&third=3rd", queryString);
+ MultiMapSolrParams out = SolrRequestParsers.parseQueryString(queryString);
+ assertEquals(in, out);
+ }
+
+ @Test
+ public void testToStringWithEmptySolrParams() {
+
+ ModifiableSolrParams in = new ModifiableSolrParams();
+ String queryString = in.toString();
+ assertEquals("", queryString);
+ MultiMapSolrParams out = SolrRequestParsers.parseQueryString(queryString);
+ assertEquals(in, out);
+ }
+
+ private ModifiableSolrParams getDummySolrParams() {
+ return params(
+ "first", "1st",
+ "second", "2nd",
+ "third", "3rd");
+ }
+
public static int getReturnCode(Runnable runnable) {
try {
runnable.run();
diff --git
a/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java
b/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java
index 83b2d302c0c..830cf0540ff 100644
--- a/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java
+++ b/solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java
@@ -459,7 +459,7 @@ public abstract class RestTestBase extends SolrTestCaseJ4 {
// empty query -> return "paramToSet=valueToSet"
builder.append(paramToSet);
builder.append('=');
- StrUtils.partialURLEncodeVal(builder, valueToSet);
+ builder.append(StrUtils.partialURLEncodeVal(valueToSet));
return builder.toString();
}
MultiMapSolrParams requestParams =
SolrRequestParsers.parseQueryString(query);
@@ -470,7 +470,7 @@ public abstract class RestTestBase extends SolrTestCaseJ4 {
builder.append('&');
builder.append(paramToSet);
builder.append('=');
- StrUtils.partialURLEncodeVal(builder, valueToSet);
+ builder.append(StrUtils.partialURLEncodeVal(valueToSet));
return builder.toString();
}
if (1 == values.length && valueToSet.equals(values[0])) {
@@ -490,14 +490,14 @@ public abstract class RestTestBase extends SolrTestCaseJ4
{
isFirst = false;
builder.append(key);
builder.append('=');
- StrUtils.partialURLEncodeVal(builder, null == val ? "" : val);
+ builder.append(StrUtils.partialURLEncodeVal(null == val ? "" : val));
}
}
}
builder.append(isFirst ? "" : '&');
builder.append(paramToSet);
builder.append('=');
- StrUtils.partialURLEncodeVal(builder, valueToSet);
+ builder.append(StrUtils.partialURLEncodeVal(valueToSet));
return builder.toString();
}
}