Skip to content

Commit

Permalink
Annotate and document members that are modeled as deprecated... (#2622)
Browse files Browse the repository at this point in the history
* Annotate and document members that are modeled as deprecated...

...and add support for using customization to override specific members
as deprecated.

## Motivation and Context

As part of a recent change to repurpose the S3 CopyObjectRequest
copySource parameter into more user-friendly parameters, we lacked the
ability in our ShapeModifiersProcessor to also deprecate the old
parameter name. Furthermore, after additional investigation, we also
appear to not have been correctly honoring the different service-2.json
models that tag certain members as deprecated. E.g., the following
parameter is marked as deprecated:

https://github.com/aws/aws-sdk-java-v2/blob/54fd5a69c74af0124e97830bef53b3690dfa2015/services/cloudwatchlogs/src/main/resources/codegen-resources/service-2.json#L1393-L1398

But the corresponding generated code and Javadoc (at the time of this
writing) do not mark any of the member's methods as deprecated, or
expose the deprecatedMessage:

https://sdk.amazonaws.com/java/api/latest/software/amazon/awssdk/services/cloudwatchlogs/model/FilterLogEventsRequest.Builder.html#interleaved-java.lang.Boolean-

## Description

* For service-2.json members that are marked as deprecated, correctly
annotate and document these fields as deprecated in their various
accessors and builder setters.
* Add support to ShapeModifiersProcessor for customization.config files
to be able to manually modify the `deprecated` and `deprecatedMessage`
attributes.

(A follow-up PR will propose marking the before-mentioned copySource
parameter as deprecated.)

## Testing

* Add a new operation with members that are modeled as deprecated and
members that are modified as deprecated, and add accompanying expected
generated code (see expected generated code for an example of how the
fields are annotated as deprecated).
  • Loading branch information
Bennett-Lynch authored Jul 24, 2021
1 parent 54fd5a6 commit 5c7b6dd
Show file tree
Hide file tree
Showing 15 changed files with 769 additions and 11 deletions.
6 changes: 6 additions & 0 deletions .changes/next-release/feature-AWSSDKforJavav2-eac6282.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"category": "AWS SDK for Java v2",
"contributor": "",
"type": "feature",
"description": "Annotate and document members that are modeled as deprecated"
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ protected final ShapeModel generateShapeModel(String javaClassName, String shape
// contains the list of c2j member names that are required for this shape.
shapeModel.setRequired(shape.getRequired());
shapeModel.setDeprecated(shape.isDeprecated());
shapeModel.setDeprecatedMessage(shape.getDeprecatedMessage());
shapeModel.setWrapper(shape.isWrapper());
shapeModel.withIsEventStream(shape.isEventstream());
shapeModel.withIsEvent(shape.isEvent());
Expand Down Expand Up @@ -172,6 +173,7 @@ private MemberModel generateMemberModel(String c2jMemberName, Member c2jMemberDe
.withJsonValue(c2jMemberDefinition.getJsonvalue());
memberModel.setDocumentation(c2jMemberDefinition.getDocumentation());
memberModel.setDeprecated(c2jMemberDefinition.isDeprecated());
memberModel.setDeprecatedMessage(c2jMemberDefinition.getDeprecatedMessage());
memberModel.setSensitive(isSensitiveShapeOrContainer(c2jMemberDefinition, allC2jShapes));
memberModel
.withFluentGetterMethodName(namingStrategy.getFluentGetterMethodName(c2jMemberName, parentShape, shape))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,13 @@ private void preprocessModifyShapeMembers(ServiceModel serviceModel, Shape shape

private void doModifyShapeMembers(ServiceModel serviceModel, Shape shape, String memberToModify,
ModifyModelShapeModifier modifyModel) {

if (modifyModel.isDeprecated()) {
Member member = shape.getMembers().get(memberToModify);
member.setDeprecated(true);
if (modifyModel.getDeprecatedMessage() != null) {
member.setDeprecatedMessage(modifyModel.getDeprecatedMessage());
}
}
// Currently only supports emitPropertyName which is to rename the member
if (modifyModel.getEmitPropertyName() != null) {
Member member = shape.getMembers().remove(memberToModify);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,16 @@

public class ModifyModelShapeModifier {

/**
* Indicates whether a member should be annotated as {@link Deprecated}.
*/
private boolean deprecated;

/**
* The Javadoc message that will be included with the {@link Deprecated} annotation.
*/
private String deprecatedMessage;

/**
* Indicates whether a renamed member should create getters and setters under the existing name
*/
Expand Down Expand Up @@ -49,6 +59,22 @@ public class ModifyModelShapeModifier {

private String unmarshallLocationName;

public String getDeprecatedMessage() {
return deprecatedMessage;
}

public void setDeprecatedMessage(String deprecatedMessage) {
this.deprecatedMessage = deprecatedMessage;
}

public boolean isDeprecated() {
return deprecated;
}

public void setDeprecated(boolean deprecated) {
this.deprecated = deprecated;
}

public boolean isExistingNameDeprecated() {
return existingNameDeprecated;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ public class MemberModel extends DocumentationModel {
private ParameterHttpMapping http;

private boolean deprecated;

private String deprecatedMessage;

private ListModel listModel;

Expand Down Expand Up @@ -301,6 +303,14 @@ public void setDeprecated(boolean deprecated) {
this.deprecated = deprecated;
}

public String getDeprecatedMessage() {
return deprecatedMessage;
}

public void setDeprecatedMessage(String deprecatedMessage) {
this.deprecatedMessage = deprecatedMessage;
}

public boolean isEventPayload() {
return eventPayload;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public class ShapeModel extends DocumentationModel implements HasDeprecation {
private String shapeName;
// the local variable name inside marshaller/unmarshaller implementation
private boolean deprecated;
private String deprecatedMessage;
private String type;
private List<String> required;
private boolean hasPayloadMember;
Expand Down Expand Up @@ -94,6 +95,14 @@ public void setDeprecated(boolean deprecated) {
this.deprecated = deprecated;
}

public String getDeprecatedMessage() {
return deprecatedMessage;
}

public void setDeprecatedMessage(String deprecatedMessage) {
this.deprecatedMessage = deprecatedMessage;
}

public String getC2jName() {
return c2jName;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public class Member {

private boolean deprecated;

private String deprecatedMessage;

private boolean jsonvalue;

private String timestampFormat;
Expand Down Expand Up @@ -153,6 +155,14 @@ public void setDeprecated(boolean deprecated) {
this.deprecated = deprecated;
}

public String getDeprecatedMessage() {
return deprecatedMessage;
}

public void setDeprecatedMessage(String deprecatedMessage) {
this.deprecatedMessage = deprecatedMessage;
}

public boolean getJsonvalue() {
return jsonvalue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ public class Shape {
private boolean fault;

private boolean deprecated;

private String deprecatedMessage;

private boolean eventstream;

Expand Down Expand Up @@ -260,6 +262,14 @@ public void setDeprecated(boolean deprecated) {
this.deprecated = deprecated;
}

public String getDeprecatedMessage() {
return deprecatedMessage;
}

public void setDeprecatedMessage(String deprecatedMessage) {
this.deprecatedMessage = deprecatedMessage;
}

public boolean isEventstream() {
return eventstream;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import static javax.lang.model.element.Modifier.FINAL;
import static javax.lang.model.element.Modifier.PRIVATE;
import static javax.lang.model.element.Modifier.PUBLIC;
import static software.amazon.awssdk.codegen.poet.model.DeprecationUtils.checkDeprecated;

import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.CodeBlock;
Expand Down Expand Up @@ -509,7 +510,7 @@ private Stream<MethodSpec> memberGetters(MemberModel member) {

result.add(memberGetter(member));

return result.stream();
return checkDeprecated(member, result).stream();
}

private boolean shouldGenerateDeprecatedNameGetter(MemberModel member) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License").
* You may not use this file except in compliance with the License.
* A copy of the License is located at
*
* http://aws.amazon.com/apache2.0
*
* or in the "license" file accompanying this file. This file is distributed
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
* express or implied. See the License for the specific language governing
* permissions and limitations under the License.
*/

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

import static java.util.stream.Collectors.toList;
import static software.amazon.awssdk.codegen.internal.Constant.LF;

import com.squareup.javapoet.AnnotationSpec;
import com.squareup.javapoet.MethodSpec;
import java.util.List;
import software.amazon.awssdk.codegen.model.intermediate.MemberModel;
import software.amazon.awssdk.utils.StringUtils;

public final class DeprecationUtils {

private static final AnnotationSpec DEPRECATED = AnnotationSpec.builder(Deprecated.class).build();

private DeprecationUtils() {
}

/**
* If a given member is modeled as deprecated, add the {@link Deprecated} annotation to the method and, if the method
* already has existing Javadoc, append a section with the {@code @deprecated} tag.
*/
public static MethodSpec checkDeprecated(MemberModel member, MethodSpec method) {
if (!member.isDeprecated() || method.annotations.contains(DEPRECATED)) {
return method;
}
MethodSpec.Builder builder = method.toBuilder().addAnnotation(DEPRECATED);
if (!method.javadoc.isEmpty()) {
builder.addJavadoc(LF + "@deprecated");
if (StringUtils.isNotBlank(member.getDeprecatedMessage())) {
builder.addJavadoc(" $L", member.getDeprecatedMessage());
}
}
return builder.build();
}

public static List<MethodSpec> checkDeprecated(MemberModel member, List<MethodSpec> methods) {
return methods.stream().map(methodSpec -> checkDeprecated(member, methodSpec)).collect(toList());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package software.amazon.awssdk.codegen.poet.model;

import static java.util.stream.Collectors.toList;
import static software.amazon.awssdk.codegen.poet.model.DeprecationUtils.checkDeprecated;

import com.squareup.javapoet.ClassName;
import com.squareup.javapoet.FieldSpec;
Expand Down Expand Up @@ -76,8 +77,10 @@ public TypeSpec builderInterface() {

shapeModel.getNonStreamingMembers()
.forEach(m -> {
builder.addMethods(accessorsFactory.fluentSetterDeclarations(m, builderInterfaceName()));
builder.addMethods(accessorsFactory.convenienceSetterDeclarations(m, builderInterfaceName()));
builder.addMethods(
checkDeprecated(m, accessorsFactory.fluentSetterDeclarations(m, builderInterfaceName())));
builder.addMethods(
checkDeprecated(m, accessorsFactory.convenienceSetterDeclarations(m, builderInterfaceName())));
});

if (isException()) {
Expand Down Expand Up @@ -212,13 +215,10 @@ private List<MethodSpec> accessors() {
List<MethodSpec> accessors = new ArrayList<>();
shapeModel.getNonStreamingMembers()
.forEach(m -> {
accessors.add(accessorsFactory.beanStyleGetter(m));

List<MethodSpec> fluentSetters = accessorsFactory.fluentSetters(m, builderInterfaceName());
accessors.addAll(fluentSetters);

accessors.addAll(accessorsFactory.beanStyleSetters(m));
accessors.addAll(accessorsFactory.convenienceSetters(m, builderInterfaceName()));
accessors.add(checkDeprecated(m, accessorsFactory.beanStyleGetter(m)));
accessors.addAll(checkDeprecated(m, accessorsFactory.fluentSetters(m, builderInterfaceName())));
accessors.addAll(checkDeprecated(m, accessorsFactory.beanStyleSetters(m)));
accessors.addAll(checkDeprecated(m, accessorsFactory.convenienceSetters(m, builderInterfaceName())));
});

if (isException()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,16 @@
}
}
]
},
"OperationWithDeprecatedMemberRequest": {
"modify": [
{
"MemberModifiedAsDeprecated": {
"deprecated": true,
"deprecatedMessage": "This field is modified as deprecated."
}
}
]
}
},
"underscoresInNameBehavior": "ALLOW"
Expand Down
Loading

0 comments on commit 5c7b6dd

Please sign in to comment.