[Java] Fix for issue #3638: Generated Java APIs incorrectly encode commas in certain query parameters (#4908)

* Fix for issue #3638

* Update tests for fix for issue #3638

* Fix bug when queryParams and collectionQueryParams are both specified

* Update tests

* Add certain tests back, address CI failures
This commit is contained in:
Michael Kourlas
2017-06-14 12:20:04 -04:00
committed by wing328
parent 1a421112ed
commit d20f83e643
22 changed files with 557 additions and 275 deletions

View File

@@ -407,62 +407,71 @@ public class ApiClient {
}
}
/*
* Format to {@code Pair} objects.
* @param collectionFormat Collection format
* @param name Name
* @param value Value
* @return List of pair
/**
* 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> parameterToPairs(String collectionFormat, String name, Object value){
public List<Pair> parameterToPair(String name, Object value) {
List<Pair> params = new ArrayList<Pair>();
// preconditions
if (name == null || name.isEmpty() || value == null) return params;
if (name == null || name.isEmpty() || value == null || value instanceof Collection) return params;
Collection<?> valueCollection;
if (value instanceof Collection<?>) {
valueCollection = (Collection<?>) value;
} else {
params.add(new Pair(name, parameterToString(value)));
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) {
return params;
}
if (valueCollection.isEmpty()){
return params;
}
// get the collection format
String format = (collectionFormat == null || collectionFormat.isEmpty() ? "csv" : collectionFormat); // default: csv
// create the params based on the collection format
if ("multi".equals(format)) {
for (Object item : valueCollection) {
params.add(new Pair(name, parameterToString(item)));
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 = ",";
if ("csv".equals(format)) {
delimiter = ",";
} else if ("ssv".equals(format)) {
delimiter = " ";
} else if ("tsv".equals(format)) {
delimiter = "\t";
} else if ("pipes".equals(format)) {
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 : valueCollection) {
for (Object item : value) {
sb.append(delimiter);
sb.append(parameterToString(item));
sb.append(escapeString(parameterToString(item)));
}
params.add(new Pair(name, sb.substring(1)));
params.add(new Pair(name, sb.substring(delimiter.length())));
return params;
}
@@ -578,9 +587,10 @@ public class ApiClient {
*
* @param path The sub path
* @param queryParams The query parameters
* @param collectionQueryParams The collection query parameters
* @return The full URL
*/
private String buildUrl(String path, List<Pair> queryParams) {
private String buildUrl(String path, List<Pair> queryParams, List<Pair> collectionQueryParams) {
final StringBuilder url = new StringBuilder();
url.append(basePath).append(path);
@@ -601,17 +611,34 @@ public class ApiClient {
}
}
if (collectionQueryParams != null && !collectionQueryParams.isEmpty()) {
String prefix = url.toString().contains("?") ? "&" : "?";
for (Pair param : collectionQueryParams) {
if (param.getValue() != null) {
if (prefix != null) {
url.append(prefix);
prefix = null;
} else {
url.append("&");
}
String value = parameterToString(param.getValue());
// collection query parameter value already escaped as part of parameterToPairs
url.append(escapeString(param.getName())).append("=").append(value);
}
}
}
return url.toString();
}
private ClientResponse getAPIResponse(String path, String method, List<Pair> queryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String accept, String contentType, String[] authNames) throws ApiException {
private ClientResponse getAPIResponse(String path, String method, List<Pair> queryParams, List<Pair> collectionQueryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String accept, String contentType, String[] authNames) throws ApiException {
if (body != null && !formParams.isEmpty()) {
throw new ApiException(500, "Cannot have body and form params");
}
updateParamsForAuth(authNames, queryParams, headerParams);
final String url = buildUrl(path, queryParams);
final String url = buildUrl(path, queryParams, collectionQueryParams);
Builder builder;
if (accept == null) {
builder = httpClient.resource(url).getRequestBuilder();
@@ -654,6 +681,7 @@ public class ApiClient {
* @param path The sub-path of the HTTP URL
* @param method The request method, one of "GET", "POST", "PUT", and "DELETE"
* @param queryParams The query parameters
* @param collectionQueryParams The collection query parameters
* @param body The request body object - if it is not binary, otherwise null
* @param headerParams The header parameters
* @param formParams The form parameters
@@ -664,9 +692,9 @@ public class ApiClient {
* @return The response body in type of string
* @throws ApiException API exception
*/
public <T> T invokeAPI(String path, String method, List<Pair> queryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String accept, String contentType, String[] authNames, GenericType<T> returnType) throws ApiException {
public <T> T invokeAPI(String path, String method, List<Pair> queryParams, List<Pair> collectionQueryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String accept, String contentType, String[] authNames, GenericType<T> returnType) throws ApiException {
ClientResponse response = getAPIResponse(path, method, queryParams, body, headerParams, formParams, accept, contentType, authNames);
ClientResponse response = getAPIResponse(path, method, queryParams, collectionQueryParams, body, headerParams, formParams, accept, contentType, authNames);
statusCode = response.getStatusInfo().getStatusCode();
responseHeaders = response.getHeaders();

View File

@@ -67,11 +67,12 @@ public class {{classname}} {
// query params
{{javaUtilPrefix}}List<Pair> {{localVariablePrefix}}localVarQueryParams = new {{javaUtilPrefix}}ArrayList<Pair>();
{{javaUtilPrefix}}List<Pair> {{localVariablePrefix}}localVarCollectionQueryParams = new {{javaUtilPrefix}}ArrayList<Pair>();
{{javaUtilPrefix}}Map<String, String> {{localVariablePrefix}}localVarHeaderParams = new {{javaUtilPrefix}}HashMap<String, String>();
{{javaUtilPrefix}}Map<String, Object> {{localVariablePrefix}}localVarFormParams = new {{javaUtilPrefix}}HashMap<String, Object>();
{{#queryParams}}
{{localVariablePrefix}}localVarQueryParams.addAll({{localVariablePrefix}}apiClient.parameterToPairs("{{#collectionFormat}}{{{collectionFormat}}}{{/collectionFormat}}", "{{baseName}}", {{paramName}}));
{{localVariablePrefix}}{{#collectionFormat}}localVarCollectionQueryParams.addAll({{localVariablePrefix}}apiClient.parameterToPairs("{{{collectionFormat}}}", {{/collectionFormat}}{{^collectionFormat}}localVarQueryParams.addAll({{localVariablePrefix}}apiClient.parameterToPair({{/collectionFormat}}"{{baseName}}", {{paramName}}));
{{/queryParams}}
{{#headerParams}}if ({{paramName}} != null)
@@ -96,9 +97,9 @@ public class {{classname}} {
{{#returnType}}
GenericType<{{{returnType}}}> {{localVariablePrefix}}localVarReturnType = new GenericType<{{{returnType}}}>() {};
return {{localVariablePrefix}}apiClient.invokeAPI({{localVariablePrefix}}localVarPath, "{{httpMethod}}", {{localVariablePrefix}}localVarQueryParams, {{localVariablePrefix}}localVarPostBody, {{localVariablePrefix}}localVarHeaderParams, {{localVariablePrefix}}localVarFormParams, {{localVariablePrefix}}localVarAccept, {{localVariablePrefix}}localVarContentType, {{localVariablePrefix}}localVarAuthNames, {{localVariablePrefix}}localVarReturnType);
return {{localVariablePrefix}}apiClient.invokeAPI({{localVariablePrefix}}localVarPath, "{{httpMethod}}", {{localVariablePrefix}}localVarQueryParams, {{localVariablePrefix}}localVarCollectionQueryParams, {{localVariablePrefix}}localVarPostBody, {{localVariablePrefix}}localVarHeaderParams, {{localVariablePrefix}}localVarFormParams, {{localVariablePrefix}}localVarAccept, {{localVariablePrefix}}localVarContentType, {{localVariablePrefix}}localVarAuthNames, {{localVariablePrefix}}localVarReturnType);
{{/returnType}}{{^returnType}}
{{localVariablePrefix}}apiClient.invokeAPI({{localVariablePrefix}}localVarPath, "{{httpMethod}}", {{localVariablePrefix}}localVarQueryParams, {{localVariablePrefix}}localVarPostBody, {{localVariablePrefix}}localVarHeaderParams, {{localVariablePrefix}}localVarFormParams, {{localVariablePrefix}}localVarAccept, {{localVariablePrefix}}localVarContentType, {{localVariablePrefix}}localVarAuthNames, null);
{{localVariablePrefix}}apiClient.invokeAPI({{localVariablePrefix}}localVarPath, "{{httpMethod}}", {{localVariablePrefix}}localVarQueryParams, {{localVariablePrefix}}localVarCollectionQueryParams, {{localVariablePrefix}}localVarPostBody, {{localVariablePrefix}}localVarHeaderParams, {{localVariablePrefix}}localVarFormParams, {{localVariablePrefix}}localVarAccept, {{localVariablePrefix}}localVarContentType, {{localVariablePrefix}}localVarAuthNames, null);
{{/returnType}}
}
{{/operation}}

View File

@@ -485,62 +485,70 @@ public class ApiClient {
}
/**
* Format to {@code Pair} objects.
* Formats the specified query parameter to a list containing a single {@code Pair} object.
*
* @param collectionFormat collection format (e.g. csv, tsv)
* @param name Name
* @param value Value
* @return A list of Pair objects
* 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> parameterToPairs(String collectionFormat, String name, Object value){
public List<Pair> parameterToPair(String name, Object value) {
List<Pair> params = new ArrayList<Pair>();
// preconditions
if (name == null || name.isEmpty() || value == null) return params;
if (name == null || name.isEmpty() || value == null || value instanceof Collection) return params;
Collection valueCollection = null;
if (value instanceof Collection) {
valueCollection = (Collection) value;
} else {
params.add(new Pair(name, parameterToString(value)));
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;
}
if (valueCollection.isEmpty()){
return params;
}
// get the collection format
collectionFormat = (collectionFormat == null || collectionFormat.isEmpty() ? "csv" : collectionFormat); // default: csv
// create the params based on the collection format
if (collectionFormat.equals("multi")) {
for (Object item : valueCollection) {
params.add(new Pair(name, parameterToString(item)));
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 = ",";
if (collectionFormat.equals("csv")) {
delimiter = ",";
} else if (collectionFormat.equals("ssv")) {
delimiter = " ";
} else if (collectionFormat.equals("tsv")) {
delimiter = "\t";
} else if (collectionFormat.equals("pipes")) {
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 : valueCollection) {
for (Object item : value) {
sb.append(delimiter);
sb.append(parameterToString(item));
sb.append(escapeString(parameterToString(item)));
}
params.add(new Pair(name, sb.substring(1)));
params.add(new Pair(name, sb.substring(delimiter.length())));
return params;
}
@@ -900,6 +908,7 @@ public class ApiClient {
* @param path The sub-path of the HTTP URL
* @param method The request method, one of "GET", "HEAD", "OPTIONS", "POST", "PUT", "PATCH" and "DELETE"
* @param queryParams The query parameters
* @param collectionQueryParams The collection query parameters
* @param body The request body object
* @param headerParams The header parameters
* @param formParams The form parameters
@@ -908,8 +917,8 @@ public class ApiClient {
* @return The HTTP call
* @throws ApiException If fail to serialize the request body object
*/
public Call buildCall(String path, String method, List<Pair> queryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String[] authNames, ProgressRequestBody.ProgressRequestListener progressRequestListener) throws ApiException {
Request request = buildRequest(path, method, queryParams, body, headerParams, formParams, authNames, progressRequestListener);
public Call buildCall(String path, String method, List<Pair> queryParams, List<Pair> collectionQueryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String[] authNames, ProgressRequestBody.ProgressRequestListener progressRequestListener) throws ApiException {
Request request = buildRequest(path, method, queryParams, collectionQueryParams, body, headerParams, formParams, authNames, progressRequestListener);
return httpClient.newCall(request);
}
@@ -920,6 +929,7 @@ public class ApiClient {
* @param path The sub-path of the HTTP URL
* @param method The request method, one of "GET", "HEAD", "OPTIONS", "POST", "PUT", "PATCH" and "DELETE"
* @param queryParams The query parameters
* @param collectionQueryParams The collection query parameters
* @param body The request body object
* @param headerParams The header parameters
* @param formParams The form parameters
@@ -928,10 +938,10 @@ public class ApiClient {
* @return The HTTP request
* @throws ApiException If fail to serialize the request body object
*/
public Request buildRequest(String path, String method, List<Pair> queryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String[] authNames, ProgressRequestBody.ProgressRequestListener progressRequestListener) throws ApiException {
public Request buildRequest(String path, String method, List<Pair> queryParams, List<Pair> collectionQueryParams, Object body, Map<String, String> headerParams, Map<String, Object> formParams, String[] authNames, ProgressRequestBody.ProgressRequestListener progressRequestListener) throws ApiException {
updateParamsForAuth(authNames, queryParams, headerParams);
final String url = buildUrl(path, queryParams);
final String url = buildUrl(path, queryParams, collectionQueryParams);
final Request.Builder reqBuilder = new Request.Builder().url(url);
processHeaderParams(headerParams, reqBuilder);
@@ -977,9 +987,10 @@ public class ApiClient {
*
* @param path The sub path
* @param queryParams The query parameters
* @param collectionQueryParams The collection query parameters
* @return The full URL
*/
public String buildUrl(String path, List<Pair> queryParams) {
public String buildUrl(String path, List<Pair> queryParams, List<Pair> collectionQueryParams) {
final StringBuilder url = new StringBuilder();
url.append(basePath).append(path);
@@ -1000,6 +1011,23 @@ public class ApiClient {
}
}
if (collectionQueryParams != null && !collectionQueryParams.isEmpty()) {
String prefix = url.toString().contains("?") ? "&" : "?";
for (Pair param : collectionQueryParams) {
if (param.getValue() != null) {
if (prefix != null) {
url.append(prefix);
prefix = null;
} else {
url.append("&");
}
String value = parameterToString(param.getValue());
// collection query parameter value already escaped as part of parameterToPairs
url.append(escapeString(param.getName())).append("=").append(value);
}
}
}
return url.toString();
}

View File

@@ -78,9 +78,10 @@ public class {{classname}} {
String {{localVariablePrefix}}localVarPath = "{{{path}}}"{{#pathParams}}
.replaceAll("\\{" + "{{baseName}}" + "\\}", {{localVariablePrefix}}apiClient.escapeString({{{paramName}}}.toString())){{/pathParams}};
{{javaUtilPrefix}}List<Pair> {{localVariablePrefix}}localVarQueryParams = new {{javaUtilPrefix}}ArrayList<Pair>();{{#queryParams}}
{{javaUtilPrefix}}List<Pair> {{localVariablePrefix}}localVarQueryParams = new {{javaUtilPrefix}}ArrayList<Pair>();
{{javaUtilPrefix}}List<Pair> {{localVariablePrefix}}localVarCollectionQueryParams = new {{javaUtilPrefix}}ArrayList<Pair>();{{#queryParams}}
if ({{paramName}} != null)
{{localVariablePrefix}}localVarQueryParams.addAll({{localVariablePrefix}}apiClient.parameterToPairs("{{#collectionFormat}}{{{collectionFormat}}}{{/collectionFormat}}", "{{baseName}}", {{paramName}}));{{/queryParams}}
{{localVariablePrefix}}{{#collectionFormat}}localVarCollectionQueryParams.addAll({{localVariablePrefix}}apiClient.parameterToPairs("{{{collectionFormat}}}", {{/collectionFormat}}{{^collectionFormat}}localVarQueryParams.addAll({{localVariablePrefix}}apiClient.parameterToPair({{/collectionFormat}}"{{baseName}}", {{paramName}}));{{/queryParams}}
{{javaUtilPrefix}}Map<String, String> {{localVariablePrefix}}localVarHeaderParams = new {{javaUtilPrefix}}HashMap<String, String>();{{#headerParams}}
if ({{paramName}} != null)
@@ -115,7 +116,7 @@ public class {{classname}} {
}
String[] {{localVariablePrefix}}localVarAuthNames = new String[] { {{#authMethods}}"{{name}}"{{#hasMore}}, {{/hasMore}}{{/authMethods}} };
return {{localVariablePrefix}}apiClient.buildCall({{localVariablePrefix}}localVarPath, "{{httpMethod}}", {{localVariablePrefix}}localVarQueryParams, {{localVariablePrefix}}localVarPostBody, {{localVariablePrefix}}localVarHeaderParams, {{localVariablePrefix}}localVarFormParams, {{localVariablePrefix}}localVarAuthNames, progressRequestListener);
return {{localVariablePrefix}}apiClient.buildCall({{localVariablePrefix}}localVarPath, "{{httpMethod}}", {{localVariablePrefix}}localVarQueryParams, {{localVariablePrefix}}localVarCollectionQueryParams, {{localVariablePrefix}}localVarPostBody, {{localVariablePrefix}}localVarHeaderParams, {{localVariablePrefix}}localVarFormParams, {{localVariablePrefix}}localVarAuthNames, progressRequestListener);
}
@SuppressWarnings("rawtypes")
@@ -225,4 +226,4 @@ public class {{classname}} {
}
{{/operation}}
}
{{/operations}}
{{/operations}}