Skip to content

Commit 42411be

Browse files
authored
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
1 parent 6be0d12 commit 42411be

File tree

5 files changed

+54
-4
lines changed

5 files changed

+54
-4
lines changed

include/boost/redis/impl/setup_request_utils.ipp

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,11 @@ void compose_setup_request(config& cfg)
4242
}
4343

4444
// In any case, the setup request should have the priority
45-
// flag set so it's executed before any other request
45+
// flag set so it's executed before any other request.
46+
// The setup request should never be retried.
4647
request_access::set_priority(cfg.setup, true);
48+
cfg.setup.get_config().cancel_if_unresponded = true;
49+
cfg.setup.get_config().cancel_on_connection_lost = true;
4750
}
4851

4952
void clear_response(generic_response& res)

include/boost/redis/request.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class request {
9898
*
9999
* @param cfg Configuration options.
100100
*/
101-
explicit request(config cfg = config{true, false, true, true})
101+
explicit request(config cfg = config{false, false, true, true})
102102
: cfg_{cfg}
103103
{ }
104104

test/test_conn_quit.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ BOOST_AUTO_TEST_CASE(test_async_run_exits)
4343
// Should fail since this request will be sent after quit.
4444
request req3;
4545
req3.get_config().cancel_if_not_connected = true;
46+
req3.get_config().cancel_on_connection_lost = true;
4647
req3.push("PING");
4748

4849
bool c1_called = false, c2_called = false, c3_called = false;

test/test_multiplexer.cpp

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -793,10 +793,18 @@ void test_cancel_on_connection_lost()
793793
test_item item_written1, item_written2, item_staged1, item_staged2, item_waiting1, item_waiting2;
794794

795795
// Different items have different configurations
796-
// (note that these are all true by default)
797796
item_written1.req.get_config().cancel_if_unresponded = false;
797+
item_written1.req.get_config().cancel_on_connection_lost = true;
798+
item_written2.req.get_config().cancel_if_unresponded = true;
799+
item_written2.req.get_config().cancel_on_connection_lost = true;
798800
item_staged1.req.get_config().cancel_if_unresponded = false;
801+
item_staged1.req.get_config().cancel_on_connection_lost = true;
802+
item_staged2.req.get_config().cancel_if_unresponded = true;
803+
item_staged2.req.get_config().cancel_on_connection_lost = true;
804+
item_waiting1.req.get_config().cancel_if_unresponded = true;
799805
item_waiting1.req.get_config().cancel_on_connection_lost = false;
806+
item_waiting2.req.get_config().cancel_if_unresponded = true;
807+
item_waiting2.req.get_config().cancel_on_connection_lost = true;
800808

801809
// Make each item reach the state it should be in
802810
mpx.add(item_written1.elem_ptr);
@@ -847,9 +855,10 @@ void test_cancel_on_connection_lost_abandoned()
847855
auto item_staged2 = std::make_unique<test_item>();
848856

849857
// Different items have different configurations
850-
// (note that these are all true by default)
851858
item_written1->req.get_config().cancel_if_unresponded = false;
859+
item_written2->req.get_config().cancel_if_unresponded = true;
852860
item_staged1->req.get_config().cancel_if_unresponded = false;
861+
item_staged2->req.get_config().cancel_if_unresponded = true;
853862

854863
// Make each item reach the state it should be in
855864
mpx.add(item_written1->elem_ptr);
@@ -938,6 +947,7 @@ void test_reset()
938947
generic_response push_resp;
939948
mpx.set_receive_adapter(any_adapter{push_resp});
940949
test_item item1, item2;
950+
item1.req.get_config().cancel_on_connection_lost = true;
941951

942952
// Add a request
943953
mpx.add(item1.elem_ptr);

test/test_setup_request_utils.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,8 @@ void test_compose_setup()
3636
std::string_view const expected = "*2\r\n$5\r\nHELLO\r\n$1\r\n3\r\n";
3737
BOOST_TEST_EQ(cfg.setup.payload(), expected);
3838
BOOST_TEST(cfg.setup.has_hello_priority());
39+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
40+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
3941
}
4042

