Skip to content

feat: update javadocs for Client classes to include table of methods #2114

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
alicejli merged 23 commits into from
Nov 16, 2023
Merged

feat: update javadocs for Client classes to include table of methods #2114

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
dd8b28d
feat: update javadocs for Client classes to include table of methods …
alicejli Oct 12, 2023
218eb5d
fix lint
alicejli Oct 12, 2023
dd3d21e
update showcase goldens
alicejli Oct 12, 2023
116ca1c
remove unnecessary newlines and encapsulate method description within…
alicejli Oct 13, 2023
83f4743
fix lint
alicejli Oct 13, 2023
a4abe4e
update showcase goldens
alicejli Oct 14, 2023
3ad0468
Merge branch 'main' into updateServiceClientComments
alicejli Nov 9, 2023
cae754e
test remove cache for golden test
alicejli Nov 9, 2023
07e6e51
update integration goldens
alicejli Nov 9, 2023
c2c5914
adding back in cache
alicejli Nov 9, 2023
c982b15
update integration goldens
alicejli Nov 10, 2023
ab1c642
test change to ci
alicejli Nov 10, 2023
adf7d97
update ci comment
alicejli Nov 10, 2023
d6f758d
update to use linkedhashmap for consistent ordering
alicejli Nov 10, 2023
a1845a9
update showcase goldens
alicejli Nov 10, 2023
52ce9c2
fix lint
alicejli Nov 10, 2023
7cad882
refactor
alicejli Nov 13, 2023
4944068
refactor
alicejli Nov 14, 2023
11f0c03
fix lint and showcase goldens
alicejli Nov 14, 2023
94a6785
Merge branch 'main' into updateServiceClientComments
alicejli Nov 14, 2023
61671e7
update showcase goldens
alicejli Nov 14, 2023
e37bf88
include all parameters, not just first one
alicejli Nov 14, 2023
32486e6
include primitive types and update indentation
alicejli Nov 14, 2023
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
4 changes: 2 additions & 2 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ jobs:
runs-on: ubuntu-22.04
steps:
- uses: actions/checkout@v3
# Java 8 tests uses JDK 11 to compile and JDK 8 to run tests.
# Java 8 tests uses JDK 17 to compile and JDK 8 to run tests.
- uses: actions/setup-java@v3
with:
java-version: 8
Expand All @@ -69,7 +69,7 @@ jobs:
shell: bash
run: |
set -x
export JAVA_HOME=$JAVA11_HOME
export JAVA_HOME=$JAVA_HOME
export PATH=${JAVA_HOME}/bin:$PATH
# Maven surefire plugin lets us to specify the JVM when running tests via
# the "jvm" system property.
Expand Down
9 changes: 9 additions & 0 deletions gapic-generator-java/DEVELOPMENT.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ than the "test" phase.
To run integration test for gapic-generator-java, run this Bazel command in the
root of the repository (where you have WORKSPACE file for Bazel.)

*Note* Make sure you run `mvn clean install` to gather any changes you have made before updating the integration tests.

```sh
# In the repository root directory
bazelisk test //... # integration tests
Expand All @@ -73,6 +75,13 @@ bazelisk test //... # integration tests
bazelisk run //test/integration:update_redis
```

- To update all integration tests you can use this command:

```sh
# In the repository root directory
bazelisk run //test/integration:update_asset && bazelisk run //test/integration:update_credentials && bazelisk run //test/integration:update_iam && bazelisk run //test/integration:update_kms && bazelisk run //test/integration:update_pubsub && bazelisk run //test/integration:update_logging && bazelisk run //test/integration:update_redis && bazelisk run //test/integration:update_storage && bazelisk run //test/integration:update_library && bazelisk run //test/integration:update_compute && bazelisk run //test/integration:update_bigtable && bazelisk run //test/integration:update_apigeeconnect
```

## Running the Plugin under googleapis with local gapic-generator-java

For running the Plugin with showcase protos and local gapic-generator-java, see
Expand Down
118 changes: 101 additions & 17 deletions ...in/java/com/google/api/generator/gapic/composer/comment/ServiceClientCommentComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

package com.google.api.generator.gapic.composer.comment;

import static java.util.stream.Collectors.toList;

import com.google.api.generator.engine.ast.CommentStatement;
import com.google.api.generator.engine.ast.JavaDocComment;
import com.google.api.generator.engine.ast.TypeNode;
Expand All @@ -27,6 +29,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Optional;

