Skip to content
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.

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

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

Merged
merged 23 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
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
alicejli marked this conversation as resolved.
Show resolved Hide resolved
```

## 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
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;
alicejli marked this conversation as resolved.
Show resolved Hide resolved

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;
alicejli marked this conversation as resolved.
Show resolved Hide resolved
}
}

public static List<CommentStatement> createRpcMethodHeaderComment(
Method method, Optional<String> sampleCodeOpt) {
return createRpcMethodHeaderComment(method, Collections.emptyList(), sampleCodeOpt);
Expand Down
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
Loading