impl(v3): ensure generator has mixin proto descriptors available (#15797)

Author: scotthart

generator/generator.cc

@@ -72,7 +72,8 @@ bool Generator::Generate(google::protobuf::FileDescriptor const* file,
   services.reserve(file->service_count());
   for (int i = 0; i < file->service_count(); ++i) {
     auto const* service = file->service(i);
-    if (!internal::Contains(omitted_services, service->name())) {
+    if (!internal::Contains(omitted_services, service->name()) &&
+        !internal::Contains(omitted_services, service->full_name())) {
       services.push_back(generator_internal::MakeGenerators(
           service, context, service_config, *command_line_args));
     }

generator/internal/mixin_utils.cc

@@ -140,27 +140,28 @@ std::unordered_set<std::string> GetMethodNames(
 
 }  // namespace
 
-std::vector<std::string> GetMixinProtoPaths(YAML::Node const& service_config) {
-  std::vector<std::string> proto_paths;
-  if (service_config.Type() != YAML::NodeType::Map) return proto_paths;
+std::vector<MixinService> GetMixinServiceProto(
+    YAML::Node const& service_config) {
+  std::vector<MixinService> mixin_services;
+  if (service_config.Type() != YAML::NodeType::Map) return mixin_services;
   auto const& apis = service_config["apis"];
-  if (apis.Type() != YAML::NodeType::Sequence) return proto_paths;
+  if (apis.Type() != YAML::NodeType::Sequence) return mixin_services;
   for (auto const& api : apis) {
     if (api.Type() != YAML::NodeType::Map) continue;
     auto const& name = api["name"];
     if (name.Type() != YAML::NodeType::Scalar) continue;
-    auto const package_path = name.as<std::string>();
+    auto const service = name.as<std::string>();
     auto const& mixin_proto_path_map = GetMixinProtoPathMap();
-    auto const it = mixin_proto_path_map.find(package_path);
+    auto const it = mixin_proto_path_map.find(service);
     if (it == mixin_proto_path_map.end()) continue;
-    proto_paths.push_back(it->second);
+    mixin_services.push_back({it->first, it->second});
   }
-  return proto_paths;
+  return mixin_services;
 }
 
-std::vector<std::string> GetMixinProtoPaths(
+std::vector<MixinService> GetMixinServiceProto(
     std::string const& service_yaml_path) {
-  return GetMixinProtoPaths(YAML::LoadFile(service_yaml_path));
+  return GetMixinServiceProto(YAML::LoadFile(service_yaml_path));
 }
 
 std::vector<MixinMethod> GetMixinMethods(YAML::Node const& service_config,
@@ -173,15 +174,16 @@ std::vector<MixinMethod> GetMixinMethods(YAML::Node const& service_config,
                    << service.full_name();
   }
   std::unordered_set<std::string> const method_names = GetMethodNames(service);
-  auto const mixin_proto_paths = GetMixinProtoPaths(service_config);
+  auto const mixin_proto_paths = GetMixinServiceProto(service_config);
   auto const mixin_http_overrides = GetMixinHttpOverrides(service_config);
 
   for (auto const& mixin_proto_path : mixin_proto_paths) {
-    FileDescriptor const* mixin_file = pool->FindFileByName(mixin_proto_path);
+    FileDescriptor const* mixin_file =
+        pool->FindFileByName(mixin_proto_path.proto_file_path);
     if (mixin_file == nullptr) {
       GCP_LOG(FATAL) << __FILE__ << ":" << __LINE__
                      << " Mixin FileDescriptor is not found for path: "
-                     << mixin_proto_path
+                     << mixin_proto_path.proto_file_path
                      << " in service: " << service.full_name();
     }
     for (int i = 0; i < mixin_file->service_count(); ++i) {

generator/internal/mixin_utils.h

@@ -38,16 +38,30 @@ struct MixinMethod {
   google::api::HttpRule http_override;
 };
 
+struct MixinService {
+  std::string service_full_name;
+  std::string proto_file_path;
+};
+
+inline bool operator==(MixinService const& lhs, MixinService const& rhs) {
+  return lhs.service_full_name == rhs.service_full_name &&
+         lhs.proto_file_path == rhs.proto_file_path;
+}
+inline bool operator!=(MixinService const& lhs, MixinService const& rhs) {
+  return !(lhs == rhs);
+}
+
 /**
  * Extract Mixin proto file paths from the YAML Node.
  */
-std::vector<std::string> GetMixinProtoPaths(YAML::Node const& service_config);
+std::vector<MixinService> GetMixinServiceProto(
+    YAML::Node const& service_config);
 
 /**
  * Extract Mixin proto file paths from the YAML Node loaded from a YAML file
  * path.
  */
-std::vector<std::string> GetMixinProtoPaths(
+std::vector<MixinService> GetMixinServiceProto(
     std::string const& service_yaml_path);
 
 /**

generator/internal/mixin_utils_test.cc

@@ -207,11 +207,16 @@ TEST_F(MixinUtilsTest, FilesParseSuccessfully) {
 }
 
 TEST_F(MixinUtilsTest, ExtractMixinProtoPathsFromYaml) {
-  auto const mixin_proto_paths = GetMixinProtoPaths(service_config_);
-  EXPECT_THAT(mixin_proto_paths,
-              AllOf(Contains("google/cloud/location/locations.proto"),
-                    Contains("google/iam/v1/iam_policy.proto"),
-                    Contains("google/longrunning/operations.proto")));
+  std::vector<MixinService> const mixin_proto_paths =
+      GetMixinServiceProto(service_config_);
+  EXPECT_THAT(
+      mixin_proto_paths,
+      AllOf(Contains(MixinService{"google.cloud.location.Locations",
+                                  "google/cloud/location/locations.proto"}),
+            Contains(MixinService{"google.iam.v1.IAMPolicy",
+                                  "google/iam/v1/iam_policy.proto"}),
+            Contains(MixinService{"google.longrunning.Operations",
+                                  "google/longrunning/operations.proto"})));
 }
 
 TEST_F(MixinUtilsTest, GetMixinMethods) {

generator/standalone_main.cc

@@ -18,6 +18,7 @@
 #include "generator/internal/descriptor_utils.h"
 #include "generator/internal/discovery_to_proto.h"
 #include "generator/internal/format_method_comments.h"
+#include "generator/internal/mixin_utils.h"
 #include "generator/internal/scaffold_generator.h"
 #include "google/cloud/internal/absl_str_cat_quiet.h"
 #include "google/cloud/internal/absl_str_join_quiet.h"
@@ -329,10 +330,6 @@ std::vector<std::future<google::cloud::Status>> GenerateCodeFromProtos(
     if (service.generate_rest_transport()) {
       args.emplace_back("--cpp_codegen_opt=generate_rest_transport=true");
     }
-    args.emplace_back(service.service_proto_path());
-    for (auto const& additional_proto_file : service.additional_proto_files()) {
-      args.emplace_back(additional_proto_file);
-    }
     if (service.experimental()) {
       args.emplace_back("--cpp_codegen_opt=experimental=true");
     }
@@ -388,10 +385,30 @@ std::vector<std::future<google::cloud::Status>> GenerateCodeFromProtos(
                        "=", kv.second));
     }
 
+    std::vector<std::string> mixin_files_to_append;
     auto path = ServiceConfigYamlPath(yaml_root, scaffold_vars);
     if (!path.empty()) {
+      auto mixins =
+          google::cloud::generator_internal::GetMixinServiceProto(path);
       args.emplace_back(absl::StrCat("--cpp_codegen_opt=service_config_yaml=",
                                      std::move(path)));
+      for (auto& mixin_service : mixins) {
+        if (mixin_service.proto_file_path != service.service_proto_path()) {
+          args.emplace_back("--cpp_codegen_opt=omit_service=" +
+                            std::move(mixin_service.service_full_name));
+          mixin_files_to_append.push_back(
+              std::move(mixin_service.proto_file_path));
+        }
+      }
+    }
+
+    args.push_back(service.service_proto_path());
+    for (auto const& additional_proto_file : service.additional_proto_files()) {
+      args.push_back(additional_proto_file);
+    }
+
+    for (auto& mixin_path : mixin_files_to_append) {
+      args.push_back(std::move(mixin_path));
     }
 
     GCP_LOG(INFO) << "Generating service code using: "