Added queryParams to the HttpRequestInformation annotation. (Azure#40376)

* Renamed `HttpRequestInformation.requestHeaders()` to `headers()`.

* Added `HttpRequestInformation.queryParams()`.

* Added logic to parse query parameters to `SwaggerMethodParser`.

* Added tests and updated `HttpClientTestsServer` and `HttpBinJSON` to handle query params.

* Added comma escaping and a test.

* Applied PR feedback and added more tests.

* Removed empty list creation for empty query param values.

* Also encoded query param names.

Author: vcolin7

sdk/clientcore/core/src/main/java/io/clientcore/core/http/annotation/HeaderParam.java

@@ -11,7 +11,7 @@
 
 /**
  * Replaces the header with the value of its target. The value specified here replaces headers specified statically in
- * the {@link HttpRequestInformation#requestHeaders()}. If the parameter this annotation is attached to is a Map type, then
+ * the {@link HttpRequestInformation#headers()}. If the parameter this annotation is attached to is a Map type, then
  * this will be treated as a header collection. In that case each of the entries in the argument's map will be
  * individual header values that use the value of this annotation as a prefix to their key/header name.
  *

sdk/clientcore/core/src/main/java/io/clientcore/core/http/annotation/HttpRequestInformation.java

@@ -42,7 +42,7 @@
      *
      * @return The list of static headers to send with the request.
      */
-    String[] requestHeaders() default {};
+    String[] headers() default {};
 
     /**
      * Get expected the status code(s) to receive with a response.
@@ -57,4 +57,12 @@
      * @return The type of the response body sent over the wire.
      */
     Class<?> returnValueWireType() default Void.class;
+
+    /**
+     * Get the query parameters to be appended to the request URL. Elements in the array shall be written in the format
+     * of 'key=value'.
+     *
+     * @return The query parameters to be appended to the request URL.
+     */
+    String[] queryParams() default {};
 }

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/http/rest/SwaggerMethodParser.java

