Repository: Unleash/unleash-client-java
Pull request #164 (2022-07-18 08:53:15)

task: Make http agent timeouts configurable
This PR makes connect and read timeouts for our http requests configurable, previously they've been hardcoded.
fixes: #163



Patch:
diff --git a/src/main/java/io/getunleash/DefaultUnleash.java b/src/main/java/io/getunleash/DefaultUnleash.java
index 6b4848ae..e66c1ab1 100644
--- a/src/main/java/io/getunleash/DefaultUnleash.java
+++ b/src/main/java/io/getunleash/DefaultUnleash.java
@@ -190,6 +190,12 @@ public class DefaultUnleash implements Unleash {
         return getVariant(toggleName, contextProvider.getContext(), defaultValue);
     }
 
+    /**
+     * Use more().getFeatureToggleDefinition() instead
+     *
+     * @return the feature toggle
+     */
+    @Deprecated
     public Optional<FeatureToggle> getFeatureToggleDefinition(String toggleName) {
         return ofNullable(featureRepository.getToggle(toggleName));
     }
@@ -245,6 +251,11 @@ public class DefaultUnleash implements Unleash {
             return featureRepository.getFeatureNames();
         }
 
+        @Override
+        public Optional<FeatureToggle> getFeatureToggleDefinition(String toggleName) {
+            return ofNullable(featureRepository.getToggle(toggleName));
+        }
+
         @Override
         public List<EvaluatedToggle> evaluateAllToggles() {
             return evaluateAllToggles(contextProvider.getContext());
diff --git a/src/main/java/io/getunleash/FakeUnleash.java b/src/main/java/io/getunleash/FakeUnleash.java
index cbe8e784..23245fac 100644
--- a/src/main/java/io/getunleash/FakeUnleash.java
+++ b/src/main/java/io/getunleash/FakeUnleash.java
@@ -1,17 +1,16 @@
 package io.getunleash;
 
+import static java.util.Collections.emptyList;
+
 import io.getunleash.lang.Nullable;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
 import java.util.function.BiFunction;
 import java.util.stream.Collectors;
 
 public class FakeUnleash implements Unleash {
     private boolean enableAll = false;
     private boolean disableAll = false;
-    private Map<String, Boolean> features = new HashMap<>();
+    private Map<String, FeatureToggle> features = new HashMap<>();
     private Map<String, Variant> variants = new HashMap<>();
 
     @Override
@@ -26,7 +25,9 @@ public class FakeUnleash implements Unleash {
         } else if (disableAll) {
             return false;
         } else {
-            return features.getOrDefault(toggleName, defaultSetting);
+            return more().getFeatureToggleDefinition(toggleName)
+                    .map(FeatureToggle::isEnabled)
+                    .orElse(defaultSetting);
         }
     }
 
@@ -102,13 +103,13 @@ public class FakeUnleash implements Unleash {
 
     public void enable(String... features) {
         for (String name : features) {
-            this.features.put(name, true);
+            this.features.put(name, new FeatureToggle(name, true, emptyList()));
         }
     }
 
     public void disable(String... features) {
         for (String name : features) {
-            this.features.put(name, false);
+            this.features.put(name, new FeatureToggle(name, false, emptyList()));
         }
     }
 
@@ -129,6 +130,11 @@ public class FakeUnleash implements Unleash {
             return new ArrayList<>(features.keySet());
         }
 
+        @Override
+        public Optional<FeatureToggle> getFeatureToggleDefinition(String toggleName) {
+            return Optional.ofNullable(features.get(toggleName));
+        }
+
         @Override
         public List<EvaluatedToggle> evaluateAllToggles() {
             return evaluateAllToggles(null);
diff --git a/src/main/java/io/getunleash/MoreOperations.java b/src/main/java/io/getunleash/MoreOperations.java
index a02d1058..ca37fdfd 100644
--- a/src/main/java/io/getunleash/MoreOperations.java
+++ b/src/main/java/io/getunleash/MoreOperations.java
@@ -1,11 +1,14 @@
 package io.getunleash;
 
 import java.util.List;
+import java.util.Optional;
 
 public interface MoreOperations {
 
     List<String> getFeatureToggleNames();
 
+    Optional<FeatureToggle> getFeatureToggleDefinition(String toggleName);
+
     List<EvaluatedToggle> evaluateAllToggles();
 
     /**
diff --git a/src/main/java/io/getunleash/metric/UnleashMetricsSender.java b/src/main/java/io/getunleash/metric/UnleashMetricsSender.java
index fcfa2c59..4976202c 100644
--- a/src/main/java/io/getunleash/metric/UnleashMetricsSender.java
+++ b/src/main/java/io/getunleash/metric/UnleashMetricsSender.java
@@ -89,8 +89,8 @@ public class UnleashMetricsSender {
             } else {
                 connection = (HttpURLConnection) url.openConnection();
             }
-            connection.setConnectTimeout(CONNECT_TIMEOUT);
-            connection.setReadTimeout(CONNECT_TIMEOUT);
+            connection.setConnectTimeout((int) unleashConfig.getSendMetricsConnectTimeout().toMillis());
+            connection.setReadTimeout((int) unleashConfig.getSendMetricsReadTimeout().toMillis());
             connection.setRequestMethod("POST");
             connection.setRequestProperty("Accept", "application/json");
             connection.setRequestProperty("Content-Type", "application/json");
diff --git a/src/main/java/io/getunleash/repository/HttpFeatureFetcher.java b/src/main/java/io/getunleash/repository/HttpFeatureFetcher.java
index 62bb8787..834bedd7 100644
--- a/src/main/java/io/getunleash/repository/HttpFeatureFetcher.java
+++ b/src/main/java/io/getunleash/repository/HttpFeatureFetcher.java
@@ -15,8 +15,6 @@ import org.slf4j.LoggerFactory;
 
 public class HttpFeatureFetcher implements FeatureFetcher {
     private static final Logger LOG = LoggerFactory.getLogger(HttpFeatureFetcher.class);
-
-    private static final int CONNECT_TIMEOUT = 10000;
     private Optional<String> etag = Optional.empty();
 
     private final UnleashConfig config;
@@ -107,8 +105,8 @@ public class HttpFeatureFetcher implements FeatureFetcher {
         } else {
             connection = (HttpURLConnection) url.openConnection();
         }
-        connection.setConnectTimeout(CONNECT_TIMEOUT);
-        connection.setReadTimeout(CONNECT_TIMEOUT);
+        connection.setConnectTimeout((int) this.config.getFetchTogglesConnectTimeout().toMillis());
+        connection.setReadTimeout((int) this.config.getFetchTogglesReadTimeout().toMillis());
         connection.setRequestProperty("Accept", "application/json");
         connection.setRequestProperty("Content-Type", "application/json");
         UnleashConfig.setRequestProperties(connection, this.config);
diff --git a/src/main/java/io/getunleash/repository/OkHttpFeatureFetcher.java b/src/main/java/io/getunleash/repository/OkHttpFeatureFetcher.java
index 82ba367d..a7561f93 100644
--- a/src/main/java/io/getunleash/repository/OkHttpFeatureFetcher.java
+++ b/src/main/java/io/getunleash/repository/OkHttpFeatureFetcher.java
@@ -19,10 +19,6 @@ import okhttp3.Request;
 import okhttp3.Response;
 
 public class OkHttpFeatureFetcher implements FeatureFetcher {
-
-    private static final Duration CONNECT_TIMEOUT = Duration.ofSeconds(10);
-    private static final Duration CALL_TIMEOUT = Duration.ofSeconds(5);
-
     private final HttpUrl toggleUrl;
     private final OkHttpClient client;
 
@@ -34,8 +30,8 @@ public class OkHttpFeatureFetcher implements FeatureFetcher {
         }
         OkHttpClient.Builder builder =
                 new OkHttpClient.Builder()
-                        .connectTimeout(CONNECT_TIMEOUT)
-                        .callTimeout(CALL_TIMEOUT)
+                        .connectTimeout(unleashConfig.getFetchTogglesConnectTimeout())
+                        .callTimeout(unleashConfig.getFetchTogglesReadTimeout())
                         .followRedirects(true);
         if (tempDir != null) {
             builder = builder.cache(new Cache(tempDir, 1024 * 1024 * 50));
diff --git a/src/main/java/io/getunleash/util/UnleashConfig.java b/src/main/java/io/getunleash/util/UnleashConfig.java
index 210d6016..96a0eae5 100644
--- a/src/main/java/io/getunleash/util/UnleashConfig.java
+++ b/src/main/java/io/getunleash/util/UnleashConfig.java
@@ -20,6 +20,7 @@ import java.net.PasswordAuthentication;
 import java.net.Proxy;
 import java.net.URI;
 import java.net.UnknownHostException;
+import java.time.Duration;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
@@ -43,7 +44,15 @@ public class UnleashConfig {
     @Nullable private final String projectName;
     @Nullable private final String namePrefix;
     private final long fetchTogglesInterval;
+
+    private final Duration fetchTogglesConnectTimeout;
+
+    private final Duration fetchTogglesReadTimeout;
     private final long sendMetricsInterval;
+
+    private final Duration sendMetricsConnectTimeout;
+
+    private final Duration sendMetricsReadTimeout;
     private final boolean disableMetrics;
     private final boolean isProxyAuthenticationByJvmProperties;
     private final UnleashFeatureFetcherFactory unleashFeatureFetcherFactory;
@@ -67,7 +76,11 @@ public class UnleashConfig {
             @Nullable String projectName,
             @Nullable String namePrefix,
             long fetchTogglesInterval,
+            Duration fetchTogglesConnectTimeout,
+            Duration fetchTogglesReadTimeout,
             long sendMetricsInterval,
+            Duration sendMetricsConnectTimeout,
+            Duration sendMetricsReadTimeout,
             boolean disableMetrics,
             UnleashContextProvider contextProvider,
             boolean isProxyAuthenticationByJvmProperties,
@@ -124,7 +137,11 @@ public class UnleashConfig {
         this.projectName = projectName;
         this.namePrefix = namePrefix;
         this.fetchTogglesInterval = fetchTogglesInterval;
+        this.fetchTogglesConnectTimeout = fetchTogglesConnectTimeout;
+        this.fetchTogglesReadTimeout = fetchTogglesReadTimeout;
         this.sendMetricsInterval = sendMetricsInterval;
+        this.sendMetricsConnectTimeout = sendMetricsConnectTimeout;
+        this.sendMetricsReadTimeout = sendMetricsReadTimeout;
         this.disableMetrics = disableMetrics;
         this.contextProvider = contextProvider;
         this.isProxyAuthenticationByJvmProperties = isProxyAuthenticationByJvmProperties;
@@ -198,10 +215,26 @@ public class UnleashConfig {
         return fetchTogglesInterval;
     }
 
+    public Duration getFetchTogglesConnectTimeout() {
+        return fetchTogglesConnectTimeout;
+    }
+
+    public Duration getFetchTogglesReadTimeout() {
+        return fetchTogglesReadTimeout;
+    }
+
     public long getSendMetricsInterval() {
         return sendMetricsInterval;
     }
 
+    public Duration getSendMetricsConnectTimeout() {
+        return sendMetricsConnectTimeout;
+    }
+
+    public Duration getSendMetricsReadTimeout() {
+        return sendMetricsReadTimeout;
+    }
+
     public UnleashURLs getUnleashURLs() {
         return unleashURLs;
     }
@@ -325,7 +358,15 @@ public class UnleashConfig {
         private @Nullable String projectName;
         private @Nullable String namePrefix;
         private long fetchTogglesInterval = 10;
+
+        private Duration fetchTogglesConnectTimeout = Duration.ofSeconds(10);
+
+        private Duration fetchTogglesReadTimeout = Duration.ofSeconds(10);
         private long sendMetricsInterval = 60;
+
+        private Duration sendMetricsConnectTimeout = Duration.ofSeconds(10);
+
+        private Duration sendMetricsReadTimeout = Duration.ofSeconds(10);
         private boolean disableMetrics = false;
         private UnleashFeatureFetcherFactory unleashFeatureFetcherFactory = HttpFeatureFetcher::new;
         private UnleashContextProvider contextProvider =
@@ -410,11 +451,51 @@ public class UnleashConfig {
             return this;
         }
 
+        public Builder fetchTogglesConnectTimeout(Duration connectTimeout) {
+            this.fetchTogglesConnectTimeout = connectTimeout;
+            return this;
+        }
+
+        public Builder fetchTogglesConnectTimeoutSeconds(long connectTimeoutSeconds) {
+            this.fetchTogglesConnectTimeout = Duration.ofSeconds(connectTimeoutSeconds);
+            return this;
+        }
+        public Builder fetchTogglesReadTimeout(Duration readTimeout) {
+            this.fetchTogglesReadTimeout = readTimeout;
+            return this;
+        }
+
+        public Builder fetchTogglesReadTimeoutSeconds(long readTimeoutSeconds) {
+            this.fetchTogglesReadTimeout = Duration.ofSeconds(readTimeoutSeconds);
+            return this;
+        }
+
         public Builder sendMetricsInterval(long sendMetricsInterval) {
             this.sendMetricsInterval = sendMetricsInterval;
             return this;
         }
 
+        public Builder sendMetricsConnectTimeout(Duration connectTimeout) {
+            this.sendMetricsConnectTimeout = connectTimeout;
+            return this;
+        }
+
+        public Builder sendMetricsConnectTimeoutSeconds(long connectTimeoutSeconds) {
+            this.sendMetricsConnectTimeout = Duration.ofSeconds(connectTimeoutSeconds);
+            return this;
+        }
+        public Builder sendMetricsReadTimeout(Duration readTimeout) {
+            this.sendMetricsReadTimeout = readTimeout;
+            return this;
+        }
+
+        public Builder sendMetricsReadTimeoutSeconds(long readTimeoutSeconds) {
+            this.sendMetricsReadTimeout = Duration.ofSeconds(readTimeoutSeconds);
+            return this;
+        }
+
+
+
         public Builder disableMetrics() {
             this.disableMetrics = true;
             return this;
@@ -499,7 +580,11 @@ public class UnleashConfig {
                     projectName,
                     namePrefix,
                     fetchTogglesInterval,
+                    fetchTogglesConnectTimeout,
+                    fetchTogglesReadTimeout,
                     sendMetricsInterval,
+                    sendMetricsConnectTimeout,
+                    sendMetricsReadTimeout,
                     disableMetrics,
                     contextProvider,
                     isProxyAuthenticationByJvmProperties,


Test patch:
diff --git a/src/test/java/io/getunleash/FakeUnleashTest.java b/src/test/java/io/getunleash/FakeUnleashTest.java
index 0336d4c3..a4c00977 100644
--- a/src/test/java/io/getunleash/FakeUnleashTest.java
+++ b/src/test/java/io/getunleash/FakeUnleashTest.java
@@ -4,6 +4,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 import java.util.Arrays;
 import java.util.List;
+import java.util.Optional;
 import org.junit.jupiter.api.Test;
 
 public class FakeUnleashTest {
@@ -101,6 +102,27 @@ public class FakeUnleashTest {
         assertThat(fakeUnleash.more().getFeatureToggleNames()).containsAll(expected);
     }
 
+    @Test
+    public void should_get_feature_definition() {
+        FakeUnleash fakeUnleash = new FakeUnleash();
+        fakeUnleash.enable("t1");
+
+        Optional<FeatureToggle> optionalToggle =
+                fakeUnleash.more().getFeatureToggleDefinition("t1");
+        assertThat(optionalToggle).isPresent();
+
+        FeatureToggle toggle = optionalToggle.get();
+        assertThat(toggle.getName()).isEqualTo("t1");
+        assertThat(toggle.getStrategies()).isEmpty();
+        assertThat(toggle.isEnabled()).isTrue();
+    }
+
+    @Test
+    public void should_get_empty_optional_when_feature_definition_not_present() {
+        FakeUnleash fakeUnleash = new FakeUnleash();
+        assertThat(fakeUnleash.more().getFeatureToggleDefinition("t1")).isEmpty();
+    }
+
     @Test
     public void should_get_variant() {
         FakeUnleash fakeUnleash = new FakeUnleash();
diff --git a/src/test/java/io/getunleash/UnleashTest.java b/src/test/java/io/getunleash/UnleashTest.java
index cf7f4d70..6ec4fb47 100644
--- a/src/test/java/io/getunleash/UnleashTest.java
+++ b/src/test/java/io/getunleash/UnleashTest.java
@@ -215,8 +215,7 @@ public class UnleashTest {
         FeatureToggle featureToggle = new FeatureToggle("test", false, asList(strategy1));
         when(toggleRepository.getToggle("test")).thenReturn(featureToggle);
 
-        assertThat(((DefaultUnleash) unleash).getFeatureToggleDefinition("test"))
-                .hasValue(featureToggle);
+        assertThat(unleash.more().getFeatureToggleDefinition("test")).hasValue(featureToggle);
     }
 
     @Test
@@ -225,8 +224,7 @@ public class UnleashTest {
         FeatureToggle featureToggle = new FeatureToggle("test", false, asList(strategy1));
         when(toggleRepository.getToggle("test")).thenReturn(featureToggle);
 
-        assertThat(((DefaultUnleash) unleash).getFeatureToggleDefinition("another toggleName"))
-                .isEmpty();
+        assertThat(unleash.more().getFeatureToggleDefinition("another toggleName")).isEmpty();
     }
 
     @Test