Skip to content

Commit e37893c

Browse files
authored
fix: allow empty services and java keywords as a method names (#985)
This is specifically needed to enable `sqladmin` API, which has the following issues: 1) Using `import`, reserved java keyword, as a method name: https://github.com/googleapis/googleapis/blob/master/google/cloud/sql/v1/cloud_sql_instances.proto#L109 2) Has an empty service (no methods whatsoevver) defined: https://github.com/googleapis/googleapis/blob/master/google/cloud/sql/v1/cloud_sql_instance_names.proto#L31 I plan to add a full integration tests with sqladmin (which will also test pure REGAPIC case) once this is pushed (need to integrate these changes first for technical reasons).
1 parent 04a6665 commit e37893c

File tree

9 files changed

+248
-5
lines changed

9 files changed

+248
-5
lines changed

BUILD.bazel

+8
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,14 @@ GOLDEN_UPDATING_UNIT_TESTS = [
192192
"com.google.api.generator.gapic.composer.grpc.ServiceSettingsClassComposerTest",
193193
"com.google.api.generator.gapic.composer.grpc.ServiceStubClassComposerTest",
194194
"com.google.api.generator.gapic.composer.grpc.ServiceStubSettingsClassComposerTest",
195+
"com.google.api.generator.gapic.composer.grpcrest.GrpcServiceCallableFactoryClassComposerTest",
196+
"com.google.api.generator.gapic.composer.grpcrest.GrpcServiceStubClassComposerTest",
197+
"com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceCallableFactoryClassComposerTest",
198+
"com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceClientTestClassComposerTest",
199+
"com.google.api.generator.gapic.composer.grpcrest.HttpJsonServiceStubClassComposerTest",
200+
"com.google.api.generator.gapic.composer.grpcrest.ServiceClientClassComposerTest",
201+
"com.google.api.generator.gapic.composer.grpcrest.ServiceClientTestClassComposerTest",
202+
"com.google.api.generator.gapic.composer.grpcrest.ServiceSettingsClassComposerTest",
195203
"com.google.api.generator.gapic.composer.resourcename.ResourceNameHelperClassComposerTest",
196204
"com.google.api.generator.gapic.composer.rest.HttpJsonServiceCallableFactoryClassComposerTest",
197205
"com.google.api.generator.gapic.composer.rest.HttpJsonServiceStubClassComposerTest",

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

+2-3
Original file line numberDiff line numberDiff line change
@@ -392,8 +392,7 @@ private static List<CommentStatement> createClassHeaderComments(
392392
.filter(m -> m.stream() == Stream.NONE && !m.hasLro() && !m.isPaged())
393393
.findFirst()
394394
.orElse(service.methods().get(0)));
395-
Optional<String> methodNameOpt =
396-
methodOpt.isPresent() ? Optional.of(methodOpt.get().name()) : Optional.empty();
395+
Optional<String> methodNameOpt = methodOpt.map(Method::name);
397396
Optional<Sample> sampleCode =
398397
SettingsSampleComposer.composeSettingsSample(
399398
methodNameOpt, ClassNames.getServiceSettingsClassName(service), classType);
@@ -437,7 +436,7 @@ private static Map<String, VariableExpr> createMethodSettingsClassMemberVarExprs
437436
!Objects.isNull(serviceConfig) && serviceConfig.hasBatchingSetting(service, method);
438437
TypeNode settingsType =
439438
getCallSettingsType(method, typeStore, hasBatchingSettings, isNestedClass);
440-
String varName = JavaStyle.toLowerCamelCase(String.format("%sSettings", method.name()));
439+
String varName = String.format("%sSettings", JavaStyle.toLowerCamelCase(method.name()));
441440
if (method.isDeprecated()) {
442441
deprecatedSettingVarNames.add(varName);
443442
}

src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.java

+5
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,18 @@ public static Sample composeClassHeaderSample(
5151
TypeNode clientType,
5252
Map<String, ResourceName> resourceNames,
5353
Map<String, Message> messageTypes) {
54+
if (service.methods().isEmpty()) {
55+
return ServiceClientMethodSampleComposer.composeEmptyServiceSample(clientType);
56+
}
57+
5458
// Use the first pure unary RPC method's sample code as showcase, if no such method exists, use
5559
// the first method in the service's methods list.
5660
Method method =
5761
service.methods().stream()
5862
.filter(m -> m.stream() == Method.Stream.NONE && !m.hasLro() && !m.isPaged())
5963
.findFirst()
6064
.orElse(service.methods().get(0));
65+
6166
if (method.stream() == Method.Stream.NONE) {
6267
if (method.methodSignatures().isEmpty()) {
6368
return ServiceClientMethodSampleComposer.composeCanonicalSample(

src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientMethodSampleComposer.java

+30
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,36 @@
4242
import java.util.stream.Collectors;
4343

4444
public class ServiceClientMethodSampleComposer {
45+
// Creates an example for an empty service (no API methods), which is a corner case but can
46+
// happen. Generated example will only show how to instantiate the client class but will not call
47+
// any API methods (because there are no API methods).
48+
public static Sample composeEmptyServiceSample(TypeNode clientType) {
49+
VariableExpr clientVarExpr =
50+
VariableExpr.withVariable(
51+
Variable.builder()
52+
.setName(JavaStyle.toLowerCamelCase(clientType.reference().name()))
53+
.setType(clientType)
54+
.build());
55+
56+
List<Statement> bodyStatements = new ArrayList<>();
57+
58+
RegionTag regionTag =
59+
RegionTag.builder()
60+
.setServiceName(clientVarExpr.variable().identifier().name())
61+
.setRpcName("emtpy")
62+
.build();
63+
64+
List<Statement> body =
65+
Arrays.asList(
66+
TryCatchStatement.builder()
67+
.setTryResourceExpr(
68+
SampleComposerUtil.assignClientVariableWithCreateMethodExpr(clientVarExpr))
69+
.setTryBody(bodyStatements)
70+
.setIsSampleCode(true)
71+
.build());
72+
return Sample.builder().setBody(body).setRegionTag(regionTag).setIsCanonical(true).build();
73+
}
74+
4575
public static Sample composeCanonicalSample(
4676
Method method,
4777
TypeNode clientType,

src/main/java/com/google/api/generator/gapic/utils/JavaStyle.java

+8-2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

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

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

35-
return capitalizeLettersAfterDigits(
36-
String.format("%s%s", s.substring(0, 1).toLowerCase(), s.substring(1)));
36+
String name =
37+
capitalizeLettersAfterDigits(
38+
String.format("%s%s", s.substring(0, 1).toLowerCase(), s.substring(1)));
39+
40+
// Some APIs use legit java keywords as method names. Both protobuf and gGRPC add an underscore
41+
// in generated stubs to resolve name conflict, so we need to do the same.
42+
return Keyword.isKeyword(name) ? name + '_' : name;
3743
}
3844

3945
public static String toUpperCamelCase(String s) {

src/test/java/com/google/api/generator/gapic/composer/grpcrest/ServiceClientClassComposerTest.java

+13
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,17 @@ public void generateServiceClasses() {
3737
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "EchoClient.golden");
3838
Assert.assertCodeEquals(goldenFilePath, visitor.write());
3939
}
40+
41+
@Test
42+
public void generateServiceClassesEmpty() {
43+
GapicContext context = GrpcRestTestProtoLoader.instance().parseShowcaseEcho();
44+
Service echoProtoService = context.services().get(1);
45+
GapicClass clazz = ServiceClientClassComposer.instance().generate(context, echoProtoService);
46+
47+
JavaWriterVisitor visitor = new JavaWriterVisitor();
48+
clazz.classDefinition().accept(visitor);
49+
Utils.saveCodegenToFile(this.getClass(), "EchoEmpty.golden", visitor.write());
50+
Path goldenFilePath = Paths.get(Utils.getGoldenDir(this.getClass()), "EchoEmpty.golden");
51+
Assert.assertCodeEquals(goldenFilePath, visitor.write());
52+
}
4053
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
package com.google.showcase.grpcrest.v1beta1;
2+
3+
import com.google.api.core.BetaApi;
4+
import com.google.api.gax.core.BackgroundResource;
5+
import com.google.showcase.grpcrest.v1beta1.stub.EchoEmpyStub;
6+
import com.google.showcase.grpcrest.v1beta1.stub.EchoEmpyStubSettings;
7+
import java.io.IOException;
8+
import java.util.concurrent.TimeUnit;
9+
import javax.annotation.Generated;
10+
11+
// AUTO-GENERATED DOCUMENTATION AND CLASS.
12+
/**
13+
* This class provides the ability to make remote calls to the backing service through method calls
14+
* that map to API methods. Sample code to get started:
15+
*
16+
* <pre>{@code
17+
* // This snippet has been automatically generated for illustrative purposes only.
18+
* // It may require modifications to work in your environment.
19+
* try (EchoEmpyClient echoEmpyClient = EchoEmpyClient.create()) {}
20+
* }</pre>
21+
*
22+
* <p>Note: close() needs to be called on the EchoEmpyClient object to clean up resources such as
23+
* threads. In the example above, try-with-resources is used, which automatically calls close().
24+
*
25+
* <p>The surface of this class includes several types of Java methods for each of the API's
26+
* methods:
27+
*
28+
* <ol>
29+
* <li>A "flattened" method. With this type of method, the fields of the request type have been
30+
* converted into function parameters. It may be the case that not all fields are available as
31+
* parameters, and not every API method will have a flattened method entry point.
32+
* <li>A "request object" method. This type of method only takes one parameter, a request object,
33+
* which must be constructed before the call. Not every API method will have a request object
34+
* method.
35+
* <li>A "callable" method. This type of method takes no parameters and returns an immutable API
36+
* callable object, which can be used to initiate calls to the service.
37+
* </ol>
38+
*
39+
* <p>See the individual methods for example code.
40+
*
41+
* <p>Many parameters require resource names to be formatted in a particular way. To assist with
42+
* these names, this class includes a format method for each type of name, and additionally a parse
43+
* method to extract the individual identifiers contained within names that are returned.
44+
*
45+
* <p>This class can be customized by passing in a custom instance of EchoEmpySettings to create().
46+
* For example:
47+
*
48+
* <p>To customize credentials:
49+
*
50+
* <pre>{@code
51+
* // This snippet has been automatically generated for illustrative purposes only.
52+
* // It may require modifications to work in your environment.
53+
* EchoEmpySettings echoEmpySettings =
54+
* EchoEmpySettings.newBuilder()
55+
* .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials))
56+
* .build();
57+
* EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
58+
* }</pre>
59+
*
60+
* <p>To customize the endpoint:
61+
*
62+
* <pre>{@code
63+
* // This snippet has been automatically generated for illustrative purposes only.
64+
* // It may require modifications to work in your environment.
65+
* EchoEmpySettings echoEmpySettings =
66+
* EchoEmpySettings.newBuilder().setEndpoint(myEndpoint).build();
67+
* EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
68+
* }</pre>
69+
*
70+
* <p>To use REST (HTTP1.1/JSON) transport (instead of gRPC) for sending an receiving requests over
71+
* the wire:
72+
*
73+
* <pre>{@code
74+
* // This snippet has been automatically generated for illustrative purposes only.
75+
* // It may require modifications to work in your environment.
76+
* EchoEmpySettings echoEmpySettings =
77+
* EchoEmpySettings.newBuilder()
78+
* .setTransportChannelProvider(
79+
* EchoEmpySettings.defaultHttpJsonTransportProviderBuilder().build())
80+
* .build();
81+
* EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
82+
* }</pre>
83+
*
84+
* <p>Please refer to the GitHub repository's samples for more quickstart code snippets.
85+
*/
86+
@BetaApi
87+
@Generated("by gapic-generator-java")
88+
public class EchoEmpyClient implements BackgroundResource {
89+
private final EchoEmpySettings settings;
90+
private final EchoEmpyStub stub;
91+
92+
/** Constructs an instance of EchoEmpyClient with default settings. */
93+
public static final EchoEmpyClient create() throws IOException {
94+
return create(EchoEmpySettings.newBuilder().build());
95+
}
96+
97+
/**
98+
* Constructs an instance of EchoEmpyClient, using the given settings. The channels are created
99+
* based on the settings passed in, or defaults for any settings that are not set.
100+
*/
101+
public static final EchoEmpyClient create(EchoEmpySettings settings) throws IOException {
102+
return new EchoEmpyClient(settings);
103+
}
104+
105+
/**
106+
* Constructs an instance of EchoEmpyClient, using the given stub for making calls. This is for
107+
* advanced usage - prefer using create(EchoEmpySettings).
108+
*/
109+
@BetaApi("A restructuring of stub classes is planned, so this may break in the future")
110+
public static final EchoEmpyClient create(EchoEmpyStub stub) {
111+
return new EchoEmpyClient(stub);
112+
}
113+
114+
/**
115+
* Constructs an instance of EchoEmpyClient, using the given settings. This is protected so that
116+
* it is easy to make a subclass, but otherwise, the static factory methods should be preferred.
117+
*/
118+
protected EchoEmpyClient(EchoEmpySettings settings) throws IOException {
119+
this.settings = settings;
120+
this.stub = ((EchoEmpyStubSettings) settings.getStubSettings()).createStub();
121+
}
122+
123+
@BetaApi("A restructuring of stub classes is planned, so this may break in the future")
124+
protected EchoEmpyClient(EchoEmpyStub stub) {
125+
this.settings = null;
126+
this.stub = stub;
127+
}
128+
129+
public final EchoEmpySettings getSettings() {
130+
return settings;
131+
}
132+
133+
@BetaApi("A restructuring of stub classes is planned, so this may break in the future")
134+
public EchoEmpyStub getStub() {
135+
return stub;
136+
}
137+
138+
@Override
139+
public final void close() {
140+
stub.close();
141+
}
142+
143+
@Override
144+
public void shutdown() {
145+
stub.shutdown();
146+
}
147+
148+
@Override
149+
public boolean isShutdown() {
150+
return stub.isShutdown();
151+
}
152+
153+
@Override
154+
public boolean isTerminated() {
155+
return stub.isTerminated();
156+
}
157+
158+
@Override
159+
public void shutdownNow() {
160+
stub.shutdownNow();
161+
}
162+
163+
@Override
164+
public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException {
165+
return stub.awaitTermination(duration, unit);
166+
}
167+
}

src/test/java/com/google/api/generator/gapic/utils/JavaStyleTest.java

+10
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,14 @@ public void acronyms() {
111111
assertEquals("iamHttpXmlDog", JavaStyle.toLowerCamelCase(value));
112112
assertEquals("IamHttpXmlDog", JavaStyle.toUpperCamelCase(value));
113113
}
114+
115+
@Test
116+
public void keyword() {
117+
String value = "import";
118+
assertEquals("import_", JavaStyle.toLowerCamelCase(value));
119+
assertEquals("Import", JavaStyle.toUpperCamelCase(value));
120+
value = "IMPORT_";
121+
assertEquals("import_", JavaStyle.toLowerCamelCase(value));
122+
assertEquals("Import", JavaStyle.toUpperCamelCase(value));
123+
}
114124
}

src/test/proto/echo_grpcrest.proto

+5
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,11 @@ service Echo {
122122
}
123123
}
124124

125+
// Generator should not fail when encounter a service without methods
126+
service EchoEmpy {
127+
option (google.api.default_host) = "localhost:7469";
128+
}
129+
125130
// A severity enum used to test enum capabilities in GAPIC surfaces
126131
enum Severity {
127132
UNNECESSARY = 0;

0 commit comments

Comments
 (0)