Skip to content

Commit 8b67e6d

Browse files
millemsmillems
committed
Revert "Revert "Updated endpoint discovery behavior for operations that require endpoint discovery.""
This reverts commit ac0c45e.
1 parent 721af23 commit 8b67e6d

File tree

15 files changed

+684
-13
lines changed

15 files changed

+684
-13
lines changed

‎codegen/src/main/java/software/amazon/awssdk/codegen/model/config/customization/CustomizationConfig.java

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,13 @@ public class CustomizationConfig {
165165
*/
166166
private boolean enableEndpointDiscoveryMethodRequired = false;
167167

168+
/**
169+
* Allow a customer to set an endpoint override AND bypass endpoint discovery on their client even when endpoint discovery
170+
* enabled is true and endpoint discovery is required for an operation. This customization should almost never be "true"
171+
* because it creates a confusing customer experience.
172+
*/
173+
private boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations = false;
174+
168175
private CustomizationConfig() {
169176
}
170177

@@ -420,4 +427,14 @@ public boolean isEnableEndpointDiscoveryMethodRequired() {
420427
public void setEnableEndpointDiscoveryMethodRequired(boolean enableEndpointDiscoveryMethodRequired) {
421428
this.enableEndpointDiscoveryMethodRequired = enableEndpointDiscoveryMethodRequired;
422429
}
430+
431+
public boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations() {
432+
return allowEndpointOverrideForEndpointDiscoveryRequiredOperations;
433+
}
434+
435+
public void setAllowEndpointOverrideForEndpointDiscoveryRequiredOperations(
436+
boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations) {
437+
this.allowEndpointOverrideForEndpointDiscoveryRequiredOperations =
438+
allowEndpointOverrideForEndpointDiscoveryRequiredOperations;
439+
}
423440
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/AsyncClientClass.java

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,17 @@ private MethodSpec constructor(Builder classBuilder) {
166166
EndpointDiscoveryRefreshCache.class,
167167
poetExtensions.getClientClass(model.getNamingStrategy().getServiceName() +
168168
"AsyncEndpointDiscoveryCacheLoader"));
169+
170+
if (model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
171+
builder.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == "
172+
+ "Boolean.TRUE)");
173+
builder.addStatement("log.warn($S)",
174+
"Endpoint discovery is enabled for this client, and an endpoint override was also "
175+
+ "specified. This will disable endpoint discovery for methods that require it, instead "
176+
+ "using the specified endpoint override. This may or may not be what you intended.");
177+
builder.endControlFlow();
178+
}
179+
169180
builder.endControlFlow();
170181
}
171182

