feat: Added configurable timeouts for fetchSegments and dispatch events ODP calls

core-httpclient-impl/src/main/java/com/optimizely/ab/HttpClientUtils.java

@@ -23,9 +23,11 @@
  */
 public final class HttpClientUtils {
 
-    private static final int CONNECTION_TIMEOUT_MS = 10000;
-    private static final int CONNECTION_REQUEST_TIMEOUT_MS = 5000;
-    private static final int SOCKET_TIMEOUT_MS = 10000;
+    public static final int CONNECTION_TIMEOUT_MS = 10000;
+    public static final int CONNECTION_REQUEST_TIMEOUT_MS = 5000;
+    public static final int SOCKET_TIMEOUT_MS = 10000;
+
+    private static RequestConfig requestConfigWithTimeout;
 
     private HttpClientUtils() {
     }
@@ -36,6 +38,17 @@ private HttpClientUtils() {
         .setSocketTimeout(SOCKET_TIMEOUT_MS)
         .build();
 
+    public static RequestConfig getDefaultRequestConfigWithTimeout(int timeoutMillis) {
+        if (requestConfigWithTimeout == null) {
+            requestConfigWithTimeout = RequestConfig.custom()
+                .setConnectTimeout(timeoutMillis)
+                .setConnectionRequestTimeout(CONNECTION_REQUEST_TIMEOUT_MS)
+                .setSocketTimeout(SOCKET_TIMEOUT_MS)
+                .build();
+        }
+        return  requestConfigWithTimeout;
+    }
+
     public static OptimizelyHttpClient getDefaultHttpClient() {
         return OptimizelyHttpClient.builder().build();
     }

core-httpclient-impl/src/main/java/com/optimizely/ab/OptimizelyHttpClient.java

@@ -1,6 +1,6 @@
 /**
  *
- *    Copyright 2019, Optimizely
+ *    Copyright 2019, 2022 Optimizely
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -25,6 +25,8 @@
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.io.Closeable;
 import java.io.IOException;
@@ -38,6 +40,8 @@
  */
 public class OptimizelyHttpClient implements Closeable {
 
+    private static final Logger logger = LoggerFactory.getLogger(OptimizelyHttpClient.class);
+
     private final CloseableHttpClient httpClient;
 
     OptimizelyHttpClient(CloseableHttpClient httpClient) {
@@ -78,6 +82,7 @@ public static class Builder {
         // force-close the connection after this idle time (with 0, eviction is disabled by default)
         long evictConnectionIdleTimePeriod = 0;
         TimeUnit evictConnectionIdleTimeUnit = TimeUnit.MILLISECONDS;
+        private int timeoutMillis = HttpClientUtils.CONNECTION_TIMEOUT_MS;
 
         private Builder() {
 
@@ -103,6 +108,11 @@ public Builder withEvictIdleConnections(long maxIdleTime, TimeUnit maxIdleTimeUn
             this.evictConnectionIdleTimeUnit = maxIdleTimeUnit;
             return this;
         }
+
+        public Builder setTimeoutMillis(int timeoutMillis) {
+            this.timeoutMillis = timeoutMillis;
+            return this;
+        }
 
         public OptimizelyHttpClient build() {
             PoolingHttpClientConnectionManager poolingHttpClientConnectionManager = new PoolingHttpClientConnectionManager();
@@ -111,11 +121,13 @@ public OptimizelyHttpClient build() {
             poolingHttpClientConnectionManager.setValidateAfterInactivity(validateAfterInactivity);
 
             HttpClientBuilder builder = HttpClients.custom()
-                .setDefaultRequestConfig(HttpClientUtils.DEFAULT_REQUEST_CONFIG)
+                .setDefaultRequestConfig(HttpClientUtils.getDefaultRequestConfigWithTimeout(timeoutMillis))
                 .setConnectionManager(poolingHttpClientConnectionManager)
                 .disableCookieManagement()
                 .useSystemProperties();
 
+            logger.debug("Creating HttpClient with timeout: " + timeoutMillis);
+
             if (evictConnectionIdleTimePeriod > 0) {
                 builder.evictIdleConnections(evictConnectionIdleTimePeriod, evictConnectionIdleTimeUnit);
             }

core-httpclient-impl/src/main/java/com/optimizely/ab/odp/DefaultODPApiManager.java

@@ -33,15 +33,27 @@
 public class DefaultODPApiManager implements ODPApiManager {
     private static final Logger logger = LoggerFactory.getLogger(DefaultODPApiManager.class);
 
-    private final OptimizelyHttpClient httpClient;
+    private final OptimizelyHttpClient httpClientSegments;
+    private final OptimizelyHttpClient httpClientEvents;
 
     public DefaultODPApiManager() {
         this(OptimizelyHttpClient.builder().build());
     }
 
+    public DefaultODPApiManager(int segmentFetchTimeoutMillis, int eventDispatchTimeoutMillis) {
+        httpClientSegments = OptimizelyHttpClient.builder().setTimeoutMillis(segmentFetchTimeoutMillis).build();
+        if (segmentFetchTimeoutMillis == eventDispatchTimeoutMillis) {
+            // If the timeouts are same, single httpClient can be used for both.
+            httpClientEvents = httpClientSegments;
+        } else {
+            httpClientEvents = OptimizelyHttpClient.builder().setTimeoutMillis(eventDispatchTimeoutMillis).build();
+        }
+    }
+
     @VisibleForTesting
     DefaultODPApiManager(OptimizelyHttpClient httpClient) {
-        this.httpClient = httpClient;
+        this.httpClientSegments = httpClient;
+        this.httpClientEvents = httpClient;
     }
 
     @VisibleForTesting
@@ -150,7 +162,7 @@ public String fetchQualifiedSegments(String apiKey, String apiEndpoint, String u
 
         CloseableHttpResponse response = null;
         try {
-            response = httpClient.execute(request);
+            response = httpClientSegments.execute(request);
         } catch (IOException e) {
             logger.error("Error retrieving response from ODP service", e);
             return null;
@@ -211,7 +223,7 @@ public Integer sendEvents(String apiKey, String apiEndpoint, String eventPayload
 
         CloseableHttpResponse response = null;
         try {
-            response = httpClient.execute(request);
+            response = httpClientEvents.execute(request);
         } catch (IOException e) {
             logger.error("Error retrieving response from event request", e);
             return 0;

core-httpclient-impl/src/test/java/com/optimizely/ab/odp/DefaultODPApiManagerTest.java

@@ -28,7 +28,6 @@
 import org.junit.Test;
 import org.mockito.ArgumentCaptor;
 
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashSet;
 
@@ -129,4 +128,20 @@ public void eventDispatchFailStatus() throws Exception {
         apiManager.sendEvents("testKey", "testEndpoint", "[]]");
         logbackVerifier.expectMessage(Level.ERROR, "ODP event send failed (Response code: 400, null)");
     }
+
+    @Test
+    public void apiTimeouts() {
+        // Default timeout is 10 seconds
+        new DefaultODPApiManager();
+        logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 10000", 1);
+
+        // Same timeouts result in single httpclient
+        new DefaultODPApiManager(2222, 2222);
+        logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 2222", 1);
+
+        // Different timeouts result in different HttpClients
+        new DefaultODPApiManager(3333, 4444);
+        logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 3333", 1);
+        logbackVerifier.expectMessage(Level.DEBUG, "Creating HttpClient with timeout: 4444", 1);
+    }
 }