Timeout validation remove

src/itest/java/com/orbitz/consul/AclTestIgnore.java

@@ -10,6 +10,7 @@
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.core.IsNot.not;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.*;
 import org.testcontainers.containers.GenericContainer;
 
@@ -36,7 +37,7 @@ public class AclTestIgnore {
 
     protected static Consul client;
 
-    protected static HostAndPort aclClientHostAndPort = HostAndPort.fromParts("localhost", consulContainerAcl.getFirstMappedPort());
+    protected static HostAndPort aclClientHostAndPort = HostAndPort.fromParts(consulContainerAcl.getHost(), consulContainerAcl.getFirstMappedPort());
 
     @BeforeClass
     public static void beforeClass() {

src/itest/java/com/orbitz/consul/BaseIntegrationTest.java

@@ -47,7 +47,7 @@ public abstract class BaseIntegrationTest {
 
     @BeforeClass
     public static void beforeClass() {
-        defaultClientHostAndPort = HostAndPort.fromParts("localhost", consulContainer.getFirstMappedPort());
+        defaultClientHostAndPort = HostAndPort.fromParts(consulContainer.getHost(), consulContainer.getFirstMappedPort());
         client = Consul.builder()
                 .withHostAndPort(defaultClientHostAndPort)
                 .withClientConfiguration(new ClientConfig(CacheConfig.builder().withWatchDuration(Duration.ofSeconds(1)).build()))

src/itest/java/com/orbitz/consul/HealthITest.java

@@ -87,7 +87,7 @@ public void shouldFetchNodeBlock() throws UnknownHostException, NotRegisteredExc
         client.agentClient().pass(serviceId);
 
         ConsulResponse<List<ServiceHealth>> response = client.healthClient().getAllServiceInstances(serviceName,
-                QueryOptions.blockSeconds(2, new BigInteger("0")).datacenter("dc1").build());
+                QueryOptions.blockSeconds(2, BigInteger.ZERO).datacenter("dc1").build());
         assertHealth(serviceId, response);
         client.agentClient().deregister(serviceId);
     }
@@ -111,7 +111,7 @@ public void shouldFetchChecksForServiceBlock() throws UnknownHostException, NotR
 
         boolean found = false;
         ConsulResponse<List<HealthCheck>> response = client.healthClient().getServiceChecks(serviceName,
-                QueryOptions.blockSeconds(20, new BigInteger("0")).datacenter("dc1").build());
+                QueryOptions.blockSeconds(20, BigInteger.ZERO).datacenter("dc1").build());
 
         List<HealthCheck> checks = response.getResponse();
         assertEquals(1, checks.size());

src/itest/java/com/orbitz/consul/KeyValueITest.java

@@ -4,6 +4,7 @@
 import java.util.Optional;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.net.HostAndPort;
 import com.orbitz.consul.async.ConsulResponseCallback;
 import com.orbitz.consul.model.ConsulResponse;
 import com.orbitz.consul.model.kv.ImmutableOperation;
@@ -38,10 +39,38 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 public class KeyValueITest extends BaseIntegrationTest {
     private static final Charset TEST_CHARSET = Charset.forName("IBM297");
 
+    @Test
+    public void shouldApplyCustomTimeoutForBlockingRequest() throws InterruptedException {
+        KeyValueClient keyValueClient = Consul.builder()
+                .withHostAndPort(HostAndPort.fromParts(consulContainer.getHost(), consulContainer.getFirstMappedPort()))
+                .withReadTimeoutMillis(500)
+                .build().keyValueClient();
+        String key = UUID.randomUUID().toString();
+        String valueContent = UUID.randomUUID().toString();
+
+        assertTrue(keyValueClient.putValue(key, valueContent));
+        CountDownLatch latch = new CountDownLatch(1);
+        keyValueClient.getValue(key, QueryOptions.BLANK, new ConsulResponseCallback<Optional<Value>>() {
+            @Override
+            public void onComplete(ConsulResponse<Optional<Value>> consulResponse) {
+                Optional<Value> v = keyValueClient.getValue(key, QueryOptions.blockSeconds(10, consulResponse.getIndex()).build());
+                assertEquals(valueContent, v.get().getValueAsString().get());
+                latch.countDown();
+            }
+
+            @Override
+            public void onFailure(Throwable throwable) {
+                fail();
+            }
+        });
+        latch.await();
+    }
+
     @Test
     public void shouldPutAndReceiveString() throws UnknownHostException {
         KeyValueClient keyValueClient = client.keyValueClient();

src/main/java/com/orbitz/consul/CatalogClient.java

@@ -20,7 +20,7 @@
 /**
  * HTTP Client for /v1/catalog/ endpoints.
  */
-public class CatalogClient extends BaseCacheableClient {
+public class CatalogClient extends BaseClient {
 
     private static String CLIENT_NAME = "catalog";
 
@@ -31,8 +31,8 @@ public class CatalogClient extends BaseCacheableClient {
      *
      * @param retrofit The {@link Retrofit} to build a client from.
      */
-    CatalogClient(Retrofit retrofit, ClientConfig config, ClientEventCallback eventCallback, Consul.NetworkTimeoutConfig networkTimeoutConfig) {
-        super(CLIENT_NAME, config, eventCallback, networkTimeoutConfig);
+    CatalogClient(Retrofit retrofit, ClientConfig config, ClientEventCallback eventCallback) {
+        super(CLIENT_NAME, config, eventCallback);
         this.api = retrofit.create(Api.class);
     }
 

src/main/java/com/orbitz/consul/Consul.java

@@ -9,7 +9,6 @@
 import java.util.concurrent.SynchronousQueue;
 import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
-import java.util.function.IntSupplier;
 
 import javax.net.ssl.*;
 
@@ -277,7 +276,9 @@ public static class Builder {
         private Interceptor headerInterceptor;
         private Interceptor consulBookendInterceptor;
         private Interceptor consulFailoverInterceptor;
-        private final NetworkTimeoutConfig.Builder networkTimeoutConfigBuilder = new NetworkTimeoutConfig.Builder();
+        private Long connectTimeoutMillis;
+        private Long readTimeoutMillis;
+        private Long writeTimeoutMillis;
         private ExecutorService executorService;
         private ConnectionPool connectionPool;
         private ClientConfig clientConfig;
@@ -495,10 +496,10 @@ public Builder withMultipleHostAndPort(Collection<HostAndPort> hostAndPort, long
          * @return The builder.
          */
         public Builder withFailoverInterceptor(ConsulFailoverStrategy strategy) {
-        	Preconditions.checkArgument(strategy != null, "Must not provide a null strategy");
-        	
-        	consulFailoverInterceptor = new ConsulFailoverInterceptor(strategy);
-        	return this;
+            Preconditions.checkArgument(strategy != null, "Must not provide a null strategy");
+
+            consulFailoverInterceptor = new ConsulFailoverInterceptor(strategy);
+            return this;
         }
 
         /**
@@ -572,7 +573,7 @@ public Builder withProxy(Proxy proxy) {
         */
         public Builder withConnectTimeoutMillis(long timeoutMillis) {
             Preconditions.checkArgument(timeoutMillis >= 0, "Negative value");
-            this.networkTimeoutConfigBuilder.withConnectTimeout((int) timeoutMillis);
+            this.connectTimeoutMillis = timeoutMillis;
             return this;
         }
 
@@ -583,7 +584,7 @@ public Builder withConnectTimeoutMillis(long timeoutMillis) {
         */
         public Builder withReadTimeoutMillis(long timeoutMillis) {
             Preconditions.checkArgument(timeoutMillis >= 0, "Negative value");
-            this.networkTimeoutConfigBuilder.withReadTimeout((int) timeoutMillis);
+            this.readTimeoutMillis = timeoutMillis;
 
             return this;
         }
@@ -595,7 +596,7 @@ public Builder withReadTimeoutMillis(long timeoutMillis) {
         */
         public Builder withWriteTimeoutMillis(long timeoutMillis) {
             Preconditions.checkArgument(timeoutMillis >= 0, "Negative value");
-            this.networkTimeoutConfigBuilder.withWriteTimeout((int) timeoutMillis);
+            this.writeTimeoutMillis = timeoutMillis;
 
             return this;
         }
@@ -608,7 +609,8 @@ public Builder withWriteTimeoutMillis(long timeoutMillis) {
         * It can only be shutdown by the {@link Consul#destroy()} method.
         *
         * When an application needs to be able to customize the ExecutorService parameters, and/or manage its lifecycle,
-        * it can provide an instance of ExecutorService to the Builder. In that case, this ExecutorService will be used instead of creating one internally.
+        * it can provide an instance of ExecutorService to the Builder. In that case,
+        * this ExecutorService will be used instead of creating one internally.
         *
         * @param executorService The ExecutorService to be injected in the internal tasks dispatcher.
         * @return
@@ -628,7 +630,8 @@ public Builder withExecutorService(ExecutorService executorService) {
         * It can only be shutdown by the {@link Consul#destroy()} method.
         *
         * When an application needs to be able to customize the ConnectionPool parameters, and/or manage its lifecycle,
-        * it can provide an instance of ConnectionPool to the Builder. In that case, this ConnectionPool will be used instead of creating one internally.
+        * it can provide an instance of ConnectionPool to the Builder. In that case,
+        * this ConnectionPool will be used instead of creating one internally.
         *
         * @param connectionPool The ConnetcionPool to be injected in the internal  OkHttpClient
         * @return
@@ -698,11 +701,6 @@ public Consul build() {
                     executorService,
                     connectionPool,
                     config);
-            NetworkTimeoutConfig networkTimeoutConfig = new NetworkTimeoutConfig.Builder()
-                .withConnectTimeout(okHttpClient::connectTimeoutMillis)
-                .withReadTimeout(okHttpClient::readTimeoutMillis)
-                .withWriteTimeout(okHttpClient::writeTimeoutMillis)
-                .build();
 
             try {
                 retrofit = createRetrofit(
@@ -718,9 +716,9 @@ public Consul build() {
                     new ClientEventCallback(){};
 
             AgentClient agentClient = new AgentClient(retrofit, config, eventCallback);
-            HealthClient healthClient = new HealthClient(retrofit, config, eventCallback, networkTimeoutConfig);
-            KeyValueClient keyValueClient = new KeyValueClient(retrofit, config, eventCallback, networkTimeoutConfig);
-            CatalogClient catalogClient = new CatalogClient(retrofit, config, eventCallback, networkTimeoutConfig);
+            HealthClient healthClient = new HealthClient(retrofit, config, eventCallback);
+            KeyValueClient keyValueClient = new KeyValueClient(retrofit, config, eventCallback);
+            CatalogClient catalogClient = new CatalogClient(retrofit, config, eventCallback);
             StatusClient statusClient = new StatusClient(retrofit, config, eventCallback);
             SessionClient sessionClient = new SessionClient(retrofit, config, eventCallback);
             EventClient eventClient = new EventClient(retrofit, config, eventCallback);
@@ -744,7 +742,8 @@ private String buildUrl(URL url) {
         }
 
         private OkHttpClient createOkHttpClient(SSLContext sslContext, X509TrustManager trustManager, HostnameVerifier hostnameVerifier,
-                                                Proxy proxy, ExecutorService executorService, ConnectionPool connectionPool, ClientConfig clientConfig) {
+                                                Proxy proxy, ExecutorService executorService, ConnectionPool connectionPool,
+                                                ClientConfig clientConfig) {
 
             final OkHttpClient.Builder builder = new OkHttpClient.Builder();
 
@@ -781,17 +780,16 @@ private OkHttpClient createOkHttpClient(SSLContext sslContext, X509TrustManager
             if(proxy != null) {
                 builder.proxy(proxy);
             }
-            NetworkTimeoutConfig networkTimeoutConfig = networkTimeoutConfigBuilder.build();
-            if (networkTimeoutConfig.getClientConnectTimeoutMillis() >= 0) {
-                builder.connectTimeout(networkTimeoutConfig.getClientConnectTimeoutMillis(), TimeUnit.MILLISECONDS);
+            if (connectTimeoutMillis != null) {
+                builder.connectTimeout(connectTimeoutMillis, TimeUnit.MILLISECONDS);
             }
 
-            if (networkTimeoutConfig.getClientReadTimeoutMillis() >= 0) {
-                builder.readTimeout(networkTimeoutConfig.getClientReadTimeoutMillis(), TimeUnit.MILLISECONDS);
+            if (readTimeoutMillis != null) {
+                builder.readTimeout(readTimeoutMillis, TimeUnit.MILLISECONDS);
             }
 
-            if (networkTimeoutConfig.getClientWriteTimeoutMillis() >= 0) {
-                builder.writeTimeout(networkTimeoutConfig.getClientWriteTimeoutMillis(), TimeUnit.MILLISECONDS);
+            if (writeTimeoutMillis != null) {
+                builder.writeTimeout(writeTimeoutMillis, TimeUnit.MILLISECONDS);
             }
 
             builder.addInterceptor(new TimeoutInterceptor(clientConfig.getCacheConfig()));
@@ -820,65 +818,4 @@ private Retrofit createRetrofit(String url, ObjectMapper mapper, OkHttpClient ok
         }
 
     }
-
-    public static class NetworkTimeoutConfig {
-        private final IntSupplier readTimeoutMillisSupplier;
-        private final IntSupplier writeTimeoutMillisSupplier;
-        private final IntSupplier connectTimeoutMillisSupplier;
-
-        private NetworkTimeoutConfig(
-                IntSupplier readTimeoutMillisSupplier,
-                IntSupplier writeTimeoutMillisSupplier,
-                IntSupplier connectTimeoutMillisSupplier) {
-            this.readTimeoutMillisSupplier = readTimeoutMillisSupplier;
-            this.writeTimeoutMillisSupplier = writeTimeoutMillisSupplier;
-            this.connectTimeoutMillisSupplier = connectTimeoutMillisSupplier;
-        }
-
-        public int getClientReadTimeoutMillis() {
-            return readTimeoutMillisSupplier.getAsInt();
-        }
-        public int getClientWriteTimeoutMillis() {
-            return writeTimeoutMillisSupplier.getAsInt();
-        }
-        public int getClientConnectTimeoutMillis() {
-            return connectTimeoutMillisSupplier.getAsInt();
-        }
-        public static class Builder {
-            private IntSupplier readTimeoutMillisSupplier = () -> -1;
-            private IntSupplier writeTimeoutMillisSupplier = () -> -1;
-            private IntSupplier connectTimeoutMillisSupplier = () -> -1;
-
-            public NetworkTimeoutConfig.Builder withReadTimeout(IntSupplier timeoutSupplier) {
-                this.readTimeoutMillisSupplier = timeoutSupplier;
-                return this;
-            }
-
-            public NetworkTimeoutConfig.Builder withReadTimeout(int millis) {
-                return withReadTimeout(() -> millis);
-            }
-
-            public NetworkTimeoutConfig.Builder withWriteTimeout(IntSupplier timeoutSupplier) {
-                this.writeTimeoutMillisSupplier = timeoutSupplier;
-                return this;
-            }
-
-            public NetworkTimeoutConfig.Builder withWriteTimeout(int millis) {
-                return withWriteTimeout(() -> millis);
-            }
-
-            public NetworkTimeoutConfig.Builder withConnectTimeout(IntSupplier timeoutSupplier) {
-                this.connectTimeoutMillisSupplier = timeoutSupplier;
-                return this;
-            }
-
-            public NetworkTimeoutConfig.Builder withConnectTimeout(int millis) {
-                return withConnectTimeout(() -> millis);
-            }
-
-            public NetworkTimeoutConfig build() {
-                return new NetworkTimeoutConfig(readTimeoutMillisSupplier, writeTimeoutMillisSupplier, connectTimeoutMillisSupplier);
-            }
-        }
-    }
 }

src/main/java/com/orbitz/consul/HealthClient.java

@@ -24,7 +24,7 @@
 /**
  * HTTP Client for /v1/health/ endpoints.
  */
-public class HealthClient extends BaseCacheableClient {
+public class HealthClient extends BaseClient {
 
     private static String CLIENT_NAME = "health";
 
@@ -35,8 +35,8 @@ public class HealthClient extends BaseCacheableClient {
      *
      * @param retrofit The {@link Retrofit} to build a client from.
      */
-    HealthClient(Retrofit retrofit, ClientConfig config, ClientEventCallback eventCallback, Consul.NetworkTimeoutConfig networkTimeoutConfig) {
-        super(CLIENT_NAME, config, eventCallback, networkTimeoutConfig);
+    HealthClient(Retrofit retrofit, ClientConfig config, ClientEventCallback eventCallback) {
+        super(CLIENT_NAME, config, eventCallback);
         this.api = retrofit.create(Api.class);
     }
 

src/main/java/com/orbitz/consul/KeyValueClient.java

@@ -49,7 +49,7 @@
 /**
  * HTTP Client for /v1/kv/ endpoints.
  */
-public class KeyValueClient extends BaseCacheableClient {
+public class KeyValueClient extends BaseClient {
 
     private static String CLIENT_NAME = "keyvalue";
     public static final int NOT_FOUND_404 = 404;
@@ -61,13 +61,13 @@ public class KeyValueClient extends BaseCacheableClient {
      *
      * @param retrofit The {@link Retrofit} to build a client from.
      */
-    KeyValueClient(Retrofit retrofit, ClientConfig config, ClientEventCallback eventCallback, Consul.NetworkTimeoutConfig networkTimeoutConfig) {
-        super(CLIENT_NAME, config, eventCallback, networkTimeoutConfig);
+    KeyValueClient(Retrofit retrofit, ClientConfig config, ClientEventCallback eventCallback) {
+        super(CLIENT_NAME, config, eventCallback);
         this.api = retrofit.create(Api.class);
     }
 
-    KeyValueClient(Api api, ClientConfig config, ClientEventCallback eventCallback, Consul.NetworkTimeoutConfig networkTimeoutConfig) {
-        super(CLIENT_NAME, config, eventCallback, networkTimeoutConfig);
+    KeyValueClient(Api api, ClientConfig config, ClientEventCallback eventCallback) {
+        super(CLIENT_NAME, config, eventCallback);
         this.api = api;
     }
 

src/main/java/com/orbitz/consul/cache/ConsulCache.java

@@ -407,11 +407,4 @@ public void shutdownNow() {
             // do nothing, since executor was externally created
         }
     }
-
-    protected static void checkWatch(int networkReadMillis, int cacheWatchSeconds) {
-        if (networkReadMillis <= TimeUnit.SECONDS.toMillis(cacheWatchSeconds)) {
-            throw new IllegalArgumentException("Cache watchInterval="+ cacheWatchSeconds + "sec >= networkClientReadTimeout="
-                + networkReadMillis + "ms. It can cause issues");
-        }
-    }
 }