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.

Sign up for GitHub

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

Draft: Selective api poc #3129

Closed
wants to merge 21 commits into from
Closed

Draft: Selective api poc #3129

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
c20c974
temp: local generate java-common-protos from googleapis with client.p…
zhumin8 Aug 12, 2024
cbcd5bc
poc for typical cases on selective generation.
zhumin8 Aug 12, 2024
cf87622
minor fixes and add comments.
zhumin8 Aug 15, 2024
e877473
temp commit: needs cleanups - experimenting with resources.:
zhumin8 Aug 16, 2024
a89bb9f
add composer test to verify methods in client generated.
zhumin8 Aug 21, 2024
7dcdf56
fix service yaml library_settings.version. add verification that it m…
zhumin8 Aug 23, 2024
f47c93a
minor code cleanups.
zhumin8 Aug 23, 2024
2496716
add note about cleanups for message and resource names parsing.
zhumin8 Aug 23, 2024
c29a319
rename method name.
zhumin8 Aug 23, 2024
7d8480a
minor fix: use parent interface.
zhumin8 Aug 23, 2024
395fa47
update test proto and config comments and namings.
zhumin8 Aug 23, 2024
950069e
review comments, combining methods for readability.
zhumin8 Aug 30, 2024
fa63021
minor cleanup.
zhumin8 Aug 30, 2024
e013458
update ComposerTest to check on non-included methods.
zhumin8 Aug 30, 2024
5a89def
update test.
zhumin8 Aug 30, 2024
a99af2c
move test for helper resource name from ComposerTest to ProtoTest.
zhumin8 Aug 30, 2024
e8f5388
review feedback, log warning text.
zhumin8 Sep 13, 2024
2012a82
fix regression when combining logic in 950069e58.
zhumin8 Sep 14, 2024
7978c15
trim proto for test. rm composer test.
zhumin8 Sep 14, 2024
c2967e9
expose private method to test, add test for version mismatch, add moc…
zhumin8 Sep 16, 2024
819bdd5
trying to refactor 2 parser tests with mockito mocks.
zhumin8 Sep 16, 2024
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
78 changes: 76 additions & 2 deletions gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

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

import com.google.api.ClientLibrarySettings;
import com.google.api.ClientProto;
import com.google.api.DocumentationRule;
import com.google.api.FieldBehavior;
Expand Down Expand Up @@ -70,6 +71,7 @@
import com.google.protobuf.Descriptors.FileDescriptor;
import com.google.protobuf.Descriptors.MethodDescriptor;
import com.google.protobuf.Descriptors.ServiceDescriptor;
import com.google.protobuf.ProtocolStringList;
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest;
import java.util.ArrayList;
import java.util.Arrays;
Expand Down Expand Up @@ -425,6 +427,66 @@ public static List<Service> parseService(
Transport.GRPC);
}

private static String getPakkageName(FileDescriptor fileDescriptor, ServiceDescriptor s,
Optional<GapicServiceConfig> serviceConfigOpt) {
String pakkage = TypeParser.getPackage(fileDescriptor);
// Override Java package with that specified in gapic.yaml.
// this override is deprecated and legacy support only
// see go/client-user-guide#configure-long-running-operation-polling-timeouts-optional
if (serviceConfigOpt.isPresent()
&& serviceConfigOpt.get().getLanguageSettingsOpt().isPresent()) {
GapicLanguageSettings languageSettings =
serviceConfigOpt.get().getLanguageSettingsOpt().get();
pakkage = languageSettings.pakkage();
}
return pakkage;
}

private static String getOverriddenServiceName(FileDescriptor fileDescriptor, ServiceDescriptor s,
Optional<GapicServiceConfig> serviceConfigOpt) {
String serviceName = s.getName();
String overriddenServiceName = serviceName;
String pakkage = TypeParser.getPackage(fileDescriptor);
// Override Java package with that specified in gapic.yaml.
// this override is deprecated and legacy support only
// see go/client-user-guide#configure-long-running-operation-polling-timeouts-optional
if (serviceConfigOpt.isPresent()
&& serviceConfigOpt.get().getLanguageSettingsOpt().isPresent()) {
GapicLanguageSettings languageSettings =
serviceConfigOpt.get().getLanguageSettingsOpt().get();
overriddenServiceName =
languageSettings.getJavaServiceName(fileDescriptor.getPackage(), s.getName());
}
return overriddenServiceName;
}
private static boolean shouldIncludeMethod(
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
MethodDescriptor method, Optional<com.google.api.Service> serviceYamlProtoOpt) {
method.getInputType().getFullName();
// default to include all when no service yaml or no library setting section.
if (!serviceYamlProtoOpt.isPresent()
|| serviceYamlProtoOpt.get().getPublishing().getLibrarySettingsCount() == 0) {
return true;
}
List<ClientLibrarySettings> librarySettingsList =
serviceYamlProtoOpt.get().getPublishing().getLibrarySettingsList();
// TODO: verify if get(0) is reliable. may need to use package version.
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
// should be okay since it's cut per version. no harm in verifying.
// library settings version (required): http://google3/google/api/client.proto;l=29-32;rcl=651426419
ProtocolStringList includeMethodsList =
librarySettingsList
.get(0)
.getJavaSettings()
.getCommon()
.getSelectiveGapicGeneration()
.getMethodsList();
// default to include all when nothing specified.
if (includeMethodsList.isEmpty()) {
return true;
}
// may need to preprocess method name with override package name
return includeMethodsList.contains(method.getFullName());
}

