Fix review issues

src/app/ClioApplication.cpp

@@ -148,7 +148,7 @@ ClioApplication::run(bool const useNgWebServer)
             LOG(util::LogService::error()) << "Error creating admin verifier: " << expectedAdminVerifier.error();
             return EXIT_FAILURE;
         }
-        auto adminVerifier = std::move(expectedAdminVerifier).value();
+        auto const adminVerifier = std::move(expectedAdminVerifier).value();
 
         auto httpServer = web::ng::make_Server(config_, ioc);
 

src/feed/Types.hpp

@@ -24,6 +24,7 @@
 #include <memory>
 
 namespace feed {
+
 using Subscriber = web::SubscriptionContextInterface;
 using SubscriberPtr = Subscriber*;
 using SubscriberSharedPtr = std::shared_ptr<Subscriber>;

src/rpc/handlers/Subscribe.cpp

@@ -22,6 +22,7 @@
 #include "data/BackendInterface.hpp"
 #include "data/Types.hpp"
 #include "feed/SubscriptionManagerInterface.hpp"
+#include "feed/Types.hpp"
 #include "rpc/Errors.hpp"
 #include "rpc/JS.hpp"
 #include "rpc/RPCHelpers.hpp"
@@ -30,7 +31,6 @@
 #include "rpc/common/Specs.hpp"
 #include "rpc/common/Types.hpp"
 #include "rpc/common/Validators.hpp"
-#include "web/SubscriptionContextInterface.hpp"
 
 #include <boost/asio/spawn.hpp>
 #include <boost/json/array.hpp>
@@ -139,7 +139,7 @@ boost::json::object
 SubscribeHandler::subscribeToStreams(
     boost::asio::yield_context yield,
     std::vector<std::string> const& streams,
-    web::SubscriptionContextPtr const& session
+    feed::SubscriberSharedPtr const& session
 ) const
 {
     auto response = boost::json::object{};
@@ -166,7 +166,7 @@ SubscribeHandler::subscribeToStreams(
 void
 SubscribeHandler::subscribeToAccountsProposed(
     std::vector<std::string> const& accounts,
-    web::SubscriptionContextPtr const& session
+    feed::SubscriberSharedPtr const& session
 ) const
 {
     for (auto const& account : accounts) {
@@ -178,7 +178,7 @@ SubscribeHandler::subscribeToAccountsProposed(
 void
 SubscribeHandler::subscribeToAccounts(
     std::vector<std::string> const& accounts,
-    web::SubscriptionContextPtr const& session
+    feed::SubscriberSharedPtr const& session
 ) const
 {
     for (auto const& account : accounts) {
@@ -190,7 +190,7 @@ SubscribeHandler::subscribeToAccounts(
 void
 SubscribeHandler::subscribeToBooks(
     std::vector<OrderBook> const& books,
-    web::SubscriptionContextPtr const& session,
+    feed::SubscriberSharedPtr const& session,
     boost::asio::yield_context yield,
     Output& output
 ) const

src/rpc/handlers/Subscribe.hpp

@@ -21,9 +21,9 @@
 
 #include "data/BackendInterface.hpp"
 #include "feed/SubscriptionManagerInterface.hpp"
+#include "feed/Types.hpp"
 #include "rpc/common/Specs.hpp"
 #include "rpc/common/Types.hpp"
-#include "web/SubscriptionContextInterface.hpp"
 
 #include <boost/asio/spawn.hpp>
 #include <boost/json/array.hpp>
@@ -129,20 +129,20 @@ class SubscribeHandler {
     subscribeToStreams(
         boost::asio::yield_context yield,
         std::vector<std::string> const& streams,
-        web::SubscriptionContextPtr const& session
+        feed::SubscriberSharedPtr const& session
     ) const;
 
     void
-    subscribeToAccounts(std::vector<std::string> const& accounts, web::SubscriptionContextPtr const& session) const;
+    subscribeToAccounts(std::vector<std::string> const& accounts, feed::SubscriberSharedPtr const& session) const;
 
     void
-    subscribeToAccountsProposed(std::vector<std::string> const& accounts, web::SubscriptionContextPtr const& session)
+    subscribeToAccountsProposed(std::vector<std::string> const& accounts, feed::SubscriberSharedPtr const& session)
         const;
 
     void
     subscribeToBooks(
         std::vector<OrderBook> const& books,
-        web::SubscriptionContextPtr const& session,
+        feed::SubscriberSharedPtr const& session,
         boost::asio::yield_context yield,
         Output& output
     ) const;

src/util/CoroutineGroup.cpp

@@ -25,14 +25,13 @@
 #include <boost/asio/steady_timer.hpp>
 
 #include <cstddef>
-#include <cstdint>
 #include <functional>
 #include <optional>
 #include <utility>
 
 namespace util {
 
-CoroutineGroup::CoroutineGroup(boost::asio::yield_context yield, std::optional<int64_t> maxChildren)
+CoroutineGroup::CoroutineGroup(boost::asio::yield_context yield, std::optional<size_t> maxChildren)
     : timer_{yield.get_executor(), boost::asio::steady_timer::duration::max()}, maxChildren_{maxChildren}
 {
 }
@@ -91,6 +90,8 @@ CoroutineGroup::isFull() const
 void
 CoroutineGroup::onCoroutineCompleted()
 {
+    ASSERT(childrenCounter_ != 0, "onCoroutineCompleted() called more times than the number of child coroutines");
+
     --childrenCounter_;
     if (childrenCounter_ == 0)
         timer_.cancel();

src/util/CoroutineGroup.hpp

@@ -24,7 +24,6 @@
 
 #include <atomic>
 #include <cstddef>
-#include <cstdint>
 #include <functional>
 #include <optional>
 
@@ -37,8 +36,8 @@ namespace util {
  */
 class CoroutineGroup {
     boost::asio::steady_timer timer_;
-    std::optional<int64_t> maxChildren_;
-    std::atomic_int64_t childrenCounter_{0};
+    std::optional<size_t> maxChildren_;
+    std::atomic_size_t childrenCounter_{0};
 
 public:
     /**
@@ -48,7 +47,7 @@ class CoroutineGroup {
      * @param maxChildren The maximum number of coroutines that can be spawned at the same time. If not provided, there
      * is no limit
      */
-    CoroutineGroup(boost::asio::yield_context yield, std::optional<int64_t> maxChildren = std::nullopt);
+    CoroutineGroup(boost::asio::yield_context yield, std::optional<size_t> maxChildren = std::nullopt);
 
     /**
      * @brief Destroy the Coroutine Group object

src/web/AdminVerificationStrategy.cpp

@@ -89,7 +89,7 @@ make_AdminVerificationStrategy(util::Config const& config)
     if (adminPassword.has_value() and localAdmin.has_value() and *localAdmin)
         return std::unexpected{"Admin config error: 'local_admin' and admin_password can not be set together."};
 
-    if (localAdmin.has_value() and not*localAdmin and not adminPassword.has_value()) {
+    if (localAdmin.has_value() and !*localAdmin and !adminPassword.has_value()) {
         return std::unexpected{
             "Admin config error: either 'local_admin' should be enabled or 'admin_password' must be specified."
         };

src/web/RPCServerHandler.hpp

@@ -161,7 +161,7 @@ class RPCServerHandler {
                     return rpc::make_WsContext(
                         yield,
                         request,
-                        connection->subscriptionContext(tagFactory_),
+                        connection->makeSubscriptionContext(tagFactory_),
                         tagFactory_.with(connection->tag()),
                         *range,
                         connection->clientIp,

src/web/impl/HttpBase.hpp

@@ -278,7 +278,7 @@ class HttpBase : public ConnectionBase {
     }
 
     SubscriptionContextPtr
-    subscriptionContext(util::TagDecoratorFactory const&) override
+    makeSubscriptionContext(util::TagDecoratorFactory const&) override
     {
         ASSERT(false, "SubscriptionContext can't be created for a HTTP connection");
         std::unreachable();

src/web/impl/WsBase.hpp

@@ -194,7 +194,7 @@ class WsBase : public ConnectionBase, public std::enable_shared_from_this<WsBase
      * @return The subscription context for this connection.
      */
     SubscriptionContextPtr
-    subscriptionContext(util::TagDecoratorFactory const& factory) override
+    makeSubscriptionContext(util::TagDecoratorFactory const& factory) override
     {
         if (subscriptionContext_ == nullptr) {
             subscriptionContext_ = std::make_shared<SubscriptionContext>(factory, shared_from_this());

src/web/interface/ConnectionBase.hpp

@@ -89,7 +89,7 @@ struct ConnectionBase : public util::Taggable {
      * @return The subscription context for this connection.
      */
     virtual SubscriptionContextPtr
-    subscriptionContext(util::TagDecoratorFactory const& factory) = 0;
+    makeSubscriptionContext(util::TagDecoratorFactory const& factory) = 0;
 
     /**
      * @brief Indicates whether the connection had an error and is considered dead.

src/web/ng/Response.cpp

@@ -122,7 +122,7 @@ Response::intoHttpResponse() &&
 }
 
 boost::asio::const_buffer
-Response::intoWsResponse() const&
+Response::asWsResponse() const&
 {
     ASSERT(std::holds_alternative<std::string>(data_), "Response must contain WebSocket data");
     auto const& message = std::get<std::string>(data_);

src/web/ng/Response.hpp

@@ -29,6 +29,7 @@
 
 #include <string>
 #include <variant>
+
 namespace web::ng {
 
 /**
@@ -91,7 +92,7 @@ class Response {
      * @return The message of the response as a const buffer.
      */
     boost::asio::const_buffer
-    intoWsResponse() const&;
+    asWsResponse() const&;
 };
 
 }  // namespace web::ng

src/web/ng/impl/ErrorHandling.cpp

@@ -105,6 +105,7 @@ ErrorHelper::makeError(rpc::Status const& err) const
             case rpc::ClioError::rpcINVALID_HOT_WALLET:
             case rpc::ClioError::rpcFIELD_NOT_FOUND_TRANSACTION:
             case rpc::ClioError::rpcMALFORMED_ORACLE_DOCUMENT_ID:
+            case rpc::ClioError::rpcMALFORMED_AUTHORIZED_CREDENTIALS:
             case rpc::ClioError::etlCONNECTION_ERROR:
             case rpc::ClioError::etlREQUEST_ERROR:
             case rpc::ClioError::etlREQUEST_TIMEOUT:

src/web/ng/impl/WsConnection.hpp

@@ -145,7 +145,7 @@ class WsConnection : public WsConnectionBase {
         std::chrono::steady_clock::duration timeout = DEFAULT_TIMEOUT
     ) override
     {
-        return sendBuffer(response.intoWsResponse(), yield, timeout);
+        return sendBuffer(response.asWsResponse(), yield, timeout);
     }
 
     std::expected<Request, Error>

tests/common/web/interface/ConnectionBaseMock.hpp

@@ -36,7 +36,7 @@ struct ConnectionBaseMock : web::ConnectionBase {
     MOCK_METHOD(void, send, (std::shared_ptr<std::string>), (override));
     MOCK_METHOD(
         web::SubscriptionContextPtr,
-        subscriptionContext,
+        makeSubscriptionContext,
         (util::TagDecoratorFactory const& factory),
         (override)
     );

tests/unit/web/RPCServerHandlerTests.cpp

@@ -63,7 +63,7 @@ struct MockWsBase : public web::ConnectionBase {
     }
 
     SubscriptionContextPtr
-    subscriptionContext(util::TagDecoratorFactory const&) override
+    makeSubscriptionContext(util::TagDecoratorFactory const&) override
     {
         return {};
     }

tests/unit/web/ng/ResponseTests.cpp

@@ -50,7 +50,7 @@ TEST_F(ResponseDeathTest, asConstBufferWithHttpData)
 {
     Request const request{http::request<http::string_body>{http::verb::get, "/", 11}};
     web::ng::Response const response{boost::beast::http::status::ok, "message", request};
-    EXPECT_DEATH(response.intoWsResponse(), "");
+    EXPECT_DEATH(response.asWsResponse(), "");
 }
 
 struct ResponseTest : testing::Test {
@@ -104,7 +104,7 @@ TEST_F(ResponseTest, asConstBuffer)
     std::string const responseMessage = "response message";
     web::ng::Response const response{responseStatus_, responseMessage, request};
 
-    auto const buffer = response.intoWsResponse();
+    auto const buffer = response.asWsResponse();
     EXPECT_EQ(buffer.size(), responseMessage.size());
 
     std::string const messageFromBuffer{static_cast<char const*>(buffer.data()), buffer.size()};
@@ -117,7 +117,7 @@ TEST_F(ResponseTest, asConstBufferJson)
     boost::json::object const responseMessage{{"key", "value"}};
     web::ng::Response const response{responseStatus_, responseMessage, request};
 
-    auto const buffer = response.intoWsResponse();
+    auto const buffer = response.asWsResponse();
     EXPECT_EQ(buffer.size(), boost::json::serialize(responseMessage).size());
 
     std::string const messageFromBuffer{static_cast<char const*>(buffer.data()), buffer.size()};