Uses class specialization instead of function overload. [skip-ci]

mzimbres · mzimbres · commit 1bacc7435f01 · 2025-02-18T00:00:55.000+01:00
diff --git a/example/cpp20_containers.cpp b/example/cpp20_containers.cpp
@@ -80,7 +80,10 @@ auto transaction(std::shared_ptr<connection> conn) -> awaitable<void>
       ignore_t, // multi
       ignore_t, // lrange
       ignore_t, // hgetall
-      response<std::optional<std::vector<int>>, std::optional<std::map<std::string, std::string>>> // exec
+      response<
+         std::optional<std::vector<int>>,
+         std::optional<std::map<std::string, std::string>>
+      > // exec
    > resp;
 
    co_await conn->async_exec(req, resp);
diff --git a/example/cpp20_json.cpp b/example/cpp20_json.cpp
@@ -21,6 +21,7 @@
 #include <boost/redis/resp3/serialization.hpp>
 
 namespace asio = boost::asio;
+namespace resp3 = boost::redis::resp3;
 using namespace boost::describe;
 using boost::redis::request;
 using boost::redis::response;
@@ -42,8 +43,14 @@ BOOST_DESCRIBE_STRUCT(user, (), (name, age, country))
 void boost_redis_to_bulk(std::string& to, user const& u)
    { boost::redis::resp3::boost_redis_to_bulk(to, boost::json::serialize(boost::json::value_from(u))); }
 
-void boost_redis_from_bulk(user& u, std::string_view sv, boost::system::error_code&)
-   { u = boost::json::value_to<user>(boost::json::parse(sv)); }
+void
+boost_redis_from_bulk(
+   user& u,
+   resp3::node_view const& node,
+   boost::system::error_code&)
+{
+   u = boost::json::value_to<user>(boost::json::parse(node.value));
+}
 
 auto co_main(config cfg) -> asio::awaitable<void>
 {
diff --git a/example/cpp20_protobuf.cpp b/example/cpp20_protobuf.cpp
@@ -19,6 +19,7 @@
 #if defined(BOOST_ASIO_HAS_CO_AWAIT)
 
 namespace asio = boost::asio;
+namespace resp3 = boost::redis::resp3;
 using boost::redis::request;
 using boost::redis::response;
 using boost::redis::operation;
@@ -45,9 +46,13 @@ void boost_redis_to_bulk(std::string& to, person const& u)
    boost::redis::resp3::boost_redis_to_bulk(to, tmp);
 }
 
-void boost_redis_from_bulk(person& u, std::string_view sv, boost::system::error_code& ec)
+void
+boost_redis_from_bulk(
+   person& u,
+   resp3::node_view const& node,
+   boost::system::error_code& ec)
 {
-   std::string const tmp {sv};
+   std::string const tmp {node.value};
    if (!u.ParseFromString(tmp))
       ec = boost::redis::error::invalid_data_type;
 }
