Skip to content

Commit 5c5b496

Browse files
alex-chewalex-chew
authored
fix: AWS SDK error codegen, part 2 (#23)
1 parent e66ed80 commit 5c5b496

File tree

11 files changed

+172
-59
lines changed

11 files changed

+172
-59
lines changed

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydafny/DafnyNameResolver.java

+1-1
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,7 @@ public String traitForResource(final ResourceShape resourceShape) {
108108
}
109109

110110
public String traitForServiceError(final ServiceShape serviceShape) {
111-
return "%sError".formatted(nameForService(serviceShape));
111+
return "I%sException".formatted(nameForService(serviceShape));
112112
}
113113

114114
public String classForSpecificError(final StructureShape structureShape) {

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkDotNetNameResolver.java

+17-7
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ private boolean isGeneratedInSdk(final ShapeId shapeId) {
2929
@Override
3030
protected String baseTypeForString(final StringShape stringShape) {
3131
if (isGeneratedInSdk(stringShape.getId()) && stringShape.hasTrait(EnumTrait.class)) {
32-
return "%s.%s".formatted(getSdkServiceNamespace(), classForEnum(stringShape.getId()));
32+
return "%s.%s".formatted(namespaceForService(), classForEnum(stringShape.getId()));
3333
}
3434

3535
return super.baseTypeForString(stringShape);
@@ -54,7 +54,7 @@ protected String baseTypeForStructure(final StructureShape structureShape) {
5454
throw new IllegalArgumentException("Trait definition structures have no corresponding generated type");
5555
}
5656

57-
return "%s.Model.%s".formatted(getSdkServiceNamespace(), structureShape.getId().getName());
57+
return "%s.Model.%s".formatted(namespaceForService(), structureShape.getId().getName());
5858
}
5959

6060
return super.baseTypeForStructure(structureShape);
@@ -63,29 +63,39 @@ protected String baseTypeForStructure(final StructureShape structureShape) {
6363
@Override
6464
protected String baseTypeForService(final ServiceShape serviceShape) {
6565
if (isGeneratedInSdk(serviceShape.getId())) {
66-
return "%s.IAmazon%s".formatted(getSdkServiceNamespace(), getServiceName());
66+
return "%s.IAmazon%s".formatted(namespaceForService(), getServiceName());
6767
}
6868

6969
return super.baseTypeForService(serviceShape);
7070
}
7171

7272
public String implForServiceClient() {
73-
return "%s.Amazon%sClient".formatted(getSdkServiceNamespace(), getServiceName());
73+
return "%s.Amazon%sClient".formatted(namespaceForService(), getServiceName());
7474
}
7575

7676
private String getServiceName() {
7777
return StringUtils.capitalize(getServiceShape().getId().getName());
7878
}
7979

80-
private String getSdkServiceNamespace() {
80+
@Override
81+
public String namespaceForService() {
8182
return "Amazon.%s".formatted(getServiceName());
8283
}
8384

85+
public String syntheticNamespaceForService() {
86+
return super.namespaceForService();
87+
}
88+
8489
public String shimClassForService() {
8590
return "%sShim".formatted(getServiceName());
8691
}
8792

88-
public String baseExceptionForService() {
89-
return "%s.Amazon%sException".formatted(getSdkServiceNamespace(), getServiceName());
93+
@Override
94+
public String classForBaseServiceException() {
95+
return "Amazon%sException".formatted(getServiceName());
96+
}
97+
98+
public String qualifiedClassForBaseServiceException() {
99+
return "%s.%s".formatted(namespaceForService(), classForBaseServiceException());
90100
}
91101
}

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkShimCodegen.java

+7-4
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ public TokenTree generateServiceShim() {
6969
final TokenTree classBody = TokenTree.of(impl, constructor, operationShims, errorTypeShim).lineSeparated();
7070
return header
7171
.append(classBody.braced())
72-
.namespaced(Token.of(nameResolver.namespaceForService()));
72+
.namespaced(Token.of(nameResolver.syntheticNamespaceForService()));
7373
}
7474

7575
public TokenTree generateServiceShimConstructor() {
@@ -109,7 +109,7 @@ public TokenTree generateOperationShim(final ShapeId operationShapeId) {
109109
final TokenTree tryBody = TokenTree.of(assignSdkResponse, callImpl, returnResponse).lineSeparated();
110110
final TokenTree tryBlock = Token.of("try").append(tryBody.braced());
111111

112-
final String baseExceptionForService = nameResolver.baseExceptionForService();
112+
final String baseExceptionForService = nameResolver.qualifiedClassForBaseServiceException();
113113
final TokenTree catchBlock = Token.of("""
114114
catch (System.AggregateException aggregate) when (aggregate.InnerException is %s ex) {
115115
return %s.create_Failure(this.%s(ex));
@@ -165,8 +165,11 @@ public TokenTree generateErrorTypeShim() {
165165
};
166166
""".formatted(dafnyUnknownErrorType, stringConverter));
167167

168-
final TokenTree signature = Token.of("private %s %s(%s error)".formatted(
169-
dafnyErrorAbstractType, CONVERT_ERROR_METHOD, nameResolver.baseExceptionForService()));
168+
final TokenTree signature = Token.of("private %s %s(%s.%s error)".formatted(
169+
dafnyErrorAbstractType,
170+
CONVERT_ERROR_METHOD,
171+
nameResolver.namespaceForService(),
172+
nameResolver.classForBaseServiceException()));
170173
final TokenTree cases = TokenTree.of(knownErrorCases, unknownErrorCase).lineSeparated();
171174
final TokenTree body = Token.of("switch (error)").append(cases.braced());
172175
return signature.append(body.braced());

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/AwsSdkTypeConversionCodegen.java

+32-1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818

1919
import java.util.List;
2020
import java.util.Set;
21+
import java.util.stream.Stream;
2122

2223
import static software.amazon.polymorph.smithydotnet.TypeConversionDirection.FROM_DAFNY;
2324
import static software.amazon.polymorph.smithydotnet.TypeConversionDirection.TO_DAFNY;
@@ -50,6 +51,23 @@ public TypeConverter generateStructureConverter(final StructureShape structureSh
5051
return super.generateStructureConverter(structureShape);
5152
}
5253

54+
/**
55+
* We can't call the {@code IsSet} methods on AWS SDK classes' member properties because they're internal.
56+
* The best we can do is to call the properties' getters, which calls {@code GetValueOrDefault}, which in turn may
57+
* improperly coalesce absent optional values to 0 (for example).
58+
*/
59+
@Override
60+
public TokenTree generateExtractOptionalMember(MemberShape memberShape) {
61+
final String type = nameResolver.baseTypeForShape(memberShape.getId());
62+
final String varName = nameResolver.variableNameForClassProperty(memberShape);
63+
final String propertyName = nameResolver.classPropertyForStructureMember(memberShape);
64+
return TokenTree.of(
65+
type,
66+
varName,
67+
"= value.%s;".formatted(propertyName)
68+
);
69+
}
70+
5371
@Override
5472
protected boolean enumListMembersAreStringsInCSharp() {
5573
return true;
@@ -78,7 +96,7 @@ private TypeConverter generateErrorStructureConverter(final StructureShape struc
7896
%s message = System.String.IsNullOrEmpty(value.Message)
7997
? Dafny.Sequence<char>.Empty
8098
: %s(value.Message);
81-
return new %s(message);
99+
return new %s { message = message };
82100
""".formatted(
83101
nameResolver.dafnyTypeForShape(messageTarget.getId()),
84102
AwsSdkDotNetNameResolver.typeConverterForShape(messageTarget.getId(), TO_DAFNY),
@@ -106,4 +124,17 @@ TypeConverter generateAwsSdkServiceReferenceStructureConverter(final StructureSh
106124

107125
return buildConverterFromMethodBodies(structureShape, fromDafnyBody, toDafnyBody);
108126
}
127+
128+
@Override
129+
protected String getTypeConversionNamespace() {
130+
return ((AwsSdkDotNetNameResolver)nameResolver).syntheticNamespaceForService();
131+
}
132+
133+
/**
134+
* No unmodeled converters are needed for the AWS SDK shims.
135+
*/
136+
@Override
137+
protected Stream<TypeConverter> generateUnmodeledConverters() {
138+
return Stream.empty();
139+
}
109140
}

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/DotNetNameResolver.java

+7-10
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ public String interfaceForService(final ShapeId serviceShapeId) {
152152
* Returns the name for the service's base exception class. This exception class does not appear in the model, but
153153
* instead serves as the parent class of all modeled (concrete) exception classes.
154154
*/
155-
public static String classForBaseServiceException(final ServiceShape serviceShape) {
155+
public String classForBaseServiceException(final ServiceShape serviceShape) {
156156
// TODO Currently we have this hardcoded to remove 'Factory' from service names
157157
// that include it, however this should likely be controlled via a custom trait
158158
final String serviceName = ModelUtils.serviceNameWithoutTrailingFactory(serviceShape);
@@ -172,7 +172,7 @@ public static String classForConcreteServiceException(final ServiceShape service
172172
}
173173

174174
public String classForBaseServiceException() {
175-
return DotNetNameResolver.classForBaseServiceException(serviceShape);
175+
return classForBaseServiceException(serviceShape);
176176
}
177177

178178
public String classForSpecificServiceException(final ShapeId structureShapeId) {
@@ -476,20 +476,20 @@ public static String qualifiedTypeConverter(final ShapeId shapeId, final TypeCon
476476
/**
477477
* Returns the type converter method name for the given service's common error shape and the given direction.
478478
*/
479-
public static String typeConverterForCommonError(final ServiceShape serviceShape, final TypeConversionDirection direction) {
479+
public String typeConverterForCommonError(final ServiceShape serviceShape, final TypeConversionDirection direction) {
480480
return switch (direction) {
481481
case TO_DAFNY -> "%s_CommonError".formatted(direction.toString());
482482
case FROM_DAFNY -> "%s_CommonError_%s".formatted(
483-
direction.toString(), DotNetNameResolver.classForBaseServiceException(serviceShape));
483+
direction.toString(), classForBaseServiceException(serviceShape));
484484
};
485485
}
486486

487487
/**
488488
* Like {@link DotNetNameResolver#typeConverterForCommonError(ServiceShape, TypeConversionDirection)}, but
489489
* qualified with the type conversion class name.
490490
*/
491-
public static String qualifiedTypeConverterForCommonError(final ServiceShape serviceShape, final TypeConversionDirection direction) {
492-
final String methodName = DotNetNameResolver.typeConverterForCommonError(serviceShape, direction);
491+
public String qualifiedTypeConverterForCommonError(final ServiceShape serviceShape, final TypeConversionDirection direction) {
492+
final String methodName = typeConverterForCommonError(serviceShape, direction);
493493
return "%s.%s".formatted(DotNetNameResolver.TYPE_CONVERSION_CLASS_NAME, methodName);
494494
}
495495

@@ -676,10 +676,7 @@ public String serviceNameWithoutFactory() {
676676
public static String dafnyTypeForCommonServiceError(final ServiceShape serviceShape) {
677677
// TODO Currently we have this hardcoded to remove 'Factory' from service names
678678
// that include it, however this should likely be controlled via a custom trait
679-
String serviceName = serviceShape.getId().getName(serviceShape);
680-
if (serviceName.endsWith("Factory")) {
681-
serviceName = serviceName.substring(0, serviceName.lastIndexOf("Factory"));
682-
}
679+
final String serviceName = ModelUtils.serviceNameWithoutTrailingFactory(serviceShape);
683680

684681
// TODO this should really end with "error"...
685682
return "%s.I%sException".formatted(

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/ShimCodegen.java

+6-1
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ public TokenTree generateCheckAndConvertFailure() {
154154
return Token.of("if (%s.is_Failure) throw %s(%s.dtor_error);"
155155
.formatted(
156156
RESULT_NAME,
157-
DotNetNameResolver.qualifiedTypeConverterForCommonError(serviceShape, FROM_DAFNY),
157+
nameResolver.qualifiedTypeConverterForCommonError(serviceShape, FROM_DAFNY),
158158
RESULT_NAME
159159
));
160160
}
@@ -235,4 +235,9 @@ public TokenTree generateOperationShim(final ShapeId operationShapeId) {
235235
public Model getModel() {
236236
return model;
237237
}
238+
239+
@VisibleForTesting
240+
public DotNetNameResolver getNameResolver() {
241+
return nameResolver;
242+
}
238243
}

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/TypeConversionCodegen.java

+24-8
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@
4747
import java.util.stream.Stream;
4848

4949
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.TYPE_CONVERSION_CLASS_NAME;
50-
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.typeConverterForCommonError;
5150
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.typeConverterForShape;
5251
import static software.amazon.polymorph.smithydotnet.TypeConversionDirection.FROM_DAFNY;
5352
import static software.amazon.polymorph.smithydotnet.TypeConversionDirection.TO_DAFNY;
@@ -91,18 +90,25 @@ public Map<Path, TokenTree> generate() {
9190
.stream()
9291
.map(model::expectShape)
9392
.map(this::generateConverter);
94-
final Stream<TypeConverter> unmodeledConverters = Stream.of(generateCommonExceptionConverter());
93+
final Stream<TypeConverter> unmodeledConverters = generateUnmodeledConverters();
9594
final Stream<TypeConverter> converters = Stream.concat(modeledConverters, unmodeledConverters);
9695
final TokenTree conversionClassBody = TokenTree.of(converters
9796
.flatMap(typeConverter -> Stream.of(typeConverter.fromDafny, typeConverter.toDafny)))
9897
.lineSeparated()
9998
.braced();
10099
final TokenTree conversionClass = conversionClassBody
101100
.prepend(TokenTree.of("internal static class", TYPE_CONVERSION_CLASS_NAME))
102-
.namespaced(Token.of(nameResolver.namespaceForService()));
101+
.namespaced(Token.of(getTypeConversionNamespace()));
103102
return Map.of(TYPE_CONVERSION_CLASS_PATH, conversionClass.prepend(prelude));
104103
}
105104

105+
/**
106+
* Returns a stream of type converters for synthetic types (types that aren't defined in the model).
107+
*/
108+
protected Stream<TypeConverter> generateUnmodeledConverters() {
109+
return Stream.of(generateCommonExceptionConverter());
110+
}
111+
106112
/**
107113
* Returns all shape IDs that require converters.
108114
*/
@@ -398,7 +404,7 @@ private TypeConverter generateRegularStructureConverter(final StructureShape str
398404
final TokenTree isSetTernaries = TokenTree.of(
399405
ModelUtils.streamStructureMembers(structureShape)
400406
.filter(nameResolver::memberShapeIsOptional)
401-
.map(this::generateIsSetTernary)
407+
.map(this::generateExtractOptionalMember)
402408
).lineSeparated();
403409

404410
final TokenTree constructorArgs = TokenTree.of(ModelUtils.streamStructureMembers(structureShape)
@@ -425,7 +431,7 @@ private TypeConverter generateRegularStructureConverter(final StructureShape str
425431
* OR :
426432
* "ToDafny_memberShape(propertyName)"
427433
*/
428-
public String generateConstructorArg(final MemberShape memberShape) {
434+
private String generateConstructorArg(final MemberShape memberShape) {
429435
if (nameResolver.memberShapeIsOptional(memberShape)) {
430436
return "%s(%s)".formatted(
431437
typeConverterForShape(memberShape.getId(), TO_DAFNY),
@@ -440,7 +446,7 @@ public String generateConstructorArg(final MemberShape memberShape) {
440446
* Returns:
441447
* "type varName = value.IsSetPropertyName() ? value.PropertyName : (type) null;"
442448
*/
443-
public TokenTree generateIsSetTernary(final MemberShape memberShape) {
449+
public TokenTree generateExtractOptionalMember(final MemberShape memberShape) {
444450
final String type = nameResolver.baseTypeForShape(memberShape.getId());
445451
final String varName = nameResolver.variableNameForClassProperty(memberShape);
446452
final String isSetMethod = nameResolver.isSetMethodForStructureMember(memberShape);
@@ -699,7 +705,7 @@ cSharpType, typeConverterForShape(ShapeId.from("smithy.api#String"), FROM_DAFNY)
699705
final TokenTree fromDafnyBody = TokenTree.of(
700706
TokenTree.of("switch(value)"), fromDafnySwitchCases).lineSeparated();
701707
final TokenTree fromDafnyConverterSignature = Token.of("public static %s %s(%s value)".formatted(
702-
cSharpType, typeConverterForCommonError(serviceShape, FROM_DAFNY), dafnyType));
708+
cSharpType, nameResolver.typeConverterForCommonError(serviceShape, FROM_DAFNY), dafnyType));
703709
final TokenTree fromDafnyConverterMethod = TokenTree.of(fromDafnyConverterSignature, fromDafnyBody.braced());
704710

705711
// Generate the TO_DAFNY method
@@ -735,7 +741,7 @@ cSharpType, typeConverterForShape(ShapeId.from("smithy.api#String"), FROM_DAFNY)
735741
TokenTree.of("%s rtn;\nswitch (value)\n".formatted(nameResolver.dafnyBaseTypeForServiceError())),
736742
toDafnySwitchCases);
737743
final TokenTree toDafnyConverterSignature = Token.of("public static %s %s(System.Exception value)".formatted(
738-
dafnyType, typeConverterForCommonError(serviceShape, TO_DAFNY)));
744+
dafnyType, nameResolver.typeConverterForCommonError(serviceShape, TO_DAFNY)));
739745
final TokenTree toDafnyConverterMethod = TokenTree.of(toDafnyConverterSignature, toDafnyBody.braced());
740746

741747
// The Common Exception Converter is novel to Polymorph, it is not native to smithy.
@@ -793,6 +799,16 @@ protected TypeConverter buildConverterFromMethodBodies(
793799
return new TypeConverter(shape.getId(), fromDafnyConverterMethod, toDafnyConverterMethod);
794800
}
795801

802+
/**
803+
* Returns the namespace in which to generate the type conversion class.
804+
*
805+
* Subclasses can override this in case it differs from the service's "main" namespace, e.g. in the case of AWS SDK
806+
* type conversion.
807+
*/
808+
protected String getTypeConversionNamespace() {
809+
return nameResolver.namespaceForService();
810+
}
811+
796812
@VisibleForTesting
797813
public Model getModel() {
798814
return model;

‎smithy-dotnet/src/main/java/software/amazon/polymorph/smithydotnet/nativeWrapper/NativeWrapperCodegen.java

+7-10
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,6 @@
33

44
package software.amazon.polymorph.smithydotnet.nativeWrapper;
55

6-
import java.util.List;
7-
import java.util.Optional;
8-
96
import software.amazon.polymorph.smithydotnet.DotNetNameResolver;
107
import software.amazon.polymorph.traits.PositionalTrait;
118
import software.amazon.polymorph.utils.Token;
@@ -18,10 +15,10 @@
1815
import software.amazon.smithy.model.shapes.ShapeId;
1916
import software.amazon.smithy.model.shapes.StructureShape;
2017

21-
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.classForBaseServiceException;
22-
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.classForConcreteServiceException;
23-
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.qualifiedTypeConverter;
24-
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.qualifiedTypeConverterForCommonError;
18+
import java.util.List;
19+
import java.util.Optional;
20+
21+
import static software.amazon.polymorph.smithydotnet.DotNetNameResolver.*;
2522
import static software.amazon.polymorph.smithydotnet.TypeConversionDirection.FROM_DAFNY;
2623
import static software.amazon.polymorph.smithydotnet.TypeConversionDirection.TO_DAFNY;
2724

@@ -164,7 +161,7 @@ TokenTree generateOperationWrapper(final ShapeId operationShapeId) {
164161
operationShape.getOutput(), nativeOutputType, methodName),
165162
inputConversion,
166163
TokenTree.of("%s finalException = null;"
167-
.formatted(classForBaseServiceException(serviceShape))),
164+
.formatted(nameResolver.classForBaseServiceException())),
168165
tryBlock,
169166
generateCatchServiceException(),
170167
generateCatchGeneralException(methodName),
@@ -260,7 +257,7 @@ TokenTree generateValidateNativeOutputMethod(
260257

261258
TokenTree generateCatchServiceException() {
262259
return generateCatch(
263-
classForBaseServiceException(serviceShape),
260+
nameResolver.classForBaseServiceException(),
264261
"finalException = e;"
265262
);
266263
}
@@ -289,7 +286,7 @@ TokenTree generateReturnFailure(String dafnyOutput) {
289286
return TokenTree.of("return %s.create_Failure(%s(finalException));".
290287
formatted(
291288
dafnyOutput,
292-
qualifiedTypeConverterForCommonError(serviceShape, TO_DAFNY)
289+
nameResolver.qualifiedTypeConverterForCommonError(serviceShape, TO_DAFNY)
293290
));
294291
}
295292
}

0 commit comments

Comments
 (0)