Replace bazel_re::Digest in LocalAC

...with ArtifactDigest.

7 files changed, 47 insertions, 41 deletions

diff --git a/src/buildtool/execution_api/execution_service/TARGETS b/src/buildtool/execution_api/execution_service/TARGETS
index ecdccf04..f52efc26 100644
--- a/src/buildtool/execution_api/execution_service/TARGETS
+++ b/src/buildtool/execution_api/execution_service/TARGETS
@@ -23,6 +23,7 @@
     , "operation_cache"
     , ["src/utils/cpp", "verify_hash"]
     , ["src/buildtool/execution_api/local", "local"]
+    , ["src/buildtool/common", "common"]
     ]
   , "private-ldflags":
     ["-pthread", "-Wl,--whole-archive,-lpthread,--no-whole-archive"]
@@ -43,7 +44,10 @@
     , ["src/buildtool/storage", "config"]
     ]
   , "private-deps":
-    [["src/buildtool/logging", "log_level"], ["src/utils/cpp", "verify_hash"]]
+    [ ["src/buildtool/logging", "log_level"]
+    , ["src/utils/cpp", "verify_hash"]
+    , ["src/buildtool/common", "common"]
+    ]
   }
 , "cas_server":
   { "type": ["@", "rules", "CC", "library"]
diff --git a/src/buildtool/execution_api/execution_service/ac_server.cpp b/src/buildtool/execution_api/execution_service/ac_server.cpp
index 0d85596e..cc59a983 100644
--- a/src/buildtool/execution_api/execution_service/ac_server.cpp
+++ b/src/buildtool/execution_api/execution_service/ac_server.cpp
@@ -15,6 +15,7 @@
 #include "src/buildtool/execution_api/execution_service/ac_server.hpp"
 
 #include "fmt/core.h"
+#include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/logging/log_level.hpp"
 #include "src/buildtool/storage/garbage_collector.hpp"
 #include "src/utils/cpp/verify_hash.hpp"
@@ -36,11 +37,12 @@ auto ActionCacheServiceImpl::GetActionResult(
         logger_.Emit(LogLevel::Error, str);
         return grpc::Status{grpc::StatusCode::INTERNAL, str};
     }
-    auto x = storage_.ActionCache().CachedResult(request->action_digest());
+
+    ArtifactDigest const a_digest{request->action_digest()};
+    auto x = storage_.ActionCache().CachedResult(a_digest);
     if (not x) {
-        return grpc::Status{
-            grpc::StatusCode::NOT_FOUND,
-            fmt::format("{} missing from AC", request->action_digest().hash())};
+        return grpc::Status{grpc::StatusCode::NOT_FOUND,
+                            fmt::format("{} missing from AC", a_digest.hash())};
     }
     *response = *x;
     return ::grpc::Status::OK;
diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp
index b862a03d..c89f06fe 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.cpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.cpp
@@ -19,8 +19,8 @@
 #include <string>
 #include <utility>
 
-#include "execution_server.hpp"
 #include "fmt/core.h"
+#include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/execution_api/execution_service/operation_cache.hpp"
 #include "src/buildtool/execution_api/local/local_cas_reader.hpp"
 #include "src/buildtool/file_system/file_system_manager.hpp"
@@ -212,8 +212,9 @@ auto ExecutionServiceImpl::Execute(
 
     // Store the result in action cache
     if (i_execution_response->ExitCode() == 0 and not action->do_not_cache()) {
+        ArtifactDigest const a_digest{request->action_digest()};
         if (not storage_.ActionCache().StoreResult(
-                request->action_digest(), execute_response->result())) {
+                a_digest, execute_response->result())) {
             auto const str =
                 fmt::format("Could not store action result for action {}",
                             request->action_digest().hash());
diff --git a/src/buildtool/execution_api/local/local_action.cpp b/src/buildtool/execution_api/local/local_action.cpp
index 8cc1d242..ad106c6f 100644
--- a/src/buildtool/execution_api/local/local_action.cpp
+++ b/src/buildtool/execution_api/local/local_action.cpp
@@ -21,6 +21,7 @@
 #include <system_error>
 #include <utility>
 
+#include "src/buildtool/common/artifact_digest.hpp"
 #include "src/buildtool/common/bazel_types.hpp"
 #include "src/buildtool/compatibility/native_support.hpp"
 #include "src/buildtool/execution_api/common/tree_reader.hpp"
@@ -104,8 +105,9 @@ auto LocalAction::Execute(Logger const* logger) noexcept
     }
 
     if (do_cache) {
+        ArtifactDigest const a_digest{*action};
         if (auto result =
-                local_context_.storage->ActionCache().CachedResult(*action)) {
+                local_context_.storage->ActionCache().CachedResult(a_digest)) {
             if (result->exit_code() == 0 and
                 ActionResultContainsExpectedOutputs(
                     *result, output_files_, output_dirs_)) {
@@ -190,8 +192,9 @@ auto LocalAction::Run(bazel_re::Digest const& action_id) const noexcept
 
         if (CollectAndStoreOutputs(&result.action, build_root / cwd_)) {
             if (cache_flag_ == CacheFlag::CacheOutput) {
+                ArtifactDigest const a_digest{action_id};
                 if (not local_context_.storage->ActionCache().StoreResult(
-                        action_id, result.action)) {
+                        a_digest, result.action)) {
                     logger_.Emit(LogLevel::Warning,
                                  "failed to store action results");
                 }
diff --git a/src/buildtool/storage/local_ac.hpp b/src/buildtool/storage/local_ac.hpp
index b39c63dd..180f1c51 100644
--- a/src/buildtool/storage/local_ac.hpp
+++ b/src/buildtool/storage/local_ac.hpp
@@ -32,7 +32,6 @@
 
 // forward declarations
 namespace build::bazel::remote::execution::v2 {
-class Digest;
 class ActionResult;
 }  // namespace build::bazel::remote::execution::v2
 namespace bazel_re = build::bazel::remote::execution::v2;
@@ -65,13 +64,13 @@ class LocalAC {
     /// \param result       The action result to store.
     /// \returns true on success.
     [[nodiscard]] auto StoreResult(
-        bazel_re::Digest const& action_id,
+        ArtifactDigest const& action_id,
         bazel_re::ActionResult const& result) const noexcept -> bool;
 
     /// \brief Read cached action result.
     /// \param action_id    The id of the action the result was produced by.
     /// \returns The action result if found or nullopt otherwise.
-    [[nodiscard]] auto CachedResult(bazel_re::Digest const& action_id)
+    [[nodiscard]] auto CachedResult(ArtifactDigest const& action_id)
         const noexcept -> std::optional<bazel_re::ActionResult>;
 
     /// \brief Uplink entry from this generation to latest LocalAC generation.
@@ -113,23 +112,23 @@ class LocalAC {
     /// \param action_id The id of the action that produced the result.
     /// \return The key of an Action pointing at an ActionResult in the LocalCAS
     /// on success or an error message on failure.
-    [[nodiscard]] auto ReadActionKey(bazel_re::Digest const& action_id)
-        const noexcept -> expected<bazel_re::Digest, std::string>;
+    [[nodiscard]] auto ReadActionKey(ArtifactDigest const& action_id)
+        const noexcept -> expected<ArtifactDigest, std::string>;
 
     /// \brief Add an action to the LocalCAS.
     /// \param action The action result to store.
     /// \return The key pointing at an ActionResult present in the LocalCAS on
     /// success or std::nullopt on failure.
     [[nodiscard]] auto WriteAction(bazel_re::ActionResult const& action)
-        const noexcept -> std::optional<bazel_re::Digest>;
+        const noexcept -> std::optional<ArtifactDigest>;
 
     /// \brief Get the action specified by a key from the LocalCAS.
     /// \param cas_key The key pointing at an ActionResult present in the
     /// LocalCAS.
     /// \return The ActionResult corresponding to a cas_key on success
     /// or std::nullopt on failure.
-    [[nodiscard]] auto ReadAction(bazel_re::Digest const& cas_key)
-        const noexcept -> std::optional<bazel_re::ActionResult>;
+    [[nodiscard]] auto ReadAction(ArtifactDigest const& cas_key) const noexcept
+        -> std::optional<bazel_re::ActionResult>;
 };
 
 #ifndef BOOTSTRAP_BUILD_TOOL
diff --git a/src/buildtool/storage/local_ac.tpp b/src/buildtool/storage/local_ac.tpp
index 4bd63b5c..3bf5ba31 100644
--- a/src/buildtool/storage/local_ac.tpp
+++ b/src/buildtool/storage/local_ac.tpp
@@ -26,16 +26,14 @@
 
 template <bool kDoGlobalUplink>
 auto LocalAC<kDoGlobalUplink>::StoreResult(
-    bazel_re::Digest const& action_id,
+    ArtifactDigest const& action_id,
     bazel_re::ActionResult const& result) const noexcept -> bool {
     auto const cas_key = WriteAction(result);
-    return cas_key.has_value() and
-           WriteActionKey(static_cast<ArtifactDigest>(action_id),
-                          static_cast<ArtifactDigest>(*cas_key));
+    return cas_key.has_value() and WriteActionKey(action_id, *cas_key);
 }
 
 template <bool kDoGlobalUplink>
-auto LocalAC<kDoGlobalUplink>::CachedResult(bazel_re::Digest const& action_id)
+auto LocalAC<kDoGlobalUplink>::CachedResult(ArtifactDigest const& action_id)
     const noexcept -> std::optional<bazel_re::ActionResult> {
     auto const cas_key = ReadActionKey(action_id);
     if (not cas_key) {
@@ -46,7 +44,7 @@ auto LocalAC<kDoGlobalUplink>::CachedResult(bazel_re::Digest const& action_id)
     if (not result) {
         logger_->Emit(LogLevel::Warning,
                       "Parsing action result failed for action {}",
-                      NativeSupport::Unprefix(action_id.hash()));
+                      action_id.hash());
         return std::nullopt;
     }
     return std::move(result);
@@ -59,13 +57,14 @@ auto LocalAC<kDoGlobalUplink>::LocalUplinkEntry(
     LocalGenerationAC const& latest,
     bazel_re::Digest const& action_id) const noexcept -> bool {
     // Determine action cache key path in latest generation.
-    auto const key_digest = NativeSupport::Unprefix(action_id.hash());
-    if (FileSystemManager::IsFile(latest.file_store_.GetPath(key_digest))) {
+    ArtifactDigest const a_digest{action_id};
+    if (FileSystemManager::IsFile(
+            latest.file_store_.GetPath(a_digest.hash()))) {
         return true;
     }
 
     // Read cache key
-    auto const cas_key = ReadActionKey(action_id);
+    auto const cas_key = ReadActionKey(a_digest);
     if (not cas_key) {
         return false;
     }
@@ -112,9 +111,9 @@ auto LocalAC<kDoGlobalUplink>::LocalUplinkEntry(
         return false;
     }
 
-    auto const ac_entry_path = file_store_.GetPath(key_digest);
+    auto const ac_entry_path = file_store_.GetPath(a_digest.hash());
     // Uplink cache key
-    return latest.file_store_.AddFromFile(key_digest,
+    return latest.file_store_.AddFromFile(a_digest.hash(),
                                           ac_entry_path,
                                           /*is_owner=*/true);
 }
@@ -128,10 +127,9 @@ auto LocalAC<kDoGlobalUplink>::WriteActionKey(
 }
 
 template <bool kDoGlobalUplink>
-auto LocalAC<kDoGlobalUplink>::ReadActionKey(bazel_re::Digest const& action_id)
-    const noexcept -> expected<bazel_re::Digest, std::string> {
-    auto const key_path =
-        file_store_.GetPath(NativeSupport::Unprefix(action_id.hash()));
+auto LocalAC<kDoGlobalUplink>::ReadActionKey(ArtifactDigest const& action_id)
+    const noexcept -> expected<ArtifactDigest, std::string> {
+    auto const key_path = file_store_.GetPath(action_id.hash());
 
     if constexpr (kDoGlobalUplink) {
         // Uplink any existing action-cache entries in storage generations
@@ -145,31 +143,30 @@ auto LocalAC<kDoGlobalUplink>::ReadActionKey(bazel_re::Digest const& action_id)
             fmt::format("Cache miss, entry not found {}", key_path.string())};
     }
 
-    std::optional<bazel_re::Digest> action_key;
+    std::optional<ArtifactDigest> action_key;
     try {
         nlohmann::json j = nlohmann::json::parse(*key_content);
         action_key = ArtifactDigest{j[0].template get<std::string>(),
                                     j[1].template get<std::size_t>(),
                                     /*is_tree=*/false};
     } catch (...) {
-        return unexpected{
-            fmt::format("Parsing cache entry failed for action {}",
-                        NativeSupport::Unprefix(action_id.hash()))};
+        return unexpected{fmt::format(
+            "Parsing cache entry failed for action {}", action_id.hash())};
     }
     return *std::move(action_key);
 }
 
 template <bool kDoGlobalUplink>
 auto LocalAC<kDoGlobalUplink>::WriteAction(bazel_re::ActionResult const& action)
-    const noexcept -> std::optional<bazel_re::Digest> {
+    const noexcept -> std::optional<ArtifactDigest> {
     return cas_.StoreBlob(action.SerializeAsString(),
                           /*is_executable=*/false);
 }
 
 template <bool kDoGlobalUplink>
-auto LocalAC<kDoGlobalUplink>::ReadAction(bazel_re::Digest const& cas_key)
+auto LocalAC<kDoGlobalUplink>::ReadAction(ArtifactDigest const& cas_key)
     const noexcept -> std::optional<bazel_re::ActionResult> {
-    auto const action_path = cas_.BlobPath(static_cast<ArtifactDigest>(cas_key),
+    auto const action_path = cas_.BlobPath(cas_key,
                                            /*is_executable=*/false);
     if (not action_path) {
         return std::nullopt;
diff --git a/test/buildtool/storage/local_ac.test.cpp b/test/buildtool/storage/local_ac.test.cpp
index c692a75c..9923108d 100644
--- a/test/buildtool/storage/local_ac.test.cpp
+++ b/test/buildtool/storage/local_ac.test.cpp
@@ -27,7 +27,7 @@
 [[nodiscard]] static auto RunDummyExecution(
     gsl::not_null<LocalAC<true> const*> const& ac,
     gsl::not_null<LocalCAS<true> const*> const& cas_,
-    bazel_re::Digest const& action_id,
+    ArtifactDigest const& action_id,
     std::string const& seed) -> bool;
 
 TEST_CASE("LocalAC: Single action, single result", "[storage]") {
@@ -136,7 +136,7 @@ TEST_CASE("LocalAC: Same two actions, two different results", "[storage]") {
 
 auto RunDummyExecution(gsl::not_null<LocalAC<true> const*> const& ac,
                        gsl::not_null<LocalCAS<true> const*> const& cas_,
-                       bazel_re::Digest const& action_id,
+                       ArtifactDigest const& action_id,
                        std::string const& seed) -> bool {
     bazel_re::ActionResult result{};
     *result.add_output_files() = [&]() {