From 0acca49620ee14866015900ca2a5b4e3f1ec8c2c Mon Sep 17 00:00:00 2001 From: Marcelo Zimbres Date: Thu, 3 Oct 2024 23:15:18 +0200 Subject: [PATCH] Fixes the adapter of empty nested responses. See https://github.com/boostorg/redis/issues/210. --- README.md | 3 + .../redis/adapter/detail/result_traits.hpp | 33 +++--- test/test_low_level_sync_sans_io.cpp | 112 ++++++++++++++++++ 3 files changed, 133 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 1f33d57e..567fb2dd 100644 --- a/README.md +++ b/README.md @@ -684,6 +684,9 @@ https://lists.boost.org/Archives/boost/2023/01/253944.php. also changed to return EOF to the user when that error is received from the server. That is a breaking change. +* ([Issue 210](https://github.com/boostorg/redis/issues/210)) + Fixes the adapter of empty nested reposponses. + ### Boost 1.85 * ([Issue 170](https://github.com/boostorg/redis/issues/170)) diff --git a/include/boost/redis/adapter/detail/result_traits.hpp b/include/boost/redis/adapter/detail/result_traits.hpp index 47f87fe6..e1193d89 100644 --- a/include/boost/redis/adapter/detail/result_traits.hpp +++ b/include/boost/redis/adapter/detail/result_traits.hpp @@ -102,8 +102,12 @@ class static_aggregate_adapter> { std::variant>, std::tuple_size::value>; + // Tuple element we are currently on. std::size_t i_ = 0; + + // Nested aggregate element counter. std::size_t aggregate_size_ = 0; + adapters_array_type adapters_; result* res_ = nullptr; @@ -117,36 +121,35 @@ class static_aggregate_adapter> { } template - void count(resp3::basic_node const& nd) + void count(resp3::basic_node const& elem) { - if (nd.depth == 1) { - if (is_aggregate(nd.data_type)) - aggregate_size_ = element_multiplicity(nd.data_type) * nd.aggregate_size; - else - ++i_; - - return; + if (elem.depth == 1 && is_aggregate(elem.data_type)) { + aggregate_size_ = element_multiplicity(elem.data_type) * elem.aggregate_size; } - if (--aggregate_size_ == 0) - ++i_; + if (aggregate_size_ == 0) { + i_ += 1; + } else { + aggregate_size_ -= 1; + } } template - void operator()(resp3::basic_node const& nd, system::error_code& ec) + void operator()(resp3::basic_node const& elem, system::error_code& ec) { using std::visit; - if (nd.depth == 0) { - auto const real_aggr_size = nd.aggregate_size * element_multiplicity(nd.data_type); + if (elem.depth == 0) { + auto const multiplicity = element_multiplicity(elem.data_type); + auto const real_aggr_size = elem.aggregate_size * multiplicity; if (real_aggr_size != std::tuple_size::value) ec = redis::error::incompatible_size; return; } - visit([&](auto& arg){arg(nd, ec);}, adapters_[i_]); - count(nd); + visit([&](auto& arg){arg(elem, ec);}, adapters_[i_]); + count(elem); } }; diff --git a/test/test_low_level_sync_sans_io.cpp b/test/test_low_level_sync_sans_io.cpp index 12e4980a..6f6acfe0 100644 --- a/test/test_low_level_sync_sans_io.cpp +++ b/test/test_low_level_sync_sans_io.cpp @@ -15,9 +15,11 @@ using boost::redis::request; using boost::redis::config; using boost::redis::detail::push_hello; +using boost::redis::response; using boost::redis::adapter::adapt2; using boost::redis::adapter::result; using boost::redis::resp3::detail::deserialize; +using boost::redis::ignore_t; BOOST_AUTO_TEST_CASE(low_level_sync_sans_io) { @@ -88,3 +90,113 @@ BOOST_AUTO_TEST_CASE(config_to_hello_cmd_auth) std::string_view const 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_CHECK_EQUAL(req.payload(), expected); } + +BOOST_AUTO_TEST_CASE(issue_210_empty_set) +{ + try { + result< + std::tuple< + result, + result>, + result, + result + > + > resp; + + char const* wire = "*4\r\n:1\r\n~0\r\n$25\r\nthis_should_not_be_in_set\r\n:2\r\n"; + + deserialize(wire, adapt2(resp)); + + BOOST_CHECK_EQUAL(std::get<0>(resp.value()).value(), 1); + BOOST_CHECK(std::get<1>(resp.value()).value().empty()); + BOOST_CHECK_EQUAL(std::get<2>(resp.value()).value(), "this_should_not_be_in_set"); + BOOST_CHECK_EQUAL(std::get<3>(resp.value()).value(), 2); + + } catch (std::exception const& e) { + std::cerr << e.what() << std::endl; + exit(EXIT_FAILURE); + } +} + +BOOST_AUTO_TEST_CASE(issue_210_non_empty_set_size_one) +{ + try { + result< + std::tuple< + result, + result>, + result, + result + > + > resp; + + char const* wire = "*4\r\n:1\r\n~1\r\n$3\r\nfoo\r\n$25\r\nthis_should_not_be_in_set\r\n:2\r\n"; + + deserialize(wire, adapt2(resp)); + + BOOST_CHECK_EQUAL(std::get<0>(resp.value()).value(), 1); + BOOST_CHECK_EQUAL(std::get<1>(resp.value()).value().size(), 1); + BOOST_CHECK_EQUAL(std::get<1>(resp.value()).value().at(0), std::string{"foo"}); + BOOST_CHECK_EQUAL(std::get<2>(resp.value()).value(), "this_should_not_be_in_set"); + BOOST_CHECK_EQUAL(std::get<3>(resp.value()).value(), 2); + + } catch (std::exception const& e) { + std::cerr << e.what() << std::endl; + exit(EXIT_FAILURE); + } +} + +BOOST_AUTO_TEST_CASE(issue_210_non_empty_set_size_two) +{ + try { + result< + std::tuple< + result, + result>, + result, + result + > + > resp; + + char const* wire = "*4\r\n:1\r\n~2\r\n$3\r\nfoo\r\n$3\r\nbar\r\n$25\r\nthis_should_not_be_in_set\r\n:2\r\n"; + + deserialize(wire, adapt2(resp)); + + BOOST_CHECK_EQUAL(std::get<0>(resp.value()).value(), 1); + BOOST_CHECK_EQUAL(std::get<1>(resp.value()).value().at(0), std::string{"foo"}); + BOOST_CHECK_EQUAL(std::get<1>(resp.value()).value().at(1), std::string{"bar"}); + BOOST_CHECK_EQUAL(std::get<2>(resp.value()).value(), "this_should_not_be_in_set"); + + } catch (std::exception const& e) { + std::cerr << e.what() << std::endl; + exit(EXIT_FAILURE); + } +} + +BOOST_AUTO_TEST_CASE(issue_210_no_nested) +{ + try { + result< + std::tuple< + result, + result, + result, + result + > + > resp; + + char const* wire = "*4\r\n:1\r\n$3\r\nfoo\r\n$3\r\nbar\r\n$25\r\nthis_should_not_be_in_set\r\n"; + + deserialize(wire, adapt2(resp)); + + BOOST_CHECK_EQUAL(std::get<0>(resp.value()).value(), 1); + BOOST_CHECK_EQUAL(std::get<1>(resp.value()).value(), std::string{"foo"}); + BOOST_CHECK_EQUAL(std::get<2>(resp.value()).value(), std::string{"bar"}); + BOOST_CHECK_EQUAL(std::get<3>(resp.value()).value(), "this_should_not_be_in_set"); + + } catch (std::exception const& e) { + std::cerr << e.what() << std::endl; + exit(EXIT_FAILURE); + } +} +