Replace FetcherConfig::AddFilesToConfig with BuildFetcherConfigMember

This function doesn't need to use any private members of the class, so
it can be an external function. The difference between `AddFilesToConfig`
and other member functions became more obvious when introducing mutex
locking.

https://embeddedartistry.com/fieldatlas/how-non-member-functions-improve-encapsulation/

Bug: b/461609135

Author: Databean

base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_context.cc

@@ -209,9 +209,14 @@ Result<void> FetchBuildContext::DesparseFiles(std::vector<std::string> files) {
 
 Result<void> FetchBuildContext::AddFileToConfig(std::string file) {
   auto [build_id, build_target] = GetBuildIdAndTarget(build_);
-  CF_EXPECT(fetch_context_.fetcher_config_.AddFilesToConfig(
-      file_source_, build_id, build_target, {file},
-      fetch_context_.target_directories_.root, true));
+
+  CvdFile config_member = CF_EXPECT(
+      BuildFetcherConfigMember(file_source_, build_id, build_target, file,
+                               fetch_context_.target_directories_.root));
+
+  CF_EXPECT(fetch_context_.fetcher_config_.add_cvd_file(
+      std::move(config_member), /* override_entry = */ true));
+
   return {};
 }
 

base/cvd/cuttlefish/host/commands/cvd/fetch/fetch_cvd.cc

@@ -176,8 +176,11 @@ Result<std::string> SaveConfig(FetcherConfig& config,
   // itself was built at.
   // https://android.googlesource.com/platform/build/+/979c9f3/Changes.md#build_number
   const std::string fetcher_path = target_directory + "/fetcher_config.json";
-  CF_EXPECT(config.AddFilesToConfig(FileSource::GENERATED, "", "",
-                                    {fetcher_path}, target_directory));
+
+  CvdFile config_member = CF_EXPECT(BuildFetcherConfigMember(
+      FileSource::GENERATED, "", "", fetcher_path, target_directory));
+  CF_EXPECT(config.add_cvd_file(std::move(config_member)));
+
   config.SaveToFile(fetcher_path);
 
   for (const auto& file : config.get_cvd_files()) {
@@ -365,8 +368,12 @@ Result<void> FetchChromeOsTarget(
   auto archive_files = CF_EXPECT(ExtractArchiveContents(
       archive_path, target_directories.chrome_os, keep_downloaded_archives));
   trace.CompletePhase("Extract");
-  CF_EXPECT(config.AddFilesToConfig(FileSource::CHROME_OS_BUILD, "", "",
-                                    archive_files, target_directories.root));
+  for (const std::string& archive_file : archive_files) {
+    CvdFile config_member = CF_EXPECT(
+        BuildFetcherConfigMember(FileSource::CHROME_OS_BUILD, "", "",
+                                 archive_file, target_directories.root));
+    CF_EXPECT(config.add_cvd_file(std::move(config_member)));
+  }
   return {};
 }
 

base/cvd/cuttlefish/host/libs/config/fetcher_config.cpp

@@ -152,6 +152,14 @@ Json::Value CvdFileToJson(const CvdFile& cvd_file) {
   return json;
 }
 
+Result<std::string> NormalizePath(std::string path) {
+  CF_EXPECT(!absl::StrContains(path, ".."));
+  while (absl::StrContains(path, "//")) {
+    absl::StrReplaceAll({{"//", "/"}}, &path);
+  }
+  return path;
+}
+
 }  // namespace
 
 bool FetcherConfig::add_cvd_file(const CvdFile& file, bool override_entry) {
@@ -200,42 +208,6 @@ std::string FetcherConfig::FindCvdFileWithSuffix(
   return "";
 }
 
-static Result<std::string> NormalizePath(std::string path) {
-  CF_EXPECT(!absl::StrContains(path, ".."));
-  while (absl::StrContains(path, "//")) {
-    absl::StrReplaceAll({{"//", "/"}}, &path);
-  }
-  return path;
-}
-
-Result<void> FetcherConfig::AddFilesToConfig(
-    FileSource purpose, const std::string& build_id,
-    const std::string& build_target, const std::vector<std::string>& paths,
-    const std::string& directory_prefix, bool override_entry) {
-  // This neither acesses dictionary_ directly or locks *mutex, but it calls
-  // `add_cvd_file` which does.
-
-  for (const std::string& path : paths) {
-    std::string_view local_path(path);
-    if (!android::base::ConsumePrefix(&local_path, directory_prefix)) {
-      LOG(ERROR) << "Failed to remove prefix " << directory_prefix << " from "
-                 << local_path;
-      return {};
-    }
-    while (android::base::StartsWith(local_path, "/")) {
-      android::base::ConsumePrefix(&local_path, "/");
-    }
-    std::string normalized = CF_EXPECT(NormalizePath(std::string(local_path)));
-    // TODO(schuffelen): Do better for local builds here.
-    CvdFile file(purpose, build_id, build_target, normalized);
-    CF_EXPECT(add_cvd_file(file, override_entry),
-              "Duplicate file \""
-                  << file << "\", Existing file: \"" << get_cvd_files()[path]
-                  << "\". Failed to add path \"" << path << "\"");
-  }
-  return {};
-}
-
 Result<void> FetcherConfig::RemoveFileFromConfig(const std::string& path) {
   std::scoped_lock lock(*mutex_);
 
@@ -249,6 +221,25 @@ Result<void> FetcherConfig::RemoveFileFromConfig(const std::string& path) {
   return {};
 }
 
+Result<CvdFile> BuildFetcherConfigMember(FileSource purpose,
+                                         const std::string& build_id,
+                                         const std::string& build_target,
+                                         const std::string& path,
+                                         const std::string& directory_prefix) {
+  std::string_view local_path(path);
+  if (!android::base::ConsumePrefix(&local_path, directory_prefix)) {
+    LOG(ERROR) << "Failed to remove prefix " << directory_prefix << " from "
+               << local_path;
+    return {};
+  }
+  while (android::base::StartsWith(local_path, "/")) {
+    android::base::ConsumePrefix(&local_path, "/");
+  }
+  std::string normalized = CF_EXPECT(NormalizePath(std::string(local_path)));
+  // TODO(schuffelen): Do better for local builds here.
+  return CvdFile(purpose, build_id, build_target, normalized);
+}
+
 FetcherConfigs FetcherConfigs::Create(std::vector<FetcherConfig> configs) {
   if (configs.empty()) {
     configs.emplace_back();

base/cvd/cuttlefish/host/libs/config/fetcher_config.h

@@ -75,19 +75,19 @@ class FetcherConfig {
 
   std::string FindCvdFileWithSuffix(const std::string& suffix) const;
 
-  Result<void> AddFilesToConfig(FileSource purpose, const std::string& build_id,
-                                const std::string& build_target,
-                                const std::vector<std::string>& paths,
-                                const std::string& directory_prefix,
-                                bool override_entry = false);
-
   Result<void> RemoveFileFromConfig(const std::string& path);
 
  private:
   Json::Value dictionary_;
   std::unique_ptr<std::mutex> mutex_;
 };
 
+Result<CvdFile> BuildFetcherConfigMember(FileSource purpose,
+                                         const std::string& build_id,
+                                         const std::string& build_target,
+                                         const std::string& path,
+                                         const std::string& directory_prefix);
+
 class FetcherConfigs {
  public:
   static FetcherConfigs Create(std::vector<FetcherConfig> configs);