Skip to content

Commit b749805

Browse files
zoewanggzoewangg
committed
Address comments
1 parent 7d92528 commit b749805

File tree

4 files changed

+89
-109
lines changed

4 files changed

+89
-109
lines changed

‎codegen-lite/src/main/java/software/amazon/awssdk/codegen/lite/defaultsmode/DefaultsModeConfigurationGenerator.java

Lines changed: 58 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@
2525
import com.squareup.javapoet.CodeBlock;
2626
import com.squareup.javapoet.FieldSpec;
2727
import com.squareup.javapoet.MethodSpec;
28+
import com.squareup.javapoet.ParameterizedTypeName;
2829
import com.squareup.javapoet.TypeSpec;
30+
import java.util.EnumMap;
2931
import java.util.HashMap;
3032
import java.util.Locale;
3133
import java.util.Map;
@@ -41,21 +43,23 @@
4143
*/
4244
public class DefaultsModeConfigurationGenerator implements PoetClass {
4345

46+
private static final String DEFAULT_CONFIG_BY_MODE_ENUM_MAP = "DEFAULT_CONFIG_BY_MODE";
47+
private static final String DEFAULT_HTTP_CONFIG_BY_MODE_ENUM_MAP = "DEFAULT_HTTP_CONFIG_BY_MODE";
48+
private static final String DEFAULTS_VAR_SUFFIX = "_DEFAULTS";
49+
private static final String HTTP_DEFAULTS_VAR_SUFFIX = "_HTTP_DEFAULTS";
50+
private static final Map<String, OptionMetadata> CONFIGURATION_MAPPING = new HashMap<>();
51+
private static final Map<String, OptionMetadata> HTTP_CONFIGURATION_MAPPING = new HashMap<>();
4452
private final String basePackage;
4553
private final String defaultsModeBase;
4654
private final DefaultConfiguration configuration;
4755

48-
private static final Map<String, OptionMetadata> CONFIGURATION_MAPPING = new HashMap<>();
49-
50-
private static final Map<String, OptionMetadata> HTTP_CONFIGURATION_MAPPING = new HashMap<>();
51-
5256
static {
5357
HTTP_CONFIGURATION_MAPPING.put("connectTimeoutInMillis",
5458
new OptionMetadata(ClassName.get("java.time", "Duration"),
5559
ClassName.get("software.amazon.awssdk.http",
5660
"SdkHttpConfigurationOption", "CONNECTION_TIMEOUT")));
5761
CONFIGURATION_MAPPING.put("retryMode", new OptionMetadata(ClassName.get("software.amazon.awssdk.core.retry", "RetryMode"
58-
), ClassName.get("software.amazon.awssdk.core.client.config","SdkClientOption", "DEFAULT_RETRY_MODE")));
62+
), ClassName.get("software.amazon.awssdk.core.client.config", "SdkClientOption", "DEFAULT_RETRY_MODE")));
5963
}
6064

6165
public DefaultsModeConfigurationGenerator(String basePackage, String defaultsModeBase, DefaultConfiguration configuration) {
@@ -75,8 +79,9 @@ public TypeSpec poetClass() {
7579
"$S",
7680
"software.amazon.awssdk:codegen")
7781
.build())
78-
.addMethod(defaultHttpConfigMethod(configuration.modeDefaults().keySet()))
79-
.addMethod(defaultSdkConfigMethod(configuration.modeDefaults().keySet()))
82+
.addMethod(defaultConfigMethod(DEFAULT_CONFIG_BY_MODE_ENUM_MAP, "defaultConfig"))
83+
.addMethod(defaultConfigMethod(DEFAULT_HTTP_CONFIG_BY_MODE_ENUM_MAP,
84+
"defaultHttpConfig"))
8085
.addMethod(createConstructor());
8186

8287

