LocalTreeMap: Prevent tree objects from being stored

... to align with the original idea of caching a flat list
of blob objects, without the need to recursively traverse
any trees. Consequently, we cannot create any map entry in
places where we do not have all sub-tree entries at hand
(e.g., LocalAPI, BazelAPI, BazelResponse).

8 files changed, 106 insertions, 126 deletions

diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp
index 34fc5380..c2c44280 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_action.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_action.cpp
@@ -6,7 +6,6 @@
 
 BazelAction::BazelAction(
     std::shared_ptr<BazelNetwork> network,
-    std::shared_ptr<LocalTreeMap> tree_map,
     bazel_re::Digest root_digest,
     std::vector<std::string> command,
     std::vector<std::string> output_files,
@@ -14,7 +13,6 @@ BazelAction::BazelAction(
     std::map<std::string, std::string> const& env_vars,
     std::map<std::string, std::string> const& properties) noexcept
     : network_{std::move(network)},
-      tree_map_{std::move(tree_map)},
       root_digest_{std::move(root_digest)},
       cmdline_{std::move(command)},
       output_files_{std::move(output_files)},
@@ -47,7 +45,7 @@ auto BazelAction::Execute(Logger const* logger) noexcept
                 network_->GetCachedActionResult(action, output_files_)) {
             if (result->exit_code() == 0) {
                 return IExecutionResponse::Ptr{new BazelResponse{
-                    action.hash(), network_, tree_map_, {*result, true}}};
+                    action.hash(), network_, {*result, true}}};
             }
         }
     }
@@ -59,14 +57,11 @@ auto BazelAction::Execute(Logger const* logger) noexcept
                 auto action_id =
                     CreateBundlesForAction(nullptr, root_digest_, false).hash();
                 output->cached_result = true;
-                return IExecutionResponse::Ptr{
-                    new BazelResponse{std::move(action_id),
-                                      network_,
-                                      tree_map_,
-                                      std::move(*output)}};
+                return IExecutionResponse::Ptr{new BazelResponse{
+                    std::move(action_id), network_, std::move(*output)}};
             }
