Skip to content

Commit ffd66db

Browse files
cindy-pengcindy-peng
committed
Fix comments from PR review.
1 parent 221eb8f commit ffd66db

30 files changed

+606
-540
lines changed

‎gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/comment/SettingsCommentComposer.java‎

Lines changed: 18 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -59,31 +59,31 @@ public class SettingsCommentComposer {
5959
"Retries are configured for idempotent methods but not for non-idempotent methods.";
6060

6161
public static final CommentStatement DEFAULT_SCOPES_COMMENT =
62-
toSimpleComment("The default scopes of the service.");
62+
toCommentStatement("The default scopes of the service.");
6363

6464
public static final CommentStatement DEFAULT_EXECUTOR_PROVIDER_BUILDER_METHOD_COMMENT =
65-
toSimpleComment("Returns a builder for the default ExecutorProvider for this service.");
65+
toCommentStatement("Returns a builder for the default ExecutorProvider for this service.");
6666

6767
public static final CommentStatement DEFAULT_SERVICE_NAME_METHOD_COMMENT =
68-
toSimpleComment("Returns the default service name.");
68+
toCommentStatement("Returns the default service name.");
6969
public static final CommentStatement DEFAULT_SERVICE_ENDPOINT_METHOD_COMMENT =
70-
toSimpleComment("Returns the default service endpoint.");
70+
toCommentStatement("Returns the default service endpoint.");
7171
public static final CommentStatement DEFAULT_SERVICE_MTLS_ENDPOINT_METHOD_COMMENT =
72-
toSimpleComment("Returns the default mTLS service endpoint.");
72+
toCommentStatement("Returns the default mTLS service endpoint.");
7373
public static final CommentStatement DEFAULT_SERVICE_SCOPES_METHOD_COMMENT =
74-
toSimpleComment("Returns the default service scopes.");
74+
toCommentStatement("Returns the default service scopes.");
7575

7676
public static final CommentStatement DEFAULT_CREDENTIALS_PROVIDER_BUILDER_METHOD_COMMENT =
77-
toSimpleComment("Returns a builder for the default credentials for this service.");
77+
toCommentStatement("Returns a builder for the default credentials for this service.");
7878

7979
public static final CommentStatement DEFAULT_TRANSPORT_PROVIDER_BUILDER_METHOD_COMMENT =
80-
toSimpleComment("Returns a builder for the default ChannelProvider for this service.");
80+
toCommentStatement("Returns a builder for the default ChannelProvider for this service.");
8181

8282
public static final CommentStatement NEW_BUILDER_METHOD_COMMENT =
83-
toSimpleComment("Returns a new builder for this class.");
83+
toCommentStatement("Returns a new builder for this class.");
8484

8585
public static final CommentStatement TO_BUILDER_METHOD_COMMENT =
86-
toSimpleComment("Returns a builder containing all the values of this settings class.");
86+
toCommentStatement("Returns a builder containing all the values of this settings class.");
8787

8888
public static final List<CommentStatement> APPLY_TO_ALL_UNARY_METHODS_METHOD_COMMENTS =
8989
Arrays.asList(
@@ -103,9 +103,9 @@ public class SettingsCommentComposer {
103103

104104
public SettingsCommentComposer(String transportPrefix) {
105105
this.newTransportBuilderMethodComment =
106-
toSimpleComment(String.format("Returns a new %s builder for this class.", transportPrefix));
106+
toCommentStatement(String.format("Returns a new %s builder for this class.", transportPrefix));
107107
this.transportProviderBuilderMethodComment =
108-
toSimpleComment(
108+
toCommentStatement(
109109
String.format(
110110
"Returns a builder for the default %s ChannelProvider for this service.",
111111
transportPrefix));
@@ -121,22 +121,20 @@ public CommentStatement getTransportProviderBuilderMethodComment() {
121121

122122
public static CommentStatement createCallSettingsGetterComment(
123123
String javaMethodName, boolean isMethodDeprecated, boolean isMethodInternal) {
124-
return toDeprecatedInternalSimpleComment(
124+
return toCommentStatement(
125125
String.format(CALL_SETTINGS_METHOD_DOC_PATTERN, javaMethodName),
126126
isMethodDeprecated,
127127
isMethodInternal);
128128
}
129129

130130
public static CommentStatement createBuilderClassComment(String outerClassName) {
131-
return toSimpleComment(String.format(BUILDER_CLASS_DOC_PATTERN, outerClassName));
131+
return toCommentStatement(String.format(BUILDER_CLASS_DOC_PATTERN, outerClassName));
132132
}
133133

134134
public static CommentStatement createCallSettingsBuilderGetterComment(
135135
String javaMethodName, boolean isMethodDeprecated, boolean isMethodInternal) {
136136
String methodComment = String.format(CALL_SETTINGS_BUILDER_METHOD_DOC_PATTERN, javaMethodName);
137-
return isMethodDeprecated || isMethodInternal
138-
? toDeprecatedInternalSimpleComment(methodComment, isMethodDeprecated, isMethodInternal)
139-
: toSimpleComment(methodComment);
137+
return toCommentStatement(methodComment, isMethodDeprecated, isMethodInternal);
140138
}
141139

142140
public static List<CommentStatement> createClassHeaderComments(
@@ -201,11 +199,11 @@ public static List<CommentStatement> createClassHeaderComments(
201199
CommentStatement.withComment(javaDocCommentBuilder.build()));
202200
}
203201

204-
private static CommentStatement toSimpleComment(String comment) {
205-
return CommentStatement.withComment(JavaDocComment.withComment(comment));
202+
private static CommentStatement toCommentStatement(String comment) {
203+
return toCommentStatement(comment, false, false);
206204
}
207205

208-
private static CommentStatement toDeprecatedInternalSimpleComment(
206+
private static CommentStatement toCommentStatement(
209207
String comment, boolean isDeprecated, boolean isInternal) {
210208
JavaDocComment.Builder docBuilder = JavaDocComment.builder().addComment(comment);
211209
docBuilder =

‎gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java‎

Lines changed: 19 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,22 @@ protected TransportContext getTransportContext() {
130130
return transportContext;
131131
}
132132

133+
private static List<AnnotationNode> addMethodAnnotations(Method method, TypeStore typeStore)
134+
{
135+
List<AnnotationNode> annotations = new ArrayList<>();
136+
if (method.isDeprecated()) {
137+
annotations.add(AnnotationNode.withType(TypeNode.DEPRECATED));
138+
}
139+
140+
if (method.isInternalApi()) {
141+
annotations.add(
142+
AnnotationNode.withTypeAndDescription(
143+
typeStore.get("InternalApi"), INTERNAL_API_WARNING));
144+
}
145+
146+
return annotations;
147+
}
148+
133149
@Override
134150
public GapicClass generate(GapicContext context, Service service) {
135151
Map<String, ResourceName> resourceNames = context.helperResourceNames();
@@ -804,18 +820,7 @@ private static List<MethodDefinition> createMethodVariants(
804820
methodVariantBuilder.setReturnType(methodOutputType).setReturnExpr(rpcInvocationExpr);
805821
}
806822

807-
List<AnnotationNode> annotations = new ArrayList<>();
808-
if (method.isDeprecated()) {
809-
annotations.add(AnnotationNode.withType(TypeNode.DEPRECATED));
810-
}
811-
812-
if (method.isInternalApi()) {
813-
annotations.add(
814-
AnnotationNode.withTypeAndDescription(
815-
typeStore.get("InternalApi"), INTERNAL_API_WARNING));
816-
}
817-
818-
methodVariantBuilder = methodVariantBuilder.setAnnotations(annotations);
823+
methodVariantBuilder = methodVariantBuilder.setAnnotations(addMethodAnnotations(method, typeStore));
819824
methodVariantBuilder = methodVariantBuilder.setBody(statements);
820825
javaMethods.add(methodVariantBuilder.build());
821826
}
@@ -837,7 +842,6 @@ private static MethodDefinition createMethodDefaultMethod(
837842
method.isPaged()
838843
? typeStore.get(String.format(PAGED_RESPONSE_TYPE_NAME_PATTERN, method.name()))
839844
: method.outputType();
840-
List<AnnotationNode> annotations = new ArrayList<>();
841845
if (method.hasLro()) {
842846
LongrunningOperation lro = method.lro();
843847
methodOutputType =
@@ -894,15 +898,6 @@ private static MethodDefinition createMethodDefaultMethod(
894898
.setName(String.format(method.hasLro() ? "%sAsync" : "%s", methodName))
895899
.setArguments(Arrays.asList(requestArgVarExpr));
896900

897-
if (method.isDeprecated()) {
898-
annotations.add(AnnotationNode.withType(TypeNode.DEPRECATED));
899-
}
900-
901-
if (method.isInternalApi()) {
902-
annotations.add(
903-
AnnotationNode.withTypeAndDescription(
904-
typeStore.get("InternalApi"), INTERNAL_API_WARNING));
905-
}
906901

907902
if (isProtoEmptyType(methodOutputType)) {
908903
methodBuilder =
@@ -914,8 +909,7 @@ private static MethodDefinition createMethodDefaultMethod(
914909
methodBuilder.setReturnExpr(callableMethodExpr).setReturnType(methodOutputType);
915910
}
916911

917-
methodBuilder.setAnnotations(annotations);
918-
912+
methodBuilder.setAnnotations(addMethodAnnotations(method, typeStore));
919913
return methodBuilder.build();
920914
}
921915

@@ -1054,17 +1048,8 @@ private static MethodDefinition createCallableMethod(
10541048
}
10551049

10561050
MethodDefinition.Builder methodDefBuilder = MethodDefinition.builder();
1057-
List<AnnotationNode> annotations = new ArrayList<>();
1058-
if (method.isDeprecated()) {
1059-
annotations.add(AnnotationNode.withType(TypeNode.DEPRECATED));
1060-
}
1061-
if (method.isInternalApi()) {
1062-
annotations.add(
1063-
AnnotationNode.withTypeAndDescription(
1064-
typeStore.get("InternalApi"), INTERNAL_API_WARNING));
1065-
}
10661051

1067-
methodDefBuilder = methodDefBuilder.setAnnotations(annotations);
1052+
methodDefBuilder = methodDefBuilder.setAnnotations(addMethodAnnotations(method, typeStore));
10681053

10691054
return methodDefBuilder
10701055
.setHeaderCommentStatements(

‎gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceSettingsClassComposer.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,10 +283,10 @@ private static List<MethodDefinition> createSettingsGetterMethods(
283283
methodMakerFn.apply(getCallSettingsType(protoMethod, typeStore), javaMethodName);
284284
javaMethods.add(methodBuilderHelper(protoMethod, methodBuilder, javaMethodName));
285285
if (protoMethod.hasLro()) {
286-
javaMethodName = String.format("%sOperationSettings", javaStyleName);
286+
String javaOperationSettingsMethodName = String.format("%sOperationSettings", javaStyleName);
287287
methodBuilder =
288-
methodMakerFn.apply(getOperationCallSettingsType(protoMethod), javaMethodName);
289-
javaMethods.add(methodBuilderHelper(protoMethod, methodBuilder, javaMethodName));
288+
methodMakerFn.apply(getOperationCallSettingsType(protoMethod), javaOperationSettingsMethodName);
289+
javaMethods.add(methodBuilderHelper(protoMethod, methodBuilder, javaOperationSettingsMethodName));
290290
}
291291
}
292292
return javaMethods;

‎gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/common/AbstractServiceStubSettingsClassComposer.java‎

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,18 @@ private List<MethodDefinition> createClassMethods(
10161016
return javaMethods;
10171017
}
10181018

1019+
private static List<AnnotationNode> createMethodAnnotation(boolean isDeprecated, boolean isInternal) {
1020+
List<AnnotationNode> annotations = new ArrayList<>();
1021+
if (isDeprecated) {
1022+
annotations.add(AnnotationNode.withType(TypeNode.DEPRECATED));
1023+
}
1024+
if (isInternal) {
1025+
annotations.add(
1026+
AnnotationNode.withTypeAndDescription(
1027+
FIXED_TYPESTORE.get("InternalApi"), INTERNAL_API_WARNING));
1028+
}
1029+
return annotations;
1030+
}
10191031
private static List<MethodDefinition> createMethodSettingsGetterMethods(
10201032
Map<String, VariableExpr> methodSettingsMemberVarExprs,
10211033
final Set<String> deprecatedSettingVarNames,
@@ -1024,20 +1036,11 @@ private static List<MethodDefinition> createMethodSettingsGetterMethods(
10241036
e -> {
10251037
boolean isDeprecated = deprecatedSettingVarNames.contains(e.getKey());
10261038
boolean isInternal = internalSettingVarNames.contains(e.getKey());
1027-
List<AnnotationNode> annotations = new ArrayList<>();
1028-
if (isDeprecated) {
1029-
annotations.add(AnnotationNode.withType(TypeNode.DEPRECATED));
1030-
}
1031-
if (isInternal) {
1032-
annotations.add(
1033-
AnnotationNode.withTypeAndDescription(
1034-
FIXED_TYPESTORE.get("InternalApi"), INTERNAL_API_WARNING));
1035-
}
10361039
return MethodDefinition.builder()
10371040
.setHeaderCommentStatements(
10381041
SettingsCommentComposer.createCallSettingsGetterComment(
10391042
getMethodNameFromSettingsVarName(e.getKey()), isDeprecated, isInternal))
1040-
.setAnnotations(annotations)
1043+
.setAnnotations(createMethodAnnotation(isDeprecated, isInternal))
10411044
.setScope(ScopeNode.PUBLIC)
10421045
.setReturnType(e.getValue().type())
10431046
.setName(e.getKey())
@@ -2007,34 +2010,22 @@ private static List<MethodDefinition> createNestedClassSettingsBuilderGetterMeth
20072010
t.reference()
20082011
.copyAndSetGenerics(ImmutableList.of())
20092012
.equals(operationCallSettingsBuilderRef);
2010-
AnnotationNode deprecatedAnnotation = AnnotationNode.withType(TypeNode.DEPRECATED);
2011-
AnnotationNode internalAnnotation =
2012-
AnnotationNode.withTypeAndDescription(
2013-
FIXED_TYPESTORE.get("InternalApi"), INTERNAL_API_WARNING);
20142013

20152014
List<MethodDefinition> javaMethods = new ArrayList<>();
20162015
for (Map.Entry<String, VariableExpr> settingsVarEntry :
20172016
nestedMethodSettingsMemberVarExprs.entrySet()) {
20182017
String varName = settingsVarEntry.getKey();
20192018
VariableExpr settingsVarExpr = settingsVarEntry.getValue();
2020-
List<AnnotationNode> annotations = new ArrayList<>();
20212019

20222020
boolean isDeprecated = nestedDeprecatedSettingVarNames.contains(varName);
2023-
if (isDeprecated) {
2024-
annotations.add(deprecatedAnnotation);
2025-
}
2026-
20272021
boolean isInternal = nestedInternalSettingVarNames.contains(varName);
2028-
if (isInternal) {
2029-
annotations.add(internalAnnotation);
2030-
}
20312022

20322023
javaMethods.add(
20332024
MethodDefinition.builder()
20342025
.setHeaderCommentStatements(
20352026
SettingsCommentComposer.createCallSettingsBuilderGetterComment(
20362027
getMethodNameFromSettingsVarName(varName), isDeprecated, isInternal))
2037-
.setAnnotations(annotations)
2028+
.setAnnotations(createMethodAnnotation(isDeprecated, isInternal))
20382029
.setScope(ScopeNode.PUBLIC)
20392030
.setReturnType(settingsVarExpr.type())
20402031
.setName(settingsVarExpr.variable().identifier().name())

‎gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,11 @@
9393

9494
public class Parser {
9595
enum SelectiveGapicType {
96+
// Methods will be generated and exposed externally as usual.
9697
PUBLIC,
98+
// Methods will not be generated.
9799
HIDDEN,
100+
// Methods will be generated and tagged @InternalApi (internal use) during generation.
98101
INTERNAL
99102
}
100103

@@ -474,9 +477,16 @@ static SelectiveGapicType getMethodSelectiveGapicType(
474477
// is in the allow list.
475478
// Otherwise, generate this method as INTERNAL or HIDDEN based on GenerateOmittedAsInternal
476479
// flag.
477-
if (includeMethodsList.isEmpty() && generateOmittedAsInternal == false
478-
|| includeMethodsList.contains(method.getFullName())) return SelectiveGapicType.PUBLIC;
479-
else return generateOmittedAsInternal ? SelectiveGapicType.INTERNAL : SelectiveGapicType.HIDDEN;
480+
if (includeMethodsList.isEmpty() && generateOmittedAsInternal == false || includeMethodsList.contains(method.getFullName())) {
481+
return SelectiveGapicType.PUBLIC;
482+
}
483+
else if (generateOmittedAsInternal)
484+
{
485+
return SelectiveGapicType.INTERNAL;
486+
}
487+
else {
488+
return SelectiveGapicType.HIDDEN;
489+
}
480490
}
481491

482492
// A service is considered empty if it contains no methods, or only methods marked as HIDDEN.

‎gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,7 @@
4242

4343
class ComposerTest {
4444
private final GapicContext context = GrpcTestProtoLoader.instance().parseShowcaseEcho();
45-
private final GapicContext selectiveGapicContext =
46-
GrpcTestProtoLoader.instance().parseSelectiveGenerationTesting();
4745
private final Service echoProtoService = context.services().get(0);
48-
private final Service selctiveGapicService = selectiveGapicContext.services().get(1);
4946
private final List<GapicClass> clazzes =
5047
Arrays.asList(
5148
GrpcServiceCallableFactoryClassComposer.instance()

‎gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpc/GrpcServiceStubClassComposerTest.java‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,16 +93,6 @@ void generateGrpcServiceStubClass_autopopulateField() {
9393
Assert.assertEmptySamples(clazz.samples());
9494
}
9595

96-
@Test
97-
void generateGrpcServiceStubClass_selectiveGeneration() {
98-
GapicContext context = GrpcTestProtoLoader.instance().parseSelectiveGenerationTesting();
99-
Service service = context.services().get(1);
100-
GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(context, service);
101-
102-
Assert.assertGoldenClass(this.getClass(), clazz, "SelectiveGapicStub.golden");
103-
Assert.assertEmptySamples(clazz.samples());
104-
}
105-
10696
@Test
10797
void generateGrpcServiceStubClass_callableNameType() {
10898
GapicContext context = GrpcTestProtoLoader.instance().parseCallabeNameType();

0 commit comments

Comments
 (0)