Skip to content

fix: pass the extra argument to event methods #5

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Oct 30, 2024
Merged

fix: pass the extra argument to event methods #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/dispatcher.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ std::string dispatcher::parse_and_process_request(const std::string& request, co
req = nlohmann::json::parse(request);
}
catch (const nlohmann::json::exception& e) {
this->on_request();
this->on_request(extra);
const auto json = dispatcher_private::generate_error_response(
exception(exception::PARSE_ERROR, e.what()), nlohmann::json(nullptr)
);

this->on_request_processed({}, exception::PARSE_ERROR);
this->on_request_processed({}, exception::PARSE_ERROR, extra);
return json.dump();
}

Expand All @@ -43,17 +43,17 @@ std::string dispatcher::process_request(const nlohmann::json& request, const nlo
return json.is_discarded() ? std::string{} : json.dump();
}

void dispatcher::on_request()
void dispatcher::on_request(const nlohmann::json&)
{
// Do nothing
}

void dispatcher::on_method(const std::string&)
void dispatcher::on_method(const std::string&, const nlohmann::json&)
{
// Do nothing
}

void dispatcher::on_request_processed(const std::string&, int)
void dispatcher::on_request_processed(const std::string&, int, const nlohmann::json&)
{
// Do nothing
}
Expand Down
34 changes: 22 additions & 12 deletions src/dispatcher.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ class WWA_JSONRPC_EXPORT dispatcher {
* return std::accumulate(v.begin(), v.end(), 0);
* });
* ```
* @note If the handler accepts a single `nlohmann::json` argument, it will *any* parameters. For example:
* @note If the handler accepts a single `nlohmann::json` argument, it will accept *any* parameters. For example:
* ```cpp
* void handler(const nlohmann::json& params)
* {
Expand All @@ -184,12 +184,12 @@ class WWA_JSONRPC_EXPORT dispatcher {
*
* @par Exception Handling:
* If the hander function throws an exception (derived from `std::exception`), the exception will be caught, and the error will be returned in the JSON response:
* 1. json_rpc::exception will be converted to a JSON RPC error object using json_rpc::exception::to_json();
* 2. other exceptions derived from std::exception will be converted to a JSON RPC error object with code @a -32603 (exception::INTERNAL_ERROR)
* 1. `json_rpc::exception` will be converted to a JSON RPC error object using json_rpc::exception::to_json();
* 2. other exceptions derived from `std::exception` will be converted to a JSON RPC error object with code @a -32603 (`exception::INTERNAL_ERROR`)
* and the exception message ([what()](https://en.cppreference.com/w/cpp/error/exception/what)) as the error message.
*/
template<typename F>
void add(std::string_view method, F&& f)
inline void add(std::string_view method, F&& f)
{
this->add(method, std::forward<F>(f), nullptr);
}
Expand Down Expand Up @@ -251,7 +251,7 @@ class WWA_JSONRPC_EXPORT dispatcher {
* @see add()
*/
template<typename F>
void add_ex(std::string_view method, F&& f)
inline void add_ex(std::string_view method, F&& f)
{
this->add_ex(method, std::forward<F>(f), nullptr);
}
Expand All @@ -274,7 +274,7 @@ class WWA_JSONRPC_EXPORT dispatcher {
* @see add()
*/
template<typename C, typename F>
void add_ex(std::string_view method, F&& f, C instance)
inline void add_ex(std::string_view method, F&& f, C instance)
{
using traits = details::function_traits<std::decay_t<F>>;
using ArgsTuple = typename traits::args_tuple;
Expand All @@ -293,7 +293,7 @@ class WWA_JSONRPC_EXPORT dispatcher {
* @brief Parses and processes a JSON RPC request.
*
* @param request The JSON RPC request as a string.
* @param extra An optional JSON object that can be passed to the handler function (only for handlers added with add_ex()).
* @param extra Optional data that can be passed to the handler function (only for handlers added with add_ex()).
* @return The response serialized into a JSON string.
* @retval "" If the request is a [Notification](https://www.jsonrpc.org/specification#notification), the method returns an empty string.
*
Expand All @@ -318,7 +318,7 @@ class WWA_JSONRPC_EXPORT dispatcher {
* @brief Processes a JSON RPC request.
*
* @param request The JSON RPC request as a `nlohmann::json` object.
* @param extra An optional JSON object that can be passed to the handler function (only for handlers added with add_ex()).
* @param extra Optional data that can be passed to the handler function (only for handlers added with add_ex()).
* @return The response serialized into a JSON string.
* @retval "" If the request is a [Notification](https://www.jsonrpc.org/specification#notification), the method returns an empty string.
*
Expand Down Expand Up @@ -365,20 +365,27 @@ class WWA_JSONRPC_EXPORT dispatcher {
* @details This method does nothing by default. It is intended to be overridden in a derived class.
* For example, it can be used to log requests or increment a counter.
*
* @param extra Additional information that was passed to `process_request()`.
* Since `on_request()` is called before the request is parsed, the @a extra` parameter will not contain fields
* from the request itself (i.e., @a `extra['extra']` will not be set).
*
* @note In the case of a valid [batch request](https://www.jsonrpc.org/specification#batch),
* this method is invoked for every request in the batch but **not** for the batch itself.
* However, if the batch request is invalid (e.g., is empty), this method is invoked once with an empty method name.
*/
virtual void on_request();
virtual void on_request(const nlohmann::json& extra);

/**
* @brief Invoked right before the method handler is called.
*
* @details This method does nothing by default. It is intended to be overridden in a derived class. For example, it can start a timer to measure the method execution time.
*
* @param extra Additional information that was passed to `process_request()`. If @a extra is a JSON object,
* it will contain all extra fields from the JSON RPC request (in @a extra['extra'], see `process_request()`).
*
* @param method The name of the method to be called.
*/
virtual void on_method(const std::string& method);
virtual void on_method(const std::string& method, const nlohmann::json& extra);

/**
* @brief Invoked after the method handler is called.
Expand All @@ -390,8 +397,11 @@ class WWA_JSONRPC_EXPORT dispatcher {
* @param code The result code: 0 if the method was processed successfully, or an error code
* if an exception was thrown (e.g., exception::INTERNAL_ERROR)
* or the request could not be processed (e.g., exception::INVALID_PARAMS).
* @param extra Additional information that was passed to `process_request()`. If the request had already been successfully parsed
* before `on_request_processed()` was called, the @a extra parameter will contain all extra fields from the JSON RPC request
* (in @a extra['extra'], see `process_request()`).
*/
virtual void on_request_processed(const std::string& method, int code);
virtual void on_request_processed(const std::string& method, int code, const nlohmann::json& extra);

private:
/**
Expand Down Expand Up @@ -442,7 +452,7 @@ class WWA_JSONRPC_EXPORT dispatcher {
* This helps catch potential errors early in the development process and improves the overall robustness of the code.
*/
template<typename C, typename F, typename Extra, typename Args>
constexpr auto create_closure(C inst, F&& f) const
inline constexpr auto create_closure(C inst, F&& f) const
{
static_assert((std::is_pointer_v<C> && std::is_class_v<std::remove_pointer_t<C>>) || std::is_null_pointer_v<C>);
return [func = std::forward<F>(f), inst](const nlohmann::json& extra, const nlohmann::json& params) {
Expand Down
20 changes: 10 additions & 10 deletions src/dispatcher_p.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ jsonrpc_request dispatcher_private::parse_request(const nlohmann::json& request,
nlohmann::json dispatcher_private::process_batch_request(const nlohmann::json& request, const nlohmann::json& extra)
{
if (request.empty()) {
this->q_ptr->on_request();
this->q_ptr->on_request_processed({}, exception::INVALID_REQUEST);
this->q_ptr->on_request(extra);
this->q_ptr->on_request_processed({}, exception::INVALID_REQUEST, extra);
return dispatcher_private::generate_error_response(
exception(exception::INVALID_REQUEST, err_empty_batch), nlohmann::json(nullptr)
);
Expand All @@ -112,13 +112,13 @@ nlohmann::json dispatcher_private::process_batch_request(const nlohmann::json& r
auto response = nlohmann::json::array();
for (const auto& req : request) {
if (!req.is_object()) {
this->q_ptr->on_request();
this->q_ptr->on_request(extra);
const auto r = dispatcher_private::generate_error_response(
exception(exception::INVALID_REQUEST, err_not_jsonrpc_2_0_request), nlohmann::json(nullptr)
);

response.push_back(r);
this->q_ptr->on_request_processed({}, exception::INVALID_REQUEST);
this->q_ptr->on_request_processed({}, exception::INVALID_REQUEST, extra);
}
else if (const auto res = this->process_request(req, extra); !res.is_discarded()) {
response.push_back(res);
Expand All @@ -135,21 +135,21 @@ nlohmann::json dispatcher_private::process_request(const nlohmann::json& request
return this->process_batch_request(request, extra);
}

this->q_ptr->on_request();
this->q_ptr->on_request(extra);
auto request_id = request.contains("id") ? request["id"] : nlohmann::json(nullptr);
if (!is_valid_request_id(request_id)) {
request_id = nlohmann::json(nullptr);
}

std::string method;
nlohmann::json extra_data = extra;
try {
nlohmann::json extra_fields;
auto req = dispatcher_private::parse_request(request, extra_fields);
dispatcher_private::validate_request(req);
method = req.method;
request_id = req.id;

nlohmann::json extra_data = extra;
if (extra.is_object() && !extra_fields.empty()) {
extra_data["extra"] = extra_fields;
}
Expand All @@ -163,12 +163,12 @@ nlohmann::json dispatcher_private::process_request(const nlohmann::json& request
return nlohmann::json(nlohmann::json::value_t::discarded);
}
catch (const exception& e) {
this->q_ptr->on_request_processed(method, e.code());
this->q_ptr->on_request_processed(method, e.code(), extra_data);
return request_id.is_discarded() ? nlohmann::json(nlohmann::json::value_t::discarded)
: dispatcher_private::generate_error_response(e, request_id);
}
catch (const std::exception& e) {
this->q_ptr->on_request_processed(method, exception::INTERNAL_ERROR);
this->q_ptr->on_request_processed(method, exception::INTERNAL_ERROR, extra_data);
return request_id.is_discarded() ? nlohmann::json(nlohmann::json::value_t::discarded)
: dispatcher_private::generate_error_response(
exception(exception::INTERNAL_ERROR, e.what()), request_id
Expand Down Expand Up @@ -204,9 +204,9 @@ nlohmann::json
dispatcher_private::invoke(const std::string& method, const nlohmann::json& params, const nlohmann::json& extra)
{
if (const auto it = this->m_methods.find(method); it != this->m_methods.end()) {
this->q_ptr->on_method(method);
this->q_ptr->on_method(method, extra);
const auto response = it->second(extra, params);
this->q_ptr->on_request_processed(method, 0);
this->q_ptr->on_request_processed(method, 0, extra);
return response;
}

Expand Down
58 changes: 37 additions & 21 deletions test/test_instrumentation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

class mocked_dispatcher : public wwa::json_rpc::dispatcher {
public:
MOCK_METHOD(void, on_request, (), (override));
MOCK_METHOD(void, on_method, (const std::string&), (override));
MOCK_METHOD(void, on_request_processed, (const std::string&, int), (override));
MOCK_METHOD(void, on_request, (const nlohmann::json&), (override));
MOCK_METHOD(void, on_method, (const std::string&, const nlohmann::json&), (override));
MOCK_METHOD(void, on_request_processed, (const std::string&, int, const nlohmann::json&), (override));
};

class InstrumentationTest : public ::testing::Test {
Expand All @@ -34,8 +34,11 @@ TEST_F(InstrumentationTest, BadRequest)

{
const testing::InSequence s;
EXPECT_CALL(dispatcher, on_request());
EXPECT_CALL(dispatcher, on_request_processed(std::string{}, wwa::json_rpc::exception::INVALID_REQUEST));
EXPECT_CALL(dispatcher, on_request(nlohmann::json::object()));
EXPECT_CALL(
dispatcher,
on_request_processed(std::string{}, wwa::json_rpc::exception::INVALID_REQUEST, nlohmann::json::object())
);
}

dispatcher.process_request(input);
Expand All @@ -44,30 +47,43 @@ TEST_F(InstrumentationTest, BadRequest)
TEST_F(InstrumentationTest, BatchRequest)
{
const auto input = R"([
{"jsonrpc": "2.0", "method": "add", "params": [1, 2], "id": 1},
{"jsonrpc": "2.0", "method": "subtract", "params": [2, 1], "id": 2},
{"jsonrpc": "2.0", "method": "add", "params": ["2", "3"], "id": 3},
{"jsonrpc": "2.0", "method": "bad", "id": 4}
{"jsonrpc": "2.0", "method": "add", "params": [1, 2], "id": 1, "extra": "extra1" },
{"jsonrpc": "2.0", "method": "subtract", "params": [2, 1], "id": 2, "extra": "extra2"},
{"jsonrpc": "2.0", "method": "add", "params": ["2", "3"], "id": 3, "extra": "extra3"},
{"jsonrpc": "2.0", "method": "bad", "id": 4, "extra": "extra4"},
{"extra": "extra5"}
])"_json;
auto& dispatcher = this->dispatcher();

const auto extra_data = R"({"ip": "127.0.0.1"})"_json;
const auto expected_data_1 = nlohmann::json({{"ip", "127.0.0.1"}, {"extra", {{"extra", "extra1"}}}});
const auto expected_data_2 = nlohmann::json({{"ip", "127.0.0.1"}, {"extra", {{"extra", "extra2"}}}});
const auto expected_data_3 = nlohmann::json({{"ip", "127.0.0.1"}, {"extra", {{"extra", "extra3"}}}});
const auto expected_data_4 = nlohmann::json({{"ip", "127.0.0.1"}, {"extra", {{"extra", "extra4"}}}});
const auto& expected_data_5 = extra_data;

{
const testing::InSequence s;
EXPECT_CALL(dispatcher, on_request());
EXPECT_CALL(dispatcher, on_method("add"));
EXPECT_CALL(dispatcher, on_request_processed("add", 0));
EXPECT_CALL(dispatcher, on_request(extra_data));
EXPECT_CALL(dispatcher, on_method("add", expected_data_1));
EXPECT_CALL(dispatcher, on_request_processed("add", 0, expected_data_1));

EXPECT_CALL(dispatcher, on_request(extra_data));
EXPECT_CALL(dispatcher, on_method("subtract", expected_data_2));
EXPECT_CALL(dispatcher, on_request_processed("subtract", 0, expected_data_2));

EXPECT_CALL(dispatcher, on_request());
EXPECT_CALL(dispatcher, on_method("subtract"));
EXPECT_CALL(dispatcher, on_request_processed("subtract", 0));
EXPECT_CALL(dispatcher, on_request(extra_data));
EXPECT_CALL(dispatcher, on_method("add", expected_data_3));
EXPECT_CALL(dispatcher, on_request_processed("add", wwa::json_rpc::exception::INVALID_PARAMS, expected_data_3));

EXPECT_CALL(dispatcher, on_request());
EXPECT_CALL(dispatcher, on_method("add"));
EXPECT_CALL(dispatcher, on_request_processed("add", wwa::json_rpc::exception::INVALID_PARAMS));
EXPECT_CALL(dispatcher, on_request(extra_data));
EXPECT_CALL(
dispatcher, on_request_processed("bad", wwa::json_rpc::exception::METHOD_NOT_FOUND, expected_data_4)
);

EXPECT_CALL(dispatcher, on_request());
EXPECT_CALL(dispatcher, on_request_processed("bad", wwa::json_rpc::exception::METHOD_NOT_FOUND));
EXPECT_CALL(dispatcher, on_request(extra_data));
EXPECT_CALL(dispatcher, on_request_processed("", wwa::json_rpc::exception::INVALID_REQUEST, expected_data_5));
}

dispatcher.process_request(input);
dispatcher.process_request(input, extra_data);
}
Loading