Skip to content

Commit

Permalink
fix(mixins): enable RPC overrides to clobber mixed-in RPCs
Browse files Browse the repository at this point in the history
  • Loading branch information
miraleung committed Mar 3, 2021
1 parent ce064cb commit 65a919d
Show file tree
Hide file tree
Showing 13 changed files with 2,137 additions and 129 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public class GrpcServiceStubClassComposer implements ClassComposer {
// Legacy support for the original reroute_to_grpc_interface option in gapic.yaml. These two APIs
// predate the modern way, which is to add the RPCs directly into the proto.
private static final Set<String> REROUTE_TO_GRPC_INTERFACE_SERVICE_ALLOWLIST =
new HashSet<>(Arrays.asList("google.cloud.kms.v1", "google.pubsub.v1"));
new HashSet<>(Arrays.asList("google.pubsub.v1"));
private static final Set<String> REROUTE_TO_GRPC_INTERFACE_IAM_METHOD_ALLOWLIST =
new HashSet<>(Arrays.asList("SetIamPolicy", "GetIamPolicy", "TestIamPermissions"));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,14 @@ public static List<Service> parseServices(
.filter(a -> MIXIN_ALLOWLIST.contains(a.getName()))
.map(a -> a.getName())
.collect(Collectors.toSet());
Set<String> apiDefinedRpcs = new HashSet<>();
for (Service service : services) {
if (blockedCodegenMixinApis.contains(service)) {
continue;
}
apiDefinedRpcs.addAll(
service.methods().stream().map(m -> m.name()).collect(Collectors.toSet()));
}
// Mix-in APIs only if the protos are present and they're defined in the service.yaml file.
Set<Service> outputMixinServiceSet = new HashSet<>();
if (servicesContainBlocklistedApi && !mixedInApis.isEmpty()) {
Expand All @@ -241,13 +249,18 @@ public static List<Service> parseServices(
String.format("%s.%s", mixinService.protoPakkage(), mixinService.name()))) {
continue;
}
List<Method> mixinMethods = new ArrayList<>(mixinService.methods());
mixinMethods.forEach(
m ->
updatedMethods.add(
m.toBuilder()
.setMixedInApiName(serviceFullNameFn.apply(mixinService))
.build()));
mixinService
.methods()
.forEach(
m -> {
// Overridden RPCs defined in the protos take precedence.
if (!apiDefinedRpcs.contains(m.name())) {
updatedMethods.add(
m.toBuilder()
.setMixedInApiName(serviceFullNameFn.apply(mixinService))
.build());
}
});
outputMixinServiceSet.add(mixinService);
}
services.set(i, originalService.toBuilder().setMethods(updatedMethods).build());
Expand Down
57 changes: 52 additions & 5 deletions test/integration/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ load(
"java_gapic_assembly_gradle_pkg",
"java_gapic_library",
"java_gapic_test",
"java_grpc_library",
"java_proto_library",
)
load(
"//:rules_bazel/java/integration_test.bzl",
Expand All @@ -19,7 +21,7 @@ package(default_visibility = ["//visibility:public"])
INTEGRATION_TEST_LIBRARIES = [
"asset", # Basic case.
"credentials", # Check that the capital name edge case is handled.
"kms", # Mixins.
"kms", # Mixins, with an override in the proto file.
"logging", # Java package remapping in gapic.yaml.
"redis", # Has a gapic.yaml.
"library", # No gRPC service config.
Expand Down Expand Up @@ -156,18 +158,63 @@ java_gapic_assembly_gradle_pkg(
)

# KMS (for mixins).
load("@rules_proto//proto:defs.bzl", "proto_library")
load("@com_google_googleapis_imports//:imports.bzl", "proto_library_with_info")

proto_library(
name = "kms_proto",
srcs = [
"apis/kms/v1/resources.proto",
"apis/kms/v1/service.proto",
],
deps = [
"@com_google_googleapis//google/api:annotations_proto",
"@com_google_googleapis//google/api:client_proto",
"@com_google_googleapis//google/api:field_behavior_proto",
"@com_google_googleapis//google/api:resource_proto",
"@com_google_googleapis//google/iam/v1:iam_policy_proto",
"@com_google_googleapis//google/iam/v1:policy_proto",
"@com_google_protobuf//:duration_proto",
"@com_google_protobuf//:field_mask_proto",
"@com_google_protobuf//:struct_proto",
"@com_google_protobuf//:timestamp_proto",
"@com_google_protobuf//:wrappers_proto",
],
)

proto_library_with_info(
name = "kms_proto_with_info",
deps = [
":kms_proto",
"@com_google_googleapis//google/cloud:common_resources_proto",
"@com_google_googleapis//google/iam/v1:iam_policy_proto",
"@com_google_googleapis//google/iam/v1:policy_proto",
],
)

java_proto_library(
name = "kms_java_proto",
deps = [":kms_proto"],
)

java_grpc_library(
name = "kms_java_grpc",
srcs = [":kms_proto"],
deps = [":kms_java_proto"],
)

java_gapic_library(
name = "kms_java_gapic",
srcs = ["@com_google_googleapis//google/cloud/kms/v1:kms_proto_with_info"],
srcs = [":kms_proto_with_info"],
grpc_service_config = "@com_google_googleapis//google/cloud/kms/v1:cloudkms_grpc_service_config.json",
# For the IAM mixin.
service_yaml = "cloudkms_test_mixins_v1.yaml",
service_yaml = "apis/kms/v1/cloudkms_test_mixins_v1.yaml",
test_deps = [
"@com_google_googleapis//google/cloud/kms/v1:kms_java_grpc",
":kms_java_grpc",
"@com_google_googleapis//google/iam/v1:iam_java_grpc",
],
deps = [
"@com_google_googleapis//google/cloud/kms/v1:kms_java_proto",
":kms_java_proto",
"@com_google_googleapis//google/iam/v1:iam_java_proto",
],
)
Expand Down
Loading

0 comments on commit 65a919d

Please sign in to comment.