Skip to content

Commit b502c6e

Browse files
committed
feat: adds request timeout option for provider
Signed-off-by: Rahul Baradol <rahul.baradol.14@gmail.com>
1 parent 7ca1e33 commit b502c6e

5 files changed

Lines changed: 95 additions & 23 deletions

File tree

providers/ofrep/src/main/java/dev/openfeature/contrib/providers/ofrep/OfrepProvider.java

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,10 +44,16 @@ public static OfrepProvider constructProvider(OfrepProviderOptions options) {
4444
throw new IllegalArgumentException("Headers cannot be null");
4545
}
4646

47-
if (options.getTimeout() == null
48-
|| options.getTimeout().isNegative()
49-
|| options.getTimeout().isZero()) {
50-
throw new IllegalArgumentException("Timeout must be a positive duration");
47+
if (options.getRequestTimeout() == null
48+
|| options.getRequestTimeout().isNegative()
49+
|| options.getRequestTimeout().isZero()) {
50+
throw new IllegalArgumentException("Request timeout must be a positive duration");
51+
}
52+
53+
if (options.getConnectTimeout() == null
54+
|| options.getConnectTimeout().isNegative()
55+
|| options.getConnectTimeout().isZero()) {
56+
throw new IllegalArgumentException("Connect timeout must be a positive duration");
5157
}
5258

5359
if (options.getProxySelector() == null) {
@@ -70,7 +76,8 @@ private OfrepProvider(OfrepProviderOptions options) {
7076
this.ofrepResolver = new Resolver(
7177
options.getBaseUrl(),
7278
options.getHeaders(),
73-
options.getTimeout(),
79+
options.getRequestTimeout(),
80+
options.getConnectTimeout(),
7481
options.getProxySelector(),
7582
options.getExecutor());
7683
}

providers/ofrep/src/main/java/dev/openfeature/contrib/providers/ofrep/OfrepProviderOptions.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,10 @@ public class OfrepProviderOptions {
2828
private final Executor executor = Executors.newFixedThreadPool(DEFAULT_THREAD_POOL_SIZE);
2929

3030
@Builder.Default
31-
private final Duration timeout = Duration.ofSeconds(10);
31+
private final Duration requestTimeout = Duration.ofSeconds(10);
32+
33+
@Builder.Default
34+
private final Duration connectTimeout = Duration.ofSeconds(10);
3235

3336
@Builder.Default
3437
private final ImmutableMap<String, ImmutableList<String>> headers = ImmutableMap.of();

providers/ofrep/src/main/java/dev/openfeature/contrib/providers/ofrep/internal/OfrepApi.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,21 +27,24 @@ public class OfrepApi {
2727
private final ObjectMapper serializer;
2828
private final ObjectMapper deserializer;
2929
private final HttpClient httpClient;
30+
private final Duration requestTimeout;
3031
private Instant nextAllowedRequestTime = Instant.now();
3132

3233
/**
3334
* Constructs an OfrepApi instance with a HTTP client and JSON serializers.
3435
*
35-
* @param timeout - The timeout duration for establishing HTTP connection.
36+
* @param requestTimeout - The request timeout duration for the request.
37+
* @param connectTimeout - The connect timeout duration for establishing HTTP connection.
3638
* @param proxySelector - The ProxySelector to use for HTTP requests.
3739
* @param executor - The Executor to use for operations.
3840
*/
39-
public OfrepApi(Duration timeout, ProxySelector proxySelector, Executor executor) {
41+
public OfrepApi(Duration requestTimeout, Duration connectTimeout, ProxySelector proxySelector, Executor executor) {
4042
httpClient = HttpClient.newBuilder()
41-
.connectTimeout(timeout)
43+
.connectTimeout(connectTimeout)
4244
.proxy(proxySelector)
4345
.executor(executor)
4446
.build();
47+
this.requestTimeout = requestTimeout;
4548
serializer = new ObjectMapper();
4649
deserializer = new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
4750
}
@@ -62,6 +65,7 @@ private <T> HttpRequest prepareHttpRequest(
6265

6366
HttpRequest.Builder reqBuilder = HttpRequest.newBuilder()
6467
.uri(url)
68+
.timeout(this.requestTimeout)
6569
.header("Content-Type", "application/json; charset=utf-8")
6670
.POST(HttpRequest.BodyPublishers.ofByteArray(serializer.writeValueAsBytes(requestBody)));
6771

providers/ofrep/src/main/java/dev/openfeature/contrib/providers/ofrep/internal/Resolver.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,19 +34,21 @@ public class Resolver {
3434
*
3535
* @param baseUrl - The base URL of the OFREP server.
3636
* @param headers - The headers to include in the requests.
37-
* @param timeout - The timeout for requests in seconds.
37+
* @param requestTimeout - The request timeout duration for the request.
38+
* @param connectTimeout - The connect timeout duration for establishing the HTTP connection.
3839
* @param proxySelector - The ProxySelector to use for HTTP requests.
3940
* @param executor - The Executor to use for operations.
4041
*/
4142
public Resolver(
4243
String baseUrl,
4344
ImmutableMap<String, ImmutableList<String>> headers,
44-
Duration timeout,
45+
Duration requestTimeout,
46+
Duration connectTimeout,
4547
ProxySelector proxySelector,
4648
Executor executor) {
4749
this.baseUrl = baseUrl;
4850
this.headers = headers;
49-
this.ofrepApi = new OfrepApi(timeout, proxySelector, executor);
51+
this.ofrepApi = new OfrepApi(requestTimeout, connectTimeout, proxySelector, executor);
5052
}
5153

5254
private <T> ProviderEvaluation<T> resolve(Class<T> type, String key, T defaultValue, EvaluationContext ctx) {

providers/ofrep/src/test/java/dev/openfeature/contrib/OfrepProviderTest.java

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,10 +97,16 @@ public class OfrepProviderTest {
9797
private static final ObjectMapper deserializer =
9898
new ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
9999

100+
private static final OfrepProvider ofrepProviderWithRequestTimeout =
101+
OfrepProvider.constructProvider(OfrepProviderOptions.builder()
102+
.baseUrl("http://10.255.255.1")
103+
.requestTimeout(Duration.ofSeconds(2))
104+
.build());
105+
100106
private static final OfrepProvider ofrepProviderWithConnectTimeout =
101107
OfrepProvider.constructProvider(OfrepProviderOptions.builder()
102108
.baseUrl("http://10.255.255.1")
103-
.timeout(Duration.ofSeconds(2))
109+
.connectTimeout(Duration.ofSeconds(2))
104110
.build());
105111

106112
private static MockWebServer mockWebServer;
@@ -452,7 +458,32 @@ void testRateLimit() throws JsonProcessingException, InterruptedException {
452458
}
453459

454460
@Test
455-
void testTimeoutConnection() throws JsonProcessingException {
461+
void testRequestTimeoutConnection() throws JsonProcessingException {
462+
OfrepResponse successfulStringResponse = new OfrepResponse();
463+
successfulStringResponse.setKey(FLAG_KEY);
464+
successfulStringResponse.setValue(SUCCESSFUL_STRING_VALUE);
465+
successfulStringResponse.setReason(SUCCESSFUL_REASON);
466+
successfulStringResponse.setVariant(SUCCESSFUL_VARIANT);
467+
successfulStringResponse.setMetadata(FLAG_METADATA);
468+
469+
mockWebServer.enqueue(new MockResponse()
470+
.setBody(serializer.writeValueAsString(successfulStringResponse))
471+
.setBodyDelay(4, TimeUnit.SECONDS)
472+
.setResponseCode(200));
473+
474+
ProviderEvaluation<String> evaluation = ofrepProviderWithRequestTimeout.getStringEvaluation(
475+
FLAG_KEY, DEFAULT_STRING_VALUE, new ImmutableContext());
476+
477+
assertTrue(DEFAULT_STRING_VALUE.equals(evaluation.getValue()));
478+
assertNull(evaluation.getVariant());
479+
assertNull(evaluation.getReason());
480+
assertTrue(ImmutableMetadata.builder().build().equals(evaluation.getFlagMetadata()));
481+
assertTrue(ERROR_CODE_GENERAL.equals(evaluation.getErrorCode().toString()));
482+
assertTrue(ERROR_DETAIL_GENERAL_HTTP_TIMEOUT.equals(evaluation.getErrorMessage()));
483+
}
484+
485+
@Test
486+
void testConnectTimeoutConnection() throws JsonProcessingException {
456487
OfrepResponse successfulStringResponse = new OfrepResponse();
457488
successfulStringResponse.setKey(FLAG_KEY);
458489
successfulStringResponse.setValue(SUCCESSFUL_STRING_VALUE);
@@ -542,23 +573,43 @@ void testConfigurationValidation() {
542573
OfrepProvider.constructProvider(options);
543574
});
544575

545-
Exception exceptionNegativeTimeout = assertThrows(IllegalArgumentException.class, () -> {
576+
Exception exceptionNegativeConnectTimeout = assertThrows(IllegalArgumentException.class, () -> {
546577
OfrepProviderOptions options = OfrepProviderOptions.builder()
547-
.timeout(Duration.ofSeconds(-10))
578+
.connectTimeout(Duration.ofSeconds(-10))
548579
.build();
549580
OfrepProvider.constructProvider(options);
550581
});
551582

552-
Exception exceptionZeroedTimeout = assertThrows(IllegalArgumentException.class, () -> {
583+
Exception exceptionZeroedConnectTimeout = assertThrows(IllegalArgumentException.class, () -> {
553584
OfrepProviderOptions options = OfrepProviderOptions.builder()
554-
.timeout(Duration.ofSeconds(0))
585+
.connectTimeout(Duration.ofSeconds(0))
555586
.build();
556587
OfrepProvider.constructProvider(options);
557588
});
558589

559-
Exception exceptionNullTimeout = assertThrows(IllegalArgumentException.class, () -> {
590+
Exception exceptionNullConnectTimeout = assertThrows(IllegalArgumentException.class, () -> {
560591
OfrepProviderOptions options =
561-
OfrepProviderOptions.builder().timeout(null).build();
592+
OfrepProviderOptions.builder().connectTimeout(null).build();
593+
OfrepProvider.constructProvider(options);
594+
});
595+
596+
Exception exceptionNegativeRequestTimeout = assertThrows(IllegalArgumentException.class, () -> {
597+
OfrepProviderOptions options = OfrepProviderOptions.builder()
598+
.requestTimeout(Duration.ofSeconds(-10))
599+
.build();
600+
OfrepProvider.constructProvider(options);
601+
});
602+
603+
Exception exceptionZeroedRequestTimeout = assertThrows(IllegalArgumentException.class, () -> {
604+
OfrepProviderOptions options = OfrepProviderOptions.builder()
605+
.requestTimeout(Duration.ofSeconds(0))
606+
.build();
607+
OfrepProvider.constructProvider(options);
608+
});
609+
610+
Exception exceptionNullRequestTimeout = assertThrows(IllegalArgumentException.class, () -> {
611+
OfrepProviderOptions options =
612+
OfrepProviderOptions.builder().requestTimeout(null).build();
562613
OfrepProvider.constructProvider(options);
563614
});
564615

@@ -577,9 +628,14 @@ void testConfigurationValidation() {
577628
assertTrue(exceptionInvalidBaseUrl.getMessage().contains("Invalid base URL"));
578629
assertTrue(exceptionNullBaseUrl.getMessage().contains("Invalid base URL"));
579630
assertTrue(exceptionNullHeaders.getMessage().contains("Headers cannot be null"));
580-
assertTrue(exceptionNegativeTimeout.getMessage().contains("Timeout must be a positive duration"));
581-
assertTrue(exceptionZeroedTimeout.getMessage().contains("Timeout must be a positive duration"));
582-
assertTrue(exceptionNullTimeout.getMessage().contains("Timeout must be a positive duration"));
631+
assertTrue(
632+
exceptionNegativeRequestTimeout.getMessage().contains("Request timeout must be a positive duration"));
633+
assertTrue(exceptionZeroedRequestTimeout.getMessage().contains("Request timeout must be a positive duration"));
634+
assertTrue(exceptionNullRequestTimeout.getMessage().contains("Request timeout must be a positive duration"));
635+
assertTrue(
636+
exceptionNegativeConnectTimeout.getMessage().contains("Connect timeout must be a positive duration"));
637+
assertTrue(exceptionZeroedConnectTimeout.getMessage().contains("Connect timeout must be a positive duration"));
638+
assertTrue(exceptionNullConnectTimeout.getMessage().contains("Connect timeout must be a positive duration"));
583639
assertTrue(exceptionNullProxySelector.getMessage().contains("ProxySelector cannot be null"));
584640
assertTrue(exceptionNullExecutor.getMessage().contains("Executor cannot be null"));
585641
}

0 commit comments

Comments
 (0)