From ff2c48543940bb0ceb78392b0f5af67568823002 Mon Sep 17 00:00:00 2001 From: Min Zhu Date: Tue, 23 Jul 2024 16:09:45 -0400 Subject: [PATCH] fix: Generator to skip generation for empty services. (#3051) fixes #2750 Skip parsing a service if no RPCs found for. In the scenario that only one service with no RPCs or all services have no RPCs, falls back to #2460 This pr also reverts a change brought by #985, and removes the relevant tests. For more context, this has been discussed [here](https://github.com/googleapis/sdk-platform-java/issues/2750#issuecomment-2231737360). --------- Co-authored-by: Lawrence Qiu --- .../ServiceClientHeaderSampleComposer.java | 4 - .../generator/gapic/protoparser/Parser.java | 11 ++ .../ServiceClientClassComposerTest.java | 14 -- .../grpcrest/goldens/EchoEmpty.golden | 154 ------------------ .../gapic/protoparser/ParserTest.java | 13 ++ .../src/test/proto/echo_grpcrest.proto | 5 - .../test/proto/service_with_no_methods.proto | 131 +++++++++++++++ .../reflect-config.json | 18 ++ 8 files changed, 173 insertions(+), 177 deletions(-) delete mode 100644 gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/EchoEmpty.golden create mode 100644 gapic-generator-java/src/test/proto/service_with_no_methods.proto diff --git a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.java b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.java index b979f87744..6403890b99 100644 --- a/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.java +++ b/gapic-generator-java/src/main/java/com/google/api/generator/gapic/composer/samplecode/ServiceClientHeaderSampleComposer.java @@ -52,10 +52,6 @@ public static Sample composeClassHeaderSample( TypeNode clientType, Map resourceNames, Map messageTypes) { - if (service.methods().isEmpty()) { - return ServiceClientMethodSampleComposer.composeEmptyServiceSample(clientType, service); - } - // Use the first pure unary RPC method's sample code as showcase, if no such method exists, use // the first method in the service's methods list. Method method = 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 e7c6bd8967..9a2da74747 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 @@ -435,6 +435,17 @@ public static List parseService( Transport transport) { 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(); + }) .map( s -> { // Workaround for a missing default_host and oauth_scopes annotation from a service diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/ServiceClientClassComposerTest.java b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/ServiceClientClassComposerTest.java index 8efc43118c..a6d057fe8d 100644 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/ServiceClientClassComposerTest.java +++ b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/ServiceClientClassComposerTest.java @@ -40,20 +40,6 @@ void generateServiceClasses() { Assert.assertCodeEquals(goldenFilePath, visitor.write()); } - @Test - void generateServiceClassesEmpty() { - GapicContext context = GrpcRestTestProtoLoader.instance().parseShowcaseEcho(); - Service echoProtoService = context.services().get(1); - GapicClass clazz = ServiceClientClassComposer.instance().generate(context, echoProtoService); - - JavaWriterVisitor visitor = new JavaWriterVisitor(); - clazz.classDefinition().accept(visitor); - GoldenFileWriter.saveCodegenToFile(this.getClass(), "EchoEmpty.golden", visitor.write()); - Path goldenFilePath = - Paths.get(GoldenFileWriter.getGoldenDir(this.getClass()), "EchoEmpty.golden"); - Assert.assertCodeEquals(goldenFilePath, visitor.write()); - } - @Test void generateServiceClassesWicked() { GapicContext context = GrpcRestTestProtoLoader.instance().parseShowcaseWicked(); diff --git a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/EchoEmpty.golden b/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/EchoEmpty.golden deleted file mode 100644 index b423bbbb94..0000000000 --- a/gapic-generator-java/src/test/java/com/google/api/generator/gapic/composer/grpcrest/goldens/EchoEmpty.golden +++ /dev/null @@ -1,154 +0,0 @@ -package com.google.showcase.grpcrest.v1beta1; - -import com.google.api.core.BetaApi; -import com.google.api.gax.core.BackgroundResource; -import com.google.showcase.grpcrest.v1beta1.stub.EchoEmpyStub; -import com.google.showcase.grpcrest.v1beta1.stub.EchoEmpyStubSettings; -import java.io.IOException; -import java.util.concurrent.TimeUnit; -import javax.annotation.Generated; - -// AUTO-GENERATED DOCUMENTATION AND CLASS. -/** - * This class provides the ability to make remote calls to the backing service through method calls - * that map to API methods. Sample code to get started: - * - * 
{@code
- * // This snippet has been automatically generated and should be regarded as a code template only.
- * // It will require modifications to work:
- * // - It may require correct/in-range values for request initialization.
- * // - It may require specifying regional endpoints when creating the service client as shown in
- * // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
- * try (EchoEmpyClient echoEmpyClient = EchoEmpyClient.create()) {}
- * }
- * - *

Note: close() needs to be called on the EchoEmpyClient object to clean up resources such as - * threads. In the example above, try-with-resources is used, which automatically calls close(). - * - * - * - * - * - * - * - * - *
Methods
MethodDescriptionMethod Variants
- * - *

See the individual methods for example code. - * - *

Many parameters require resource names to be formatted in a particular way. To assist with - * these names, this class includes a format method for each type of name, and additionally a parse - * method to extract the individual identifiers contained within names that are returned. - * - *

This class can be customized by passing in a custom instance of EchoEmpySettings to create(). - * For example: - * - *

To customize credentials: - * - * 

{@code
- * // This snippet has been automatically generated and should be regarded as a code template only.
- * // It will require modifications to work:
- * // - It may require correct/in-range values for request initialization.
- * // - It may require specifying regional endpoints when creating the service client as shown in
- * // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
- * EchoEmpySettings echoEmpySettings =
- *     EchoEmpySettings.newBuilder()
- *         .setCredentialsProvider(FixedCredentialsProvider.create(myCredentials))
- *         .build();
- * EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
- * }
- * - *

To customize the endpoint: - * - * 

{@code
- * // This snippet has been automatically generated and should be regarded as a code template only.
- * // It will require modifications to work:
- * // - It may require correct/in-range values for request initialization.
- * // - It may require specifying regional endpoints when creating the service client as shown in
- * // https://cloud.google.com/java/docs/setup#configure_endpoints_for_the_client_library
- * EchoEmpySettings echoEmpySettings =
- *     EchoEmpySettings.newBuilder().setEndpoint(myEndpoint).build();
- * EchoEmpyClient echoEmpyClient = EchoEmpyClient.create(echoEmpySettings);
- * }
- * - *

Please refer to the GitHub repository's samples for more quickstart code snippets. - */ -@BetaApi -@Generated("by gapic-generator-java") -public class EchoEmpyClient implements BackgroundResource { - private final EchoEmpySettings settings; - private final EchoEmpyStub stub; - - /** Constructs an instance of EchoEmpyClient with default settings. */ - public static final EchoEmpyClient create() throws IOException { - return create(EchoEmpySettings.newBuilder().build()); - } - - /** - * Constructs an instance of EchoEmpyClient, using the given settings. The channels are created - * based on the settings passed in, or defaults for any settings that are not set. - */ - public static final EchoEmpyClient create(EchoEmpySettings settings) throws IOException { - return new EchoEmpyClient(settings); - } - - /** - * Constructs an instance of EchoEmpyClient, using the given stub for making calls. This is for - * advanced usage - prefer using create(EchoEmpySettings). - */ - public static final EchoEmpyClient create(EchoEmpyStub stub) { - return new EchoEmpyClient(stub); - } - - /** - * Constructs an instance of EchoEmpyClient, 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 EchoEmpyClient(EchoEmpySettings settings) throws IOException { - this.settings = settings; - this.stub = ((EchoEmpyStubSettings) settings.getStubSettings()).createStub(); - } - - protected EchoEmpyClient(EchoEmpyStub stub) { - this.settings = null; - this.stub = stub; - } - - public final EchoEmpySettings getSettings() { - return settings; - } - - public EchoEmpyStub getStub() { - return stub; - } - - @Override - public final void close() { - stub.close(); - } - - @Override - public void shutdown() { - stub.shutdown(); - } - - @Override - public boolean isShutdown() { - return stub.isShutdown(); - } - - @Override - public boolean isTerminated() { - return stub.isTerminated(); - } - - @Override - public void shutdownNow() { - stub.shutdownNow(); - } - - @Override - public boolean awaitTermination(long duration, TimeUnit unit) throws InterruptedException { - return stub.awaitTermination(duration, unit); - } -} 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 93c4eb3599..2776fec687 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 @@ -692,6 +692,19 @@ void testServiceWithoutApiVersionParsed() { assertNull(parsedBookshopService.apiVersion()); } + @Test + void parseServiceWithNoMethodsTest() { + FileDescriptor fileDescriptor = + com.google.api.service.without.methods.test.ServiceWithNoMethodsOuterClass.getDescriptor(); + Map messageTypes = Parser.parseMessages(fileDescriptor); + Map resourceNames = Parser.parseResourceNames(fileDescriptor); + List services = + Parser.parseService( + fileDescriptor, messageTypes, resourceNames, Optional.empty(), new HashSet<>()); + assertEquals(1, services.size()); + assertEquals("EchoWithMethods", services.get(0).overriddenName()); + } + private void assertMethodArgumentEquals( String name, TypeNode type, List nestedFields, MethodArgument argument) { assertEquals(name, argument.name()); diff --git a/gapic-generator-java/src/test/proto/echo_grpcrest.proto b/gapic-generator-java/src/test/proto/echo_grpcrest.proto index ea4a9392f3..e02b4e35db 100644 --- a/gapic-generator-java/src/test/proto/echo_grpcrest.proto +++ b/gapic-generator-java/src/test/proto/echo_grpcrest.proto @@ -163,11 +163,6 @@ service Echo { } } -// Generator should not fail when encounter a service without methods -service EchoEmpy { - option (google.api.default_host) = "localhost:7469"; -} - // A severity enum used to test enum capabilities in GAPIC surfaces enum Severity { UNNECESSARY = 0; diff --git a/gapic-generator-java/src/test/proto/service_with_no_methods.proto b/gapic-generator-java/src/test/proto/service_with_no_methods.proto new file mode 100644 index 0000000000..906ade5dcb --- /dev/null +++ b/gapic-generator-java/src/test/proto/service_with_no_methods.proto @@ -0,0 +1,131 @@ +// Copyright 2018 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.api.service.without.methods.test.v1; + +option java_package = "com.google.api.service.without.methods.test"; +option java_multiple_files = true; +option java_outer_classname = "ServiceWithNoMethodsOuterClass"; + +option (google.api.resource_definition) = { + type: "showcase.googleapis.com/AnythingGoes" + pattern: "*" +}; +// This proto is used to test scenarios where a service does not have any rpc methods + +// This service is used as control group when it is not empty. +service EchoWithMethods { + // This service is meant to only run locally on the port 7469 (keypad digits + // for "show"). + option (google.api.default_host) = "localhost:7469"; + option (google.api.oauth_scopes) = + "https://www.googleapis.com/auth/cloud-platform"; + + // This method simply echos the request. This method is showcases unary rpcs. + rpc EchoWithMethod(EchoRequest) returns (EchoResponse) { + option (google.api.http) = { + post: "/v1beta1/echo:echo" + body: "*" + }; + option (google.api.method_signature) = "content"; + option (google.api.method_signature) = "error"; + option (google.api.method_signature) = "content,severity"; + option (google.api.method_signature) = "name"; + option (google.api.method_signature) = "parent"; + option (google.api.method_signature) = ""; + } +} + +// This service is to test when no method specified. +service EchoWithoutMethods { + // This service is meant to only run locally on the port 7469 (keypad digits + // for "show"). + option (google.api.default_host) = "localhost:7469"; + option (google.api.oauth_scopes) = + "https://www.googleapis.com/auth/cloud-platform"; +} + +// A severity enum used to test enum capabilities in GAPIC surfaces +enum Severity { + UNNECESSARY = 0; + NECESSARY = 1; + URGENT = 2; + CRITICAL = 3; +} + +message Foobar { + option (google.api.resource) = { + type: "showcase.googleapis.com/Foobar" + pattern: "projects/{project}/foobars/{foobar}" + pattern: "projects/{project}/chocolate/variants/{variant}/foobars/{foobar}" + pattern: "foobars/{foobar}" + pattern: "bar_foos/{bar_foo}/foobars/{foobar}" + pattern: "*" + }; + + string name = 1; + string info = 2; +} + +// The request message used for the Echo, Collect and Chat methods. +// If content or opt are set in this message then the request will succeed. +// If status is set in this message +// then the status will be returned as an error. +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; + } + + // The severity to be echoed by the server. + Severity severity = 3; + + Foobar foobar = 4; +} + +// The response message for the Echo methods. +message EchoResponse { + // The content specified in the request. + string content = 1; + + // The severity specified in the request. + Severity severity = 2; +} + diff --git a/showcase/gapic-showcase/src/main/resources/META-INF/native-image/com.google.showcase.v1beta1/reflect-config.json b/showcase/gapic-showcase/src/main/resources/META-INF/native-image/com.google.showcase.v1beta1/reflect-config.json index 27722fa934..3d3a14c9ba 100644 --- a/showcase/gapic-showcase/src/main/resources/META-INF/native-image/com.google.showcase.v1beta1/reflect-config.json +++ b/showcase/gapic-showcase/src/main/resources/META-INF/native-image/com.google.showcase.v1beta1/reflect-config.json @@ -440,6 +440,24 @@ "allDeclaredClasses": true, "allPublicClasses": true }, + { + "name": "com.google.api.TypeReference", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, + { + "name": "com.google.api.TypeReference$Builder", + "queryAllDeclaredConstructors": true, + "queryAllPublicConstructors": true, + "queryAllDeclaredMethods": true, + "allPublicMethods": true, + "allDeclaredClasses": true, + "allPublicClasses": true + }, { "name": "com.google.cloud.location.GetLocationRequest", "queryAllDeclaredConstructors": true,