Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(specs): allow POST methods to send read requests #525

Merged
merged 8 commits into from
May 23, 2022
Merged
Show file tree
Hide file tree
Changes from 5 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
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package com.algolia.utils;

public class UseReadTransporter {}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.algolia.utils.retry;

import com.algolia.exceptions.*;
import com.algolia.utils.UseReadTransporter;
import com.algolia.utils.Utils;
import java.io.IOException;
import java.net.SocketTimeoutException;
Expand Down Expand Up @@ -28,8 +29,11 @@ public Interceptor getRetryInterceptor() {
@Override
public Response intercept(Chain chain) throws IOException {
Request request = chain.request();
UseReadTransporter useReadTransporter = (UseReadTransporter) request.tag();
Iterator<StatefulHost> hostsIter = getTryableHosts(
request.method().equals("GET") ? CallType.READ : CallType.WRITE
(useReadTransporter != null || request.method().equals("GET"))
? CallType.READ
: CallType.WRITE
)
.iterator();
while (hostsIter.hasNext()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ export function createTransporter({
requestOptions: RequestOptions
): Promise<TResponse> {
const stackTrace: StackFrame[] = [];
const isRead = request.method === 'GET';
const isRead = request?.useReadTransporter || request.method === 'GET';
shortcuts marked this conversation as resolved.
Show resolved Hide resolved

/**
* First we prepare the payload that do not depend from hosts.
Expand All @@ -99,12 +99,13 @@ export function createTransporter({
const method = request.method;

// On `GET`, the data is proxied to query parameters.
const dataQueryParameters: Record<string, any> = isRead
? {
...request.data,
...requestOptions.data,
}
: {};
const dataQueryParameters: Record<string, any> =
request.method === 'GET'
? {
...request.data,
...requestOptions.data,
}
: {};

const queryParameters: QueryParameters = {
'x-algolia-agent': algoliaAgent.value,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ export type Request = {
path: string;
data?: Array<Record<string, any>> | Record<string, any>;
cacheable?: boolean;
/**
* Some POST methods in the Algolia REST API uses the `read` transporter.
* This information is defined at the spec level.
*/
useReadTransporter?: boolean;
};

export type EndRequest = {
Expand Down
44 changes: 21 additions & 23 deletions clients/algoliasearch-client-php/lib/RetryStrategy/ApiWrapper.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,30 +66,24 @@ public function __construct(
}
}

public function read($method, $path, $requestOptions = [])
{
if ('GET' === mb_strtoupper($method)) {
public function sendRequest(
$method,
$path,
$data = [],
$requestOptions = [],
$useReadTransporter = false
) {
/**
* Some POST methods in the Algolia REST API uses the `read` transporter.
* This information is defined at the spec level.
*/
$isRead = $useReadTransporter || 'GET' === mb_strtoupper($method);

if ($isRead) {
$requestOptions = $this->requestOptionsFactory->createBodyLess(
$requestOptions
);
} else {
$requestOptions = $this->requestOptionsFactory->create(
$requestOptions
);
}

return $this->request(
$method,
$path,
$requestOptions,
$this->clusterHosts->read(),
$requestOptions->getReadTimeout()
);
}

public function write($method, $path, $data = [], $requestOptions = [])
{
if ('DELETE' === mb_strtoupper($method)) {
} elseif ('DELETE' === mb_strtoupper($method)) {
$requestOptions = $this->requestOptionsFactory->createBodyLess(
$requestOptions
);
Expand All @@ -104,8 +98,12 @@ public function write($method, $path, $data = [], $requestOptions = [])
$method,
$path,
$requestOptions,
$this->clusterHosts->write(),
$requestOptions->getWriteTimeout(),
$isRead
? $this->clusterHosts->read()
: $this->clusterHosts->write(),
$isRead
? $requestOptions->getReadTimeout()
: $requestOptions->getWriteTimeout(),
$data
shortcuts marked this conversation as resolved.
Show resolved Hide resolved
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,13 @@

interface ApiWrapperInterface
{
public function read($method, $path, $requestOptions = []);

public function write($method, $path, $data = [], $requestOptions = []);
public function sendRequest(
$method,
$path,
$data = [],
$requestOptions = [],
$useReadTransporter = false
);

public function send($method, $path, $requestOptions = [], $hosts = null);
}
4 changes: 2 additions & 2 deletions scripts/playground.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ export async function playground({
break;
case 'php':
await run(
`cd playground/php && \
`cd clients/algoliasearch-client-php/ && \
composer update && \
composer dump-autoload && \
cd src && \
cd ../../playground/php/src && \
php8 ${client}.php`,
{ verbose }
);
Expand Down
1 change: 1 addition & 0 deletions specs/recommend/paths/getRecommendations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- recommendations
operationId: getRecommendations
x-use-read-transporter: true
summary: Get results.
description: Returns recommendations or trending results, for a specific model and `objectID`.
requestBody:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Dictionaries
operationId: searchDictionaryEntries
x-use-read-transporter: true
shortcuts marked this conversation as resolved.
Show resolved Hide resolved
description: Search the dictionary entries.
summary: Search a dictionary entries.
parameters:
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/multiclusters/searchUserIds.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Clusters
operationId: searchUserIds
x-use-read-transporter: true
summary: Search userID.
description: >
Search for userIDs.
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/objects/multipleGetObjects.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Records
operationId: getObjects
x-use-read-transporter: true
summary: Retrieve one or more objects.
description: Retrieve one or more objects, potentially from different indices, in a single API call.
requestBody:
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/rules/searchRules.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Rules
operationId: searchRules
x-use-read-transporter: true
summary: Search for rules.
description: Search for rules matching various criteria.
parameters:
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/search/search.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Search
operationId: search
x-use-read-transporter: true
summary: Search multiple indices.
description: Perform a search operation targeting one or many indices.
requestBody:
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/search/searchForFacetValues.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Search
operationId: searchForFacetValues
x-use-read-transporter: true
summary: Search for values of a given facet.
description: Search for values of a given facet, optionally restricting the returned values to those contained in objects matching other search criteria.
parameters:
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/search/searchSingleIndex.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Search
operationId: searchSingleIndex
x-use-read-transporter: true
summary: Search in a single index.
description: Perform a search operation targeting one specific index.
parameters:
Expand Down
1 change: 1 addition & 0 deletions specs/search/paths/synonyms/searchSynonyms.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ post:
tags:
- Synonyms
operationId: searchSynonyms
x-use-read-transporter: true
summary: Search synonyms.
description: Search or browse all synonyms, optionally filtering them by type.
parameters:
Expand Down
93 changes: 45 additions & 48 deletions templates/java/libraries/okhttp-gson/ApiClient.mustache
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package {{invokerPackage}};

import com.algolia.utils.Requester;
import com.algolia.exceptions.*;
import com.algolia.utils.AlgoliaAgent;
import com.algolia.utils.JSON;
import com.algolia.utils.RequestOptions;
import com.algolia.utils.*;

import okhttp3.*;
import okhttp3.internal.http.HttpMethod;
Expand Down Expand Up @@ -270,11 +267,12 @@ public class ApiClient {
* @param body The request body object
* @param headerParams The header parameters
* @param requestOptions The requestOptions to send along with the query, they will be merged with the transporter requestOptions.
* @param useReadTransporter Some POST methods in the Algolia REST API uses the `read` transporter. This information is defined at the spec level.
* @return The HTTP call
* @throws AlgoliaRuntimeException If fail to serialize the request body object
*/
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);
public Call buildCall(String path, String method, Map<String, Object> queryParameters, Object body, Map<String, String> headerParams, RequestOptions requestOptions, Boolean useReadTransporter) throws AlgoliaRuntimeException {
Request request = buildRequest(path, method, queryParameters, body, headerParams, requestOptions, useReadTransporter);

return requester.newCall(request);
}
Expand All @@ -288,39 +286,52 @@ public class ApiClient {
* @param body The request body object
* @param headerParams The header parameters
* @param requestOptions The requestOptions to send along with the query, they will be merged with the transporter requestOptions.
* @param useReadTransporter Some POST methods in the Algolia REST API uses the `read` transporter. This information is defined at the spec level.
* @return The HTTP request
* @throws AlgoliaRuntimeException If fail to serialize the request body object
*/
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,
queryParameters,
hasRequestOptions ? requestOptions.getExtraQueryParameters() : null
);
final Request.Builder reqBuilder = new Request.Builder().url(url);
processHeaderParams(
headerParams,
hasRequestOptions ? requestOptions.getExtraHeaders() : null,
reqBuilder
);
public Request buildRequest(
String path,
String method,
Map<String, Object> queryParameters,
Object body,
Map<String, String> headerParams,
RequestOptions requestOptions,
Boolean useReadTransporter
) throws AlgoliaRuntimeException {
boolean hasRequestOptions = requestOptions != null;
final String url = buildUrl(
path,
queryParameters,
hasRequestOptions ? requestOptions.getExtraQueryParameters() : null
);
final Request.Builder reqBuilder = new Request.Builder().url(url);
processHeaderParams(
headerParams,
hasRequestOptions ? requestOptions.getExtraHeaders() : null,
reqBuilder
);

RequestBody reqBody;
if (!HttpMethod.permitsRequestBody(method)) {
reqBody = null;
} else if (body == null) {
if ("DELETE".equals(method)) {
// allow calling DELETE without sending a request body
reqBody = null;
} else {
// use an empty request body (for POST, PUT and PATCH)
reqBody = RequestBody.create("", MediaType.parse(this.contentType));
}
} else {
reqBody = serialize(body);
}
RequestBody reqBody;
if (!HttpMethod.permitsRequestBody(method)) {
reqBody = null;
} else if (body == null) {
if ("DELETE".equals(method)) {
// allow calling DELETE without sending a request body
reqBody = null;
} else {
// use an empty request body (for POST, PUT and PATCH)
reqBody = RequestBody.create("", MediaType.parse(this.contentType));
}
} else {
reqBody = serialize(body);
}

return reqBuilder.method(method, reqBody).build();
if (useReadTransporter) {
reqBuilder.tag(new UseReadTransporter());
}
Comment on lines +330 to +332
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 is the only difference, the rest is indenting sorry


return reqBuilder.method(method, reqBody).build();
}

/**
Expand Down Expand Up @@ -401,18 +412,4 @@ public class ApiClient {
}
}
}

/**
Copy link
Member Author

Choose a reason for hiding this comment

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

It was not used, could you confirm?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I concur

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok my lord

* Build a form-encoding request body with the given form parameters.
*
* @param formParams Form parameters in the form of Map
* @return RequestBody
*/
public RequestBody buildRequestBodyFormEncoding(Map<String, Object> formParams) {
okhttp3.FormBody.Builder formBuilder = new okhttp3.FormBody.Builder();
for (Entry<String, Object> param : formParams.entrySet()) {
formBuilder.add(param.getKey(), parameterToString(param.getValue()));
}
return formBuilder.build();
}
}
2 changes: 1 addition & 1 deletion templates/java/libraries/okhttp-gson/api.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public class {{classname}} extends ApiClient {
}

{{/headerParams}}
Call call = this.buildCall(requestPath, "{{httpMethod}}", queryParameters, bodyObj, headers, requestOptions);
Call call = this.buildCall(requestPath, "{{httpMethod}}", queryParameters, bodyObj, headers, requestOptions, {{#vendorExtensions.x-use-read-transporter}}true{{/vendorExtensions.x-use-read-transporter}}{{^vendorExtensions.x-use-read-transporter}}false{{/vendorExtensions.x-use-read-transporter}});
Type returnType = new TypeToken<{{{returnType}}}>() {}.getType();
return this.executeAsync(call, returnType);
}
Expand Down
3 changes: 3 additions & 0 deletions templates/javascript/api-single.mustache
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,9 @@ export function create{{capitalizedApiName}}(options: CreateClientOptions{{#hasR
{{#bodyParam}}
data: {{paramName}},
{{/bodyParam}}
{{#vendorExtensions.x-use-read-transporter}}
useReadTransporter: true,
{{/vendorExtensions.x-use-read-transporter}}
};

return transporter.request(request, {
Expand Down
Loading