Skip to content

Commit 0cddadb

Browse files
zhumin8zhumin8
authoredNov 5, 2024··
feat: enable selective generation based on service config include list (#3323)
following changes in client.proto that propagated to `ClientProto.java` ([pr](https://github.com/googleapis/sdk-platform-java/pull/3309/files#diff-44c330ef5cfa380744be6c58a14aa543edceb6043d5b17da7b32bb728ef5d85f)), apply changes from poc pr (#3129). For context: [go/selective-api-gen-java-one-pager](http://goto.google.com/selective-api-gen-java-one-pager), b/356380016
·
v2.55.1api-common/v2.41.0
1 parent 25023af commit 0cddadb

File tree

4 files changed

+397
-17
lines changed

4 files changed

+397
-17
lines changed
 

‎gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java

+82-17
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

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

17+
import com.google.api.ClientLibrarySettings;
1718
import com.google.api.ClientProto;
1819
import com.google.api.DocumentationRule;
1920
import com.google.api.FieldBehavior;
@@ -84,6 +85,7 @@
8485
import java.util.Optional;
8586
import java.util.Set;
8687
import java.util.function.Function;
88+
import java.util.logging.Level;
8789
import java.util.logging.Logger;
8890
import java.util.stream.Collectors;
8991
import java.util.stream.IntStream;
@@ -160,11 +162,11 @@ public static GapicContext parse(CodeGeneratorRequest request) {
160162
messages = updateResourceNamesInMessages(messages, resourceNames.values());
161163

162164
// Contains only resource names that are actually used. Usage refers to the presence of a
163-
// request message's field in an RPC's method_signature annotation. That is, resource name
164-
// definitions
165-
// or references that are simply defined, but not used in such a manner, will not have
166-
// corresponding Java helper
167-
// classes generated.
165+
// request message's field in an RPC's method_signature annotation. That is, resource name
166+
// definitions or references that are simply defined, but not used in such a manner,
167+
// will not have corresponding Java helper classes generated.
168+
// If selective api generation is configured via service yaml, Java helper classes are only
169+
// generated if resource names are actually used by methods selected to generate.
168170
Set<ResourceName> outputArgResourceNames = new HashSet<>();
169171
List<Service> mixinServices = new ArrayList<>();
170172
Transport transport = Transport.parse(transportOpt.orElse(Transport.GRPC.toString()));
@@ -425,6 +427,71 @@ public static List<Service> parseService(
425427
Transport.GRPC);
426428
}
427429

430+
static boolean shouldIncludeMethodInGeneration(
431+
MethodDescriptor method,
432+
Optional<com.google.api.Service> serviceYamlProtoOpt,
433+
String protoPackage) {
434+
// default to include all when no service yaml or no library setting section.
435+
if (!serviceYamlProtoOpt.isPresent()
436+
|| serviceYamlProtoOpt.get().getPublishing().getLibrarySettingsCount() == 0) {
437+
return true;
438+
}
439+
List<ClientLibrarySettings> librarySettingsList =
440+
serviceYamlProtoOpt.get().getPublishing().getLibrarySettingsList();
441+
// Validate for logging purpose, this should be validated upstream.
442+
// If library_settings.version does not match with proto package name
443+
// Give warnings and disregard this config. default to include all.
444+
if (!librarySettingsList.get(0).getVersion().isEmpty()
445+
&& !protoPackage.equals(librarySettingsList.get(0).getVersion())) {
446+
if (LOGGER.isLoggable(Level.WARNING)) {
447+
LOGGER.warning(
448+
String.format(
449+
"Service yaml config is misconfigured. Version in "
450+
+ "publishing.library_settings (%s) does not match proto package (%s)."
451+
+ "Disregarding selective generation settings.",
452+
librarySettingsList.get(0).getVersion(), protoPackage));
453+
}
454+
return true;
455+
}
456+
// librarySettingsList is technically a list, but is processed upstream and
457+
// only leave with 1 element. Otherwise, it is a misconfiguration and
458+
// should be caught upstream.
459+
List<String> includeMethodsList =
460+
librarySettingsList
461+
.get(0)
462+
.getJavaSettings()
463+
.getCommon()
464+
.getSelectiveGapicGeneration()
465+
.getMethodsList();
466+
// default to include all when nothing specified, this could be no java section
467+
// specified in library setting, or the method list is empty
468+
if (includeMethodsList.isEmpty()) {
469+
return true;
470+
}
471+
472+
return includeMethodsList.contains(method.getFullName());
473+
}
474+
475+
private static boolean isEmptyService(
476+
ServiceDescriptor serviceDescriptor,
477+
Optional<com.google.api.Service> serviceYamlProtoOpt,
478+
String protoPackage) {
479+
List<MethodDescriptor> methodsList = serviceDescriptor.getMethods();
480+
List<MethodDescriptor> methodListSelected =
481+
methodsList.stream()
482+
.filter(
483+
method ->
484+
shouldIncludeMethodInGeneration(method, serviceYamlProtoOpt, protoPackage))
485+
.collect(Collectors.toList());
486+
if (methodListSelected.isEmpty()) {
487+
LOGGER.log(
488+
Level.WARNING,
489+
"Service {0} has no RPC methods and will not be generated",
490+
serviceDescriptor.getName());
491+
}
492+
return methodListSelected.isEmpty();
493+
}
494+
428495
public static List<Service> parseService(
429496
FileDescriptor fileDescriptor,
430497
Map<String, Message> messageTypes,
@@ -433,19 +500,11 @@ public static List<Service> parseService(
433500
Optional<GapicServiceConfig> serviceConfigOpt,
434501
Set<ResourceName> outputArgResourceNames,
435502
Transport transport) {
436-
503+
String protoPackage = fileDescriptor.getPackage();
437504
return fileDescriptor.getServices().stream()
438505
.filter(
439-
serviceDescriptor -> {
440-
List<MethodDescriptor> methodsList = serviceDescriptor.getMethods();
441-
if (methodsList.isEmpty()) {
442-
LOGGER.warning(
443-
String.format(
444-
"Service %s has no RPC methods and will not be generated",
445-
serviceDescriptor.getName()));
446-
}
447-
return !methodsList.isEmpty();
448-
})
506+
serviceDescriptor ->
507+
!isEmptyService(serviceDescriptor, serviceYamlProtoOpt, protoPackage))
449508
.map(
450509
s -> {
451510
// Workaround for a missing default_host and oauth_scopes annotation from a service
@@ -498,6 +557,8 @@ public static List<Service> parseService(
498557
String pakkage = TypeParser.getPackage(fileDescriptor);
499558
String originalJavaPackage = pakkage;
500559
// Override Java package with that specified in gapic.yaml.
560+
// this override is deprecated and legacy support only
561+
// see go/client-user-guide#configure-long-running-operation-polling-timeouts-optional
501562
if (serviceConfigOpt.isPresent()
502563
&& serviceConfigOpt.get().getLanguageSettingsOpt().isPresent()) {
503564
GapicLanguageSettings languageSettings =
@@ -518,6 +579,7 @@ public static List<Service> parseService(
518579
.setMethods(
519580
parseMethods(
520581
s,
582+
protoPackage,
521583
pakkage,
522584
messageTypes,
523585
resourceNames,
@@ -709,6 +771,7 @@ public static Map<String, ResourceName> parseResourceNames(
709771
@VisibleForTesting
710772
static List<Method> parseMethods(
711773
ServiceDescriptor serviceDescriptor,
774+
String protoPackage,
712775
String servicePackage,
713776
Map<String, Message> messageTypes,
714777
Map<String, ResourceName> resourceNames,
@@ -721,8 +784,10 @@ static List<Method> parseMethods(
721784
// Parse the serviceYaml for autopopulated methods and fields once and put into a map
722785
Map<String, List<String>> autoPopulatedMethodsWithFields =
723786
parseAutoPopulatedMethodsAndFields(serviceYamlProtoOpt);
724-
725787
for (MethodDescriptor protoMethod : serviceDescriptor.getMethods()) {
788+
if (!shouldIncludeMethodInGeneration(protoMethod, serviceYamlProtoOpt, protoPackage)) {
789+
continue;
790+
}
726791
// Parse the method.
727792
TypeNode inputType = TypeParser.parseType(protoMethod.getInputType());
728793
Method.Builder methodBuilder = Method.builder();

‎gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java

+129
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@
2121
import static org.junit.jupiter.api.Assertions.assertThrows;
2222
import static org.junit.jupiter.api.Assertions.assertTrue;
2323

24+
import com.google.api.ClientLibrarySettings;
2425
import com.google.api.FieldInfo.Format;
2526
import com.google.api.MethodSettings;
2627
import com.google.api.Publishing;
28+
import com.google.api.PythonSettings;
2729
import com.google.api.Service;
2830
import com.google.api.generator.engine.ast.ConcreteReference;
2931
import com.google.api.generator.engine.ast.Reference;
@@ -46,6 +48,7 @@
4648
import com.google.protobuf.Descriptors.MethodDescriptor;
4749
import com.google.protobuf.Descriptors.ServiceDescriptor;
4850
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest;
51+
import com.google.selective.generate.v1beta1.SelectiveApiGenerationOuterClass;
4952
import com.google.showcase.v1beta1.EchoOuterClass;
5053
import com.google.showcase.v1beta1.TestingOuterClass;
5154
import com.google.testgapic.v1beta1.LockerProto;
@@ -58,6 +61,8 @@
5861
import java.util.Map;
5962
import java.util.Optional;
6063
import java.util.Set;
64+
import java.util.stream.Collectors;
65+
import org.junit.Assert;
6166
import org.junit.jupiter.api.BeforeEach;
6267
import org.junit.jupiter.api.Test;
6368

@@ -137,6 +142,7 @@ void parseMethods_basic() {
137142
Parser.parseMethods(
138143
echoService,
139144
ECHO_PACKAGE,
145+
ECHO_PACKAGE,
140146
messageTypes,
141147
resourceNames,
142148
Optional.empty(),
@@ -200,6 +206,7 @@ void parseMethods_basicLro() {
200206
Parser.parseMethods(
201207
echoService,
202208
ECHO_PACKAGE,
209+
ECHO_PACKAGE,
203210
messageTypes,
204211
resourceNames,
205212
Optional.empty(),
@@ -705,6 +712,128 @@ void parseServiceWithNoMethodsTest() {
705712
assertEquals("EchoWithMethods", services.get(0).overriddenName());
706713
}
707714

715+
@Test
716+
void selectiveGenerationTest_shouldExcludeUnusedResourceNames() {
717+
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
718+
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
719+
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
720+
721+
String serviceYamlFilename = "selective_api_generation_v1beta1.yaml";
722+
String testFilesDirectory = "src/test/resources/";
723+
Path serviceYamlPath = Paths.get(testFilesDirectory, serviceYamlFilename);
724+
Optional<com.google.api.Service> serviceYamlOpt =
725+
ServiceYamlParser.parse(serviceYamlPath.toString());
726+
Assert.assertTrue(serviceYamlOpt.isPresent());
727+
728+
Set<ResourceName> helperResourceNames = new HashSet<>();
729+
Parser.parseService(
730+
fileDescriptor, messageTypes, resourceNames, serviceYamlOpt, helperResourceNames);
731+
// resource Name Foobarbaz is not present
732+
assertEquals(2, helperResourceNames.size());
733+
assertTrue(
734+
helperResourceNames.stream()
735+
.map(ResourceName::variableName)
736+
.collect(Collectors.toSet())
737+
.containsAll(ImmutableList.of("foobar", "anythingGoes")));
738+
}
739+
740+
@Test
741+
void selectiveGenerationTest_shouldGenerateOnlySelectiveMethods() {
742+
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
743+
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
744+
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
745+
746+
// test with service yaml file to show usage of this feature, test itself
747+
// can be done without this file and build a Service object from code.
748+
String serviceYamlFilename = "selective_api_generation_v1beta1.yaml";
749+
String testFilesDirectory = "src/test/resources/";
750+
Path serviceYamlPath = Paths.get(testFilesDirectory, serviceYamlFilename);
751+
Optional<com.google.api.Service> serviceYamlOpt =
752+
ServiceYamlParser.parse(serviceYamlPath.toString());
753+
Assert.assertTrue(serviceYamlOpt.isPresent());
754+
755+
List<com.google.api.generator.gapic.model.Service> services =
756+
Parser.parseService(
757+
fileDescriptor, messageTypes, resourceNames, serviceYamlOpt, new HashSet<>());
758+
assertEquals(1, services.size());
759+
assertEquals("EchoServiceShouldGeneratePartial", services.get(0).overriddenName());
760+
assertEquals(3, services.get(0).methods().size());
761+
for (Method method : services.get(0).methods()) {
762+
assertTrue(method.name().contains("ShouldInclude"));
763+
}
764+
}
765+
766+
@Test
767+
void selectiveGenerationTest_shouldGenerateAllIfNoPublishingSectionInServiceYaml() {
768+
Service service =
769+
Service.newBuilder()
770+
.setTitle("Selective generation testing with no publishing section")
771+
.build();
772+
Publishing publishing = service.getPublishing();
773+
Assert.assertEquals(0, publishing.getLibrarySettingsCount());
774+
775+
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
776+
List<MethodDescriptor> methods = fileDescriptor.getServices().get(0).getMethods();
777+
String protoPackage = "google.selective.generate.v1beta1";
778+
779+
assertTrue(
780+
Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage));
781+
}
782+
783+
@Test
784+
void selectiveGenerationTest_shouldIncludeMethodInGenerationWhenProtoPackageMismatch() {
785+
String protoPackage = "google.selective.generate.v1beta1";
786+
787+
// situation where service yaml has different version stated
788+
ClientLibrarySettings clientLibrarySettings =
789+
ClientLibrarySettings.newBuilder().setVersion("google.selective.generate.v1").build();
790+
Publishing publishing =
791+
Publishing.newBuilder().addLibrarySettings(clientLibrarySettings).build();
792+
Service service =
793+
Service.newBuilder()
794+
.setTitle(
795+
"Selective generation test when proto package "
796+
+ "does not match library_settings version from service yaml")
797+
.setPublishing(publishing)
798+
.build();
799+
800+
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
801+
List<MethodDescriptor> methods = fileDescriptor.getServices().get(0).getMethods();
802+
803+
assertTrue(
804+
Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage));
805+
}
806+
807+
@Test
808+
void selectiveGenerationTest_shouldGenerateAllIfNoJavaSectionInServiceYaml() {
809+
String protoPackage = "google.selective.generate.v1beta1";
810+
811+
// situation where service yaml has other language settings but no
812+
// java settings in library_settings.
813+
ClientLibrarySettings clientLibrarySettings =
814+
ClientLibrarySettings.newBuilder()
815+
.setVersion(protoPackage)
816+
.setPythonSettings(PythonSettings.newBuilder().build())
817+
.build();
818+
Publishing publishing =
819+
Publishing.newBuilder().addLibrarySettings(clientLibrarySettings).build();
820+
Service service =
821+
Service.newBuilder()
822+
.setTitle(
823+
"Selective generation test when no java section in "
824+
+ "library_settings from service yaml")
825+
.setPublishing(publishing)
826+
.build();
827+
828+
Assert.assertEquals(1, publishing.getLibrarySettingsCount());
829+
830+
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
831+
List<MethodDescriptor> methods = fileDescriptor.getServices().get(0).getMethods();
832+
833+
assertTrue(
834+
Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage));
835+
}
836+
708837
private void assertMethodArgumentEquals(
709838
String name, TypeNode type, List<TypeNode> nestedFields, MethodArgument argument) {
710839
assertEquals(name, argument.name());

‎gapic-generator-java/src/test/proto/selective_api_generation.proto

+163
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
// Copyright 2024 Google LLC
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// https://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
syntax = "proto3";
16+
17+
import "google/api/annotations.proto";
18+
import "google/api/client.proto";
19+
import "google/api/field_behavior.proto";
20+
import "google/api/field_info.proto";
21+
import "google/api/resource.proto";
22+
import "google/longrunning/operations.proto";
23+
import "google/protobuf/duration.proto";
24+
import "google/protobuf/timestamp.proto";
25+
import "google/rpc/status.proto";
26+
27+
package google.selective.generate.v1beta1;
28+
29+
option java_package = "com.google.selective.generate.v1beta1";
30+
option java_multiple_files = true;
31+
option java_outer_classname = "SelectiveApiGenerationOuterClass";
32+
33+
// resource not tied to message
34+
option (google.api.resource_definition) = {
35+
type: "showcase.googleapis.com/AnythingGoes"
36+
pattern: "*"
37+
};
38+
39+
// This proto is used to test selective api generation
40+
// covered scenarios:
41+
// - A service with several rpcs, part of them should be generated
42+
// - A service with several rpcs, none of them should be generated
43+
// This proto should be tested side-by-side with yaml file:
44+
// - selective_api_generation_v1beta1.yaml
45+
46+
service EchoServiceShouldGeneratePartial {
47+
option (google.api.default_host) = "localhost:7469";
48+
49+
rpc EchoShouldInclude(EchoRequest) returns (EchoResponse) {
50+
option (google.api.http) = {
51+
post: "/v1beta1/echo:echo"
52+
body: "*"
53+
};
54+
option (google.api.method_signature) = "name";
55+
option (google.api.method_signature) = "";
56+
}
57+
58+
rpc ChatShouldInclude(stream EchoRequest) returns (stream EchoResponse);
59+
60+
rpc ChatAgainShouldInclude(stream EchoRequest) returns (stream EchoResponse) {
61+
option (google.api.method_signature) = "content";
62+
}
63+
64+
rpc AnExcludedMethod(stream EchoRequestWithFoobarbaz) returns (stream EchoResponse);
65+
66+
rpc AnotherExcludedMethod(stream EchoRequest) returns (stream EchoResponse);
67+
68+
}
69+
70+
service EchoServiceShouldGenerateNone {
71+
option (google.api.default_host) = "localhost:7469";
72+
option (google.api.oauth_scopes) =
73+
"https://www.googleapis.com/auth/cloud-platform";
74+
75+
rpc Echo(EchoRequest) returns (EchoResponse) {
76+
option (google.api.method_signature) = "content";
77+
}
78+
79+
rpc ChatAgain(stream EchoRequest) returns (stream EchoResponse) {
80+
option (google.api.method_signature) = "content";
81+
}
82+
}
83+
84+
// resource name used for message EchoRequest.
85+
message Foobar {
86+
option (google.api.resource) = {
87+
type: "showcase.googleapis.com/Foobar"
88+
pattern: "projects/{project}/foobars/{foobar}"
89+
};
90+
91+
string name = 1;
92+
string info = 2;
93+
}
94+
95+
// resource name used only for message EchoRequestWithFoobarbaz.
96+
// should not be generated with selective generation when
97+
// AnExcludedMethod is not config as included.
98+
message Foobarbaz {
99+
option (google.api.resource) = {
100+
type: "showcase.googleapis.com/Foobarbaz"
101+
pattern: "projects/{project}/foobarsbaz/{foobarbaz}"
102+
pattern: "projects/{project}/chocolate/variants/{variant}/foobars/{foobar}"
103+
};
104+
105+
string name = 1;
106+
string info = 2;
107+
}
108+
109+
// RPCs in inclusion list and not in the list both relies on this request message.
110+
message EchoRequest {
111+
string name = 5 [
112+
(google.api.resource_reference).type = "showcase.googleapis.com/Foobar",
113+
(google.api.field_behavior) = REQUIRED
114+
];
115+
116+
string parent = 6 [
117+
(google.api.resource_reference).child_type =
118+
"showcase.googleapis.com/AnythingGoes",
119+
(google.api.field_behavior) = REQUIRED
120+
];
121+
122+
oneof response {
123+
// The content to be echoed by the server.
124+
string content = 1;
125+
126+
// The error to be thrown by the server.
127+
google.rpc.Status error = 2;
128+
}
129+
130+
Foobar foobar = 4;
131+
}
132+
133+
// This request message is used by AnExcludedMethod rpc.
134+
// To demonstrate that if AnExcludedMethod is not included in generation,
135+
// then the resource name Foobarbaz, which is only used by this method,
136+
// should not be generated.
137+
message EchoRequestWithFoobarbaz {
138+
string name = 5 [
139+
(google.api.resource_reference).type = "showcase.googleapis.com/Foobarbaz",
140+
(google.api.field_behavior) = REQUIRED
141+
];
142+
143+
string parent = 6 [
144+
(google.api.resource_reference).child_type =
145+
"showcase.googleapis.com/AnythingGoes",
146+
(google.api.field_behavior) = REQUIRED
147+
];
148+
149+
oneof response {
150+
// The content to be echoed by the server.
151+
string content = 1;
152+
153+
// The error to be thrown by the server.
154+
google.rpc.Status error = 2;
155+
}
156+
157+
Foobarbaz foobar = 4;
158+
}
159+
160+
message EchoResponse {
161+
// The content specified in the request.
162+
string content = 1;
163+
}

‎gapic-generator-java/src/test/resources/selective_api_generation_v1beta1.yaml

+23
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
type: google.api.Service
2+
config_version: 3
3+
name: selective_api_generation_testing.googleapis.com
4+
title: Selective Generation Testing API
5+
6+
publishing:
7+
# ...
8+
library_settings:
9+
- version: google.selective.generate.v1beta1
10+
java_settings:
11+
common:
12+
selective_gapic_generation:
13+
methods:
14+
- google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial.EchoShouldInclude
15+
- google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial.ChatShouldInclude
16+
- google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial.ChatAgainShouldInclude
17+
reference_docs_uri: www.abc.net
18+
destinations:
19+
- PACKAGE_MANAGER
20+
python_settings:
21+
common:
22+
destinations:
23+
- PACKAGE_MANAGER

0 commit comments

Comments
 (0)
Please sign in to comment.