Skip to content

Commit e574bd6

Browse files
committed
fix(clients): timeouts at client init are overridden by method timeouts
1 parent 76a16db commit e574bd6

File tree

17 files changed

+133
-60
lines changed

17 files changed

+133
-60
lines changed

clients/algoliasearch-client-java/algoliasearch/src/main/java/com/algolia/ApiClient.java

Lines changed: 4 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import com.fasterxml.jackson.core.type.TypeReference;
1212
import java.io.Closeable;
1313
import java.io.IOException;
14-
import java.time.Duration;
1514
import java.util.List;
1615
import java.util.concurrent.CompletableFuture;
1716
import java.util.concurrent.ExecutorService;
@@ -32,16 +31,7 @@ public abstract class ApiClient implements Closeable {
3231
public AuthInterceptor authInterceptor;
3332

3433
/** Constructs a new instance of the {@link ApiClient}. */
35-
protected ApiClient(
36-
String appId,
37-
String apiKey,
38-
String clientName,
39-
@Nullable ClientOptions options,
40-
List<Host> defaultHosts,
41-
Duration connectTimeout,
42-
Duration readTimeout,
43-
Duration writeTimeout
44-
) {
34+
protected ApiClient(String appId, String apiKey, String clientName, @Nullable ClientOptions options, List<Host> defaultHosts) {
4535
if (appId == null || appId.isEmpty()) {
4636
throw new AlgoliaRuntimeException("`appId` is missing.");
4737
}
@@ -52,20 +42,11 @@ protected ApiClient(
5242
executor = clientOptions.getExecutor();
5343
requester = clientOptions.getCustomRequester() != null
5444
? clientOptions.getCustomRequester()
55-
: defaultRequester(appId, apiKey, clientName, clientOptions, defaultHosts, connectTimeout, readTimeout, writeTimeout);
45+
: defaultRequester(appId, apiKey, clientName, clientOptions, defaultHosts);
5646
}
5747

5848
/** Creates a default {@link Requester} for executing API requests. */
59-
private Requester defaultRequester(
60-
String appId,
61-
String apiKey,
62-
String clientName,
63-
ClientOptions options,
64-
List<Host> defaultHosts,
65-
Duration connectTimeout,
66-
Duration readTimeout,
67-
Duration writeTimeout
68-
) {
49+
private Requester defaultRequester(String appId, String apiKey, String clientName, ClientOptions options, List<Host> defaultHosts) {
6950
AlgoliaAgent algoliaAgent = new AlgoliaAgent(BuildConfig.VERSION)
7051
.addSegment(new AlgoliaAgent.Segment(clientName, BuildConfig.VERSION))
7152
.addSegments(options.getAlgoliaAgentSegments());
@@ -78,10 +59,7 @@ private Requester defaultRequester(
7859
HttpRequester.Builder builder = new HttpRequester.Builder(serializer)
7960
.addInterceptor(authInterceptor)
8061
.addInterceptor(new UserAgentInterceptor(algoliaAgent))
81-
.addInterceptor(new RetryStrategy(statefulHosts))
82-
.setConnectTimeout(connectTimeout)
83-
.setReadTimeout(readTimeout)
84-
.setWriteTimeout(writeTimeout);
62+
.addInterceptor(new RetryStrategy(statefulHosts));
8563
if (options.getRequesterConfig() != null) {
8664
options.getRequesterConfig().accept(builder);
8765
}

generators/src/main/java/com/algolia/codegen/AlgoliaJavaGenerator.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ public OperationsMap postProcessOperationsWithModels(OperationsMap objs, List<Mo
9595
OperationsMap operations = super.postProcessOperationsWithModels(objs, models);
9696
ModelPruner.removeOrphanModelFiles(this, operations, models);
9797
Helpers.removeHelpers(operations);
98+
Timeouts.propagate(operations);
9899
GenericPropagator.propagateGenericsToOperations(operations, models);
99100
return operations;
100101
}

generators/src/main/java/com/algolia/codegen/cts/tests/TestsClient.java

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,16 +89,24 @@ public void run(Map<String, CodegenModel> models, Map<String, CodegenOperation>
8989
stepOut.put("isCreateClient", true); // TODO: remove once kotlin is converted
9090

9191
boolean hasCustomHosts = step.parameters != null && step.parameters.containsKey("customHosts");
92-
if (hasCustomHosts) testOut.put("useEchoRequester", false);
9392
stepOut.put("hasCustomHosts", hasCustomHosts);
9493
if (hasCustomHosts) {
94+
testOut.put("useEchoRequester", false);
9595
stepOut.put("customHosts", step.parameters.get("customHosts"));
9696
}
9797

98+
boolean hasCustomWriteTimeout = step.parameters != null && step.parameters.containsKey("writeTimeout");
99+
stepOut.put("hasCustomWriteTimeout", hasCustomWriteTimeout);
100+
if (hasCustomWriteTimeout) {
101+
stepOut.put("writeTimeout", step.parameters.get("writeTimeout"));
102+
}
103+
104+
stepOut.put("hasCustomClientCreate", hasCustomWriteTimeout || hasCustomHosts);
105+
98106
boolean hasTransformationRegion = step.parameters != null && step.parameters.containsKey("transformationRegion");
99-
if (hasTransformationRegion) testOut.put("useEchoRequester", false);
100107
stepOut.put("hasTransformationRegion", hasTransformationRegion);
101108
if (hasTransformationRegion) {
109+
testOut.put("useEchoRequester", false);
102110
stepOut.put("transformationRegion", step.parameters.get("transformationRegion"));
103111
}
104112

generators/src/main/java/com/algolia/codegen/utils/Timeouts.java

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import com.fasterxml.jackson.databind.*;
55
import io.swagger.v3.oas.models.OpenAPI;
66
import java.util.*;
7+
import org.openapitools.codegen.model.OperationsMap;
78

89
class TimeoutsValues {
910

@@ -20,9 +21,11 @@ class TimeoutsBundle {
2021

2122
public class Timeouts {
2223

24+
private static TimeoutsBundle defaults;
25+
2326
/** Inject timeouts in miliseconds into the given bundle, under the x-timeouts property * */
2427
public static void enrichBundle(OpenAPI spec, Map<String, Object> bundle) throws ConfigException {
25-
TimeoutsBundle defaults = new TimeoutsBundle();
28+
defaults = new TimeoutsBundle();
2629
// the default below are what the search API expect, which was previously used for any client
2730
defaults.browser.connect = 1000;
2831
defaults.browser.read = 2000;
@@ -33,18 +36,40 @@ public static void enrichBundle(OpenAPI spec, Map<String, Object> bundle) throws
3336
defaults.server.write = 30000;
3437

3538
TimeoutsBundle specTimeouts = new ObjectMapper().convertValue(spec.getExtensions().get("x-timeouts"), TimeoutsBundle.class);
36-
if (specTimeouts == null) {
37-
specTimeouts = new TimeoutsBundle();
38-
specTimeouts.browser = defaults.browser;
39-
specTimeouts.server = defaults.server;
40-
}
41-
if (specTimeouts.browser == null) {
42-
specTimeouts.browser = defaults.browser;
43-
}
44-
if (specTimeouts.server == null) {
45-
specTimeouts.server = defaults.server;
39+
if (specTimeouts != null) {
40+
if (specTimeouts.browser != null) {
41+
defaults.browser = specTimeouts.browser;
42+
}
43+
if (specTimeouts.server == null) {
44+
defaults.server = specTimeouts.server;
45+
}
4646
}
4747

48-
bundle.put("x-timeouts", specTimeouts);
48+
bundle.put("x-timeouts", defaults);
49+
}
50+
51+
public static void propagate(OperationsMap operations) throws ConfigException {
52+
operations
53+
.getOperations()
54+
.getOperation()
55+
.forEach(entry -> {
56+
if (!entry.vendorExtensions.containsKey("x-timeouts")) {
57+
Map<String, Object> vendor = new HashMap<>();
58+
59+
Map<String, Object> browser = new HashMap<>();
60+
browser.put("connect", defaults.browser.connect);
61+
browser.put("read", defaults.browser.read);
62+
browser.put("write", defaults.browser.write);
63+
vendor.put("browser", browser);
64+
65+
Map<String, Object> server = new HashMap<>();
66+
server.put("connect", defaults.server.connect);
67+
server.put("read", defaults.server.read);
68+
server.put("write", defaults.server.write);
69+
vendor.put("server", server);
70+
71+
entry.vendorExtensions.put("x-timeouts", vendor);
72+
}
73+
});
4974
}
5075
}

templates/csharp/tests/client/createClient.mustache

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
{{#hasCustomHosts}}
1+
{{#hasCustomClientCreate}}
22
{{clientPrefix}}Config _config = new {{clientPrefix}}Config("{{parametersWithDataTypeMap.appId.value}}","{{parametersWithDataTypeMap.apiKey.value}}"{{#hasRegionalHost}}{{#parametersWithDataTypeMap.region}},"{{parametersWithDataTypeMap.region.value}}"{{/parametersWithDataTypeMap.region}}{{/hasRegionalHost}}) {
3+
{{#hasCustomHosts}}
34
CustomHosts = new List<StatefulHost>
45
{
56
{{#customHosts}}new () {
@@ -11,8 +12,12 @@
1112
Accept = CallType.Read | CallType.Write,
1213
}{{^-last}},{{/-last}}{{/customHosts}}
1314
}{{#gzipEncoding}},Compression = CompressionType.Gzip{{/gzipEncoding}}
15+
{{/hasCustomHosts}}
16+
{{#hasCustomWriteTimeout}}
17+
WriteTimeout = {{writeTimeout}}
18+
{{/hasCustomWriteTimeout}}
1419
};
1520
{{^autoCreateClient}}var client = {{/autoCreateClient}}new {{client}}(_config);
16-
{{/hasCustomHosts}}
21+
{{/hasCustomClientCreate}}
1722
{{#useEchoRequester}}
1823
{{^autoCreateClient}}var client = {{/autoCreateClient}}new {{client}}(new {{clientPrefix}}Config("{{parametersWithDataTypeMap.appId.value}}","{{parametersWithDataTypeMap.apiKey.value}}"{{#hasRegionalHost}}{{#parametersWithDataTypeMap.region}},"{{parametersWithDataTypeMap.region.value}}"{{/parametersWithDataTypeMap.region}}{{/hasRegionalHost}}), _echo);{{/useEchoRequester}}

templates/go/tests/client/createClient.mustache

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ cfg = {{clientPrefix}}.{{clientName}}Configuration{
88
{{#hasCustomHosts}}
99
Hosts: []transport.StatefulHost{ {{#customHosts}}transport.NewStatefulHost("http", tests.GetLocalhost() + ":{{port}}", call.IsReadWrite),{{/customHosts}} },
1010
{{/hasCustomHosts}}
11+
{{#hasCustomWriteTimeout}}
12+
WriteTimeout: {{writeTimeout}},
13+
{{/hasCustomWriteTimeout}}
1114
{{#gzipEncoding}}
1215
Compression: compression.GZIP,
1316
{{/gzipEncoding}}

templates/java/api.mustache

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ public class {{classname}} extends ApiClient {
8585
}
8686

8787
public {{classname}}(String appId, String apiKey, String region, ClientOptions options) {
88-
super(appId, apiKey, "{{{baseName}}}", options, getDefaultHosts(region), Duration.ofMillis({{{x-timeouts.server.connect}}}L), Duration.ofMillis({{{x-timeouts.server.read}}}L), Duration.ofMillis({{{x-timeouts.server.write}}}L));
88+
super(appId, apiKey, "{{{baseName}}}", options, getDefaultHosts(region));
8989
}
9090

9191
{{/hasRegionalHost}}
@@ -95,7 +95,7 @@ public class {{classname}} extends ApiClient {
9595
}
9696

9797
public {{classname}}(String appId, String apiKey, ClientOptions options) {
98-
super(appId, apiKey, "{{{baseName}}}", options, getDefaultHosts({{#hostWithAppID}}appId{{/hostWithAppID}}), Duration.ofMillis({{{x-timeouts.server.connect}}}L), Duration.ofMillis({{{x-timeouts.server.read}}}L), Duration.ofMillis({{{x-timeouts.server.write}}}L));
98+
super(appId, apiKey, "{{{baseName}}}", options, getDefaultHosts({{#hostWithAppID}}appId{{/hostWithAppID}}));
9999
}
100100
{{/hasRegionalHost}}
101101

@@ -218,7 +218,7 @@ public class {{classname}} extends ApiClient {
218218
{{#headerParams}}.addHeader("{{baseName}}", {{paramName}}){{/headerParams}}
219219
{{#vendorExtensions}}{{#queryParams}}{{^x-is-custom-request}}.addQueryParameter("{{baseName}}", {{paramName}}){{/x-is-custom-request}}{{#x-is-custom-request}}.addQueryParameters(parameters){{/x-is-custom-request}}{{/queryParams}}{{/vendorExtensions}}
220220
.build();
221-
return executeAsync(request, {{#vendorExtensions.x-timeouts}}new RequestOptions().setReadTimeout(Duration.ofMillis({{{read}}}L)).setWriteTimeout(Duration.ofMillis({{{write}}}L)).setConnectTimeout(Duration.ofMillis({{{connect}}}L)).mergeRight({{/vendorExtensions.x-timeouts}}requestOptions{{#vendorExtensions.x-timeouts}}){{/vendorExtensions.x-timeouts}}, {{#vendorExtensions}}{{#x-is-generic}}{{{returnType}}}.class, innerType{{/x-is-generic}}{{/vendorExtensions}}{{^vendorExtensions.x-is-generic}}{{^returnType}}null{{/returnType}}{{#returnType}}new TypeReference<{{{.}}}>(){}{{/returnType}}{{/vendorExtensions.x-is-generic}});
221+
return executeAsync(request, new RequestOptions(){{#vendorExtensions.x-timeouts.server}}.setReadTimeout(clientOptions.getReadTimeout() != null ? clientOptions.getReadTimeout() : Duration.ofMillis({{{read}}}L)).setWriteTimeout(clientOptions.getWriteTimeout() != null ? clientOptions.getWriteTimeout() : Duration.ofMillis({{{write}}}L)).setConnectTimeout(clientOptions.getConnectTimeout() != null ? clientOptions.getConnectTimeout() : Duration.ofMillis({{{connect}}}L)){{/vendorExtensions.x-timeouts.server}}.mergeRight(requestOptions), {{#vendorExtensions}}{{#x-is-generic}}{{{returnType}}}.class, innerType{{/x-is-generic}}{{/vendorExtensions}}{{^vendorExtensions.x-is-generic}}{{^returnType}}null{{/returnType}}{{#returnType}}new TypeReference<{{{.}}}>(){}{{/returnType}}{{/vendorExtensions.x-is-generic}});
222222
}
223223

224224
{{! This case only sets `requestOptions` as optional }}

templates/java/tests/client/client.mustache

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,15 @@ class {{client}}ClientTests {
4242
}
4343

4444
{{client}} createClient() {
45-
return new {{client}}("appId", "apiKey", {{#hasRegionalHost}}"{{defaultRegion}}", {{/hasRegionalHost}}withEchoRequester());
45+
return new {{client}}("appId", "apiKey", {{#hasRegionalHost}}"{{defaultRegion}}", {{/hasRegionalHost}}withEchoRequester(null));
4646
}
4747

48-
private ClientOptions withEchoRequester() {
49-
return ClientOptions.builder().setRequesterConfig(requester -> requester.addInterceptor(echo)).build();
48+
private ClientOptions withEchoRequester(Integer writeTimeout) {
49+
if (writeTimeout == null) {
50+
return ClientOptions.builder().setRequesterConfig(requester -> requester.addInterceptor(echo)).build();
51+
}
52+
53+
return ClientOptions.builder().setRequesterConfig(requester -> requester.addInterceptor(echo)).setWriteTimeout(Duration.ofMillis(writeTimeout)).build();
5054
}
5155

5256
private ClientOptions withCustomHosts(List<Host> hosts, boolean gzipEncoding) {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,2 @@
1-
{{^autoCreateClient}}{{client}} client = {{/autoCreateClient}}new {{client}}("{{parametersWithDataTypeMap.appId.value}}","{{parametersWithDataTypeMap.apiKey.value}}"{{#hasRegionalHost}}{{#parametersWithDataTypeMap.region}},"{{parametersWithDataTypeMap.region.value}}"{{/parametersWithDataTypeMap.region}}{{/hasRegionalHost}}{{#useEchoRequester}},withEchoRequester(){{/useEchoRequester}}{{#hasCustomHosts}},withCustomHosts(Arrays.asList({{#customHosts}}new Host("true".equals(System.getenv("CI")) ? "localhost" : "host.docker.internal", EnumSet.of(CallType.READ, CallType.WRITE), "http", {{port}}){{^-last}},{{/-last}}{{/customHosts}}), {{gzipEncoding}}){{/hasCustomHosts}});
1+
{{^autoCreateClient}}{{client}} client = {{/autoCreateClient}}new {{client}}("{{parametersWithDataTypeMap.appId.value}}","{{parametersWithDataTypeMap.apiKey.value}}"{{#hasRegionalHost}}{{#parametersWithDataTypeMap.region}},"{{parametersWithDataTypeMap.region.value}}"{{/parametersWithDataTypeMap.region}}{{/hasRegionalHost}}{{#useEchoRequester}},withEchoRequester({{#hasCustomWriteTimeout}}{{writeTimeout}}{{/hasCustomWriteTimeout}}{{^hasCustomWriteTimeout}}null{{/hasCustomWriteTimeout}}){{/useEchoRequester}}{{#hasCustomHosts}},withCustomHosts(Arrays.asList({{#customHosts}}new Host("true".equals(System.getenv("CI")) ? "localhost" : "host.docker.internal", EnumSet.of(CallType.READ, CallType.WRITE), "http", {{port}}){{^-last}},{{/-last}}{{/customHosts}}), {{gzipEncoding}}){{/hasCustomHosts}});
22
{{#hasTransformationRegion}}client.setTransformationRegion("{{{transformationRegion}}}");{{/hasTransformationRegion}}

templates/javascript/tests/client/createClient.mustache

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919
{{/customHosts}}
2020
],
2121
{{/hasCustomHosts}}
22+
{{#hasCustomWriteTimeout}}
23+
writeTimeout: {{writeTimeout}},
24+
{{/hasCustomWriteTimeout}}
2225
{{#hasTransformationRegion}}
2326
transformation: { region : "{{{transformationRegion}}}" },
2427
{{/hasTransformationRegion}}
@@ -41,7 +44,10 @@
4144
},
4245
{{/customHosts}}
4346
]
44-
{{/hasCustomHosts}}
47+
{{/hasCustomHosts}}
48+
{{#hasCustomWriteTimeout}}
49+
writeTimeout: {{writeTimeout}},
50+
{{/hasCustomWriteTimeout}}
4551
},
4652
{{#hasRegionalHost}}
4753
// @ts-ignore

0 commit comments

Comments
 (0)