From 0cddadb8ad3eddfffa356a479964d8a720937503 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Tue, 5 Nov 2024 16:59:03 -0500 Subject: [PATCH] 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 (https://github.com/googleapis/sdk-platform-java/pull/3129). For context: [go/selective-api-gen-java-one-pager](http://goto.google.com/selective-api-gen-java-one-pager), b/356380016 --- .../generator/gapic/protoparser/Parser.java | 99 +++++++++-- .../gapic/protoparser/ParserTest.java | 129 ++++++++++++++ .../test/proto/selective_api_generation.proto | 163 ++++++++++++++++++ .../selective_api_generation_v1beta1.yaml | 23 +++ 4 files changed, 397 insertions(+), 17 deletions(-) create mode 100644 gapic-generator-java/src/test/proto/selective_api_generation.proto create mode 100644 gapic-generator-java/src/test/resources/selective_api_generation_v1beta1.yaml diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java index 9a2da74747..2e17b9026b 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/protoparser/Parser.java @@ -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; @@ -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; @@ -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 outputArgResourceNames = new HashSet<>(); List mixinServices = new ArrayList<>(); Transport transport = Transport.parse(transportOpt.orElse(Transport.GRPC.toString())); @@ -425,6 +427,71 @@ public static List parseService( Transport.GRPC); } + static boolean shouldIncludeMethodInGeneration( + MethodDescriptor method, + Optional 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 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 includeMethodsList = + librarySettingsList + .get(0) + .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 serviceYamlProtoOpt, + String protoPackage) { + List methodsList = serviceDescriptor.getMethods(); + List 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 parseService( FileDescriptor fileDescriptor, Map messageTypes, @@ -433,19 +500,11 @@ public static List parseService( Optional serviceConfigOpt, Set outputArgResourceNames, Transport transport) { - + String protoPackage = fileDescriptor.getPackage(); return fileDescriptor.getServices().stream() .filter( - serviceDescriptor -> { - List 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 @@ -498,6 +557,8 @@ public static List 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 = @@ -518,6 +579,7 @@ public static List parseService( .setMethods( parseMethods( s, + protoPackage, pakkage, messageTypes, resourceNames, @@ -709,6 +771,7 @@ public static Map parseResourceNames( @VisibleForTesting static List parseMethods( ServiceDescriptor serviceDescriptor, + String protoPackage, String servicePackage, Map messageTypes, Map resourceNames, @@ -721,8 +784,10 @@ static List parseMethods( // Parse the serviceYaml for autopopulated methods and fields once and put into a map Map> 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(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java index 2776fec687..6e8ffa7232 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/protoparser/ParserTest.java @@ -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; @@ -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; @@ -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; @@ -137,6 +142,7 @@ void parseMethods_basic() { Parser.parseMethods( echoService, ECHO_PACKAGE, + ECHO_PACKAGE, messageTypes, resourceNames, Optional.empty(), @@ -200,6 +206,7 @@ void parseMethods_basicLro() { Parser.parseMethods( echoService, ECHO_PACKAGE, + ECHO_PACKAGE, messageTypes, resourceNames, Optional.empty(), @@ -705,6 +712,128 @@ void parseServiceWithNoMethodsTest() { assertEquals("EchoWithMethods", services.get(0).overriddenName()); } + @Test + void selectiveGenerationTest_shouldExcludeUnusedResourceNames() { + FileDescriptor fileDescriptor = SelectiveApiGenerationOuterClass.getDescriptor(); + Map messageTypes = Parser.parseMessages(fileDescriptor); + Map resourceNames = Parser.parseResourceNames(fileDescriptor); + + String serviceYamlFilename = "selective_api_generation_v1beta1.yaml"; + String testFilesDirectory = "src/test/resources/"; + Path serviceYamlPath = Paths.get(testFilesDirectory, serviceYamlFilename); + Optional serviceYamlOpt = + ServiceYamlParser.parse(serviceYamlPath.toString()); + Assert.assertTrue(serviceYamlOpt.isPresent()); + + Set 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 messageTypes = Parser.parseMessages(fileDescriptor); + Map 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"; + String testFilesDirectory = "src/test/resources/"; + Path serviceYamlPath = Paths.get(testFilesDirectory, serviceYamlFilename); + Optional serviceYamlOpt = + ServiceYamlParser.parse(serviceYamlPath.toString()); + Assert.assertTrue(serviceYamlOpt.isPresent()); + + List 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 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 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 methods = fileDescriptor.getServices().get(0).getMethods(); + + assertTrue( + Parser.shouldIncludeMethodInGeneration(methods.get(0), Optional.of(service), protoPackage)); + } + private void assertMethodArgumentEquals( String name, TypeNode type, List nestedFields, MethodArgument argument) { assertEquals(name, argument.name()); diff --git a/gapic-generator-java/src/test/proto/selective_api_generation.proto b/gapic-generator-java/src/test/proto/selective_api_generation.proto new file mode 100644 index 0000000000..06da2c2e41 --- /dev/null +++ b/gapic-generator-java/src/test/proto/selective_api_generation.proto @@ -0,0 +1,163 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +syntax = "proto3"; + +import "google/api/annotations.proto"; +import "google/api/client.proto"; +import "google/api/field_behavior.proto"; +import "google/api/field_info.proto"; +import "google/api/resource.proto"; +import "google/longrunning/operations.proto"; +import "google/protobuf/duration.proto"; +import "google/protobuf/timestamp.proto"; +import "google/rpc/status.proto"; + +package google.selective.generate.v1beta1; + +option java_package = "com.google.selective.generate.v1beta1"; +option java_multiple_files = true; +option java_outer_classname = "SelectiveApiGenerationOuterClass"; + +// resource not tied to message +option (google.api.resource_definition) = { + type: "showcase.googleapis.com/AnythingGoes" + pattern: "*" +}; + +// This proto is used to test selective api generation +// covered scenarios: +// - A service with several rpcs, part of them should be generated +// - A service with several rpcs, none of them should be generated +// This proto should be tested side-by-side with yaml file: +// - selective_api_generation_v1beta1.yaml + +service EchoServiceShouldGeneratePartial { + option (google.api.default_host) = "localhost:7469"; + + rpc EchoShouldInclude(EchoRequest) returns (EchoResponse) { + option (google.api.http) = { + post: "/v1beta1/echo:echo" + body: "*" + }; + option (google.api.method_signature) = "name"; + option (google.api.method_signature) = ""; + } + + rpc ChatShouldInclude(stream EchoRequest) returns (stream EchoResponse); + + rpc ChatAgainShouldInclude(stream EchoRequest) returns (stream EchoResponse) { + option (google.api.method_signature) = "content"; + } + + rpc AnExcludedMethod(stream EchoRequestWithFoobarbaz) returns (stream EchoResponse); + + rpc AnotherExcludedMethod(stream EchoRequest) returns (stream EchoResponse); + +} + +service EchoServiceShouldGenerateNone { + option (google.api.default_host) = "localhost:7469"; + option (google.api.oauth_scopes) = + "https://www.googleapis.com/auth/cloud-platform"; + + rpc Echo(EchoRequest) returns (EchoResponse) { + option (google.api.method_signature) = "content"; + } + + rpc ChatAgain(stream EchoRequest) returns (stream EchoResponse) { + option (google.api.method_signature) = "content"; + } +} + +// resource name used for message EchoRequest. +message Foobar { + option (google.api.resource) = { + type: "showcase.googleapis.com/Foobar" + pattern: "projects/{project}/foobars/{foobar}" + }; + + string name = 1; + string info = 2; +} + +// resource name used only for message EchoRequestWithFoobarbaz. +// should not be generated with selective generation when +// AnExcludedMethod is not config as included. +message Foobarbaz { + option (google.api.resource) = { + type: "showcase.googleapis.com/Foobarbaz" + pattern: "projects/{project}/foobarsbaz/{foobarbaz}" + pattern: "projects/{project}/chocolate/variants/{variant}/foobars/{foobar}" + }; + + string name = 1; + string info = 2; +} + +// RPCs in inclusion list and not in the list both relies on this request message. +message EchoRequest { + string name = 5 [ + (google.api.resource_reference).type = "showcase.googleapis.com/Foobar", + (google.api.field_behavior) = REQUIRED + ]; + + string parent = 6 [ + (google.api.resource_reference).child_type = + "showcase.googleapis.com/AnythingGoes", + (google.api.field_behavior) = REQUIRED + ]; + + oneof response { + // The content to be echoed by the server. + string content = 1; + + // The error to be thrown by the server. + google.rpc.Status error = 2; + } + + Foobar foobar = 4; +} + +// This request message is used by AnExcludedMethod rpc. +// To demonstrate that if AnExcludedMethod is not included in generation, +// then the resource name Foobarbaz, which is only used by this method, +// should not be generated. +message EchoRequestWithFoobarbaz { + string name = 5 [ + (google.api.resource_reference).type = "showcase.googleapis.com/Foobarbaz", + (google.api.field_behavior) = REQUIRED + ]; + + string parent = 6 [ + (google.api.resource_reference).child_type = + "showcase.googleapis.com/AnythingGoes", + (google.api.field_behavior) = REQUIRED + ]; + + oneof response { + // The content to be echoed by the server. + string content = 1; + + // The error to be thrown by the server. + google.rpc.Status error = 2; + } + + Foobarbaz foobar = 4; +} + +message EchoResponse { + // The content specified in the request. + string content = 1; +} diff --git a/gapic-generator-java/src/test/resources/selective_api_generation_v1beta1.yaml b/gapic-generator-java/src/test/resources/selective_api_generation_v1beta1.yaml new file mode 100644 index 0000000000..021e257c50 --- /dev/null +++ b/gapic-generator-java/src/test/resources/selective_api_generation_v1beta1.yaml @@ -0,0 +1,23 @@ +type: google.api.Service +config_version: 3 +name: selective_api_generation_testing.googleapis.com +title: Selective Generation Testing API + +publishing: + # ... + library_settings: + - version: google.selective.generate.v1beta1 + java_settings: + common: + selective_gapic_generation: + methods: + - google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial.EchoShouldInclude + - google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial.ChatShouldInclude + - google.selective.generate.v1beta1.EchoServiceShouldGeneratePartial.ChatAgainShouldInclude + reference_docs_uri: www.abc.net + destinations: + - PACKAGE_MANAGER + python_settings: + common: + destinations: + - PACKAGE_MANAGER