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 16 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
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;

public class ServiceClientCommentComposer {
// Tokens.
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,33 @@ 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
Map<String, String> mapOfMethodsAndDescriptions =
Collections.unmodifiableMap(
service.methods().stream()
.collect(
Collectors.toMap(
Method::getName,
method -> {
String description = method.getDescription();
return description != null ? description : "";
},
(existingValue, newValue) -> existingValue,
LinkedHashMap::new)));

// Build a list of MethodAndVariants to create the table
List<MethodAndVariants> methodAndVariantsList = new ArrayList<>();
for (Map.Entry<String, String> method : mapOfMethodsAndDescriptions.entrySet()) {
MethodAndVariants methodAndVariants =
new MethodAndVariants(
method.getKey(),
method.getValue(),
methodVariantsForClientHeader.get(method.getKey()));
methodAndVariantsList.add(methodAndVariants);
}
alicejli marked this conversation as resolved.
Show resolved Hide resolved

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

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

private static String createTableOfMethods(List<MethodAndVariants> methodAndVariantsList) {
String FLATTENED_METHODS =
"<p>\"Flattened\" method variants have the fields of the request type converted into function parameters to enable multiple ways to call the same method.</p>\n";
alicejli marked this conversation as resolved.
Show resolved Hide resolved
String REQUEST_OBJECT_METHODS =
"<p>Request object method variants only takes one parameter, a request object, which must be constructed before the call.</p>\n";
alicejli marked this conversation as resolved.
Show resolved Hide resolved
String CALLABLE_METHODS =
"<p>Callable method variants take no parameters and returns an immutable API callable object, which can be used to initiate calls to the service.</p>\n";
alicejli marked this conversation as resolved.
Show resolved Hide resolved
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";
alicejli marked this conversation as resolved.
Show resolved Hide resolved

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");
if (method.hasRequestObjectVariants) {
tableBuilder
.append(" " + REQUEST_OBJECT_METHODS + " ")
.append("<ul>\n")
.append(" <li>")
.append(String.join("\n <li>", method.requestObjectVariants))
.append("\n")
.append(" </ul>")
.append("\n");
}
if (method.hasFlattenedVariants) {
tableBuilder
.append(" " + FLATTENED_METHODS + " ")
.append("<ul>\n")
.append(" <li>")
.append(String.join("\n <li>", method.flattenedVariants))
.append("\n")
.append(" </ul>")
.append("\n");
}
if (method.hasAsyncVariants) {
tableBuilder
.append(" " + ASYNC_METHODS + " ")
.append("<ul>\n")
.append(" <li>")
.append(String.join("\n <li>", method.asyncVariants))
.append("\n")
.append(" </ul>")
.append("\n");
}
alicejli marked this conversation as resolved.
Show resolved Hide resolved
if (method.hasCallableVariants) {
tableBuilder
.append(" " + CALLABLE_METHODS + " ")
.append("<ul>\n")
.append(" <li>")
.append(String.join("\n <li>", method.callableVariants))
.append("\n")
.append(" </ul>")
.append("\n");
}
tableBuilder.append(" </td>\n").append(" </tr>\n");
}
tableBuilder.append(" </table>\n");
return tableBuilder.toString();
}

public static class MethodAndVariants {
alicejli marked this conversation as resolved.
Show resolved Hide resolved
String method;
String description;
boolean hasFlattenedVariants = false;
boolean hasRequestObjectVariants;
boolean hasCallableVariants;
boolean hasAsyncVariants;

List<String> flattenedVariants;
List<String> requestObjectVariants;
List<String> callableVariants;
List<String> asyncVariants;
alicejli marked this conversation as resolved.
Show resolved Hide resolved

public MethodAndVariants(String method, String description, List<String> methodVariants) {
this.method = method;
this.description = description;
requestObjectVariants =
methodVariants.stream().filter(s -> s.contains("request")).collect(Collectors.toList());
hasRequestObjectVariants = methodVariants.removeAll(requestObjectVariants);
// hasRequestObjectVariants = methodVariants.stream().anyMatch(s -> s.contains("request"));
callableVariants =
methodVariants.stream().filter(s -> s.contains("Callable")).collect(Collectors.toList());
hasCallableVariants = methodVariants.removeAll(callableVariants);
// hasCallableVariants = methodVariants.stream().anyMatch(s -> s.contains("Callable"));
asyncVariants =
methodVariants.stream().filter(s -> s.contains("Async")).collect(Collectors.toList());
hasAsyncVariants = methodVariants.removeAll(asyncVariants);
// hasAsyncVariants = methodVariants.stream().anyMatch(s -> s.contains("Async"));
alicejli marked this conversation as resolved.
Show resolved Hide resolved
// Whatever is remaining should just be flattened variants
flattenedVariants = methodVariants;
alicejli marked this conversation as resolved.
Show resolved Hide resolved
if (!flattenedVariants.isEmpty()) {
hasFlattenedVariants = true;
}
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 @@ -567,12 +584,35 @@ private static List<MethodDefinition> createServiceMethods(
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();
// This generates the list of method variants with arguments for the Client header statements
Function<MethodDefinition, String> javaMethodFn =
alicejli marked this conversation as resolved.
Show resolved Hide resolved
m -> {
String s;
if (m.arguments().isEmpty()) {
s = m.methodIdentifier().name() + "()";
alicejli marked this conversation as resolved.
Show resolved Hide resolved
} else {
String argument =
m.arguments().get(0).variable().type().reference() != null
? m.arguments().get(0).variable().type().reference().name() + " "
: "";
s =
m.methodIdentifier().name()
+ "("
+ argument
+ m.arguments().get(0).variable().identifier().name()
+ ")";
}
;
return s;
};
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 +632,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(m -> javaMethodFn.apply(m))
.collect(Collectors.toList()));
javaMethods.addAll(generatedMethods);

MethodDefinition generatedMethod =
Expand All @@ -604,33 +652,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(javaMethodFn.apply(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(javaMethodFn.apply(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(javaMethodFn.apply(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(javaMethodFn.apply(generatedMethod));
javaMethods.add(generatedMethod);
}
return javaMethods;
Expand Down
Loading
Loading