diff --git a/include/boost/redis/adapter/detail/adapters.hpp b/include/boost/redis/adapter/detail/adapters.hpp
@@ -37,49 +37,120 @@
 namespace boost::redis::adapter::detail
 {
 
-// Serialization.
+template <class> struct is_integral : std::false_type {};
 
-template <class T>
-auto boost_redis_from_bulk(T& i, std::string_view sv, system::error_code& ec) -> typename std::enable_if<std::is_integral<T>::value, void>::type
-{
-   auto const res = std::from_chars(sv.data(), sv.data() + std::size(sv), i);
-   if (res.ec != std::errc())
-      ec = redis::error::not_a_number;
-}
+template <> struct is_integral<long long int         > : std::true_type {};
+template <> struct is_integral<unsigned long long int> : std::true_type {};
+template <> struct is_integral<int                   > : std::true_type {};
 
-inline
-void boost_redis_from_bulk(bool& t, std::string_view sv, system::error_code&)
-{
-   t = *sv.data() == 't';
-}
+template<class T, bool = is_integral<T>::value>
+struct converter;
 
-inline
-void boost_redis_from_bulk(double& d, std::string_view sv, system::error_code& ec)
-{
+template<class T>
+struct converter<T, true> {
+   template <class String>
+   static void
+   apply(
+      T& i,
+      resp3::basic_node<String> const& node,
+      system::error_code& ec)
+   {
+      auto const res =
+         std::from_chars(node.value.data(), node.value.data() + node.value.size(), i);
+      if (res.ec != std::errc())
+         ec = redis::error::not_a_number;
+   }
+};
+
+template<>
+struct converter<bool, false> {
+   template <class String>
+   static void
+   apply(
+      bool& t,
+      resp3::basic_node<String> const& node,
+      system::error_code& ec)
+   {
+      t = *node.value.data() == 't';
+   }
+};
+
+template<>
+struct converter<double, false> {
+   template <class String>
+   static void
+   apply(
+      double& d,
+      resp3::basic_node<String> const& node,
+      system::error_code& ec)
+   {
 #ifdef _LIBCPP_VERSION
-   // The string in sv is not null terminated and we also don't know
-   // if there is enough space at the end for a null char. The easiest
-   // thing to do is to create a temporary.
-   std::string const tmp{sv.data(), sv.data() + std::size(sv)};
-   char* end{};
-   d = std::strtod(tmp.data(), &end);
-   if (d == HUGE_VAL || d == 0)
-      ec = redis::error::not_a_double;
+      // The string in node.value is not null terminated and we also
+      // don't know if there is enough space at the end for a null
+      // char. The easiest thing to do is to create a temporary.
+      std::string const tmp{node.value.data(), node.value.data() + node.value.size()};
+      char* end{};
+      d = std::strtod(tmp.data(), &end);
+      if (d == HUGE_VAL || d == 0)
+         ec = redis::error::not_a_double;
 #else
-   auto const res = std::from_chars(sv.data(), sv.data() + std::size(sv), d);
-   if (res.ec != std::errc())
-      ec = redis::error::not_a_double;
+      auto const res = std::from_chars(node.value.data(), node.value.data() + node.value.size(), d);
+      if (res.ec != std::errc())
+         ec = redis::error::not_a_double;
 #endif // _LIBCPP_VERSION
-}
+   }
+};
 
 template <class CharT, class Traits, class Allocator>
+struct converter<std::basic_string<CharT, Traits, Allocator>, false> {
+   template <class String>
+   static void
+   apply(
+      std::basic_string<CharT, Traits, Allocator>& s,
+      resp3::basic_node<String> const& node,
+      system::error_code&)
+   {
+      s.append(node.value.data(), node.value.size());
+   }
+};
+
+template <class T>
+struct from_bulk_impl {
+   template <class String>
+   static void
+   apply(
+      T& t,
+      resp3::basic_node<String> const& node,
+      system::error_code& ec)
+   {
+      converter<T>::apply(t, node, ec);
+   }
+};
+
+template <class T>
+struct from_bulk_impl<std::optional<T>> {
+   template <class String>
+   static void
+   apply(
+      std::optional<T>& op,
+      resp3::basic_node<String> const& node,
+      system::error_code& ec)
+   {
+      if (node.data_type != resp3::type::null) {
+         op.emplace(T{});
+         converter<T>::apply(op.value(), node, ec);
+      }
+   }
+};
+
+template <class T, class String>
 void
 boost_redis_from_bulk(
-   std::basic_string<CharT, Traits, Allocator>& s,
-   std::string_view sv,
-   system::error_code&)
+  T& t,
+  resp3::basic_node<String> const& node,
+  system::error_code& ec)
 {
-  s.append(sv.data(), sv.size());
+   from_bulk_impl<T>::apply(t, node, ec);
 }
 
 //================================================
@@ -138,14 +209,14 @@ class simple_impl {
    void on_value_available(Result&) {}
 
    template <class String>
-   void operator()(Result& result, resp3::basic_node<String> const& n, system::error_code& ec)
+   void operator()(Result& result, resp3::basic_node<String> const& node, system::error_code& ec)
    {
-      if (is_aggregate(n.data_type)) {
+      if (is_aggregate(node.data_type)) {
          ec = redis::error::expects_resp3_simple_type;
          return;
       }
 
-      boost_redis_from_bulk(result, n.value, ec);
+      boost_redis_from_bulk(result, node, ec);
    }
 };
 
@@ -175,7 +246,7 @@ class set_impl {
       }
 
       typename Result::key_type obj;
-      boost_redis_from_bulk(obj, nd.value, ec);
+      boost_redis_from_bulk(obj, nd, ec);
       hint_ = result.insert(hint_, std::move(obj));
    }
 };
@@ -208,11 +279,11 @@ class map_impl {
 
       if (on_key_) {
          typename Result::key_type obj;
-         boost_redis_from_bulk(obj, nd.value, ec);
+         boost_redis_from_bulk(obj, nd, ec);
          current_ = result.insert(current_, {std::move(obj), {}});
       } else {
          typename Result::mapped_type obj;
-         boost_redis_from_bulk(obj, nd.value, ec);
+         boost_redis_from_bulk(obj, nd, ec);
          current_->second = std::move(obj);
       }
 
@@ -233,7 +304,7 @@ class vector_impl {
          result.reserve(result.size() + m * nd.aggregate_size);
       } else {
          result.push_back({});
-         boost_redis_from_bulk(result.back(), nd.value, ec);
+         boost_redis_from_bulk(result.back(), nd, ec);
       }
    }
 };
