From 90f52cd3d011a683f53ac15d31d27825204d03c3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 13:19:07 +0200 Subject: [PATCH 01/11] Initial impl --- include/boost/redis/connection.hpp | 66 ++++------- .../boost/redis/detail/is_cancellation.hpp | 25 ++++ include/boost/redis/detail/writer_fsm.hpp | 108 ++++++++++++++++++ include/boost/redis/impl/exec_fsm.ipp | 8 +- 4 files changed, 157 insertions(+), 50 deletions(-) create mode 100644 include/boost/redis/detail/is_cancellation.hpp create mode 100644 include/boost/redis/detail/writer_fsm.hpp diff --git a/include/boost/redis/connection.hpp b/include/boost/redis/connection.hpp index 93b7e7fd..c1afec39 100644 --- a/include/boost/redis/connection.hpp +++ b/include/boost/redis/connection.hpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -209,52 +210,31 @@ struct connection_impl { template struct writer_op { connection_impl* conn_; - asio::coroutine coro{}; + writer_fsm fsm_; + + explicit writer_op(connection_impl& conn) noexcept + : conn_(&conn) + , fsm_(conn.mpx_, conn.logger_) + { } template - void operator()(Self& self, system::error_code ec = {}, std::size_t n = 0) + void operator()(Self& self, system::error_code ec = {}, std::size_t = {}) { - ignore_unused(n); - - BOOST_ASIO_CORO_REENTER(coro) for (;;) - { - while (conn_->mpx_.prepare_write() != 0) { - BOOST_ASIO_CORO_YIELD - asio::async_write( - conn_->stream_, - asio::buffer(conn_->mpx_.get_write_buffer()), - std::move(self)); - - conn_->logger_.on_write(ec, conn_->mpx_.get_write_buffer().size()); - - if (ec) { - conn_->logger_.trace("writer_op (1)", ec); - conn_->cancel(operation::run); - self.complete(ec); - return; - } - - conn_->mpx_.commit_write(); - - // A socket.close() may have been called while a - // successful write might had already been queued, so we - // have to check here before proceeding. - if (!conn_->is_open()) { - conn_->logger_.trace("writer_op (2): connection is closed."); - self.complete({}); + for (;;) { + auto act = fsm_.resume(ec, self.get_cancellation_state().cancelled()); + + switch (act.type()) { + case writer_action_type::done: self.complete(act.error()); return; + case writer_action_type::write: + asio::async_write( + conn_->stream_, + asio::buffer(conn_->mpx_.get_write_buffer()), + std::move(self)); return; - } - } - - BOOST_ASIO_CORO_YIELD - conn_->writer_timer_.async_wait(std::move(self)); - if (!conn_->is_open()) { - conn_->logger_.trace("writer_op (3): connection is closed."); - // Notice this is not an error of the op, stoping was - // requested from the outside, so we complete with - // success. - self.complete({}); - return; + case writer_action_type::wait: conn_->writer_timer_.async_wait(std::move(self)); return; + case writer_action_type::cancel_run: + conn_->cancel(operation::run); + continue; // This op doesn't need yielding } } } @@ -339,7 +319,7 @@ class run_op { auto writer(CompletionToken&& token) { return asio::async_compose( - writer_op{conn_}, + writer_op{*conn_}, std::forward(token), conn_->writer_timer_); } diff --git a/include/boost/redis/detail/is_cancellation.hpp b/include/boost/redis/detail/is_cancellation.hpp new file mode 100644 index 00000000..7a5d88f2 --- /dev/null +++ b/include/boost/redis/detail/is_cancellation.hpp @@ -0,0 +1,25 @@ +// +// Copyright (c) 2025 Marcelo Zimbres Silva (mzimbres@gmail.com), +// Ruben Perez Hidalgo (rubenperez038 at gmail dot com) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#ifndef BOOST_REDIS_IS_CANCELLATION_HPP +#define BOOST_REDIS_IS_CANCELLATION_HPP + +#include + +namespace boost::redis::detail { + +inline bool is_cancellation(asio::cancellation_type_t type) +{ + return !!( + type & (asio::cancellation_type_t::total | asio::cancellation_type_t::partial | + asio::cancellation_type_t::terminal)); +} + +} // namespace boost::redis::detail + +#endif diff --git a/include/boost/redis/detail/writer_fsm.hpp b/include/boost/redis/detail/writer_fsm.hpp new file mode 100644 index 00000000..e2efec58 --- /dev/null +++ b/include/boost/redis/detail/writer_fsm.hpp @@ -0,0 +1,108 @@ +// +// Copyright (c) 2025 Marcelo Zimbres Silva (mzimbres@gmail.com), +// Ruben Perez Hidalgo (rubenperez038 at gmail dot com) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#ifndef BOOST_REDIS_WRITER_FSM_HPP +#define BOOST_REDIS_WRITER_FSM_HPP + +#include +#include +#include +#include + +#include +#include +#include + +// Sans-io algorithm for the writer task, as a finite state machine + +namespace boost::redis::detail { + +// What should we do next? +enum class writer_action_type +{ + done, // Call the final handler + write, // Issue a write on the stream + wait, // Wait until there is data to be written + cancel_run, // Cancel the connection's run operation +}; + +class writer_action { + writer_action_type type_; + system::error_code ec_; + +public: + writer_action(writer_action_type type) noexcept + : type_{type} + { } + + writer_action(system::error_code ec) noexcept + : type_{writer_action_type::done} + , ec_{ec} + { } + + writer_action_type type() const { return type_; } + system::error_code error() const { return ec_; } +}; + +class writer_fsm { + int resume_point_{0}; + multiplexer* mpx_; + connection_logger* logger_; + system::error_code stored_ec_; + +public: + writer_fsm(multiplexer& mpx, connection_logger& logger) noexcept + : mpx_(&mpx) + , logger_(&logger) + { } + + writer_action resume(system::error_code ec, asio::cancellation_type_t cancel_state) + { + // TODO: move logging + // TODO: move to ipp + + switch (resume_point_) { + BOOST_REDIS_CORO_INITIAL + + while (mpx_->prepare_write() != 0u) { + BOOST_REDIS_YIELD(resume_point_, 1, writer_action_type::write) + logger_->on_write(ec, mpx_->get_write_buffer().size()); + + if (ec) { + logger_->trace("writer_op (1): error: ", ec); + stored_ec_ = ec; + BOOST_REDIS_YIELD(resume_point_, 2, writer_action_type::cancel_run) + return ec; + } + + mpx_->commit_write(); + + // Check for cancellations + if (is_cancellation(cancel_state)) { + logger_->trace("writer_op (2): cancelled."); + return system::error_code(asio::error::operation_aborted); + } + } + + // Wait for a request to be ready + BOOST_REDIS_YIELD(resume_point_, 3, writer_action_type::wait) + + // Check for cancellations + if (is_cancellation(cancel_state)) { + logger_->trace("writer_op (3): cancelled."); + return system::error_code(asio::error::operation_aborted); + } + } + + return system::error_code(); + } +}; + +} // namespace boost::redis::detail + +#endif // BOOST_REDIS_CONNECTOR_HPP diff --git a/include/boost/redis/impl/exec_fsm.ipp b/include/boost/redis/impl/exec_fsm.ipp index 25675e02..b44b704f 100644 --- a/include/boost/redis/impl/exec_fsm.ipp +++ b/include/boost/redis/impl/exec_fsm.ipp @@ -11,6 +11,7 @@ #include #include +#include #include #include @@ -18,13 +19,6 @@ namespace boost::redis::detail { -inline bool is_cancellation(asio::cancellation_type_t type) -{ - return !!( - type & (asio::cancellation_type_t::total | asio::cancellation_type_t::partial | - asio::cancellation_type_t::terminal)); -} - exec_action exec_fsm::resume(bool connection_is_open, asio::cancellation_type_t cancel_state) { switch (resume_point_) { From c5f25e5b29779fc542514b0c0972b6803ea0a78b Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 13:24:15 +0200 Subject: [PATCH 02/11] move to ipp --- include/boost/redis/detail/writer_fsm.hpp | 51 ++--------------- include/boost/redis/impl/writer_fsm.ipp | 69 +++++++++++++++++++++++ include/boost/redis/src.hpp | 1 + 3 files changed, 75 insertions(+), 46 deletions(-) create mode 100644 include/boost/redis/impl/writer_fsm.ipp diff --git a/include/boost/redis/detail/writer_fsm.hpp b/include/boost/redis/detail/writer_fsm.hpp index e2efec58..0f4361ba 100644 --- a/include/boost/redis/detail/writer_fsm.hpp +++ b/include/boost/redis/detail/writer_fsm.hpp @@ -9,19 +9,17 @@ #ifndef BOOST_REDIS_WRITER_FSM_HPP #define BOOST_REDIS_WRITER_FSM_HPP -#include -#include -#include -#include - #include -#include #include // Sans-io algorithm for the writer task, as a finite state machine namespace boost::redis::detail { +// Forward decls +class connection_logger; +class multiplexer; + // What should we do next? enum class writer_action_type { @@ -61,46 +59,7 @@ class writer_fsm { , logger_(&logger) { } - writer_action resume(system::error_code ec, asio::cancellation_type_t cancel_state) - { - // TODO: move logging - // TODO: move to ipp - - switch (resume_point_) { - BOOST_REDIS_CORO_INITIAL - - while (mpx_->prepare_write() != 0u) { - BOOST_REDIS_YIELD(resume_point_, 1, writer_action_type::write) - logger_->on_write(ec, mpx_->get_write_buffer().size()); - - if (ec) { - logger_->trace("writer_op (1): error: ", ec); - stored_ec_ = ec; - BOOST_REDIS_YIELD(resume_point_, 2, writer_action_type::cancel_run) - return ec; - } - - mpx_->commit_write(); - - // Check for cancellations - if (is_cancellation(cancel_state)) { - logger_->trace("writer_op (2): cancelled."); - return system::error_code(asio::error::operation_aborted); - } - } - - // Wait for a request to be ready - BOOST_REDIS_YIELD(resume_point_, 3, writer_action_type::wait) - - // Check for cancellations - if (is_cancellation(cancel_state)) { - logger_->trace("writer_op (3): cancelled."); - return system::error_code(asio::error::operation_aborted); - } - } - - return system::error_code(); - } + writer_action resume(system::error_code ec, asio::cancellation_type_t cancel_state); }; } // namespace boost::redis::detail diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp new file mode 100644 index 00000000..843e30f4 --- /dev/null +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -0,0 +1,69 @@ +// +// Copyright (c) 2025 Marcelo Zimbres Silva (mzimbres@gmail.com), +// Ruben Perez Hidalgo (rubenperez038 at gmail dot com) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#ifndef BOOST_REDIS_WRITER_FSM_IPP +#define BOOST_REDIS_WRITER_FSM_IPP + +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace boost::redis::detail { + +writer_action writer_fsm::resume(system::error_code ec, asio::cancellation_type_t cancel_state) +{ + // TODO: move logging + + switch (resume_point_) { + BOOST_REDIS_CORO_INITIAL + + while (mpx_->prepare_write() != 0u) { + BOOST_REDIS_YIELD(resume_point_, 1, writer_action_type::write) + logger_->on_write(ec, mpx_->get_write_buffer().size()); + + if (ec) { + logger_->trace("writer_op (1): error: ", ec); + stored_ec_ = ec; + BOOST_REDIS_YIELD(resume_point_, 2, writer_action_type::cancel_run) + return ec; + } + + mpx_->commit_write(); + + // Check for cancellations + if (is_cancellation(cancel_state)) { + logger_->trace("writer_op (2): cancelled."); + return system::error_code(asio::error::operation_aborted); + } + } + + // Wait for a request to be ready + BOOST_REDIS_YIELD(resume_point_, 3, writer_action_type::wait) + + // Check for cancellations + if (is_cancellation(cancel_state)) { + logger_->trace("writer_op (3): cancelled."); + return system::error_code(asio::error::operation_aborted); + } + } + + // We should never reach here + BOOST_ASSERT(false); + return system::error_code(); +} + +} // namespace boost::redis::detail + +#endif diff --git a/include/boost/redis/src.hpp b/include/boost/redis/src.hpp index 144b3a4a..26b6cd95 100644 --- a/include/boost/redis/src.hpp +++ b/include/boost/redis/src.hpp @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include From 5b39db06ca33066cc2bf79e246def692d71cceb9 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 13:30:29 +0200 Subject: [PATCH 03/11] missing while true --- include/boost/redis/impl/writer_fsm.ipp | 44 ++++++++++++++----------- 1 file changed, 25 insertions(+), 19 deletions(-) diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp index 843e30f4..ded54c9b 100644 --- a/include/boost/redis/impl/writer_fsm.ipp +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -29,34 +29,40 @@ writer_action writer_fsm::resume(system::error_code ec, asio::cancellation_type_ switch (resume_point_) { BOOST_REDIS_CORO_INITIAL - while (mpx_->prepare_write() != 0u) { - BOOST_REDIS_YIELD(resume_point_, 1, writer_action_type::write) - logger_->on_write(ec, mpx_->get_write_buffer().size()); + for (;;) { + // Attempt to write while we have requests ready to send + while (mpx_->prepare_write() != 0u) { + // Write + BOOST_REDIS_YIELD(resume_point_, 1, writer_action_type::write) + logger_->on_write(ec, mpx_->get_write_buffer().size()); - if (ec) { - logger_->trace("writer_op (1): error: ", ec); - stored_ec_ = ec; - BOOST_REDIS_YIELD(resume_point_, 2, writer_action_type::cancel_run) - return ec; + // A failed write means that we should tear down the connection + if (ec) { + logger_->trace("writer_op (1): error: ", ec); + stored_ec_ = ec; + BOOST_REDIS_YIELD(resume_point_, 2, writer_action_type::cancel_run) + return ec; + } + + // Mark requests as written + mpx_->commit_write(); + + // Check for cancellations + if (is_cancellation(cancel_state)) { + logger_->trace("writer_op (2): cancelled."); + return system::error_code(asio::error::operation_aborted); + } } - mpx_->commit_write(); + // No more requests ready to be written. Wait for more + BOOST_REDIS_YIELD(resume_point_, 3, writer_action_type::wait) // Check for cancellations if (is_cancellation(cancel_state)) { - logger_->trace("writer_op (2): cancelled."); + logger_->trace("writer_op (3): cancelled."); return system::error_code(asio::error::operation_aborted); } } - - // Wait for a request to be ready - BOOST_REDIS_YIELD(resume_point_, 3, writer_action_type::wait) - - // Check for cancellations - if (is_cancellation(cancel_state)) { - logger_->trace("writer_op (3): cancelled."); - return system::error_code(asio::error::operation_aborted); - } } // We should never reach here From 0a6c452d298fe238f48216416ca99fc5167159a0 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 13:46:04 +0200 Subject: [PATCH 04/11] Make test more reliable --- test/test_conn_exec.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_conn_exec.cpp b/test/test_conn_exec.cpp index e02c3bce..657b948a 100644 --- a/test/test_conn_exec.cpp +++ b/test/test_conn_exec.cpp @@ -150,7 +150,7 @@ BOOST_AUTO_TEST_CASE(correct_database) auto conn = std::make_shared(ioc); request req; - req.push("CLIENT", "LIST"); + req.push("CLIENT", "INFO"); generic_response resp; From 6951ca8afdc5f31464cfd683a03d433e41d924e4 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 14:03:32 +0200 Subject: [PATCH 05/11] Move logging --- .../boost/redis/detail/connection_logger.hpp | 10 +++++++- .../boost/redis/impl/connection_logger.ipp | 17 +------------- include/boost/redis/impl/writer_fsm.ipp | 23 +++++++++++++------ 3 files changed, 26 insertions(+), 24 deletions(-) diff --git a/include/boost/redis/detail/connection_logger.hpp b/include/boost/redis/detail/connection_logger.hpp index 34d6a4b7..04b78288 100644 --- a/include/boost/redis/detail/connection_logger.hpp +++ b/include/boost/redis/detail/connection_logger.hpp @@ -37,7 +37,6 @@ class connection_logger { void on_connect(system::error_code const& ec, asio::ip::tcp::endpoint const& ep); void on_connect(system::error_code const& ec, std::string_view unix_socket_ep); void on_ssl_handshake(system::error_code const& ec); - void on_write(system::error_code const& ec, std::size_t n); void on_fsm_resume(reader_fsm::action const& action); void on_hello(system::error_code const& ec, generic_response const& resp); void log(logger::level lvl, std::string_view msg); @@ -47,6 +46,15 @@ class connection_logger { { log(logger::level::debug, op, ec); } + + template + void log(logger::level lvl, Fn fn) + { + if (logger_.lvl < lvl) + return; + fn(msg_); + logger_.fn(lvl, msg_); + } }; } // namespace boost::redis::detail diff --git a/include/boost/redis/impl/connection_logger.ipp b/include/boost/redis/impl/connection_logger.ipp index 54f8b33f..d285c69a 100644 --- a/include/boost/redis/impl/connection_logger.ipp +++ b/include/boost/redis/impl/connection_logger.ipp @@ -5,6 +5,7 @@ */ #include +#include #include #include @@ -141,22 +142,6 @@ void connection_logger::on_ssl_handshake(system::error_code const& ec) logger_.fn(logger::level::info, msg_); } -void connection_logger::on_write(system::error_code const& ec, std::size_t n) -{ - if (logger_.lvl < logger::level::info) - return; - - msg_ = "writer_op: "; - if (ec) { - format_error_code(ec, msg_); - } else { - msg_ += std::to_string(n); - msg_ += " bytes written."; - } - - logger_.fn(logger::level::info, msg_); -} - void connection_logger::on_fsm_resume(reader_fsm::action const& action) { if (logger_.lvl < logger::level::debug) diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp index ded54c9b..cd61aca4 100644 --- a/include/boost/redis/impl/writer_fsm.ipp +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -22,10 +22,17 @@ namespace boost::redis::detail { -writer_action writer_fsm::resume(system::error_code ec, asio::cancellation_type_t cancel_state) +inline void log_write_success(connection_logger& logger, std::size_t bytes_written) { - // TODO: move logging + logger.log(logger::level::info, [bytes_written](std::string& buff) { + buff = "Writer task: "; + buff += std::to_string(bytes_written); + buff += " bytes written."; + }); +} +writer_action writer_fsm::resume(system::error_code ec, asio::cancellation_type_t cancel_state) +{ switch (resume_point_) { BOOST_REDIS_CORO_INITIAL @@ -34,22 +41,24 @@ writer_action writer_fsm::resume(system::error_code ec, asio::cancellation_type_ while (mpx_->prepare_write() != 0u) { // Write BOOST_REDIS_YIELD(resume_point_, 1, writer_action_type::write) - logger_->on_write(ec, mpx_->get_write_buffer().size()); // A failed write means that we should tear down the connection if (ec) { - logger_->trace("writer_op (1): error: ", ec); + logger_->log(logger::level::err, "Writer task error: ", ec); stored_ec_ = ec; BOOST_REDIS_YIELD(resume_point_, 2, writer_action_type::cancel_run) - return ec; + return stored_ec_; } + // Log what we wrote + log_write_success(*logger_, mpx_->get_write_buffer().size()); + // Mark requests as written mpx_->commit_write(); // Check for cancellations if (is_cancellation(cancel_state)) { - logger_->trace("writer_op (2): cancelled."); + logger_->trace("Writer task cancelled (1)."); return system::error_code(asio::error::operation_aborted); } } @@ -59,7 +68,7 @@ writer_action writer_fsm::resume(system::error_code ec, asio::cancellation_type_ // Check for cancellations if (is_cancellation(cancel_state)) { - logger_->trace("writer_op (3): cancelled."); + logger_->trace("Writer task cancelled (2)."); return system::error_code(asio::error::operation_aborted); } } From 7dba877adbc05c165470d04433eb4ed57e630f93 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 14:04:25 +0200 Subject: [PATCH 06/11] move is_cancellation --- include/boost/redis/impl/exec_fsm.ipp | 2 +- include/boost/redis/{detail => impl}/is_cancellation.hpp | 0 include/boost/redis/impl/writer_fsm.ipp | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename include/boost/redis/{detail => impl}/is_cancellation.hpp (100%) diff --git a/include/boost/redis/impl/exec_fsm.ipp b/include/boost/redis/impl/exec_fsm.ipp index b44b704f..fdea6804 100644 --- a/include/boost/redis/impl/exec_fsm.ipp +++ b/include/boost/redis/impl/exec_fsm.ipp @@ -11,7 +11,7 @@ #include #include -#include +#include #include #include diff --git a/include/boost/redis/detail/is_cancellation.hpp b/include/boost/redis/impl/is_cancellation.hpp similarity index 100% rename from include/boost/redis/detail/is_cancellation.hpp rename to include/boost/redis/impl/is_cancellation.hpp diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp index cd61aca4..1a6d660e 100644 --- a/include/boost/redis/impl/writer_fsm.ipp +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -11,9 +11,9 @@ #include #include -#include #include #include +#include #include #include From eadb4faa7013c685030da66614bf650f471fbe8e Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 18:28:42 +0200 Subject: [PATCH 07/11] rename log to log_fn --- include/boost/redis/detail/connection_logger.hpp | 2 +- include/boost/redis/impl/writer_fsm.ipp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/boost/redis/detail/connection_logger.hpp b/include/boost/redis/detail/connection_logger.hpp index 04b78288..73a02bec 100644 --- a/include/boost/redis/detail/connection_logger.hpp +++ b/include/boost/redis/detail/connection_logger.hpp @@ -48,7 +48,7 @@ class connection_logger { } template - void log(logger::level lvl, Fn fn) + void log_fn(logger::level lvl, Fn fn) { if (logger_.lvl < lvl) return; diff --git a/include/boost/redis/impl/writer_fsm.ipp b/include/boost/redis/impl/writer_fsm.ipp index 1a6d660e..0a098f5a 100644 --- a/include/boost/redis/impl/writer_fsm.ipp +++ b/include/boost/redis/impl/writer_fsm.ipp @@ -24,7 +24,7 @@ namespace boost::redis::detail { inline void log_write_success(connection_logger& logger, std::size_t bytes_written) { - logger.log(logger::level::info, [bytes_written](std::string& buff) { + logger.log_fn(logger::level::info, [bytes_written](std::string& buff) { buff = "Writer task: "; buff += std::to_string(bytes_written); buff += " bytes written."; From e3a756d0ee0a14ed8a3c18b7fa4121c0959bfaed Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 18:28:58 +0200 Subject: [PATCH 08/11] test_connection_logger 1 --- test/CMakeLists.txt | 1 + test/Jamfile | 1 + test/test_connection_logger.cpp | 95 +++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+) create mode 100644 test/test_connection_logger.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 24cf9410..b17c4c68 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -41,6 +41,7 @@ make_test(test_exec_fsm) make_test(test_log_to_file) make_test(test_conn_logging) make_test(test_reader_fsm) +make_test(test_connection_logger) # Tests that require a real Redis server make_test(test_conn_quit) diff --git a/test/Jamfile b/test/Jamfile index f5425573..ded74dae 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -57,6 +57,7 @@ local tests = test_log_to_file test_conn_logging test_reader_fsm + test_connection_logger ; # Build and run the tests diff --git a/test/test_connection_logger.cpp b/test/test_connection_logger.cpp new file mode 100644 index 00000000..75b4bb21 --- /dev/null +++ b/test/test_connection_logger.cpp @@ -0,0 +1,95 @@ +// +// Copyright (c) 2025 Marcelo Zimbres Silva (mzimbres@gmail.com), +// Ruben Perez Hidalgo (rubenperez038 at gmail dot com) +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +// + +#include + +#include + +#include "boost/redis/logger.hpp" + +#include +#include +#include +#include + +using namespace boost::redis; +using detail::connection_logger; + +namespace boost::redis { + +// Printing +std::ostream& operator<<(std::ostream& os, logger::level lvl) +{ + switch (lvl) { + case logger::level::disabled: return os << "logger::level::disabled"; + case logger::level::emerg: return os << "logger::level::emerg"; + case logger::level::alert: return os << "logger::level::alert"; + case logger::level::crit: return os << "logger::level::crit"; + case logger::level::err: return os << "logger::level::err"; + case logger::level::warning: return os << "logger::level::warning"; + case logger::level::notice: return os << "logger::level::notice"; + case logger::level::info: return os << "logger::level::info"; + case logger::level::debug: + return os << "logger::level::debug"; + return os << ""; + } +} + +} // namespace boost::redis + +namespace { + +// Mock logger that records the last issued message and +// the number of issued messages +struct fixture { + std::size_t num_msgs{}; + logger::level msg_level{}; + std::string msg; + connection_logger logger; + + explicit fixture(logger::level lvl) + : logger({lvl, [this](logger::level lvl, std::string_view msg) { + ++this->num_msgs; + this->msg_level = lvl; + this->msg = msg; + }}) + { } +}; + +// log with only a message +void test_log_message() +{ + // Setup + fixture fix{logger::level::warning}; + + // Issuing a message with level > the one configured logs it + fix.logger.log(logger::level::alert, "some message"); + BOOST_TEST_EQ(fix.num_msgs, 1u); + BOOST_TEST_EQ(fix.msg_level, logger::level::alert); + BOOST_TEST_EQ(fix.msg, "some message"); + + // Issuing a message with level == the one configured logs it. + // Internal buffers are cleared + fix.logger.log(logger::level::warning, "other thing"); + BOOST_TEST_EQ(fix.num_msgs, 2u); + BOOST_TEST_EQ(fix.msg_level, logger::level::warning); + BOOST_TEST_EQ(fix.msg, "other thing"); + + // Issuing a message with level < the one configured does not log it. + fix.logger.log(logger::level::info, "bad"); + BOOST_TEST_EQ(fix.num_msgs, 2u); +} + +} // namespace + +int main() +{ + test_log_message(); + + return boost::report_errors(); +} From 5d053207f015c52042535ba968912635e43ecd13 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 18:35:19 +0200 Subject: [PATCH 09/11] log with error --- test/test_connection_logger.cpp | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/test/test_connection_logger.cpp b/test/test_connection_logger.cpp index 75b4bb21..0717b6ac 100644 --- a/test/test_connection_logger.cpp +++ b/test/test_connection_logger.cpp @@ -7,11 +7,12 @@ // #include +#include +#include +#include #include -#include "boost/redis/logger.hpp" - #include #include #include @@ -19,6 +20,7 @@ using namespace boost::redis; using detail::connection_logger; +using boost::system::error_code; namespace boost::redis { @@ -85,11 +87,38 @@ void test_log_message() BOOST_TEST_EQ(fix.num_msgs, 2u); } +// log with a message and an error code +void test_log_message_errorcode() +{ + // Setup + fixture fix{logger::level::warning}; + + // Issuing a message with level > the one configured logs it + fix.logger.log(logger::level::alert, "Some message", error::connect_timeout); + BOOST_TEST_EQ(fix.num_msgs, 1u); + BOOST_TEST_EQ(fix.msg_level, logger::level::alert); + BOOST_TEST_EQ(fix.msg, "Some message: Connect timeout. [boost.redis:18]"); + + // Issuing a message with level == the one configured logs it. + // Internal buffers are cleared. + // Source code info is not printed + constexpr auto loc = BOOST_CURRENT_LOCATION; + fix.logger.log(logger::level::warning, "Other thing", error_code(error::empty_field, &loc)); + BOOST_TEST_EQ(fix.num_msgs, 2u); + BOOST_TEST_EQ(fix.msg_level, logger::level::warning); + BOOST_TEST_EQ(fix.msg, "Other thing: Expected field value is empty. [boost.redis:5]"); + + // Issuing a message with level < the one configured does not log it. + fix.logger.log(logger::level::info, "bad", error::expects_resp3_map); + BOOST_TEST_EQ(fix.num_msgs, 2u); +} + } // namespace int main() { test_log_message(); + test_log_message_errorcode(); return boost::report_errors(); } From 2f6d7b554d13348c24495cc80a38a9912b91b121 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 18:38:03 +0200 Subject: [PATCH 10/11] Finished test_connection_logger --- test/test_connection_logger.cpp | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/test_connection_logger.cpp b/test/test_connection_logger.cpp index 0717b6ac..a69ad5a5 100644 --- a/test/test_connection_logger.cpp +++ b/test/test_connection_logger.cpp @@ -113,12 +113,43 @@ void test_log_message_errorcode() BOOST_TEST_EQ(fix.num_msgs, 2u); } +// log with a function +void test_log_fn() +{ + // Setup + fixture fix{logger::level::warning}; + + // Issuing a message with level > the one configured logs it + fix.logger.log_fn(logger::level::alert, [](std::string& buff) { + buff = "Some message"; + }); + BOOST_TEST_EQ(fix.num_msgs, 1u); + BOOST_TEST_EQ(fix.msg_level, logger::level::alert); + BOOST_TEST_EQ(fix.msg, "Some message"); + + // Issuing a message with level == the one configured logs it. + // Internal buffers are cleared. + fix.logger.log_fn(logger::level::warning, [](std::string& buff) { + buff = "This is another message."; + }); + BOOST_TEST_EQ(fix.num_msgs, 2u); + BOOST_TEST_EQ(fix.msg_level, logger::level::warning); + BOOST_TEST_EQ(fix.msg, "This is another message."); + + // Issuing a message with level < the one configured does not log it. + fix.logger.log_fn(logger::level::info, [](std::string& buff) { + buff = "This message should not be logged."; + }); + BOOST_TEST_EQ(fix.num_msgs, 2u); +} + } // namespace int main() { test_log_message(); test_log_message_errorcode(); + test_log_fn(); return boost::report_errors(); } From ea2fbca206c6b7b86f98ceba314d14367cf7b532 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Sat, 26 Jul 2025 18:38:47 +0200 Subject: [PATCH 11/11] fix printing --- test/test_connection_logger.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/test_connection_logger.cpp b/test/test_connection_logger.cpp index a69ad5a5..0bc9d3b9 100644 --- a/test/test_connection_logger.cpp +++ b/test/test_connection_logger.cpp @@ -36,9 +36,8 @@ std::ostream& operator<<(std::ostream& os, logger::level lvl) case logger::level::warning: return os << "logger::level::warning"; case logger::level::notice: return os << "logger::level::notice"; case logger::level::info: return os << "logger::level::info"; - case logger::level::debug: - return os << "logger::level::debug"; - return os << ""; + case logger::level::debug: return os << "logger::level::debug"; + default: return os << ""; } }