@@ -87,18 +92,52 @@ public TypeSpec poetClass() {
8792

8893
addDefaultsFieldForLegacy(builder, "LEGACY_DEFAULTS");
8994
addDefaultsFieldForLegacy(builder, "LEGACY_HTTP_DEFAULTS");
95+
96+
addEnumMapField(builder, DEFAULT_CONFIG_BY_MODE_ENUM_MAP);
97+
addEnumMapField(builder, DEFAULT_HTTP_CONFIG_BY_MODE_ENUM_MAP);
98+
99+
addStaticEnumMapBlock(builder);
90100
return builder.build();
91101
}
92102

103+
private void addStaticEnumMapBlock(TypeSpec.Builder builder) {
104+
CodeBlock.Builder staticCodeBlock = CodeBlock.builder();
105+
106+
putItemsToEnumMap(staticCodeBlock, configuration.modeDefaults().keySet(), DEFAULTS_VAR_SUFFIX,
107+
DEFAULT_CONFIG_BY_MODE_ENUM_MAP);
108+
putItemsToEnumMap(staticCodeBlock, configuration.modeDefaults().keySet(), HTTP_DEFAULTS_VAR_SUFFIX,
109+
DEFAULT_HTTP_CONFIG_BY_MODE_ENUM_MAP);
110+
111+
builder.addStaticBlock(staticCodeBlock.build());
112+
}
113+
114+
private void addEnumMapField(TypeSpec.Builder builder, String name) {
115+
ParameterizedTypeName map = ParameterizedTypeName.get(ClassName.get(Map.class),
116+
defaultsModeClassName(),
117+
ClassName.get(AttributeMap.class));
118+
FieldSpec field = FieldSpec.builder(map, name, PRIVATE, STATIC, FINAL)
119+
.initializer("new $T<>(DefaultsMode.class)", EnumMap.class).build();
120+
builder.addField(field);
121+
}
122+
123+
private void putItemsToEnumMap(CodeBlock.Builder codeBlock, Set<String> modes, String suffix, String mapName) {
124+
modes.forEach(m -> {
125+
String mode = sanitizeMode(m);
126+
codeBlock.addStatement("$N.put(DefaultsMode.$N, $N)", mapName, mode, mode + suffix);
127+
});
128+
129+
// Add LEGACY since LEGACY is not in the modes set
130+
codeBlock.addStatement("$N.put(DefaultsMode.LEGACY, LEGACY$N)", mapName, suffix);
131+
}
132+
93133
@Override
94134
public ClassName className() {
95135
return ClassName.get(basePackage, "DefaultsModeConfiguration");
96136
}
97137

