From df68fb023516fd5885af698ebf8042c73b07f076 Mon Sep 17 00:00:00 2001 From: Marcelo Zimbres Date: Mon, 31 Oct 2022 22:09:10 +0100 Subject: [PATCH] Changes: - Ports from boost::container::pmr to std::pmr. - Fixes clang-tidy issues. - Adds resp3::request unit-tests. --- .clang-tidy | 4 ++ CMakeLists.txt | 48 +++++---------- README.md | 8 ++- examples/serialization.cpp | 60 ++++++++++--------- examples/subscriber.cpp | 4 +- include/aedis/detail/connection_base.hpp | 12 ++-- include/aedis/detail/connection_ops.hpp | 2 +- include/aedis/resp3/detail/parser.hpp | 2 +- include/aedis/resp3/node.hpp | 8 +-- include/aedis/resp3/request.hpp | 22 +++---- tests/conn_connect.cpp | 2 +- tests/{conn_request.cpp => conn_exec.cpp} | 11 +--- ...n_cancel_exec.cpp => conn_exec_cancel.cpp} | 0 ...onn_cancel_run.cpp => conn_run_cancel.cpp} | 0 tests/request.cpp | 57 ++++++++++++++++++ 15 files changed, 140 insertions(+), 100 deletions(-) rename tests/{conn_request.cpp => conn_exec.cpp} (83%) rename tests/{conn_cancel_exec.cpp => conn_exec_cancel.cpp} (100%) rename tests/{conn_cancel_run.cpp => conn_run_cancel.cpp} (100%) create mode 100644 tests/request.cpp diff --git a/.clang-tidy b/.clang-tidy index 5a0253ca..f96d3bd4 100644 --- a/.clang-tidy +++ b/.clang-tidy @@ -9,6 +9,7 @@ Checks: "*,\ -google-readability-braces-around-statements,\ -hicpp-braces-around-statements,\ -hicpp-named-parameter,\ + -hicpp-avoid-goto,\ -google-build-using-namespace,\ -altera-*,\ -fuchsia-*,\ @@ -24,6 +25,9 @@ Checks: "*,\ -cppcoreguidelines-pro-bounds-pointer-arithmetic,\ -cppcoreguidelines-avoid-magic-numbers,\ -cppcoreguidelines-pro-bounds-constant-array-index,\ + -cppcoreguidelines-interfaces-global-init,\ + -cppcoreguidelines-macro-usage,\ + -cppcoreguidelines-avoid-goto,\ -cppcoreguidelines-non-private-member-variables-in-classes" WarningsAsErrors: '' CheckOptions: diff --git a/CMakeLists.txt b/CMakeLists.txt index 80d322e2..4ebd9923 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -26,7 +26,6 @@ target_link_libraries(aedis Boost::asio Boost::assert Boost::config - Boost::container Boost::core Boost::mp11 Boost::optional @@ -43,7 +42,7 @@ write_basic_package_version_file( COMPATIBILITY AnyNewerVersion ) -find_package(Boost 1.79 REQUIRED container) +find_package(Boost 1.79 REQUIRED) include_directories(${Boost_INCLUDE_DIRS}) find_package(OpenSSL REQUIRED) @@ -67,18 +66,17 @@ add_executable(serialization examples/serialization.cpp) add_executable(subscriber examples/subscriber.cpp) add_executable(subscriber_sentinel examples/subscriber_sentinel.cpp) add_executable(test_conn_connect tests/conn_connect.cpp) -add_executable(test_conn_request tests/conn_request.cpp) +add_executable(test_conn_exec tests/conn_exec.cpp) add_executable(test_conn_push tests/conn_push.cpp) add_executable(test_conn_quit tests/conn_quit.cpp) add_executable(test_conn_quit_coalesce tests/conn_quit_coalesce.cpp) add_executable(test_conn_reconnect tests/conn_reconnect.cpp) add_executable(test_conn_tls tests/conn_tls.cpp) add_executable(test_low_level tests/low_level.cpp) -add_executable(test_conn_cancel_run tests/conn_cancel_run.cpp) -add_executable(test_conn_cancel_exec tests/conn_cancel_exec.cpp) +add_executable(test_conn_run_cancel tests/conn_run_cancel.cpp) +add_executable(test_conn_exec_cancel tests/conn_exec_cancel.cpp) add_executable(test_conn_echo_stress tests/conn_echo_stress.cpp) - - +add_executable(test_request tests/request.cpp) target_compile_features(chat_room PUBLIC cxx_std_20) target_compile_features(containers PUBLIC cxx_std_20) @@ -92,37 +90,20 @@ target_compile_features(serialization PUBLIC cxx_std_17) target_compile_features(subscriber PUBLIC cxx_std_20) target_compile_features(subscriber_sentinel PUBLIC cxx_std_20) target_compile_features(test_conn_connect PUBLIC cxx_std_17) -target_compile_features(test_conn_request PUBLIC cxx_std_20) +target_compile_features(test_conn_exec PUBLIC cxx_std_20) target_compile_features(test_conn_push PUBLIC cxx_std_20) target_compile_features(test_conn_quit PUBLIC cxx_std_17) target_compile_features(test_conn_quit_coalesce PUBLIC cxx_std_17) target_compile_features(test_conn_reconnect PUBLIC cxx_std_20) target_compile_features(test_conn_tls PUBLIC cxx_std_17) target_compile_features(test_low_level PUBLIC cxx_std_17) -target_compile_features(test_conn_cancel_run PUBLIC cxx_std_20) -target_compile_features(test_conn_cancel_exec PUBLIC cxx_std_20) +target_compile_features(test_conn_run_cancel PUBLIC cxx_std_20) +target_compile_features(test_conn_exec_cancel PUBLIC cxx_std_20) target_compile_features(test_conn_echo_stress PUBLIC cxx_std_20) +target_compile_features(test_request PUBLIC cxx_std_17) -target_link_libraries(intro Boost::container) -target_link_libraries(intro_tls OpenSSL::Crypto OpenSSL::SSL Boost::container) -target_link_libraries(chat_room PUBLIC Boost::container) -target_link_libraries(containers PUBLIC Boost::container) -target_link_libraries(echo_server PUBLIC Boost::container) -target_link_libraries(low_level_sync PUBLIC Boost::container) -target_link_libraries(serialization PUBLIC Boost::container) -target_link_libraries(subscriber PUBLIC Boost::container) -target_link_libraries(subscriber_sentinel PUBLIC Boost::container) -target_link_libraries(test_conn_cancel_exec PUBLIC Boost::container) -target_link_libraries(test_conn_cancel_run PUBLIC Boost::container) -target_link_libraries(test_conn_connect PUBLIC Boost::container) -target_link_libraries(test_conn_push PUBLIC Boost::container) -target_link_libraries(test_conn_quit PUBLIC Boost::container) -target_link_libraries(test_conn_quit_coalesce PUBLIC Boost::container) -target_link_libraries(test_conn_reconnect PUBLIC Boost::container) -target_link_libraries(test_conn_reconnect PUBLIC Boost::container) -target_link_libraries(test_conn_request PUBLIC Boost::container) -target_link_libraries(test_conn_tls OpenSSL::Crypto OpenSSL::SSL Boost::container) - +target_link_libraries(intro_tls OpenSSL::Crypto OpenSSL::SSL) +target_link_libraries(test_conn_tls OpenSSL::Crypto OpenSSL::SSL) # Tests #======================================================================= @@ -134,16 +115,17 @@ add_test(intro_tls intro_tls) add_test(serialization serialization) add_test(low_level_sync low_level_sync) add_test(test_low_level test_low_level) -add_test(test_conn_request test_conn_request) +add_test(test_conn_exec test_conn_exec) add_test(test_conn_connect test_conn_connect) add_test(test_conn_push test_conn_push) add_test(test_conn_quit test_conn_quit) add_test(test_conn_quit_coalesce test_conn_quit_coalesce) add_test(test_conn_reconnect test_conn_reconnect) add_test(test_conn_tls test_conn_tls) -add_test(test_conn_cancel_run test_conn_cancel_run) -add_test(test_conn_cancel_exec test_conn_cancel_exec) +add_test(test_conn_run_cancel test_conn_run_cancel) +add_test(test_conn_exec_cancel test_conn_exec_cancel) add_test(test_conn_echo_stress test_conn_echo_stress) +add_test(test_request test_request) # Install #======================================================================= diff --git a/README.md b/README.md index fc6e7a6f..f651d3fb 100644 --- a/README.md +++ b/README.md @@ -257,7 +257,7 @@ Redis documentation they are called request req; // Command with variable length of arguments. -req.push("SET", "key", "some value", value, "EX", "2"); +req.push("SET", "key", "some value", "EX", "2"); // Pushes a list. std::list list @@ -817,6 +817,12 @@ another. ### master +* Adds allocator support in the `aedis::resp3::request` (a + contribution from Klemens Morgenstern). + +* Renames `aedis::resp3::request::push_range2` to `push_range`. The + suffix 2 was used for disambiguation. Klemens fixed it with SFINAE. + * Renames `fail_on_connection_lost` to `aedis::resp3::request::config::cancel_on_connection_lost`. Now, it will only cause connections to be canceled when `async_run` completes. diff --git a/examples/serialization.cpp b/examples/serialization.cpp index 661e07e0..5f447a4f 100644 --- a/examples/serialization.cpp +++ b/examples/serialization.cpp @@ -46,7 +46,7 @@ void extract(object const& obj, T& t, boost::string_view key) t = value_to(obj.at(key)); } -user tag_invoke(value_to_tag, value const& jv) +auto tag_invoke(value_to_tag, value const& jv) { user u; object const& obj = jv.as_object(); @@ -57,7 +57,7 @@ user tag_invoke(value_to_tag, value const& jv) } // Serializes -void to_bulk(boost::container::pmr::string& to, user const& u) +void to_bulk(std::pmr::string& to, user const& u) { aedis::resp3::to_bulk(to, serialize(value_from(u))); } @@ -69,7 +69,7 @@ void from_bulk(user& u, boost::string_view sv, boost::system::error_code&) u = value_to(jv); } -std::ostream& operator<<(std::ostream& os, user const& u) +auto operator<<(std::ostream& os, user const& u) -> std::ostream& { os << "Name: " << u.name << "\n" << "Age: " << u.age << "\n" @@ -78,36 +78,40 @@ std::ostream& operator<<(std::ostream& os, user const& u) return os; } -bool operator<(user const& a, user const& b) +auto operator<(user const& a, user const& b) { return std::tie(a.name, a.age, a.country) < std::tie(b.name, b.age, b.country); } -auto logger = [](auto ec, auto...) +auto const logger = [](auto ec, auto...) { std::cout << ec.message() << std::endl; }; -int main() +auto main() -> int { - net::io_context ioc; - connection conn{ioc}; - - std::set users - {{"Joao", "58", "Brazil"} , {"Serge", "60", "France"}}; - - request req; - req.get_config().cancel_on_connection_lost = true; - req.push("HELLO", 3); - req.push_range("SADD", "sadd-key", users); // Sends - req.push("SMEMBERS", "sadd-key"); // Retrieves - req.push("QUIT"); - - std::tuple, std::string> resp; - - endpoint ep{"127.0.0.1", "6379"}; - conn.async_exec(req, adapt(resp),logger); - conn.async_run(ep, {}, logger); - ioc.run(); - - // Print - print(std::get<2>(resp)); + try { + net::io_context ioc; + connection conn{ioc}; + + std::set users + {{"Joao", "58", "Brazil"} , {"Serge", "60", "France"}}; + + request req; + req.get_config().cancel_on_connection_lost = true; + req.push("HELLO", 3); + req.push_range("SADD", "sadd-key", users); // Sends + req.push("SMEMBERS", "sadd-key"); // Retrieves + req.push("QUIT"); + + std::tuple, std::string> resp; + + endpoint ep{"127.0.0.1", "6379"}; + conn.async_exec(req, adapt(resp),logger); + conn.async_run(ep, {}, logger); + ioc.run(); + + // Print + print(std::get<2>(resp)); + } catch (std::exception const& e) { + std::cerr << "Error: " << e.what() << std::endl; + } } diff --git a/examples/subscriber.cpp b/examples/subscriber.cpp index 62466c11..dfda48b1 100644 --- a/examples/subscriber.cpp +++ b/examples/subscriber.cpp @@ -80,7 +80,7 @@ net::awaitable reconnect(std::shared_ptr conn) } } -int main() +auto main() -> int { try { net::io_context ioc; @@ -99,5 +99,5 @@ int main() } #else // defined(BOOST_ASIO_HAS_CO_AWAIT) -int main() {std::cout << "Requires coroutine support." << std::endl; return 0;} +auto main() -> int {std::cout << "Requires coroutine support." << std::endl; return 0;} #endif // defined(BOOST_ASIO_HAS_CO_AWAIT) diff --git a/include/aedis/detail/connection_base.hpp b/include/aedis/detail/connection_base.hpp index 849520ac..a8c606d5 100644 --- a/include/aedis/detail/connection_base.hpp +++ b/include/aedis/detail/connection_base.hpp @@ -109,7 +109,7 @@ class connection_base { return ret; } - std::size_t cancel_on_conn_lost() + auto cancel_on_conn_lost() -> std::size_t { auto cond = [](auto const& ptr) { @@ -233,10 +233,10 @@ class connection_base { action_ = action::stop; } - auto is_written() const noexcept + [[nodiscard]] auto is_written() const noexcept { return status_ == status::written; } - auto is_staged() const noexcept + [[nodiscard]] auto is_staged() const noexcept { return status_ == status::staged; } void mark_written() noexcept @@ -248,13 +248,13 @@ class connection_base { void reset_status() noexcept { status_ = status::none; } - auto get_number_of_commands() const noexcept + [[nodiscard]] auto get_number_of_commands() const noexcept { return cmds_; } - auto const& get_request() const noexcept + [[nodiscard]] auto get_request() const noexcept -> auto const& { return *req_; } - auto get_action() const noexcept + [[nodiscard]] auto get_action() const noexcept { return action_;} template diff --git a/include/aedis/detail/connection_ops.hpp b/include/aedis/detail/connection_ops.hpp index 75384a2b..0aecce81 100644 --- a/include/aedis/detail/connection_ops.hpp +++ b/include/aedis/detail/connection_ops.hpp @@ -56,7 +56,7 @@ struct connect_with_timeout_op { template struct resolve_with_timeout_op { Conn* conn = nullptr; - std::chrono::steady_clock::duration resolve_timeout; + std::chrono::steady_clock::duration resolve_timeout{}; boost::asio::coroutine coro{}; template diff --git a/include/aedis/resp3/detail/parser.hpp b/include/aedis/resp3/detail/parser.hpp index 5b026bc7..c32679b3 100644 --- a/include/aedis/resp3/detail/parser.hpp +++ b/include/aedis/resp3/detail/parser.hpp @@ -212,7 +212,7 @@ class parser { // The bulk type expected in the next read. If none is expected returns // type::invalid. - auto bulk() const noexcept { return bulk_; } + [[nodiscard]] auto bulk() const noexcept { return bulk_; } // The length expected in the the next bulk. [[nodiscard]] auto bulk_length() const noexcept { return bulk_length_; } diff --git a/include/aedis/resp3/node.hpp b/include/aedis/resp3/node.hpp index cbb91078..4dce3b5d 100644 --- a/include/aedis/resp3/node.hpp +++ b/include/aedis/resp3/node.hpp @@ -26,16 +26,16 @@ namespace aedis::resp3 { template struct node { /// The RESP3 type of the data in this node. - resp3::type data_type; + type data_type = type::invalid; /// The number of elements of an aggregate. - std::size_t aggregate_size; + std::size_t aggregate_size{}; /// The depth of this node in the response tree. - std::size_t depth; + std::size_t depth{}; /// The actual data. For aggregate types this is usually empty. - String value; + String value{}; }; /** @brief Converts the node to a string. diff --git a/include/aedis/resp3/request.hpp b/include/aedis/resp3/request.hpp index 689fd665..053c8099 100644 --- a/include/aedis/resp3/request.hpp +++ b/include/aedis/resp3/request.hpp @@ -9,10 +9,9 @@ #include #include +#include #include -#include -#include #include #include @@ -203,18 +202,15 @@ class request { bool retry = true; }; - /** @brief Constructor + /** \brief Constructor * - * @param cfg Configuration options. + * \param cfg Configuration options. + * \param resource Memory resource. */ - explicit request(boost::container::pmr::memory_resource * resource) - : payload_(resource), cfg_{false, true, false, true} - {} - - explicit request(config cfg = config{false, true, false, true}, - boost::container::pmr::memory_resource * resource = - boost::container::pmr::get_default_resource()) - : payload_(resource), cfg_{cfg} + explicit + request(config cfg = config{false, true, false, true}, + std::pmr::memory_resource* resource = std::pmr::get_default_resource()) + : payload_(resource), cfg_{cfg} {} @@ -389,7 +385,7 @@ class request { [[nodiscard]] auto get_config() noexcept -> auto& {return cfg_; } private: - boost::container::pmr::string payload_; + std::pmr::string payload_; std::size_t commands_ = 0; config cfg_; }; diff --git a/tests/conn_connect.cpp b/tests/conn_connect.cpp index f1ee034b..a3fec2b2 100644 --- a/tests/conn_connect.cpp +++ b/tests/conn_connect.cpp @@ -117,7 +117,7 @@ BOOST_AUTO_TEST_CASE(plain_conn_on_tls_endpoint) ep.port = "443"; auto const ec = test_async_run(ep); - BOOST_CHECK_EQUAL(ec, net::error::misc_errors::eof); + BOOST_TEST(!!ec); } auto auth_fail_error(boost::system::error_code ec) diff --git a/tests/conn_request.cpp b/tests/conn_exec.cpp similarity index 83% rename from tests/conn_request.cpp rename to tests/conn_exec.cpp index cd4e0e1b..600d5510 100644 --- a/tests/conn_request.cpp +++ b/tests/conn_exec.cpp @@ -8,8 +8,6 @@ #include #include -#include - #define BOOST_TEST_MODULE low level #include @@ -31,7 +29,6 @@ using namespace net::experimental::awaitable_operators; BOOST_AUTO_TEST_CASE(wrong_response_data_type) { - std::cout << boost::unit_test::framework::current_test_case().p_name << std::endl; request req; req.push("QUIT"); @@ -51,11 +48,7 @@ BOOST_AUTO_TEST_CASE(wrong_response_data_type) BOOST_AUTO_TEST_CASE(cancel_request_if_not_connected) { - std::cout << boost::unit_test::framework::current_test_case().p_name << std::endl; - namespace pmr = boost::container::pmr; - char buf[4096]; - pmr::monotonic_buffer_resource resource{buf, 4096}; - request req{&resource}; + request req; req.get_config().cancel_if_not_connected = true; req.push("PING"); @@ -70,8 +63,6 @@ BOOST_AUTO_TEST_CASE(cancel_request_if_not_connected) BOOST_AUTO_TEST_CASE(request_retry) { - std::cout << boost::unit_test::framework::current_test_case().p_name << std::endl; - request req1; req1.get_config().cancel_on_connection_lost = true; req1.push("CLIENT", "PAUSE", 7000); diff --git a/tests/conn_cancel_exec.cpp b/tests/conn_exec_cancel.cpp similarity index 100% rename from tests/conn_cancel_exec.cpp rename to tests/conn_exec_cancel.cpp diff --git a/tests/conn_cancel_run.cpp b/tests/conn_run_cancel.cpp similarity index 100% rename from tests/conn_cancel_run.cpp rename to tests/conn_run_cancel.cpp diff --git a/tests/request.cpp b/tests/request.cpp new file mode 100644 index 00000000..d0b31da0 --- /dev/null +++ b/tests/request.cpp @@ -0,0 +1,57 @@ +/* Copyright (c) 2018-2022 Marcelo Zimbres Silva (mzimbres@gmail.com) + * + * Distributed under the Boost Software License, Version 1.0. (See + * accompanying file LICENSE.txt) + */ + +#include +#include + +#define BOOST_TEST_MODULE low level +#include + +#include +#include + +using aedis::resp3::request; + +// TODO: Serialization. + +BOOST_AUTO_TEST_CASE(single_arg_allocator) +{ + char buf[4096]; + std::pmr::monotonic_buffer_resource resource{buf, 4096}; + request req1{{}, &resource}; + req1.push("PING"); + BOOST_CHECK_EQUAL(req1.payload(), std::pmr::string{"*1\r\n$4\r\nPING\r\n"}); +} + +BOOST_AUTO_TEST_CASE(arg_int) +{ + request req; + req.push("PING", 42); + BOOST_CHECK_EQUAL(req.payload(), std::pmr::string{"*2\r\n$4\r\nPING\r\n$2\r\n42\r\n"}); +} + +BOOST_AUTO_TEST_CASE(multiple_args) +{ + char const* res = "*5\r\n$3\r\nSET\r\n$3\r\nkey\r\n$5\r\nvalue\r\n$2\r\nEX\r\n$1\r\n2\r\n"; + request req; + req.push("SET", "key", "value", "EX", "2"); + BOOST_CHECK_EQUAL(req.payload(), std::pmr::string{res}); +} + +BOOST_AUTO_TEST_CASE(container_and_range) +{ + std::map in{{"key1", "value1"}, {"key2", "value2"}}; + + char const* res = "*6\r\n$4\r\nHSET\r\n$3\r\nkey\r\n$4\r\nkey1\r\n$6\r\nvalue1\r\n$4\r\nkey2\r\n$6\r\nvalue2\r\n"; + + request req1; + req1.push_range("HSET", "key", in); + BOOST_CHECK_EQUAL(req1.payload(), std::pmr::string{res}); + + request req2; + req2.push_range("HSET", "key", std::cbegin(in), std::cend(in)); + BOOST_CHECK_EQUAL(req2.payload(), std::pmr::string{res}); +}