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

fix: allow empty services and java keywords as a method names #985

Merged
merged 4 commits into from
Apr 29, 2022
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
8 changes: 8 additions & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,14 @@ GOLDEN_UPDATING_UNIT_TESTS = [
"com.google.api.generator.gapic.composer.grpc.ServiceSettingsClassComposerTest",
"com.google.api.generator.gapic.composer.grpc.ServiceStubClassComposerTest",
"com.google.api.generator.gapic.composer.grpc.ServiceStubSettingsClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.GrpcServiceCallableFactoryClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.GrpcServiceStubClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceCallableFactoryClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceClientTestClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceStubClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.ServiceClientClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.ServiceClientTestClassComposerTest",
"com.google.api.generator.gapic.composer.grpcrest.ServiceSettingsClassComposerTest",
"com.google.api.generator.gapic.composer.resourcename.ResourceNameHelperClassComposerTest",
"com.google.api.generator.gapic.composer.rest.HttpJsonServiceCallableFactoryClassComposerTest",
"com.google.api.generator.gapic.composer.rest.HttpJsonServiceStubClassComposerTest",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,7 @@ private static List<CommentStatement> createClassHeaderComments(
.filter(m -> m.stream() == Stream.NONE && !m.hasLro() && !m.isPaged())
.findFirst()
.orElse(service.methods().get(0)));
Optional<String> methodNameOpt =
methodOpt.isPresent() ? Optional.of(methodOpt.get().name()) : Optional.empty();
Optional<String> methodNameOpt = methodOpt.map(Method::name);
Optional<Sample> sampleCode =
SettingsSampleComposer.composeSettingsSample(
methodNameOpt, ClassNames.getServiceSettingsClassName(service), classType);
Expand Down Expand Up @@ -437,7 +436,7 @@ private static Map<String, VariableExpr> createMethodSettingsClassMemberVarExprs
!Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method);
TypeNode settingsType =
getCallSettingsType(method, typeStore, hasBatchingSettings, isNestedClass);
String varName = JavaStyle.toLowerCamelCase(String.format("%sSettings", method.name()));
String varName = String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name()));
if (method.isDeprecated()) {
deprecatedSettingVarNames.add(varName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,18 @@ public static Sample composeClassHeaderSample(
TypeNode clientType,
Map<String, ResourceName> resourceNames,
Map<String, Message> messageTypes) {
if (service.methods().isEmpty()) {
return ServiceClientMethodSampleComposer.composeEmptyServiceSample(clientType);
}

// Use the first pure unary RPC method's sample code as showcase, if no such method exists, use
// the first method in the service's methods list.
Method method =
service.methods().stream()
.filter(m -> m.stream() == Method.Stream.NONE && !m.hasLro() && !m.isPaged())
.findFirst()
.orElse(service.methods().get(0));

if (method.stream() == Method.Stream.NONE) {
if (method.methodSignatures().isEmpty()) {
return ServiceClientMethodSampleComposer.composeCanonicalSample(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,36 @@
import java.util.stream.Collectors;

public class ServiceClientMethodSampleComposer {
// Creates an example for an empty service (no API methods), which is a corner case but can
// happen. Generated example will only show how to instantiate the client class but will not call
// any API methods (because there are no API methods).
public static Sample composeEmptyServiceSample(TypeNode clientType) {
VariableExpr clientVarExpr =
VariableExpr.withVariable(
Variable.builder()
.setName(JavaStyle.toLowerCamelCase(clientType.reference().name()))
.setType(clientType)
.build());

List<Statement> bodyStatements = new ArrayList<>();

RegionTag regionTag =
RegionTag.builder()
.setServiceName(clientVarExpr.variable().identifier().name())
.setRpcName("emtpy")
.build();

List<Statement> body =
Arrays.asList(
TryCatchStatement.builder()
.setTryResourceExpr(
SampleComposerUtil.assignClientVariableWithCreateMethodExpr(clientVarExpr))
.setTryBody(bodyStatements)
.setIsSampleCode(true)
.build());
return Sample.builder().setBody(body).setRegionTag(regionTag).setIsCanonical(true).build();
}

public static Sample composeCanonicalSample(
Method method,
TypeNode clientType,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.google.api.generator.gapic.utils;

import com.google.api.generator.engine.lexicon.Keyword;
import com.google.common.base.CaseFormat;
import com.google.common.base.Strings;
import java.util.stream.IntStream;
Expand All @@ -32,8 +33,13 @@ public static String toLowerCamelCase(String s) {
s = CaseFormat.LOWER_UNDERSCORE.to(CaseFormat.UPPER_CAMEL, s);
}

return capitalizeLettersAfterDigits(
String.format("%s%s", s.substring(0, 1).toLowerCase(), s.substring(1)));
String name =
capitalizeLettersAfterDigits(
String.format("%s%s", s.substring(0, 1).toLowerCase(), s.substring(1)));

// Some APIs use legit java keywords as method names. Both protobuf and gGRPC add an underscore
// in generated stubs to resolve name conflict, so we need to do the same.
return Keyword.isKeyword(name) ? name + '_' : name;
}

public static String toUpperCamelCase(String s) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,4 +37,17 @@ public void generateServiceClasses() {
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "EchoClient.golden");
Assert.assertCodeEquals(goldenFilePath, visitor.write());
}

@Test
public void generateServiceClassesEmpty() {
GapicContext context = GrpcRestTestProtoLoader.instance().parseShowcaseEcho();
Service echoProtoService = context.services().get(1);
GapicClass clazz = ServiceClientClassComposer.instance().generate(context, echoProtoService);

JavaWriterVisitor visitor = new JavaWriterVisitor();
clazz.classDefinition().accept(visitor);
Utils.saveCodegenToFile(this.getClass(), "EchoEmpty.golden", visitor.write());
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "EchoEmpty.golden");
Assert.assertCodeEquals(goldenFilePath, visitor.write());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
package com.google.showcase.grpcrest.v1beta1;

import com.google.api.core.BetaApi;
import com.google.api.gax.core.BackgroundResource;
import com.google.showcase.grpcrest.v1beta1.stub.EchoEmpyStub;
import com.google.showcase.grpcrest.v1beta1.stub.EchoEmpyStubSettings;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
import javax.annotation.Generated;

// AUTO-GENERATED DOCUMENTATION AND CLASS.
/**
* 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:
*
* <pre>{@code
* // This snippet has been automatically generated for illustrative purposes only.
* // It may require modifications to work in your environment.
* try (EchoEmpyClient echoEmpyClient = EchoEmpyClient.create()) {}
* }</pre>
*
* <p>Note: close() needs to be called on the EchoEmpyClient object to clean up resources such as
* threads. In the example above, try-with-resources is used, which automatically calls close().
*
* <p>The surface of this class includes several types of Java methods for each of the API's
* methods:
*
* <ol>
* <li>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.
* <li>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.
* <li>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.
* </ol>
*
* <p>See the individual methods for example code.
*
* <p>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.
*
* <p>This class can be customized by passing in a custom instance of EchoEmpySettings to create().
* For example:
*
* <p>To customize credentials:
*
* <pre>{@code
* // This snippet has been automatically generated for illustrative purposes only.
* // It may require modifications to work in your environment.
* EchoEmpySettings echoEmpySettings =
* EchoEmpySettings.newBuilder()
* .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials))
* .build();
* EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
* }</pre>
*
* <p>To customize the endpoint:
*
* <pre>{@code
* // This snippet has been automatically generated for illustrative purposes only.
* // It may require modifications to work in your environment.
* EchoEmpySettings echoEmpySettings =
* EchoEmpySettings.newBuilder().setEndpoint(myEndpoint).build();
* EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
* }</pre>
*
* <p>To use REST (HTTP1.1/JSON) transport (instead of gRPC) for sending an receiving requests over
* the wire:
*
* <pre>{@code
* // This snippet has been automatically generated for illustrative purposes only.
* // It may require modifications to work in your environment.
* EchoEmpySettings echoEmpySettings =
* EchoEmpySettings.newBuilder()
* .setTransportChannelProvider(
* EchoEmpySettings.defaultHttpJsonTransportProviderBuilder().build())
* .build();
* EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
* }</pre>
*
* <p>Please refer to the GitHub repository's samples for more quickstart code snippets.
*/
@BetaApi
@Generated("by gapic-generator-java")
public class EchoEmpyClient implements BackgroundResource {
private final EchoEmpySettings settings;
private final EchoEmpyStub stub;

/** Constructs an instance of EchoEmpyClient with default settings. */
public static final EchoEmpyClient create() throws IOException {
return create(EchoEmpySettings.newBuilder().build());
}

/**
* Constructs an instance of EchoEmpyClient, using the given settings. The channels are created
* based on the settings passed in, or defaults for any settings that are not set.
*/
public static final EchoEmpyClient create(EchoEmpySettings settings) throws IOException {
return new EchoEmpyClient(settings);
}

/**
* Constructs an instance of EchoEmpyClient, using the given stub for making calls. This is for
* advanced usage - prefer using create(EchoEmpySettings).
*/
@BetaApi("A restructuring of stub classes is planned, so this may break in the future")
public static final EchoEmpyClient create(EchoEmpyStub stub) {
return new EchoEmpyClient(stub);
}

/**
* Constructs an instance of EchoEmpyClient, using the given settings. This is protected so that
* it is easy to make a subclass, but otherwise, the static factory methods should be preferred.
*/
protected EchoEmpyClient(EchoEmpySettings settings) throws IOException {
this.settings = settings;
this.stub = ((EchoEmpyStubSettings) settings.getStubSettings()).createStub();
}

@BetaApi("A restructuring of stub classes is planned, so this may break in the future")
protected EchoEmpyClient(EchoEmpyStub stub) {
this.settings = null;
this.stub = stub;
}

public final EchoEmpySettings getSettings() {
return settings;
}

@BetaApi("A restructuring of stub classes is planned, so this may break in the future")
public EchoEmpyStub getStub() {
return stub;
}

@Override
public final void close() {
stub.close();
}

@Override
public void shutdown() {
stub.shutdown();
}

@Override
public boolean isShutdown() {
return stub.isShutdown();
}

@Override
public boolean isTerminated() {
return stub.isTerminated();
}

@Override
public void shutdownNow() {
stub.shutdownNow();
}

@Override
public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException {
return stub.awaitTermination(duration, unit);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -111,4 +111,14 @@ public void acronyms() {
assertEquals("iamHttpXmlDog", JavaStyle.toLowerCamelCase(value));
assertEquals("IamHttpXmlDog", JavaStyle.toUpperCamelCase(value));
}

@Test
public void keyword() {
String value = "import";
assertEquals("import_", JavaStyle.toLowerCamelCase(value));
assertEquals("Import", JavaStyle.toUpperCamelCase(value));
value = "IMPORT_";
assertEquals("import_", JavaStyle.toLowerCamelCase(value));
assertEquals("Import", JavaStyle.toUpperCamelCase(value));
}
}
5 changes: 5 additions & 0 deletions src/test/proto/echo_grpcrest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,11 @@ service Echo {
}
}

// Generator should not fail when encounter a service without methods
service EchoEmpy {
option (google.api.default_host) = "localhost:7469";
}

// A severity enum used to test enum capabilities in GAPIC surfaces
enum Severity {
UNNECESSARY = 0;
Expand Down