Skip to content

fix(java): use Map for query parameters #484

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.algolia.utils.echo;

import com.algolia.Pair;
import java.util.List;
import java.util.Map;

public interface EchoResponseInterface {
public String getPath();
Expand All @@ -10,5 +9,5 @@ public interface EchoResponseInterface {

public String getBody();

public List<Pair> getQueryParams();
public Map<String, String> getQueryParams();
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,8 @@ public void processOpts() {
supportingFiles.removeIf(file ->
file.getTemplateFile().equals("build.gradle.mustache") ||
file.getTemplateFile().equals("settings.gradle.mustache") ||
file.getTemplateFile().equals("gitignore.mustache")
file.getTemplateFile().equals("gitignore.mustache") ||
file.getTemplateFile().equals("Pair.mustache")
);
}

Expand Down
8 changes: 4 additions & 4 deletions scripts/ci/githubActions/createMatrix.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,10 @@ async function getSpecMatrix(baseBranch: string): Promise<void> {
branch: baseBranch,
path,
});
const baseChanged = await isBaseChanged(
baseBranch,
MATRIX_DEPENDENCIES.common
);
const baseChanged = await isBaseChanged(baseBranch, {
...MATRIX_DEPENDENCIES.common,
...MATRIX_DEPENDENCIES.clients.common,
});

// No changes found, we don't put this job in the matrix
if (specChanges === 0 && !baseChanged) {
Expand Down
9 changes: 5 additions & 4 deletions templates/java/EchoResponse.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import java.io.IOException;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import okhttp3.HttpUrl;
import okhttp3.Request;
Expand All @@ -24,12 +25,12 @@ public class EchoResponse{{#apiInfo}}{{#apis}}{{{baseName}}}{{/apis}}{{/apiInfo}
}
}

private static List<Pair> buildQueryParams(Request req) {
List<Pair> params = new ArrayList<Pair>();
private static Map<String, String> buildQueryParams(Request req) {
Map<String, String> params = new HashMap<String, String>();
HttpUrl url = req.url();
for (String name : url.queryParameterNames()) {
for (String value : url.queryParameterValues(name)) {
params.add(new Pair(name, value));
params.put(name, value);
}
}
return params;
Expand All @@ -56,7 +57,7 @@ public class EchoResponse{{#apiInfo}}{{#apis}}{{{baseName}}}{{/apis}}{{/apiInfo}
return parseRequestBody(request);
}

public List<Pair> getQueryParams() {
public Map<String, String> getQueryParams() {
return buildQueryParams(request);
}

Expand Down
47 changes: 0 additions & 47 deletions templates/java/Pair.mustache

This file was deleted.

121 changes: 7 additions & 114 deletions templates/java/libraries/okhttp-gson/ApiClient.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -190,111 +190,6 @@ public class ApiClient {
}
}

/**
* Formats the specified query parameter to a list containing a single {@code Pair} object.
*
* Note that {@code value} must not be a collection.
*
* @param name The name of the parameter.
* @param value The value of the parameter.
* @return A list containing a single {@code Pair} object.
*/
public List<Pair> parameterToPair(String name, Object value) {
List<Pair> params = new ArrayList<Pair>();

// preconditions
if (name == null || name.isEmpty() || value == null) {
return params;
}

params.add(new Pair(name, parameterToString(value)));
return params;
}

/**
* Formats the specified collection query parameters to a list of {@code Pair} objects.
*
* Note that the values of each of the returned Pair objects are percent-encoded.
*
* @param collectionFormat The collection format of the parameter.
* @param name The name of the parameter.
* @param value The value of the parameter.
* @return A list of {@code Pair} objects.
*/
public List<Pair> parameterToPairs(String collectionFormat, String name, Collection value) {
List<Pair> params = new ArrayList<Pair>();

// preconditions
if (name == null || name.isEmpty() || value == null || value.isEmpty()) {
return params;
}

// create the params based on the collection format
if ("multi".equals(collectionFormat)) {
for (Object item : value) {
params.add(new Pair(name, escapeString(parameterToString(item))));
}
return params;
}

// collectionFormat is assumed to be "csv" by default
String delimiter = ",";

// escape all delimiters except commas, which are URI reserved
// characters
if ("ssv".equals(collectionFormat)) {
delimiter = escapeString(" ");
} else if ("tsv".equals(collectionFormat)) {
delimiter = escapeString("\t");
} else if ("pipes".equals(collectionFormat)) {
delimiter = escapeString("|");
}

StringBuilder sb = new StringBuilder();
for (Object item : value) {
sb.append(delimiter);
sb.append(escapeString(parameterToString(item)));
}

params.add(new Pair(name, sb.substring(delimiter.length())));

return params;
}

/**
* Formats the specified collection path parameter to a string value.
*
* @param collectionFormat The collection format of the parameter.
* @param value The value of the parameter.
* @return String representation of the parameter
*/
public String collectionPathParameterToString(String collectionFormat, Collection value) {
// create the value based on the collection format
if ("multi".equals(collectionFormat)) {
// not valid for path params
return parameterToString(value);
}

// collectionFormat is assumed to be "csv" by default
String delimiter = ",";

if ("ssv".equals(collectionFormat)) {
delimiter = " ";
} else if ("tsv".equals(collectionFormat)) {
delimiter = "\t";
} else if ("pipes".equals(collectionFormat)) {
delimiter = "|";
}

StringBuilder sb = new StringBuilder() ;
for (Object item : value) {
sb.append(delimiter);
sb.append(parameterToString(item));
}

return sb.substring(delimiter.length());
}

/**
* Check if the given MIME is a JSON MIME.
* JSON MIME examples:
Expand Down Expand Up @@ -534,7 +429,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, List<Pair> queryParams, Object body, Map<String, String> headerParams, ApiCallback callback) throws AlgoliaRuntimeException {
public Call buildCall(String path, String method, Map<String, String> queryParams, Object body, Map<String, String> headerParams, ApiCallback callback) throws AlgoliaRuntimeException {
Request request = buildRequest(path, method, queryParams, body, headerParams, callback);

return requester.newCall(request);
Expand All @@ -552,19 +447,17 @@ public class ApiClient {
* @return The HTTP request
* @throws AlgoliaRuntimeException If fail to serialize the request body object
*/
public Request buildRequest(String path, String method, List<Pair> queryParams, Object body, Map<String, String> headerParams, ApiCallback callback) throws AlgoliaRuntimeException {
public Request buildRequest(String path, String method, Map<String, String> queryParams, Object body, Map<String, String> headerParams, ApiCallback callback) throws AlgoliaRuntimeException {
headerParams.put("X-Algolia-Application-Id", this.appId);
headerParams.put("X-Algolia-API-Key", this.apiKey);
headerParams.put("Accept", "application/json");
headerParams.put("Content-Type", "application/json");
Comment on lines +453 to +454
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those were in every methods so I've moved them here


final String url = buildUrl(path, queryParams);
final Request.Builder reqBuilder = new Request.Builder().url(url);
processHeaderParams(headerParams, reqBuilder);

String contentType = (String) headerParams.get("Content-Type");
// ensuring a default content type
if (contentType == null) {
contentType = "application/json";
}
Comment on lines -564 to -567
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we set it right above, it can't be null


RequestBody reqBody;
if (!HttpMethod.permitsRequestBody(method)) {
Expand Down Expand Up @@ -604,7 +497,7 @@ public class ApiClient {
* @param queryParams The query parameters
* @return The full URL
*/
public String buildUrl(String path, List<Pair> queryParams) {
public String buildUrl(String path, Map<String, String> queryParams) {
final StringBuilder url = new StringBuilder();

//The real host will be assigned by the retry strategy
Expand All @@ -613,7 +506,7 @@ public class ApiClient {
if (queryParams != null && !queryParams.isEmpty()) {
// support (constant) query string in `path`, e.g. "/posts?draft=1"
String prefix = path.contains("?") ? "&" : "?";
for (Pair param : queryParams) {
for (Entry<String, String> param : queryParams.entrySet()) {
if (param.getValue() != null) {
if (prefix != null) {
url.append(prefix);
Expand All @@ -622,7 +515,7 @@ public class ApiClient {
url.append("&");
}
String value = parameterToString(param.getValue());
url.append(escapeString(param.getName())).append("=").append(escapeString(value));
url.append(escapeString(param.getKey())).append("=").append(escapeString(value));
}
}
}
Expand Down
8 changes: 3 additions & 5 deletions templates/java/libraries/okhttp-gson/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -120,17 +120,17 @@ 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}}List<Pair> queryParams = new {{javaUtilPrefix}}ArrayList<Pair>();
{{javaUtilPrefix}}Map<String, String> queryParams = new {{javaUtilPrefix}}HashMap<String, String>();
{{javaUtilPrefix}}Map<String, String> headers = new {{javaUtilPrefix}}HashMap<String, String>();

{{#vendorExtensions}}{{#queryParams}}
if ({{paramName}} != null) {
{{^x-is-custom-request}}
queryParams.addAll(this.parameterToPair("{{baseName}}", {{paramName}}));
queryParams.put("{{baseName}}", parameterToString({{paramName}}));
{{/x-is-custom-request}}
{{#x-is-custom-request}}
for (Map.Entry<String, Object> parameter : parameters.entrySet()) {
queryParams.addAll(this.parameterToPair(parameter.getKey(), parameter.getValue()));
queryParams.put(parameter.getKey().toString(), parameterToString(parameter.getValue()));
}
{{/x-is-custom-request}}
}
Expand All @@ -142,8 +142,6 @@ public class {{classname}} extends ApiClient {
}

{{/headerParams}}
headers.put("Accept", "application/json");
headers.put("Content-Type", "application/json");

return this.buildCall(requestPath, "{{httpMethod}}", queryParams, bodyObj, headers, callback);
}
Expand Down
1 change: 1 addition & 0 deletions templates/javascript/api-single.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ export function create{{capitalizedApiName}}(options: CreateClientOptions{{#hasR
{{/queryParams.0}}
{{/bodyParams.0}}
{{/allParams.0}}
* @param requestOptions - The requestOptions to send along with the query, they will be merged with the transporter requestOptions.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was missing in the JS client

*/
{{nickname}}(
{{#allParams.0}}
Expand Down
6 changes: 3 additions & 3 deletions tests/CTS/methods/requests/templates/java/requests.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,10 @@ class {{client}}Tests {

{{#request.queryParameters}}
Map<String, String> expectedQuery = JSON.deserialize("{{#lambda.escapequotes}}{{{request.queryParameters}}}{{/lambda.escapequotes}}", new TypeToken<HashMap<String, String>>() {}.getType());
List<Pair> actualQuery = req.getQueryParams();
Map<String, String> actualQuery = req.getQueryParams();
assertEquals(expectedQuery.size(), actualQuery.size());
for (Pair p : actualQuery) {
assertEquals(expectedQuery.get(p.getName()), p.getValue());
for (Map.Entry<String, String> p : actualQuery.entrySet()) {
assertEquals(expectedQuery.get(p.getKey()), p.getValue());
}
{{/request.queryParameters}}
}
Expand Down
Loading