4143
void test_compose_setup_select()
@@ -51,6 +53,8 @@ void test_compose_setup_select()
5153
"*2\r\n$6\r\nSELECT\r\n$2\r\n10\r\n";
5254
BOOST_TEST_EQ(cfg.setup.payload(), expected);
5355
BOOST_TEST(cfg.setup.has_hello_priority());
56+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
57+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
5458
}
5559

5660
void test_compose_setup_clientname()
@@ -63,6 +67,8 @@ void test_compose_setup_clientname()
6367
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";
6468
BOOST_TEST_EQ(cfg.setup.payload(), expected);
6569
BOOST_TEST(cfg.setup.has_hello_priority());
70+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
71+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
6672
}
6773

6874
void test_compose_setup_auth()
@@ -78,6 +84,8 @@ void test_compose_setup_auth()
7884
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";
7985
BOOST_TEST_EQ(cfg.setup.payload(), expected);
8086
BOOST_TEST(cfg.setup.has_hello_priority());
87+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
88+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
8189
}
8290

8391
void test_compose_setup_auth_empty_password()
@@ -92,6 +100,8 @@ void test_compose_setup_auth_empty_password()
92100
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";
93101
BOOST_TEST_EQ(cfg.setup.payload(), expected);
94102
BOOST_TEST(cfg.setup.has_hello_priority());
103+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
104+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
95105
}
96106

97107
void test_compose_setup_auth_setname()
@@ -108,6 +118,8 @@ void test_compose_setup_auth_setname()
108118
"6\r\nmytest\r\n";
109119
BOOST_TEST_EQ(cfg.setup.payload(), expected);
110120
BOOST_TEST(cfg.setup.has_hello_priority());
121+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
122+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
111123
}
112124

113125
void test_compose_setup_use_setup()
@@ -127,6 +139,8 @@ void test_compose_setup_use_setup()
127139
"*2\r\n$6\r\nSELECT\r\n$1\r\n8\r\n";
128140
BOOST_TEST_EQ(cfg.setup.payload(), expected);
129141
BOOST_TEST(cfg.setup.has_hello_priority());
142+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
143+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
130144
}
131145

132146
// Regression check: we set the priority flag
@@ -142,6 +156,27 @@ void test_compose_setup_use_setup_no_hello()
142156
std::string_view const expected = "*2\r\n$6\r\nSELECT\r\n$1\r\n8\r\n";
143157
BOOST_TEST_EQ(cfg.setup.payload(), expected);
144158
BOOST_TEST(cfg.setup.has_hello_priority());
159+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
160+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
161+
}
162+
163+
// Regression check: we set the relevant cancellation flags in the request
164+
void test_compose_setup_use_setup_flags()
165+
{
166+
redis::config cfg;
167+
cfg.use_setup = true;
168+
cfg.setup.clear();
169+
cfg.setup.push("SELECT", 8);
170+
cfg.setup.get_config().cancel_if_unresponded = false;
171+
cfg.setup.get_config().cancel_on_connection_lost = false;
172+
173+
compose_setup_request(cfg);
174+
175+
std::string_view const expected = "*2\r\n$6\r\nSELECT\r\n$1\r\n8\r\n";
176+
BOOST_TEST_EQ(cfg.setup.payload(), expected);
177+
BOOST_TEST(cfg.setup.has_hello_priority());
178+
BOOST_TEST(cfg.setup.get_config().cancel_if_unresponded);
179+
BOOST_TEST(cfg.setup.get_config().cancel_on_connection_lost);
145180
}
146181

147182
// clear response
@@ -185,6 +220,7 @@ int main()
185220
test_compose_setup_auth_setname();
186221
test_compose_setup_use_setup();
187222
test_compose_setup_use_setup_no_hello();
223+
test_compose_setup_use_setup_flags();
188224

189225
test_clear_response_empty();
190226
test_clear_response_nonempty();

0 commit comments

Comments
 (0)