From bd94b6ebc90e6c8cafd2db2dd1920ba7303cf164 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Thu, 21 Sep 2023 08:15:02 -0700 Subject: [PATCH 1/5] Add missing properties and polish the codegen path --- .../ServiceClientConfigurationClass.java | 243 ++++++++++++------ .../model/serviceclientconfiguration.java | 92 ++++--- .../builder/SdkDefaultClientBuilder.java | 6 +- .../config/ClientOverrideConfiguration.java | 46 ---- .../internal/SdkClientConfigurationUtil.java | 79 ++++++ .../SdkInternalAdvancedClientOption.java | 15 +- ...CodegenServiceClientConfigurationTest.java | 215 ++++++++++++++++ 7 files changed, 536 insertions(+), 160 deletions(-) create mode 100644 core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java rename core/sdk-core/src/main/java/software/amazon/awssdk/core/{ => client/config}/internal/SdkInternalAdvancedClientOption.java (67%) create mode 100644 test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java index aa58f50223df..142890c890e8 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java @@ -38,11 +38,13 @@ import software.amazon.awssdk.codegen.poet.ClassSpec; import software.amazon.awssdk.codegen.poet.PoetUtils; import software.amazon.awssdk.codegen.poet.auth.scheme.AuthSchemeSpecUtils; +import software.amazon.awssdk.core.SdkServiceClientConfiguration; import software.amazon.awssdk.core.client.config.ClientOption; import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; -import software.amazon.awssdk.core.internal.SdkInternalAdvancedClientOption; +import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; +import software.amazon.awssdk.core.client.config.internal.SdkInternalAdvancedClientOption; import software.amazon.awssdk.endpoints.EndpointProvider; import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; @@ -59,23 +61,32 @@ public ServiceClientConfigurationClass(IntermediateModel model) { String serviceId = model.getMetadata().getServiceName(); this.defaultClientMetadataClassName = ClassName.get(basePackage, serviceId + "ServiceClientConfiguration"); this.authSchemeSpecUtils = new AuthSchemeSpecUtils(model); - } @Override public TypeSpec poetSpec() { - return PoetUtils.createClassBuilder(defaultClientMetadataClassName) - .superclass(AwsServiceClientConfiguration.class) - .addJavadoc("Class to expose the service client settings to the user. Implementation of {@link $T}", - AwsServiceClientConfiguration.class) - .addMethod(constructor()) - .addMethod(builderMethod()) - .addMethod(builderFromSdkClientConfiguration()) - .addModifiers(PUBLIC, FINAL) - .addAnnotation(SdkPublicApi.class) - .addType(builderInterfaceSpec()) - .addType(builderImplSpec()) - .build(); + TypeSpec.Builder builder = PoetUtils.createClassBuilder(defaultClientMetadataClassName) + .superclass(AwsServiceClientConfiguration.class) + .addJavadoc("Class to expose the service client settings to the user. " + + "Implementation of {@link $T}", + AwsServiceClientConfiguration.class); + + builder.addMethod(constructor()); + for (Field field : serviceClientConfigurationFields()) { + addLocalFieldForDataIfNeeded(field, builder); + if (field.isLocalField()) { + builder.addMethod(getterForDataField(field)); + } + } + + return builder.addMethod(builderMethod()) + .addMethod(builderFromSdkClientConfiguration()) + .addModifiers(PUBLIC, FINAL) + .addAnnotation(SdkPublicApi.class) + .addType(builderInterfaceSpec()) + .addType(builderInternalInterfaceSpec()) + .addType(builderImplSpec()) + .build(); } @Override @@ -84,11 +95,16 @@ public ClassName className() { } private MethodSpec constructor() { - return MethodSpec.constructorBuilder() - .addModifiers(PRIVATE) - .addParameter(className().nestedClass("Builder"), "builder") - .addStatement("super(builder)") - .build(); + MethodSpec.Builder builder = MethodSpec.constructorBuilder() + .addModifiers(PRIVATE) + .addParameter(className().nestedClass("Builder"), "builder"); + builder.addStatement("super(builder)"); + for (Field field : serviceClientConfigurationFields()) { + if (field.isLocalField()) { + builder.addStatement("this.$L = builder.$L()", field.name, field.name); + } + } + return builder.build(); } private MethodSpec builderMethod() { @@ -104,9 +120,8 @@ private MethodSpec builderFromSdkClientConfiguration() { return MethodSpec.methodBuilder("builder") .addModifiers(PUBLIC, STATIC) .addParameter(SdkClientConfiguration.Builder.class, "builder") + .returns(className().nestedClass("BuilderInternal")) .addStatement("return new BuilderImpl(builder)") - .returns(className().nestedClass("Builder")) - .addJavadoc("") .build(); } @@ -132,12 +147,26 @@ private TypeSpec builderInterfaceSpec() { return builder.build(); } + private TypeSpec builderInternalInterfaceSpec() { + TypeSpec.Builder builder = TypeSpec.interfaceBuilder("BuilderInternal") + .addModifiers(PUBLIC) + .addSuperinterface(className().nestedClass("Builder")) + .addJavadoc("An internal builder for creating a {@link $T}", className()); + + builder.addMethod(MethodSpec.methodBuilder("buildSdkClientConfiguration") + .addModifiers(PUBLIC, ABSTRACT) + .returns(SdkClientConfiguration.class) + .build()); + return builder.build(); + } + private TypeSpec builderImplSpec() { TypeSpec.Builder builder = TypeSpec.classBuilder("BuilderImpl") .addModifiers(PRIVATE, STATIC, FINAL) - .addSuperinterface(className().nestedClass("Builder")); + .addSuperinterface(className().nestedClass("BuilderInternal")); builder.addField(SdkClientConfiguration.Builder.class, "builder", PRIVATE, FINAL); + builder.addMethod(MethodSpec.constructorBuilder() .addModifiers(PRIVATE) .addStatement("this.builder = $T.builder()", SdkClientConfiguration.class) @@ -148,11 +177,10 @@ private TypeSpec builderImplSpec() { .addParameter(SdkClientConfiguration.Builder.class, "builder") .addStatement("this.builder = builder", SdkClientConfiguration.class) .build()); - for (Field field : serviceClientConfigurationFields()) { - addLocalFieldIfNeeded(field, builder); + addLocalFieldForBuilderIfNeeded(field, builder); builder.addMethod(setterForField(field)); - builder.addMethod(getterForField(field)); + builder.addMethod(getterForBuilderField(field)); } builder.addMethod(MethodSpec.methodBuilder("build") @@ -164,24 +192,34 @@ private TypeSpec builderImplSpec() { builder.addMethod(MethodSpec.methodBuilder("buildSdkClientConfiguration") .addModifiers(PUBLIC) + .addAnnotation(Override.class) .returns(SdkClientConfiguration.class) + .addStatement("$T overrideConfiguration = overrideConfiguration()", + ClientOverrideConfiguration.class) .beginControlFlow("if (overrideConfiguration != null)") - .addStatement("overrideConfiguration.addOverridesToConfiguration(builder)") + .addStatement("$T.copyOverridesToConfiguration(overrideConfiguration, builder)", + SdkClientConfigurationUtil.class) .endControlFlow() .addStatement("return builder.build()") .build()); return builder.build(); } - private void addLocalFieldIfNeeded(Field field, TypeSpec.Builder builder) { + private void addLocalFieldForBuilderIfNeeded(Field field, TypeSpec.Builder builder) { if (field.optionClass == null) { builder.addField(field.type, field.name, PRIVATE); } } + private void addLocalFieldForDataIfNeeded(Field field, TypeSpec.Builder builder) { + if (field.isLocalField()) { + builder.addField(field.type, field.name, PRIVATE, FINAL); + } + } + private MethodSpec setterForField(Field field) { MethodSpec.Builder builder = baseSetterForField(field); - if (!field.isInherited) { + if (field.isLocalField()) { builder.addAnnotation(Override.class); } if (field.optionClass == null) { @@ -202,24 +240,36 @@ private MethodSpec.Builder baseSetterForField(Field field) { .addParameter(field.type, field.name) .addJavadoc("Sets the value for " + field.doc) .returns(className().nestedClass("Builder")); - if (field.isInherited) { + if (!field.isLocalField()) { builder.addAnnotation(Override.class); } return builder; } - private MethodSpec getterForField(Field field) { + private MethodSpec getterForBuilderField(Field field) { + return getterForField(field, "builder", false); + } + + private MethodSpec getterForDataField(Field field) { + return getterForField(field, "config", true); + } + + private MethodSpec getterForField(Field field, String fieldName, boolean forDataGetter) { MethodSpec.Builder builder = baseGetterForField(field); - if (!field.isInherited) { + if (!forDataGetter && field.isLocalField()) { builder.addAnnotation(Override.class); } + if (forDataGetter && field.isLocalField()) { + return builder.addStatement("return $L", field.name) + .build(); + } if (field.optionClass == null) { return builder.addStatement("return $L", field.name) .build(); } if (field.baseType != null) { - return builder.addStatement("$T result = builder.option($T.$L)", - field.baseType, field.optionClass, field.optionName) + return builder.addStatement("$T result = $L.option($T.$L)", + field.baseType, fieldName, field.optionClass, field.optionName) .beginControlFlow("if (result == null)") .addStatement("return null") .endControlFlow() @@ -227,9 +277,8 @@ private MethodSpec getterForField(Field field) { Validate.class, field.type, "Expected an instance of ", field.type) .build(); - } - return builder.addStatement("return builder.option($T.$L)", field.optionClass, field.optionName) + return builder.addStatement("return $L.option($T.$L)", fieldName, field.optionClass, field.optionName) .build(); } @@ -238,7 +287,7 @@ private MethodSpec.Builder baseGetterForField(Field field) { .addModifiers(PUBLIC) .addJavadoc("Gets the value for " + field.doc) .returns(field.type); - if (field.isInherited) { + if (!field.isLocalField()) { builder.addAnnotation(Override.class); } return builder; @@ -251,13 +300,10 @@ private MethodSpec.Builder baseGetterForField(Field field) { */ private List serviceClientConfigurationFields() { List fields = new ArrayList<>(baseServiceClientConfigurationFields()); - fields.add(Field.builder() - .name("authSchemeProvider") - .type(authSchemeSpecUtils.providerInterfaceName()) + fields.add(Field.builder("authSchemeProvider", authSchemeSpecUtils.providerInterfaceName()) .doc("auth scheme provider") .optionClass(SdkClientOption.class) - .optionName("AUTH_SCHEME_PROVIDER") - .isInherited(false) + .optionValue(SdkClientOption.AUTH_SCHEME_PROVIDER) .baseType(ClassName.get(AuthSchemeProvider.class)) .build()); return fields; @@ -265,39 +311,37 @@ private List serviceClientConfigurationFields() { private static List baseServiceClientConfigurationFields() { return Arrays.asList( - Field.builder() + Field.builder("overrideConfiguration", ClientOverrideConfiguration.class) .doc("client override configuration") - .name("overrideConfiguration") - .type(ClientOverrideConfiguration.class) - .build(), - Field.builder() - .doc("AWS region") - .name("region") - .type(Region.class) - .optionClass(AwsClientOption.class) - .optionName("AWS_REGION") + .definingClass(SdkServiceClientConfiguration.class) + .optionClass(SdkInternalAdvancedClientOption.class) + .optionValue(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION) .build(), - Field.builder() + Field.builder("endpointOverride", URI.class) .doc("endpoint override") - .name("endpointOverride") - .type(URI.class) + .definingClass(SdkServiceClientConfiguration.class) .optionClass(SdkInternalAdvancedClientOption.class) - .optionName("ENDPOINT_OVERRIDE_VALUE") + .optionValue(SdkInternalAdvancedClientOption.ENDPOINT_OVERRIDE_VALUE) .build(), - Field.builder() - .name("endpointProvider") - .type(EndpointProvider.class) + Field.builder("endpointProvider", EndpointProvider.class) .doc("endpoint provider") + .definingClass(SdkServiceClientConfiguration.class) .optionClass(SdkClientOption.class) - .optionName("ENDPOINT_PROVIDER") + .optionValue(SdkClientOption.ENDPOINT_PROVIDER) + .build(), + Field.builder("region", Region.class) + .doc("AWS region") + .definingClass(AwsServiceClientConfiguration.class) + .optionClass(AwsClientOption.class) + .optionValue(AwsClientOption.AWS_REGION) .build(), - Field.builder() - .name("credentialsProvider") - .type(ParameterizedTypeName.get(ClassName.get(IdentityProvider.class), - WildcardTypeName.subtypeOf(AwsCredentialsIdentity.class))) + Field.builder("credentialsProvider", + ParameterizedTypeName.get(ClassName.get(IdentityProvider.class), + WildcardTypeName.subtypeOf(AwsCredentialsIdentity.class))) .doc("credentials provider") + .definingClass(AwsServiceClientConfiguration.class) .optionClass(AwsClientOption.class) - .optionName("CREDENTIALS_IDENTITY_PROVIDER") + .optionValue(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER) .build() ); } @@ -305,33 +349,50 @@ private static List baseServiceClientConfigurationFields() { static class Field { private final String name; private final TypeName type; + private final Class definingClass; private final Class optionClass; private final String optionName; private final String doc; - private final boolean isInherited; private final TypeName baseType; Field(Builder builder) { this.name = Validate.paramNotNull(builder.name, "name"); this.type = Validate.paramNotNull(builder.type, "type"); + this.definingClass = builder.definingClass; this.doc = Validate.paramNotNull(builder.doc, "doc"); this.optionClass = builder.optionClass; this.optionName = builder.optionName; - this.isInherited = builder.isInherited; this.baseType = builder.baseType; } + public boolean isLocalField() { + return definingClass == null; + } + public static Builder builder() { return new Builder(); } + public static Builder builder(String name, TypeName type) { + return new Builder() + .name(name) + .type(type); + } + + public static Builder builder(String name, Class type) { + return new Builder() + .name(name) + .type(type); + } + static class Builder { private String name; private TypeName type; private String doc; + private Class definingClass; private Class optionClass; + private ClientOption value; private String optionName; - private boolean isInherited = true; private TypeName baseType; public Field.Builder name(String name) { @@ -359,24 +420,60 @@ public Field.Builder optionClass(Class optionClass) { return this; } - public Field.Builder optionName(String optionName) { - this.optionName = optionName; + public Field.Builder optionValue(ClientOption value) { + this.value = value; return this; } - public Field.Builder isInherited(boolean isInherited) { - this.isInherited = isInherited; + public Field.Builder baseType(TypeName baseType) { + this.baseType = baseType; return this; } - public Field.Builder baseType(TypeName baseType) { - this.baseType = baseType; + public Field.Builder definingClass(Class definingClass) { + this.definingClass = definingClass; return this; } public Field build() { + if (value != null && optionClass != null) { + optionName = getFieldName(value, optionClass); + } return new Field(this); } } + + // This method resolves an static reference to its name, for instance, when called with + // + // getFieldName(AwsClientOption.AWS_REGION, AwsClientOption.class) + // + // it will return the string "AWS_REGION" that we can use for codegen. + // Using the value directly avoid typo bugs and allows the compiler and the IDE to know about this relationship. + // + // This method uses the fully qualified names in the reflection package to avoid polluting this class imports. + // Adapted from https://stackoverflow.com/a/35416606 + private static String getFieldName(Object fieldObject, Class parent) { + java.lang.reflect.Field[] allFields = parent.getFields(); + for (java.lang.reflect.Field field : allFields) { + int modifiers = field.getModifiers(); + if (!java.lang.reflect.Modifier.isStatic(modifiers)) { + continue; + } + Object currentFieldObject; + try { + // For static fields you can pass a null to get back its value. + currentFieldObject = field.get(null); + } catch (Exception e) { + throw new IllegalArgumentException(e); + } + boolean isWantedField = fieldObject.equals(currentFieldObject); + if (isWantedField) { + return field.getName(); + } + } + throw new java.util.NoSuchElementException(String.format("cannot find constant %s in class %s", + fieldObject, + parent.getClass().getName())); + } } } diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java index 4f4e992b0b13..baa51c424734 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java @@ -8,7 +8,8 @@ import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; -import software.amazon.awssdk.core.internal.SdkInternalAdvancedClientOption; +import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; +import software.amazon.awssdk.core.client.config.internal.SdkInternalAdvancedClientOption; import software.amazon.awssdk.endpoints.EndpointProvider; import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; @@ -23,15 +24,25 @@ @Generated("software.amazon.awssdk:codegen") @SdkPublicApi public final class JsonProtocolTestsServiceClientConfiguration extends AwsServiceClientConfiguration { + private final JsonProtocolTestsAuthSchemeProvider authSchemeProvider; + private JsonProtocolTestsServiceClientConfiguration(Builder builder) { super(builder); + this.authSchemeProvider = builder.authSchemeProvider(); + } + + /** + * Gets the value for auth scheme provider + */ + public JsonProtocolTestsAuthSchemeProvider authSchemeProvider() { + return authSchemeProvider; } public static Builder builder() { return new BuilderImpl(); } - public static Builder builder(SdkClientConfiguration.Builder builder) { + public static BuilderInternal builder(SdkClientConfiguration.Builder builder) { return new BuilderImpl(builder); } @@ -51,18 +62,6 @@ public interface Builder extends AwsServiceClientConfiguration.Builder { @Override ClientOverrideConfiguration overrideConfiguration(); - /** - * Sets the value for AWS region - */ - @Override - Builder region(Region region); - - /** - * Gets the value for AWS region - */ - @Override - Region region(); - /** * Sets the value for endpoint override */ @@ -87,6 +86,18 @@ public interface Builder extends AwsServiceClientConfiguration.Builder { @Override EndpointProvider endpointProvider(); + /** + * Sets the value for AWS region + */ + @Override + Builder region(Region region); + + /** + * Gets the value for AWS region + */ + @Override + Region region(); + /** * Sets the value for credentials provider */ @@ -113,10 +124,15 @@ public interface Builder extends AwsServiceClientConfiguration.Builder { JsonProtocolTestsServiceClientConfiguration build(); } - private static final class BuilderImpl implements Builder { - private final SdkClientConfiguration.Builder builder; + /** + * An internal builder for creating a {@link JsonProtocolTestsServiceClientConfiguration} + */ + public interface BuilderInternal extends Builder { + SdkClientConfiguration buildSdkClientConfiguration(); + } - private ClientOverrideConfiguration overrideConfiguration; + private static final class BuilderImpl implements BuilderInternal { + private final SdkClientConfiguration.Builder builder; private BuilderImpl() { this.builder = SdkClientConfiguration.builder(); @@ -131,7 +147,7 @@ private BuilderImpl(SdkClientConfiguration.Builder builder) { */ @Override public Builder overrideConfiguration(ClientOverrideConfiguration overrideConfiguration) { - this.overrideConfiguration = overrideConfiguration; + builder.option(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION, overrideConfiguration); return this; } @@ -140,24 +156,7 @@ public Builder overrideConfiguration(ClientOverrideConfiguration overrideConfigu */ @Override public ClientOverrideConfiguration overrideConfiguration() { - return overrideConfiguration; - } - - /** - * Sets the value for AWS region - */ - @Override - public Builder region(Region region) { - builder.option(AwsClientOption.AWS_REGION, region); - return this; - } - - /** - * Gets the value for AWS region - */ - @Override - public Region region() { - return builder.option(AwsClientOption.AWS_REGION); + return builder.option(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION); } /** @@ -194,6 +193,23 @@ public EndpointProvider endpointProvider() { return builder.option(SdkClientOption.ENDPOINT_PROVIDER); } + /** + * Sets the value for AWS region + */ + @Override + public Builder region(Region region) { + builder.option(AwsClientOption.AWS_REGION, region); + return this; + } + + /** + * Gets the value for AWS region + */ + @Override + public Region region() { + return builder.option(AwsClientOption.AWS_REGION); + } + /** * Sets the value for credentials provider */ @@ -238,9 +254,11 @@ public JsonProtocolTestsServiceClientConfiguration build() { return new JsonProtocolTestsServiceClientConfiguration(this); } + @Override public SdkClientConfiguration buildSdkClientConfiguration() { + ClientOverrideConfiguration overrideConfiguration = overrideConfiguration(); if (overrideConfiguration != null) { - overrideConfiguration.addOverridesToConfiguration(builder); + SdkClientConfigurationUtil.copyOverridesToConfiguration(overrideConfiguration, builder); } return builder.build(); } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java index a4941dbbc5c2..e353519e359d 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java @@ -62,9 +62,10 @@ import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; +import software.amazon.awssdk.core.client.config.internal.SdkInternalAdvancedClientOption; import software.amazon.awssdk.core.interceptor.ClasspathInterceptorChainFactory; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; -import software.amazon.awssdk.core.internal.SdkInternalAdvancedClientOption; import software.amazon.awssdk.core.internal.http.loader.DefaultSdkAsyncHttpClientBuilder; import software.amazon.awssdk.core.internal.http.loader.DefaultSdkHttpClientBuilder; import software.amazon.awssdk.core.internal.http.pipeline.stages.ApplyUserAgentStage; @@ -215,7 +216,8 @@ private SdkClientConfiguration setOverrides(SdkClientConfiguration configuration if (clientOverrideConfiguration == null) { return configuration; } - return clientOverrideConfiguration.addOverridesToConfiguration(configuration.toBuilder()).build(); + return SdkClientConfigurationUtil.copyOverridesToConfiguration(clientOverrideConfiguration, configuration.toBuilder()) + .build(); } /** diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java index d088124b2929..dc3ce704a4d9 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/ClientOverrideConfiguration.java @@ -32,14 +32,11 @@ import software.amazon.awssdk.core.interceptor.ExecutionAttribute; import software.amazon.awssdk.core.interceptor.ExecutionAttributes; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; -import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; import software.amazon.awssdk.core.retry.RetryMode; import software.amazon.awssdk.core.retry.RetryPolicy; -import software.amazon.awssdk.core.signer.Signer; import software.amazon.awssdk.core.sync.ResponseTransformer; import software.amazon.awssdk.metrics.MetricPublisher; import software.amazon.awssdk.profiles.ProfileFile; -import software.amazon.awssdk.profiles.ProfileFileSupplier; import software.amazon.awssdk.profiles.ProfileFileSystemSetting; import software.amazon.awssdk.utils.AttributeMap; import software.amazon.awssdk.utils.CollectionUtils; @@ -263,49 +260,6 @@ public String toString() { .build(); } - /** - * Adds the overridden values to the client configuration builder. - */ - public SdkClientConfiguration.Builder addOverridesToConfiguration(SdkClientConfiguration.Builder builder) { - // misc - builder.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS, this.headers()); - builder.option(SdkClientOption.RETRY_POLICY, this.retryPolicy().orElse(null)); - builder.option(SdkClientOption.API_CALL_TIMEOUT, this.apiCallTimeout().orElse(null)); - builder.option(SdkClientOption.API_CALL_ATTEMPT_TIMEOUT, this.apiCallAttemptTimeout().orElse(null)); - builder.option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE, this.scheduledExecutorService().orElse(null)); - builder.option(SdkClientOption.EXECUTION_INTERCEPTORS, this.executionInterceptors()); - builder.option(SdkClientOption.EXECUTION_ATTRIBUTES, this.executionAttributes()); - - // advanced option - Signer signer = this.advancedOption(SdkAdvancedClientOption.SIGNER).orElse(null); - builder.option(SdkAdvancedClientOption.SIGNER, signer); - builder.option(SdkClientOption.SIGNER_OVERRIDDEN, signer != null); - builder.option(SdkAdvancedClientOption.USER_AGENT_SUFFIX, - this.advancedOption(SdkAdvancedClientOption.USER_AGENT_SUFFIX).orElse(null)); - builder.option(SdkAdvancedClientOption.USER_AGENT_PREFIX, - this.advancedOption(SdkAdvancedClientOption.USER_AGENT_PREFIX).orElse(null)); - builder.option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION, - this.advancedOption(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION).orElse(null)); - - // profile - builder.option(SdkClientOption.PROFILE_FILE_SUPPLIER, this.defaultProfileFile() - .map(ProfileFileSupplier::fixedProfileFile) - .orElse(null)); - builder.option(SdkClientOption.PROFILE_NAME, this.defaultProfileName().orElse(null)); - - // misc - builder.option(SdkClientOption.METRIC_PUBLISHERS, this.metricPublishers()); - builder.option(SdkAdvancedClientOption.TOKEN_SIGNER, - this.advancedOption(SdkAdvancedClientOption.TOKEN_SIGNER).orElse(null)); - builder.option(SdkClientOption.COMPRESSION_CONFIGURATION, this.compressionConfiguration().orElse(null)); - - this.advancedOption(SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE).ifPresent(value -> { - builder.option(SdkClientOption.ENDPOINT_OVERRIDDEN, value); - }); - - return builder; - } - /** * A builder for {@link ClientOverrideConfiguration}. * diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java new file mode 100644 index 000000000000..22c676fcb67d --- /dev/null +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java @@ -0,0 +1,79 @@ +/* + * 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.client.config.internal; + +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; +import software.amazon.awssdk.core.client.config.SdkClientConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; +import software.amazon.awssdk.core.signer.Signer; +import software.amazon.awssdk.profiles.ProfileFileSupplier; + +@SdkInternalApi +public final class SdkClientConfigurationUtil { + private SdkClientConfigurationUtil() { + } + + /** + * Copies the {@link ClientOverrideConfiguration} values to the configuration builder. + */ + public static SdkClientConfiguration.Builder copyOverridesToConfiguration( + ClientOverrideConfiguration overrides, + SdkClientConfiguration.Builder builder + ) { + // Save the ClientOverrideConfiguration instance + builder.option(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION, overrides); + + // misc + builder.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS, overrides.headers()); + builder.option(SdkClientOption.RETRY_POLICY, overrides.retryPolicy().orElse(null)); + builder.option(SdkClientOption.API_CALL_TIMEOUT, overrides.apiCallTimeout().orElse(null)); + builder.option(SdkClientOption.API_CALL_ATTEMPT_TIMEOUT, overrides.apiCallAttemptTimeout().orElse(null)); + builder.option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE, overrides.scheduledExecutorService().orElse(null)); + builder.option(SdkClientOption.EXECUTION_INTERCEPTORS, overrides.executionInterceptors()); + builder.option(SdkClientOption.EXECUTION_ATTRIBUTES, overrides.executionAttributes()); + + // advanced option + Signer signer = overrides.advancedOption(SdkAdvancedClientOption.SIGNER).orElse(null); + builder.option(SdkAdvancedClientOption.SIGNER, signer); + builder.option(SdkClientOption.SIGNER_OVERRIDDEN, signer != null); + builder.option(SdkAdvancedClientOption.USER_AGENT_SUFFIX, + overrides.advancedOption(SdkAdvancedClientOption.USER_AGENT_SUFFIX).orElse(null)); + builder.option(SdkAdvancedClientOption.USER_AGENT_PREFIX, + overrides.advancedOption(SdkAdvancedClientOption.USER_AGENT_PREFIX).orElse(null)); + builder.option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION, + overrides.advancedOption(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION).orElse(null)); + overrides.advancedOption(SdkInternalTestAdvancedClientOption.ENDPOINT_OVERRIDDEN_OVERRIDE).ifPresent(value -> { + builder.option(SdkClientOption.ENDPOINT_OVERRIDDEN, value); + }); + + // profile + builder.option(SdkClientOption.PROFILE_FILE_SUPPLIER, overrides.defaultProfileFile() + .map(ProfileFileSupplier::fixedProfileFile) + .orElse(null)); + builder.option(SdkClientOption.PROFILE_NAME, overrides.defaultProfileName().orElse(null)); + + // misc + builder.option(SdkClientOption.METRIC_PUBLISHERS, overrides.metricPublishers()); + builder.option(SdkAdvancedClientOption.TOKEN_SIGNER, + overrides.advancedOption(SdkAdvancedClientOption.TOKEN_SIGNER).orElse(null)); + builder.option(SdkClientOption.COMPRESSION_CONFIGURATION, overrides.compressionConfiguration().orElse(null)); + + return builder; + } +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/SdkInternalAdvancedClientOption.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java similarity index 67% rename from core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/SdkInternalAdvancedClientOption.java rename to core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java index 41d9a1f09197..73030818061c 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/SdkInternalAdvancedClientOption.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java @@ -13,11 +13,13 @@ * permissions and limitations under the License. */ -package software.amazon.awssdk.core.internal; +package software.amazon.awssdk.core.client.config.internal; import java.net.URI; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.SdkServiceClientConfiguration; +import software.amazon.awssdk.core.client.config.ClientOption; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientOption; @@ -26,7 +28,7 @@ * class are not guaranteed to be backwards compatible. */ @SdkInternalApi -public class SdkInternalAdvancedClientOption extends SdkAdvancedClientOption { +public class SdkInternalAdvancedClientOption extends ClientOption { /** * The endpoint override is not currently reflected in the client options. We set its value using this internal option to * allow to reconstruct the {@link SdkServiceClientConfiguration} with the same set of values without having to branch on @@ -36,6 +38,15 @@ public class SdkInternalAdvancedClientOption extends SdkAdvancedClientOption< public static final SdkInternalAdvancedClientOption ENDPOINT_OVERRIDE_VALUE = new SdkInternalAdvancedClientOption<>(URI.class); + /** + * The client override configuration is not currently reflected in the client options. We set its value using this internal + * option to allow to reconstruct the {@link SdkServiceClientConfiguration} with the same set of values without having to + * branch on whether the different options are set. + */ + @SdkInternalApi + public static final SdkInternalAdvancedClientOption CLIENT_OVERRIDE_CONFIGURATION = + new SdkInternalAdvancedClientOption<>(ClientOverrideConfiguration.class); + protected SdkInternalAdvancedClientOption(Class valueClass) { super(valueClass); } diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java new file mode 100644 index 000000000000..389957cd39c6 --- /dev/null +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java @@ -0,0 +1,215 @@ +/* + * 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.services; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; +import static org.mockito.Mockito.mock; + +import java.net.URI; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.ScheduledExecutorService; +import java.util.function.BiConsumer; +import java.util.function.Function; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.awssdk.awscore.AwsServiceClientConfiguration; +import software.amazon.awssdk.awscore.client.config.AwsClientOption; +import software.amazon.awssdk.core.client.config.ClientOption; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; +import software.amazon.awssdk.core.client.config.SdkClientConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.signer.Signer; +import software.amazon.awssdk.endpoints.EndpointProvider; +import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; +import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; +import software.amazon.awssdk.identity.spi.IdentityProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonServiceClientConfiguration; +import software.amazon.awssdk.services.protocolrestjson.auth.scheme.ProtocolRestJsonAuthSchemeProvider; + +public class CodegenServiceClientConfigurationTest { + private static final EndpointProvider MOCK_ENDPOINT_PROVIDER = mock(EndpointProvider.class); + private static final IdentityProvider MOCK_IDENTITY_PROVIDER = mock(IdentityProvider.class); + private static final ProtocolRestJsonAuthSchemeProvider MOCK_AUTH_SCHEME_PROVIDER = + mock(ProtocolRestJsonAuthSchemeProvider.class); + private static final ScheduledExecutorService MOCK_SCHEDULED_EXECUTOR_SERVICE = mock(ScheduledExecutorService.class); + private static final Signer MOCK_SIGNER = mock(Signer.class); + + @ParameterizedTest + @MethodSource("testCases") + void fromExternalToInternal(TestCase testCase) { + ProtocolRestJsonServiceClientConfiguration.BuilderInternal builder = + ProtocolRestJsonServiceClientConfiguration.builder(SdkClientConfiguration.builder()); + + // Verify that initially the value is null for properties with direct mapping. + if (testCase.hasDirectMapping) { + assertThat(testCase.getter.apply(builder)).isNull(); + } + + // Set the value + testCase.setter.accept(builder, testCase.value); + + // Assert that we can retrieve the same value + assertThat(testCase.getter.apply(builder)).isEqualTo(testCase.value); + + // Build the ServiceConfiguration instance + ProtocolRestJsonServiceClientConfiguration config = builder.build(); + + // Assert that we can retrieve the same value + assertThat(testCase.dataGetter.apply(config)).isEqualTo(testCase.value); + + // Validate round trip + SdkClientConfiguration clientConfig = builder.buildSdkClientConfiguration(); + + // Build a new builder with the created client config + ProtocolRestJsonServiceClientConfiguration.BuilderInternal anotherBuilder = + ProtocolRestJsonServiceClientConfiguration.builder(clientConfig.toBuilder()); + + // Assert that we can retrieve the same value + assertThat(testCase.getter.apply(anotherBuilder)).isEqualTo(testCase.value); + } + + public static List> testCases() throws Exception { + return Arrays.asList( + TestCase.builder() + .option(AwsClientOption.AWS_REGION) + .value(Region.US_WEST_2) + .setter(ProtocolRestJsonServiceClientConfiguration.Builder::region) + .getter(ProtocolRestJsonServiceClientConfiguration.Builder::region) + .dataGetter(AwsServiceClientConfiguration::region) + .build(), + TestCase.builder() + .option(SdkClientOption.ENDPOINT) + .value(new URI("http://localhost:8080")) + .setter(ProtocolRestJsonServiceClientConfiguration.Builder::endpointOverride) + .getter(ProtocolRestJsonServiceClientConfiguration.Builder::endpointOverride) + .dataGetter(x -> x.endpointOverride().orElse(null)) + .build(), + TestCase.builder() + .option(SdkClientOption.ENDPOINT_PROVIDER) + .value(MOCK_ENDPOINT_PROVIDER) + .setter(ProtocolRestJsonServiceClientConfiguration.Builder::endpointProvider) + .getter(ProtocolRestJsonServiceClientConfiguration.Builder::endpointProvider) + .dataGetter(x -> x.endpointProvider().orElse(null)) + .build(), + TestCase.>builder() + .option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER) + .value(MOCK_IDENTITY_PROVIDER) + .setter(ProtocolRestJsonServiceClientConfiguration.Builder::credentialsProvider) + .getter(ProtocolRestJsonServiceClientConfiguration.Builder::credentialsProvider) + .dataGetter(AwsServiceClientConfiguration::credentialsProvider) + .build(), + TestCase.builder() + .option(SdkClientOption.AUTH_SCHEME_PROVIDER) + .value(MOCK_AUTH_SCHEME_PROVIDER) + .setter((b, p) -> b.authSchemeProvider((ProtocolRestJsonAuthSchemeProvider) p)) + .getter(ProtocolRestJsonServiceClientConfiguration.Builder::authSchemeProvider) + .dataGetter(ProtocolRestJsonServiceClientConfiguration::authSchemeProvider) + .build(), + // Override configuration gets tricky + TestCase.builder() + .option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE) + .value(MOCK_SCHEDULED_EXECUTOR_SERVICE) + .setter((b, p) -> b.overrideConfiguration( + ClientOverrideConfiguration.builder() + .scheduledExecutorService(p) + .build())) + .getter(b -> b.overrideConfiguration().scheduledExecutorService().orElse(null)) + .dataGetter(d -> d.overrideConfiguration().scheduledExecutorService().orElse(null)) + .withoutDirectMapping() + .build(), + TestCase.builder() + .option(SdkAdvancedClientOption.SIGNER) + .value(MOCK_SIGNER) + .setter((b, p) -> b.overrideConfiguration( + ClientOverrideConfiguration.builder() + .putAdvancedOption(SdkAdvancedClientOption.SIGNER, p) + .build())) + .getter(b -> b.overrideConfiguration().advancedOption(SdkAdvancedClientOption.SIGNER).orElse(null)) + .dataGetter(d -> d.overrideConfiguration().advancedOption(SdkAdvancedClientOption.SIGNER).orElse(null)) + .withoutDirectMapping() + .build() + ); + } + + static class TestCase { + private final ClientOption option; + private final T value; + private final BiConsumer setter; + private final Function getter; + private Function dataGetter; + private final boolean hasDirectMapping; + + public TestCase(Builder builder) { + this.option = builder.option; + this.value = builder.value; + this.setter = builder.setter; + this.getter = builder.getter; + this.dataGetter = builder.dataGetter; + this.hasDirectMapping = builder.hasDirectMapping; + } + + public static Builder builder() { + return new Builder(); + } + + + static class Builder { + private ClientOption option; + private T value; + private BiConsumer setter; + private Function getter; + private Function dataGetter; + private boolean hasDirectMapping = true; + + Builder option(ClientOption option) { + this.option = option; + return this; + } + + Builder value(T value) { + this.value = value; + return this; + } + + Builder setter(BiConsumer setter) { + this.setter = setter; + return this; + } + + Builder getter(Function getter) { + this.getter = getter; + return this; + } + + Builder dataGetter(Function dataGetter) { + this.dataGetter = dataGetter; + return this; + } + + Builder withoutDirectMapping() { + this.hasDirectMapping = false; + return this; + } + + TestCase build() { + return new TestCase<>(this); + } + } + } +} From 48d3139896390eef19ff51586e6144c68e181d57 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Sat, 23 Sep 2023 14:47:10 -0700 Subject: [PATCH 2/5] Avoid storing special values and instead read them from the config --- .../tasks/CommonClientGeneratorTasks.java | 8 +- ...erviceClientConfigurationBuilderClass.java | 239 ++++++++++++ .../ServiceClientConfigurationClass.java | 365 ++---------------- .../ServiceClientConfigurationUtils.java | 356 +++++++++++++++++ ...iceClientConfigurationBuilderSpecTest.java | 55 +++ .../serviceclientconfiguration-builder.java | 182 +++++++++ .../model/serviceclientconfiguration.java | 156 +------- .../builder/SdkDefaultClientBuilder.java | 3 - .../internal/SdkClientConfigurationUtil.java | 60 ++- .../SdkInternalAdvancedClientOption.java | 53 --- ...CodegenServiceClientConfigurationTest.java | 9 +- 11 files changed, 936 insertions(+), 550 deletions(-) create mode 100644 codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java create mode 100644 codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java create mode 100644 codegen/src/test/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderSpecTest.java create mode 100644 codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java delete mode 100644 core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/emitters/tasks/CommonClientGeneratorTasks.java b/codegen/src/main/java/software/amazon/awssdk/codegen/emitters/tasks/CommonClientGeneratorTasks.java index ee772e46040b..96036eb1ee08 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/emitters/tasks/CommonClientGeneratorTasks.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/emitters/tasks/CommonClientGeneratorTasks.java @@ -22,6 +22,7 @@ import software.amazon.awssdk.codegen.emitters.GeneratorTaskParams; import software.amazon.awssdk.codegen.poet.builder.BaseClientBuilderClass; import software.amazon.awssdk.codegen.poet.builder.BaseClientBuilderInterface; +import software.amazon.awssdk.codegen.poet.model.ServiceClientConfigurationBuilderClass; import software.amazon.awssdk.codegen.poet.model.ServiceClientConfigurationClass; /** @@ -36,7 +37,8 @@ public CommonClientGeneratorTasks(GeneratorTaskParams dependencies) { protected List createTasks() throws Exception { return Arrays.asList(createBaseBuilderTask(), createBaseBuilderInterfaceTask(), - createServiceClientConfigurationTask()); + createServiceClientConfigurationTask(), + createServiceClientConfigurationBuilderTask()); } private GeneratorTask createBaseBuilderTask() throws IOException { @@ -50,4 +52,8 @@ private GeneratorTask createBaseBuilderInterfaceTask() throws IOException { private GeneratorTask createServiceClientConfigurationTask() throws IOException { return createPoetGeneratorTask(new ServiceClientConfigurationClass(model)); } + + private GeneratorTask createServiceClientConfigurationBuilderTask() throws IOException { + return createPoetGeneratorTask(new ServiceClientConfigurationBuilderClass(model)); + } } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java new file mode 100644 index 000000000000..a87c552a20f5 --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java @@ -0,0 +1,239 @@ +/* + * 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.codegen.poet.model; + +import static javax.lang.model.element.Modifier.ABSTRACT; +import static javax.lang.model.element.Modifier.FINAL; +import static javax.lang.model.element.Modifier.PRIVATE; +import static javax.lang.model.element.Modifier.PUBLIC; +import static javax.lang.model.element.Modifier.STATIC; +import static software.amazon.awssdk.codegen.poet.model.ServiceClientConfigurationUtils.Field; + +import com.squareup.javapoet.ClassName; +import com.squareup.javapoet.CodeBlock; +import com.squareup.javapoet.MethodSpec; +import com.squareup.javapoet.TypeSpec; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.poet.ClassSpec; +import software.amazon.awssdk.codegen.poet.PoetUtils; +import software.amazon.awssdk.core.client.config.SdkClientConfiguration; +import software.amazon.awssdk.utils.Validate; + +public class ServiceClientConfigurationBuilderClass implements ClassSpec { + private final ServiceClientConfigurationUtils utils; + private final ClassName builderInterface; + + public ServiceClientConfigurationBuilderClass(IntermediateModel model) { + this.utils = new ServiceClientConfigurationUtils(model); + this.builderInterface = utils.serviceClientConfigurationClassName().nestedClass("Builder"); + } + + @Override + public ClassName className() { + return utils.serviceClientConfigurationBuilderClassName(); + } + + @Override + public TypeSpec poetSpec() { + TypeSpec.Builder builder = PoetUtils.createClassBuilder(className()) + .addModifiers(PUBLIC) + .addAnnotation(SdkInternalApi.class); + + return builder.addMethod(builderMethod()) + .addMethod(builderFromSdkClientConfiguration()) + .addType(builderInternalSpec()) + .addType(builderImplSpec()) + .build(); + } + + + private MethodSpec builderFromSdkClientConfiguration() { + return MethodSpec.methodBuilder("builder") + .addModifiers(PUBLIC, STATIC) + .addParameter(SdkClientConfiguration.Builder.class, "builder") + .returns(className().nestedClass("BuilderInternal")) + .addStatement("return new BuilderImpl(builder)") + .build(); + } + + + private MethodSpec builderMethod() { + return MethodSpec.methodBuilder("builder") + .addModifiers(PUBLIC, STATIC) + .returns(builderInterface) + .addStatement("return new BuilderImpl()") + .build(); + } + + private TypeSpec builderInternalSpec() { + TypeSpec.Builder builder = TypeSpec.interfaceBuilder("BuilderInternal") + .addModifiers(PUBLIC) + .addSuperinterface(builderInterface); + builder.addMethod(MethodSpec.methodBuilder("buildSdkClientConfiguration") + .addModifiers(PUBLIC, ABSTRACT) + .returns(SdkClientConfiguration.class) + .build()); + return builder.build(); + } + + private TypeSpec builderImplSpec() { + TypeSpec.Builder builder = TypeSpec.classBuilder("BuilderImpl") + .addModifiers(PUBLIC, STATIC) + .addSuperinterface(className().nestedClass("BuilderInternal")); + + builder.addField(SdkClientConfiguration.Builder.class, "builder", PRIVATE, FINAL); + + builder.addMethod(MethodSpec.constructorBuilder() + .addModifiers(PRIVATE) + .addStatement("this.builder = $T.builder()", SdkClientConfiguration.class) + .build()); + builder.addMethod(constructorFromSdkClientConfiguration()); + + for (Field field : utils.serviceClientConfigurationFields()) { + addLocalFieldForBuilderIfNeeded(field, builder); + builder.addMethod(setterForField(field)); + builder.addMethod(getterForBuilderField(field)); + } + + builder.addMethod(MethodSpec.methodBuilder("build") + .addAnnotation(Override.class) + .addModifiers(PUBLIC) + .returns(utils.serviceClientConfigurationClassName()) + .addStatement("return new $T(this)", utils.serviceClientConfigurationClassName()) + .build()); + + builder.addMethod(buildSdkClientConfigurationMethod()); + + return builder.build(); + } + + private MethodSpec buildSdkClientConfigurationMethod() { + MethodSpec.Builder builder = MethodSpec.methodBuilder("buildSdkClientConfiguration") + .addModifiers(PUBLIC) + .addAnnotation(Override.class) + .returns(SdkClientConfiguration.class); + + for (Field field : utils.serviceClientConfigurationFields()) { + if (field.optionClass() == null) { + CodeBlock block = field.copyToConfiguration(); + if (block != null) { + builder.addCode(block); + } + } + } + return builder + .addStatement("return builder.build()") + .build(); + } + + private MethodSpec constructorFromSdkClientConfiguration() { + MethodSpec.Builder builder = MethodSpec.constructorBuilder() + .addModifiers(PRIVATE) + .addParameter(SdkClientConfiguration.Builder.class, "builder") + .addStatement("this.builder = builder"); + + for (Field field : utils.serviceClientConfigurationFields()) { + if (field.optionClass() == null) { + CodeBlock block = field.constructFromConfiguration(); + if (block != null) { + builder.addCode(block); + } + } + } + return builder.build(); + } + + + private void addLocalFieldForBuilderIfNeeded(Field field, TypeSpec.Builder builder) { + if (field.optionClass() == null) { + builder.addField(field.type(), field.name(), PRIVATE); + } + } + + private MethodSpec setterForField(Field field) { + MethodSpec.Builder builder = baseSetterForField(field); + if (field.isLocalField()) { + builder.addAnnotation(Override.class); + } + if (field.optionClass() == null) { + return builder.addStatement("this.$1L = $1L", field.name()) + .addStatement("return this") + .build(); + + } + return builder.addStatement("builder.option($T.$L, $L)", + field.optionClass(), field.optionName(), field.name()) + .addStatement("return this") + .build(); + } + + private MethodSpec.Builder baseSetterForField(Field field) { + MethodSpec.Builder builder = MethodSpec.methodBuilder(field.name()) + .addModifiers(PUBLIC) + .addParameter(field.type(), field.name()) + .addJavadoc("Sets the value for " + field.doc()) + .returns(builderInterface); + if (!field.isLocalField()) { + builder.addAnnotation(Override.class); + } + return builder; + } + + private MethodSpec getterForBuilderField(Field field) { + return getterForField(field, "builder", false); + } + + + private MethodSpec getterForField(Field field, String fieldName, boolean forDataGetter) { + MethodSpec.Builder builder = baseGetterForField(field); + if (!forDataGetter && field.isLocalField()) { + builder.addAnnotation(Override.class); + } + if (forDataGetter && field.isLocalField()) { + return builder.addStatement("return $L", field.name()) + .build(); + } + if (field.optionClass() == null) { + return builder.addStatement("return $L", field.name()) + .build(); + } + if (field.baseType() != null) { + return builder.addStatement("$T result = $L.option($T.$L)", + field.baseType(), fieldName, field.optionClass(), field.optionName()) + .beginControlFlow("if (result == null)") + .addStatement("return null") + .endControlFlow() + .addStatement("return $T.isInstanceOf($T.class, result, $S + $T.class.getSimpleName())", + Validate.class, field.type(), + "Expected an instance of ", field.type()) + .build(); + } + return builder.addStatement("return $L.option($T.$L)", fieldName, field.optionClass(), field.optionName()) + .build(); + } + + private MethodSpec.Builder baseGetterForField(Field field) { + MethodSpec.Builder builder = MethodSpec.methodBuilder(field.name()) + .addModifiers(PUBLIC) + .addJavadoc("Gets the value for " + field.doc()) + .returns(field.type()); + if (!field.isLocalField()) { + builder.addAnnotation(Override.class); + } + return builder; + } +} diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java index 142890c890e8..962f0b552ac0 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationClass.java @@ -20,59 +20,46 @@ import static javax.lang.model.element.Modifier.PRIVATE; import static javax.lang.model.element.Modifier.PUBLIC; import static javax.lang.model.element.Modifier.STATIC; +import static software.amazon.awssdk.codegen.poet.model.ServiceClientConfigurationUtils.Field; import com.squareup.javapoet.ClassName; import com.squareup.javapoet.MethodSpec; -import com.squareup.javapoet.ParameterizedTypeName; -import com.squareup.javapoet.TypeName; import com.squareup.javapoet.TypeSpec; -import com.squareup.javapoet.WildcardTypeName; -import java.net.URI; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.awscore.AwsServiceClientConfiguration; -import software.amazon.awssdk.awscore.client.config.AwsClientOption; import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; import software.amazon.awssdk.codegen.poet.ClassSpec; import software.amazon.awssdk.codegen.poet.PoetUtils; -import software.amazon.awssdk.codegen.poet.auth.scheme.AuthSchemeSpecUtils; -import software.amazon.awssdk.core.SdkServiceClientConfiguration; -import software.amazon.awssdk.core.client.config.ClientOption; -import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; -import software.amazon.awssdk.core.client.config.SdkClientConfiguration; -import software.amazon.awssdk.core.client.config.SdkClientOption; -import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; -import software.amazon.awssdk.core.client.config.internal.SdkInternalAdvancedClientOption; -import software.amazon.awssdk.endpoints.EndpointProvider; -import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; -import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; -import software.amazon.awssdk.identity.spi.IdentityProvider; -import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.utils.Validate; public class ServiceClientConfigurationClass implements ClassSpec { private final ClassName defaultClientMetadataClassName; - private final AuthSchemeSpecUtils authSchemeSpecUtils; + private final ServiceClientConfigurationUtils utils; public ServiceClientConfigurationClass(IntermediateModel model) { String basePackage = model.getMetadata().getFullClientPackageName(); String serviceId = model.getMetadata().getServiceName(); this.defaultClientMetadataClassName = ClassName.get(basePackage, serviceId + "ServiceClientConfiguration"); - this.authSchemeSpecUtils = new AuthSchemeSpecUtils(model); + this.utils = new ServiceClientConfigurationUtils(model); + } + + @Override + public ClassName className() { + return utils.serviceClientConfigurationClassName(); } @Override public TypeSpec poetSpec() { TypeSpec.Builder builder = PoetUtils.createClassBuilder(defaultClientMetadataClassName) + .addModifiers(PUBLIC, FINAL) + .addAnnotation(SdkPublicApi.class) .superclass(AwsServiceClientConfiguration.class) .addJavadoc("Class to expose the service client settings to the user. " + "Implementation of {@link $T}", AwsServiceClientConfiguration.class); builder.addMethod(constructor()); - for (Field field : serviceClientConfigurationFields()) { + for (Field field : utils.serviceClientConfigurationFields()) { addLocalFieldForDataIfNeeded(field, builder); if (field.isLocalField()) { builder.addMethod(getterForDataField(field)); @@ -80,28 +67,18 @@ public TypeSpec poetSpec() { } return builder.addMethod(builderMethod()) - .addMethod(builderFromSdkClientConfiguration()) - .addModifiers(PUBLIC, FINAL) - .addAnnotation(SdkPublicApi.class) .addType(builderInterfaceSpec()) - .addType(builderInternalInterfaceSpec()) - .addType(builderImplSpec()) .build(); } - @Override - public ClassName className() { - return defaultClientMetadataClassName; - } - private MethodSpec constructor() { MethodSpec.Builder builder = MethodSpec.constructorBuilder() - .addModifiers(PRIVATE) + .addModifiers(PUBLIC) .addParameter(className().nestedClass("Builder"), "builder"); builder.addStatement("super(builder)"); - for (Field field : serviceClientConfigurationFields()) { + for (Field field : utils.serviceClientConfigurationFields()) { if (field.isLocalField()) { - builder.addStatement("this.$L = builder.$L()", field.name, field.name); + builder.addStatement("this.$L = builder.$L()", field.name(), field.name()); } } return builder.build(); @@ -110,18 +87,9 @@ private MethodSpec constructor() { private MethodSpec builderMethod() { return MethodSpec.methodBuilder("builder") .addModifiers(PUBLIC, STATIC) - .addStatement("return new BuilderImpl()") + .addStatement("return $T.builder()", + utils.serviceClientConfigurationBuilderClassName()) .returns(className().nestedClass("Builder")) - .addJavadoc("") - .build(); - } - - private MethodSpec builderFromSdkClientConfiguration() { - return MethodSpec.methodBuilder("builder") - .addModifiers(PUBLIC, STATIC) - .addParameter(SdkClientConfiguration.Builder.class, "builder") - .returns(className().nestedClass("BuilderInternal")) - .addStatement("return new BuilderImpl(builder)") .build(); } @@ -131,7 +99,7 @@ private TypeSpec builderInterfaceSpec() { .addSuperinterface(ClassName.get(AwsServiceClientConfiguration.class).nestedClass( "Builder")) .addJavadoc("A builder for creating a {@link $T}", className()); - for (Field field : serviceClientConfigurationFields()) { + for (Field field : utils.serviceClientConfigurationFields()) { builder.addMethod(baseSetterForField(field) .addModifiers(ABSTRACT) .build()); @@ -147,98 +115,17 @@ private TypeSpec builderInterfaceSpec() { return builder.build(); } - private TypeSpec builderInternalInterfaceSpec() { - TypeSpec.Builder builder = TypeSpec.interfaceBuilder("BuilderInternal") - .addModifiers(PUBLIC) - .addSuperinterface(className().nestedClass("Builder")) - .addJavadoc("An internal builder for creating a {@link $T}", className()); - - builder.addMethod(MethodSpec.methodBuilder("buildSdkClientConfiguration") - .addModifiers(PUBLIC, ABSTRACT) - .returns(SdkClientConfiguration.class) - .build()); - return builder.build(); - } - - private TypeSpec builderImplSpec() { - TypeSpec.Builder builder = TypeSpec.classBuilder("BuilderImpl") - .addModifiers(PRIVATE, STATIC, FINAL) - .addSuperinterface(className().nestedClass("BuilderInternal")); - - builder.addField(SdkClientConfiguration.Builder.class, "builder", PRIVATE, FINAL); - - builder.addMethod(MethodSpec.constructorBuilder() - .addModifiers(PRIVATE) - .addStatement("this.builder = $T.builder()", SdkClientConfiguration.class) - .build()); - - builder.addMethod(MethodSpec.constructorBuilder() - .addModifiers(PRIVATE) - .addParameter(SdkClientConfiguration.Builder.class, "builder") - .addStatement("this.builder = builder", SdkClientConfiguration.class) - .build()); - for (Field field : serviceClientConfigurationFields()) { - addLocalFieldForBuilderIfNeeded(field, builder); - builder.addMethod(setterForField(field)); - builder.addMethod(getterForBuilderField(field)); - } - - builder.addMethod(MethodSpec.methodBuilder("build") - .addAnnotation(Override.class) - .addModifiers(PUBLIC) - .returns(className()) - .addStatement("return new $T(this)", className()) - .build()); - - builder.addMethod(MethodSpec.methodBuilder("buildSdkClientConfiguration") - .addModifiers(PUBLIC) - .addAnnotation(Override.class) - .returns(SdkClientConfiguration.class) - .addStatement("$T overrideConfiguration = overrideConfiguration()", - ClientOverrideConfiguration.class) - .beginControlFlow("if (overrideConfiguration != null)") - .addStatement("$T.copyOverridesToConfiguration(overrideConfiguration, builder)", - SdkClientConfigurationUtil.class) - .endControlFlow() - .addStatement("return builder.build()") - .build()); - return builder.build(); - } - - private void addLocalFieldForBuilderIfNeeded(Field field, TypeSpec.Builder builder) { - if (field.optionClass == null) { - builder.addField(field.type, field.name, PRIVATE); - } - } - private void addLocalFieldForDataIfNeeded(Field field, TypeSpec.Builder builder) { if (field.isLocalField()) { - builder.addField(field.type, field.name, PRIVATE, FINAL); - } - } - - private MethodSpec setterForField(Field field) { - MethodSpec.Builder builder = baseSetterForField(field); - if (field.isLocalField()) { - builder.addAnnotation(Override.class); + builder.addField(field.type(), field.name(), PRIVATE, FINAL); } - if (field.optionClass == null) { - return builder.addStatement("this.$1L = $1L", field.name) - .addStatement("return this") - .build(); - - } - return builder.addStatement("builder.option($T.$L, $L)", - field.optionClass, field.optionName, field.name) - .addStatement("return this") - .build(); } private MethodSpec.Builder baseSetterForField(Field field) { - MethodSpec.Builder builder = MethodSpec.methodBuilder(field.name) + MethodSpec.Builder builder = MethodSpec.methodBuilder(field.name()) .addModifiers(PUBLIC) - .addParameter(field.type, field.name) - .addJavadoc("Sets the value for " + field.doc) + .addParameter(field.type(), field.name()) + .addJavadoc("Sets the value for " + field.doc()) .returns(className().nestedClass("Builder")); if (!field.isLocalField()) { builder.addAnnotation(Override.class); @@ -246,10 +133,6 @@ private MethodSpec.Builder baseSetterForField(Field field) { return builder; } - private MethodSpec getterForBuilderField(Field field) { - return getterForField(field, "builder", false); - } - private MethodSpec getterForDataField(Field field) { return getterForField(field, "config", true); } @@ -260,220 +143,36 @@ private MethodSpec getterForField(Field field, String fieldName, boolean forData builder.addAnnotation(Override.class); } if (forDataGetter && field.isLocalField()) { - return builder.addStatement("return $L", field.name) + return builder.addStatement("return $L", field.name()) .build(); } - if (field.optionClass == null) { - return builder.addStatement("return $L", field.name) + if (field.optionClass() == null) { + return builder.addStatement("return $L", field.name()) .build(); } - if (field.baseType != null) { + if (field.baseType() != null) { return builder.addStatement("$T result = $L.option($T.$L)", - field.baseType, fieldName, field.optionClass, field.optionName) + field.baseType(), fieldName, field.optionClass(), field.optionName()) .beginControlFlow("if (result == null)") .addStatement("return null") .endControlFlow() .addStatement("return $T.isInstanceOf($T.class, result, $S + $T.class.getSimpleName())", - Validate.class, field.type, - "Expected an instance of ", field.type) + Validate.class, field.type(), + "Expected an instance of ", field.type()) .build(); } - return builder.addStatement("return $L.option($T.$L)", fieldName, field.optionClass, field.optionName) + return builder.addStatement("return $L.option($T.$L)", fieldName, field.optionClass(), field.optionName()) .build(); } private MethodSpec.Builder baseGetterForField(Field field) { - MethodSpec.Builder builder = MethodSpec.methodBuilder(field.name) + MethodSpec.Builder builder = MethodSpec.methodBuilder(field.name()) .addModifiers(PUBLIC) - .addJavadoc("Gets the value for " + field.doc) - .returns(field.type); + .addJavadoc("Gets the value for " + field.doc()) + .returns(field.type()); if (!field.isLocalField()) { builder.addAnnotation(Override.class); } return builder; } - - - /** - * Returns the list of fields present in the service client configuration class with its corresponding {@link ClientOption} - * mapping to the {@link SdkClientConfiguration} class. - */ - private List serviceClientConfigurationFields() { - List fields = new ArrayList<>(baseServiceClientConfigurationFields()); - fields.add(Field.builder("authSchemeProvider", authSchemeSpecUtils.providerInterfaceName()) - .doc("auth scheme provider") - .optionClass(SdkClientOption.class) - .optionValue(SdkClientOption.AUTH_SCHEME_PROVIDER) - .baseType(ClassName.get(AuthSchemeProvider.class)) - .build()); - return fields; - } - - private static List baseServiceClientConfigurationFields() { - return Arrays.asList( - Field.builder("overrideConfiguration", ClientOverrideConfiguration.class) - .doc("client override configuration") - .definingClass(SdkServiceClientConfiguration.class) - .optionClass(SdkInternalAdvancedClientOption.class) - .optionValue(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION) - .build(), - Field.builder("endpointOverride", URI.class) - .doc("endpoint override") - .definingClass(SdkServiceClientConfiguration.class) - .optionClass(SdkInternalAdvancedClientOption.class) - .optionValue(SdkInternalAdvancedClientOption.ENDPOINT_OVERRIDE_VALUE) - .build(), - Field.builder("endpointProvider", EndpointProvider.class) - .doc("endpoint provider") - .definingClass(SdkServiceClientConfiguration.class) - .optionClass(SdkClientOption.class) - .optionValue(SdkClientOption.ENDPOINT_PROVIDER) - .build(), - Field.builder("region", Region.class) - .doc("AWS region") - .definingClass(AwsServiceClientConfiguration.class) - .optionClass(AwsClientOption.class) - .optionValue(AwsClientOption.AWS_REGION) - .build(), - Field.builder("credentialsProvider", - ParameterizedTypeName.get(ClassName.get(IdentityProvider.class), - WildcardTypeName.subtypeOf(AwsCredentialsIdentity.class))) - .doc("credentials provider") - .definingClass(AwsServiceClientConfiguration.class) - .optionClass(AwsClientOption.class) - .optionValue(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER) - .build() - ); - } - - static class Field { - private final String name; - private final TypeName type; - private final Class definingClass; - private final Class optionClass; - private final String optionName; - private final String doc; - private final TypeName baseType; - - Field(Builder builder) { - this.name = Validate.paramNotNull(builder.name, "name"); - this.type = Validate.paramNotNull(builder.type, "type"); - this.definingClass = builder.definingClass; - this.doc = Validate.paramNotNull(builder.doc, "doc"); - this.optionClass = builder.optionClass; - this.optionName = builder.optionName; - this.baseType = builder.baseType; - } - - public boolean isLocalField() { - return definingClass == null; - } - - public static Builder builder() { - return new Builder(); - } - - public static Builder builder(String name, TypeName type) { - return new Builder() - .name(name) - .type(type); - } - - public static Builder builder(String name, Class type) { - return new Builder() - .name(name) - .type(type); - } - - static class Builder { - private String name; - private TypeName type; - private String doc; - private Class definingClass; - private Class optionClass; - private ClientOption value; - private String optionName; - private TypeName baseType; - - public Field.Builder name(String name) { - this.name = name; - return this; - } - - public Field.Builder type(Class type) { - this.type = ClassName.get(type); - return this; - } - - public Field.Builder type(TypeName type) { - this.type = type; - return this; - } - - public Field.Builder doc(String doc) { - this.doc = doc; - return this; - } - - public Field.Builder optionClass(Class optionClass) { - this.optionClass = optionClass; - return this; - } - - public Field.Builder optionValue(ClientOption value) { - this.value = value; - return this; - } - - public Field.Builder baseType(TypeName baseType) { - this.baseType = baseType; - return this; - } - - public Field.Builder definingClass(Class definingClass) { - this.definingClass = definingClass; - return this; - } - - public Field build() { - if (value != null && optionClass != null) { - optionName = getFieldName(value, optionClass); - } - return new Field(this); - } - } - - // This method resolves an static reference to its name, for instance, when called with - // - // getFieldName(AwsClientOption.AWS_REGION, AwsClientOption.class) - // - // it will return the string "AWS_REGION" that we can use for codegen. - // Using the value directly avoid typo bugs and allows the compiler and the IDE to know about this relationship. - // - // This method uses the fully qualified names in the reflection package to avoid polluting this class imports. - // Adapted from https://stackoverflow.com/a/35416606 - private static String getFieldName(Object fieldObject, Class parent) { - java.lang.reflect.Field[] allFields = parent.getFields(); - for (java.lang.reflect.Field field : allFields) { - int modifiers = field.getModifiers(); - if (!java.lang.reflect.Modifier.isStatic(modifiers)) { - continue; - } - Object currentFieldObject; - try { - // For static fields you can pass a null to get back its value. - currentFieldObject = field.get(null); - } catch (Exception e) { - throw new IllegalArgumentException(e); - } - boolean isWantedField = fieldObject.equals(currentFieldObject); - if (isWantedField) { - return field.getName(); - } - } - throw new java.util.NoSuchElementException(String.format("cannot find constant %s in class %s", - fieldObject, - parent.getClass().getName())); - } - } } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java new file mode 100644 index 000000000000..1a65625e29fb --- /dev/null +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java @@ -0,0 +1,356 @@ +/* + * 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.codegen.poet.model; + +import com.squareup.javapoet.ClassName; +import com.squareup.javapoet.CodeBlock; +import com.squareup.javapoet.ParameterizedTypeName; +import com.squareup.javapoet.TypeName; +import com.squareup.javapoet.WildcardTypeName; +import java.net.URI; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import software.amazon.awssdk.awscore.AwsServiceClientConfiguration; +import software.amazon.awssdk.awscore.client.config.AwsClientOption; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.poet.auth.scheme.AuthSchemeSpecUtils; +import software.amazon.awssdk.core.SdkServiceClientConfiguration; +import software.amazon.awssdk.core.client.config.ClientOption; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; +import software.amazon.awssdk.endpoints.EndpointProvider; +import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; +import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; +import software.amazon.awssdk.identity.spi.IdentityProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.utils.Validate; + +public class ServiceClientConfigurationUtils { + private static final List BASE_FIELDS = baseServiceClientConfigurationFields(); + private final AuthSchemeSpecUtils authSchemeSpecUtils; + private final ClassName configurationClassName; + private final ClassName configurationBuilderClassName; + + + public ServiceClientConfigurationUtils(IntermediateModel model) { + String basePackage = model.getMetadata().getFullClientPackageName(); + String serviceId = model.getMetadata().getServiceName(); + configurationClassName = ClassName.get(basePackage, serviceId + "ServiceClientConfiguration"); + configurationBuilderClassName = ClassName.get(model.getMetadata().getFullClientInternalPackageName(), + serviceId + "ServiceClientConfigurationBuilder"); + this.authSchemeSpecUtils = new AuthSchemeSpecUtils(model); + } + + /** + * Returns the {@link ClassName} of the service client configuration class. + */ + public ClassName serviceClientConfigurationClassName() { + return configurationClassName; + } + + /** + * Returns the {@link ClassName} of the builder for the service client configuration class. + */ + public ClassName serviceClientConfigurationBuilderClassName() { + return configurationBuilderClassName; + } + + /** + * Returns the list of fields present in the service client configuration class with its corresponding {@link ClientOption} + * mapping to the {@link SdkClientConfiguration} class. + */ + public List serviceClientConfigurationFields() { + List fields = new ArrayList<>(BASE_FIELDS); + fields.add(Field.builder("authSchemeProvider", authSchemeSpecUtils.providerInterfaceName()) + .doc("auth scheme provider") + .optionClass(SdkClientOption.class) + .optionValue(SdkClientOption.AUTH_SCHEME_PROVIDER) + .baseType(ClassName.get(AuthSchemeProvider.class)) + .build()); + return fields; + } + + private static List baseServiceClientConfigurationFields() { + return Arrays.asList( + overrideConfigurationField(), + endpointOverrideField(), + Field.builder("endpointProvider", EndpointProvider.class) + .doc("endpoint provider") + .definingClass(SdkServiceClientConfiguration.class) + .optionClass(SdkClientOption.class) + .optionValue(SdkClientOption.ENDPOINT_PROVIDER) + .build(), + Field.builder("region", Region.class) + .doc("AWS region") + .definingClass(AwsServiceClientConfiguration.class) + .optionClass(AwsClientOption.class) + .optionValue(AwsClientOption.AWS_REGION) + .build(), + Field.builder("credentialsProvider", + ParameterizedTypeName.get(ClassName.get(IdentityProvider.class), + WildcardTypeName.subtypeOf(AwsCredentialsIdentity.class))) + .doc("credentials provider") + .definingClass(AwsServiceClientConfiguration.class) + .optionClass(AwsClientOption.class) + .optionValue(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER) + .build() + ); + } + + private static Field endpointOverrideField() { + Field.Builder builder = Field.builder("endpointOverride", URI.class) + .doc("endpoint override") + .definingClass(SdkServiceClientConfiguration.class); + builder.constructFromConfiguration( + CodeBlock.builder() + .beginControlFlow("if (!Boolean.TRUE.equals(builder.option($T.$L)))", + SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT_OVERRIDDEN, + SdkClientOption.class)) + .addStatement("this.endpointOverride = builder.option($T.$L)", + SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT, + SdkClientOption.class)) + .endControlFlow() + .build() + ); + + builder.copyToConfiguration( + CodeBlock.builder() + .beginControlFlow("if (endpointOverride != null)") + .addStatement("builder.option($T.$L, endpointOverride)", + SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT, SdkClientOption.class)) + .addStatement("builder.option($T.$L, true)", + SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT_OVERRIDDEN, SdkClientOption.class)) + .endControlFlow() + .build() + ); + + return builder.build(); + } + + private static Field overrideConfigurationField() { + Field.Builder builder = Field.builder("overrideConfiguration", ClientOverrideConfiguration.class) + .doc("client override configuration") + .definingClass(SdkServiceClientConfiguration.class); + + builder.constructFromConfiguration( + CodeBlock.builder() + .addStatement("this.overrideConfiguration = $T.copyConfigurationToOverrides(" + + "$T.builder(), builder).build()", SdkClientConfigurationUtil.class, + ClientOverrideConfiguration.class) + .build() + ); + + builder.copyToConfiguration( + CodeBlock.builder() + .beginControlFlow("if (overrideConfiguration != null)") + .addStatement("$T.copyOverridesToConfiguration(overrideConfiguration, builder)", + SdkClientConfigurationUtil.class) + .endControlFlow() + .build() + ); + + return builder.build(); + } + + static class Field { + private final String name; + private final TypeName type; + private final Class definingClass; + private final Class optionClass; + private final String optionName; + private final String doc; + private final TypeName baseType; + private final CodeBlock constructFromConfiguration; + private final CodeBlock copyToConfiguration; + + Field(Field.Builder builder) { + this.name = Validate.paramNotNull(builder.name, "name"); + this.type = Validate.paramNotNull(builder.type, "type"); + this.definingClass = builder.definingClass; + this.doc = Validate.paramNotNull(builder.doc, "doc"); + this.optionClass = builder.optionClass; + this.optionName = builder.optionName; + this.baseType = builder.baseType; + this.constructFromConfiguration = builder.constructFromConfiguration; + this.copyToConfiguration = builder.copyToConfiguration; + } + + public boolean isLocalField() { + return definingClass == null; + } + + public String name() { + return name; + } + + public TypeName type() { + return type; + } + + public Class definingClass() { + return definingClass; + } + + public Class optionClass() { + return optionClass; + } + + public String optionName() { + return optionName; + } + + public String doc() { + return doc; + } + + public TypeName baseType() { + return baseType; + } + + public CodeBlock constructFromConfiguration() { + return constructFromConfiguration; + } + + public CodeBlock copyToConfiguration() { + return copyToConfiguration; + } + + public static Field.Builder builder() { + return new Field.Builder(); + } + + public static Field.Builder builder(String name, TypeName type) { + return new Field.Builder() + .name(name) + .type(type); + } + + public static Field.Builder builder(String name, Class type) { + return new Field.Builder() + .name(name) + .type(type); + } + + static class Builder { + private String name; + private TypeName type; + private String doc; + private Class definingClass; + private Class optionClass; + private ClientOption value; + private String optionName; + private TypeName baseType; + private CodeBlock constructFromConfiguration; + private CodeBlock copyToConfiguration; + + + public Field.Builder name(String name) { + this.name = name; + return this; + } + + public Field.Builder type(Class type) { + this.type = ClassName.get(type); + return this; + } + + public Field.Builder type(TypeName type) { + this.type = type; + return this; + } + + public Field.Builder doc(String doc) { + this.doc = doc; + return this; + } + + public Field.Builder optionClass(Class optionClass) { + this.optionClass = optionClass; + return this; + } + + public Field.Builder optionValue(ClientOption value) { + this.value = value; + return this; + } + + public Field.Builder baseType(TypeName baseType) { + this.baseType = baseType; + return this; + } + + public Field.Builder definingClass(Class definingClass) { + this.definingClass = definingClass; + return this; + } + + public Field.Builder constructFromConfiguration(CodeBlock constructFromConfiguration) { + this.constructFromConfiguration = constructFromConfiguration; + return this; + } + + public Field.Builder copyToConfiguration(CodeBlock copyToConfiguration) { + this.copyToConfiguration = copyToConfiguration; + return this; + } + + public Field build() { + if (value != null && optionClass != null) { + optionName = fieldName(value, optionClass); + } + return new Field(this); + } + } + } + + /** + * This method resolves an static reference to its name, for instance, when called with + * 
+     * fieldName(AwsClientOption.AWS_REGION, AwsClientOption.class)
+     *
+ * it will return the string "AWS_REGION" that we can use for codegen. + * Using the value directly avoid typo bugs and allows the compiler and the IDE to know about this relationship. + *

+ * This method uses the fully qualified names in the reflection package to avoid polluting this class imports. + * Adapted from https://stackoverflow.com/a/35416606 + */ + private static String fieldName(Object fieldObject, Class parent) { + java.lang.reflect.Field[] allFields = parent.getFields(); + for (java.lang.reflect.Field field : allFields) { + int modifiers = field.getModifiers(); + if (!java.lang.reflect.Modifier.isStatic(modifiers)) { + continue; + } + Object currentFieldObject; + try { + // For static fields you can pass a null to get back its value. + currentFieldObject = field.get(null); + } catch (Exception e) { + throw new IllegalArgumentException(e); + } + boolean isWantedField = fieldObject.equals(currentFieldObject); + if (isWantedField) { + return field.getName(); + } + } + throw new java.util.NoSuchElementException(String.format("cannot find constant %s in class %s", + fieldObject, + parent.getClass().getName())); + } +} diff --git a/codegen/src/test/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderSpecTest.java b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderSpecTest.java new file mode 100644 index 000000000000..ea30f4f02e75 --- /dev/null +++ b/codegen/src/test/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderSpecTest.java @@ -0,0 +1,55 @@ +/* + * 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.codegen.poet.model; + +import static software.amazon.awssdk.codegen.poet.PoetMatchers.generatesTo; + +import java.io.File; +import java.io.IOException; +import org.hamcrest.MatcherAssert; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import software.amazon.awssdk.codegen.C2jModels; +import software.amazon.awssdk.codegen.IntermediateModelBuilder; +import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig; +import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel; +import software.amazon.awssdk.codegen.model.service.ServiceModel; +import software.amazon.awssdk.codegen.utils.ModelLoaderUtils; + +public class ServiceClientConfigurationBuilderSpecTest { + private static IntermediateModel intermediateModel; + + @BeforeAll + public static void setUp() throws IOException { + File serviceModelFile = new File(AwsModelSpecTest.class.getResource("service-2.json").getFile()); + File customizationConfigFile = new File(AwsModelSpecTest.class + .getResource("customization.config") + .getFile()); + + intermediateModel = new IntermediateModelBuilder( + C2jModels.builder() + .serviceModel(ModelLoaderUtils.loadModel(ServiceModel.class, serviceModelFile)) + .customizationConfig(ModelLoaderUtils.loadModel(CustomizationConfig.class, customizationConfigFile)) + .build()) + .build(); + } + + @Test + public void testGeneration() { + ServiceClientConfigurationBuilderClass spec = new ServiceClientConfigurationBuilderClass(intermediateModel); + MatcherAssert.assertThat(spec, generatesTo("serviceclientconfiguration-builder.java")); + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java new file mode 100644 index 000000000000..a672cd1e874b --- /dev/null +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java @@ -0,0 +1,182 @@ +package software.amazon.awssdk.services.jsonprotocoltests.internal; + +import java.net.URI; +import software.amazon.awssdk.annotations.Generated; +import software.amazon.awssdk.annotations.SdkInternalApi; +import software.amazon.awssdk.awscore.client.config.AwsClientOption; +import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientConfiguration; +import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; +import software.amazon.awssdk.endpoints.EndpointProvider; +import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; +import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; +import software.amazon.awssdk.identity.spi.IdentityProvider; +import software.amazon.awssdk.regions.Region; +import software.amazon.awssdk.services.jsonprotocoltests.JsonProtocolTestsServiceClientConfiguration; +import software.amazon.awssdk.services.jsonprotocoltests.auth.scheme.JsonProtocolTestsAuthSchemeProvider; +import software.amazon.awssdk.utils.Validate; + +@Generated("software.amazon.awssdk:codegen") +@SdkInternalApi +public class JsonProtocolTestsServiceClientConfigurationBuilder { + public static JsonProtocolTestsServiceClientConfiguration.Builder builder() { + return new BuilderImpl(); + } + + public static BuilderInternal builder(SdkClientConfiguration.Builder builder) { + return new BuilderImpl(builder); + } + + public interface BuilderInternal extends JsonProtocolTestsServiceClientConfiguration.Builder { + SdkClientConfiguration buildSdkClientConfiguration(); + } + + public static class BuilderImpl implements BuilderInternal { + private final SdkClientConfiguration.Builder builder; + + private ClientOverrideConfiguration overrideConfiguration; + + private URI endpointOverride; + + private BuilderImpl() { + this.builder = SdkClientConfiguration.builder(); + } + + private BuilderImpl(SdkClientConfiguration.Builder builder) { + this.builder = builder; + this.overrideConfiguration = SdkClientConfigurationUtil.copyConfigurationToOverrides( + ClientOverrideConfiguration.builder(), builder).build(); + if (!Boolean.TRUE.equals(builder.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) { + this.endpointOverride = builder.option(SdkClientOption.ENDPOINT); + } + } + + /** + * Sets the value for client override configuration + */ + @Override + public JsonProtocolTestsServiceClientConfiguration.Builder overrideConfiguration( + ClientOverrideConfiguration overrideConfiguration) { + this.overrideConfiguration = overrideConfiguration; + return this; + } + + /** + * Gets the value for client override configuration + */ + @Override + public ClientOverrideConfiguration overrideConfiguration() { + return overrideConfiguration; + } + + /** + * Sets the value for endpoint override + */ + @Override + public JsonProtocolTestsServiceClientConfiguration.Builder endpointOverride(URI endpointOverride) { + this.endpointOverride = endpointOverride; + return this; + } + + /** + * Gets the value for endpoint override + */ + @Override + public URI endpointOverride() { + return endpointOverride; + } + + /** + * Sets the value for endpoint provider + */ + @Override + public JsonProtocolTestsServiceClientConfiguration.Builder endpointProvider(EndpointProvider endpointProvider) { + builder.option(SdkClientOption.ENDPOINT_PROVIDER, endpointProvider); + return this; + } + + /** + * Gets the value for endpoint provider + */ + @Override + public EndpointProvider endpointProvider() { + return builder.option(SdkClientOption.ENDPOINT_PROVIDER); + } + + /** + * Sets the value for AWS region + */ + @Override + public JsonProtocolTestsServiceClientConfiguration.Builder region(Region region) { + builder.option(AwsClientOption.AWS_REGION, region); + return this; + } + + /** + * Gets the value for AWS region + */ + @Override + public Region region() { + return builder.option(AwsClientOption.AWS_REGION); + } + + /** + * Sets the value for credentials provider + */ + @Override + public JsonProtocolTestsServiceClientConfiguration.Builder credentialsProvider( + IdentityProvider credentialsProvider) { + builder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER, credentialsProvider); + return this; + } + + /** + * Gets the value for credentials provider + */ + @Override + public IdentityProvider credentialsProvider() { + return builder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER); + } + + /** + * Sets the value for auth scheme provider + */ + @Override + public JsonProtocolTestsServiceClientConfiguration.Builder authSchemeProvider( + JsonProtocolTestsAuthSchemeProvider authSchemeProvider) { + builder.option(SdkClientOption.AUTH_SCHEME_PROVIDER, authSchemeProvider); + return this; + } + + /** + * Gets the value for auth scheme provider + */ + @Override + public JsonProtocolTestsAuthSchemeProvider authSchemeProvider() { + AuthSchemeProvider result = builder.option(SdkClientOption.AUTH_SCHEME_PROVIDER); + if (result == null) { + return null; + } + return Validate.isInstanceOf(JsonProtocolTestsAuthSchemeProvider.class, result, "Expected an instance of " + + JsonProtocolTestsAuthSchemeProvider.class.getSimpleName()); + } + + @Override + public JsonProtocolTestsServiceClientConfiguration build() { + return new JsonProtocolTestsServiceClientConfiguration(this); + } + + @Override + public SdkClientConfiguration buildSdkClientConfiguration() { + if (overrideConfiguration != null) { + SdkClientConfigurationUtil.copyOverridesToConfiguration(overrideConfiguration, builder); + } + if (endpointOverride != null) { + builder.option(SdkClientOption.ENDPOINT, endpointOverride); + builder.option(SdkClientOption.ENDPOINT_OVERRIDDEN, true); + } + return builder.build(); + } + } +} diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java index baa51c424734..b71265026522 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration.java @@ -4,19 +4,13 @@ import software.amazon.awssdk.annotations.Generated; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.awscore.AwsServiceClientConfiguration; -import software.amazon.awssdk.awscore.client.config.AwsClientOption; import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; -import software.amazon.awssdk.core.client.config.SdkClientConfiguration; -import software.amazon.awssdk.core.client.config.SdkClientOption; -import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; -import software.amazon.awssdk.core.client.config.internal.SdkInternalAdvancedClientOption; import software.amazon.awssdk.endpoints.EndpointProvider; -import software.amazon.awssdk.http.auth.spi.scheme.AuthSchemeProvider; import software.amazon.awssdk.identity.spi.AwsCredentialsIdentity; import software.amazon.awssdk.identity.spi.IdentityProvider; import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.jsonprotocoltests.auth.scheme.JsonProtocolTestsAuthSchemeProvider; -import software.amazon.awssdk.utils.Validate; +import software.amazon.awssdk.services.jsonprotocoltests.internal.JsonProtocolTestsServiceClientConfigurationBuilder; /** * Class to expose the service client settings to the user. Implementation of {@link AwsServiceClientConfiguration} @@ -26,7 +20,7 @@ public final class JsonProtocolTestsServiceClientConfiguration extends AwsServiceClientConfiguration { private final JsonProtocolTestsAuthSchemeProvider authSchemeProvider; - private JsonProtocolTestsServiceClientConfiguration(Builder builder) { + public JsonProtocolTestsServiceClientConfiguration(Builder builder) { super(builder); this.authSchemeProvider = builder.authSchemeProvider(); } @@ -39,11 +33,7 @@ public JsonProtocolTestsAuthSchemeProvider authSchemeProvider() { } public static Builder builder() { - return new BuilderImpl(); - } - - public static BuilderInternal builder(SdkClientConfiguration.Builder builder) { - return new BuilderImpl(builder); + return JsonProtocolTestsServiceClientConfigurationBuilder.builder(); } /** @@ -123,144 +113,4 @@ public interface Builder extends AwsServiceClientConfiguration.Builder { @Override JsonProtocolTestsServiceClientConfiguration build(); } - - /** - * An internal builder for creating a {@link JsonProtocolTestsServiceClientConfiguration} - */ - public interface BuilderInternal extends Builder { - SdkClientConfiguration buildSdkClientConfiguration(); - } - - private static final class BuilderImpl implements BuilderInternal { - private final SdkClientConfiguration.Builder builder; - - private BuilderImpl() { - this.builder = SdkClientConfiguration.builder(); - } - - private BuilderImpl(SdkClientConfiguration.Builder builder) { - this.builder = builder; - } - - /** - * Sets the value for client override configuration - */ - @Override - public Builder overrideConfiguration(ClientOverrideConfiguration overrideConfiguration) { - builder.option(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION, overrideConfiguration); - return this; - } - - /** - * Gets the value for client override configuration - */ - @Override - public ClientOverrideConfiguration overrideConfiguration() { - return builder.option(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION); - } - - /** - * Sets the value for endpoint override - */ - @Override - public Builder endpointOverride(URI endpointOverride) { - builder.option(SdkInternalAdvancedClientOption.ENDPOINT_OVERRIDE_VALUE, endpointOverride); - return this; - } - - /** - * Gets the value for endpoint override - */ - @Override - public URI endpointOverride() { - return builder.option(SdkInternalAdvancedClientOption.ENDPOINT_OVERRIDE_VALUE); - } - - /** - * Sets the value for endpoint provider - */ - @Override - public Builder endpointProvider(EndpointProvider endpointProvider) { - builder.option(SdkClientOption.ENDPOINT_PROVIDER, endpointProvider); - return this; - } - - /** - * Gets the value for endpoint provider - */ - @Override - public EndpointProvider endpointProvider() { - return builder.option(SdkClientOption.ENDPOINT_PROVIDER); - } - - /** - * Sets the value for AWS region - */ - @Override - public Builder region(Region region) { - builder.option(AwsClientOption.AWS_REGION, region); - return this; - } - - /** - * Gets the value for AWS region - */ - @Override - public Region region() { - return builder.option(AwsClientOption.AWS_REGION); - } - - /** - * Sets the value for credentials provider - */ - @Override - public Builder credentialsProvider(IdentityProvider credentialsProvider) { - builder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER, credentialsProvider); - return this; - } - - /** - * Gets the value for credentials provider - */ - @Override - public IdentityProvider credentialsProvider() { - return builder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER); - } - - /** - * Sets the value for auth scheme provider - */ - @Override - public Builder authSchemeProvider(JsonProtocolTestsAuthSchemeProvider authSchemeProvider) { - builder.option(SdkClientOption.AUTH_SCHEME_PROVIDER, authSchemeProvider); - return this; - } - - /** - * Gets the value for auth scheme provider - */ - @Override - public JsonProtocolTestsAuthSchemeProvider authSchemeProvider() { - AuthSchemeProvider result = builder.option(SdkClientOption.AUTH_SCHEME_PROVIDER); - if (result == null) { - return null; - } - return Validate.isInstanceOf(JsonProtocolTestsAuthSchemeProvider.class, result, "Expected an instance of " - + JsonProtocolTestsAuthSchemeProvider.class.getSimpleName()); - } - - @Override - public JsonProtocolTestsServiceClientConfiguration build() { - return new JsonProtocolTestsServiceClientConfiguration(this); - } - - @Override - public SdkClientConfiguration buildSdkClientConfiguration() { - ClientOverrideConfiguration overrideConfiguration = overrideConfiguration(); - if (overrideConfiguration != null) { - SdkClientConfigurationUtil.copyOverridesToConfiguration(overrideConfiguration, builder); - } - return builder.build(); - } - } } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java index e353519e359d..31b33b5cb530 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/builder/SdkDefaultClientBuilder.java @@ -63,7 +63,6 @@ import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; import software.amazon.awssdk.core.client.config.internal.SdkClientConfigurationUtil; -import software.amazon.awssdk.core.client.config.internal.SdkInternalAdvancedClientOption; import software.amazon.awssdk.core.interceptor.ClasspathInterceptorChainFactory; import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.internal.http.loader.DefaultSdkAsyncHttpClientBuilder; @@ -487,12 +486,10 @@ private List sdkInterceptors() { public final B endpointOverride(URI endpointOverride) { if (endpointOverride == null) { clientConfiguration.option(SdkClientOption.ENDPOINT, null); - clientConfiguration.option(SdkInternalAdvancedClientOption.ENDPOINT_OVERRIDE_VALUE, null); clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN, false); } else { Validate.paramNotNull(endpointOverride.getScheme(), "The URI scheme of endpointOverride"); clientConfiguration.option(SdkClientOption.ENDPOINT, endpointOverride); - clientConfiguration.option(SdkInternalAdvancedClientOption.ENDPOINT_OVERRIDE_VALUE, endpointOverride); clientConfiguration.option(SdkClientOption.ENDPOINT_OVERRIDDEN, true); } return thisBuilder(); diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java index 22c676fcb67d..1fcc796d0267 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java @@ -15,6 +15,10 @@ package software.amazon.awssdk.core.client.config.internal; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.function.Supplier; import software.amazon.awssdk.annotations.SdkInternalApi; import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; @@ -31,14 +35,13 @@ private SdkClientConfigurationUtil() { /** * Copies the {@link ClientOverrideConfiguration} values to the configuration builder. + *

+ * NOTE make sure to mirror the properties in the method below or create a new abstraction to avoid this coupling. */ public static SdkClientConfiguration.Builder copyOverridesToConfiguration( ClientOverrideConfiguration overrides, SdkClientConfiguration.Builder builder ) { - // Save the ClientOverrideConfiguration instance - builder.option(SdkInternalAdvancedClientOption.CLIENT_OVERRIDE_CONFIGURATION, overrides); - // misc builder.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS, overrides.headers()); builder.option(SdkClientOption.RETRY_POLICY, overrides.retryPolicy().orElse(null)); @@ -76,4 +79,55 @@ public static SdkClientConfiguration.Builder copyOverridesToConfiguration( return builder; } + + /** + * Copies the {@link SdkClientConfiguration} values to the {@link ClientOverrideConfiguration.Builder} builder + *

+ * NOTE make sure to mirror the properties in the method above or create a new abstraction to avoid this coupling. + */ + public static ClientOverrideConfiguration.Builder copyConfigurationToOverrides( + ClientOverrideConfiguration.Builder clientOverrides, + SdkClientConfiguration.Builder clientConfiguration + ) { + // misc + Map> additionalHeaders =clientConfiguration.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS); + if (additionalHeaders != null) { + clientOverrides.headers(additionalHeaders); + } + clientOverrides.retryPolicy(clientConfiguration.option(SdkClientOption.RETRY_POLICY)); + clientOverrides.apiCallTimeout(clientConfiguration.option(SdkClientOption.API_CALL_TIMEOUT)); + clientOverrides.apiCallAttemptTimeout(clientConfiguration.option(SdkClientOption.API_CALL_ATTEMPT_TIMEOUT)); + clientOverrides.scheduledExecutorService(clientConfiguration.option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE)); + clientOverrides.executionInterceptors(clientConfiguration.option(SdkClientOption.EXECUTION_INTERCEPTORS)); + clientOverrides.executionAttributes(clientConfiguration.option(SdkClientOption.EXECUTION_ATTRIBUTES)); + + // advanced option + if (Boolean.TRUE.equals(clientConfiguration.option(SdkClientOption.SIGNER_OVERRIDDEN))) { + Signer signer = clientConfiguration.option(SdkAdvancedClientOption.SIGNER); + clientOverrides.putAdvancedOption(SdkAdvancedClientOption.SIGNER, signer); + } + + clientOverrides.putAdvancedOption(SdkAdvancedClientOption.USER_AGENT_SUFFIX, + clientConfiguration.option(SdkAdvancedClientOption.USER_AGENT_SUFFIX)); + clientOverrides.putAdvancedOption(SdkAdvancedClientOption.USER_AGENT_PREFIX, + clientConfiguration.option(SdkAdvancedClientOption.USER_AGENT_PREFIX)); + clientOverrides.putAdvancedOption(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION, + clientConfiguration.option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION)); + clientOverrides.putAdvancedOption(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION, + clientConfiguration.option(SdkAdvancedClientOption.DISABLE_HOST_PREFIX_INJECTION)); + clientOverrides.putAdvancedOption(SdkAdvancedClientOption.TOKEN_SIGNER, + clientConfiguration.option(SdkAdvancedClientOption.TOKEN_SIGNER)); + + // profile + Optional.ofNullable(clientConfiguration.option(SdkClientOption.PROFILE_FILE_SUPPLIER)) + .map(Supplier::get) + .ifPresent(clientOverrides::defaultProfileFile); + + // misc + clientOverrides.defaultProfileName(clientConfiguration.option(SdkClientOption.PROFILE_NAME)); + clientOverrides.metricPublishers(clientConfiguration.option(SdkClientOption.METRIC_PUBLISHERS)); + clientOverrides.compressionConfiguration(clientConfiguration.option(SdkClientOption.COMPRESSION_CONFIGURATION)); + + return clientOverrides; + } } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java deleted file mode 100644 index 73030818061c..000000000000 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkInternalAdvancedClientOption.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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.client.config.internal; - -import java.net.URI; -import software.amazon.awssdk.annotations.SdkInternalApi; -import software.amazon.awssdk.core.SdkServiceClientConfiguration; -import software.amazon.awssdk.core.client.config.ClientOption; -import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration; -import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; -import software.amazon.awssdk.core.client.config.SdkClientOption; - -/** - * Options of {@link SdkAdvancedClientOption} that must not be used outside of internals of this project. Changes to this - * class are not guaranteed to be backwards compatible. - */ -@SdkInternalApi -public class SdkInternalAdvancedClientOption extends ClientOption { - /** - * The endpoint override is not currently reflected in the client options. We set its value using this internal option to - * allow to reconstruct the {@link SdkServiceClientConfiguration} with the same set of values without having to branch on - * whether the {@link SdkClientOption#ENDPOINT_OVERRIDDEN} was set. - */ - @SdkInternalApi - public static final SdkInternalAdvancedClientOption ENDPOINT_OVERRIDE_VALUE = - new SdkInternalAdvancedClientOption<>(URI.class); - - /** - * The client override configuration is not currently reflected in the client options. We set its value using this internal - * option to allow to reconstruct the {@link SdkServiceClientConfiguration} with the same set of values without having to - * branch on whether the different options are set. - */ - @SdkInternalApi - public static final SdkInternalAdvancedClientOption CLIENT_OVERRIDE_CONFIGURATION = - new SdkInternalAdvancedClientOption<>(ClientOverrideConfiguration.class); - - protected SdkInternalAdvancedClientOption(Class valueClass) { - super(valueClass); - } -} diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java index 389957cd39c6..6d47dc4220ff 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java @@ -41,6 +41,7 @@ import software.amazon.awssdk.regions.Region; import software.amazon.awssdk.services.protocolrestjson.ProtocolRestJsonServiceClientConfiguration; import software.amazon.awssdk.services.protocolrestjson.auth.scheme.ProtocolRestJsonAuthSchemeProvider; +import software.amazon.awssdk.services.protocolrestjson.internal.ProtocolRestJsonServiceClientConfigurationBuilder; public class CodegenServiceClientConfigurationTest { private static final EndpointProvider MOCK_ENDPOINT_PROVIDER = mock(EndpointProvider.class); @@ -53,8 +54,8 @@ public class CodegenServiceClientConfigurationTest { @ParameterizedTest @MethodSource("testCases") void fromExternalToInternal(TestCase testCase) { - ProtocolRestJsonServiceClientConfiguration.BuilderInternal builder = - ProtocolRestJsonServiceClientConfiguration.builder(SdkClientConfiguration.builder()); + ProtocolRestJsonServiceClientConfigurationBuilder.BuilderInternal builder = + ProtocolRestJsonServiceClientConfigurationBuilder.builder(SdkClientConfiguration.builder()); // Verify that initially the value is null for properties with direct mapping. if (testCase.hasDirectMapping) { @@ -77,8 +78,8 @@ void fromExternalToInternal(TestCase testCase) { SdkClientConfiguration clientConfig = builder.buildSdkClientConfiguration(); // Build a new builder with the created client config - ProtocolRestJsonServiceClientConfiguration.BuilderInternal anotherBuilder = - ProtocolRestJsonServiceClientConfiguration.builder(clientConfig.toBuilder()); + ProtocolRestJsonServiceClientConfigurationBuilder.BuilderInternal anotherBuilder = + ProtocolRestJsonServiceClientConfigurationBuilder.builder(clientConfig.toBuilder()); // Assert that we can retrieve the same value assertThat(testCase.getter.apply(anotherBuilder)).isEqualTo(testCase.value); From 9e97282b2207066beaab57837c8ac472e880bb07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Mon, 25 Sep 2023 14:15:01 -0700 Subject: [PATCH 3/5] Address PR comments --- .../client/config/internal/SdkClientConfigurationUtil.java | 2 +- .../awssdk/services/CodegenServiceClientConfigurationTest.java | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java index 1fcc796d0267..2245e394c13e 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java @@ -90,7 +90,7 @@ public static ClientOverrideConfiguration.Builder copyConfigurationToOverrides( SdkClientConfiguration.Builder clientConfiguration ) { // misc - Map> additionalHeaders =clientConfiguration.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS); + Map> additionalHeaders = clientConfiguration.option(SdkClientOption.ADDITIONAL_HTTP_HEADERS); if (additionalHeaders != null) { clientOverrides.headers(additionalHeaders); } diff --git a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java index 6d47dc4220ff..9e45f40f64f2 100644 --- a/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java +++ b/test/codegen-generated-classes-test/src/test/java/software/amazon/awssdk/services/CodegenServiceClientConfigurationTest.java @@ -53,7 +53,7 @@ public class CodegenServiceClientConfigurationTest { @ParameterizedTest @MethodSource("testCases") - void fromExternalToInternal(TestCase testCase) { + void externalInternalTransforms_preserves_propertyValues(TestCase testCase) { ProtocolRestJsonServiceClientConfigurationBuilder.BuilderInternal builder = ProtocolRestJsonServiceClientConfigurationBuilder.builder(SdkClientConfiguration.builder()); @@ -169,7 +169,6 @@ public static Builder builder() { return new Builder(); } - static class Builder { private ClientOption option; private T value; From 287cf62ddc697f58c9628c80cdf6ad122c26172a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Mon, 25 Sep 2023 17:56:55 -0700 Subject: [PATCH 4/5] Rename internal SdkClientConfiguration.Builder to internalBuilder --- ...erviceClientConfigurationBuilderClass.java | 14 +++---- .../ServiceClientConfigurationUtils.java | 12 +++--- .../serviceclientconfiguration-builder.java | 38 +++++++++---------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java index a87c552a20f5..4fd0b5f85783 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationBuilderClass.java @@ -95,11 +95,11 @@ private TypeSpec builderImplSpec() { .addModifiers(PUBLIC, STATIC) .addSuperinterface(className().nestedClass("BuilderInternal")); - builder.addField(SdkClientConfiguration.Builder.class, "builder", PRIVATE, FINAL); + builder.addField(SdkClientConfiguration.Builder.class, "internalBuilder", PRIVATE, FINAL); builder.addMethod(MethodSpec.constructorBuilder() .addModifiers(PRIVATE) - .addStatement("this.builder = $T.builder()", SdkClientConfiguration.class) + .addStatement("this.internalBuilder = $T.builder()", SdkClientConfiguration.class) .build()); builder.addMethod(constructorFromSdkClientConfiguration()); @@ -136,15 +136,15 @@ private MethodSpec buildSdkClientConfigurationMethod() { } } return builder - .addStatement("return builder.build()") + .addStatement("return internalBuilder.build()") .build(); } private MethodSpec constructorFromSdkClientConfiguration() { MethodSpec.Builder builder = MethodSpec.constructorBuilder() .addModifiers(PRIVATE) - .addParameter(SdkClientConfiguration.Builder.class, "builder") - .addStatement("this.builder = builder"); + .addParameter(SdkClientConfiguration.Builder.class, "internalBuilder") + .addStatement("this.internalBuilder = internalBuilder"); for (Field field : utils.serviceClientConfigurationFields()) { if (field.optionClass() == null) { @@ -175,7 +175,7 @@ private MethodSpec setterForField(Field field) { .build(); } - return builder.addStatement("builder.option($T.$L, $L)", + return builder.addStatement("internalBuilder.option($T.$L, $L)", field.optionClass(), field.optionName(), field.name()) .addStatement("return this") .build(); @@ -194,7 +194,7 @@ private MethodSpec.Builder baseSetterForField(Field field) { } private MethodSpec getterForBuilderField(Field field) { - return getterForField(field, "builder", false); + return getterForField(field, "internalBuilder", false); } diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java index 1a65625e29fb..0e50e35755a5 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java @@ -119,10 +119,10 @@ private static Field endpointOverrideField() { .definingClass(SdkServiceClientConfiguration.class); builder.constructFromConfiguration( CodeBlock.builder() - .beginControlFlow("if (!Boolean.TRUE.equals(builder.option($T.$L)))", + .beginControlFlow("if (!Boolean.TRUE.equals(internalBuilder.option($T.$L)))", SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT_OVERRIDDEN, SdkClientOption.class)) - .addStatement("this.endpointOverride = builder.option($T.$L)", + .addStatement("this.endpointOverride = internalBuilder.option($T.$L)", SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT, SdkClientOption.class)) .endControlFlow() @@ -132,9 +132,9 @@ SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT, builder.copyToConfiguration( CodeBlock.builder() .beginControlFlow("if (endpointOverride != null)") - .addStatement("builder.option($T.$L, endpointOverride)", + .addStatement("internalBuilder.option($T.$L, endpointOverride)", SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT, SdkClientOption.class)) - .addStatement("builder.option($T.$L, true)", + .addStatement("internalBuilder.option($T.$L, true)", SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT_OVERRIDDEN, SdkClientOption.class)) .endControlFlow() .build() @@ -151,7 +151,7 @@ private static Field overrideConfigurationField() { builder.constructFromConfiguration( CodeBlock.builder() .addStatement("this.overrideConfiguration = $T.copyConfigurationToOverrides(" - + "$T.builder(), builder).build()", SdkClientConfigurationUtil.class, + + "$T.builder(), internalBuilder).build()", SdkClientConfigurationUtil.class, ClientOverrideConfiguration.class) .build() ); @@ -159,7 +159,7 @@ private static Field overrideConfigurationField() { builder.copyToConfiguration( CodeBlock.builder() .beginControlFlow("if (overrideConfiguration != null)") - .addStatement("$T.copyOverridesToConfiguration(overrideConfiguration, builder)", + .addStatement("$T.copyOverridesToConfiguration(overrideConfiguration, internalBuilder)", SdkClientConfigurationUtil.class) .endControlFlow() .build() diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java index a672cd1e874b..87a96292994e 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java @@ -33,22 +33,22 @@ public interface BuilderInternal extends JsonProtocolTestsServiceClientConfigura } public static class BuilderImpl implements BuilderInternal { - private final SdkClientConfiguration.Builder builder; + private final SdkClientConfiguration.Builder internalBuilder; private ClientOverrideConfiguration overrideConfiguration; private URI endpointOverride; private BuilderImpl() { - this.builder = SdkClientConfiguration.builder(); + this.internalBuilder = SdkClientConfiguration.builder(); } - private BuilderImpl(SdkClientConfiguration.Builder builder) { - this.builder = builder; + private BuilderImpl(SdkClientConfiguration.Builder internalBuilder) { + this.internalBuilder = internalBuilder; this.overrideConfiguration = SdkClientConfigurationUtil.copyConfigurationToOverrides( - ClientOverrideConfiguration.builder(), builder).build(); - if (!Boolean.TRUE.equals(builder.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) { - this.endpointOverride = builder.option(SdkClientOption.ENDPOINT); + ClientOverrideConfiguration.builder(), internalBuilder).build(); + if (!Boolean.TRUE.equals(internalBuilder.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) { + this.endpointOverride = internalBuilder.option(SdkClientOption.ENDPOINT); } } @@ -92,7 +92,7 @@ public URI endpointOverride() { */ @Override public JsonProtocolTestsServiceClientConfiguration.Builder endpointProvider(EndpointProvider endpointProvider) { - builder.option(SdkClientOption.ENDPOINT_PROVIDER, endpointProvider); + internalBuilder.option(SdkClientOption.ENDPOINT_PROVIDER, endpointProvider); return this; } @@ -101,7 +101,7 @@ public JsonProtocolTestsServiceClientConfiguration.Builder endpointProvider(Endp */ @Override public EndpointProvider endpointProvider() { - return builder.option(SdkClientOption.ENDPOINT_PROVIDER); + return internalBuilder.option(SdkClientOption.ENDPOINT_PROVIDER); } /** @@ -109,7 +109,7 @@ public EndpointProvider endpointProvider() { */ @Override public JsonProtocolTestsServiceClientConfiguration.Builder region(Region region) { - builder.option(AwsClientOption.AWS_REGION, region); + internalBuilder.option(AwsClientOption.AWS_REGION, region); return this; } @@ -118,7 +118,7 @@ public JsonProtocolTestsServiceClientConfiguration.Builder region(Region region) */ @Override public Region region() { - return builder.option(AwsClientOption.AWS_REGION); + return internalBuilder.option(AwsClientOption.AWS_REGION); } /** @@ -127,7 +127,7 @@ public Region region() { @Override public JsonProtocolTestsServiceClientConfiguration.Builder credentialsProvider( IdentityProvider credentialsProvider) { - builder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER, credentialsProvider); + internalBuilder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER, credentialsProvider); return this; } @@ -136,7 +136,7 @@ public JsonProtocolTestsServiceClientConfiguration.Builder credentialsProvider( */ @Override public IdentityProvider credentialsProvider() { - return builder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER); + return internalBuilder.option(AwsClientOption.CREDENTIALS_IDENTITY_PROVIDER); } /** @@ -145,7 +145,7 @@ public IdentityProvider credentialsProvider() @Override public JsonProtocolTestsServiceClientConfiguration.Builder authSchemeProvider( JsonProtocolTestsAuthSchemeProvider authSchemeProvider) { - builder.option(SdkClientOption.AUTH_SCHEME_PROVIDER, authSchemeProvider); + internalBuilder.option(SdkClientOption.AUTH_SCHEME_PROVIDER, authSchemeProvider); return this; } @@ -154,7 +154,7 @@ public JsonProtocolTestsServiceClientConfiguration.Builder authSchemeProvider( */ @Override public JsonProtocolTestsAuthSchemeProvider authSchemeProvider() { - AuthSchemeProvider result = builder.option(SdkClientOption.AUTH_SCHEME_PROVIDER); + AuthSchemeProvider result = internalBuilder.option(SdkClientOption.AUTH_SCHEME_PROVIDER); if (result == null) { return null; } @@ -170,13 +170,13 @@ public JsonProtocolTestsServiceClientConfiguration build() { @Override public SdkClientConfiguration buildSdkClientConfiguration() { if (overrideConfiguration != null) { - SdkClientConfigurationUtil.copyOverridesToConfiguration(overrideConfiguration, builder); + SdkClientConfigurationUtil.copyOverridesToConfiguration(overrideConfiguration, internalBuilder); } if (endpointOverride != null) { - builder.option(SdkClientOption.ENDPOINT, endpointOverride); - builder.option(SdkClientOption.ENDPOINT_OVERRIDDEN, true); + internalBuilder.option(SdkClientOption.ENDPOINT, endpointOverride); + internalBuilder.option(SdkClientOption.ENDPOINT_OVERRIDDEN, true); } - return builder.build(); + return internalBuilder.build(); } } } From 4c13ca6a3d7719dc43a438508b1c34b940eaa9b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20Sugawara=20=28=E2=88=A9=EF=BD=80-=C2=B4=29?= =?UTF-8?q?=E2=8A=83=E2=94=81=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E=E7=82=8E?= Date: Tue, 26 Sep 2023 09:54:47 -0700 Subject: [PATCH 5/5] Fix the logic to set the endpoint override --- .../poet/model/ServiceClientConfigurationUtils.java | 2 +- .../model/serviceclientconfiguration-builder.java | 2 +- .../config/internal/SdkClientConfigurationUtil.java | 13 ++++++++++--- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java index 0e50e35755a5..47c6935e532a 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ServiceClientConfigurationUtils.java @@ -119,7 +119,7 @@ private static Field endpointOverrideField() { .definingClass(SdkServiceClientConfiguration.class); builder.constructFromConfiguration( CodeBlock.builder() - .beginControlFlow("if (!Boolean.TRUE.equals(internalBuilder.option($T.$L)))", + .beginControlFlow("if (Boolean.TRUE.equals(internalBuilder.option($T.$L)))", SdkClientOption.class, fieldName(SdkClientOption.ENDPOINT_OVERRIDDEN, SdkClientOption.class)) .addStatement("this.endpointOverride = internalBuilder.option($T.$L)", diff --git a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java index 87a96292994e..4afcc913838d 100644 --- a/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java +++ b/codegen/src/test/resources/software/amazon/awssdk/codegen/poet/model/serviceclientconfiguration-builder.java @@ -47,7 +47,7 @@ private BuilderImpl(SdkClientConfiguration.Builder internalBuilder) { this.internalBuilder = internalBuilder; this.overrideConfiguration = SdkClientConfigurationUtil.copyConfigurationToOverrides( ClientOverrideConfiguration.builder(), internalBuilder).build(); - if (!Boolean.TRUE.equals(internalBuilder.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) { + if (Boolean.TRUE.equals(internalBuilder.option(SdkClientOption.ENDPOINT_OVERRIDDEN))) { this.endpointOverride = internalBuilder.option(SdkClientOption.ENDPOINT); } } diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java index 2245e394c13e..9310a28bea26 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/client/config/internal/SdkClientConfigurationUtil.java @@ -24,8 +24,10 @@ import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption; import software.amazon.awssdk.core.client.config.SdkClientConfiguration; import software.amazon.awssdk.core.client.config.SdkClientOption; +import software.amazon.awssdk.core.interceptor.ExecutionInterceptor; import software.amazon.awssdk.core.internal.SdkInternalTestAdvancedClientOption; import software.amazon.awssdk.core.signer.Signer; +import software.amazon.awssdk.metrics.MetricPublisher; import software.amazon.awssdk.profiles.ProfileFileSupplier; @SdkInternalApi @@ -98,8 +100,10 @@ public static ClientOverrideConfiguration.Builder copyConfigurationToOverrides( clientOverrides.apiCallTimeout(clientConfiguration.option(SdkClientOption.API_CALL_TIMEOUT)); clientOverrides.apiCallAttemptTimeout(clientConfiguration.option(SdkClientOption.API_CALL_ATTEMPT_TIMEOUT)); clientOverrides.scheduledExecutorService(clientConfiguration.option(SdkClientOption.SCHEDULED_EXECUTOR_SERVICE)); - clientOverrides.executionInterceptors(clientConfiguration.option(SdkClientOption.EXECUTION_INTERCEPTORS)); - clientOverrides.executionAttributes(clientConfiguration.option(SdkClientOption.EXECUTION_ATTRIBUTES)); + List executionInterceptors = clientConfiguration.option(SdkClientOption.EXECUTION_INTERCEPTORS); + if (executionInterceptors != null) { + clientOverrides.executionInterceptors(executionInterceptors); + } // advanced option if (Boolean.TRUE.equals(clientConfiguration.option(SdkClientOption.SIGNER_OVERRIDDEN))) { @@ -125,7 +129,10 @@ public static ClientOverrideConfiguration.Builder copyConfigurationToOverrides( // misc clientOverrides.defaultProfileName(clientConfiguration.option(SdkClientOption.PROFILE_NAME)); - clientOverrides.metricPublishers(clientConfiguration.option(SdkClientOption.METRIC_PUBLISHERS)); + List metricPublishers = clientConfiguration.option(SdkClientOption.METRIC_PUBLISHERS); + if (metricPublishers != null) { + clientOverrides.metricPublishers(metricPublishers); + } clientOverrides.compressionConfiguration(clientConfiguration.option(SdkClientOption.COMPRESSION_CONFIGURATION)); return clientOverrides;