Skip to content

Commit 35c5f89

Browse files
committed
Added a customization to allow underscores in shape names.
1. Throw an exception if an invalid shape name is used instead of trying to transform it. This should be backwards-compatible, because our transformation is currently broken. 2. Throw an exception if a customer-visible identifier contains underscores. Added customizations to rename any existing shapes that violate this rule. 3. Add validation to the code generator to verify that certain names will actually compile. This provides a more useful error message when services contain invalid identifiers. 4. Deprecate SsoOidc's "error_description" getters for ones with idiomatic names.
1 parent 19a6879 commit 35c5f89

File tree

26 files changed

+601
-89
lines changed

26 files changed

+601
-89
lines changed
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "deprecation",
3+
"category": "AWS SSO OIDC",
4+
"contributor": "",
5+
"description": "Renamed/deprecated 'error_description' fields in exceptions in favor of 'errorDescription'."
6+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "feature",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"description": "Added code generation validation that customer-visible identifiers are idiomatic (do not contain underscores). Services with underscores in their models can use rename customizations to fix these issues, or apply the 'underscoresInNameBehavior = ALLOW' customization."
6+
}

codegen/src/main/java/software/amazon/awssdk/codegen/AddModelShapes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ private Map<String, ShapeModel> constructModelShapes(Set<String> shapesToSkip) {
5353
ShapeType shapeType = getModelShapeType(shape);
5454

5555
if (shapeType != null) {
56-
String javaClassName = getNamingStrategy().getJavaClassName(shapeName);
56+
String javaClassName = getNamingStrategy().getShapeClassName(shapeName);
5757
if (shapesToSkip.contains(javaClassName)) {
5858
continue;
5959
}

codegen/src/main/java/software/amazon/awssdk/codegen/AddShapes.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ private MemberModel generateMemberModel(String c2jMemberName, Member c2jMemberDe
159159
variableType + " type.");
160160
}
161161

162-
163162
MemberModel memberModel = new MemberModel();
164163

165164
memberModel.withC2jName(c2jMemberName)
@@ -443,7 +442,7 @@ private void fillContainerTypeMemberMetadata(Map<String, Shape> c2jShapes,
443442
mapValueModel));
444443

445444
} else if (memberC2jShape.getEnumValues() != null) { // enum values
446-
memberModel.withEnumType(getNamingStrategy().getJavaClassName(memberC2jShapeName));
445+
memberModel.withEnumType(getNamingStrategy().getShapeClassName(memberC2jShapeName));
447446
}
448447
}
449448

codegen/src/main/java/software/amazon/awssdk/codegen/IntermediateModelBuilder.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ public IntermediateModel build() {
157157

158158
setSimpleMethods(trimmedModel);
159159

160+
namingStrategy.validateCustomerVisibleNaming(trimmedModel);
161+
160162
return trimmedModel;
161163
}
162164

