Skip to content

Commit 0645de4

Browse files
miraleungmiraleung
authored
fix(mixins): enable RPC overrides to clobber mixed-in RPCs (#678)
1 parent ce064cb commit 0645de4

13 files changed

Lines changed: 2137 additions & 129 deletions

‎src/main/java/com/google/api/generator/gapic/composer/GrpcServiceStubClassComposer.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ public class GrpcServiceStubClassComposer implements ClassComposer {
105105
// Legacy support for the original reroute_to_grpc_interface option in gapic.yaml. These two APIs
106106
// predate the modern way, which is to add the RPCs directly into the proto.
107107
private static final Set<String> REROUTE_TO_GRPC_INTERFACE_SERVICE_ALLOWLIST =
108-
new HashSet<>(Arrays.asList("google.cloud.kms.v1", "google.pubsub.v1"));
108+
new HashSet<>(Arrays.asList("google.pubsub.v1"));
109109
private static final Set<String> REROUTE_TO_GRPC_INTERFACE_IAM_METHOD_ALLOWLIST =
110110
new HashSet<>(Arrays.asList("SetIamPolicy", "GetIamPolicy", "TestIamPermissions"));
111111

‎src/main/java/com/google/api/generator/gapic/protoparser/Parser.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,14 @@ public static List<Service> parseServices(
229229
.filter(a -> MIXIN_ALLOWLIST.contains(a.getName()))
230230
.map(a -> a.getName())
231231
.collect(Collectors.toSet());
232+
Set<String> apiDefinedRpcs = new HashSet<>();
233+
for (Service service : services) {
234+
if (blockedCodegenMixinApis.contains(service)) {
235+
continue;
236+
}
237+
apiDefinedRpcs.addAll(
238+
service.methods().stream().map(m -> m.name()).collect(Collectors.toSet()));
239+
}
232240
// Mix-in APIs only if the protos are present and they're defined in the service.yaml file.
233241
Set<Service> outputMixinServiceSet = new HashSet<>();
234242
if (servicesContainBlocklistedApi && !mixedInApis.isEmpty()) {
@@ -241,13 +249,18 @@ public static List<Service> parseServices(
241249
String.format("%s.%s", mixinService.protoPakkage(), mixinService.name()))) {
242250
continue;
243251
}
244-
List<Method> mixinMethods = new ArrayList<>(mixinService.methods());
245-
mixinMethods.forEach(
246-
m ->
247-
updatedMethods.add(
248-
m.toBuilder()
249-
.setMixedInApiName(serviceFullNameFn.apply(mixinService))
250-
.build()));
252+
mixinService
253+
.methods()
254+
.forEach(
255+
m -> {
256+
// Overridden RPCs defined in the protos take precedence.
257+
if (!apiDefinedRpcs.contains(m.name())) {
258+
updatedMethods.add(
259+
m.toBuilder()
260+
.setMixedInApiName(serviceFullNameFn.apply(mixinService))
261+
.build());
262+
}
263+
});
251264
outputMixinServiceSet.add(mixinService);
252265
}
253266
services.set(i, originalService.toBuilder().setMethods(updatedMethods).build());

‎test/integration/BUILD.bazel‎

Lines changed: 52 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ load(
33
"java_gapic_assembly_gradle_pkg",
44
"java_gapic_library",
55
"java_gapic_test",
6+
"java_grpc_library",
7+
"java_proto_library",
68
)
79
load(
810
"//:rules_bazel/java/integration_test.bzl",
@@ -19,7 +21,7 @@ package(default_visibility = ["//visibility:public"])
1921
INTEGRATION_TEST_LIBRARIES = [
2022
"asset", # Basic case.
2123
"credentials", # Check that the capital name edge case is handled.
22-
"kms", # Mixins.
24+
"kms", # Mixins, with an override in the proto file.
2325
"logging", # Java package remapping in gapic.yaml.
2426
"redis", # Has a gapic.yaml.
2527
"library", # No gRPC service config.
@@ -156,18 +158,63 @@ java_gapic_assembly_gradle_pkg(
156158
)
157159

158160
# KMS (for mixins).
161+
load("@rules_proto//proto:defs.bzl", "proto_library")
162+
load("@com_google_googleapis_imports//:imports.bzl", "proto_library_with_info")
163+
164+
proto_library(
165+
name = "kms_proto",
166+
srcs = [
167+
"apis/kms/v1/resources.proto",
168+
"apis/kms/v1/service.proto",
169+
],
170+
deps = [
171+
"@com_google_googleapis//google/api:annotations_proto",
172+
"@com_google_googleapis//google/api:client_proto",
173+
"@com_google_googleapis//google/api:field_behavior_proto",
174+
"@com_google_googleapis//google/api:resource_proto",
175+
"@com_google_googleapis//google/iam/v1:iam_policy_proto",
176+
"@com_google_googleapis//google/iam/v1:policy_proto",
177+
"@com_google_protobuf//:duration_proto",
178+
"@com_google_protobuf//:field_mask_proto",
179+
"@com_google_protobuf//:struct_proto",
180+
"@com_google_protobuf//:timestamp_proto",
181+
"@com_google_protobuf//:wrappers_proto",
182+
],
183+
)
184+
185+
proto_library_with_info(
186+
name = "kms_proto_with_info",
187+
deps = [
188+
":kms_proto",
189+
"@com_google_googleapis//google/cloud:common_resources_proto",
190+
"@com_google_googleapis//google/iam/v1:iam_policy_proto",
191+
"@com_google_googleapis//google/iam/v1:policy_proto",
192+
],
193+
)
194+
195+
java_proto_library(
196+
name = "kms_java_proto",
197+
deps = [":kms_proto"],
198+
)
199+
200+
java_grpc_library(
201+
name = "kms_java_grpc",
202+
srcs = [":kms_proto"],
203+
deps = [":kms_java_proto"],
204+
)
205+
159206
java_gapic_library(
160207
name = "kms_java_gapic",
161-
srcs = ["@com_google_googleapis//google/cloud/kms/v1:kms_proto_with_info"],
208+
srcs = [":kms_proto_with_info"],
162209
grpc_service_config = "@com_google_googleapis//google/cloud/kms/v1:cloudkms_grpc_service_config.json",
163210
# For the IAM mixin.
164-
service_yaml = "cloudkms_test_mixins_v1.yaml",
211+
service_yaml = "apis/kms/v1/cloudkms_test_mixins_v1.yaml",
165212
test_deps = [
166-
"@com_google_googleapis//google/cloud/kms/v1:kms_java_grpc",
213+
":kms_java_grpc",
167214
"@com_google_googleapis//google/iam/v1:iam_java_grpc",
168215
],
169216
deps = [
170-
"@com_google_googleapis//google/cloud/kms/v1:kms_java_proto",
217+
":kms_java_proto",
171218
"@com_google_googleapis//google/iam/v1:iam_java_proto",
172219
],
173220
)

test/integration/cloudkms_test_mixins_v1.yaml renamed to test/integration/apis/kms/v1/cloudkms_test_mixins_v1.yaml

File renamed without changes.

0 commit comments

Comments
 (0)