Make OperationCache a general class, not singleton

As it is used by just execute only, instantiate it inside the
ExecutionServer, which reads and writes to the cache map, and pass
a const ref to OperationsServer, which only queries.

8 files changed, 57 insertions, 25 deletions

diff --git a/src/buildtool/execution_api/execution_service/TARGETS b/src/buildtool/execution_api/execution_service/TARGETS
index caaf2b7d..7d82602c 100644
--- a/src/buildtool/execution_api/execution_service/TARGETS
+++ b/src/buildtool/execution_api/execution_service/TARGETS
@@ -6,7 +6,8 @@
   , "proto": [["@", "bazel_remote_apis", "", "remote_execution_proto"]]
   , "stage": ["src", "buildtool", "execution_api", "execution_service"]
   , "deps":
-    [ ["@", "gsl", "", "gsl"]
+    [ "operation_cache"
+    , ["@", "gsl", "", "gsl"]
     , ["src/buildtool/execution_api/common", "common"]
     , ["src/buildtool/logging", "logging"]
     , ["src/buildtool/common", "bazel_types"]
@@ -141,7 +142,11 @@
   , "name": ["operations_server"]
   , "hdrs": ["operations_server.hpp"]
   , "srcs": ["operations_server.cpp"]
-  , "deps": [["src/buildtool/logging", "logging"]]
+  , "deps":
+    [ "operation_cache"
+    , ["@", "gsl", "", "gsl"]
+    , ["src/buildtool/logging", "logging"]
+    ]
   , "proto": [["@", "googleapis", "", "google_longrunning_operations_proto"]]
   , "stage": ["src", "buildtool", "execution_api", "execution_service"]
   , "private-deps":
diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp
index 62deaeea..b77ad9d3 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.cpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.cpp
@@ -427,7 +427,7 @@ auto ExecutionServiceImpl::StoreActionResult(
     }
     return std::nullopt;
 }
-static void WriteResponse(
+void ExecutionServiceImpl::WriteResponse(
     ::bazel_re::ExecuteResponse const& execute_response,
     ::grpc::ServerWriter<::google::longrunning::Operation>* writer,
     ::google::longrunning::Operation* op) noexcept {
@@ -437,7 +437,7 @@ static void WriteResponse(
     op->set_done(true);
     UpdateTimeStamp(op);
 
-    OperationCache::Set(op->name(), *op);
+    op_cache_.Set(op->name(), *op);
     writer->Write(*op);
 }
 
@@ -469,7 +469,7 @@ auto ExecutionServiceImpl::Execute(
     op.set_name(op_name);
     op.set_done(false);
     UpdateTimeStamp(&op);
-    OperationCache::Set(op_name, op);
+    op_cache_.Set(op_name, op);
     writer->Write(op);
     auto t0 = std::chrono::high_resolution_clock::now();
     auto i_execution_response = i_execution_action->get()->Execute(&logger_);
@@ -507,7 +507,7 @@ auto ExecutionServiceImpl::WaitExecution(
     logger_.Emit(LogLevel::Trace, "WaitExecution: {}", hash);
     std::optional<::google::longrunning::Operation> op;
     do {
-        op = OperationCache::Query(hash);
+        op = op_cache_.Query(hash);
         if (!op) {
             auto const& str = fmt::format(
                 "Executing action {} not found in internal cache.", hash);
diff --git a/src/buildtool/execution_api/execution_service/execution_server.hpp b/src/buildtool/execution_api/execution_service/execution_server.hpp
index cc72a1e3..7107ed45 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.hpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.hpp
@@ -15,10 +15,16 @@
 #ifndef EXECUTION_SERVER_HPP
 #define EXECUTION_SERVER_HPP
 
+#include <cstdint>
+#include <optional>
+#include <string>
+#include <utility>
+
 #include "build/bazel/remote/execution/v2/remote_execution.grpc.pb.h"
 #include "gsl/gsl"
 #include "src/buildtool/common/bazel_types.hpp"
 #include "src/buildtool/execution_api/common/execution_api.hpp"
+#include "src/buildtool/execution_api/execution_service/operation_cache.hpp"
 #include "src/buildtool/logging/logger.hpp"
 #include "src/buildtool/storage/config.hpp"
 #include "src/buildtool/storage/storage.hpp"
@@ -28,10 +34,19 @@ class ExecutionServiceImpl final : public bazel_re::Execution::Service {
     explicit ExecutionServiceImpl(
         gsl::not_null<StorageConfig const*> const& storage_config,
         gsl::not_null<Storage const*> const& storage,
-        gsl::not_null<IExecutionApi const*> const& local_api) noexcept
+        gsl::not_null<IExecutionApi const*> const& local_api,
+        std::optional<std::uint8_t> op_exponent) noexcept
         : storage_config_{*storage_config},
           storage_{*storage},
-          api_{*local_api} {}
+          api_{*local_api} {
+        if (op_exponent) {
+            op_cache_.SetExponent(*op_exponent);
+        }
+    }
+
+    [[nodiscard]] auto GetOpCache() const noexcept -> OperationCache const& {
+        return op_cache_;
+    }
 
     // Execute an action remotely.
     //
@@ -119,6 +134,7 @@ class ExecutionServiceImpl final : public bazel_re::Execution::Service {
     StorageConfig const& storage_config_;
     Storage const& storage_;
     IExecutionApi const& api_;
+    OperationCache op_cache_;
     Logger logger_{"execution-service"};
 
     [[nodiscard]] auto GetAction(::bazel_re::ExecuteRequest const* request)
@@ -147,6 +163,11 @@ class ExecutionServiceImpl final : public bazel_re::Execution::Service {
         ::bazel_re::Action const& action) const noexcept
         -> std::optional<std::string>;
 
+    void WriteResponse(
+        ::bazel_re::ExecuteResponse const& execute_response,
+        ::grpc::ServerWriter<::google::longrunning::Operation>* writer,
+        ::google::longrunning::Operation* op) noexcept;
+
     [[nodiscard]] auto AddResult(
         ::bazel_re::ExecuteResponse* response,
         IExecutionResponse::Ptr const& i_execution_response,
diff --git a/src/buildtool/execution_api/execution_service/operation_cache.hpp b/src/buildtool/execution_api/execution_service/operation_cache.hpp
index 317bade4..a53aff98 100644
--- a/src/buildtool/execution_api/execution_service/operation_cache.hpp
+++ b/src/buildtool/execution_api/execution_service/operation_cache.hpp
@@ -29,14 +29,10 @@
 #include "google/longrunning/operations.pb.h"
 #include "google/protobuf/timestamp.pb.h"
 
-class OperationCache {
+class OperationCache final {
     using Operation = ::google::longrunning::Operation;
 
   public:
-    [[nodiscard]] static auto Instance() -> OperationCache& {
-        static OperationCache x;
-        return x;
-    }
     OperationCache() noexcept = default;
     ~OperationCache() noexcept = default;
 
@@ -45,18 +41,16 @@ class OperationCache {
     OperationCache(OperationCache&&) = delete;
     auto operator=(OperationCache&&) -> OperationCache& = delete;
 
-    static void Set(std::string const& action, Operation const& op) {
-        Instance().SetInternal(action, op);
+    void Set(std::string const& action, Operation const& op) {
+        SetInternal(action, op);
     }
 
-    [[nodiscard]] static auto Query(std::string const& x) noexcept
+    [[nodiscard]] auto Query(std::string const& x) const noexcept
         -> std::optional<Operation> {
-        return Instance().QueryInternal(x);
+        return QueryInternal(x);
     }
 
-    static void SetExponent(std::uint8_t x) noexcept {
-        Instance().threshold_ = 1U << x;
-    }
+    void SetExponent(std::uint8_t x) noexcept { threshold_ = 1U << x; }
 
   private:
     mutable std::shared_mutex mutex_;
diff --git a/src/buildtool/execution_api/execution_service/operations_server.cpp b/src/buildtool/execution_api/execution_service/operations_server.cpp
index 6e082bf4..23023c7d 100644
--- a/src/buildtool/execution_api/execution_service/operations_server.cpp
+++ b/src/buildtool/execution_api/execution_service/operations_server.cpp
@@ -29,7 +29,7 @@ auto OperarationsServiceImpl::GetOperation(
     }
     logger_.Emit(LogLevel::Trace, "GetOperation: {}", hash);
     std::optional<::google::longrunning::Operation> op;
-    op = OperationCache::Query(hash);
+    op = op_cache_.Query(hash);
     if (!op) {
         auto const& str = fmt::format(
             "Executing action {} not found in internal cache.", hash);
diff --git a/src/buildtool/execution_api/execution_service/operations_server.hpp b/src/buildtool/execution_api/execution_service/operations_server.hpp
index 44e3b887..faa87602 100644
--- a/src/buildtool/execution_api/execution_service/operations_server.hpp
+++ b/src/buildtool/execution_api/execution_service/operations_server.hpp
@@ -16,11 +16,17 @@
 #define OPERATIONS_SERVER_HPP
 
 #include "google/longrunning/operations.grpc.pb.h"
+#include "gsl/gsl"
+#include "src/buildtool/execution_api/execution_service/operation_cache.hpp"
 #include "src/buildtool/logging/logger.hpp"
 
 class OperarationsServiceImpl final
     : public ::google::longrunning::Operations::Service {
   public:
+    explicit OperarationsServiceImpl(
+        gsl::not_null<OperationCache const*> const& op_cache)
+        : op_cache_{*op_cache} {};
+
     // Lists operations that match the specified filter in the request. If the
     // server doesn't support this method, it returns `UNIMPLEMENTED`.
     //
@@ -64,6 +70,7 @@ class OperarationsServiceImpl final
         ::google::protobuf::Empty* response) -> ::grpc::Status override;
 
   private:
+    OperationCache const& op_cache_;
     Logger logger_{"execution-service:operations"};
 };
 
diff --git a/src/buildtool/execution_api/execution_service/server_implementation.cpp b/src/buildtool/execution_api/execution_service/server_implementation.cpp
index 42cb6242..63322451 100644
--- a/src/buildtool/execution_api/execution_service/server_implementation.cpp
+++ b/src/buildtool/execution_api/execution_service/server_implementation.cpp
@@ -55,13 +55,15 @@ auto TryWrite(std::string const& file, T const& content) noexcept -> bool {
 
 auto ServerImpl::Run(StorageConfig const& storage_config,
                      Storage const& storage,
-                     ApiBundle const& apis) -> bool {
-    ExecutionServiceImpl es{&storage_config, &storage, &*apis.local};
+                     ApiBundle const& apis,
+                     std::optional<std::uint8_t> op_exponent) -> bool {
+    ExecutionServiceImpl es{
+        &storage_config, &storage, &*apis.local, op_exponent};
     ActionCacheServiceImpl ac{&storage_config, &storage};
     CASServiceImpl cas{&storage_config, &storage};
     BytestreamServiceImpl b{&storage_config, &storage};
     CapabilitiesServiceImpl cap{};
-    OperarationsServiceImpl op{};
+    OperarationsServiceImpl op{&es.GetOpCache()};
 
     grpc::ServerBuilder builder;
 
diff --git a/src/buildtool/execution_api/execution_service/server_implementation.hpp b/src/buildtool/execution_api/execution_service/server_implementation.hpp
index ecddb8bc..97264fe1 100644
--- a/src/buildtool/execution_api/execution_service/server_implementation.hpp
+++ b/src/buildtool/execution_api/execution_service/server_implementation.hpp
@@ -15,7 +15,9 @@
 #ifndef SERVER_IMPLEMENATION_HPP
 #define SERVER_IMPLEMENATION_HPP
 
+#include <cstdint>
 #include <fstream>
+#include <optional>
 #include <string>
 
 #include "src/buildtool/execution_api/common/api_bundle.hpp"
@@ -51,7 +53,8 @@ class ServerImpl {
 
     auto Run(StorageConfig const& storage_config,
              Storage const& storage,
-             ApiBundle const& apis) -> bool;
+             ApiBundle const& apis,
+             std::optional<std::uint8_t> op_exponent) -> bool;
     ~ServerImpl() = default;
 
   private: