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

Draft: Selective api poc #3129

Closed
wants to merge 21 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions gapic-generator-java-pom-parent/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
<j2objc-annotations.version>3.0.0</j2objc-annotations.version>
<threetenbp.version>1.6.9</threetenbp.version>
<junit.version>5.10.3</junit.version>
<mockito.version>4.11.0</mockito.version>
</properties>

<developers>
Expand Down
6 changes: 6 additions & 0 deletions gapic-generator-java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -488,5 +488,11 @@
<version>${junit.version}</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
<version>${mockito.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,8 @@

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

import com.google.api.ClientProto;
import com.google.api.DocumentationRule;
import com.google.api.FieldBehavior;
import com.google.api.FieldBehaviorProto;
import com.google.api.*;
import com.google.api.FieldInfo.Format;
import com.google.api.FieldInfoProto;
import com.google.api.HttpRule;
import com.google.api.MethodSettings;
import com.google.api.ResourceDescriptor;
import com.google.api.ResourceProto;
import com.google.api.generator.engine.ast.TypeNode;
import com.google.api.generator.engine.ast.VaporReference;
import com.google.api.generator.gapic.model.Field;
Expand Down Expand Up @@ -154,6 +146,14 @@ public static GapicContext parse(CodeGeneratorRequest request) {
// Keep message and resource name parsing separate for cleaner logic.
// While this takes an extra pass through the protobufs, the extra time is relatively trivial
// and is worth the larger reduced maintenance cost.

// Note that if selective api generation is configured via service yaml, resource names
// and messages corresponding to RPC's methods excluded for generation are not removed.
// However, these resource names will not be composed.
// refer to ComposerTest.testComposeSelectively_shouldComposeOnlyOneHelperResource for
// verification.
// TODO: remove messages and resource names that's only used by excluded RPCs configured
// via selective api generation from parsed GapicContext.
Map<String, Message> messages = parseMessages(request, outputResourceReferencesSeen);

Map<String, ResourceName> resourceNames = parseResourceNames(request);
Expand Down Expand Up @@ -425,6 +425,45 @@ public static List<Service> parseService(
Transport.GRPC);
}

static boolean shouldIncludeMethodInGeneration(
MethodDescriptor method,
Optional<com.google.api.Service> serviceYamlProtoOpt,
String protoPackage) {
// 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();
// Validate for logging purpose, this should be validated upstream.
// If library_settings.version does not match with proto package name
// Give warnings and disregard this config. default to include all.
if (!librarySettingsList.get(0).getVersion().isEmpty()
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
&& !protoPackage.equals(librarySettingsList.get(0).getVersion())) {
LOGGER.warning(
String.format(
"Service yaml config is misconfigured. Version in "
+ "publishing.library_settings (%s) does not match proto package (%s)."
+ "Disregarding selective generation settings.",
librarySettingsList.get(0).getVersion(), protoPackage));
return true;
}
List<String> includeMethodsList =
librarySettingsList
.get(0)
.getJavaSettings()
.getCommon()
.getSelectiveGapicGeneration()
.getMethodsList();
// default to include all when nothing specified
if (includeMethodsList.isEmpty()) {
return true;
}

return includeMethodsList.contains(method.getFullName());
}

public static List<Service> parseService(
FileDescriptor fileDescriptor,
Map<String, Message> messageTypes,
Expand All @@ -433,18 +472,25 @@ public static List<Service> parseService(
Optional<GapicServiceConfig> serviceConfigOpt,
Set<ResourceName> outputArgResourceNames,
Transport transport) {

String protoPackage = fileDescriptor.getPackage();
return fileDescriptor.getServices().stream()
.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 ->
shouldIncludeMethodInGeneration(
method, serviceYamlProtoOpt, protoPackage))
.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 +544,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 All @@ -518,6 +566,7 @@ public static List<Service> parseService(
.setMethods(
parseMethods(
s,
protoPackage,
pakkage,
messageTypes,
resourceNames,
Expand Down Expand Up @@ -709,6 +758,7 @@ public static Map<String, ResourceName> parseResourceNames(
@VisibleForTesting
static List<Method> parseMethods(
ServiceDescriptor serviceDescriptor,
String protoPackage,
String servicePackage,
Map<String, Message> messageTypes,
Map<String, ResourceName> resourceNames,
Expand All @@ -721,8 +771,10 @@ static List<Method> parseMethods(
// Parse the serviceYaml for autopopulated methods and fields once and put into a map
Map<String, List<String>> autoPopulatedMethodsWithFields =
parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt);

for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) {
if (!shouldIncludeMethodInGeneration(protoMethod, serviceYamlProtoOpt, protoPackage)) {
continue;
}
// Parse the method.
TypeNode inputType = TypeParser.parseType(protoMethod.getInputType());
Method.Builder methodBuilder = Method.builder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@ void generateGrpcServiceStubClass_autopopulateField() {
Assert.assertGoldenClass(this.getClass(), clazz, "GrpcAutoPopulateFieldStub.golden");
Assert.assertEmptySamples(clazz.samples());
}

@Test
void generateGrpcServiceStubClass_selectiveGeneration() {
GapicContext context = GrpcTestProtoLoader.instance().parseSelectiveGenerationTesting();
Service service = context.services().get(0);
GapicClass clazz = GrpcServiceStubClassComposer.instance().generate(context, service);

Assert.assertGoldenClass(this.getClass(), clazz, "SelectiveGeneratedStub.golden");
Assert.assertEmptySamples(clazz.samples());
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
package com.google.selective.generate.v1beta1.stub;

import com.google.api.core.BetaApi;
import com.google.api.gax.core.BackgroundResource;
import com.google.api.gax.core.BackgroundResourceAggregation;
import com.google.api.gax.grpc.GrpcCallSettings;
import com.google.api.gax.grpc.GrpcStubCallableFactory;
import com.google.api.gax.rpc.BidiStreamingCallable;
import com.google.api.gax.rpc.ClientContext;
import com.google.api.gax.rpc.UnaryCallable;
import com.google.longrunning.stub.GrpcOperationsStub;
import com.google.selective.generate.v1beta1.EchoRequest;
import com.google.selective.generate.v1beta1.EchoResponse;
import io.grpc.MethodDescriptor;
import io.grpc.protobuf.ProtoUtils;
import java.io.IOException;
import java.util.concurrent.TimeUnit;
import javax.annotation.Generated;

// AUTO-GENERATED DOCUMENTATION AND CLASS.
/**
* gRPC stub implementation for the EchoServiceShouldGeneratePartial service API.
*
* <p>This class is for advanced usage and reflects the underlying API directly.
*/
@BetaApi
@Generated("by gapic-generator-java")
public class GrpcEchoServiceShouldGeneratePartialStub extends EchoServiceShouldGeneratePartialStub {
private static final MethodDescriptor<EchoRequest, EchoResponse>
echoShouldIncludeMethodDescriptor =
MethodDescriptor.<EchoRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.UNARY)
.setFullMethodName(
"google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial/EchoShouldInclude")
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<EchoRequest, EchoResponse>
chatShouldIncludeMethodDescriptor =
MethodDescriptor.<EchoRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
.setFullMethodName(
"google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial/ChatShouldInclude")
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private static final MethodDescriptor<EchoRequest, EchoResponse>
chatAgainShouldIncludeMethodDescriptor =
MethodDescriptor.<EchoRequest, EchoResponse>newBuilder()
.setType(MethodDescriptor.MethodType.BIDI_STREAMING)
.setFullMethodName(
"google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial/ChatAgainShouldInclude")
.setRequestMarshaller(ProtoUtils.marshaller(EchoRequest.getDefaultInstance()))
.setResponseMarshaller(ProtoUtils.marshaller(EchoResponse.getDefaultInstance()))
.build();

private final UnaryCallable<EchoRequest, EchoResponse> echoShouldIncludeCallable;
private final BidiStreamingCallable<EchoRequest, EchoResponse> chatShouldIncludeCallable;
private final BidiStreamingCallable<EchoRequest, EchoResponse> chatAgainShouldIncludeCallable;

private final BackgroundResource backgroundResources;
private final GrpcOperationsStub operationsStub;
private final GrpcStubCallableFactory callableFactory;

public static final GrpcEchoServiceShouldGeneratePartialStub create(
EchoServiceShouldGeneratePartialStubSettings settings) throws IOException {
return new GrpcEchoServiceShouldGeneratePartialStub(settings, ClientContext.create(settings));
}

public static final GrpcEchoServiceShouldGeneratePartialStub create(ClientContext clientContext)
throws IOException {
return new GrpcEchoServiceShouldGeneratePartialStub(
EchoServiceShouldGeneratePartialStubSettings.newBuilder().build(), clientContext);
}

public static final GrpcEchoServiceShouldGeneratePartialStub create(
ClientContext clientContext, GrpcStubCallableFactory callableFactory) throws IOException {
return new GrpcEchoServiceShouldGeneratePartialStub(
EchoServiceShouldGeneratePartialStubSettings.newBuilder().build(),
clientContext,
callableFactory);
}

/**
* Constructs an instance of GrpcEchoServiceShouldGeneratePartialStub, 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 GrpcEchoServiceShouldGeneratePartialStub(
EchoServiceShouldGeneratePartialStubSettings settings, ClientContext clientContext)
throws IOException {
this(settings, clientContext, new GrpcEchoServiceShouldGeneratePartialCallableFactory());
}

/**
* Constructs an instance of GrpcEchoServiceShouldGeneratePartialStub, 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 GrpcEchoServiceShouldGeneratePartialStub(
EchoServiceShouldGeneratePartialStubSettings settings,
ClientContext clientContext,
GrpcStubCallableFactory callableFactory)
throws IOException {
this.callableFactory = callableFactory;
this.operationsStub = GrpcOperationsStub.create(clientContext, callableFactory);

GrpcCallSettings<EchoRequest, EchoResponse> echoShouldIncludeTransportSettings =
GrpcCallSettings.<EchoRequest, EchoResponse>newBuilder()
.setMethodDescriptor(echoShouldIncludeMethodDescriptor)
.build();
GrpcCallSettings<EchoRequest, EchoResponse> chatShouldIncludeTransportSettings =
GrpcCallSettings.<EchoRequest, EchoResponse>newBuilder()
.setMethodDescriptor(chatShouldIncludeMethodDescriptor)
.build();
GrpcCallSettings<EchoRequest, EchoResponse> chatAgainShouldIncludeTransportSettings =
GrpcCallSettings.<EchoRequest, EchoResponse>newBuilder()
.setMethodDescriptor(chatAgainShouldIncludeMethodDescriptor)
.build();

this.echoShouldIncludeCallable =
callableFactory.createUnaryCallable(
echoShouldIncludeTransportSettings,
settings.echoShouldIncludeSettings(),
clientContext);
this.chatShouldIncludeCallable =
callableFactory.createBidiStreamingCallable(
chatShouldIncludeTransportSettings,
settings.chatShouldIncludeSettings(),
clientContext);
this.chatAgainShouldIncludeCallable =
callableFactory.createBidiStreamingCallable(
chatAgainShouldIncludeTransportSettings,
settings.chatAgainShouldIncludeSettings(),
clientContext);

this.backgroundResources =
new BackgroundResourceAggregation(clientContext.getBackgroundResources());
}

public GrpcOperationsStub getOperationsStub() {
return operationsStub;
}

@Override
public UnaryCallable<EchoRequest, EchoResponse> echoShouldIncludeCallable() {
return echoShouldIncludeCallable;
}

@Override
public BidiStreamingCallable<EchoRequest, EchoResponse> chatShouldIncludeCallable() {
return chatShouldIncludeCallable;
}

@Override
public BidiStreamingCallable<EchoRequest, EchoResponse> chatAgainShouldIncludeCallable() {
return chatAgainShouldIncludeCallable;
}

@Override
public final void close() {
try {
backgroundResources.close();
} catch (RuntimeException e) {
throw e;
} catch (Exception e) {
throw new IllegalStateException("Failed to close resource", e);
}
}

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

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

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

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

@Override
public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException {
return backgroundResources.awaitTermination(duration, unit);
}
}
Loading