Skip to content

Commit 0cddadb

Browse files
authored
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
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());

0 commit comments

Comments
 (0)