From 1112e23320ce4d045ca62cdfd2a5e8c7e26fe437 Mon Sep 17 00:00:00 2001 From: "d.zharikhin" Date: Thu, 25 Feb 2021 20:08:24 +0300 Subject: [PATCH 1/3] remove timeout validation code --- .../java/com/orbitz/consul/HealthITest.java | 4 +- .../java/com/orbitz/consul/KeyValueITest.java | 31 ++++- .../orbitz/consul/BaseCacheableClient.java | 21 ---- .../java/com/orbitz/consul/CatalogClient.java | 6 +- src/main/java/com/orbitz/consul/Consul.java | 113 ++++-------------- .../java/com/orbitz/consul/HealthClient.java | 6 +- .../com/orbitz/consul/KeyValueClient.java | 10 +- .../com/orbitz/consul/cache/ConsulCache.java | 7 -- .../orbitz/consul/cache/HealthCheckCache.java | 1 - .../java/com/orbitz/consul/cache/KVCache.java | 1 - .../consul/cache/NodesCatalogCache.java | 5 +- .../consul/cache/ServiceCatalogCache.java | 5 +- .../consul/cache/ServiceHealthCache.java | 1 - .../orbitz/consul/KeyValueClientFactory.java | 5 +- .../com/orbitz/consul/cache/KVCacheTest.java | 3 +- .../java/com/orbitz/consul/util/HttpTest.java | 2 +- 16 files changed, 74 insertions(+), 147 deletions(-) delete mode 100644 src/main/java/com/orbitz/consul/BaseCacheableClient.java diff --git a/src/itest/java/com/orbitz/consul/HealthITest.java b/src/itest/java/com/orbitz/consul/HealthITest.java index 8d775865..0cebe251 100644 --- a/src/itest/java/com/orbitz/consul/HealthITest.java +++ b/src/itest/java/com/orbitz/consul/HealthITest.java @@ -87,7 +87,7 @@ public void shouldFetchNodeBlock() throws UnknownHostException, NotRegisteredExc client.agentClient().pass(serviceId); ConsulResponse> 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> response = client.healthClient().getServiceChecks(serviceName, - QueryOptions.blockSeconds(20, new BigInteger("0")).datacenter("dc1").build()); + QueryOptions.blockSeconds(20, BigInteger.ZERO).datacenter("dc1").build()); List checks = response.getResponse(); assertEquals(1, checks.size()); diff --git a/src/itest/java/com/orbitz/consul/KeyValueITest.java b/src/itest/java/com/orbitz/consul/KeyValueITest.java index 77c7e4b9..f133492e 100644 --- a/src/itest/java/com/orbitz/consul/KeyValueITest.java +++ b/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>() { + @Override + public void onComplete(ConsulResponse> consulResponse) { + Optional 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(); @@ -472,7 +501,7 @@ public void testBasicTxn() throws Exception { ConsulResponse response = keyValueClient.performTransaction(operation); - assertEquals(value, keyValueClient.getValueAsString(key).get()); + assertEquals(response.getIndex(), keyValueClient.getValue(key).get().getModifyIndex()); assertEquals(response.getIndex(), keyValueClient.getValue(key).get().getModifyIndex()); } diff --git a/src/main/java/com/orbitz/consul/BaseCacheableClient.java b/src/main/java/com/orbitz/consul/BaseCacheableClient.java deleted file mode 100644 index 7e09bd33..00000000 --- a/src/main/java/com/orbitz/consul/BaseCacheableClient.java +++ /dev/null @@ -1,21 +0,0 @@ -package com.orbitz.consul; - -import com.orbitz.consul.config.ClientConfig; -import com.orbitz.consul.monitoring.ClientEventCallback; -import com.orbitz.consul.monitoring.ClientEventHandler; -import com.orbitz.consul.util.Http; - -abstract class BaseCacheableClient extends BaseClient { - - private final Consul.NetworkTimeoutConfig networkTimeoutConfig; - - protected BaseCacheableClient(String name, ClientConfig config, ClientEventCallback eventCallback, - Consul.NetworkTimeoutConfig networkTimeoutConfig) { - super(name, config, eventCallback); - this.networkTimeoutConfig = networkTimeoutConfig; - } - - public Consul.NetworkTimeoutConfig getNetworkTimeoutConfig() { - return networkTimeoutConfig; - } -} diff --git a/src/main/java/com/orbitz/consul/CatalogClient.java b/src/main/java/com/orbitz/consul/CatalogClient.java index 39aefb92..f11edac9 100644 --- a/src/main/java/com/orbitz/consul/CatalogClient.java +++ b/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); } diff --git a/src/main/java/com/orbitz/consul/Consul.java b/src/main/java/com/orbitz/consul/Consul.java index ad07d900..bba609c3 100644 --- a/src/main/java/com/orbitz/consul/Consul.java +++ b/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, 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); - } - } - } } diff --git a/src/main/java/com/orbitz/consul/HealthClient.java b/src/main/java/com/orbitz/consul/HealthClient.java index 8a5ca171..b3aa110b 100644 --- a/src/main/java/com/orbitz/consul/HealthClient.java +++ b/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); } diff --git a/src/main/java/com/orbitz/consul/KeyValueClient.java b/src/main/java/com/orbitz/consul/KeyValueClient.java index 1c227540..e0cde676 100644 --- a/src/main/java/com/orbitz/consul/KeyValueClient.java +++ b/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; } diff --git a/src/main/java/com/orbitz/consul/cache/ConsulCache.java b/src/main/java/com/orbitz/consul/cache/ConsulCache.java index de33831f..0a5c8b94 100644 --- a/src/main/java/com/orbitz/consul/cache/ConsulCache.java +++ b/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"); - } - } } diff --git a/src/main/java/com/orbitz/consul/cache/HealthCheckCache.java b/src/main/java/com/orbitz/consul/cache/HealthCheckCache.java index 0ac96dd5..c15baf8d 100644 --- a/src/main/java/com/orbitz/consul/cache/HealthCheckCache.java +++ b/src/main/java/com/orbitz/consul/cache/HealthCheckCache.java @@ -19,7 +19,6 @@ private HealthCheckCache(HealthClient healthClient, Scheduler callbackScheduler) { super(keyExtractor, (index, callback) -> { - checkWatch(healthClient.getNetworkTimeoutConfig().getClientReadTimeoutMillis(), watchSeconds); QueryOptions params = watchParams(index, watchSeconds, queryOptions); healthClient.getChecksByState(state, params, callback); }, diff --git a/src/main/java/com/orbitz/consul/cache/KVCache.java b/src/main/java/com/orbitz/consul/cache/KVCache.java index aa67b12c..17334948 100644 --- a/src/main/java/com/orbitz/consul/cache/KVCache.java +++ b/src/main/java/com/orbitz/consul/cache/KVCache.java @@ -21,7 +21,6 @@ private KVCache(KeyValueClient kvClient, Scheduler callbackScheduler) { super(getKeyExtractorFunction(keyPath), (index, callback) -> { - checkWatch(kvClient.getNetworkTimeoutConfig().getClientReadTimeoutMillis(), watchSeconds); QueryOptions params = watchParams(index, watchSeconds, queryOptions); kvClient.getValues(keyPath, params, callback); }, diff --git a/src/main/java/com/orbitz/consul/cache/NodesCatalogCache.java b/src/main/java/com/orbitz/consul/cache/NodesCatalogCache.java index 6b776460..445e702f 100644 --- a/src/main/java/com/orbitz/consul/cache/NodesCatalogCache.java +++ b/src/main/java/com/orbitz/consul/cache/NodesCatalogCache.java @@ -15,10 +15,7 @@ private NodesCatalogCache(CatalogClient catalogClient, int watchSeconds, Scheduler callbackScheduler) { super(Node::getNode, - (index, callback) -> { - checkWatch(catalogClient.getNetworkTimeoutConfig().getClientReadTimeoutMillis(), watchSeconds); - catalogClient.getNodes(watchParams(index, watchSeconds, queryOptions), callback); - }, + (index, callback) -> catalogClient.getNodes(watchParams(index, watchSeconds, queryOptions), callback), catalogClient.getConfig().getCacheConfig(), catalogClient.getEventHandler(), new CacheDescriptor("catalog.nodes"), diff --git a/src/main/java/com/orbitz/consul/cache/ServiceCatalogCache.java b/src/main/java/com/orbitz/consul/cache/ServiceCatalogCache.java index 5c3b1bf9..5fedc121 100644 --- a/src/main/java/com/orbitz/consul/cache/ServiceCatalogCache.java +++ b/src/main/java/com/orbitz/consul/cache/ServiceCatalogCache.java @@ -16,10 +16,7 @@ private ServiceCatalogCache(CatalogClient catalogClient, Scheduler callbackScheduler) { super(CatalogService::getServiceId, - (index, callback) -> { - checkWatch(catalogClient.getNetworkTimeoutConfig().getClientReadTimeoutMillis(), watchSeconds); - catalogClient.getService(serviceName, watchParams(index, watchSeconds, queryOptions), callback); - }, + (index, callback) -> catalogClient.getService(serviceName, watchParams(index, watchSeconds, queryOptions), callback), catalogClient.getConfig().getCacheConfig(), catalogClient.getEventHandler(), new CacheDescriptor("catalog.service", serviceName), diff --git a/src/main/java/com/orbitz/consul/cache/ServiceHealthCache.java b/src/main/java/com/orbitz/consul/cache/ServiceHealthCache.java index ef7eda3f..88a5f0a7 100644 --- a/src/main/java/com/orbitz/consul/cache/ServiceHealthCache.java +++ b/src/main/java/com/orbitz/consul/cache/ServiceHealthCache.java @@ -21,7 +21,6 @@ private ServiceHealthCache(HealthClient healthClient, Scheduler callbackScheduler) { super(keyExtractor, (index, callback) -> { - checkWatch(healthClient.getNetworkTimeoutConfig().getClientReadTimeoutMillis(), watchSeconds); QueryOptions params = watchParams(index, watchSeconds, queryOptions); if (passing) { healthClient.getHealthyServiceInstances(serviceName, params, callback); diff --git a/src/test/java/com/orbitz/consul/KeyValueClientFactory.java b/src/test/java/com/orbitz/consul/KeyValueClientFactory.java index 7c1b6b25..b9b2ce02 100644 --- a/src/test/java/com/orbitz/consul/KeyValueClientFactory.java +++ b/src/test/java/com/orbitz/consul/KeyValueClientFactory.java @@ -10,8 +10,7 @@ public class KeyValueClientFactory { private KeyValueClientFactory() { } - public static KeyValueClient create(KeyValueClient.Api api, ClientConfig config, ClientEventCallback eventCallback, - Consul.NetworkTimeoutConfig networkTimeoutConfig) { - return new KeyValueClient(api, config, eventCallback, networkTimeoutConfig); + public static KeyValueClient create(KeyValueClient.Api api, ClientConfig config, ClientEventCallback eventCallback) { + return new KeyValueClient(api, config, eventCallback); } } diff --git a/src/test/java/com/orbitz/consul/cache/KVCacheTest.java b/src/test/java/com/orbitz/consul/cache/KVCacheTest.java index e3476969..5f7da501 100644 --- a/src/test/java/com/orbitz/consul/cache/KVCacheTest.java +++ b/src/test/java/com/orbitz/consul/cache/KVCacheTest.java @@ -1,7 +1,6 @@ package com.orbitz.consul.cache; import com.google.common.util.concurrent.ThreadFactoryBuilder; -import com.orbitz.consul.Consul; import com.orbitz.consul.KeyValueClient; import com.orbitz.consul.KeyValueClientFactory; import com.orbitz.consul.MockApiService; @@ -97,7 +96,7 @@ public void testListenerWithMockRetrofit() throws InterruptedException { final KeyValueClient kvClient = KeyValueClientFactory.create(mockApiService, new ClientConfig(cacheConfig), new ClientEventCallback() { - }, new Consul.NetworkTimeoutConfig.Builder().withReadTimeout(10500).build()); + }); try (final KVCache kvCache = KVCache.newCache(kvClient, "")) { diff --git a/src/test/java/com/orbitz/consul/util/HttpTest.java b/src/test/java/com/orbitz/consul/util/HttpTest.java index 453f7fc9..f30c9f39 100644 --- a/src/test/java/com/orbitz/consul/util/HttpTest.java +++ b/src/test/java/com/orbitz/consul/util/HttpTest.java @@ -257,7 +257,7 @@ public void onFailure(Throwable throwable) { when(call.request()).thenReturn(request); Callback callCallback = http.createCallback(call, callback); - Response response = Response.error(400, ResponseBody.create(MediaType.parse(""), "failure")); + Response response = Response.error(400, ResponseBody.create("failure", MediaType.parse(""))); callCallback.onResponse(call, response); latch.await(1, TimeUnit.SECONDS); From 25eb42bf3ccceda27a60b4f6a940c6a1ce88f2b7 Mon Sep 17 00:00:00 2001 From: "d.zharikhin" Date: Fri, 20 Aug 2021 11:42:41 +0300 Subject: [PATCH 2/3] improve test infrastructure --- src/itest/java/com/orbitz/consul/AclTestIgnore.java | 3 ++- src/itest/java/com/orbitz/consul/BaseIntegrationTest.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/itest/java/com/orbitz/consul/AclTestIgnore.java b/src/itest/java/com/orbitz/consul/AclTestIgnore.java index 2d40ac01..2a216a61 100644 --- a/src/itest/java/com/orbitz/consul/AclTestIgnore.java +++ b/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() { diff --git a/src/itest/java/com/orbitz/consul/BaseIntegrationTest.java b/src/itest/java/com/orbitz/consul/BaseIntegrationTest.java index c2d9e335..5ef51357 100644 --- a/src/itest/java/com/orbitz/consul/BaseIntegrationTest.java +++ b/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())) From b87a221bdb0b116fef0e5f427bb98df525b81c77 Mon Sep 17 00:00:00 2001 From: "d.zharikhin" Date: Fri, 20 Aug 2021 14:03:17 +0300 Subject: [PATCH 3/3] fi merge issue --- src/itest/java/com/orbitz/consul/KeyValueITest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/itest/java/com/orbitz/consul/KeyValueITest.java b/src/itest/java/com/orbitz/consul/KeyValueITest.java index f133492e..6382ed35 100644 --- a/src/itest/java/com/orbitz/consul/KeyValueITest.java +++ b/src/itest/java/com/orbitz/consul/KeyValueITest.java @@ -501,7 +501,7 @@ public void testBasicTxn() throws Exception { ConsulResponse response = keyValueClient.performTransaction(operation); - assertEquals(response.getIndex(), keyValueClient.getValue(key).get().getModifyIndex()); + assertEquals(value, keyValueClient.getValueAsString(key).get()); assertEquals(response.getIndex(), keyValueClient.getValue(key).get().getModifyIndex()); }