public class ServiceClientCommentComposer {
Expand All @@ -39,9 +42,6 @@ public class ServiceClientCommentComposer {
private static final String SERVICE_DESCRIPTION_INTRO_STRING =
"This class provides the ability to make remote calls to the backing service through method "
+ "calls that map to API methods. Sample code to get started:";
private static final String SERVICE_DESCRIPTION_SURFACE_SUMMARY_STRING =
"The surface of this class includes several types of Java methods for each of the API's "
+ "methods:";
private static final String SERVICE_DESCRIPTION_SURFACE_CODA_STRING =
"See the individual methods for example code.";
private static final String SERVICE_DESCRIPTION_RESOURCE_NAMES_FORMATTING_STRING =
Expand All @@ -61,18 +61,6 @@ public class ServiceClientCommentComposer {

private static final String METHOD_DESCRIPTION_SAMPLE_CODE_SUMMARY_STRING = "Sample code:";

private static final List<String> SERVICE_DESCRIPTION_SURFACE_DESCRIPTION =
Arrays.asList(
"A \"flattened\" method. With this type of method, the fields of the request type have"
+ " been converted into function parameters. It may be the case that not all fields"
+ " are available as parameters, and not every API method will have a flattened"
+ " method entry point.",
"A \"request object\" method. This type of method only takes one parameter, a request"
+ " object, which must be constructed before the call. Not every API method will"
+ " have a request object method.",
"A \"callable\" method. This type of method takes no parameters and returns an immutable "
+ "API callable object, which can be used to initiate calls to the service.");

// Patterns.
private static final String CREATE_METHOD_STUB_ARG_PATTERN =
"Constructs an instance of %s, using the given stub for making calls. This is for"
Expand Down Expand Up @@ -109,6 +97,7 @@ public class ServiceClientCommentComposer {
+ " operation returned by another API method call.");

public static List<CommentStatement> createClassHeaderComments(
Map<String, List<String>> methodVariantsForClientHeader,
Service service,
String classMethodSampleCode,
String credentialsSampleCode,
Expand All @@ -132,8 +121,14 @@ public static List<CommentStatement> createClassHeaderComments(
classHeaderJavadocBuilder.addParagraph(
String.format(
SERVICE_DESCRIPTION_CLOSE_PATTERN, ClassNames.getServiceClientClassName(service)));
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_SURFACE_SUMMARY_STRING);
classHeaderJavadocBuilder.addOrderedList(SERVICE_DESCRIPTION_SURFACE_DESCRIPTION);

// Build the map of methods and descriptions to create the table in Client Overviews
List<MethodAndVariants> methodAndVariantsList =
service.methods().stream()
.map((Method method) -> createMethodAndVariants(method, methodVariantsForClientHeader))
.collect(toList());

classHeaderJavadocBuilder.addUnescapedComment(createTableOfMethods(methodAndVariantsList));
classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_SURFACE_CODA_STRING);

// Formatting resource names.
Expand Down Expand Up @@ -215,6 +210,95 @@ public static List<CommentStatement> createRpcMethodHeaderComment(
return comments;
}

private static MethodAndVariants createMethodAndVariants(
Method method, Map<String, List<String>> methodVariantsForClientHeader) {
String name = method.name();
String description = method.description();
if (description == null) description = "";
return new MethodAndVariants(name, description, methodVariantsForClientHeader.get(name));
}

private static String createTableOfMethods(List<MethodAndVariants> methodAndVariantsList) {
String FLATTENED_METHODS =
"<p>\"Flattened\" method variants have converted the fields of the request object into function parameters to enable multiple ways to call the same method.</p>\n";
String REQUEST_OBJECT_METHODS =
"<p>Request object method variants only take one parameter, a request object, which must be constructed before the call.</p>\n";
String CALLABLE_METHODS =
"<p>Callable method variants take no parameters and return an immutable API callable object, which can be used to initiate calls to the service.</p>\n";
String ASYNC_METHODS =
"<p>Methods that return long-running operations have \"Async\" method variants that return `OperationFuture`, which is used to track polling of the service.</p>\n";

StringBuilder tableBuilder = new StringBuilder();
tableBuilder
.append("<table>\n")
.append(" <tr>\n")
.append(" <th>Method</th>\n")
.append(" <th>Description</th>\n")
.append(" <th>Method Variants</th>\n");
for (MethodAndVariants method : methodAndVariantsList) {
tableBuilder
.append(" <tr>\n")
.append(" <td>")
.append(method.method)
.append("</td>\n")
.append(" <td>")
.append("<p>" + method.description + "</p>")
.append("</td>\n")
.append(" <td>\n");
generateUnorderedListMethodVariants(
tableBuilder, REQUEST_OBJECT_METHODS, method.requestObjectVariants);
generateUnorderedListMethodVariants(
tableBuilder, FLATTENED_METHODS, method.flattenedVariants);
generateUnorderedListMethodVariants(tableBuilder, ASYNC_METHODS, method.asyncVariants);
generateUnorderedListMethodVariants(tableBuilder, CALLABLE_METHODS, method.callableVariants);
tableBuilder.append(" </td>\n").append(" </tr>\n");
}
tableBuilder.append(" </tr>\n").append(" </table>\n");
return tableBuilder.toString();
}

private static void generateUnorderedListMethodVariants(
StringBuilder tableBuilder, String methodType, List<String> methodVariants) {
if (!methodVariants.isEmpty()) {
tableBuilder
.append(" " + methodType + " ")
.append("<ul>\n")
.append(" <li>")
.append(String.join("\n <li>", methodVariants))
.append("\n")
.append(" </ul>")
.append("\n");
}
}

private static class MethodAndVariants {
private final String method;
// Description may be empty. It comes from the proto comments above the method. If it is empty,
// then nothing will be displayed.
private final String description;

private final List<String> flattenedVariants;
private final List<String> requestObjectVariants;
private final List<String> callableVariants;
private final List<String> asyncVariants;

private MethodAndVariants(String method, String description, List<String> methodVariants) {
this.method = method;
this.description = description;
requestObjectVariants =
methodVariants.stream().filter(s -> s.contains("request")).collect(toList());
// Flattened method variants do not have a suffix, so the easiest way to identify them is by
// removing all other method variant types.
methodVariants.removeAll(requestObjectVariants);
callableVariants =
methodVariants.stream().filter(s -> s.contains("Callable")).collect(toList());
methodVariants.removeAll(callableVariants);
asyncVariants = methodVariants.stream().filter(s -> s.contains("Async")).collect(toList());
methodVariants.removeAll(asyncVariants);
flattenedVariants = methodVariants;
}
}

public static List<CommentStatement> createRpcMethodHeaderComment(
Method method, Optional<String> sampleCodeOpt) {
return createRpcMethodHeaderComment(method, Collections.emptyList(), sampleCodeOpt);
Expand Down
78 changes: 71 additions & 7 deletions ...va/com/google/api/generator/gapic/composer/common/AbstractServiceClientClassComposer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,11 +139,10 @@ public GapicClass generate(GapicContext context, Service service) {

List<Sample> samples = new ArrayList<>();
Map<String, List<String>> grpcRpcsToJavaMethodNames = new HashMap<>();
Map<String, List<String>> methodVariantsForClientHeader = new HashMap<>();

ClassDefinition classDef =
ClassDefinition.builder()
.setHeaderCommentStatements(
createClassHeaderComments(service, typeStore, resourceNames, messageTypes, samples))
.setPackageString(pakkage)
.setAnnotations(createClassAnnotations(service, typeStore))
.setScope(ScopeNode.PUBLIC)
Expand All @@ -158,8 +157,17 @@ public GapicClass generate(GapicContext context, Service service) {
resourceNames,
hasLroClient,
grpcRpcsToJavaMethodNames,
methodVariantsForClientHeader,
samples))
.setNestedClasses(createNestedPagingClasses(service, messageTypes, typeStore))
.setHeaderCommentStatements(
createClassHeaderComments(
methodVariantsForClientHeader,
service,
typeStore,
resourceNames,
messageTypes,
samples))
.build();

updateGapicMetadata(context, service, className, grpcRpcsToJavaMethodNames);
Expand Down Expand Up @@ -189,6 +197,7 @@ private static List<TypeNode> createClassImplements(TypeStore typeStore) {
}

protected List<CommentStatement> createClassHeaderComments(
Map<String, List<String>> methodVariantsForClientHeader,
Service service,
TypeStore typeStore,
Map<String, ResourceName> resourceNames,
Expand All @@ -207,6 +216,7 @@ protected List<CommentStatement> createClassHeaderComments(
clientType, settingsType, service);
samples.addAll(Arrays.asList(classMethodSampleCode, credentialsSampleCode, endpointSampleCode));
return ServiceClientCommentComposer.createClassHeaderComments(
methodVariantsForClientHeader,
service,
SampleCodeWriter.writeInlineSample(classMethodSampleCode.body()),
SampleCodeWriter.writeInlineSample(credentialsSampleCode.body()),
Expand All @@ -223,14 +233,21 @@ private List<MethodDefinition> createClassMethods(
Map<String, ResourceName> resourceNames,
boolean hasLroClient,
Map<String, List<String>> grpcRpcToJavaMethodMetadata,
Map<String, List<String>> methodVariantsForClientHeader,
List<Sample> samples) {
List<MethodDefinition> methods = new ArrayList<>();
methods.addAll(createStaticCreatorMethods(service, typeStore));
methods.addAll(createConstructorMethods(service, typeStore, hasLroClient));
methods.addAll(createGetterMethods(service, typeStore, hasLroClient));
methods.addAll(
createServiceMethods(
service, messageTypes, typeStore, resourceNames, grpcRpcToJavaMethodMetadata, samples));
service,
messageTypes,
typeStore,
resourceNames,
grpcRpcToJavaMethodMetadata,
methodVariantsForClientHeader,
samples));
methods.addAll(createBackgroundResourceMethods(service, typeStore));
return methods;
}
Expand Down Expand Up @@ -561,18 +578,53 @@ private List<MethodDefinition> createGetterMethods(
.collect(Collectors.toList());
}

private static String getJavaMethod(MethodDefinition m) {
StringBuilder methodSignature = new StringBuilder();

// Method name
methodSignature.append(m.methodIdentifier().name()).append("(");

// Iterate through and add all parameters
List<Variable> parameters =
m.arguments().stream().map(VariableExpr::variable).collect(Collectors.toList());

// If reference is empty, that means the parameter is a non-Object type. Therefore, use the
// typeKind directly.
for (int i = 0; i < parameters.size(); i++) {
Variable param = parameters.get(i);
String paramType =
param.type().reference() != null
? param.type().reference().name() + " "
: param.type().typeKind().name().toLowerCase() + " ";
String paramName = param.identifier().name();

methodSignature.append(paramType).append(paramName);

// Add a comma if there are more parameters
if (i < parameters.size() - 1) {
methodSignature.append(", ");
}
}

methodSignature.append(")");

return methodSignature.toString();
}

private static List<MethodDefinition> createServiceMethods(
Service service,
Map<String, Message> messageTypes,
TypeStore typeStore,
Map<String, ResourceName> resourceNames,
Map<String, List<String>> grpcRpcToJavaMethodMetadata,
Map<String, List<String>> methodVariantsForClientHeader,
List<Sample> samples) {
List<MethodDefinition> javaMethods = new ArrayList<>();
Function<MethodDefinition, String> javaMethodNameFn = m -> m.methodIdentifier().name();
for (Method method : service.methods()) {
if (!grpcRpcToJavaMethodMetadata.containsKey(method.name())) {
grpcRpcToJavaMethodMetadata.put(method.name(), new ArrayList<>());
methodVariantsForClientHeader.put(method.name(), new ArrayList<>());
}
if (method.stream().equals(Stream.NONE)) {
List<MethodDefinition> generatedMethods =
Expand All @@ -592,6 +644,14 @@ private static List<MethodDefinition> createServiceMethods(
generatedMethods.stream()
.map(m -> javaMethodNameFn.apply(m))
.collect(Collectors.toList()));

// Collect data for Client header
methodVariantsForClientHeader
.get(method.name())
.addAll(
generatedMethods.stream()
.map(AbstractServiceClientClassComposer::getJavaMethod)
.collect(Collectors.toList()));
javaMethods.addAll(generatedMethods);

MethodDefinition generatedMethod =
Expand All @@ -604,33 +664,37 @@ private static List<MethodDefinition> createServiceMethods(
samples,
service);

// Collect data for gapic_metadata.json.
// Collect data for gapic_metadata.json and client header.
grpcRpcToJavaMethodMetadata.get(method.name()).add(javaMethodNameFn.apply(generatedMethod));
methodVariantsForClientHeader.get(method.name()).add(getJavaMethod(generatedMethod));
javaMethods.add(generatedMethod);
}
if (method.hasLro()) {
MethodDefinition generatedMethod =
createLroCallableMethod(
service, method, typeStore, messageTypes, resourceNames, samples);

// Collect data for gapic_metadata.json.
// Collect data for gapic_metadata.json and client header.
grpcRpcToJavaMethodMetadata.get(method.name()).add(javaMethodNameFn.apply(generatedMethod));
methodVariantsForClientHeader.get(method.name()).add(getJavaMethod(generatedMethod));
javaMethods.add(generatedMethod);
}
if (method.isPaged()) {
MethodDefinition generatedMethod =
createPagedCallableMethod(
service, method, typeStore, messageTypes, resourceNames, samples);

// Collect data for gapic_metadata.json.
// Collect data for gapic_metadata.json and client header.
grpcRpcToJavaMethodMetadata.get(method.name()).add(javaMethodNameFn.apply(generatedMethod));
methodVariantsForClientHeader.get(method.name()).add(getJavaMethod(generatedMethod));
javaMethods.add(generatedMethod);
}
MethodDefinition generatedMethod =
createCallableMethod(service, method, typeStore, messageTypes, resourceNames, samples);

// Collect data for the gapic_metadata.json file.
// Collect data for the gapic_metadata.json file and client header.
grpcRpcToJavaMethodMetadata.get(method.name()).add(javaMethodNameFn.apply(generatedMethod));
methodVariantsForClientHeader.get(method.name()).add(getJavaMethod(generatedMethod));
javaMethods.add(generatedMethod);
}
return javaMethods;
Expand Down
Loading