@@ -266,7 +337,7 @@ class array_impl {
          }
 
          BOOST_ASSERT(nd.aggregate_size == 1);
-         boost_redis_from_bulk(result.at(i_), nd.value, ec);
+         boost_redis_from_bulk(result.at(i_), nd, ec);
       }
 
       ++i_;
@@ -289,7 +360,7 @@ struct list_impl {
         }
 
         result.push_back({});
-        boost_redis_from_bulk(result.back(), nd.value, ec);
+        boost_redis_from_bulk(result.back(), nd, ec);
       }
    }
 };
@@ -340,13 +411,14 @@ struct impl_map<std::deque<T, Allocator>> { using type = list_impl<std::deque<T,
 template <class>
 class wrapper;
 
-template <class Result>
-class wrapper<result<Result>> {
+template <class T>
+class wrapper<result<T>> {
 public:
-   using response_type = result<Result>;
+   using response_type = result<T>;
 private:
    response_type* result_;
-   typename impl_map<Result>::type impl_;
+   typename impl_map<T>::type impl_;
+   bool called_once_ = false;
 
    template <class String>
    bool set_if_resp3_error(resp3::basic_node<String> const& nd) noexcept
@@ -366,7 +438,7 @@ class wrapper<result<Result>> {
    explicit wrapper(response_type* t = nullptr) : result_(t)
    {
       if (result_) {
-         result_->value() = Result{};
+         result_->value() = T{};
          impl_.on_value_available(result_->value());
       }
    }
@@ -379,7 +451,7 @@ class wrapper<result<Result>> {
       if (result_->has_error())
          return;
 
-      if (set_if_resp3_error(nd))
+      if (!std::exchange(called_once_, true) && set_if_resp3_error(nd))
          return;
 
       BOOST_ASSERT(result_);
@@ -395,6 +467,7 @@ class wrapper<result<std::optional<T>>> {
 private:
    response_type* result_;
    typename impl_map<T>::type impl_{};
+   bool called_once_ = false;
 
    template <class String>
    bool set_if_resp3_error(resp3::basic_node<String> const& nd) noexcept
@@ -426,7 +499,7 @@ class wrapper<result<std::optional<T>>> {
       if (set_if_resp3_error(nd))
          return;
 
-      if (nd.data_type == resp3::type::null)
+      if (!std::exchange(called_once_, true) && nd.data_type == resp3::type::null)
          return;
 
       if (!result_->value().has_value()) {
diff --git a/include/boost/redis/resp3/node.hpp b/include/boost/redis/resp3/node.hpp
@@ -59,6 +59,8 @@ auto operator==(basic_node<String> const& a, basic_node<String> const& b)
  */
 using node = basic_node<std::string>;
 
+using node_view = basic_node<std::string_view>;
+
 } // boost::redis::resp3
 
 #endif // BOOST_REDIS_RESP3_NODE_HPP
diff --git a/test/test_low_level_sync_sans_io.cpp b/test/test_low_level_sync_sans_io.cpp
@@ -200,3 +200,38 @@ BOOST_AUTO_TEST_CASE(issue_210_no_nested)
    }
 }
 
+BOOST_AUTO_TEST_CASE(issue_233_array_with_null)
+{
+   try {
+      result<std::vector<std::optional<std::string>>> resp;
+
+      char const* wire = "*3\r\n+one\r\n_\r\n+two\r\n";
+      deserialize(wire, adapt2(resp));
+
+      BOOST_CHECK_EQUAL(resp.value().at(0).value(), "one");
+      BOOST_TEST(!resp.value().at(1).has_value());
+      BOOST_CHECK_EQUAL(resp.value().at(2).value(), "two");
+
+   } catch (std::exception const& e) {
+      std::cerr << e.what() << std::endl;
+      exit(EXIT_FAILURE);
+   }
+}
+
+BOOST_AUTO_TEST_CASE(issue_233_optional_array_with_null)
+{
+   try {
+      result<std::optional<std::vector<std::optional<std::string>>>> resp;
+
+      char const* wire = "*3\r\n+one\r\n_\r\n+two\r\n";
+      deserialize(wire, adapt2(resp));
+
+      BOOST_CHECK_EQUAL(resp.value().value().at(0).value(), "one");
+      BOOST_TEST(!resp.value().value().at(1).has_value());
+      BOOST_CHECK_EQUAL(resp.value().value().at(2).value(), "two");
+
+   } catch (std::exception const& e) {
+      std::cerr << e.what() << std::endl;
+      exit(EXIT_FAILURE);
+   }
+}
