Skip to content

Rewrote generated copiers to support builders for arbitrarily nested containers. #2423

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 21, 2021
Merged

Rewrote generated copiers to support builders for arbitrarily nested containers. #2423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/next-release/bugfix-AmazonDynamoDB-a0ae32a.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "Amazon DynamoDB",
"contributor": "",
"type": "bugfix",
"description": "Fixed bean-based marshalling for model builder types containing complex collections."
}
26 changes: 19 additions & 7 deletions codegen/src/main/java/software/amazon/awssdk/codegen/model/intermediate/MemberModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import java.util.Optional;
import software.amazon.awssdk.codegen.internal.TypeUtils;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.core.SdkField;
import software.amazon.awssdk.core.protocol.MarshallingType;
import software.amazon.awssdk.core.util.SdkAutoConstructList;
import software.amazon.awssdk.core.util.SdkAutoConstructMap;
Expand Down Expand Up @@ -615,15 +616,26 @@ public boolean hasBuilder() {
}

@JsonIgnore
public boolean isCollectionWithBuilderMember() {
return (isList() && getListModel().getListMemberModel() != null && getListModel().getListMemberModel().hasBuilder()) ||
(isMap() && getMapModel().getValueModel() != null && getMapModel().getValueModel().hasBuilder());
public boolean containsBuildable() {
return containsBuildable(true);
}

@JsonIgnore
public boolean isCollectionWithNestedBuilderMember() {
return isList() && getListModel().getListMemberModel() != null && getListModel().isMap() &&
getListModel().getListMemberModel().getMapModel().getValueModel().hasBuilder();
private boolean containsBuildable(boolean root) {
if (!root && hasBuilder()) {
return true;
}

if (isList()) {
return getListModel().getListMemberModel().containsBuildable(false);
}

if (isMap()) {
MapModel mapModel = getMapModel();
return mapModel.getKeyModel().containsBuildable(false) ||
mapModel.getValueModel().containsBuildable(false);
}

return false;
}

@JsonIgnore
Expand Down
76 changes: 10 additions & 66 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AbstractMemberSetters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,21 @@

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

import static software.amazon.awssdk.codegen.poet.model.TypeProvider.ShapeTransformation.USE_BUILDER_IMPL;

import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.MethodSpec;
import com.squareup.javapoet.ParameterSpec;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeName;
import java.nio.ByteBuffer;
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import javax.lang.model.element.Modifier;
import software.amazon.awssdk.codegen.model.intermediate.IntermediateModel;
import software.amazon.awssdk.codegen.model.intermediate.MapModel;
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
import software.amazon.awssdk.codegen.model.intermediate.ShapeModel;
import software.amazon.awssdk.codegen.poet.PoetExtensions;
import software.amazon.awssdk.codegen.poet.model.TypeProvider.TypeNameOptions;
import software.amazon.awssdk.core.SdkBytes;

/**
Expand Down Expand Up @@ -125,8 +123,8 @@ protected CodeBlock copySetterBuilderBody() {
"this.$1N = $1N != null ? $1N.build() : null",
serviceModelCopiers.copyMethodName());
}
if (memberModel.isCollectionWithBuilderMember() || memberModel.isCollectionWithNestedBuilderMember()) {
return copySetterBody("this.$1N = $2T.$3N($1N)", null, serviceModelCopiers.builderCopyMethodName());
if (memberModel.containsBuildable()) {
return copySetterBody("this.$1N = $2T.$3N($1N)", null, serviceModelCopiers.copyFromBuilderMethodName());
}
return copySetterBody();
}
Expand Down Expand Up @@ -170,60 +168,11 @@ protected ParameterSpec memberAsParameter() {
}

protected ParameterSpec memberAsBeanStyleParameter() {
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();
Comment on lines -173 to -226
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for removing this!

TypeName type = typeProvider.typeName(memberModel, new TypeNameOptions().shapeTransformation(USE_BUILDER_IMPL)
.useSubtypeWildcardsForCollections(true)
.useCollectionForList(true)
.useByteBufferTypes(true));
return ParameterSpec.builder(type, fieldName()).build();
}

protected ShapeModel shapeModel() {
Expand Down Expand Up @@ -259,9 +208,4 @@ private CodeBlock copySetterBody(String copyAssignment, String regularAssignment
.build())
.orElseGet(() -> CodeBlock.builder().addStatement(regularAssignment, fieldName()).build());
}

private boolean hasBuilder(MemberModel model) {
return model != null && model.hasBuilder();
}

}
2 changes: 1 addition & 1 deletion codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/AccessorsFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class AccessorsFactory {
this.shapeModel = shapeModel;
this.typeProvider = typeProvider;
this.intermediateModel = intermediateModel;
this.getterHelper = new BeanGetterHelper(poetExtensions, typeProvider);
this.getterHelper = new BeanGetterHelper(poetExtensions, new ServiceModelCopiers(intermediateModel), typeProvider);
}

public MethodSpec beanStyleGetter(MemberModel memberModel) {
Expand Down
101 changes: 44 additions & 57 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/BeanGetterHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,42 +15,43 @@

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

import static software.amazon.awssdk.codegen.poet.model.TypeProvider.ShapeTransformation.USE_BUILDER;

import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
import com.squareup.javapoet.MethodSpec;
import com.squareup.javapoet.ParameterizedTypeName;
import com.squareup.javapoet.TypeName;
import java.nio.ByteBuffer;
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import javax.lang.model.element.Modifier;
import software.amazon.awssdk.codegen.model.intermediate.MapModel;
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
import software.amazon.awssdk.codegen.poet.PoetExtensions;
import software.amazon.awssdk.codegen.poet.PoetUtils;
import software.amazon.awssdk.codegen.poet.model.TypeProvider.TypeNameOptions;
import software.amazon.awssdk.core.SdkBytes;
import software.amazon.awssdk.utils.CollectionUtils;
import software.amazon.awssdk.core.util.SdkAutoConstructList;
import software.amazon.awssdk.core.util.SdkAutoConstructMap;

public final class BeanGetterHelper {
private final PoetExtensions poetExtensions;
private final ServiceModelCopiers copiers;
private final TypeProvider typeProvider;

public BeanGetterHelper(PoetExtensions poetExtensions, TypeProvider typeProvider) {
public BeanGetterHelper(PoetExtensions poetExtensions, ServiceModelCopiers copiers, TypeProvider typeProvider) {
this.poetExtensions = poetExtensions;
this.copiers = copiers;
this.typeProvider = typeProvider;
}

public MethodSpec beanStyleGetter(MemberModel memberModel) {
if (memberModel.hasBuilder()) {
return builderGetter(memberModel);
}
if (memberModel.isCollectionWithBuilderMember()) {
return memberModel.isList() ? listOfBuildersGetter(memberModel) : mapOfBuildersGetter(memberModel);
}
if (memberModel.isCollectionWithNestedBuilderMember()) {
return listOfMapOfBuilderGetter(memberModel);
if (memberModel.containsBuildable()) {
return buildableContainerGetter(memberModel);
}
if (memberModel.isSdkBytesType()) {
return byteBufferGetter(memberModel);
Expand All @@ -64,6 +65,40 @@ public MethodSpec beanStyleGetter(MemberModel memberModel) {
return regularGetter(memberModel);
}

private MethodSpec buildableContainerGetter(MemberModel memberModel) {
TypeName returnType = typeProvider.typeName(memberModel, new TypeNameOptions().shapeTransformation(USE_BUILDER)
.useByteBufferTypes(true));

ClassName copierClass = copiers.copierClassFor(memberModel)
.orElseThrow(() -> new IllegalStateException("Copier class not found for " + memberModel));

String variableName = memberModel.getVariable().getVariableName();

CodeBlock.Builder code = CodeBlock.builder();

code.add("$T result = $T.$N(this.$N);", returnType, copierClass, copiers.copyToBuilderMethodName(), variableName);

if (memberModel.isList()) {
code.add("if (result instanceof $T) {", SdkAutoConstructList.class)
.add("return null;")
.add("}");
}

if (memberModel.isMap()) {
code.add("if (result instanceof $T) {", SdkAutoConstructMap.class)
.add("return null;")
.add("}");
}

code.add("return result;");

return MethodSpec.methodBuilder(memberModel.getBeanStyleGetterMethodName())
.addModifiers(Modifier.PUBLIC, Modifier.FINAL)
.returns(returnType)
.addCode(code.build())
.build();
}

private MethodSpec byteBufferGetter(MemberModel memberModel) {
return basicGetter(memberModel,
ClassName.get(ByteBuffer.class),
Expand Down Expand Up @@ -100,54 +135,6 @@ private MethodSpec builderGetter(MemberModel memberModel) {
memberModel.getVariable().getVariableName()));
}

private MethodSpec listOfMapOfBuilderGetter(MemberModel memberModel) {
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("Builder"));

TypeName returnType = ParameterizedTypeName.get(ClassName.get(Collection.class), nestedMapReturnType);

CodeBlock mapReturnStatement =
CodeBlock.of("Collectors.toMap(entry -> entry.getKey(), entry -> entry.getValue().toBuilder())");

CodeBlock returnStatement = CodeBlock.of("return $1N != null ? $1N.stream().map(m -> m.entrySet().stream().collect("
+ mapReturnStatement + ")).collect($2T.toList()) : null;",
memberModel.getVariable().getVariableName(),
Collectors.class);

return basicGetter(memberModel, returnType, returnStatement);
}

private MethodSpec mapOfBuildersGetter(MemberModel memberModel) {
TypeName keyType = typeProvider.getTypeNameForSimpleType(memberModel.getMapModel().getKeyModel()
.getVariable().getVariableType());
ClassName valueType = poetExtensions.getModelClass(memberModel.getMapModel().getValueModel().getC2jShape());
TypeName returnType = ParameterizedTypeName.get(ClassName.get(Map.class), keyType, valueType.nestedClass("Builder"));

return basicGetter(memberModel,
returnType,
CodeBlock.of("return $1N != null ? $2T.mapValues($1N, $3T::toBuilder) : null;",
memberModel.getVariable().getVariableName(),
CollectionUtils.class,
valueType));
}

private MethodSpec listOfBuildersGetter(MemberModel memberModel) {
ClassName memberType = poetExtensions.getModelClass(memberModel.getListModel().getListMemberModel().getC2jShape());
TypeName returnType = ParameterizedTypeName.get(ClassName.get(Collection.class), memberType.nestedClass("Builder"));

return basicGetter(memberModel,
returnType,
CodeBlock.of("return $1N != null ? $1N.stream().map($2T::toBuilder).collect($3T.toList()) : null;",
memberModel.getVariable().getVariableName(),
memberType,
Collectors.class));
}

private MethodSpec basicGetter(MemberModel memberModel, TypeName returnType, CodeBlock body) {
CodeBlock.Builder getterBody = CodeBlock.builder();

Expand Down
7 changes: 1 addition & 6 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/ListSetters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,7 @@ public List<MethodSpec> fluent(TypeName returnType) {
@Override
public List<MethodSpec> beanStyle() {
MethodSpec.Builder builder = beanStyleSetterBuilder();
if (memberModel().isCollectionWithBuilderMember()) {
builder.addCode(copySetterBuilderBody());
} else {
builder.addCode(beanCopySetterBody());
}

builder.addCode(beanCopySetterBody());
return Collections.singletonList(builder.build());

}
Expand Down
5 changes: 2 additions & 3 deletions codegen/src/main/java/software/amazon/awssdk/codegen/poet/model/MapSetters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,8 @@ public List<MethodSpec> fluent(TypeName returnType) {

@Override
public List<MethodSpec> beanStyle() {
MethodSpec.Builder builder = beanStyleSetterBuilder()
.addCode(memberModel().isCollectionWithBuilderMember() ? copySetterBuilderBody() : beanCopySetterBody());

MethodSpec.Builder builder = beanStyleSetterBuilder();
builder.addCode(beanCopySetterBody());
return Collections.singletonList(builder.build());
}

Expand Down
Loading