From 42411be44425f2a50b5d7e570e15d5f602c8743d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anarthal=20=28Rub=C3=A9n=20P=C3=A9rez=29?= <34971811+anarthal@users.noreply.github.com> Date: Mon, 20 Oct 2025 16:37:02 +0200 Subject: [PATCH] Changes request default's cancel_on_connection_lost to false (#334) Changes request default constructor to set cancel_on_connection_lost to false, matching request::config's default initializer Overwrites this flag to true for the setup request Adds unit tests for the latter --- .../boost/redis/impl/setup_request_utils.ipp | 5 ++- include/boost/redis/request.hpp | 2 +- test/test_conn_quit.cpp | 1 + test/test_multiplexer.cpp | 14 ++++++-- test/test_setup_request_utils.cpp | 36 +++++++++++++++++++ 5 files changed, 54 insertions(+), 4 deletions(-) diff --git a/include/boost/redis/impl/setup_request_utils.ipp b/include/boost/redis/impl/setup_request_utils.ipp index c8f562f5..5f789670 100644 --- a/include/boost/redis/impl/setup_request_utils.ipp +++ b/include/boost/redis/impl/setup_request_utils.ipp @@ -42,8 +42,11 @@ void compose_setup_request(config& cfg) } // In any case, the setup request should have the priority - // flag set so it's executed before any other request + // flag set so it's executed before any other request. + // The setup request should never be retried. request_access::set_priority(cfg.setup, true); + cfg.setup.get_config().cancel_if_unresponded = true; + cfg.setup.get_config().cancel_on_connection_lost = true; } void clear_response(generic_response& res) diff --git a/include/boost/redis/request.hpp b/include/boost/redis/request.hpp index 5d165529..534d92c6 100644 --- a/include/boost/redis/request.hpp +++ b/include/boost/redis/request.hpp @@ -98,7 +98,7 @@ public: * * @param cfg Configuration options. */ - explicit request(config cfg = config{true, false, true, true}) + explicit request(config cfg = config{false, false, true, true}) : cfg_{cfg} { } diff --git a/test/test_conn_quit.cpp b/test/test_conn_quit.cpp index efad6d7b..5a614a37 100644 --- a/test/test_conn_quit.cpp +++ b/test/test_conn_quit.cpp @@ -43,6 +43,7 @@ BOOST_AUTO_TEST_CASE(test_async_run_exits) // Should fail since this request will be sent after quit. request req3; req3.get_config().cancel_if_not_connected = true; + req3.get_config().cancel_on_connection_lost = true; req3.push("PING"); bool c1_called = false, c2_called = false, c3_called = false; diff --git a/test/test_multiplexer.cpp b/test/test_multiplexer.cpp index 22b56246..52db5e5c 100644 --- a/test/test_multiplexer.cpp +++ b/test/test_multiplexer.cpp @@ -793,10 +793,18 @@ void test_cancel_on_connection_lost() test_item item_written1, item_written2, item_staged1, item_staged2, item_waiting1, item_waiting2; // Different items have different configurations - // (note that these are all true by default) item_written1.req.get_config().cancel_if_unresponded = false; + item_written1.req.get_config().cancel_on_connection_lost = true; + item_written2.req.get_config().cancel_if_unresponded = true; + item_written2.req.get_config().cancel_on_connection_lost = true; item_staged1.req.get_config().cancel_if_unresponded = false; + item_staged1.req.get_config().cancel_on_connection_lost = true; + item_staged2.req.get_config().cancel_if_unresponded = true; + item_staged2.req.get_config().cancel_on_connection_lost = true; + item_waiting1.req.get_config().cancel_if_unresponded = true; item_waiting1.req.get_config().cancel_on_connection_lost = false; + item_waiting2.req.get_config().cancel_if_unresponded = true; + item_waiting2.req.get_config().cancel_on_connection_lost = true; // Make each item reach the state it should be in mpx.add(item_written1.elem_ptr); @@ -847,9 +855,10 @@ void test_cancel_on_connection_lost_abandoned() auto item_staged2 = std::make_unique(); // Different items have different configurations - // (note that these are all true by default) item_written1->req.get_config().cancel_if_unresponded = false; + item_written2->req.get_config().cancel_if_unresponded = true; item_staged1->req.get_config().cancel_if_unresponded = false; + item_staged2->req.get_config().cancel_if_unresponded = true; // Make each item reach the state it should be in mpx.add(item_written1->elem_ptr); @@ -938,6 +947,7 @@ void test_reset() generic_response push_resp; mpx.set_receive_adapter(any_adapter{push_resp}); test_item item1, item2; + item1.req.get_config().cancel_on_connection_lost = true; // Add a request mpx.add(item1.elem_ptr); diff --git a/test/test_setup_request_utils.cpp b/test/test_setup_request_utils.cpp index 2e2860a3..519fe53e 100644 --- a/test/test_setup_request_utils.cpp +++ b/test/test_setup_request_utils.cpp @@ -36,6 +36,8 @@ void test_compose_setup() std::string_view const expected = "*2\r\n$5\r\nHELLO\r\n$1\r\n3\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } void test_compose_setup_select() @@ -51,6 +53,8 @@ void test_compose_setup_select() "*2\r\n$6\r\nSELECT\r\n$2\r\n10\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } void test_compose_setup_clientname() @@ -63,6 +67,8 @@ void test_compose_setup_clientname() expected = "*4\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$7\r\nSETNAME\r\n$11\r\nBoost.Redis\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } void test_compose_setup_auth() @@ -78,6 +84,8 @@ void test_compose_setup_auth() expected = "*5\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$4\r\nAUTH\r\n$3\r\nfoo\r\n$3\r\nbar\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } void test_compose_setup_auth_empty_password() @@ -92,6 +100,8 @@ void test_compose_setup_auth_empty_password() expected = "*5\r\n$5\r\nHELLO\r\n$1\r\n3\r\n$4\r\nAUTH\r\n$3\r\nfoo\r\n$0\r\n\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } void test_compose_setup_auth_setname() @@ -108,6 +118,8 @@ void test_compose_setup_auth_setname() "6\r\nmytest\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } void test_compose_setup_use_setup() @@ -127,6 +139,8 @@ void test_compose_setup_use_setup() "*2\r\n$6\r\nSELECT\r\n$1\r\n8\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } // Regression check: we set the priority flag @@ -142,6 +156,27 @@ void test_compose_setup_use_setup_no_hello() std::string_view const expected = "*2\r\n$6\r\nSELECT\r\n$1\r\n8\r\n"; BOOST_TEST_EQ(cfg.setup.payload(), expected); BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); +} + +// Regression check: we set the relevant cancellation flags in the request +void test_compose_setup_use_setup_flags() +{ + redis::config cfg; + cfg.use_setup = true; + cfg.setup.clear(); + cfg.setup.push("SELECT", 8); + cfg.setup.get_config().cancel_if_unresponded = false; + cfg.setup.get_config().cancel_on_connection_lost = false; + + compose_setup_request(cfg); + + std::string_view const expected = "*2\r\n$6\r\nSELECT\r\n$1\r\n8\r\n"; + BOOST_TEST_EQ(cfg.setup.payload(), expected); + BOOST_TEST(cfg.setup.has_hello_priority()); + BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded); + BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost); } // clear response @@ -185,6 +220,7 @@ int main() test_compose_setup_auth_setname(); test_compose_setup_use_setup(); test_compose_setup_use_setup_no_hello(); + test_compose_setup_use_setup_flags(); test_clear_response_empty(); test_clear_response_nonempty();