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

* feat: update javadocs for Client classes to include table of methods and method variants

* fix lint

* update showcase goldens

* remove unnecessary newlines and encapsulate method description within <p> tags

* fix lint

* update showcase goldens

* test remove cache for golden test

* update integration goldens

* adding back in cache

* update integration goldens

* test change to ci

* update ci comment

* update to use linkedhashmap for consistent ordering

* update showcase goldens

* fix lint

* refactor

* refactor

* fix lint and showcase goldens

* update showcase goldens

* include all parameters, not just first one

* include primitive types and update indentation

Author: alicejli

.github/workflows/ci.yaml

@@ -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
@@ -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.

gapic-generator-java/DEVELOPMENT.md

@@ -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
@@ -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

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

@@ -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;
@@ -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 {
@@ -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 =
@@ -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"
@@ -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,
@@ -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.
@@ -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);

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

@@ -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)
@@ -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);
@@ -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,
@@ -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()),
@@ -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;
   }
@@ -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 =
@@ -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 =
@@ -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;