Rewrote generated copiers to support builders for arbitrarily nested containers. #2423
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
millems
force-pushed
the
millem/complex-types-serialization-fix-2
branch
5 times, most recently
from
82bbd76 to
33e509f
Compare
millems
commented
Apr 20, 2021
Comment on lines
+515
to
+519
| <exclude>*.SubscribeToShardEvent$BuilderImpl</exclude> | ||
| <exclude>*.ConfigurationEvent$BuilderImpl</exclude> | ||
| <exclude>*.IntentResultEvent$BuilderImpl</exclude> | ||
| <exclude>*.TextResponseEvent$BuilderImpl</exclude> | ||
| <exclude>*.IntentResultEvent$BuilderImpl</exclude> |
There was a problem hiding this comment.
Bean-style getter return type had to be changed.
cenedhryn
reviewed
Apr 21, 2021
Comment on lines
-173
to
-226
| if (memberModel.hasBuilder()) { | ||
| TypeName builderName = poetExtensions.getModelClass(memberModel.getC2jShape()).nestedClass("BuilderImpl"); | ||
| return ParameterSpec.builder(builderName, fieldName()).build(); | ||
| } | ||
|
|
||
| if (memberModel.isList()) { | ||
| if (memberModel.isCollectionWithNestedBuilderMember()) { | ||
| MapModel nestedMapModel = memberModel.getListModel().getListMemberModel().getMapModel(); | ||
| TypeName nestedMapKeyType = typeProvider.getTypeNameForSimpleType(nestedMapModel.getKeyModel() | ||
| .getVariable() | ||
| .getVariableType()); | ||
| ClassName nestedMapValueType = poetExtensions.getModelClass(nestedMapModel.getValueModel().getC2jShape()); | ||
| TypeName nestedMapReturnType = ParameterizedTypeName.get(ClassName.get(Map.class), | ||
| nestedMapKeyType, | ||
| nestedMapValueType.nestedClass("BuilderImpl")); | ||
| TypeName listType = ParameterizedTypeName.get(ClassName.get(Collection.class), nestedMapReturnType); | ||
| return ParameterSpec.builder(listType, fieldName()).build(); | ||
| } | ||
|
|
||
| MemberModel listMember = memberModel.getListModel().getListMemberModel(); | ||
|
|
||
| if (hasBuilder(listMember)) { | ||
| TypeName memberName = poetExtensions.getModelClass(listMember.getC2jShape()).nestedClass("BuilderImpl"); | ||
| TypeName listType = ParameterizedTypeName.get(ClassName.get(Collection.class), memberName); | ||
| return ParameterSpec.builder(listType, fieldName()).build(); | ||
| } else if (listMember.isSdkBytesType()) { | ||
| TypeName listType = ParameterizedTypeName.get(Collection.class, ByteBuffer.class); | ||
| return ParameterSpec.builder(listType, fieldName()).build(); | ||
| } | ||
| } | ||
|
|
||
| if (memberModel.isMap()) { | ||
| MemberModel keyModel = memberModel.getMapModel().getKeyModel(); | ||
| TypeName keyType = typeProvider.getTypeNameForSimpleType(keyModel.getVariable().getVariableType()); | ||
| MemberModel valueModel = memberModel.getMapModel().getValueModel(); | ||
| TypeName valueType = null; | ||
|
|
||
| if (hasBuilder(valueModel)) { | ||
| valueType = poetExtensions.getModelClass(valueModel.getC2jShape()).nestedClass("BuilderImpl"); | ||
| } else if (valueModel.isSdkBytesType()) { | ||
| valueType = TypeName.get(ByteBuffer.class); | ||
| } | ||
|
|
||
| if (valueType != null) { | ||
| TypeName mapType = ParameterizedTypeName.get(ClassName.get(Map.class), keyType, valueType); | ||
| return ParameterSpec.builder(mapType, fieldName()).build(); | ||
| } | ||
| } | ||
|
|
||
| if (memberModel.isSdkBytesType()) { | ||
| return ParameterSpec.builder(ByteBuffer.class, fieldName()).build(); | ||
| } | ||
|
|
||
| return memberAsParameter(); |
There was a problem hiding this comment.
Thanks for removing this!
| @@ -482,4 +350,17 @@ private String memberParamName() { | |||
| } | |||
| return Utils.unCapitalize(memberModel.getC2jShape()) + "Param"; | |||
| } | |||
|
|
|||
| private static final class VariableSource { | |||
There was a problem hiding this comment.
This class name is too generic for me, I'd prefer if it was renamed to be more specific to its task.
IDK if I love disambiguator but it's the only think I can think of now :-)
There was a problem hiding this comment.
I can't think of a better name. It provides unique variable names... UniqueVariableSource? VariableUniquifier?
There was a problem hiding this comment.
UniqueVariableSource is better than VariableSource
| NONE | ||
| } | ||
|
|
||
| public static final class TypeNameOptions { |
There was a problem hiding this comment.
This class makes it much easier to reason around how to build types
…containers. This fixes an issue where DynamoDB types like BatchGetResponse.Builder could not be serialized using a bean-based serializer, because it contained a Map<K, List<Map<K2, V>>.
millems
force-pushed
the
millem/complex-types-serialization-fix-2
branch
from
b352025 to
0dfee39
Compare
cenedhryn
approved these changes
Apr 21, 2021
|
Kudos, SonarCloud Quality Gate passed! |
aws-sdk-java-automation
added a commit
that referenced
this pull request
Mar 17, 2023
…ed10593a8 Pull request: release <- staging/4849877e-99ce-403a-ad32-ac2ed10593a8
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes an issue where DynamoDB types like BatchGetResponse.Builder could not be serialized using
a bean-based serializer, because it contained a Map<K, List<Map<K2, V>>.