Skip to content

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

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.

Sign up for GitHub

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 6 commits into from
May 17, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
837b478
fix(clients): ensure `requestOptions` follow same format
shortcuts May 16, 2022
7191e79
fix `expectedQuery` type
shortcuts May 17, 2022
12cfa31
use type as current client for `queryParameters`
shortcuts May 17, 2022
9e21e56
skip stringify objects
shortcuts May 17, 2022
a91d4d9
remove unused factory import
shortcuts May 17, 2022
8fc5b3d
good now
shortcuts May 17, 2022
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
6 changes: 3 additions & 3 deletions ...arch-client-java-2/algoliasearch-core/src/main/java/com/algolia/utils/RequestOptions.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand All @@ -34,7 +34,7 @@ public Map<String, String> getExtraHeaders() {
return headers;
}

public Map<String, String> getExtraQueryParameters() {
public Map<String, Object> getExtraQueryParameters() {
return queryParameters;
}

Expand Down
21 changes: 19 additions & 2 deletions ...goliasearch-client-javascript/packages/client-common/src/transporter/createTransporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]'
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand how we can get object here

Copy link
Member Author

Choose a reason for hiding this comment

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

const requestOptions = {
  queryParameters: {
    myParam: { a: 'b' },
  },
}

idk if we should allow this in JS but as it's the case in the current client, we can keep the same logic

) {
queryParameters[key] = value;
} else {
queryParameters[key] = value.toString();
}
}
}

let timeoutsCount = 0;

const retry = async (
Expand Down
5 changes: 2 additions & 3 deletions clients/algoliasearch-client-javascript/packages/client-common/src/transporter/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type {
Host,
Request,
RequestOptions,
QueryParameters,
Response,
StackFrame,
} from '../types';
Expand Down Expand Up @@ -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]';
Expand Down
3 changes: 1 addition & 2 deletions clients/algoliasearch-client-javascript/packages/client-common/src/types/Transporter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
/**
Expand Down
18 changes: 7 additions & 11 deletions clients/algoliasearch-client-php/lib/RequestOptions/RequestOptionsFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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'];
}
}
43 changes: 17 additions & 26 deletions templates/java/libraries/okhttp-gson/ApiClient.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -376,7 +367,7 @@ public class ApiClient {
}
}

return url;
return url.toString();
}

/**
Expand Down
2 changes: 1 addition & 1 deletion templates/java/libraries/okhttp-gson/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
10 changes: 1 addition & 9 deletions templates/php/api.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}};
Expand Down
4 changes: 2 additions & 2 deletions tests/CTS/methods/requests/templates/java/requests.mustache
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}}
Expand Down
4 changes: 2 additions & 2 deletions tests/output/java/src/test/java/com/algolia/CallEcho.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
2 changes: 1 addition & 1 deletion tests/output/java/src/test/java/com/algolia/EchoResponse.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}