Skip to content

Commit

Permalink
Merge pull request #941 from aws/dongie/revert-2004
Browse files Browse the repository at this point in the history
Revert "Updated endpoint discovery behavior for operations that requi…
  • Loading branch information
dagnir authored Aug 28, 2020
2 parents 7ce6281 + ac0c45e commit e5272e3
Show file tree
Hide file tree
Showing 15 changed files with 13 additions and 684 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -165,13 +165,6 @@ public class CustomizationConfig {
*/
private boolean enableEndpointDiscoveryMethodRequired = false;

/**
* Allow a customer to set an endpoint override AND bypass endpoint discovery on their client even when endpoint discovery
* enabled is true and endpoint discovery is required for an operation. This customization should almost never be "true"
* because it creates a confusing customer experience.
*/
private boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations = false;

private CustomizationConfig() {
}

Expand Down Expand Up @@ -427,14 +420,4 @@ public boolean isEnableEndpointDiscoveryMethodRequired() {
public void setEnableEndpointDiscoveryMethodRequired(boolean enableEndpointDiscoveryMethodRequired) {
this.enableEndpointDiscoveryMethodRequired = enableEndpointDiscoveryMethodRequired;
}

public boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations() {
return allowEndpointOverrideForEndpointDiscoveryRequiredOperations;
}

public void setAllowEndpointOverrideForEndpointDiscoveryRequiredOperations(
boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations) {
this.allowEndpointOverrideForEndpointDiscoveryRequiredOperations =
allowEndpointOverrideForEndpointDiscoveryRequiredOperations;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,17 +166,6 @@ private MethodSpec constructor(Builder classBuilder) {
EndpointDiscoveryRefreshCache.class,
poetExtensions.getClientClass(model.getNamingStrategy().getServiceName() +
"AsyncEndpointDiscoveryCacheLoader"));

if (model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
builder.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == "
+ "Boolean.TRUE)");
builder.addStatement("log.warn($S)",
"Endpoint discovery is enabled for this client, and an endpoint override was also "
+ "specified. This will disable endpoint discovery for methods that require it, instead "
+ "using the specified endpoint override. This may or may not be what you intended.");
builder.endControlFlow();
}

builder.endControlFlow();
}

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

if (opModel.getEndpointDiscovery() != null) {
builder.addStatement("boolean endpointDiscoveryEnabled = "
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)");
builder.addStatement("boolean endpointOverridden = "
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE");

if (opModel.getEndpointDiscovery().isRequired()) {
if (!model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
builder.beginControlFlow("if (endpointOverridden)");
builder.addStatement("throw new $T($S)", IllegalStateException.class,
"This operation requires endpoint discovery, but an endpoint override was specified "
+ "when the client was created. This is not supported.");
builder.endControlFlow();

builder.beginControlFlow("if (!endpointDiscoveryEnabled)");
builder.addStatement("throw new $T($S)", IllegalStateException.class,
"This operation requires endpoint discovery, but endpoint discovery was disabled on the "
+ "client.");
builder.endControlFlow();
} else {
builder.beginControlFlow("if (endpointOverridden)");
builder.addStatement("endpointDiscoveryEnabled = false");
builder.nextControlFlow("else if (!endpointDiscoveryEnabled)");
builder.addStatement("throw new $T($S)", IllegalStateException.class,
"This operation requires endpoint discovery to be enabled, or for you to specify an "
+ "endpoint override when the client is created.");
builder.endControlFlow();
}
}

builder.addStatement("$T cachedEndpoint = null", URI.class);
builder.beginControlFlow("if (endpointDiscoveryEnabled)");
builder.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED))");
builder.addStatement("\n\nString key = clientConfiguration.option($T.CREDENTIALS_PROVIDER).resolveCredentials()" +
".accessKeyId()", AwsClientOption.class);
builder.addStatement("EndpointDiscoveryRequest endpointDiscoveryRequest = $T.builder().required($L)" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
import software.amazon.awssdk.metrics.MetricCollector;
import software.amazon.awssdk.metrics.MetricPublisher;
import software.amazon.awssdk.metrics.NoOpMetricCollector;
import software.amazon.awssdk.utils.Logger;

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

private FieldSpec logger() {
return FieldSpec.builder(Logger.class, "log", PRIVATE, STATIC, FINAL)
.initializer("$T.loggerFor($T.class)", Logger.class, className)
.build();
}

private MethodSpec nameMethod() {
return MethodSpec.methodBuilder("serviceName")
.addAnnotation(Override.class)
Expand Down Expand Up @@ -162,17 +154,6 @@ private MethodSpec constructor() {
EndpointDiscoveryRefreshCache.class,
poetExtensions.getClientClass(model.getNamingStrategy().getServiceName() +
"EndpointDiscoveryCacheLoader"));

if (model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
builder.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == "
+ "Boolean.TRUE)");
builder.addStatement("log.warn(() -> $S)",
"Endpoint discovery is enabled for this client, and an endpoint override was also "
+ "specified. This will disable endpoint discovery for methods that require it, instead "
+ "using the specified endpoint override. This may or may not be what you intended.");
builder.endControlFlow();
}

builder.endControlFlow();
}

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

if (opModel.getEndpointDiscovery() != null) {
method.addStatement("boolean endpointDiscoveryEnabled = "
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)");
method.addStatement("boolean endpointOverridden = "
+ "clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE");

if (opModel.getEndpointDiscovery().isRequired()) {
if (!model.getCustomizationConfig().allowEndpointOverrideForEndpointDiscoveryRequiredOperations()) {
method.beginControlFlow("if (endpointOverridden)");
method.addStatement("throw new $T($S)", IllegalStateException.class,
"This operation requires endpoint discovery, but an endpoint override was specified "
+ "when the client was created. This is not supported.");
method.endControlFlow();

method.beginControlFlow("if (!endpointDiscoveryEnabled)");
method.addStatement("throw new $T($S)", IllegalStateException.class,
"This operation requires endpoint discovery, but endpoint discovery was disabled on the "
+ "client.");
method.endControlFlow();
} else {
method.beginControlFlow("if (endpointOverridden)");
method.addStatement("endpointDiscoveryEnabled = false");
method.nextControlFlow("else if (!endpointDiscoveryEnabled)");
method.addStatement("throw new $T($S)", IllegalStateException.class,
"This operation requires endpoint discovery to be enabled, or for you to specify an "
+ "endpoint override when the client is created.");
method.endControlFlow();
}
}

method.addStatement("$T cachedEndpoint = null", URI.class);
method.beginControlFlow("if (endpointDiscoveryEnabled)");
method.beginControlFlow("if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED))");
method.addStatement("\n\nString key = clientConfiguration.option($T.CREDENTIALS_PROVIDER)." +
"resolveCredentials().accessKeyId()", AwsClientOption.class);
method.addStatement("EndpointDiscoveryRequest endpointDiscoveryRequest = $T.builder().required($L)" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,18 +167,8 @@ public CompletableFuture<TestDiscoveryIdentifiersRequiredResponse> testDiscovery

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
operationMetadata);
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
if (endpointOverridden) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
}
if (!endpointDiscoveryEnabled) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
}
URI cachedEndpoint = null;
if (endpointDiscoveryEnabled) {
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {

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

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
operationMetadata);
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
URI cachedEndpoint = null;
if (endpointDiscoveryEnabled) {
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {

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

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
operationMetadata);
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
if (endpointOverridden) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
}
if (!endpointDiscoveryEnabled) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
}
URI cachedEndpoint = null;
if (endpointDiscoveryEnabled) {
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {

String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
import software.amazon.awssdk.services.endpointdiscoverytest.transform.TestDiscoveryIdentifiersRequiredRequestMarshaller;
import software.amazon.awssdk.services.endpointdiscoverytest.transform.TestDiscoveryOptionalRequestMarshaller;
import software.amazon.awssdk.services.endpointdiscoverytest.transform.TestDiscoveryRequiredRequestMarshaller;
import software.amazon.awssdk.utils.Logger;

/**
* Internal implementation of {@link EndpointDiscoveryTestClient}.
Expand All @@ -48,8 +47,6 @@
@Generated("software.amazon.awssdk:codegen")
@SdkInternalApi
final class DefaultEndpointDiscoveryTestClient implements EndpointDiscoveryTestClient {
private static final Logger log = Logger.loggerFor(DefaultEndpointDiscoveryTestClient.class);

private final SyncClientHandler clientHandler;

private final AwsJsonProtocolFactory protocolFactory;
Expand Down Expand Up @@ -142,18 +139,8 @@ public TestDiscoveryIdentifiersRequiredResponse testDiscoveryIdentifiersRequired

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
operationMetadata);
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
if (endpointOverridden) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
}
if (!endpointDiscoveryEnabled) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
}
URI cachedEndpoint = null;
if (endpointDiscoveryEnabled) {
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {

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

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
operationMetadata);
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
URI cachedEndpoint = null;
if (endpointDiscoveryEnabled) {
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {

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

HttpResponseHandler<AwsServiceException> errorResponseHandler = createErrorResponseHandler(protocolFactory,
operationMetadata);
boolean endpointDiscoveryEnabled = clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED);
boolean endpointOverridden = clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN) == Boolean.TRUE;
if (endpointOverridden) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but an endpoint override was specified when the client was created. This is not supported.");
}
if (!endpointDiscoveryEnabled) {
throw new IllegalStateException(
"This operation requires endpoint discovery, but endpoint discovery was disabled on the client.");
}
URI cachedEndpoint = null;
if (endpointDiscoveryEnabled) {
if (clientConfiguration.option(SdkClientOption.ENDPOINT_DISCOVERY_ENABLED)) {

String key = clientConfiguration.option(AwsClientOption.CREDENTIALS_PROVIDER).resolveCredentials().accessKeyId();
EndpointDiscoveryRequest endpointDiscoveryRequest = EndpointDiscoveryRequest.builder().required(true)
Expand Down
Loading

0 comments on commit e5272e3

Please sign in to comment.