This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_10x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 015a8745fed79801b4972221bb7d8d47883c57fe 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]> (cherry picked from commit 29b483ada4df3831b14fb1df8d5e08728be5354c) --- .../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(); } }
