Skip to content

Commit

Permalink
fix: Generator to skip generation for empty services. (#3051)
Browse files Browse the repository at this point in the history
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](#2750 (comment)).

---------

Co-authored-by: Lawrence Qiu <[email protected]>
  • Loading branch information
zhumin8 and lqiu96 authored Jul 23, 2024
1 parent 5af127b commit ff2c485
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 177 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,6 @@ public static Sample composeClassHeaderSample(
TypeNode clientType,
Map<String, ResourceName> resourceNames,
Map<String, Message> 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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,17 @@ public static List<Service> parseService(
Transport transport) {

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();
})
.map(
s -> {
// Workaround for a missing default_host and oauth_scopes annotation from a service
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -692,6 +692,19 @@ void testServiceWithoutApiVersionParsed() {
assertNull(parsedBookshopService.apiVersion());
}

@Test
void parseServiceWithNoMethodsTest() {
FileDescriptor fileDescriptor =
com.google.api.service.without.methods.test.ServiceWithNoMethodsOuterClass.getDescriptor();
Map<String, Message> messageTypes = Parser.parseMessages(fileDescriptor);
Map<String, ResourceName> resourceNames = Parser.parseResourceNames(fileDescriptor);
List<com.google.api.generator.gapic.model.Service> 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<TypeNode> nestedFields, MethodArgument argument) {
assertEquals(name, argument.name());
Expand Down
5 changes: 0 additions & 5 deletions gapic-generator-java/src/test/proto/echo_grpcrest.proto
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
131 changes: 131 additions & 0 deletions gapic-generator-java/src/test/proto/service_with_no_methods.proto
Original file line number Diff line number Diff line change
@@ -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;
}

Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit ff2c485

Please sign in to comment.