codegen/src/main/java/software/amazon/awssdk/codegen/internal/TypeUtils.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ public String getJavaDataType(Map<String, Shape> shapes, String shapeName,
133133
String shapeType = shape.getType();
134134

135135
if (Structure.getName().equals(shapeType)) {
136-
return namingStrategy.getJavaClassName(shapeName);
136+
return namingStrategy.getShapeClassName(shapeName);
137137
} else if (List.getName().equals(shapeType)) {
138138
String listContainerType = DATA_TYPE_MAPPINGS.get(TypeKey.LIST_INTERFACE);
139139
return listContainerType + "<" +

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,13 +165,19 @@ public class CustomizationConfig {
165165
* Arnable fields used in s3 control
166166
*/
167167
private Map<String, S3ArnableFieldConfig> s3ArnableFields;
168+
168169
/**
169170
* Allow a customer to set an endpoint override AND bypass endpoint discovery on their client even when endpoint discovery
170171
* enabled is true and endpoint discovery is required for an operation. This customization should almost never be "true"
171172
* because it creates a confusing customer experience.
172173
*/
173174
private boolean allowEndpointOverrideForEndpointDiscoveryRequiredOperations = false;
174175

176+
/**
177+
* How the code generator should behave when it encounters shapes with underscores in the name.
178+
*/
179+
private UnderscoresInNameBehavior underscoresInNameBehavior;
180+
175181
private CustomizationConfig() {
176182
}
177183

@@ -434,4 +440,17 @@ public void setAllowEndpointOverrideForEndpointDiscoveryRequiredOperations(
434440
this.allowEndpointOverrideForEndpointDiscoveryRequiredOperations =
435441
allowEndpointOverrideForEndpointDiscoveryRequiredOperations;
436442
}
443+
444+
public UnderscoresInNameBehavior getUnderscoresInNameBehavior() {
445+
return underscoresInNameBehavior;
446+
}
447+
448+
public void setUnderscoresInNameBehavior(UnderscoresInNameBehavior behavior) {
449+
this.underscoresInNameBehavior = behavior;
450+
}
451+
452+
public CustomizationConfig withUnderscoresInShapeNameBehavior(UnderscoresInNameBehavior behavior) {
453+
this.underscoresInNameBehavior = behavior;
454+
return this;
455+
}
437456
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.codegen.model.config.customization;
17+
18+
/**
19+
* Valid values for the {@link CustomizationConfig#setUnderscoresInNameBehavior} customization.
20+
*/
21+
public enum UnderscoresInNameBehavior {
22+
/**
23+
* Allow the underscores in names, and generating shapes with names that are non-idiomatic to the language.
24+
*/
25+
ALLOW
26+
}

codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/IntermediateModel.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import java.io.InputStream;
2121
import java.io.UncheckedIOException;
2222
import java.util.Collections;
23+
import java.util.HashMap;
2324
import java.util.List;
2425
import java.util.Map;
2526
import java.util.Optional;
@@ -57,9 +58,11 @@ public final class IntermediateModel {
5758
}
5859

5960
public IntermediateModel() {
61+
this.operations = new HashMap<>();
62+
this.shapes = new HashMap<>();
6063
this.endpointOperation = Optional.empty();
61-
this.paginators = Collections.emptyMap();
62-
this.waiters = Collections.emptyMap();
64+
this.paginators = new HashMap<>();
65+
this.waiters = new HashMap<>();
6366
this.namingStrategy = null;
6467
}
6568

codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -220,22 +220,6 @@ public MemberModel withBeanStyleSetterMethodName(String beanStyleSetterName) {
220220
return this;
221221
}
222222

223-
// TODO: Remove when all marshallers switch over to new style
224-
public String getSetterMethodName() {
225-
return getBeanStyleSetterMethodName();
226-
}
227-
228-
// TODO: Remove when all marshallers switch over to new style
229-
public void setSetterMethodName(String setterMethodName) {
230-
setBeanStyleGetterMethodName(setterMethodName);
231-
}
232-
233-
// TODO: Remove when all marshallers switch over to new style
234-
public MemberModel withSetterMethodName(String setterMethodName) {
235-
setSetterMethodName(setterMethodName);
236-
return this;
237-
}
238-
239223
public String getFluentSetterMethodName() {
240224
return fluentSetterMethodName;
241225
}

codegen/src/main/java/software/amazon/awssdk/codegen/model/service/Shape.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
import java.util.Map;
2121

2222
public class Shape {
23-
2423
private String type;
2524

2625
private Map<String, Member> members = Collections.emptyMap();

codegen/src/main/java/software/amazon/awssdk/codegen/naming/DefaultNamingStrategy.java

Lines changed: 70 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,15 +32,20 @@
3232
import java.util.Objects;
3333
import java.util.Optional;
3434
import java.util.Set;
35+
import java.util.regex.Pattern;
3536
import java.util.stream.Stream;
3637
import software.amazon.awssdk.codegen.internal.Constant;
3738
import software.amazon.awssdk.codegen.internal.Utils;
3839
import software.amazon.awssdk.codegen.model.config.customization.CustomizationConfig;
40+
import software.amazon.awssdk.codegen.model.config.customization.UnderscoresInNameBehavior;
41+
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
3942
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
43+
import software.amazon.awssdk.codegen.model.intermediate.Metadata;
4044
import software.amazon.awssdk.codegen.model.service.ServiceModel;
4145
import software.amazon.awssdk.codegen.model.service.Shape;
4246
import software.amazon.awssdk.utils.Logger;
4347
import software.amazon.awssdk.utils.StringUtils;
48+
import software.amazon.awssdk.utils.Validate;
4449

4550
/**
4651
* Default implementation of naming strategy respecting.
@@ -49,6 +54,9 @@ public class DefaultNamingStrategy implements NamingStrategy {
4954

5055
private static final Logger log = Logger.loggerFor(DefaultNamingStrategy.class);
5156

57+
private static final Pattern VALID_IDENTIFIER_NAME =
58+
Pattern.compile("\\p{javaJavaIdentifierStart}\\p{javaJavaIdentifierPart}*");
59+
5260
private static final String COLLISION_DISAMBIGUATION_PREFIX = "Default";
5361

5462
private static final Set<String> RESERVED_KEYWORDS;
@@ -96,7 +104,7 @@ public class DefaultNamingStrategy implements NamingStrategy {
96104
public DefaultNamingStrategy(ServiceModel serviceModel,
97105
CustomizationConfig customizationConfig) {
98106
this.serviceModel = serviceModel;
99-
this.customizationConfig = customizationConfig;
107+
this.customizationConfig = customizationConfig == null ? CustomizationConfig.create() : customizationConfig;
100108
}
101109

102110
private static boolean isJavaKeyword(String word) {
@@ -178,7 +186,6 @@ public String getSmokeTestPackageName(String serviceName) {
178186
* if provided or service name with the shared service name.
179187
*/
180188
private String concatServiceNameIfShareModel(String serviceName) {
181-
182189
if (customizationConfig.getShareModelConfig() != null) {
183190
return customizationConfig.getShareModelConfig().getShareModelWith() + "." +
184191
Optional.ofNullable(customizationConfig.getShareModelConfig().getPackageName()).orElse(serviceName);
@@ -268,11 +275,8 @@ public String getEnumValueName(String enumValue) {
268275
}
269276

270277
@Override
271-
public String getJavaClassName(String shapeName) {
272-
return Arrays.stream(shapeName.split("[._-]|\\W"))
273-
.filter(s -> !StringUtils.isEmpty(s))
274-
.map(Utils::capitalize)
275-
.collect(joining());
278+
public String getShapeClassName(String shapeName) {
279+
return Utils.capitalize(shapeName);
276280
}
277281

278282
@Override
@@ -374,4 +378,63 @@ private boolean isDisallowedNameForShape(String name, Shape parentShape) {
374378
return RESERVED_STRUCTURE_METHOD_NAMES.contains(name);
375379
}
376380
}
381+
382+
@Override
383+
public void validateCustomerVisibleNaming(IntermediateModel trimmedModel) {
384+
Metadata metadata = trimmedModel.getMetadata();
385+
validateCustomerVisibleName(metadata.getSyncInterface(), "metadata-derived interface name");
386+
validateCustomerVisibleName(metadata.getSyncBuilderInterface(), "metadata-derived builder interface name");
387+
validateCustomerVisibleName(metadata.getAsyncInterface(), "metadata-derived async interface name");
388+
validateCustomerVisibleName(metadata.getAsyncBuilderInterface(), "metadata-derived async builder interface name");
389+
validateCustomerVisibleName(metadata.getBaseBuilderInterface(), "metadata-derived builder interface name");
390+
validateCustomerVisibleName(metadata.getBaseExceptionName(), "metadata-derived exception name");
391+
validateCustomerVisibleName(metadata.getBaseRequestName(), "metadata-derived request name");
392+
validateCustomerVisibleName(metadata.getBaseResponseName(), "metadata-derived response name");
393+
394+
trimmedModel.getOperations().values().forEach(operation -> {
395+
validateCustomerVisibleName(operation.getOperationName(), "operations");
396+
});
397+
398+
trimmedModel.getWaiters().forEach((name, waiter) -> {
399+
validateCustomerVisibleName(name, "waiters");
400+
});
401+
402+
trimmedModel.getShapes().values().forEach(shape -> {
403+
String shapeName = shape.getShapeName();
404+
validateCustomerVisibleName(shapeName, "shapes");
405+
shape.getMembers().forEach(member -> {
406+
validateCustomerVisibleName(member.getFluentGetterMethodName(), shapeName + " shape");
407+
validateCustomerVisibleName(member.getFluentSetterMethodName(), shapeName + " shape");
408+
validateCustomerVisibleName(member.getFluentEnumGetterMethodName(), shapeName + " shape");
409+
validateCustomerVisibleName(member.getFluentEnumSetterMethodName(), shapeName + " shape");
410+
validateCustomerVisibleName(member.getExistenceCheckMethodName(), shapeName + " shape");
411+
validateCustomerVisibleName(member.getBeanStyleGetterMethodName(), shapeName + " shape");
412+
validateCustomerVisibleName(member.getBeanStyleSetterMethodName(), shapeName + " shape");
413+
validateCustomerVisibleName(member.getEnumType(), shapeName + " shape");
414+
});
415+
});
416+
}
417+
418+
private void validateCustomerVisibleName(String name, String location) {
419+
if (name == null) {
420+
return;
421+
}
422+
423+
if (name.contains("_")) {
424+
UnderscoresInNameBehavior behavior = customizationConfig.getUnderscoresInNameBehavior();
425+
426+
String supportedBehaviors = Arrays.toString(UnderscoresInNameBehavior.values());
427+
Validate.notNull(behavior,
428+
"Encountered a name or identifier that the customer will see (%s in the %s) with an underscore. "
429+
+ "This isn't idiomatic in Java. Please either remove the underscores or apply the "
430+
+ "'underscoresInNameBehavior' customization for this service (Supported "
431+
+ "'underscoresInNameBehavior' values: %s).", name, location, supportedBehaviors);
432+
Validate.isTrue(behavior == UnderscoresInNameBehavior.ALLOW,
433+
"Unsupported underscoresInShapeNameBehavior: %s. Supported values: %s", behavior, supportedBehaviors);
434+
}
435+
436+
Validate.isTrue(VALID_IDENTIFIER_NAME.matcher(name).matches(),
437+
"Encountered a name or identifier that is invalid within Java (%s in %s). Please remove invalid "
438+
+ "characters.", name, location);
439+
}
377440
}

codegen/src/main/java/software/amazon/awssdk/codegen/naming/NamingStrategy.java

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
package software.amazon.awssdk.codegen.naming;
1717

18+
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
1819
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
1920
import software.amazon.awssdk.codegen.model.service.Shape;
2021

@@ -97,7 +98,7 @@ public interface NamingStrategy {
9798
* @param shapeName Name of structure used to derive Java class name.
9899
* @return Appropriate name to use for a Java class for an arbitrary (not a request, response, error) structure.
99100
*/
100-
String getJavaClassName(String shapeName);
101+
String getShapeClassName(String shapeName);
101102

102103
/**
103104
* @param memberName Member name to name getter for.
@@ -154,4 +155,9 @@ public interface NamingStrategy {
154155
* @return Name of an existence check method.
155156
*/
156157
String getExistenceCheckMethodName(String memberName, Shape parentShape);
158+
159+
/**
160+
* Verify the customer-visible naming in the provided intermediate model will compile and is idiomatic to Java.
161+
*/
162+
void validateCustomerVisibleNaming(IntermediateModel trimmedModel);
157163
}

0 commit comments

Comments
 (0)