public static List<Service> parseService(
FileDescriptor fileDescriptor,
Map<String, Message> messageTypes,
Expand All @@ -438,13 +500,20 @@ public static List<Service> parseService(
.filter(
serviceDescriptor -> {
List<MethodDescriptor> methodsList = serviceDescriptor.getMethods();
if (methodsList.isEmpty()) {
List<MethodDescriptor> methodListSelected =
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scenario is in case that all methods from a service are excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. And this is likely now e.g. in vertexai's requirement it only exposes methods from 2 services.

methodsList.stream()
.filter(
method -> {
return shouldIncludeMethod(method, serviceYamlProtoOpt);
})
.collect(Collectors.toList());
if (methodListSelected.isEmpty()) {
LOGGER.warning(
String.format(
"Service %s has no RPC methods and will not be generated",
serviceDescriptor.getName()));
}
return !methodsList.isEmpty();
return !methodListSelected.isEmpty();
})
.map(
s -> {
Expand Down Expand Up @@ -498,6 +567,8 @@ public static List<Service> parseService(
String pakkage = TypeParser.getPackage(fileDescriptor);
String originalJavaPackage = pakkage;
// Override Java package with that specified in gapic.yaml.
// this override is deprecated and legacy support only
// see go/client-user-guide#configure-long-running-operation-polling-timeouts-optional
if (serviceConfigOpt.isPresent()
&& serviceConfigOpt.get().getLanguageSettingsOpt().isPresent()) {
GapicLanguageSettings languageSettings =
Expand Down Expand Up @@ -723,6 +794,9 @@ static List<Method> parseMethods(
parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt);

for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) {
if (!shouldIncludeMethod(protoMethod, serviceYamlProtoOpt)) {
continue;
}
// Parse the method.
TypeNode inputType = TypeParser.parseType(protoMethod.getInputType());
Method.Builder methodBuilder = Method.builder();
Expand Down
45 changes: 45 additions & 0 deletions gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/ComposerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.google.api.generator.engine.ast.ClassDefinition;
import com.google.api.generator.engine.ast.IdentifierNode;
import com.google.api.generator.engine.ast.MethodDefinition;
import com.google.api.generator.engine.ast.ScopeNode;
import com.google.api.generator.engine.writer.JavaWriterVisitor;
import com.google.api.generator.gapic.composer.comment.CommentComposer;
Expand Down Expand Up @@ -172,6 +174,49 @@ void testComposePackageInfo_emptyGapicContext_returnsNull() {
assertNull(Composer.composePackageInfo(GapicContext.EMPTY));
}

@Test
void testComposeSelectively_shouldComposeOnlyOneHelperResource() {
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
GapicContext context = GrpcTestProtoLoader.instance().parseSelectiveGenerationTesting();
List<GapicClass> resourceNameHelperClasses = Composer.generateResourceNameHelperClasses(context);
assertEquals(1, resourceNameHelperClasses.size());
for (GapicClass clazz: resourceNameHelperClasses) {
String className = clazz.classDefinition().classIdentifier().name();
Assert.assertGoldenClass(this.getClass(), clazz,
"SelectiveGenerated" + className + ".golden");
}
}

@Test
void testComposeSelectively_serviceClientShouldOnlyContainSelectedMethods() {
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
GapicContext context = GrpcTestProtoLoader.instance().parseSelectiveGenerationTesting();
List<GapicClass> serviceClasses = Composer.composeServiceClasses(context);
assertEquals(10, serviceClasses.size());
for (GapicClass clazz: serviceClasses) {
if (clazz
.classDefinition()
.classIdentifier()
.name()
.equals("EchoServiceShouldGeneratePartialClient")) {
assertEquals(25, clazz.classDefinition().methods().size());
for (MethodDefinition method : clazz.classDefinition().methods()) {
String methodName = method.methodIdentifier().name();
if (method.isConstructor()) {
continue;
}
assertTrue(
methodName.startsWith("echo")
|| methodName.startsWith("chat")
|| methodName.startsWith("create")
|| methodName.startsWith("get")
|| methodName.startsWith("close")
|| methodName.startsWith("shutdown")
|| methodName.startsWith("is")
|| methodName.startsWith("awaitTermination"));
}
}
}
}

private List<GapicClass> getTestClassListFromService(Service testService) {
GapicClass testClass =
GrpcServiceCallableFactoryClassComposer.instance()
Expand Down
Loading