Remove ExecutionServiceImpl::StoreActionResult method

...and move the storing logic to Execute directly. There is no need to pass additional parameters to the method to just perform a check inside, and after this removal there is no need to preserve a one-line method.

2 files changed, 11 insertions, 28 deletions

diff --git a/src/buildtool/execution_api/execution_service/execution_server.cpp b/src/buildtool/execution_api/execution_service/execution_server.cpp
index e0685315..9e34d321 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.cpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.cpp
@@ -288,22 +288,6 @@ auto ExecutionServiceImpl::GetResponse(
     return std::move(response);
 }
 
-auto ExecutionServiceImpl::StoreActionResult(
-    ::bazel_re::ExecuteRequest const* request,
-    IExecutionResponse::Ptr const& i_execution_response,
-    ::bazel_re::ExecuteResponse const& execute_response,
-    ::bazel_re::Action const& action) const noexcept
-    -> expected<std::monostate, std::string> {
-    if (i_execution_response->ExitCode() == 0 and not action.do_not_cache() and
-        not storage_.ActionCache().StoreResult(request->action_digest(),
-                                               execute_response.result())) {
-        auto str = fmt::format("Could not store action result for action {}",
-                               request->action_digest().hash());
-        logger_.Emit(LogLevel::Error, "{}", str);
-        return unexpected{std::move(str)};
-    }
-    return std::monostate{};
-}
 void ExecutionServiceImpl::WriteResponse(
     ::bazel_re::ExecuteResponse const& execute_response,
     ::grpc::ServerWriter<::google::longrunning::Operation>* writer,
@@ -365,12 +349,18 @@ auto ExecutionServiceImpl::Execute(
                               std::move(execute_response).error()};
     }
 
-    auto store_result = StoreActionResult(
-        request, i_execution_response, *execute_response, *action);
-    if (not store_result) {
-        return ::grpc::Status{grpc::StatusCode::INTERNAL,
-                              std::move(store_result).error()};
+    if (i_execution_response->ExitCode() == 0 and not action->do_not_cache()) {
+        if (not storage_.ActionCache().StoreResult(
+                request->action_digest(), execute_response->result())) {
+            auto const str =
+                fmt::format("Could not store action result for action {}",
+                            request->action_digest().hash());
+
+            logger_.Emit(LogLevel::Error, str);
+            return ::grpc::Status{grpc::StatusCode::INTERNAL, str};
+        }
     }
+
     WriteResponse(*execute_response, writer, &op);
     return ::grpc::Status::OK;
 }
diff --git a/src/buildtool/execution_api/execution_service/execution_server.hpp b/src/buildtool/execution_api/execution_service/execution_server.hpp
index 9ffaa9a7..f1337604 100644
--- a/src/buildtool/execution_api/execution_service/execution_server.hpp
+++ b/src/buildtool/execution_api/execution_service/execution_server.hpp
@@ -154,13 +154,6 @@ class ExecutionServiceImpl final : public bazel_re::Execution::Service {
         IExecutionResponse::Ptr const& i_execution_response) const noexcept
         -> expected<::bazel_re::ExecuteResponse, std::string>;
 
-    [[nodiscard]] auto StoreActionResult(
-        ::bazel_re::ExecuteRequest const* request,
-        IExecutionResponse::Ptr const& i_execution_response,
-        ::bazel_re::ExecuteResponse const& execute_response,
-        ::bazel_re::Action const& action) const noexcept
-        -> expected<std::monostate, std::string>;
-
     void WriteResponse(
         ::bazel_re::ExecuteResponse const& execute_response,
         ::grpc::ServerWriter<::google::longrunning::Operation>* writer,