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

feat: enable selective generation based on service config include list #3323

Merged
merged 12 commits into from
Nov 5, 2024
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 @@ -84,6 +85,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
Expand Down Expand Up @@ -160,11 +162,11 @@ public static GapicContext parse(CodeGeneratorRequest request) {
messages = updateResourceNamesInMessages(messages, resourceNames.values());

// Contains only resource names that are actually used. Usage refers to the presence of a
// request message's field in an RPC's method_signature annotation. That is, resource name
// definitions
// or references that are simply defined, but not used in such a manner, will not have
// corresponding Java helper
// classes generated.
// request message's field in an RPC's method_signature annotation. That is, resource name
// definitions or references that are simply defined, but not used in such a manner,
// will not have corresponding Java helper classes generated.
// If selective api generation is configured via service yaml, Java helper classes are only
// generated if resource names are actually used by methods selected to generate.
Set<ResourceName> outputArgResourceNames = new HashSet<>();
List<Service> mixinServices = new ArrayList<>();
Transport transport = Transport.parse(transportOpt.orElse(Transport.GRPC.toString()));
Expand Down Expand Up @@ -425,6 +427,71 @@ 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()
&& !protoPackage.equals(librarySettingsList.get(0).getVersion())) {
if (LOGGER.isLoggable(Level.WARNING)) {
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;
}
// librarySettingsList is technically a list, but is processed upstream and
// only leave with 1 element. Otherwise, it is a misconfiguration and
// should be caught upstream.
List<String> includeMethodsList =
librarySettingsList
.get(0)
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
.getJavaSettings()
.getCommon()
.getSelectiveGapicGeneration()
.getMethodsList();
// default to include all when nothing specified, this could be no java section
// specified in library setting, or the method list is empty
if (includeMethodsList.isEmpty()) {
return true;
}

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

private static boolean isEmptyService(
ServiceDescriptor serviceDescriptor,
Optional<com.google.api.Service> serviceYamlProtoOpt,
String protoPackage) {
List<MethodDescriptor> methodsList = serviceDescriptor.getMethods();
List<MethodDescriptor> methodListSelected =
methodsList.stream()
.filter(
method ->
shouldIncludeMethodInGeneration(method, serviceYamlProtoOpt, protoPackage))
.collect(Collectors.toList());
if (methodListSelected.isEmpty()) {
LOGGER.log(
Level.WARNING,
"Service {0} has no RPC methods and will not be generated",
serviceDescriptor.getName());
}
return methodListSelected.isEmpty();
}

public static List<Service> parseService(
FileDescriptor fileDescriptor,
Map<String, Message> messageTypes,
Expand All @@ -433,19 +500,11 @@ 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()) {
LOGGER.warning(
String.format(
"Service %s has no RPC methods and will not be generated",
serviceDescriptor.getName()));
}
return !methodsList.isEmpty();
})
serviceDescriptor ->
!isEmptyService(serviceDescriptor, serviceYamlProtoOpt, protoPackage))
.map(
s -> {
// Workaround for a missing default_host and oauth_scopes annotation from a service
Expand Down Expand Up @@ -498,6 +557,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 +579,7 @@ public static List<Service> parseService(
.setMethods(
parseMethods(
s,
protoPackage,
pakkage,
messageTypes,
resourceNames,
Expand Down Expand Up @@ -709,6 +771,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 +784,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 @@ -21,9 +21,11 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import com.google.api.ClientLibrarySettings;
import com.google.api.FieldInfo.Format;
import com.google.api.MethodSettings;
import com.google.api.Publishing;
import com.google.api.PythonSettings;
import com.google.api.Service;
import com.google.api.generator.engine.ast.ConcreteReference;
import com.google.api.generator.engine.ast.Reference;
Expand All @@ -46,6 +48,7 @@
import com.google.protobuf.Descriptors.MethodDescriptor;
import com.google.protobuf.Descriptors.ServiceDescriptor;
import com.google.protobuf.compiler.PluginProtos.CodeGeneratorRequest;
import com.google.selective.generate.v1beta1.SelectiveApiGenerationOuterClass;
import com.google.showcase.v1beta1.EchoOuterClass;
import com.google.showcase.v1beta1.TestingOuterClass;
import com.google.testgapic.v1beta1.LockerProto;
Expand All @@ -58,6 +61,8 @@
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import org.junit.Assert;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -137,6 +142,7 @@ void parseMethods_basic() {
Parser.parseMethods(
echoService,
ECHO_PACKAGE,
ECHO_PACKAGE,
messageTypes,
resourceNames,
Optional.empty(),
Expand Down Expand Up @@ -200,6 +206,7 @@ void parseMethods_basicLro() {
Parser.parseMethods(
echoService,
ECHO_PACKAGE,
ECHO_PACKAGE,
messageTypes,
resourceNames,
Optional.empty(),
Expand Down Expand Up @@ -705,6 +712,128 @@ void parseServiceWithNoMethodsTest() {
assertEquals("EchoWithMethods", services.get(0).overriddenName());
}

@Test
void selectiveGenerationTest_shouldExcludeUnusedResourceNames() {
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);

String serviceYamlFilename = "selective_api_generation_v1beta1.yaml";
String testFilesDirectory = "src/test/resources/";
Path serviceYamlPath = Paths.get(testFilesDirectory, serviceYamlFilename);
Optional<com.google.api.Service> serviceYamlOpt =
ServiceYamlParser.parse(serviceYamlPath.toString());
Assert.assertTrue(serviceYamlOpt.isPresent());

Set<ResourceName> helperResourceNames = new HashSet<>();
Parser.parseService(
fileDescriptor, messageTypes, resourceNames, serviceYamlOpt, helperResourceNames);
// resource Name Foobarbaz is not present
assertEquals(2, helperResourceNames.size());
assertTrue(
helperResourceNames.stream()
.map(ResourceName::variableName)
.collect(Collectors.toSet())
.containsAll(ImmutableList.of("foobar", "anythingGoes")));
}

@Test
void selectiveGenerationTest_shouldGenerateOnlySelectiveMethods() {
FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);

// test with service yaml file to show usage of this feature, test itself
// can be done without this file and build a Service object from code.
String serviceYamlFilename = "selective_api_generation_v1beta1.yaml";
zhumin8 marked this conversation as resolved.
Show resolved Hide resolved
String testFilesDirectory = "src/test/resources/";
Path serviceYamlPath = Paths.get(testFilesDirectory, serviceYamlFilename);
Optional<com.google.api.Service> serviceYamlOpt =
ServiceYamlParser.parse(serviceYamlPath.toString());
Assert.assertTrue(serviceYamlOpt.isPresent());

List<com.google.api.generator.gapic.model.Service> services =
Parser.parseService(
fileDescriptor, messageTypes, resourceNames, serviceYamlOpt, new HashSet<>());
assertEquals(1, services.size());
assertEquals("EchoServiceShouldGeneratePartial", services.get(0).overriddenName());
assertEquals(3, services.get(0).methods().size());
for (Method method : services.get(0).methods()) {
assertTrue(method.name().contains("ShouldInclude"));
}
}

@Test
void selectiveGenerationTest_shouldGenerateAllIfNoPublishingSectionInServiceYaml() {
Service service =
Service.newBuilder()
.setTitle("Selective generation testing with no publishing section")
.build();
Publishing publishing = service.getPublishing();
Assert.assertEquals(0, publishing.getLibrarySettingsCount());

FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
List<MethodDescriptor> methods = fileDescriptor.getServices().get(0).getMethods();
String protoPackage = "google.selective.generate.v1beta1";

assertTrue(
Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage));
}

@Test
void selectiveGenerationTest_shouldIncludeMethodInGenerationWhenProtoPackageMismatch() {
String protoPackage = "google.selective.generate.v1beta1";

// situation where service yaml has different version stated
ClientLibrarySettings clientLibrarySettings =
ClientLibrarySettings.newBuilder().setVersion("google.selective.generate.v1").build();
Publishing publishing =
Publishing.newBuilder().addLibrarySettings(clientLibrarySettings).build();
Service service =
Service.newBuilder()
.setTitle(
"Selective generation test when proto package "
+ "does not match library_settings version from service yaml")
.setPublishing(publishing)
.build();

FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
List<MethodDescriptor> methods = fileDescriptor.getServices().get(0).getMethods();

assertTrue(
Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage));
}

@Test
void selectiveGenerationTest_shouldGenerateAllIfNoJavaSectionInServiceYaml() {
String protoPackage = "google.selective.generate.v1beta1";

// situation where service yaml has other language settings but no
// java settings in library_settings.
ClientLibrarySettings clientLibrarySettings =
ClientLibrarySettings.newBuilder()
.setVersion(protoPackage)
.setPythonSettings(PythonSettings.newBuilder().build())
.build();
Publishing publishing =
Publishing.newBuilder().addLibrarySettings(clientLibrarySettings).build();
Service service =
Service.newBuilder()
.setTitle(
"Selective generation test when no java section in "
+ "library_settings from service yaml")
.setPublishing(publishing)
.build();

Assert.assertEquals(1, publishing.getLibrarySettingsCount());

FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor();
List<MethodDescriptor> methods = fileDescriptor.getServices().get(0).getMethods();

assertTrue(
Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage));
}

private void assertMethodArgumentEquals(
String name, TypeNode type, List<TypeNode> nestedFields, MethodArgument argument) {
assertEquals(name, argument.name());
Expand Down
Loading
Loading