From e6099b847c537323584fac730001c183964a86b1 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Wed, 8 May 2024 22:43:58 -0700 Subject: [PATCH 01/17] Make calls to endpoint discovery non-blocking for asynchornous clients --- .../codegen/poet/client/AsyncClientClass.java | 4 +- .../client/test-endpoint-discovery-async.java | 12 +-- .../EndpointDiscoveryRefreshCache.java | 88 ++++++++++++++++--- 3 files changed, 82 insertions(+), 22 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/AsyncClientClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/AsyncClientClass.java index 4490e00fb9f4..034ad680ffb8 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/AsyncClientClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/AsyncClientClass.java @@ -429,14 +429,14 @@ protected MethodSpec.Builder operationBody(MethodSpec.Builder builder, Operation AwsClientOption.class) .addCode(" .resolveIdentity();"); - builder.addCode("endpointFuture = identityFuture.thenApply(credentials -> {") + builder.addCode("endpointFuture = identityFuture.thenCompose(credentials -> {") .addCode(" $1T endpointDiscoveryRequest = $1T.builder()", EndpointDiscoveryRequest.class) .addCode(" .required($L)", opModel.getInputShape().getEndpointDiscovery().isRequired()) .addCode(" .defaultEndpoint(clientConfiguration.option($T.ENDPOINT))", SdkClientOption.class) .addCode(" .overrideConfiguration($N.overrideConfiguration().orElse(null))", opModel.getInput().getVariableName()) .addCode(" .build();") - .addCode(" return endpointDiscoveryCache.get(credentials.accessKeyId(), endpointDiscoveryRequest);") + .addCode(" return endpointDiscoveryCache.getAsync(credentials.accessKeyId(), endpointDiscoveryRequest);") .addCode("});"); builder.endControlFlow(); diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-endpoint-discovery-async.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-endpoint-discovery-async.java index 8518ad4db2fc..5618a0538bee 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-endpoint-discovery-async.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-endpoint-discovery-async.java @@ -193,12 +193,12 @@ public CompletableFuture testDiscovery .overrideConfiguration().flatMap(AwsRequestOverrideConfiguration::credentialsIdentityProvider) .orElseGet(() -> clientConfiguration.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER)) .resolveIdentity(); - endpointFuture = identityFuture.thenApply(credentials -> { + endpointFuture = identityFuture.thenCompose(credentials -> { EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true) .defaultEndpoint(clientConfiguration.option(SdkClientOption.ENDPOINT)) .overrideConfiguration(testDiscoveryIdentifiersRequiredRequest.overrideConfiguration().orElse(null)) .build(); - return endpointDiscoveryCache.get(credentials.accessKeyId(), endpointDiscoveryRequest); + return endpointDiscoveryCache.getAsync(credentials.accessKeyId(), endpointDiscoveryRequest); }); } @@ -267,11 +267,11 @@ public CompletableFuture testDiscoveryOptional( .overrideConfiguration().flatMap(AwsRequestOverrideConfiguration::credentialsIdentityProvider) .orElseGet(() -> clientConfiguration.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER)) .resolveIdentity(); - endpointFuture = identityFuture.thenApply(credentials -> { + endpointFuture = identityFuture.thenCompose(credentials -> { EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(false) .defaultEndpoint(clientConfiguration.option(SdkClientOption.ENDPOINT)) .overrideConfiguration(testDiscoveryOptionalRequest.overrideConfiguration().orElse(null)).build(); - return endpointDiscoveryCache.get(credentials.accessKeyId(), endpointDiscoveryRequest); + return endpointDiscoveryCache.getAsync(credentials.accessKeyId(), endpointDiscoveryRequest); }); } @@ -348,11 +348,11 @@ public CompletableFuture testDiscoveryRequired( .overrideConfiguration().flatMap(AwsRequestOverrideConfiguration::credentialsIdentityProvider) .orElseGet(() -> clientConfiguration.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER)) .resolveIdentity(); - endpointFuture = identityFuture.thenApply(credentials -> { + endpointFuture = identityFuture.thenCompose(credentials -> { EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true) .defaultEndpoint(clientConfiguration.option(SdkClientOption.ENDPOINT)) .overrideConfiguration(testDiscoveryRequiredRequest.overrideConfiguration().orElse(null)).build(); - return endpointDiscoveryCache.get(credentials.accessKeyId(), endpointDiscoveryRequest); + return endpointDiscoveryCache.getAsync(credentials.accessKeyId(), endpointDiscoveryRequest); }); } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index 982b3427216d..b4ce8e5953dd 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -19,6 +19,7 @@ import java.time.Instant; import java.util.Map; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import software.amazon.awssdk.annotations.SdkProtectedApi; @@ -29,6 +30,8 @@ public final class EndpointDiscoveryRefreshCache { private final EndpointDiscoveryCacheLoader client; + private String key; + private EndpointDiscoveryRefreshCache(EndpointDiscoveryCacheLoader client) { this.client = client; } @@ -61,22 +64,21 @@ public URI get(String accessKey, EndpointDiscoveryRequest request) { if (endpoint == null) { if (request.required()) { return cache.computeIfAbsent(key, k -> getAndJoin(request)).endpoint(); + } + EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() + .endpoint(request.defaultEndpoint()) + .expirationTime(Instant.now().plusSeconds(60)) + .build(); + + EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); + if (previousValue != null) { + // Someone else primed the cache. Use that endpoint (which may be temporary). + return previousValue.endpoint(); } else { - EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() - .endpoint(request.defaultEndpoint()) - .expirationTime(Instant.now().plusSeconds(60)) - .build(); - - EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); - if (previousValue != null) { - // Someone else primed the cache. Use that endpoint (which may be temporary). - return previousValue.endpoint(); - } else { - // We primed the cache with the temporary endpoint. Kick off discovery in the background. - refreshCacheAsync(request, key); - } - return tempEndpoint.endpoint(); + // We primed the cache with the temporary endpoint. Kick off discovery in the background. + refreshCacheAsync(request, key); } + return tempEndpoint.endpoint(); } if (endpoint.expirationTime().isBefore(Instant.now())) { @@ -87,6 +89,64 @@ public URI get(String accessKey, EndpointDiscoveryRequest request) { return endpoint.endpoint(); } + public CompletableFuture getAsync(String accessKey, EndpointDiscoveryRequest request) { + key = accessKey; + + // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. + if (key == null) { + key = ""; + } + + if (request.cacheKey().isPresent()) { + key = key + ":" + request.cacheKey().get(); + } + + EndpointDiscoveryEndpoint endpoint = cache.get(key); + + if (endpoint == null) { + if (request.required()) { + return discoverEndpoint(request).handle( + (endpointDiscoveryEndpoint, throwable) -> { + if (throwable != null) { + if (throwable instanceof InterruptedException) { + Thread.currentThread().interrupt(); + throw EndpointDiscoveryFailedException.create(throwable); + } + if (throwable instanceof ExecutionException + || throwable instanceof CompletionException) { + throw EndpointDiscoveryFailedException.create(throwable.getCause()); + } + throw new RuntimeException("new exception"); + } + return cache.computeIfAbsent( + key, k -> endpointDiscoveryEndpoint + ).endpoint(); + }); + } + EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() + .endpoint(request.defaultEndpoint()) + .expirationTime(Instant.now().plusSeconds(60)) + .build(); + + EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); + if (previousValue != null) { + // Someone else primed the cache. Use that endpoint (which may be temporary). + return CompletableFuture.completedFuture(previousValue.endpoint()); + } else { + // We primed the cache with the temporary endpoint. Kick off discovery in the background. + refreshCacheAsync(request, key); + } + return CompletableFuture.completedFuture(tempEndpoint.endpoint()); + } + + if (endpoint.expirationTime().isBefore(Instant.now())) { + cache.put(key, endpoint.toBuilder().expirationTime(Instant.now().plusSeconds(60)).build()); + refreshCacheAsync(request, key); + } + + return CompletableFuture.completedFuture(endpoint.endpoint()); + } + private EndpointDiscoveryEndpoint getAndJoin(EndpointDiscoveryRequest request) { try { return discoverEndpoint(request).get(); From e7174a1b2b91d04eaf3c36691f937c09688ef1d9 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Wed, 8 May 2024 22:52:57 -0700 Subject: [PATCH 02/17] Remove code style changed --- .../EndpointDiscoveryRefreshCache.java | 27 ++++++++++--------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index b4ce8e5953dd..25e6b9b646a2 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -64,21 +64,22 @@ public URI get(String accessKey, EndpointDiscoveryRequest request) { if (endpoint == null) { if (request.required()) { return cache.computeIfAbsent(key, k -> getAndJoin(request)).endpoint(); - } - EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() - .endpoint(request.defaultEndpoint()) - .expirationTime(Instant.now().plusSeconds(60)) - .build(); - - EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); - if (previousValue != null) { - // Someone else primed the cache. Use that endpoint (which may be temporary). - return previousValue.endpoint(); } else { - // We primed the cache with the temporary endpoint. Kick off discovery in the background. - refreshCacheAsync(request, key); + EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() + .endpoint(request.defaultEndpoint()) + .expirationTime(Instant.now().plusSeconds(60)) + .build(); + + EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); + if (previousValue != null) { + // Someone else primed the cache. Use that endpoint (which may be temporary). + return previousValue.endpoint(); + } else { + // We primed the cache with the temporary endpoint. Kick off discovery in the background. + refreshCacheAsync(request, key); + } + return tempEndpoint.endpoint(); } - return tempEndpoint.endpoint(); } if (endpoint.expirationTime().isBefore(Instant.now())) { From 53cd91741ff4b00329f7b7ac23a0f955b0f692bb Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 13 May 2024 03:03:57 -0700 Subject: [PATCH 03/17] Added more tests and improved code style --- .../EndpointDiscoveryRefreshCache.java | 124 +++++++----------- services/sqs/pom.xml | 6 + test/codegen-generated-classes-test/pom.xml | 12 ++ .../services/EndpointDiscoveryTest.java | 74 ++++++++++- 4 files changed, 137 insertions(+), 79 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index 25e6b9b646a2..55d7b4220de5 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -19,7 +19,6 @@ import java.time.Instant; import java.util.Map; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ExecutionException; import software.amazon.awssdk.annotations.SdkProtectedApi; @@ -30,8 +29,6 @@ public final class EndpointDiscoveryRefreshCache { private final EndpointDiscoveryCacheLoader client; - private String key; - private EndpointDiscoveryRefreshCache(EndpointDiscoveryCacheLoader client) { this.client = client; } @@ -48,81 +45,13 @@ public static EndpointDiscoveryRefreshCache create(EndpointDiscoveryCacheLoader * @return The endpoint to use for this request */ public URI get(String accessKey, EndpointDiscoveryRequest request) { - String key = accessKey; - - // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. - if (key == null) { - key = ""; - } - - if (request.cacheKey().isPresent()) { - key = key + ":" + request.cacheKey().get(); - } + String key = getKey(accessKey, request); EndpointDiscoveryEndpoint endpoint = cache.get(key); if (endpoint == null) { if (request.required()) { return cache.computeIfAbsent(key, k -> getAndJoin(request)).endpoint(); - } else { - EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() - .endpoint(request.defaultEndpoint()) - .expirationTime(Instant.now().plusSeconds(60)) - .build(); - - EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); - if (previousValue != null) { - // Someone else primed the cache. Use that endpoint (which may be temporary). - return previousValue.endpoint(); - } else { - // We primed the cache with the temporary endpoint. Kick off discovery in the background. - refreshCacheAsync(request, key); - } - return tempEndpoint.endpoint(); - } - } - - if (endpoint.expirationTime().isBefore(Instant.now())) { - cache.put(key, endpoint.toBuilder().expirationTime(Instant.now().plusSeconds(60)).build()); - refreshCacheAsync(request, key); - } - - return endpoint.endpoint(); - } - - public CompletableFuture getAsync(String accessKey, EndpointDiscoveryRequest request) { - key = accessKey; - - // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. - if (key == null) { - key = ""; - } - - if (request.cacheKey().isPresent()) { - key = key + ":" + request.cacheKey().get(); - } - - EndpointDiscoveryEndpoint endpoint = cache.get(key); - - if (endpoint == null) { - if (request.required()) { - return discoverEndpoint(request).handle( - (endpointDiscoveryEndpoint, throwable) -> { - if (throwable != null) { - if (throwable instanceof InterruptedException) { - Thread.currentThread().interrupt(); - throw EndpointDiscoveryFailedException.create(throwable); - } - if (throwable instanceof ExecutionException - || throwable instanceof CompletionException) { - throw EndpointDiscoveryFailedException.create(throwable.getCause()); - } - throw new RuntimeException("new exception"); - } - return cache.computeIfAbsent( - key, k -> endpointDiscoveryEndpoint - ).endpoint(); - }); } EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() .endpoint(request.defaultEndpoint()) @@ -132,12 +61,11 @@ public CompletableFuture getAsync(String accessKey, EndpointDiscoveryReques EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); if (previousValue != null) { // Someone else primed the cache. Use that endpoint (which may be temporary). - return CompletableFuture.completedFuture(previousValue.endpoint()); - } else { - // We primed the cache with the temporary endpoint. Kick off discovery in the background. - refreshCacheAsync(request, key); + return previousValue.endpoint(); } - return CompletableFuture.completedFuture(tempEndpoint.endpoint()); + // We primed the cache with the temporary endpoint. Kick off discovery in the background. + refreshCacheAsync(request, key); + return tempEndpoint.endpoint(); } if (endpoint.expirationTime().isBefore(Instant.now())) { @@ -145,7 +73,21 @@ public CompletableFuture getAsync(String accessKey, EndpointDiscoveryReques refreshCacheAsync(request, key); } - return CompletableFuture.completedFuture(endpoint.endpoint()); + return endpoint.endpoint(); + } + + public CompletableFuture getAsync(String accessKey, EndpointDiscoveryRequest request) { + String key = getKey(accessKey, request); + EndpointDiscoveryEndpoint endpoint = cache.get(key); + + // If a service call needs to be made to discover endpoint + // a completable future for the service call is returned, unblocking I/O + // and then completed asynchronously + if (endpoint == null && request.required()) { + return discoverEndpointHandler(key, request); + } + // In the event of a cache hit, i.e. service call not required, defer to the synchronous code path method. + return CompletableFuture.completedFuture(get(accessKey, request)); } private EndpointDiscoveryEndpoint getAndJoin(EndpointDiscoveryRequest request) { @@ -170,4 +112,30 @@ public CompletableFuture discoverEndpoint(EndpointDis public void evict(String key) { cache.remove(key); } + + private String getKey(String accessKey, EndpointDiscoveryRequest request) { + String key = accessKey; + + // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. + if (key == null) { + key = ""; + } + + if (request.cacheKey().isPresent()) { + key = key + ":" + request.cacheKey().get(); + } + return key; + } + + private CompletableFuture discoverEndpointHandler(String key, EndpointDiscoveryRequest request) { + return discoverEndpoint(request).handle( + (endpointDiscoveryEndpoint, throwable) -> { + if (throwable != null) { + throw EndpointDiscoveryFailedException.create(throwable.getCause()); + } + return cache.computeIfAbsent( + key, k -> endpointDiscoveryEndpoint + ).endpoint(); + }); + } } diff --git a/services/sqs/pom.xml b/services/sqs/pom.xml index c704e85b4175..82af9e0b511f 100644 --- a/services/sqs/pom.xml +++ b/services/sqs/pom.xml @@ -91,5 +91,11 @@ ${awsjavasdk.version} test + + software.amazon.awssdk + timestreamwrite + 2.25.45-SNAPSHOT + test + diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index 438c7452f1e0..6bf30acb1664 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -268,6 +268,18 @@ mockito-junit-jupiter test + + software.amazon.awssdk + sqs + 2.25.45-SNAPSHOT + test + + + software.amazon.awssdk + timestreamwrite + 2.25.45-SNAPSHOT + test + diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 5c0b81074cbd..8c3b500a6682 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -15,12 +15,17 @@ package software.amazon.awssdk.services; +import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; +import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; +import static com.github.tomakehurst.wiremock.client.WireMock.post; +import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; import com.github.tomakehurst.wiremock.junit.WireMockRule; import io.reactivex.Flowable; import java.io.ByteArrayInputStream; @@ -29,14 +34,17 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; import org.assertj.core.api.AbstractThrowableAssert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.jupiter.api.Timeout; import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption; import software.amazon.awssdk.core.endpointdiscovery.EndpointDiscoveryFailedException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; @@ -47,10 +55,15 @@ import software.amazon.awssdk.http.SdkHttpResponse; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; +import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestAsyncClient; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; +import software.amazon.awssdk.services.sqs.SqsAsyncClient; +import software.amazon.awssdk.services.sqs.model.ListQueuesResponse; +import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -64,6 +77,8 @@ public class EndpointDiscoveryTest { private EndpointDiscoveryTestClient client; private EndpointDiscoveryTestAsyncClient asyncClient; + private SqsAsyncClient sqsClient; + private TimestreamWriteAsyncClient timestreamClient; @Before public void setupClient() { @@ -155,6 +170,15 @@ public void syncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Cause .isInstanceOf(SdkClientException.class); } + @Test + public void asyncRequiredOperation_RequestCancelled_throws_EndpointDiscoveryFailure() { + stubResponse(mockAsyncClient, 500, "invalid", 15); + + assertThatThrownBy(() -> client.testDiscoveryRequired(r -> { + })) + .isInstanceOf(EndpointDiscoveryFailedException.class); + } + @Test public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_CausesSdkException() { stubResponse(mockAsyncClient, 500, "invalid", 15); @@ -163,6 +187,43 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus .isInstanceOf(SdkClientException.class); } + @Test + @Timeout(5) + public void endpointDiscoveryRequired_makes_nonBlockingCalls() throws InterruptedException, ExecutionException { + SdkEventLoopGroup group = SdkEventLoopGroup.builder() + .numberOfThreads(1) + .build(); + + SqsAsyncClient sqs = SqsAsyncClient.builder() + .asyncConfiguration(o -> o.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR, Runnable::run)) + .region(Region.US_WEST_2) + .httpClient(NettyNioAsyncHttpClient.builder() + .eventLoopGroup(group) + .build()) + .endpointOverride(URI.create("http://localhost:" + wireMock.port())) + .build(); + + TimestreamWriteAsyncClient timestream = TimestreamWriteAsyncClient.builder().region(Region.US_WEST_2) + .httpClient(NettyNioAsyncHttpClient.builder() + .eventLoopGroup(group).build()) + .endpointOverride(URI.create("http://localhost:" + wireMock.port())) + .build(); + + + stubFor(post(anyUrl()) + .willReturn(getResponse().get())); + + for(int i =0;i<100;i++) { + sqs.listQueues().whenComplete((r, e) -> { + timestream.listDatabases(req -> req.maxResults(1)); + assertThat(1).isEqualTo(1); + } + ).join(); + + assertThat(1).isEqualTo(1); + } + } + private AbstractThrowableAssert assertAsyncRequiredOperationCallThrowable() { try { asyncClient.testDiscoveryRequired(r -> { @@ -170,7 +231,7 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus throw new AssertionError(); } catch (InterruptedException e) { return assertThat(e); - } catch (ExecutionException e) { + } catch (ExecutionException | CompletionException e) { return assertThat(e.getCause()); } } @@ -244,4 +305,15 @@ private Answer> stubAsyncResponse(SdkHttpResponse respon return cf; }; } + + private CompletableFuture getResponse() { + return CompletableFuture.supplyAsync(() -> { + try { + Thread.sleep(10_000); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + return aResponse().withStatus(200); + }); + } } From 60d7eb25e03ba2e3b1f19ddcc7f168534b6064ea Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 13 May 2024 03:06:34 -0700 Subject: [PATCH 04/17] Removed changes to existing method code style --- .../EndpointDiscoveryRefreshCache.java | 39 ++++++++++++------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index 55d7b4220de5..1da6b45ff4ce 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -45,27 +45,38 @@ public static EndpointDiscoveryRefreshCache create(EndpointDiscoveryCacheLoader * @return The endpoint to use for this request */ public URI get(String accessKey, EndpointDiscoveryRequest request) { + String key = accessKey; + + // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. + if (key == null) { + key = ""; + } + + if (request.cacheKey().isPresent()) { + key = key + ":" + request.cacheKey().get(); + } - String key = getKey(accessKey, request); EndpointDiscoveryEndpoint endpoint = cache.get(key); if (endpoint == null) { if (request.required()) { return cache.computeIfAbsent(key, k -> getAndJoin(request)).endpoint(); + } else { + EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() + .endpoint(request.defaultEndpoint()) + .expirationTime(Instant.now().plusSeconds(60)) + .build(); + + EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); + if (previousValue != null) { + // Someone else primed the cache. Use that endpoint (which may be temporary). + return previousValue.endpoint(); + } else { + // We primed the cache with the temporary endpoint. Kick off discovery in the background. + refreshCacheAsync(request, key); + } + return tempEndpoint.endpoint(); } - EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() - .endpoint(request.defaultEndpoint()) - .expirationTime(Instant.now().plusSeconds(60)) - .build(); - - EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); - if (previousValue != null) { - // Someone else primed the cache. Use that endpoint (which may be temporary). - return previousValue.endpoint(); - } - // We primed the cache with the temporary endpoint. Kick off discovery in the background. - refreshCacheAsync(request, key); - return tempEndpoint.endpoint(); } if (endpoint.expirationTime().isBefore(Instant.now())) { From 9139dc4a0f5169ff12dfc5ead0a3ea1283651a33 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 13 May 2024 03:09:51 -0700 Subject: [PATCH 05/17] Remove unnecessary code --- .../awssdk/services/EndpointDiscoveryTest.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 8c3b500a6682..7fd31ac191c9 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -62,7 +62,6 @@ import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; import software.amazon.awssdk.services.sqs.SqsAsyncClient; -import software.amazon.awssdk.services.sqs.model.ListQueuesResponse; import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -77,8 +76,6 @@ public class EndpointDiscoveryTest { private EndpointDiscoveryTestClient client; private EndpointDiscoveryTestAsyncClient asyncClient; - private SqsAsyncClient sqsClient; - private TimestreamWriteAsyncClient timestreamClient; @Before public void setupClient() { @@ -170,15 +167,6 @@ public void syncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Cause .isInstanceOf(SdkClientException.class); } - @Test - public void asyncRequiredOperation_RequestCancelled_throws_EndpointDiscoveryFailure() { - stubResponse(mockAsyncClient, 500, "invalid", 15); - - assertThatThrownBy(() -> client.testDiscoveryRequired(r -> { - })) - .isInstanceOf(EndpointDiscoveryFailedException.class); - } - @Test public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_CausesSdkException() { stubResponse(mockAsyncClient, 500, "invalid", 15); From 9953d424188746a9d2aab80feb844b7146ae86d8 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 13 May 2024 11:05:32 -0700 Subject: [PATCH 06/17] modify get method to use getKey --- .../EndpointDiscoveryRefreshCache.java | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index 1da6b45ff4ce..869f42d98fb9 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -45,17 +45,8 @@ public static EndpointDiscoveryRefreshCache create(EndpointDiscoveryCacheLoader * @return The endpoint to use for this request */ public URI get(String accessKey, EndpointDiscoveryRequest request) { - String key = accessKey; - - // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. - if (key == null) { - key = ""; - } - - if (request.cacheKey().isPresent()) { - key = key + ":" + request.cacheKey().get(); - } + String key = getKey(accessKey, request); EndpointDiscoveryEndpoint endpoint = cache.get(key); if (endpoint == null) { From 2d93924417b70726e2ac476f97564a32acece222 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 13 May 2024 15:30:13 -0700 Subject: [PATCH 07/17] Add Unit Tests --- .../EndpointDiscoveryRefreshCache.java | 2 +- .../EndpointDiscoveryRefreshCacheTest.java | 85 +++++++++++++++++++ .../services/EndpointDiscoveryTest.java | 62 +------------- 3 files changed, 87 insertions(+), 62 deletions(-) create mode 100644 core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index 869f42d98fb9..b57be9bec5b2 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -115,7 +115,7 @@ public void evict(String key) { cache.remove(key); } - private String getKey(String accessKey, EndpointDiscoveryRequest request) { + String getKey(String accessKey, EndpointDiscoveryRequest request) { String key = accessKey; // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java new file mode 100644 index 000000000000..6207a8deaa02 --- /dev/null +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java @@ -0,0 +1,85 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.core.endpointdiscovery; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.net.URI; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class EndpointDiscoveryRefreshCacheTest { + + private EndpointDiscoveryRefreshCache endpointDiscoveryRefreshCache; + private EndpointDiscoveryCacheLoader mockClient; + private static final URI testURI = URI.create("test_endpoint"); + private static final String requestCacheKey = "request_cache_key"; + private static final String accessKey = "access_cache_key"; + + @BeforeEach + public void setup() { + this.mockClient= mock(EndpointDiscoveryCacheLoader.class); + this.endpointDiscoveryRefreshCache = EndpointDiscoveryRefreshCache.create(mockClient); + } + + @Test + public void getAsync_notRequired_returns_CompletedFuture() throws ExecutionException, InterruptedException { + when(mockClient.discoverEndpoint(any())).thenReturn(new CompletableFuture<>()); + EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() + .required(false) + .defaultEndpoint(testURI) + .build(); + assertThat(endpointDiscoveryRefreshCache.getAsync("key", request).isDone()).isEqualTo(true); + assertThat(endpointDiscoveryRefreshCache.getAsync("key", request).get()).isEqualTo(testURI); + + } + + @Test + public void getAsync_returns_CompletedFuture() throws ExecutionException, InterruptedException { + + when(mockClient.discoverEndpoint(any())).thenReturn(new CompletableFuture<>()); + EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() + .required(true) + .defaultEndpoint(testURI) + .build(); + CompletableFuture future = endpointDiscoveryRefreshCache.getAsync("key", request); + assertThat(future.isDone()).isEqualTo(false); + + future.complete(testURI); + + assertThat(future.isDone()).isEqualTo(true); + assertThat(future.get()).isEqualTo(testURI); + + } + + @Test + public void getKeyWithCacheKey() { + + EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() + .required(true) + .cacheKey(requestCacheKey) + .build(); + + assertThat(endpointDiscoveryRefreshCache.getKey(accessKey, request)).isEqualTo(accessKey + ":" + requestCacheKey); + + + } +} diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 7fd31ac191c9..5c0b81074cbd 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -15,17 +15,12 @@ package software.amazon.awssdk.services; -import static com.github.tomakehurst.wiremock.client.WireMock.aResponse; -import static com.github.tomakehurst.wiremock.client.WireMock.anyUrl; -import static com.github.tomakehurst.wiremock.client.WireMock.post; -import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder; import com.github.tomakehurst.wiremock.junit.WireMockRule; import io.reactivex.Flowable; import java.io.ByteArrayInputStream; @@ -34,17 +29,14 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.CompletionException; import java.util.concurrent.ExecutionException; import org.assertj.core.api.AbstractThrowableAssert; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.jupiter.api.Timeout; import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption; import software.amazon.awssdk.core.endpointdiscovery.EndpointDiscoveryFailedException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; @@ -55,14 +47,10 @@ import software.amazon.awssdk.http.SdkHttpResponse; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; -import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; -import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestAsyncClient; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; -import software.amazon.awssdk.services.sqs.SqsAsyncClient; -import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -175,43 +163,6 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus .isInstanceOf(SdkClientException.class); } - @Test - @Timeout(5) - public void endpointDiscoveryRequired_makes_nonBlockingCalls() throws InterruptedException, ExecutionException { - SdkEventLoopGroup group = SdkEventLoopGroup.builder() - .numberOfThreads(1) - .build(); - - SqsAsyncClient sqs = SqsAsyncClient.builder() - .asyncConfiguration(o -> o.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR, Runnable::run)) - .region(Region.US_WEST_2) - .httpClient(NettyNioAsyncHttpClient.builder() - .eventLoopGroup(group) - .build()) - .endpointOverride(URI.create("http://localhost:" + wireMock.port())) - .build(); - - TimestreamWriteAsyncClient timestream = TimestreamWriteAsyncClient.builder().region(Region.US_WEST_2) - .httpClient(NettyNioAsyncHttpClient.builder() - .eventLoopGroup(group).build()) - .endpointOverride(URI.create("http://localhost:" + wireMock.port())) - .build(); - - - stubFor(post(anyUrl()) - .willReturn(getResponse().get())); - - for(int i =0;i<100;i++) { - sqs.listQueues().whenComplete((r, e) -> { - timestream.listDatabases(req -> req.maxResults(1)); - assertThat(1).isEqualTo(1); - } - ).join(); - - assertThat(1).isEqualTo(1); - } - } - private AbstractThrowableAssert assertAsyncRequiredOperationCallThrowable() { try { asyncClient.testDiscoveryRequired(r -> { @@ -219,7 +170,7 @@ public void endpointDiscoveryRequired_makes_nonBlockingCalls() throws Interrupte throw new AssertionError(); } catch (InterruptedException e) { return assertThat(e); - } catch (ExecutionException | CompletionException e) { + } catch (ExecutionException e) { return assertThat(e.getCause()); } } @@ -293,15 +244,4 @@ private Answer> stubAsyncResponse(SdkHttpResponse respon return cf; }; } - - private CompletableFuture getResponse() { - return CompletableFuture.supplyAsync(() -> { - try { - Thread.sleep(10_000); - } catch (InterruptedException e) { - throw new RuntimeException(e); - } - return aResponse().withStatus(200); - }); - } } From d6c2f58da78a281adadfbf18717a9c58858da615 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 13 May 2024 15:38:59 -0700 Subject: [PATCH 08/17] Remove pom changes --- services/sqs/pom.xml | 6 ------ test/codegen-generated-classes-test/pom.xml | 12 ------------ 2 files changed, 18 deletions(-) diff --git a/services/sqs/pom.xml b/services/sqs/pom.xml index 82af9e0b511f..c704e85b4175 100644 --- a/services/sqs/pom.xml +++ b/services/sqs/pom.xml @@ -91,11 +91,5 @@ ${awsjavasdk.version} test - - software.amazon.awssdk - timestreamwrite - 2.25.45-SNAPSHOT - test - diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index 6bf30acb1664..438c7452f1e0 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -268,18 +268,6 @@ mockito-junit-jupiter test - - software.amazon.awssdk - sqs - 2.25.45-SNAPSHOT - test - - - software.amazon.awssdk - timestreamwrite - 2.25.45-SNAPSHOT - test - From 0b9e9b757c3057c45d63ae39fda0e73d694e2fc2 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 04:19:00 -0700 Subject: [PATCH 09/17] Added more tests --- .../EndpointDiscoveryRefreshCacheTest.java | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java index 6207a8deaa02..c58664b6f3c8 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java @@ -16,11 +16,13 @@ package software.amazon.awssdk.core.endpointdiscovery; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.net.URI; +import java.util.concurrent.CancellationException; import java.util.concurrent.CompletableFuture; import java.util.concurrent.ExecutionException; import org.junit.jupiter.api.BeforeEach; @@ -67,11 +69,27 @@ public void getAsync_returns_CompletedFuture() throws ExecutionException, Interr assertThat(future.isDone()).isEqualTo(true); assertThat(future.get()).isEqualTo(testURI); + } + + @Test + public void getAsync_future_cancelled() { + + when(mockClient.discoverEndpoint(any())).thenReturn(new CompletableFuture<>()); + EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() + .required(true) + .defaultEndpoint(testURI) + .build(); + CompletableFuture future = endpointDiscoveryRefreshCache.getAsync("key", request); + assertThat(future.isDone()).isEqualTo(false); + + future.cancel(true); + assertThat(future.isCancelled()).isEqualTo(true); + assertThrows(CancellationException.class, () -> future.get()); } @Test - public void getKeyWithCacheKey() { + public void getKeyWithCacheKeyAndCacheKey() { EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() .required(true) @@ -81,5 +99,18 @@ public void getKeyWithCacheKey() { assertThat(endpointDiscoveryRefreshCache.getKey(accessKey, request)).isEqualTo(accessKey + ":" + requestCacheKey); + } + + @Test + public void getKeyWithNullKey() { + + EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() + .required(true) + .cacheKey(requestCacheKey) + .build(); + + assertThat(endpointDiscoveryRefreshCache.getKey(null, request)).isEqualTo(":" + requestCacheKey); + + } } From eb594d7fd8343b71d1cd8b8f39adc11154d98602 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 14:22:52 -0700 Subject: [PATCH 10/17] Added Functional Test --- .../EndpointDiscoveryRefreshCache.java | 58 ++++++++++--------- .../EndpointDiscoveryRefreshCacheTest.java | 25 -------- .../services/EndpointDiscoveryTest.java | 46 +++++++++++++++ 3 files changed, 76 insertions(+), 53 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java index b57be9bec5b2..988e49ac3ea8 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCache.java @@ -49,33 +49,10 @@ public URI get(String accessKey, EndpointDiscoveryRequest request) { String key = getKey(accessKey, request); EndpointDiscoveryEndpoint endpoint = cache.get(key); - if (endpoint == null) { - if (request.required()) { - return cache.computeIfAbsent(key, k -> getAndJoin(request)).endpoint(); - } else { - EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() - .endpoint(request.defaultEndpoint()) - .expirationTime(Instant.now().plusSeconds(60)) - .build(); - - EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); - if (previousValue != null) { - // Someone else primed the cache. Use that endpoint (which may be temporary). - return previousValue.endpoint(); - } else { - // We primed the cache with the temporary endpoint. Kick off discovery in the background. - refreshCacheAsync(request, key); - } - return tempEndpoint.endpoint(); - } - } - - if (endpoint.expirationTime().isBefore(Instant.now())) { - cache.put(key, endpoint.toBuilder().expirationTime(Instant.now().plusSeconds(60)).build()); - refreshCacheAsync(request, key); + if (endpoint == null && request.required()) { + return cache.computeIfAbsent(key, k -> getAndJoin(request)).endpoint(); } - - return endpoint.endpoint(); + return returnCachedOrDefaultEndpoint(key, endpoint, request); } public CompletableFuture getAsync(String accessKey, EndpointDiscoveryRequest request) { @@ -89,7 +66,7 @@ public CompletableFuture getAsync(String accessKey, EndpointDiscoveryReques return discoverEndpointHandler(key, request); } // In the event of a cache hit, i.e. service call not required, defer to the synchronous code path method. - return CompletableFuture.completedFuture(get(accessKey, request)); + return CompletableFuture.completedFuture(returnCachedOrDefaultEndpoint(key, endpoint, request)); } private EndpointDiscoveryEndpoint getAndJoin(EndpointDiscoveryRequest request) { @@ -115,7 +92,7 @@ public void evict(String key) { cache.remove(key); } - String getKey(String accessKey, EndpointDiscoveryRequest request) { + private String getKey(String accessKey, EndpointDiscoveryRequest request) { String key = accessKey; // Support null (anonymous credentials) by mapping to empty-string. The backing cache does not support null. @@ -140,4 +117,29 @@ private CompletableFuture discoverEndpointHandler(String key, EndpointDisco ).endpoint(); }); } + + private URI returnCachedOrDefaultEndpoint(String key, EndpointDiscoveryEndpoint endpoint, EndpointDiscoveryRequest request) { + EndpointDiscoveryEndpoint tempEndpoint = EndpointDiscoveryEndpoint.builder() + .endpoint(request.defaultEndpoint()) + .expirationTime(Instant.now().plusSeconds(60)) + .build(); + + if (endpoint == null) { + EndpointDiscoveryEndpoint previousValue = cache.putIfAbsent(key, tempEndpoint); + if (previousValue != null) { + // Someone else primed the cache. Use that endpoint (which may be temporary). + return previousValue.endpoint(); + } + // We primed the cache with the temporary endpoint. Kick off discovery in the background. + refreshCacheAsync(request, key); + return tempEndpoint.endpoint(); + } + + if (endpoint.expirationTime().isBefore(Instant.now())) { + cache.put(key, endpoint.toBuilder().expirationTime(Instant.now().plusSeconds(60)).build()); + refreshCacheAsync(request, key); + } + + return endpoint.endpoint(); + } } diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java index c58664b6f3c8..e7d6eec2fe12 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java @@ -88,29 +88,4 @@ public void getAsync_future_cancelled() { } - @Test - public void getKeyWithCacheKeyAndCacheKey() { - - EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() - .required(true) - .cacheKey(requestCacheKey) - .build(); - - assertThat(endpointDiscoveryRefreshCache.getKey(accessKey, request)).isEqualTo(accessKey + ":" + requestCacheKey); - - - } - - @Test - public void getKeyWithNullKey() { - - EndpointDiscoveryRequest request = EndpointDiscoveryRequest.builder() - .required(true) - .cacheKey(requestCacheKey) - .build(); - - assertThat(endpointDiscoveryRefreshCache.getKey(null, request)).isEqualTo(":" + requestCacheKey); - - - } } diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 5c0b81074cbd..0fb28dca290b 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -37,6 +37,7 @@ import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption; import software.amazon.awssdk.core.endpointdiscovery.EndpointDiscoveryFailedException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; @@ -47,10 +48,15 @@ import software.amazon.awssdk.http.SdkHttpResponse; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; +import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestAsyncClient; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; +import software.amazon.awssdk.services.sqs.SqsAsyncClient; +import software.amazon.awssdk.services.sqs.model.ListQueuesResponse; +import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -64,6 +70,8 @@ public class EndpointDiscoveryTest { private EndpointDiscoveryTestClient client; private EndpointDiscoveryTestAsyncClient asyncClient; + private TimestreamWriteAsyncClient timestreamWriteAsyncNettyClient; + private SqsAsyncClient sqsAsyncNettyClient; @Before public void setupClient() { @@ -91,6 +99,31 @@ public void setupClient() { false)) .httpClient(mockAsyncClient) .build(); + + SdkEventLoopGroup group = SdkEventLoopGroup.builder() + .numberOfThreads(1) + .build(); + + timestreamWriteAsyncNettyClient = TimestreamWriteAsyncClient.builder().region(Region.US_WEST_2) + .endpointOverride(URI.create("http://localhost:" + wireMock.port())) + .overrideConfiguration(c -> c.putAdvancedOption( + SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, + false)) + .httpClient(NettyNioAsyncHttpClient.builder() + .eventLoopGroup(group).build()) + .build(); + + sqsAsyncNettyClient = SqsAsyncClient.builder() + .endpointOverride(URI.create("http://localhost:" + wireMock.port())) + .overrideConfiguration(c -> c.putAdvancedOption( + SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, + false)) + .asyncConfiguration(o -> o.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR, Runnable::run)) + .region(Region.US_WEST_2) + .httpClient(NettyNioAsyncHttpClient.builder() + .eventLoopGroup(group) + .build()) + .build(); } @Test @@ -163,6 +196,19 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus .isInstanceOf(SdkClientException.class); } + @Test + public void validate_endpointDiscoveryIsNotBlockingCall() { + + CompletableFuture future = sqsAsyncNettyClient.listQueues(); + future.whenComplete((r, e) -> { + timestreamWriteAsyncNettyClient.listDatabases(req -> req.maxResults(1)); + assertThat(1).isEqualTo(1); + } + ).join(); + + assertThat(1).isEqualTo(1); + } + private AbstractThrowableAssert assertAsyncRequiredOperationCallThrowable() { try { asyncClient.testDiscoveryRequired(r -> { From 61c775683333e3110cf74ee47b784fcfd2aadf19 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 15:09:53 -0700 Subject: [PATCH 11/17] Added service dependencies --- test/codegen-generated-classes-test/pom.xml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index 438c7452f1e0..cfed689de776 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -268,6 +268,18 @@ mockito-junit-jupiter test + + software.amazon.awssdk + sqs + 2.25.52-SNAPSHOT + test + + + software.amazon.awssdk + timestreamwrite + 2.25.52-SNAPSHOT + test + From 77fc52e40759235b37fdfa798a50792ed6aa5bfc Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 15:45:24 -0700 Subject: [PATCH 12/17] Removed hacky test. Specified one time runnable integration test in pr description --- test/codegen-generated-classes-test/pom.xml | 12 ----- .../services/EndpointDiscoveryTest.java | 46 ------------------- 2 files changed, 58 deletions(-) diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index 947a057908e5..df0dd72594ea 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -268,18 +268,6 @@ mockito-junit-jupiter test - - software.amazon.awssdk - sqs - 2.25.52-SNAPSHOT - test - - - software.amazon.awssdk - timestreamwrite - 2.25.52-SNAPSHOT - test - diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 0fb28dca290b..5c0b81074cbd 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -37,7 +37,6 @@ import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption; import software.amazon.awssdk.core.endpointdiscovery.EndpointDiscoveryFailedException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; @@ -48,15 +47,10 @@ import software.amazon.awssdk.http.SdkHttpResponse; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; -import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; -import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestAsyncClient; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; -import software.amazon.awssdk.services.sqs.SqsAsyncClient; -import software.amazon.awssdk.services.sqs.model.ListQueuesResponse; -import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -70,8 +64,6 @@ public class EndpointDiscoveryTest { private EndpointDiscoveryTestClient client; private EndpointDiscoveryTestAsyncClient asyncClient; - private TimestreamWriteAsyncClient timestreamWriteAsyncNettyClient; - private SqsAsyncClient sqsAsyncNettyClient; @Before public void setupClient() { @@ -99,31 +91,6 @@ public void setupClient() { false)) .httpClient(mockAsyncClient) .build(); - - SdkEventLoopGroup group = SdkEventLoopGroup.builder() - .numberOfThreads(1) - .build(); - - timestreamWriteAsyncNettyClient = TimestreamWriteAsyncClient.builder().region(Region.US_WEST_2) - .endpointOverride(URI.create("http://localhost:" + wireMock.port())) - .overrideConfiguration(c -> c.putAdvancedOption( - SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, - false)) - .httpClient(NettyNioAsyncHttpClient.builder() - .eventLoopGroup(group).build()) - .build(); - - sqsAsyncNettyClient = SqsAsyncClient.builder() - .endpointOverride(URI.create("http://localhost:" + wireMock.port())) - .overrideConfiguration(c -> c.putAdvancedOption( - SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, - false)) - .asyncConfiguration(o -> o.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR, Runnable::run)) - .region(Region.US_WEST_2) - .httpClient(NettyNioAsyncHttpClient.builder() - .eventLoopGroup(group) - .build()) - .build(); } @Test @@ -196,19 +163,6 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus .isInstanceOf(SdkClientException.class); } - @Test - public void validate_endpointDiscoveryIsNotBlockingCall() { - - CompletableFuture future = sqsAsyncNettyClient.listQueues(); - future.whenComplete((r, e) -> { - timestreamWriteAsyncNettyClient.listDatabases(req -> req.maxResults(1)); - assertThat(1).isEqualTo(1); - } - ).join(); - - assertThat(1).isEqualTo(1); - } - private AbstractThrowableAssert assertAsyncRequiredOperationCallThrowable() { try { asyncClient.testDiscoveryRequired(r -> { From 34ec1bad1fd8be118918d5a8e3e1563717c6c274 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 15:56:16 -0700 Subject: [PATCH 13/17] Added Functional tests --- test/codegen-generated-classes-test/pom.xml | 12 +++++ .../services/EndpointDiscoveryTest.java | 46 +++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index df0dd72594ea..c9e26d6154e6 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -268,6 +268,18 @@ mockito-junit-jupiter test + + software.amazon.awssdk + sqs + 2.25.53-SNAPSHOT + test + + + software.amazon.awssdk + timestreamwrite + 2.25.53-SNAPSHOT + test + diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 5c0b81074cbd..af8c617cd32a 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -37,6 +37,7 @@ import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; +import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption; import software.amazon.awssdk.core.endpointdiscovery.EndpointDiscoveryFailedException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; @@ -47,10 +48,15 @@ import software.amazon.awssdk.http.SdkHttpResponse; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; +import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; +import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestAsyncClient; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; +import software.amazon.awssdk.services.sqs.SqsAsyncClient; +import software.amazon.awssdk.services.sqs.model.ListQueuesResponse; +import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -64,6 +70,8 @@ public class EndpointDiscoveryTest { private EndpointDiscoveryTestClient client; private EndpointDiscoveryTestAsyncClient asyncClient; + private TimestreamWriteAsyncClient timestreamWriteAsyncNettyClient; + private SqsAsyncClient sqsAsyncNettyClient; @Before public void setupClient() { @@ -91,6 +99,31 @@ public void setupClient() { false)) .httpClient(mockAsyncClient) .build(); + + SdkEventLoopGroup group = SdkEventLoopGroup.builder() + .numberOfThreads(1) + .build(); + + timestreamWriteAsyncNettyClient = TimestreamWriteAsyncClient.builder().region(Region.US_WEST_2) + .endpointOverride(URI.create("http://localhost:" + wireMock.port())) + .overrideConfiguration(c -> c.putAdvancedOption( + SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, + false)) + .httpClient(NettyNioAsyncHttpClient.builder() + .eventLoopGroup(group).build()) + .build(); + + sqsAsyncNettyClient = SqsAsyncClient.builder() + .endpointOverride(URI.create("http://localhost:" + wireMock.port())) + .overrideConfiguration(c -> c.putAdvancedOption( + SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, + false)) + .asyncConfiguration(o -> o.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR, Runnable::run)) + .region(Region.US_WEST_2) + .httpClient(NettyNioAsyncHttpClient.builder() + .eventLoopGroup(group) + .build()) + .build(); } @Test @@ -162,6 +195,19 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus assertAsyncRequiredOperationCallThrowable() .isInstanceOf(SdkClientException.class); } + @Test + public void validate_endpointDiscoveryIsNotBlockingCall() { + + CompletableFuture future = sqsAsyncNettyClient.listQueues(); + future.whenComplete((r, e) -> { + timestreamWriteAsyncNettyClient.listDatabases(req -> req.maxResults(1)); + assertThat(1).isEqualTo(1); + } + ).join(); + + assertThat(1).isEqualTo(1); + } + private AbstractThrowableAssert assertAsyncRequiredOperationCallThrowable() { try { From 9411986a048f704e7da51bcabce9a0e20d3c7b25 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 15:58:36 -0700 Subject: [PATCH 14/17] Added timeout to test --- .../software/amazon/awssdk/services/EndpointDiscoveryTest.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index af8c617cd32a..6c42eb94a1a6 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -34,6 +34,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; +import org.junit.jupiter.api.Timeout; import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; @@ -196,6 +197,7 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus .isInstanceOf(SdkClientException.class); } @Test + @Timeout(5) public void validate_endpointDiscoveryIsNotBlockingCall() { CompletableFuture future = sqsAsyncNettyClient.listQueues(); From dd817b6b8846a2152e66c1adf2e9f88a8eb00b49 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 16:21:39 -0700 Subject: [PATCH 15/17] Remove Functional tests --- test/codegen-generated-classes-test/pom.xml | 14 +----- .../services/EndpointDiscoveryTest.java | 48 ------------------- 2 files changed, 1 insertion(+), 61 deletions(-) diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index c9e26d6154e6..438c7452f1e0 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -21,7 +21,7 @@ aws-sdk-java-pom software.amazon.awssdk - 2.25.53-SNAPSHOT + 2.25.45-SNAPSHOT ../../pom.xml @@ -268,18 +268,6 @@ mockito-junit-jupiter test - - software.amazon.awssdk - sqs - 2.25.53-SNAPSHOT - test - - - software.amazon.awssdk - timestreamwrite - 2.25.53-SNAPSHOT - test - diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java index 6c42eb94a1a6..5c0b81074cbd 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/EndpointDiscoveryTest.java @@ -34,11 +34,9 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.junit.jupiter.api.Timeout; import org.mockito.stubbing.Answer; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.auth.credentials.StaticCredentialsProvider; -import software.amazon.awssdk.core.client.config.SdkAdvancedAsyncClientOption; import software.amazon.awssdk.core.endpointdiscovery.EndpointDiscoveryFailedException; import software.amazon.awssdk.core.exception.SdkClientException; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; @@ -49,15 +47,10 @@ import software.amazon.awssdk.http.SdkHttpResponse; import software.amazon.awssdk.http.async.AsyncExecuteRequest; import software.amazon.awssdk.http.async.SdkAsyncHttpClient; -import software.amazon.awssdk.http.nio.netty.NettyNioAsyncHttpClient; -import software.amazon.awssdk.http.nio.netty.SdkEventLoopGroup; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestAsyncClient; import software.amazon.awssdk.services.endpointdiscoverytest.EndpointDiscoveryTestClient; import software.amazon.awssdk.services.endpointdiscoverytest.model.EndpointDiscoveryTestException; -import software.amazon.awssdk.services.sqs.SqsAsyncClient; -import software.amazon.awssdk.services.sqs.model.ListQueuesResponse; -import software.amazon.awssdk.services.timestreamwrite.TimestreamWriteAsyncClient; public class EndpointDiscoveryTest { @@ -71,8 +64,6 @@ public class EndpointDiscoveryTest { private EndpointDiscoveryTestClient client; private EndpointDiscoveryTestAsyncClient asyncClient; - private TimestreamWriteAsyncClient timestreamWriteAsyncNettyClient; - private SqsAsyncClient sqsAsyncNettyClient; @Before public void setupClient() { @@ -100,31 +91,6 @@ public void setupClient() { false)) .httpClient(mockAsyncClient) .build(); - - SdkEventLoopGroup group = SdkEventLoopGroup.builder() - .numberOfThreads(1) - .build(); - - timestreamWriteAsyncNettyClient = TimestreamWriteAsyncClient.builder().region(Region.US_WEST_2) - .endpointOverride(URI.create("http://localhost:" + wireMock.port())) - .overrideConfiguration(c -> c.putAdvancedOption( - SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, - false)) - .httpClient(NettyNioAsyncHttpClient.builder() - .eventLoopGroup(group).build()) - .build(); - - sqsAsyncNettyClient = SqsAsyncClient.builder() - .endpointOverride(URI.create("http://localhost:" + wireMock.port())) - .overrideConfiguration(c -> c.putAdvancedOption( - SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE, - false)) - .asyncConfiguration(o -> o.advancedOption(SdkAdvancedAsyncClientOption.FUTURE_COMPLETION_EXECUTOR, Runnable::run)) - .region(Region.US_WEST_2) - .httpClient(NettyNioAsyncHttpClient.builder() - .eventLoopGroup(group) - .build()) - .build(); } @Test @@ -196,20 +162,6 @@ public void asyncRequiredOperation_InvalidEndpointEndpointDiscoveryResponse_Caus assertAsyncRequiredOperationCallThrowable() .isInstanceOf(SdkClientException.class); } - @Test - @Timeout(5) - public void validate_endpointDiscoveryIsNotBlockingCall() { - - CompletableFuture future = sqsAsyncNettyClient.listQueues(); - future.whenComplete((r, e) -> { - timestreamWriteAsyncNettyClient.listDatabases(req -> req.maxResults(1)); - assertThat(1).isEqualTo(1); - } - ).join(); - - assertThat(1).isEqualTo(1); - } - private AbstractThrowableAssert assertAsyncRequiredOperationCallThrowable() { try { From 64fb88980c8ba723f3553703b679dada4414de24 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 16:22:33 -0700 Subject: [PATCH 16/17] Rebased changes --- test/codegen-generated-classes-test/pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/codegen-generated-classes-test/pom.xml b/test/codegen-generated-classes-test/pom.xml index 438c7452f1e0..df0dd72594ea 100644 --- a/test/codegen-generated-classes-test/pom.xml +++ b/test/codegen-generated-classes-test/pom.xml @@ -21,7 +21,7 @@ aws-sdk-java-pom software.amazon.awssdk - 2.25.45-SNAPSHOT + 2.25.53-SNAPSHOT ../../pom.xml From dea8e9d085f931049bc7348024c50de2075f0f72 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Tue, 14 May 2024 17:10:16 -0700 Subject: [PATCH 17/17] Modified test method to use assertThrownBy --- .../endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java index e7d6eec2fe12..8daa4f0909b4 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/endpointdiscovery/EndpointDiscoveryRefreshCacheTest.java @@ -16,6 +16,7 @@ package software.amazon.awssdk.core.endpointdiscovery; import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -84,7 +85,7 @@ public void getAsync_future_cancelled() { future.cancel(true); assertThat(future.isCancelled()).isEqualTo(true); - assertThrows(CancellationException.class, () -> future.get()); + assertThatThrownBy(future::get).isInstanceOf(CancellationException.class); }