@@ -47,6 +47,7 @@
 import java.util.BitSet;
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -72,6 +73,7 @@ public class SwaggerMethodParser implements HttpResponseDecodeData {
     private final ClientLogger methodLogger;
     private final HttpMethod httpMethod;
     private final String relativePath;
+    private final Map<String, List<String>> queryParams = new LinkedHashMap<>();
     final List<RangeReplaceSubstitution> hostSubstitutions = new ArrayList<>();
     private final List<RangeReplaceSubstitution> pathSubstitutions = new ArrayList<>();
     private final List<QuerySubstitution> querySubstitutions = new ArrayList<>();
@@ -122,7 +124,7 @@ public SwaggerMethodParser(Method swaggerMethod) {
 
         returnType = swaggerMethod.getGenericReturnType();
 
-        final String[] requestHeaders = httpRequestInformation.requestHeaders();
+        final String[] requestHeaders = httpRequestInformation.headers();
 
         if (requestHeaders != null) {
             for (final String requestHeader : requestHeaders) {
@@ -148,6 +150,50 @@ public SwaggerMethodParser(Method swaggerMethod) {
             }
         }
 
+        final String[] requestQueryParams = httpRequestInformation.queryParams();
+
+        if (requestQueryParams != null) {
+            for (final String queryParam : requestQueryParams) {
+                if (CoreUtils.isNullOrEmpty(queryParam)) {
+                    throw new IllegalStateException("Query parameters cannot be null or empty.");
+                }
+
+                // We take the first equals sign as the delimiter between the name and value of the query parameter.
+                // If more than one equals sign is present, the rest of the string is considered part of the value.
+                final int equalsIndex = queryParam.indexOf("=");
+                final String paramName;
+                final String paramValue;
+
+                if (equalsIndex >= 0) {
+                    paramName = UrlEscapers.QUERY_ESCAPER.escape(queryParam.substring(0, equalsIndex));
+
+                    if (!paramName.isEmpty()) {
+                        paramValue = UrlEscapers.QUERY_ESCAPER.escape(queryParam.substring(equalsIndex + 1));
+                    } else {
+                        throw new IllegalStateException("Names for query parameters cannot be empty.");
+                    }
+                } else {
+                    // No equals sign was found, so the entire string is considered the name of the query parameter.
+                    paramName = UrlEscapers.QUERY_ESCAPER.escape(queryParam);
+                    paramValue = null;
+                }
+
+                List<String> currentValues = queryParams.get(paramName);
+
+                if (!CoreUtils.isNullOrEmpty(paramValue)) {
+                    if (currentValues == null) {
+                        currentValues = new ArrayList<>();
+                    }
+
+                    currentValues.add(paramValue);
+
+                    queryParams.put(paramName, currentValues);
+                } else {
+                    queryParams.put(paramName, null);
+                }
+            }
+        }
+
         Class<?> returnValueWireType = httpRequestInformation.returnValueWireType();
 
         if (returnValueWireType == Base64Url.class || returnValueWireType == DateTimeRfc1123.class) {
@@ -227,8 +273,10 @@ public SwaggerMethodParser(Method swaggerMethod) {
         Class<?>[] parameterTypes = swaggerMethod.getParameterTypes();
         int requestOptionsPosition = -1;
         int serverSentEventListenerPosition = -1;
+
         for (int i = 0; i < parameterTypes.length; i++) {
             Class<?> parameterType = parameterTypes[i];
+
             // Check for the RequestOptions position.
             // To retain previous behavior, only track the first instance found.
             if (parameterType == RequestOptions.class && requestOptionsPosition == -1) {
@@ -327,10 +375,23 @@ public String setPath(Object[] methodArguments, ObjectSerializer serializer) {
     @SuppressWarnings("unchecked")
     public void setEncodedQueryParameters(Object[] swaggerMethodArguments, UrlBuilder urlBuilder,
                                           ObjectSerializer serializer) {
+        // First we add the constant query parameters.
+        for (Map.Entry<String, List<String>> entry : queryParams.entrySet()) {
+            if (entry.getValue() == null || entry.getValue().isEmpty()) {
+                urlBuilder.addQueryParameter(entry.getKey(), null);
+            } else {
+                for (String paramValue : entry.getValue()) {
+                    urlBuilder.addQueryParameter(entry.getKey(), paramValue);
+                }
+            }
+        }
+
         if (swaggerMethodArguments == null) {
             return;
         }
 
+        // Then we add query parameters passed as arguments to the request method. If any of them share a name with the
+        // constant query parameters, the constant query parameter will be overwritten.
         for (QuerySubstitution substitution : querySubstitutions) {
             final int parameterIndex = substitution.getMethodParameterIndex();
 

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/util/QueryParameter.java

@@ -153,7 +153,7 @@ public String toString() {
         if (value != null) {
             return name + "=" + value;
         } else if (CoreUtils.isNullOrEmpty(values)) {
-            return "";
+            return name;
         }
 
         checkCachedStringValue();

sdk/clientcore/core/src/main/java/io/clientcore/core/implementation/util/UrlBuilder.java

@@ -282,13 +282,24 @@ private void appendQueryString(StringBuilder stringBuilder) {
     }
 
     private static boolean writeQueryValues(StringBuilder builder, String key, List<String> values, boolean first) {
-        for (String value : values) {
+        if (CoreUtils.isNullOrEmpty(values)) {
             if (!first) {
                 builder.append('&');
             }
 
-            builder.append(key).append('=').append(value);
+            builder.append(key);
+
             first = false;
+        } else {
+            for (String value : values) {
+                if (!first) {
+                    builder.append('&');
+                }
+
+                builder.append(key).append('=').append(value);
+
+                first = false;
+            }
         }
 
         return first;

sdk/clientcore/core/src/test/java/io/clientcore/core/implementation/http/rest/SwaggerMethodParserTests.java

@@ -170,14 +170,14 @@ interface HeaderMethods {
         @HttpRequestInformation(method = HttpMethod.GET, path = "test")
         void noHeaders();
 
-        @HttpRequestInformation(method = HttpMethod.GET, path = "test", requestHeaders = {"", ":", "nameOnly:", ":valueOnly"})
+        @HttpRequestInformation(method = HttpMethod.GET, path = "test", headers = {"", ":", "nameOnly:", ":valueOnly"})
         void malformedHeaders();
 
         @HttpRequestInformation(method = HttpMethod.GET, path = "test",
-            requestHeaders = {"name1:value1", "name2:value2", "name3:value3"})
+            headers = {"name1:value1", "name2:value2", "name3:value3"})
         void headers();
 
-        @HttpRequestInformation(method = HttpMethod.GET, path = "test", requestHeaders = {"name:value1", "name:value2"})
+        @HttpRequestInformation(method = HttpMethod.GET, path = "test", headers = {"name:value1", "name:value2"})
         void sameKeyTwiceLastWins();
     }
 
@@ -395,7 +395,7 @@ interface HeaderSubstitutionMethods {
         @HttpRequestInformation(method = HttpMethod.GET, path = "test")
         void addHeaders(@HeaderParam("sub1") String sub1, @HeaderParam("sub2") boolean sub2);
 
-        @HttpRequestInformation(method = HttpMethod.GET, path = "test", requestHeaders = {"sub1:sub1", "sub2:false"})
+        @HttpRequestInformation(method = HttpMethod.GET, path = "test", headers = {"sub1:sub1", "sub2:false"})
         void overrideHeaders(@HeaderParam("sub1") String sub1, @HeaderParam("sub2") boolean sub2);
 
         @HttpRequestInformation(method = HttpMethod.GET, path = "test")

sdk/clientcore/core/src/test/java/io/clientcore/core/shared/HttpBinJSON.java

@@ -23,6 +23,8 @@ public class HttpBinJSON implements JsonSerializable<HttpBinJSON> {
 
     private Object data;
 
+    private Map<String, List<String>> queryParams;
+
     /**
      * Gets the URL associated with this request.
      *
@@ -39,6 +41,7 @@ public String url() {
      */
     public HttpBinJSON url(String url) {
         this.url = url;
+
         return this;
     }
 
@@ -58,6 +61,7 @@ public Map<String, List<String>> headers() {
      */
     public HttpBinJSON headers(Map<String, List<String>> headers) {
         this.headers = headers;
+
         return this;
     }
 
@@ -77,6 +81,27 @@ public Object data() {
      */
     public HttpBinJSON data(Object data) {
         this.data = data;
+
+        return this;
+    }
+
+    /**
+     * Gets the response headers.
+     *
+     * @return The response headers.
+     */
+    public Map<String, List<String>> queryParams() {
+        return queryParams;
+    }
+
+    /**
+     * Sets the response headers.
+     *
+     * @param queryParams The response headers.
+     */
+    public HttpBinJSON queryParams(Map<String, List<String>> queryParams) {
+        this.queryParams = queryParams;
+
         return this;
     }
 
@@ -107,6 +132,8 @@ public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
         jsonWriter.writeMapField("headers", headers, (headerWriter, headerList) ->
             headerWriter.writeArray(headerList, JsonWriter::writeString));
         jsonWriter.writeUntypedField("data", data);
+        jsonWriter.writeMapField("queryParams", queryParams, (paramWriter, paramList) ->
+            paramWriter.writeArray(paramList, JsonWriter::writeString));
 
         return jsonWriter.writeEndObject();
     }
@@ -140,6 +167,10 @@ public static HttpBinJSON fromJson(JsonReader jsonReader) throws IOException {
                             reader.readMap(headerReader -> headerReader.readArray(JsonReader::getString));
                     } else if ("data".equalsIgnoreCase(fieldName)) {
                         httpBinJSON.data = reader.readUntyped();
+                    } else if ("queryParams".equalsIgnoreCase(fieldName)) {
+                        // Pass the JsonReader to another JsonSerializable to read the inner object.
+                        httpBinJSON.queryParams =
+                            reader.readMap(paramReader -> paramReader.readArray(JsonReader::getString));
                     } else {
                         reader.skipChildren();
                     }