Skip to content

Commit

Permalink
fix: pass the extra argument to event methods
Browse files Browse the repository at this point in the history
  • Loading branch information
@sjinks
sjinks committed Oct 30, 2024
1 parent 5045090 commit 877eafe
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 48 deletions.
10 changes: 5 additions & 5 deletions src/dispatcher.cpp
View file
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
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
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
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);
}

0 comments on commit 877eafe

Please sign in to comment.