replacing StringBuilder with StringJoiner in order to simplify logic

solr/core/src/java/org/apache/solr/jersey/PostRequestLoggingFilter.java

@@ -37,6 +37,7 @@
 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;
@@ -164,7 +165,7 @@ public static String buildRequestBodyString(ContainerRequestContext requestConte
   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(
@@ -173,13 +174,11 @@ public static String filterAndStringifyQueryParameters(
               if (!paramNamesToLog.contains(name)) return;
 
               for (String val : entry.getValue()) {
-                if (sb.length() != 0) 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) {

solr/solrj/src/java/org/apache/solr/common/params/SolrParams.java

@@ -25,6 +25,7 @@
 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;
@@ -459,19 +460,16 @@ public NamedList<Object> toNamedList() {
    */
   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();
       for (String val : getParams(name)) {
-        sb.append(first ? '?' : '&')
-            .append(nameEnc)
-            .append('=')
-            .append(URLEncoder.encode(val, charset));
-        first = false;
+        output.add(URLEncoder.encode(name, charset) + "=" + URLEncoder.encode(val, charset));
       }
     }
-    return sb.toString();
+    return output.toString();
   }
 
   /**
@@ -510,19 +508,14 @@ public String toLocalParamsString() {
    */
   @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();
   }
 
   /**

solr/solrj/src/java/org/apache/solr/common/util/StrUtils.java

@@ -302,36 +302,38 @@ public static boolean parseBool(String s, boolean def) {
    *
    * <p>Characters with a numeric value less than 32 are encoded. &amp;,=,%,+,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();
   }
 
   /**

solr/solrj/src/test/org/apache/solr/common/params/SolrParamTest.java

@@ -27,6 +27,8 @@
 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,44 @@ public void testGetParams() {
     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);
+  }
+
+  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();

solr/test-framework/src/java/org/apache/solr/util/RestTestBase.java

@@ -459,7 +459,7 @@ private static String setParam(String query, String paramToSet, String valueToSe
       // 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 @@ private static String setParam(String query, String paramToSet, String valueToSe
       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 @@ private static String setParam(String query, String paramToSet, String valueToSe
           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();
   }
 }