@@ -220,8 +231,37 @@ protected MethodSpec.Builder operationBody(MethodSpec.Builder builder, Operation
220231
builder.addCode(eventToByteBufferPublisher(opModel));
221232

222233
if (opModel.getEndpointDiscovery() != null) {
234+
builder.addStatement("boolean endpointDiscoveryEnabled = "
235+
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)");
236+
builder.addStatement("boolean endpointOverridden = "
237+
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE");
238+
239+
if (opModel.getEndpointDiscovery().isRequired()) {
240+
if (!model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
241+
builder.beginControlFlow("if (endpointOverridden)");
242+
builder.addStatement("throw new $T($S)", IllegalStateException.class,
243+
"This operation requires endpoint discovery, but an endpoint override was specified "
244+
+ "when the client was created. This is not supported.");
245+
builder.endControlFlow();
246+
247+
builder.beginControlFlow("if (!endpointDiscoveryEnabled)");
248+
builder.addStatement("throw new $T($S)", IllegalStateException.class,
249+
"This operation requires endpoint discovery, but endpoint discovery was disabled on the "
250+
+ "client.");
251+
builder.endControlFlow();
252+
} else {
253+
builder.beginControlFlow("if (endpointOverridden)");
254+
builder.addStatement("endpointDiscoveryEnabled = false");
255+
builder.nextControlFlow("else if (!endpointDiscoveryEnabled)");
256+
builder.addStatement("throw new $T($S)", IllegalStateException.class,
257+
"This operation requires endpoint discovery to be enabled, or for you to specify an "
258+
+ "endpoint override when the client is created.");
259+
builder.endControlFlow();
260+
}
261+
}
262+
223263
builder.addStatement("$T cachedEndpoint = null", URI.class);
224-
builder.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED))");
264+
builder.beginControlFlow("if (endpointDiscoveryEnabled)");
225265
builder.addStatement("\n\nString key = clientConfiguration.option($T.CREDENTIALS_PROVIDER).resolveCredentials()" +
226266
".accessKeyId()", AwsClientOption.class);
227267
builder.addStatement("EndpointDiscoveryRequest endpointDiscoveryRequest = $T.builder().required($L)" +

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/SyncClientClass.java

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import software.amazon.awssdk.metrics.MetricCollector;
6161
import software.amazon.awssdk.metrics.MetricPublisher;
6262
import software.amazon.awssdk.metrics.NoOpMetricCollector;
63+
import software.amazon.awssdk.utils.Logger;
6364

6465
//TODO Make SyncClientClass extend SyncClientInterface (similar to what we do in AsyncClientClass)
6566
public class SyncClientClass implements ClassSpec {
@@ -86,6 +87,7 @@ public TypeSpec poetSpec() {
8687
.addSuperinterface(interfaceClass)
8788
.addJavadoc("Internal implementation of {@link $1T}.\n\n@see $1T#builder()",
8889
interfaceClass)
90+
.addField(logger())
8991
.addField(SyncClientHandler.class, "clientHandler", PRIVATE, FINAL)
9092
.addField(protocolSpec.protocolFactory(model))
9193
.addField(SdkClientConfiguration.class, "clientConfiguration", PRIVATE, FINAL)
@@ -119,6 +121,12 @@ public TypeSpec poetSpec() {
119121
return classBuilder.build();
120122
}
121123

124+
private FieldSpec logger() {
125+
return FieldSpec.builder(Logger.class, "log", PRIVATE, STATIC, FINAL)
126+
.initializer("$T.loggerFor($T.class)", Logger.class, className)
127+
.build();
128+
}
129+
122130
private MethodSpec nameMethod() {
123131
return MethodSpec.methodBuilder("serviceName")
124132
.addAnnotation(Override.class)
@@ -154,6 +162,17 @@ private MethodSpec constructor() {
154162
EndpointDiscoveryRefreshCache.class,
155163
poetExtensions.getClientClass(model.getNamingStrategy().getServiceName() +
156164
"EndpointDiscoveryCacheLoader"));
165+
166+
if (model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
167+
builder.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == "
168+
+ "Boolean.TRUE)");
169+
builder.addStatement("log.warn(() -> $S)",
170+
"Endpoint discovery is enabled for this client, and an endpoint override was also "
171+
+ "specified. This will disable endpoint discovery for methods that require it, instead "
172+
+ "using the specified endpoint override. This may or may not be what you intended.");
173+
builder.endControlFlow();
174+
}
175+
157176
builder.endControlFlow();
158177
}
159178

@@ -181,8 +200,37 @@ private List<MethodSpec> operationMethodSpecs(OperationModel opModel) {
181200
protocolSpec.errorResponseHandler(opModel).ifPresent(method::addCode);
182201

183202
if (opModel.getEndpointDiscovery() != null) {
203+
method.addStatement("boolean endpointDiscoveryEnabled = "
204+
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)");
205+
method.addStatement("boolean endpointOverridden = "
206+
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE");
207+
208+
if (opModel.getEndpointDiscovery().isRequired()) {
209+
if (!model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
210+
method.beginControlFlow("if (endpointOverridden)");
211+
method.addStatement("throw new $T($S)", IllegalStateException.class,
212+
"This operation requires endpoint discovery, but an endpoint override was specified "
213+
+ "when the client was created. This is not supported.");
214+
method.endControlFlow();
215+
216+
method.beginControlFlow("if (!endpointDiscoveryEnabled)");
217+
method.addStatement("throw new $T($S)", IllegalStateException.class,
218+
"This operation requires endpoint discovery, but endpoint discovery was disabled on the "
219+
+ "client.");
220+
method.endControlFlow();
221+
} else {
222+
method.beginControlFlow("if (endpointOverridden)");
223+
method.addStatement("endpointDiscoveryEnabled = false");
224+
method.nextControlFlow("else if (!endpointDiscoveryEnabled)");
225+
method.addStatement("throw new $T($S)", IllegalStateException.class,
226+
"This operation requires endpoint discovery to be enabled, or for you to specify an "
227+
+ "endpoint override when the client is created.");
228+
method.endControlFlow();
229+
}
230+
}
231+
184232
method.addStatement("$T cachedEndpoint = null", URI.class);
185-
method.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED))");
233+
method.beginControlFlow("if (endpointDiscoveryEnabled)");
186234
method.addStatement("\n\nString key = clientConfiguration.option($T.CREDENTIALS_PROVIDER)." +
187235
"resolveCredentials().accessKeyId()", AwsClientOption.class);
188236
method.addStatement("EndpointDiscoveryRequest endpointDiscoveryRequest = $T.builder().required($L)" +

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-endpoint-discovery-async.java

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,18 @@ public CompletableFuture<TestDiscoveryIdentifiersRequiredResponse> testDiscovery
167167

168168
HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
169169
operationMetadata);
170+
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
171+
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
172+
if (endpointOverridden) {
173+
throw new IllegalStateException(
174+
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
175+
}
176+
if (!endpointDiscoveryEnabled) {
177+
throw new IllegalStateException(
178+
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
179+
}
170180
URI cachedEndpoint = null;
171-
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {
181+
if (endpointDiscoveryEnabled) {
172182

173183
String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
174184
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true)
@@ -231,8 +241,10 @@ public CompletableFuture<TestDiscoveryOptionalResponse> testDiscoveryOptional(
231241

232242
HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
233243
operationMetadata);
244+
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
245+
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
234246
URI cachedEndpoint = null;
235-
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {
247+
if (endpointDiscoveryEnabled) {
236248

237249
String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
238250
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(false)
@@ -295,8 +307,18 @@ public CompletableFuture<TestDiscoveryRequiredResponse> testDiscoveryRequired(
295307

296308
HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
297309
operationMetadata);
310+
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
311+
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
312+
if (endpointOverridden) {
313+
throw new IllegalStateException(
314+
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
315+
}
316+
if (!endpointDiscoveryEnabled) {
317+
throw new IllegalStateException(
318+
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
319+
}
298320
URI cachedEndpoint = null;
299-
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {
321+
if (endpointDiscoveryEnabled) {
300322

301323
String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
302324
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true)

‎codegen/src/test/resources/software/amazon/awssdk/codegen/poet/client/test-endpoint-discovery-sync.java

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
import software.amazon.awssdk.services.endpointdiscoverytest.transform.TestDiscoveryIdentifiersRequiredRequestMarshaller;
3939
import software.amazon.awssdk.services.endpointdiscoverytest.transform.TestDiscoveryOptionalRequestMarshaller;
4040
import software.amazon.awssdk.services.endpointdiscoverytest.transform.TestDiscoveryRequiredRequestMarshaller;
41+
import software.amazon.awssdk.utils.Logger;
4142

4243
/**
4344
* Internal implementation of {@link EndpointDiscoveryTestClient}.
@@ -47,6 +48,8 @@
4748
@Generated("software.amazon.awssdk:codegen")
4849
@SdkInternalApi
4950
final class DefaultEndpointDiscoveryTestClient implements EndpointDiscoveryTestClient {
51+
private static final Logger log = Logger.loggerFor(DefaultEndpointDiscoveryTestClient.class);
52+
5053
private final SyncClientHandler clientHandler;
5154

5255
private final AwsJsonProtocolFactory protocolFactory;
@@ -139,8 +142,18 @@ public TestDiscoveryIdentifiersRequiredResponse testDiscoveryIdentifiersRequired
139142

140143
HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
141144
operationMetadata);
145+
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
146+
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
147+
if (endpointOverridden) {
148+
throw new IllegalStateException(
149+
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
150+
}
151+
if (!endpointDiscoveryEnabled) {
152+
throw new IllegalStateException(
153+
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
154+
}
142155
URI cachedEndpoint = null;
143-
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {
156+
if (endpointDiscoveryEnabled) {
144157

145158
String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
146159
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true)
@@ -191,8 +204,10 @@ public TestDiscoveryOptionalResponse testDiscoveryOptional(TestDiscoveryOptional
191204

192205
HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
193206
operationMetadata);
207+
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
208+
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
194209
URI cachedEndpoint = null;
195-
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {
210+
if (endpointDiscoveryEnabled) {
196211

197212
String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
198213
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(false)
@@ -242,8 +257,18 @@ public TestDiscoveryRequiredResponse testDiscoveryRequired(TestDiscoveryRequired
242257

243258
HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
244259
operationMetadata);
260+
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
261+
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
262+
if (endpointOverridden) {
263+
throw new IllegalStateException(
264+
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
265+
}
266+
if (!endpointDiscoveryEnabled) {
267+
throw new IllegalStateException(
268+
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
269+
}
245270
URI cachedEndpoint = null;
246-
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {
271+
if (endpointDiscoveryEnabled) {
247272

248273
String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
249274
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true)

0 commit comments

Comments
 (0)