-            return IExecutionResponse::Ptr{new BazelResponse{
-                action.hash(), network_, tree_map_, std::move(*output)}};
+            return IExecutionResponse::Ptr{
+                new BazelResponse{action.hash(), network_, std::move(*output)}};
         }
     }
 
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_action.hpp b/src/buildtool/execution_api/remote/bazel/bazel_action.hpp
index 7eb9a9e0..84922a86 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_action.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_action.hpp
@@ -26,7 +26,6 @@ class BazelAction final : public IExecutionAction {
 
   private:
     std::shared_ptr<BazelNetwork> network_;
-    std::shared_ptr<LocalTreeMap> tree_map_;
     bazel_re::Digest const root_digest_;
     std::vector<std::string> const cmdline_;
     std::vector<std::string> output_files_;
@@ -37,7 +36,6 @@ class BazelAction final : public IExecutionAction {
     std::chrono::milliseconds timeout_{kDefaultTimeout};
 
     BazelAction(std::shared_ptr<BazelNetwork> network,
-                std::shared_ptr<LocalTreeMap> tree_map,
                 bazel_re::Digest root_digest,
                 std::vector<std::string> command,
                 std::vector<std::string> output_files,
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
index a8846325..d1de9a68 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.cpp
@@ -24,9 +24,8 @@ BazelApi::BazelApi(std::string const& instance_name,
                    std::string const& host,
                    Port port,
                    ExecutionConfiguration const& exec_config) noexcept {
-    tree_map_ = std::make_shared<LocalTreeMap>();
-    network_ = std::make_shared<BazelNetwork>(
-        instance_name, host, port, exec_config, tree_map_);
+    network_ =
+        std::make_shared<BazelNetwork>(instance_name, host, port, exec_config);
 }
 
 // implement move constructor in cpp, where all members are complete types
@@ -44,7 +43,6 @@ auto BazelApi::CreateAction(
     std::map<std::string, std::string> const& properties) noexcept
     -> IExecutionAction::Ptr {
     return std::unique_ptr<BazelAction>{new BazelAction{network_,
-                                                        tree_map_,
                                                         root_digest,
                                                         command,
                                                         output_files,
@@ -227,11 +225,9 @@ auto BazelApi::CreateAction(
     std::vector<DependencyGraph::NamedArtifactNodePtr> const&
         artifacts) noexcept -> std::optional<ArtifactDigest> {
     BlobContainer blobs{};
-    auto tree = tree_map_->CreateTree();
     auto digest = BazelMsgFactory::CreateDirectoryDigestFromTree(
         artifacts,
-        [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); },
-        [&tree](auto path, auto info) { return tree.AddInfo(path, info); });
+        [&blobs](BazelBlob&& blob) { blobs.Emplace(std::move(blob)); });
     if (not digest) {
         Logger::Log(LogLevel::Debug, "failed to create digest for tree.");
         return std::nullopt;
@@ -248,10 +244,8 @@ auto BazelApi::CreateAction(
         Logger::Log(LogLevel::Debug, "failed to upload blobs for tree.");
         return std::nullopt;
     }
-    if (tree_map_->AddTree(*digest, std::move(tree))) {
-        return ArtifactDigest{*digest};
-    }
-    return std::nullopt;
+
+    return ArtifactDigest{*digest};
 }
 
 [[nodiscard]] auto BazelApi::IsAvailable(
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp
index 3fccf226..27310f07 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_api.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_api.hpp
@@ -10,7 +10,6 @@
 #include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/execution_api/bazel_msg/bazel_common.hpp"
 #include "src/buildtool/execution_api/common/execution_api.hpp"
-#include "src/buildtool/execution_api/common/local_tree_map.hpp"
 #include "src/buildtool/execution_api/remote/config.hpp"
 
 // forward declaration for actual implementations
@@ -68,7 +67,6 @@ class BazelApi final : public IExecutionApi {
 
   private:
     std::shared_ptr<BazelNetwork> network_;
-    std::shared_ptr<LocalTreeMap> tree_map_;
 };
 
 #endif  // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BAZEL_API_HPP
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
index ff151824..fa09b16c 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network.cpp
@@ -80,14 +80,12 @@ namespace {
 BazelNetwork::BazelNetwork(std::string instance_name,
                            std::string const& host,
                            Port port,
-                           ExecutionConfiguration const& exec_config,
-                           std::shared_ptr<LocalTreeMap> tree_map) noexcept
+                           ExecutionConfiguration const& exec_config) noexcept
     : instance_name_{std::move(instance_name)},
       exec_config_{exec_config},
       cas_{std::make_unique<BazelCasClient>(host, port)},
       ac_{std::make_unique<BazelAcClient>(host, port)},
-      exec_{std::make_unique<BazelExecutionClient>(host, port)},
-      tree_map_{std::move(tree_map)} {}
+      exec_{std::make_unique<BazelExecutionClient>(host, port)} {}
 
 auto BazelNetwork::IsAvailable(bazel_re::Digest const& digest) const noexcept
     -> bool {
@@ -245,8 +243,7 @@ auto BazelNetwork::ReadTreeInfos(bazel_re::Digest const& tree_digest,
     -> std::optional<std::pair<std::vector<std::filesystem::path>,
                                std::vector<Artifact::ObjectInfo>>> {
     std::optional<DirectoryMap> dir_map{std::nullopt};
-    if (Compatibility::IsCompatible() and
-        (request_remote_tree or not tree_map_)) {
+    if (Compatibility::IsCompatible() and request_remote_tree) {
         // Query full tree from remote CAS. Note that this is currently not
         // supported by Buildbarn revision c3c06bbe2a.
         auto dirs =
@@ -289,102 +286,117 @@ auto BazelNetwork::ReadObjectInfosRecursively(
     std::filesystem::path const& parent,
     bazel_re::Digest const& digest) const noexcept -> bool {
     // read from in-memory tree map
-    if (tree_map_) {
-        auto const* tree = tree_map_->GetTree(digest);
-        if (tree != nullptr) {
-            return std::all_of(
-                tree->begin(),
-                tree->end(),
-                // NOLINTNEXTLINE(misc-no-recursion)
-                [this, &dir_map, &store_info, &parent](auto const& entry) {
+    auto const* tree = tree_map_.GetTree(digest);
+    if (tree != nullptr) {
+        return std::all_of(
+            tree->begin(),
+            tree->end(),
+            [&store_info, &parent](auto const& entry) {
+                try {
+                    // LocalTree (from tree_map_) is flat, no recursion needed
                     auto const& [path, info] = entry;
-                    try {
-                        return IsTreeObject(info->type)
-                                   ? ReadObjectInfosRecursively(dir_map,
-                                                                store_info,
-                                                                parent / path,
-                                                                info->digest)
-                                   : store_info(parent / path, *info);
-                    } catch (
-                        ...) {  // satisfy clang-tidy, store_info() could throw
-                        return false;
-                    }
-                });
-        }
-        Logger::Log(
-            LogLevel::Debug, "tree {} not found in tree map", digest.hash());
+                    return store_info(parent / path, *info);
+                } catch (...) {  // satisfy clang-tidy, store_info() could throw
+                    return false;
+                }
+            });
     }
+    Logger::Log(
+        LogLevel::Debug, "tree {} not found in tree map", digest.hash());
 
     if (Compatibility::IsCompatible()) {
         // read from in-memory Directory map and cache it in in-memory tree map
         if (dir_map) {
             if (dir_map->contains(digest)) {
-                auto tree = tree_map_
-                                ? std::make_optional(tree_map_->CreateTree())
-                                : std::nullopt;
+                auto tree = tree_map_.CreateTree();
                 return BazelMsgFactory::ReadObjectInfosFromDirectory(
                            dir_map->at(digest),
                            [this, &dir_map, &store_info, &parent, &tree](
                                auto path, auto info) {
-                               return (not tree or
-                                       tree->AddInfo(path, info)) and
-                                      (IsTreeObject(info.type)
-                                           ? (not tree or
-                                              tree->AddInfo(path, info)) and
-                                                 ReadObjectInfosRecursively(
-                                                     dir_map,
-                                                     store_info,
-                                                     parent / path,
-                                                     info.digest)
-                                           : store_info(parent / path, info));
+                               if (IsTreeObject(info.type)) {
+                                   // LocalTree (from tree_map_) is flat, so
+                                   // recursively traverse subtrees and add
+                                   // blobs.
+                                   auto tree_store_info = [&store_info,
+                                                           &tree,
+                                                           &parent](auto path,
+                                                                    auto info) {
+                                       auto tree_path =
+                                           path.lexically_relative(parent);
+                                       return tree.AddInfo(tree_path, info) and
+                                              store_info(path, info);
+                                   };
+                                   return ReadObjectInfosRecursively(
+                                       dir_map,
+                                       tree_store_info,
+                                       parent / path,
+                                       info.digest);
+                               }
+                               return tree.AddInfo(path, info) and
+                                      store_info(parent / path, info);
                            }) and
-                       (not tree_map_ or
-                        tree_map_->AddTree(digest, std::move(*tree)));
+                       tree_map_.AddTree(digest, std::move(tree));
             }
-            Logger::Log(
-                LogLevel::Debug, "tree {} not found in dir map", digest.hash());
         }
 
         // fallback read from CAS and cache it in in-memory tree map
         if (auto dir = ReadDirectory(this, digest)) {
-            auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree())
-                                  : std::nullopt;
+            auto tree = tree_map_.CreateTree();
             return BazelMsgFactory::ReadObjectInfosFromDirectory(
                        *dir,
                        [this, &dir_map, &store_info, &parent, &tree](
                            auto path, auto info) {
-                           return (not tree or tree->AddInfo(path, info)) and
-                                  (IsTreeObject(info.type)
-                                       ? ReadObjectInfosRecursively(
-                                             dir_map,
-                                             store_info,
-                                             parent / path,
-                                             info.digest)
-                                       : store_info(parent / path, info));
+                           if (IsTreeObject(info.type)) {
+                               // LocalTree (from tree_map_) is flat, so
+                               // recursively traverse subtrees and add blobs.
+                               auto tree_store_info =
+                                   [&store_info, &tree, &parent](auto path,
+                                                                 auto info) {
+                                       auto tree_path =
+                                           path.lexically_relative(parent);
+                                       return tree.AddInfo(tree_path, info) and
+                                              store_info(path, info);
+                                   };
+                               return ReadObjectInfosRecursively(
+                                   dir_map,
+                                   tree_store_info,
+                                   parent / path,
+                                   info.digest);
+                           }
+                           return tree.AddInfo(path, info) and
+                                  store_info(parent / path, info);
                        }) and
-                   (not tree_map_ or
-                    tree_map_->AddTree(digest, std::move(*tree)));
+                   tree_map_.AddTree(digest, std::move(tree));
         }
     }
     else {
         if (auto entries = ReadGitTree(this, digest)) {
-            auto tree = tree_map_ ? std::make_optional(tree_map_->CreateTree())
-                                  : std::nullopt;
+            auto tree = tree_map_.CreateTree();
             return BazelMsgFactory::ReadObjectInfosFromGitTree(
                        *entries,
                        [this, &dir_map, &store_info, &parent, &tree](
                            auto path, auto info) {
-                           return (not tree or tree->AddInfo(path, info)) and
-                                  (IsTreeObject(info.type)
-                                       ? ReadObjectInfosRecursively(
-                                             dir_map,
-                                             store_info,
-                                             parent / path,
-                                             info.digest)
-                                       : store_info(parent / path, info));
+                           if (IsTreeObject(info.type)) {
+                               // LocalTree (from tree_map_) is flat, so
+                               // recursively traverse subtrees and add blobs.
+                               auto tree_store_info =
+                                   [&store_info, &tree, &parent](auto path,
+                                                                 auto info) {
+                                       auto tree_path =
+                                           path.lexically_relative(parent);
+                                       return tree.AddInfo(tree_path, info) and
+                                              store_info(path, info);
+                                   };
+                               return ReadObjectInfosRecursively(
+                                   dir_map,
+                                   tree_store_info,
+                                   parent / path,
+                                   info.digest);
+                           }
+                           return tree.AddInfo(path, info) and
+                                  store_info(parent / path, info);
                        }) and
-                   (not tree_map_ or
-                    tree_map_->AddTree(digest, std::move(*tree)));
+                   tree_map_.AddTree(digest, std::move(tree));
         }
     }
     return false;
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp
index 13a5bcf7..6e9d9c85 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_network.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_network.hpp
@@ -45,8 +45,7 @@ class BazelNetwork {
     BazelNetwork(std::string instance_name,
                  std::string const& host,
                  Port port,
-                 ExecutionConfiguration const& exec_config,
-                 std::shared_ptr<LocalTreeMap> tree_map = nullptr) noexcept;
+                 ExecutionConfiguration const& exec_config) noexcept;
 
     /// \brief Check if digest exists in CAS
     /// \param[in]  digest  The digest to look up
@@ -105,7 +104,7 @@ class BazelNetwork {
     std::unique_ptr<BazelCasClient> cas_{};
     std::unique_ptr<BazelAcClient> ac_{};
     std::unique_ptr<BazelExecutionClient> exec_{};
-    std::shared_ptr<LocalTreeMap> tree_map_{};
+    mutable LocalTreeMap tree_map_{};
 
     template <class T_Iter>
     [[nodiscard]] auto DoUploadBlobs(T_Iter const& first,
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
index d05517a1..6ae2457e 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_response.cpp
@@ -5,6 +5,17 @@
 #include "src/buildtool/execution_api/remote/bazel/bazel_cas_client.hpp"
 #include "src/buildtool/logging/logger.hpp"
 
+namespace {
+
+auto ProcessDirectoryMessage(bazel_re::Directory const& dir) noexcept
+    -> std::optional<BazelBlob> {
+    auto data = dir.SerializeAsString();
+    auto digest = ArtifactDigest::Create(data);
+    return BazelBlob{std::move(digest), std::move(data)};
+}
+
+}  // namespace
+
 auto BazelResponse::ReadStringBlob(bazel_re::Digest const& id) noexcept
     -> std::string {
     auto blobs = network_->ReadBlobs({id}).Next();
@@ -120,24 +131,3 @@ auto BazelResponse::UploadTreeMessageDirectories(
     }
     return ArtifactDigest{root_digest};
 }
-
-auto BazelResponse::ProcessDirectoryMessage(
-    bazel_re::Directory const& dir) const noexcept -> std::optional<BazelBlob> {
-    auto data = dir.SerializeAsString();
-    auto digest = ArtifactDigest::Create(data);
-
-    if (tree_map_ and not tree_map_->HasTree(digest)) {
-        // cache in local tree map
-        auto tree = tree_map_->CreateTree();
-        if (not BazelMsgFactory::ReadObjectInfosFromDirectory(
-                dir,
-                [&tree](auto path, auto info) {
-                    return tree.AddInfo(path, info);
-                }) or
-            not tree_map_->AddTree(digest, std::move(tree))) {
-            return std::nullopt;
-        }
-    }
-
-    return BazelBlob{std::move(digest), std::move(data)};
-}
diff --git a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp
index 778efa0a..f304210f 100644
--- a/src/buildtool/execution_api/remote/bazel/bazel_response.hpp
+++ b/src/buildtool/execution_api/remote/bazel/bazel_response.hpp
@@ -47,16 +47,13 @@ class BazelResponse final : public IExecutionResponse {
   private:
     std::string action_id_{};
     std::shared_ptr<BazelNetwork> const network_{};
-    std::shared_ptr<LocalTreeMap> const tree_map_{};
     BazelExecutionClient::ExecutionOutput output_{};
 
     BazelResponse(std::string action_id,
                   std::shared_ptr<BazelNetwork> network,
-                  std::shared_ptr<LocalTreeMap> tree_map,
                   BazelExecutionClient::ExecutionOutput output)
         : action_id_{std::move(action_id)},
           network_{std::move(network)},
-          tree_map_{std::move(tree_map)},
           output_{std::move(output)} {}
 
     [[nodiscard]] auto ReadStringBlob(bazel_re::Digest const& id) noexcept
@@ -69,9 +66,6 @@ class BazelResponse final : public IExecutionResponse {
 
     [[nodiscard]] auto UploadTreeMessageDirectories(
         bazel_re::Tree const& tree) const -> std::optional<ArtifactDigest>;
-
-    [[nodiscard]] auto ProcessDirectoryMessage(bazel_re::Directory const& dir)
-        const noexcept -> std::optional<BazelBlob>;
 };
 
 #endif  // INCLUDED_SRC_BUILDTOOL_EXECUTION_API_REMOTE_BAZEL_BAZEL_RESPONSE_HPP