-
Notifications
You must be signed in to change notification settings - Fork 43
Generic flat response implementation #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from 7 commits
7e7e3dd
da13036
6971fa6
7b59e28
359c8bc
621367d
1e5803b
3545f89
79e262a
2d866f1
51add31
d580f2e
babbbdd
3ae996d
b228fd5
d8bf31d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,10 +34,13 @@ namespace boost::redis { | |
*/ | ||
class any_adapter { | ||
public: | ||
using fn_type = std::function<void(std::size_t, resp3::node_view const&, system::error_code&)>; | ||
using adapt_fn_type = std::function< | ||
void(std::size_t, resp3::node_view const&, system::error_code&)>; | ||
using done_fn_type = adapter::detail::done_fn_type; | ||
|
||
struct impl_t { | ||
fn_type adapt_fn; | ||
adapt_fn_type adapt_fn; | ||
done_fn_type prepare_done_fn; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to avoid the creation of two There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have been thinking about this and there seems to be a simpler way of avoiding the extra constexpr resp3::node_view done_node{resp3::type::invalid, -1, -1, {}}; then call the adapter with this node when the parser is done here template <class Adapter>
bool parse(resp3::parser& p, std::string_view const& msg, Adapter& adapter, system::error_code& ec)
{
while (!p.done()) {
...
}
// ---> New adapter call here to inform parsing is finished.
adapter(std::make_optional<resp3::parser::result>(done_node), system::error_code());
return true;
} Then call template <>
class general_aggregate<result<flat_response_value>> {
private:
result<flat_response_value>* result_;
public:
template <class String>
void operator()(resp3::basic_node<String> const& nd, system::error_code&)
{
// ---> Check whether done here.
if (nd == done_node) {
result_->value().set_views();
return;
}
...
}
}; I totally missed this possibility earlier but it looks cleaner than adding a new There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, it looks much better and we don't have to care about heap allocations. I will try it! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Am I correct that this case must be handled by others adapters as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @D0zee, yes other adapters will have to handle this as well, for example if (nd == done_node)
return; I forgot to say this in my previous comment. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi again, there is still one problem to solve. The way I suggested above will cause the Or perhaps there is a way to wrap the adapters at other location such as here template <class T>
static auto create_impl(T& resp) -> impl_t
{
using namespace boost::redis::adapter;
auto adapter = boost_redis_adapt(resp);
std::size_t size = adapter.get_supported_response_size();
return {std::move(adapter), size};
} or here adapter_ = [this, adapter](resp3::node_view const& nd, system::error_code& ec) {
auto const i = req_->get_expected_responses() - remaining_responses_;
adapter(i, nd, ec);
}; so that only the last call to If I have time I will review all of this to see if there is any simplification possible. |
||
std::size_t supported_response_size; | ||
} impl_; | ||
|
||
|
@@ -47,7 +50,7 @@ class any_adapter { | |
using namespace boost::redis::adapter; | ||
auto adapter = boost_redis_adapt(resp); | ||
std::size_t size = adapter.get_supported_response_size(); | ||
return {std::move(adapter), size}; | ||
return {std::move(adapter), detail::prepare_done(resp), size}; | ||
} | ||
|
||
template <class Executor> | ||
|
Uh oh!
There was an error while loading. Please reload this page.