fix(templates): ensure requestOptions follow same format (#506)

shortcuts · web-flow · commit 9320a5fd9ae0 · 2022-05-17T11:54:45.000+02:00
diff --git a/clients/algoliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/utils/RequestOptions.java b/clients/algoliasearch-client-java-2/algoliasearch-core/src/main/java/com/algolia/utils/RequestOptions.java
@@ -11,7 +11,7 @@
 public class RequestOptions {
 
   private final Map<String, String> headers = new HashMap<String, String>();
-  private final Map<String, String> queryParameters = new HashMap<String, String>();
+  private final Map<String, Object> queryParameters = new HashMap<String, Object>();
   private Integer timeout = null;
 
   public RequestOptions addExtraHeader(
@@ -24,7 +24,7 @@ public RequestOptions addExtraHeader(
 
   public RequestOptions addExtraQueryParameters(
     @Nonnull String key,
-    @Nonnull String value
+    @Nonnull Object value
   ) {
     queryParameters.put(key, value);
     return this;
@@ -34,7 +34,7 @@ public Map<String, String> getExtraHeaders() {
     return headers;
   }
 
-  public Map<String, String> getExtraQueryParameters() {
+  public Map<String, Object> getExtraQueryParameters() {
     return queryParameters;
   }
 
diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts
@@ -106,13 +106,30 @@ export function createTransporter({
         }
       : {};
 
-    const queryParameters = {
+    const queryParameters: QueryParameters = {
       'x-algolia-agent': userAgent.value,
       ...baseQueryParameters,
       ...dataQueryParameters,
-      ...requestOptions.queryParameters,
     };
 
+    if (requestOptions?.queryParameters) {
+      for (const [key, value] of Object.entries(
+        requestOptions.queryParameters
+      )) {
+        // We want to keep `undefined` and `null` values,
+        // but also avoid stringifying `object`s, as they are
+        // handled in the `serializeUrl` step right after.
+        if (
+          !value ||
+          Object.prototype.toString.call(value) === '[object Object]'
+        ) {
+          queryParameters[key] = value;
+        } else {
+          queryParameters[key] = value.toString();
+        }
+      }
+    }
+
     let timeoutsCount = 0;
 
     const retry = async (
diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/helpers.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/transporter/helpers.ts
@@ -3,6 +3,7 @@ import type {
   Host,
   Request,
   RequestOptions,
+  QueryParameters,
   Response,
   StackFrame,
 } from '../types';
@@ -40,9 +41,7 @@ export function serializeUrl(
   return url;
 }
 
-export function serializeQueryParameters(
-  parameters: Readonly<Record<string, any>>
-): string {
+export function serializeQueryParameters(parameters: QueryParameters): string {
   const isObjectOrArray = (value: any): boolean =>
     Object.prototype.toString.call(value) === '[object Object]' ||
     Object.prototype.toString.call(value) === '[object Array]';
diff --git a/clients/algoliasearch-client-javascript/packages/client-common/src/types/Transporter.ts b/clients/algoliasearch-client-javascript/packages/client-common/src/types/Transporter.ts
@@ -3,8 +3,7 @@ import type { Host } from './Host';
 import type { Request, Requester, EndRequest, Response } from './Requester';
 
 export type Headers = Record<string, string>;
-
-export type QueryParameters = Record<string, string>;
+export type QueryParameters = Record<string, any>;
 
 export type RequestOptions = {
   /**
diff --git a/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php b/clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php
@@ -75,7 +75,10 @@ private function normalize($options)
                         $headersToLowerCase
                     );
                 } else {
-                    $normalized[$optionName] = $this->format($value);
+                    $normalized[$optionName] = $this->format(
+                        $value,
+                        $optionName === 'queryParameters'
+                    );
                 }
             } else {
                 $normalized[$optionName] = $value;
@@ -85,21 +88,14 @@ private function normalize($options)
         return $normalized;
     }
 
-    private function format($options)
+    private function format($options, $isQueryParameters = false)
     {
         foreach ($options as $name => $value) {
-            if (in_array($name, self::getAttributesToFormat(), true)) {
-                if (is_array($value)) {
-                    $options[$name] = implode(',', $value);
-                }
+            if (is_array($value) && $isQueryParameters) {
+                $options[$name] = implode(',', $value);
             }
         }
 
         return $options;
     }
-
-    public static function getAttributesToFormat()
-    {
-        return ['attributesToRetrieve', 'type'];
-    }
 }
diff --git a/templates/java/libraries/okhttp-gson/ApiClient.mustache b/templates/java/libraries/okhttp-gson/ApiClient.mustache
@@ -273,7 +273,7 @@ public class ApiClient {
      * @return The HTTP call
      * @throws AlgoliaRuntimeException If fail to serialize the request body object
      */
-    public Call buildCall(String path, String method, Map<String, String> queryParameters, Object body, Map<String, String> headerParams, RequestOptions requestOptions) throws AlgoliaRuntimeException {
+    public Call buildCall(String path, String method, Map<String, Object> queryParameters, Object body, Map<String, String> headerParams, RequestOptions requestOptions) throws AlgoliaRuntimeException {
         Request request = buildRequest(path, method, queryParameters, body, headerParams, requestOptions);
 
         return requester.newCall(request);
@@ -291,7 +291,7 @@ public class ApiClient {
      * @return The HTTP request
      * @throws AlgoliaRuntimeException If fail to serialize the request body object
      */
-    public Request buildRequest(String path, String method, Map<String, String> queryParameters, Object body, Map<String, String> headerParams, RequestOptions requestOptions) throws AlgoliaRuntimeException {
+    public Request buildRequest(String path, String method, Map<String, Object> queryParameters, Object body, Map<String, String> headerParams, RequestOptions requestOptions) throws AlgoliaRuntimeException {
         boolean hasRequestOptions = requestOptions != null;
         final String url = buildUrl(
           path,
@@ -331,35 +331,26 @@ public class ApiClient {
      * @param extraQueryParameters The query parameters, coming from the requestOptions
      * @return The full URL
      */
-    public String buildUrl(String path, Map<String, String> queryParameters, Map<String, String> extraQueryParameters) {
-        StringBuilder url = new StringBuilder();
-
-        //The real host will be assigned by the retry strategy
-        url.append("http://temp.path").append(path);
+    public String buildUrl(
+      String path,
+      Map<String, Object> queryParameters,
+      Map<String, Object> extraQueryParameters
+    ) {
+      if (extraQueryParameters != null) {
+        for (Entry<String, Object> param : extraQueryParameters.entrySet()) {
+          queryParameters.put(param.getKey(), param.getValue());
+        }
+      }
 
-        url = parseQueryParameters(path, url, queryParameters);
-        url = parseQueryParameters(path, url, extraQueryParameters);
+      final StringBuilder url = new StringBuilder();
 
-        return url.toString();
-    }
+      // The real host will be assigned by the retry strategy
+      url.append("http://temp.path").append(path);
 
-    /**
-     * Parses the given map of Query Parameters to a given URL.
-     *
-     * @param path The sub path
-     * @param url The url to add queryParameters to
-     * @param queryParameters The query parameters
-     * @return The URL
-     */
-    public StringBuilder parseQueryParameters(
-      String path,
-      StringBuilder url,
-      Map<String, String> queryParameters
-    ) {
       if (queryParameters != null && !queryParameters.isEmpty()) {
         // support (constant) query string in `path`, e.g. "/posts?draft=1"
         String prefix = path.contains("?") ? "&" : "?";
-        for (Entry<String, String> param : queryParameters.entrySet()) {
+        for (Entry<String, Object> param : queryParameters.entrySet()) {
           if (param.getValue() != null) {
             if (prefix != null) {
               url.append(prefix);
@@ -376,7 +367,7 @@ public class ApiClient {
         }
       }
 
-      return url;
+      return url.toString();
     }
 
     /**
diff --git a/templates/java/libraries/okhttp-gson/api.mustache b/templates/java/libraries/okhttp-gson/api.mustache
@@ -170,7 +170,7 @@ public class {{classname}} extends ApiClient {
         {{#x-is-custom-request}}{{{paramName}}}.toString(){{/x-is-custom-request}}{{^x-is-custom-request}}this.escapeString({{{paramName}}}.toString()){{/x-is-custom-request}}
     ){{/pathParams}}{{/vendorExtensions}};
 
-    {{javaUtilPrefix}}Map<String, String> queryParameters = new {{javaUtilPrefix}}HashMap<String, String>();
+    {{javaUtilPrefix}}Map<String, Object> queryParameters = new {{javaUtilPrefix}}HashMap<String, Object>();
     {{javaUtilPrefix}}Map<String, String> headers = new {{javaUtilPrefix}}HashMap<String, String>();
 
     {{#vendorExtensions}}{{#queryParams}}
diff --git a/templates/php/api.mustache b/templates/php/api.mustache
@@ -6,7 +6,6 @@ use {{invokerPackage}}\Algolia;
 use {{invokerPackage}}\ApiException;
 use {{invokerPackage}}\Configuration\{{configClassname}};
 use {{invokerPackage}}\ObjectSerializer;
-use {{invokerPackage}}\RequestOptions\RequestOptionsFactory;
 use {{invokerPackage}}\RetryStrategy\ApiWrapper;
 use {{invokerPackage}}\RetryStrategy\ApiWrapperInterface;
 use {{invokerPackage}}\RetryStrategy\ClusterHosts;
@@ -213,14 +212,7 @@ use {{invokerPackage}}\RetryStrategy\ClusterHosts;
         {{#isExplode}}
         if (${{paramName}} !== null) {
         {{#style}}
-            if(is_array(${{paramName}}) && ! in_array('{{paramName}}', RequestOptionsFactory::getAttributesToFormat())) {
-                foreach(${{paramName}} as $key => $value) {
-                    $queryParameters[$key] = $value;
-                }
-            }
-            else {
-                $queryParameters{{^x-is-custom-request}}['{{baseName}}']{{/x-is-custom-request}} = ${{paramName}};
-            }
+            $queryParameters{{^x-is-custom-request}}['{{baseName}}']{{/x-is-custom-request}} = ${{paramName}};
         {{/style}}
         {{^style}}
             $queryParameters['{{baseName}}'] = ${{paramName}};
diff --git a/tests/CTS/methods/requests/templates/java/requests.mustache b/tests/CTS/methods/requests/templates/java/requests.mustache
@@ -55,10 +55,10 @@ class {{client}}Tests {
 
         {{#request.queryParameters}}
         Map<String, String> expectedQuery = JSON.deserialize("{{#lambda.escapequotes}}{{{request.queryParameters}}}{{/lambda.escapequotes}}", new TypeToken<HashMap<String, String>>() {}.getType());
-        Map<String, String> actualQuery = req.queryParameters;
+        Map<String, Object> actualQuery = req.queryParameters;
 
         assertEquals(expectedQuery.size(), actualQuery.size());
-        for (Map.Entry<String, String> p : actualQuery.entrySet()) {
+        for (Map.Entry<String, Object> p : actualQuery.entrySet()) {
           assertEquals(expectedQuery.get(p.getKey()), p.getValue());
         }
         {{/request.queryParameters}}
diff --git a/tests/output/java/src/test/java/com/algolia/CallEcho.java b/tests/output/java/src/test/java/com/algolia/CallEcho.java
@@ -51,8 +51,8 @@ private String processResponseBody() {
     }
   }
 
-  private Map<String, String> buildQueryParameters() {
-    Map<String, String> params = new HashMap<>();
+  private Map<String, Object> buildQueryParameters() {
+    Map<String, Object> params = new HashMap<>();
     HttpUrl url = request.url();
     for (String name : url.queryParameterNames()) {
       for (String value : url.queryParameterValues(name)) {
diff --git a/tests/output/java/src/test/java/com/algolia/EchoResponse.java b/tests/output/java/src/test/java/com/algolia/EchoResponse.java
@@ -7,6 +7,6 @@ public class EchoResponse {
   public String path;
   public String method;
   public String body;
-  public Map<String, String> queryParameters;
+  public Map<String, Object> queryParameters;
   public Map<String, String> headers;
 }

Original file line number	Diff line number	Diff line change
`@@ -75,7 +75,10 @@ private function normalize($options)`
`75`	`75`	`$headersToLowerCase`
`76`	`76`	`);`
`77`	`77`	`} else {`
`78`		`- $normalized[$optionName] = $this->format($value);`
	`78`	`+ $normalized[$optionName] = $this->format(`
	`79`	`+ $value,`
	`80`	`+ $optionName === 'queryParameters'`
	`81`	`+ );`
`79`	`82`	`}`
`80`	`83`	`} else {`
`81`	`84`	`$normalized[$optionName] = $value;`
`@@ -85,21 +88,14 @@ private function normalize($options)`
`85`	`88`	`return $normalized;`
`86`	`89`	`}`
`87`	`90`
`88`		`- private function format($options)`
	`91`	`+ private function format($options, $isQueryParameters = false)`
`89`	`92`	`{`
`90`	`93`	`foreach ($options as $name => $value) {`
`91`		`- if (in_array($name, self::getAttributesToFormat(), true)) {`
`92`		`- if (is_array($value)) {`
`93`		`- $options[$name] = implode(',', $value);`
`94`		`- }`
	`94`	`+ if (is_array($value) && $isQueryParameters) {`
	`95`	`+ $options[$name] = implode(',', $value);`
`95`	`96`	`}`
`96`	`97`	`}`
`97`	`98`
`98`	`99`	`return $options;`
`99`	`100`	`}`
`100`		`-`
`101`		`- public static function getAttributesToFormat()`
`102`		`- {`
`103`		`- return ['attributesToRetrieve', 'type'];`
`104`		`- }`
`105`	`101`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,8 +51,8 @@ private String processResponseBody() {`
`51`	`51`	`}`
`52`	`52`	`}`
`53`	`53`
`54`		`- private Map<String, String> buildQueryParameters() {`
`55`		`- Map<String, String> params = new HashMap<>();`
	`54`	`+ private Map<String, Object> buildQueryParameters() {`
	`55`	`+ Map<String, Object> params = new HashMap<>();`
`56`	`56`	`HttpUrl url = request.url();`
`57`	`57`	`for (String name : url.queryParameterNames()) {`
`58`	`58`	`for (String value : url.queryParameterValues(name)) {`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,6 @@ public class EchoResponse {`
`7`	`7`	`public String path;`
`8`	`8`	`public String method;`
`9`	`9`	`public String body;`
`10`		`- public Map<String, String> queryParameters;`
	`10`	`+ public Map<String, Object> queryParameters;`
`11`	`11`	`public Map<String, String> headers;`
`12`	`12`	`}`