[ggj][codegen] fix: ServiceClient member variables and method calls

src/main/java/com/google/api/generator/engine/ast/TypeNode.java

@@ -23,7 +23,7 @@
 import javax.annotation.Nullable;
 
 @AutoValue
-public abstract class TypeNode implements AstNode {
+public abstract class TypeNode implements AstNode, Comparable<TypeNode> {
   static final Reference EXCEPTION_REFERENCE = ConcreteReference.withClazz(Exception.class);
   public static final Reference WILDCARD_REFERENCE = ConcreteReference.wildcard();
 
@@ -88,6 +88,31 @@ public enum TypeKind {
   @Nullable
   public abstract Reference reference();
 
+  @Override
+  public int compareTo(TypeNode other) {
+    // Ascending order of name.
+    if (isPrimitiveType()) {
+      if (other.isPrimitiveType()) {
+        return typeKind().name().compareTo(other.typeKind().name());
+      }
+      // b is a reference type or null, so a < b.
+      return -1;
+    }
+
+    if (this.equals(TypeNode.NULL)) {
+      // Can't self-compare, so we don't need to check whether the other one is TypeNode.NULL.
+      return other.isPrimitiveType() ? 1 : -1;
+    }
+
+    if (other.isPrimitiveType() || other.equals(TypeNode.NULL)) {
+      return 1;
+    }
+
+    // Both are reference types.
+    // TODO(miraleung): Replace this with a proper reference Comaparator.
+    return reference().fullName().compareTo(other.reference().fullName());
+  }
+
   public static Builder builder() {
     return new AutoValue_TypeNode.Builder().setIsArray(false);
   }

src/main/java/com/google/api/generator/gapic/composer/ServiceClientClassComposer.java

@@ -68,6 +68,7 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.concurrent.TimeUnit;
 import java.util.stream.Collectors;
 import javax.annotation.Generated;
@@ -101,6 +102,8 @@ public GapicClass generate(Service service, Map<String, Message> messageTypes) {
 
     ClassDefinition classDef =
         ClassDefinition.builder()
+            .setHeaderCommentStatements(
+                ServiceClientCommentComposer.createClassHeaderComments(service))
             .setPackageString(pakkage)
             .setAnnotations(createClassAnnotations(types))
             .setImplementsTypes(createClassImplements(types))
@@ -132,7 +135,7 @@ private static List<MethodDefinition> createClassMethods(
       boolean hasLroClient) {
     List<MethodDefinition> methods = new ArrayList<>();
     methods.addAll(createStaticCreatorMethods(service, types));
-    methods.addAll(createConstructorMethods(service, types));
+    methods.addAll(createConstructorMethods(service, types, hasLroClient));
     methods.addAll(createGetterMethods(service, types, hasLroClient));
     methods.addAll(createServiceMethods(service, messageTypes, types));
     methods.addAll(createBackgroundResourceMethods(service, types));
@@ -272,7 +275,7 @@ private static List<MethodDefinition> createStaticCreatorMethods(
   }
 
   private static List<MethodDefinition> createConstructorMethods(
-      Service service, Map<String, TypeNode> types) {
+      Service service, Map<String, TypeNode> types, boolean hasLroClient) {
     List<MethodDefinition> methods = new ArrayList<>();
     String thisClientName = String.format("%sClient", service.name());
     String settingsName = String.format("%sSettings", service.name());
@@ -340,7 +343,9 @@ private static List<MethodDefinition> createConstructorMethods(
                     .setReturnType(operationsClientVarExpr.type())
                     .build())
             .build();
-    ctorAssignmentExprs.add(operationsClientAssignExpr);
+    if (hasLroClient) {
+      ctorAssignmentExprs.add(operationsClientAssignExpr);
+    }
 
     methods.add(
         MethodDefinition.constructorBuilder()
@@ -368,7 +373,9 @@ private static List<MethodDefinition> createConstructorMethods(
             .setVariableExpr(stubVarExpr.toBuilder().setExprReferenceExpr(thisExpr).build())
             .setValueExpr(stubVarExpr)
             .build());
-    ctorAssignmentExprs.add(operationsClientAssignExpr);
+    if (hasLroClient) {
+      ctorAssignmentExprs.add(operationsClientAssignExpr);
+    }
     AnnotationNode betaAnnotation =
         AnnotationNode.builder()
             .setType(types.get("BetaApi"))
@@ -471,7 +478,26 @@ private static List<MethodDefinition> createMethodVariants(
     Preconditions.checkNotNull(
         inputMessage, String.format("Message %s not found", methodInputTypeName));
 
-    for (List<MethodArgument> signature : method.methodSignatures()) {
+    // Make the method signature order deterministic, which helps with unit testing and per-version
+    // diffs.
+    List<List<MethodArgument>> sortedMethodSignatures =
+        method.methodSignatures().stream()
+            .sorted(
+                (s1, s2) -> {
+                  if (s1.size() != s2.size()) {
+                    return s1.size() - s2.size();
+                  }
+                  for (int i = 0; i < s1.size(); i++) {
+                    int compareVal = s1.get(i).compareTo(s2.get(i));
+                    if (compareVal != 0) {
+                      return compareVal;
+                    }
+                  }
+                  return 0;
+                })
+            .collect(Collectors.toList());
+
+    for (List<MethodArgument> signature : sortedMethodSignatures) {
       // Get the argument list.
       List<VariableExpr> arguments =
           signature.stream()
@@ -512,8 +538,8 @@ private static List<MethodDefinition> createMethodVariants(
         if (argument.isResourceNameHelper()) {
           MethodInvocationExpr isNullCheckExpr =
               MethodInvocationExpr.builder()
-                  .setStaticReferenceType(types.get("Strings"))
-                  .setMethodName("isNullOrEmpty")
+                  .setStaticReferenceType(types.get("Objects"))
+                  .setMethodName("isNull")
                   .setArguments(Arrays.asList(argVarExpr))
                   .setReturnType(TypeNode.BOOLEAN)
                   .build();
@@ -563,6 +589,8 @@ private static List<MethodDefinition> createMethodVariants(
 
       javaMethods.add(
           MethodDefinition.builder()
+              .setHeaderCommentStatements(
+                  ServiceClientCommentComposer.createRpcMethodHeaderComment(method, signature))
               .setScope(ScopeNode.PUBLIC)
               .setIsFinal(true)
               .setReturnType(methodOutputType)
@@ -593,6 +621,8 @@ private static List<MethodDefinition> createMethodVariants(
             .build();
     javaMethods.add(
         MethodDefinition.builder()
+            .setHeaderCommentStatements(
+                ServiceClientCommentComposer.createRpcMethodHeaderComment(method))
             .setScope(ScopeNode.PUBLIC)
             .setIsFinal(true)
             .setReturnType(methodOutputType)
@@ -606,6 +636,7 @@ private static List<MethodDefinition> createMethodVariants(
 
   private static MethodDefinition createLroAsyncMethod(
       String serviceName, Method method, Map<String, TypeNode> types) {
+    // TODO(miraleung): Create variants as well.
     Preconditions.checkState(
         method.hasLro(), String.format("Method %s does not have an LRO", method.name()));
     String methodName = JavaStyle.toLowerCamelCase(method.name());
@@ -640,6 +671,8 @@ private static MethodDefinition createLroAsyncMethod(
             .build();
 
     return MethodDefinition.builder()
+        .setHeaderCommentStatements(
+            ServiceClientCommentComposer.createRpcMethodHeaderComment(method))
         .setScope(ScopeNode.PUBLIC)
         .setIsFinal(true)
         .setReturnType(returnType)
@@ -711,6 +744,8 @@ private static MethodDefinition createCallableMethod(
             .build();
 
     return MethodDefinition.builder()
+        .setHeaderCommentStatements(
+            ServiceClientCommentComposer.createRpcCallableMethodHeaderComment(method))
         .setScope(ScopeNode.PUBLIC)
         .setIsFinal(true)
         .setName(methodName)
@@ -863,6 +898,7 @@ private static Map<String, TypeNode> createConcreteTypes() {
             InterruptedException.class,
             IOException.class,
             MoreExecutors.class,
+            Objects.class,
             Operation.class,
             OperationFuture.class,
             OperationCallable.class,

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

@@ -17,14 +17,70 @@
 import com.google.api.generator.engine.ast.CommentStatement;
 import com.google.api.generator.engine.ast.JavaDocComment;
 import com.google.api.generator.engine.ast.TypeNode;
+import com.google.api.generator.gapic.model.Method;
+import com.google.api.generator.gapic.model.MethodArgument;
+import com.google.api.generator.gapic.model.Service;
+import com.google.api.generator.gapic.utils.JavaStyle;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.List;
 
 class ServiceClientCommentComposer {
+  // Tokens.
   private static final String COLON = ":";
+  private static final String EMPTY_STRING = "";
+  private static final String API_EXCEPTION_TYPE_NAME = "com.google.api.gax.rpc.ApiException";
+  private static final String EXCEPTION_CONDITION = "if the remote call fails";
 
+  // Constants.
+  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_CLOSE_STRING =
+      "Note: close() needs to be called on the echoClient object to clean up resources such as "
+          + "threads. In the example above, try-with-resources is used, which automatically calls "
+          + "close().";
+  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 =
+      "Many parameters require resource names to be formatted in a particular way. To assist with"
+          + " these names, this class includes a format method for each type of name, and"
+          + " additionally a parse  method to extract the individual identifiers contained within"
+          + " names that are returned.";
+  private static final String SERVICE_DESCRIPTION_CREDENTIALS_SUMMARY_STRING =
+      "To customize credentials:";
+  private static final String SERVICE_DESCRIPTION_ENDPOINT_SUMMARY_STRING =
+      "To customize the endpoint:";
+
+  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 EchoClient, using the given stub for making calls. This is for"
           + " advanced usage - prefer using create(%s).";
 
+  private static final String SERVICE_DESCRIPTION_CUSTOMIZE_SUMMARY_PATTERN =
+      "This class can be customized by passing in a custom instance of %s to create(). For"
+          + " example:";
+
+  private static final String SERVICE_DESCRIPTION_SUMMARY_PATTERN = "Service Description: %s";
+
+  // Comments.
   static final CommentStatement CREATE_METHOD_NO_ARG_COMMENT =
       toSimpleComment("Constructs an instance of EchoClient with default settings.");
 
@@ -45,11 +101,93 @@ class ServiceClientCommentComposer {
           "Returns the OperationsClient that can be used to query the status of a long-running"
               + " operation returned by another API method call.");
 
+  static List<CommentStatement> createClassHeaderComments(Service service) {
+    JavaDocComment.Builder classHeaderJavadocBuilder = JavaDocComment.builder();
+    if (service.hasDescription()) {
+      classHeaderJavadocBuilder.addComment(
+          String.format(SERVICE_DESCRIPTION_SUMMARY_PATTERN, service.description()));
+    }
+
+    // Service introduction.
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_INTRO_STRING);
+    // TODO(summerji): Add sample code here.
+
+    // API surface description.
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_CLOSE_STRING);
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_SURFACE_SUMMARY_STRING);
+    classHeaderJavadocBuilder.addOrderedList(SERVICE_DESCRIPTION_SURFACE_DESCRIPTION);
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_SURFACE_CODA_STRING);
+
+    // Formatting resource names.
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_RESOURCE_NAMES_FORMATTING_STRING);
+
+    // Customization examples.
+    classHeaderJavadocBuilder.addParagraph(
+        String.format(
+            SERVICE_DESCRIPTION_CUSTOMIZE_SUMMARY_PATTERN,
+            String.format("%sSettings", JavaStyle.toUpperCamelCase(service.name()))));
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_CREDENTIALS_SUMMARY_STRING);
+    // TODO(summerji): Add credentials' customization sample code here.
+    classHeaderJavadocBuilder.addParagraph(SERVICE_DESCRIPTION_ENDPOINT_SUMMARY_STRING);
+    // TODO(summerji): Add endpoint customization sample code here.
+
+    return Arrays.asList(
+        CommentComposer.AUTO_GENERATED_CLASS_COMMENT,
+        CommentStatement.withComment(classHeaderJavadocBuilder.build()));
+  }
+
   static CommentStatement createCreateMethodStubArgComment(TypeNode settingsType) {
     return toSimpleComment(
         String.format(CREATE_METHOD_STUB_ARG_PATTERN, settingsType.reference().name()));
   }
 
+  static List<CommentStatement> createRpcMethodHeaderComment(
+      Method method, List<MethodArgument> methodArguments) {
+    JavaDocComment.Builder methodJavadocBuilder = JavaDocComment.builder();
+
+    if (method.hasDescription()) {
+      methodJavadocBuilder.addComment(method.description());
+    }
+
+    methodJavadocBuilder.addParagraph(METHOD_DESCRIPTION_SAMPLE_CODE_SUMMARY_STRING);
+    // TODO(summerji): Add sample code here.
+
+    if (methodArguments.isEmpty()) {
+      methodJavadocBuilder.addParam(
+          "request", "The request object containing all of the parameters for the API call.");
+    } else {
+      for (MethodArgument argument : methodArguments) {
+        methodJavadocBuilder.addParam(
+            argument.name(), argument.hasDescription() ? argument.description() : EMPTY_STRING);
+      }
+    }
+
+    methodJavadocBuilder.setThrows(API_EXCEPTION_TYPE_NAME, EXCEPTION_CONDITION);
+
+    return Arrays.asList(
+        CommentComposer.AUTO_GENERATED_METHOD_COMMENT,
+        CommentStatement.withComment(methodJavadocBuilder.build()));
+  }
+
+  static List<CommentStatement> createRpcMethodHeaderComment(Method method) {
+    return createRpcMethodHeaderComment(method, Collections.emptyList());
+  }
+
+  static List<CommentStatement> createRpcCallableMethodHeaderComment(Method method) {
+    JavaDocComment.Builder methodJavadocBuilder = JavaDocComment.builder();
+
+    if (method.hasDescription()) {
+      methodJavadocBuilder.addComment(method.description());
+    }
+
+    methodJavadocBuilder.addParagraph(METHOD_DESCRIPTION_SAMPLE_CODE_SUMMARY_STRING);
+    // TODO(summerji): Add sample code here.
+
+    return Arrays.asList(
+        CommentComposer.AUTO_GENERATED_METHOD_COMMENT,
+        CommentStatement.withComment(methodJavadocBuilder.build()));
+  }
+
   private static CommentStatement toSimpleComment(String comment) {
     return CommentStatement.withComment(JavaDocComment.withComment(comment));
   }

src/main/java/com/google/api/generator/gapic/model/Field.java

@@ -31,6 +31,13 @@ public abstract class Field {
   @Nullable
   public abstract ResourceReference resourceReference();
 
+  @Nullable
+  public abstract String description();
+
+  public boolean hasDescription() {
+    return description() != null;
+  }
+
   public boolean hasResourceReference() {
     return type().equals(TypeNode.STRING) && resourceReference() != null;
   }
@@ -51,6 +58,8 @@ public abstract static class Builder {
 
     public abstract Builder setResourceReference(ResourceReference resourceReference);
 
+    public abstract Builder setDescription(String description);
+
     public abstract Field build();
   }
 }