98138
private FieldSpec addDefaultsFieldForMode(Map.Entry<String, Map<String, String>> modeEntry) {
99139
String mode = modeEntry.getKey();
100-
String fieldName = sanitizeMode(mode) + "_DEFAULTS";
101-
140+
String fieldName = sanitizeMode(mode) + DEFAULTS_VAR_SUFFIX;
102141

103142
CodeBlock.Builder attributeBuilder = CodeBlock.builder()
104143
.add("$T.builder()", AttributeMap.class);
@@ -129,7 +168,8 @@ private void attributeMapBuilder(String option, String value, CodeBlock.Builder
129168
OptionMetadata optionMetadata = CONFIGURATION_MAPPING.get(option);
130169
switch (option) {
131170
case "retryMode":
132-
attributeBuilder.add(".put($T, $T.$N)", optionMetadata.attribute, optionMetadata.type, value.toUpperCase(Locale.US));
171+
attributeBuilder.add(".put($T, $T.$N)", optionMetadata.attribute, optionMetadata.type,
172+
value.toUpperCase(Locale.US));
133173
break;
134174
default:
135175
throw new IllegalStateException("Unsupported option " + option);
@@ -149,7 +189,7 @@ private void httpAttributeMapBuilder(String option, String value, CodeBlock.Buil
149189

150190
private FieldSpec addHttpDefaultsFieldForMode(Map.Entry<String, Map<String, String>> modeEntry) {
151191
String mode = modeEntry.getKey();
152-
String fieldName = sanitizeMode(mode) + "_HTTP_DEFAULTS";
192+
String fieldName = sanitizeMode(mode) + HTTP_DEFAULTS_VAR_SUFFIX;
153193

154194
CodeBlock.Builder attributeBuilder = CodeBlock.builder()
155195
.add("$T.builder()", AttributeMap.class);
@@ -180,59 +220,17 @@ private CodeBlock documentation() {
180220
return builder.build();
181221
}
182222

183-
184-
private MethodSpec defaultHttpConfigMethod(Set<String> modes) {
185-
String nameSuffix = "_HTTP_DEFAULTS";
186-
MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder("defaultHttpConfig")
223+
private MethodSpec defaultConfigMethod(String enumMap, String methodName) {
224+
MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder(methodName)
187225
.returns(AttributeMap.class)
188226
.addModifiers(PUBLIC, STATIC)
189-
.addJavadoc("Return the default HTTP config options for a given defaults "
227+
.addJavadoc("Return the default config options for a given defaults "
190228
+ "mode")
191229
.addParameter(defaultsModeClassName(), "mode")
192-
.beginControlFlow("switch (mode)");
193-
194-
195-
addSwitchCaseForEachMode(modes, nameSuffix, methodBuilder);
196-
197-
addLegacyCase(methodBuilder, "LEGACY" + nameSuffix);
198-
199-
return methodBuilder
200-
.addStatement("default: throw new IllegalArgumentException($S + $N)", "Unsupported mode: ", "mode")
201-
.endControlFlow()
202-
.build();
203-
}
204-
205-
private void addSwitchCaseForEachMode(Set<String> modes, String nameSuffix, MethodSpec.Builder methodBuilder) {
206-
modes.forEach(m -> {
207-
String mode = sanitizeMode(m);
208-
methodBuilder.addCode("case $N:", mode);
209-
methodBuilder.addStatement("return $N", mode + nameSuffix);
210-
});
211-
}
212-
213-
private MethodSpec defaultSdkConfigMethod(Set<String> modes) {
214-
String nameSuffix = "_DEFAULTS";
215-
MethodSpec.Builder methodBuilder = MethodSpec.methodBuilder("defaultConfig")
216-
.returns(AttributeMap.class)
217-
.addModifiers(PUBLIC, STATIC)
218-
.addJavadoc("Return the default SDK config options for a given defaults "
219-
+ "mode")
220-
.addParameter(defaultsModeClassName(), "mode")
221-
.beginControlFlow("switch (mode)");
222-
223-
224-
addSwitchCaseForEachMode(modes, nameSuffix, methodBuilder);
225-
addLegacyCase(methodBuilder, "LEGACY" + nameSuffix);
226-
227-
return methodBuilder
228-
.addStatement("default: throw new IllegalArgumentException($S + $N)", "Unsupported mode: ", "mode")
229-
.endControlFlow()
230-
.build();
231-
}
230+
.addStatement("return $N.getOrDefault(mode, $T.empty())",
231+
enumMap, AttributeMap.class);
232232

233-
private void addLegacyCase(MethodSpec.Builder methodBuilder, String name) {
234-
methodBuilder.addCode("case LEGACY:");
235-
methodBuilder.addStatement("return $N", name);
233+
return methodBuilder.build();
236234
}
237235

238236
private ClassName defaultsModeClassName() {
@@ -249,7 +247,7 @@ private static final class OptionMetadata {
249247
private final ClassName type;
250248
private final ClassName attribute;
251249

252-
public OptionMetadata(ClassName type, ClassName attribute) {
250+
OptionMetadata(ClassName type, ClassName attribute) {
253251
this.type = type;
254252
this.attribute = attribute;
255253
}

‎codegen-lite/src/test/resources/software/amazon/awssdk/codegen/lite/defaultsmode/defaults-mode-configuration.java

Lines changed: 25 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package software.amazon.awssdk.defaultsmode;
22

33
import java.time.Duration;
4+
import java.util.EnumMap;
5+
import java.util.Map;
46
import software.amazon.awssdk.annotations.Generated;
57
import software.amazon.awssdk.annotations.SdkInternalApi;
68
import software.amazon.awssdk.core.client.config.SdkClientOption;
@@ -42,46 +44,37 @@ public final class DefaultsModeConfiguration {
4244

4345
private static final AttributeMap LEGACY_HTTP_DEFAULTS = AttributeMap.empty();
4446

47+
private static final Map<DefaultsMode, AttributeMap> DEFAULT_CONFIG_BY_MODE = new EnumMap<>(DefaultsMode.class);
48+
49+
private static final Map<DefaultsMode, AttributeMap> DEFAULT_HTTP_CONFIG_BY_MODE = new EnumMap<>(DefaultsMode.class);
50+
51+
static {
52+
DEFAULT_CONFIG_BY_MODE.put(DefaultsMode.STANDARD, STANDARD_DEFAULTS);
53+
DEFAULT_CONFIG_BY_MODE.put(DefaultsMode.MOBILE, MOBILE_DEFAULTS);
54+
DEFAULT_CONFIG_BY_MODE.put(DefaultsMode.CROSS_REGION, CROSS_REGION_DEFAULTS);
55+
DEFAULT_CONFIG_BY_MODE.put(DefaultsMode.IN_REGION, IN_REGION_DEFAULTS);
56+
DEFAULT_CONFIG_BY_MODE.put(DefaultsMode.LEGACY, LEGACY_DEFAULTS);
57+
DEFAULT_HTTP_CONFIG_BY_MODE.put(DefaultsMode.STANDARD, STANDARD_HTTP_DEFAULTS);
58+
DEFAULT_HTTP_CONFIG_BY_MODE.put(DefaultsMode.MOBILE, MOBILE_HTTP_DEFAULTS);
59+
DEFAULT_HTTP_CONFIG_BY_MODE.put(DefaultsMode.CROSS_REGION, CROSS_REGION_HTTP_DEFAULTS);
60+
DEFAULT_HTTP_CONFIG_BY_MODE.put(DefaultsMode.IN_REGION, IN_REGION_HTTP_DEFAULTS);
61+
DEFAULT_HTTP_CONFIG_BY_MODE.put(DefaultsMode.LEGACY, LEGACY_HTTP_DEFAULTS);
62+
}
63+
4564
private DefaultsModeConfiguration() {
4665
}
4766

4867
/**
49-
* Return the default HTTP config options for a given defaults mode
68+
* Return the default config options for a given defaults mode
5069
*/
51-
public static AttributeMap defaultHttpConfig(DefaultsMode mode) {
52-
switch (mode) {
53-
case STANDARD:
54-
return STANDARD_HTTP_DEFAULTS;
55-
case MOBILE:
56-
return MOBILE_HTTP_DEFAULTS;
57-
case CROSS_REGION:
58-
return CROSS_REGION_HTTP_DEFAULTS;
59-
case IN_REGION:
60-
return IN_REGION_HTTP_DEFAULTS;
61-
case LEGACY:
62-
return LEGACY_HTTP_DEFAULTS;
63-
default:
64-
throw new IllegalArgumentException("Unsupported mode: " + mode);
65-
}
70+
public static AttributeMap defaultConfig(DefaultsMode mode) {
71+
return DEFAULT_CONFIG_BY_MODE.getOrDefault(mode, AttributeMap.empty());
6672
}
6773

6874
/**
69-
* Return the default SDK config options for a given defaults mode
75+
* Return the default config options for a given defaults mode
7076
*/
71-
public static AttributeMap defaultConfig(DefaultsMode mode) {
72-
switch (mode) {
73-
case STANDARD:
74-
return STANDARD_DEFAULTS;
75-
case MOBILE:
76-
return MOBILE_DEFAULTS;
77-
case CROSS_REGION:
78-
return CROSS_REGION_DEFAULTS;
79-
case IN_REGION:
80-
return IN_REGION_DEFAULTS;
81-
case LEGACY:
82-
return LEGACY_DEFAULTS;
83-
default:
84-
throw new IllegalArgumentException("Unsupported mode: " + mode);
85-
}
77+
public static AttributeMap defaultHttpConfig(DefaultsMode mode) {
78+
return DEFAULT_HTTP_CONFIG_BY_MODE.getOrDefault(mode, AttributeMap.empty());
8679
}
8780
}

‎core/aws-core/src/main/java/software/amazon/awssdk/awscore/client/builder/AwsDefaultClientBuilder.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@
5050
import software.amazon.awssdk.regions.providers.DefaultAwsRegionProviderChain;
5151
import software.amazon.awssdk.utils.AttributeMap;
5252
import software.amazon.awssdk.utils.CollectionUtils;
53+
import software.amazon.awssdk.utils.Logger;
5354

5455
/**
5556
* An SDK-internal implementation of the methods in {@link AwsClientBuilder}, {@link AwsAsyncClientBuilder} and
@@ -73,6 +74,7 @@
7374
public abstract class AwsDefaultClientBuilder<BuilderT extends AwsClientBuilder<BuilderT, ClientT>, ClientT>
7475
extends SdkDefaultClientBuilder<BuilderT, ClientT>
7576
implements AwsClientBuilder<BuilderT, ClientT> {
77+
private static final Logger log = Logger.loggerFor(AwsClientBuilder.class);
7678
private static final String DEFAULT_ENDPOINT_PROTOCOL = "https";
7779
private final AutoDefaultsModeDiscovery autoDefaultsModeDiscovery;
7880

@@ -253,8 +255,10 @@ private DefaultsMode resolveDefaultsMode(SdkClientConfiguration config) {
253255
.profileName(config.option(SdkClientOption.PROFILE_NAME))
254256
.resolve();
255257

256-
if (defaultsMode.equals(DefaultsMode.AUTO)) {
258+
if (defaultsMode == DefaultsMode.AUTO) {
257259
defaultsMode = autoDefaultsModeDiscovery.discover(config.option(AwsClientOption.AWS_REGION));
260+
DefaultsMode finalDefaultsMode = defaultsMode;
261+
log.debug(() -> "The resolved defaults mode is: " + finalDefaultsMode);
258262
}
259263

260264
return defaultsMode;

‎core/aws-core/src/test/java/software/amazon/awssdk/awscore/client/builder/DefaultsModeTest.java

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@
3333
import software.amazon.awssdk.awscore.internal.defaultsmode.AutoDefaultsModeDiscovery;
3434
import software.amazon.awssdk.core.client.config.ClientOverrideConfiguration;
3535
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
36-
import software.amazon.awssdk.core.client.config.SdkClientOption;
3736
import software.amazon.awssdk.core.retry.RetryMode;
3837
import software.amazon.awssdk.defaultsmode.DefaultsMode;
3938
import software.amazon.awssdk.http.SdkHttpClient;
@@ -75,7 +74,7 @@ public void defaultClient_shouldUseLegacyModeWithExistingDefaults() {
7574
.build();
7675

7776
assertThat(client.clientConfiguration.option(DEFAULTS_MODE)).isEqualTo(DefaultsMode.LEGACY);
78-
assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(client.clientConfiguration.option(DEFAULT_RETRY_MODE));
77+
assertThat(client.clientConfiguration.option(RETRY_POLICY).retryMode()).isEqualTo(RetryMode.defaultRetryMode());
7978
}
8079

8180
@Test
@@ -198,13 +197,6 @@ protected String serviceName() {
198197
return SERVICE_NAME;
199198
}
200199

201-
@Override
202-
protected final SdkClientConfiguration mergeInternalDefaults(SdkClientConfiguration config) {
203-
return config.merge(c -> {
204-
c.option(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.ADAPTIVE);
205-
});
206-
}
207-
208200
@Override
209201
protected AttributeMap serviceHttpConfig() {
210202
return SERVICE_DEFAULTS;
@@ -238,13 +230,6 @@ protected String serviceName() {
238230
return SERVICE_NAME;
239231
}
240232

241-
@Override
242-
protected final SdkClientConfiguration mergeInternalDefaults(SdkClientConfiguration config) {
243-
return config.merge(c -> {
244-
c.option(SdkClientOption.DEFAULT_RETRY_MODE, RetryMode.ADAPTIVE);
245-
});
246-
}
247-
248233
@Override
249234
protected AttributeMap serviceHttpConfig() {
250235
return SERVICE_DEFAULTS;

0 commit comments

Comments
 (0)