A few improvements to QueryParamsHelper

Author: georgweiss

app/logbook/olog/client-es/src/main/java/org/phoebus/olog/es/api/OlogHttpClient.java

@@ -240,7 +240,7 @@ public List<LogEntry> findLogs(Map<String, String> map) {
     private SearchResult findLogs(MultivaluedMap<String, String> searchParams) throws RuntimeException {
         HttpRequest request = HttpRequest.newBuilder()
                 .uri(URI.create(Preferences.olog_url +
-                        "/logs/search?" + mapToQueryParams(searchParams)))
+                        "/logs/search?" + QueryParamsHelper.mapToQueryParams(searchParams)))
                 .header(OLOG_CLIENT_INFO_HEADER, CLIENT_INFO)
                 .header("Content-Type", CONTENT_TYPE_JSON)
                 .GET()
@@ -467,21 +467,6 @@ public InputStream getAttachment(Long logId, String attachmentName) {
         }
     }
 
-
-    private String mapToQueryParams(MultivaluedMap<String, String> map) {
-        StringBuilder stringBuilder = new StringBuilder();
-        map.keySet().forEach(k -> {
-            List<String> value = map.get(k);
-            if (value != null && !value.isEmpty()) {
-                stringBuilder.append(k).append("=");
-                stringBuilder.append(String.join(",",
-                        value.stream().map(v -> URLEncoder.encode(v, StandardCharsets.UTF_8)).toList()));
-                stringBuilder.append("&");
-            }
-        });
-        return stringBuilder.toString();
-    }
-
     /**
      * @param id Unique log entry id
      * @return A {@link SearchResult} containing a list of {@link LogEntry} objects representing the

core/util/src/main/java/org/phoebus/util/http/QueryParamsHelper.java

@@ -11,7 +11,17 @@
 
 public class QueryParamsHelper {
 
+    /**
+     * Converts a {@link MultivaluedMap<String, String>} to a URL encoded query parameter {@link String}. Attempts to
+     * be fault-tolerant: <code>null</code> or empty {@link List}s values are ignored and not appended to the
+     * returned query parameter {@link String}. See also unit test class.
+     * @param map Map of key/value pairs where each value is expected to be a {@link List} of {@link String}s.
+     * @return A URL encoded {@link String} that can be appended in a http URL, e.g. <code>key1=value1,value2&key2=value3</code>.
+     */
     public static String mapToQueryParams(MultivaluedMap<String, String> map) {
+        if(map == null || map.isEmpty()){
+            return "";
+        }
         StringBuilder stringBuilder = new StringBuilder();
         map.keySet().forEach(k -> {
             List<String> value = map.get(k);
@@ -22,6 +32,10 @@ public static String mapToQueryParams(MultivaluedMap<String, String> map) {
                 stringBuilder.append("&");
             }
         });
-        return stringBuilder.toString();
+        String queryParams = stringBuilder.toString();
+        if("".equals(queryParams)){
+            return "";
+        }
+        return queryParams.substring(0, queryParams.length() - 1);
     }
 }

core/util/src/test/java/org/phoebus/util/http/QueryParamsHelperTest.java

@@ -8,6 +8,7 @@
 
 import javax.ws.rs.core.MultivaluedHashMap;
 import javax.ws.rs.core.MultivaluedMap;
+import java.util.Collections;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.*;
@@ -23,6 +24,27 @@ public void testBasic(){
 
         String queryParamString = QueryParamsHelper.mapToQueryParams(map);
 
-        assertEquals(queryParamString, "key1=value1,value2&key2=value+3&key3=value%3A4&");
+        assertEquals("key1=value1,value2&key2=value+3&key3=value%3A4", queryParamString);
+
+        queryParamString = QueryParamsHelper.mapToQueryParams(new MultivaluedHashMap<>());
+        assertEquals("", queryParamString);
+
+        queryParamString = QueryParamsHelper.mapToQueryParams(null);
+        assertEquals("", queryParamString);
+
+        map = new MultivaluedHashMap<>();
+        map.put("key1", null);
+        map.put("key2", List.of("value3"));
+
+        queryParamString = QueryParamsHelper.mapToQueryParams(map);
+        assertEquals("key2=value3", queryParamString);
+
+        map = new MultivaluedHashMap<>();
+        map.put("key1", null);
+        map.put("key2", Collections.emptyList());
+
+        queryParamString = QueryParamsHelper.mapToQueryParams(map);
+        assertEquals("", queryParamString);
+
     }
 }