feat: resolve ambiguous ServiceAccountConfig constructors (#15886)

Author: antonjfernando2021

google/cloud/credentials.cc

@@ -48,13 +48,14 @@ std::shared_ptr<Credentials> MakeImpersonateServiceAccountCredentials(
 std::shared_ptr<Credentials> MakeServiceAccountCredentials(
     std::string json_object, Options opts) {
   return std::make_shared<internal::ServiceAccountConfig>(
-      std::move(json_object), absl::nullopt, std::move(opts));
+      internal::ServiceAccountJsonObject{std::move(json_object)},
+      std::move(opts));
 }
 
 std::shared_ptr<Credentials> MakeServiceAccountCredentialsFromFile(
     std::string const& file_path, Options opts) {
   return std::make_shared<internal::ServiceAccountConfig>(
-      absl::nullopt, file_path, std::move(opts));
+      internal::ServiceAccountFilePath{file_path}, std::move(opts));
 }
 
 std::shared_ptr<Credentials> MakeExternalAccountCredentials(

google/cloud/internal/credentials_impl.cc

@@ -87,11 +87,14 @@ std::vector<std::string> const& ImpersonateServiceAccountConfig::delegates()
   return options_.get<DelegatesOption>();
 }
 
-ServiceAccountConfig::ServiceAccountConfig(
-    absl::optional<std::string> json_object,
-    absl::optional<std::string> file_path, Options opts)
-    : json_object_(std::move(json_object)),
-      file_path_(std::move(file_path)),
+ServiceAccountConfig::ServiceAccountConfig(ServiceAccountFilePath path,
+                                           Options opts)
+    : file_path_(std::move(path.value)),
+      options_(PopulateAuthOptions(std::move(opts))) {}
+
+ServiceAccountConfig::ServiceAccountConfig(ServiceAccountJsonObject json,
+                                           Options opts)
+    : json_object_(std::move(json.value)),
       options_(PopulateAuthOptions(std::move(opts))) {}
 
 ExternalAccountConfig::ExternalAccountConfig(std::string json_object,

google/cloud/internal/credentials_impl.h

@@ -142,13 +142,22 @@ class ImpersonateServiceAccountConfig : public Credentials {
   Options options_;
 };
 
+struct ServiceAccountFilePath {
+  std::string value;
+  explicit ServiceAccountFilePath(std::string p) : value(std::move(p)) {}
+};
+
+struct ServiceAccountJsonObject {
+  std::string value;
+  explicit ServiceAccountJsonObject(std::string j) : value(std::move(j)) {}
+};
+
 class ServiceAccountConfig : public Credentials {
  public:
-  // Only one of json_object or file_path should have a value.
-  // TODO(#15886): Use the C++ type system to make better constructors that
-  //   enforces this comment.
-  ServiceAccountConfig(absl::optional<std::string> json_object,
-                       absl::optional<std::string> file_path, Options opts);
+  // Just declarations ending with a semicolon
+  explicit ServiceAccountConfig(ServiceAccountFilePath path, Options opts);
+
+  explicit ServiceAccountConfig(ServiceAccountJsonObject json, Options opts);
 
   absl::optional<std::string> const& json_object() const {
     return json_object_;

google/cloud/internal/credentials_impl_test.cc

@@ -97,11 +97,14 @@ TEST(Credentials, ImpersonateServiceAccountCredentialsDefaultWithOptions) {
 }
 
 TEST(Credentials, ServiceAccount) {
+  // If MakeServiceAccountCredentials treats this string as a JSON object:
   auto credentials = MakeServiceAccountCredentials("test-only-invalid");
   TestCredentialsVisitor visitor;
   CredentialsVisitor::dispatch(*credentials, visitor);
   ASSERT_EQ("ServiceAccountConfig", visitor.name);
+  // Update: Ensure the correct field is populated and the other is empty
   EXPECT_EQ("test-only-invalid", visitor.json_object);
+  EXPECT_EQ(absl::nullopt, visitor.file_path);
 }
 
 TEST(Credentials, ExternalAccount) {

google/cloud/internal/unified_rest_credentials_test.cc

@@ -404,8 +404,9 @@ TEST(UnifiedRestCredentialsTest, ServiceAccount) {
   MockClientFactory client_factory;
   EXPECT_CALL(client_factory, Call).Times(0);
 
-  auto const config =
-      internal::ServiceAccountConfig(contents.dump(), absl::nullopt, Options{});
+  auto const config = internal::ServiceAccountConfig(
+      internal::ServiceAccountJsonObject{contents.dump()}, Options{});
+
   auto credentials = MapCredentials(config, client_factory.AsStdFunction());
   auto access_token = credentials->GetToken(now);
   ASSERT_STATUS_OK(access_token);