Skip to content

Commit 06d23db

Browse files
millemsmillems
committed
Rewrote generated copiers to support builders for arbitrarily nested 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>>.
1 parent 57aaf9d commit 06d23db

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+1353
-996
lines changed

‎.changes/next-release/bugfix-AmazonDynamoDB-a0ae32a.json

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"category": "Amazon DynamoDB",
3+
"contributor": "",
4+
"type": "bugfix",
5+
"description": "Fixed bean-based marshalling for model builder types containing complex collections."
6+
}

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

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Optional;
3232
import software.amazon.awssdk.codegen.internal.TypeUtils;
3333
import software.amazon.awssdk.core.SdkBytes;
34+
import software.amazon.awssdk.core.SdkField;
3435
import software.amazon.awssdk.core.protocol.MarshallingType;
3536
import software.amazon.awssdk.core.util.SdkAutoConstructList;
3637
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
@@ -615,15 +616,26 @@ public boolean hasBuilder() {
615616
}
616617

617618
@JsonIgnore
618-
public boolean isCollectionWithBuilderMember() {
619-
return (isList() && getListModel().getListMemberModel() != null && getListModel().getListMemberModel().hasBuilder()) ||
620-
(isMap() && getMapModel().getValueModel() != null && getMapModel().getValueModel().hasBuilder());
619+
public boolean containsBuildable() {
620+
return containsBuildable(true);
621621
}
622622

623-
@JsonIgnore
624-
public boolean isCollectionWithNestedBuilderMember() {
625-
return isList() && getListModel().getListMemberModel() != null && getListModel().isMap() &&
626-
getListModel().getListMemberModel().getMapModel().getValueModel().hasBuilder();
623+
private boolean containsBuildable(boolean root) {
624+
if (!root && hasBuilder()) {
625+
return true;
626+
}
627+
628+
if (isList()) {
629+
return getListModel().getListMemberModel().containsBuildable(false);
630+
}
631+
632+
if (isMap()) {
633+
MapModel mapModel = getMapModel();
634+
return mapModel.getKeyModel().containsBuildable(false) ||
635+
mapModel.getValueModel().containsBuildable(false);
636+
}
637+
638+
return false;
627639
}
628640

629641
@JsonIgnore

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AbstractMemberSetters.java

Lines changed: 10 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,21 @@
1515

1616
package software.amazon.awssdk.codegen.poet.model;
1717

18+
import static software.amazon.awssdk.codegen.poet.model.TypeProvider.ShapeTransformation.USE_BUILDER_IMPL;
19+
1820
import com.squareup.javapoet.ClassName;
1921
import com.squareup.javapoet.CodeBlock;
2022
import com.squareup.javapoet.MethodSpec;
2123
import com.squareup.javapoet.ParameterSpec;
22-
import com.squareup.javapoet.ParameterizedTypeName;
2324
import com.squareup.javapoet.TypeName;
24-
import java.nio.ByteBuffer;
25-
import java.util.Collection;
26-
import java.util.Map;
2725
import java.util.Optional;
2826
import java.util.stream.Collectors;
2927
import javax.lang.model.element.Modifier;
3028
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
31-
import software.amazon.awssdk.codegen.model.intermediate.MapModel;
3229
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
3330
import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
3431
import software.amazon.awssdk.codegen.poet.PoetExtensions;
32+
import software.amazon.awssdk.codegen.poet.model.TypeProvider.TypeNameOptions;
3533
import software.amazon.awssdk.core.SdkBytes;
3634

3735
/**
@@ -125,8 +123,8 @@ protected CodeBlock copySetterBuilderBody() {
125123
"this.$1N = $1N != null ? $1N.build() : null",
126124
serviceModelCopiers.copyMethodName());
127125
}
128-
if (memberModel.isCollectionWithBuilderMember() || memberModel.isCollectionWithNestedBuilderMember()) {
129-
return copySetterBody("this.$1N = $2T.$3N($1N)", null, serviceModelCopiers.builderCopyMethodName());
126+
if (memberModel.containsBuildable()) {
127+
return copySetterBody("this.$1N = $2T.$3N($1N)", null, serviceModelCopiers.copyFromBuilderMethodName());
130128
}
131129
return copySetterBody();
132130
}
@@ -170,60 +168,11 @@ protected ParameterSpec memberAsParameter() {
170168
}
171169

172170
protected ParameterSpec memberAsBeanStyleParameter() {
173-
if (memberModel.hasBuilder()) {
174-
TypeName builderName = poetExtensions.getModelClass(memberModel.getC2jShape()).nestedClass("BuilderImpl");
175-
return ParameterSpec.builder(builderName, fieldName()).build();
176-
}
177-
178-
if (memberModel.isList()) {
179-
if (memberModel.isCollectionWithNestedBuilderMember()) {
180-
MapModel nestedMapModel = memberModel.getListModel().getListMemberModel().getMapModel();
181-
TypeName nestedMapKeyType = typeProvider.getTypeNameForSimpleType(nestedMapModel.getKeyModel()
182-
.getVariable()
183-
.getVariableType());
184-
ClassName nestedMapValueType = poetExtensions.getModelClass(nestedMapModel.getValueModel().getC2jShape());
185-
TypeName nestedMapReturnType = ParameterizedTypeName.get(ClassName.get(Map.class),
186-
nestedMapKeyType,
187-
nestedMapValueType.nestedClass("BuilderImpl"));
188-
TypeName listType = ParameterizedTypeName.get(ClassName.get(Collection.class), nestedMapReturnType);
189-
return ParameterSpec.builder(listType, fieldName()).build();
190-
}
191-
192-
MemberModel listMember = memberModel.getListModel().getListMemberModel();
193-
194-
if (hasBuilder(listMember)) {
195-
TypeName memberName = poetExtensions.getModelClass(listMember.getC2jShape()).nestedClass("BuilderImpl");
196-
TypeName listType = ParameterizedTypeName.get(ClassName.get(Collection.class), memberName);
197-
return ParameterSpec.builder(listType, fieldName()).build();
198-
} else if (listMember.isSdkBytesType()) {
199-
TypeName listType = ParameterizedTypeName.get(Collection.class, ByteBuffer.class);
200-
return ParameterSpec.builder(listType, fieldName()).build();
201-
}
202-
}
203-
204-
if (memberModel.isMap()) {
205-
MemberModel keyModel = memberModel.getMapModel().getKeyModel();
206-
TypeName keyType = typeProvider.getTypeNameForSimpleType(keyModel.getVariable().getVariableType());
207-
MemberModel valueModel = memberModel.getMapModel().getValueModel();
208-
TypeName valueType = null;
209-
210-
if (hasBuilder(valueModel)) {
211-
valueType = poetExtensions.getModelClass(valueModel.getC2jShape()).nestedClass("BuilderImpl");
212-
} else if (valueModel.isSdkBytesType()) {
213-
valueType = TypeName.get(ByteBuffer.class);
214-
}
215-
216-
if (valueType != null) {
217-
TypeName mapType = ParameterizedTypeName.get(ClassName.get(Map.class), keyType, valueType);
218-
return ParameterSpec.builder(mapType, fieldName()).build();
219-
}
220-
}
221-
222-
if (memberModel.isSdkBytesType()) {
223-
return ParameterSpec.builder(ByteBuffer.class, fieldName()).build();
224-
}
225-
226-
return memberAsParameter();
171+
TypeName type = typeProvider.typeName(memberModel, new TypeNameOptions().shapeTransformation(USE_BUILDER_IMPL)
172+
.useSubtypeWildcardsForCollections(true)
173+
.useCollectionForList(true)
174+
.useByteBufferTypes(true));
175+
return ParameterSpec.builder(type, fieldName()).build();
227176
}
228177

229178
protected ShapeModel shapeModel() {
@@ -259,9 +208,4 @@ private CodeBlock copySetterBody(String copyAssignment, String regularAssignment
259208
.build())
260209
.orElseGet(() -> CodeBlock.builder().addStatement(regularAssignment, fieldName()).build());
261210
}
262-
263-
private boolean hasBuilder(MemberModel model) {
264-
return model != null && model.hasBuilder();
265-
}
266-
267211
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AccessorsFactory.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ class AccessorsFactory {
3939
this.shapeModel = shapeModel;
4040
this.typeProvider = typeProvider;
4141
this.intermediateModel = intermediateModel;
42-
this.getterHelper = new BeanGetterHelper(poetExtensions, typeProvider);
42+
this.getterHelper = new BeanGetterHelper(poetExtensions, new ServiceModelCopiers(intermediateModel), typeProvider);
4343
}
4444

4545
public MethodSpec beanStyleGetter(MemberModel memberModel) {

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/BeanGetterHelper.java

Lines changed: 44 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -15,42 +15,43 @@
1515

1616
package software.amazon.awssdk.codegen.poet.model;
1717

18+
import static software.amazon.awssdk.codegen.poet.model.TypeProvider.ShapeTransformation.USE_BUILDER;
19+
1820
import com.squareup.javapoet.ClassName;
1921
import com.squareup.javapoet.CodeBlock;
2022
import com.squareup.javapoet.MethodSpec;
2123
import com.squareup.javapoet.ParameterizedTypeName;
2224
import com.squareup.javapoet.TypeName;
2325
import java.nio.ByteBuffer;
24-
import java.util.Collection;
2526
import java.util.List;
2627
import java.util.Map;
2728
import java.util.stream.Collectors;
2829
import javax.lang.model.element.Modifier;
29-
import software.amazon.awssdk.codegen.model.intermediate.MapModel;
3030
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
3131
import software.amazon.awssdk.codegen.poet.PoetExtensions;
3232
import software.amazon.awssdk.codegen.poet.PoetUtils;
33+
import software.amazon.awssdk.codegen.poet.model.TypeProvider.TypeNameOptions;
3334
import software.amazon.awssdk.core.SdkBytes;
34-
import software.amazon.awssdk.utils.CollectionUtils;
35+
import software.amazon.awssdk.core.util.SdkAutoConstructList;
36+
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
3537

3638
public final class BeanGetterHelper {
3739
private final PoetExtensions poetExtensions;
40+
private final ServiceModelCopiers copiers;
3841
private final TypeProvider typeProvider;
3942

40-
public BeanGetterHelper(PoetExtensions poetExtensions, TypeProvider typeProvider) {
43+
public BeanGetterHelper(PoetExtensions poetExtensions, ServiceModelCopiers copiers, TypeProvider typeProvider) {
4144
this.poetExtensions = poetExtensions;
45+
this.copiers = copiers;
4246
this.typeProvider = typeProvider;
4347
}
4448

4549
public MethodSpec beanStyleGetter(MemberModel memberModel) {
4650
if (memberModel.hasBuilder()) {
4751
return builderGetter(memberModel);
4852
}
49-
if (memberModel.isCollectionWithBuilderMember()) {
50-
return memberModel.isList() ? listOfBuildersGetter(memberModel) : mapOfBuildersGetter(memberModel);
51-
}
52-
if (memberModel.isCollectionWithNestedBuilderMember()) {
53-
return listOfMapOfBuilderGetter(memberModel);
53+
if (memberModel.containsBuildable()) {
54+
return buildableContainerGetter(memberModel);
5455
}
5556
if (memberModel.isSdkBytesType()) {
5657
return byteBufferGetter(memberModel);
@@ -64,6 +65,40 @@ public MethodSpec beanStyleGetter(MemberModel memberModel) {
6465
return regularGetter(memberModel);
6566
}
6667

68+
private MethodSpec buildableContainerGetter(MemberModel memberModel) {
69+
TypeName returnType = typeProvider.typeName(memberModel, new TypeNameOptions().shapeTransformation(USE_BUILDER)
70+
.useByteBufferTypes(true));
71+
72+
ClassName copierClass = copiers.copierClassFor(memberModel)
73+
.orElseThrow(() -> new IllegalStateException("Copier class not found for " + memberModel));
74+
75+
String variableName = memberModel.getVariable().getVariableName();
76+
77+
CodeBlock.Builder code = CodeBlock.builder();
78+
79+
code.add("$T result = $T.$N(this.$N);", returnType, copierClass, copiers.copyToBuilderMethodName(), variableName);
80+
81+
if (memberModel.isList()) {
82+
code.add("if (result instanceof $T) {", SdkAutoConstructList.class)
83+
.add("return null;")
84+
.add("}");
85+
}
86+
87+
if (memberModel.isMap()) {
88+
code.add("if (result instanceof $T) {", SdkAutoConstructMap.class)
89+
.add("return null;")
90+
.add("}");
91+
}
92+
93+
code.add("return result;");
94+
95+
return MethodSpec.methodBuilder(memberModel.getBeanStyleGetterMethodName())
96+
.addModifiers(Modifier.PUBLIC, Modifier.FINAL)
97+
.returns(returnType)
98+
.addCode(code.build())
99+
.build();
100+
}
101+
67102
private MethodSpec byteBufferGetter(MemberModel memberModel) {
68103
return basicGetter(memberModel,
69104
ClassName.get(ByteBuffer.class),
@@ -100,54 +135,6 @@ private MethodSpec builderGetter(MemberModel memberModel) {
100135
memberModel.getVariable().getVariableName()));
101136
}
102137

103-
private MethodSpec listOfMapOfBuilderGetter(MemberModel memberModel) {
104-
MapModel nestedMapModel = memberModel.getListModel().getListMemberModel().getMapModel();
105-
TypeName nestedMapKeyType = typeProvider.getTypeNameForSimpleType(nestedMapModel.getKeyModel()
106-
.getVariable().getVariableType());
107-
ClassName nestedMapValueType = poetExtensions.getModelClass(nestedMapModel.getValueModel().getC2jShape());
108-
TypeName nestedMapReturnType = ParameterizedTypeName.get(ClassName.get(Map.class),
109-
nestedMapKeyType,
110-
nestedMapValueType.nestedClass("Builder"));
111-
112-
TypeName returnType = ParameterizedTypeName.get(ClassName.get(Collection.class), nestedMapReturnType);
113-
114-
CodeBlock mapReturnStatement =
115-
CodeBlock.of("Collectors.toMap(entry -> entry.getKey(), entry -> entry.getValue().toBuilder())");
116-
117-
CodeBlock returnStatement = CodeBlock.of("return $1N != null ? $1N.stream().map(m -> m.entrySet().stream().collect("
118-
+ mapReturnStatement + ")).collect($2T.toList()) : null;",
119-
memberModel.getVariable().getVariableName(),
120-
Collectors.class);
121-
122-
return basicGetter(memberModel, returnType, returnStatement);
123-
}
124-
125-
private MethodSpec mapOfBuildersGetter(MemberModel memberModel) {
126-
TypeName keyType = typeProvider.getTypeNameForSimpleType(memberModel.getMapModel().getKeyModel()
127-
.getVariable().getVariableType());
128-
ClassName valueType = poetExtensions.getModelClass(memberModel.getMapModel().getValueModel().getC2jShape());
129-
TypeName returnType = ParameterizedTypeName.get(ClassName.get(Map.class), keyType, valueType.nestedClass("Builder"));
130-
131-
return basicGetter(memberModel,
132-
returnType,
133-
CodeBlock.of("return $1N != null ? $2T.mapValues($1N, $3T::toBuilder) : null;",
134-
memberModel.getVariable().getVariableName(),
135-
CollectionUtils.class,
136-
valueType));
137-
}
138-
139-
private MethodSpec listOfBuildersGetter(MemberModel memberModel) {
140-
ClassName memberType = poetExtensions.getModelClass(memberModel.getListModel().getListMemberModel().getC2jShape());
141-
TypeName returnType = ParameterizedTypeName.get(ClassName.get(Collection.class), memberType.nestedClass("Builder"));
142-
143-
return basicGetter(memberModel,
144-
returnType,
145-
CodeBlock.of("return $1N != null ? $1N.stream().map($2T::toBuilder).collect($3T.toList()) : null;",
146-
memberModel.getVariable().getVariableName(),
147-
memberType,
148-
Collectors.class));
149-
}
150-
151138
private MethodSpec basicGetter(MemberModel memberModel, TypeName returnType, CodeBlock body) {
152139
CodeBlock.Builder getterBody = CodeBlock.builder();
153140

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ListSetters.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -120,12 +120,7 @@ public List<MethodSpec> fluent(TypeName returnType) {
120120
@Override
121121
public List<MethodSpec> beanStyle() {
122122
MethodSpec.Builder builder = beanStyleSetterBuilder();
123-
if (memberModel().isCollectionWithBuilderMember()) {
124-
builder.addCode(copySetterBuilderBody());
125-
} else {
126-
builder.addCode(beanCopySetterBody());
127-
}
128-
123+
builder.addCode(beanCopySetterBody());
129124
return Collections.singletonList(builder.build());
130125

131126
}

‎codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/MapSetters.java

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -77,9 +77,8 @@ public List<MethodSpec> fluent(TypeName returnType) {
7777

7878
@Override
7979
public List<MethodSpec> beanStyle() {
80-
MethodSpec.Builder builder = beanStyleSetterBuilder()
81-
.addCode(memberModel().isCollectionWithBuilderMember() ? copySetterBuilderBody() : beanCopySetterBody());
82-
80+
MethodSpec.Builder builder = beanStyleSetterBuilder();
81+
builder.addCode(beanCopySetterBody());
8382
return Collections.singletonList(builder.build());
8483
}
8584

0 commit comments

Comments
 (0)