From 877eafe138ab5abb7aa71a0a7e25614c7ba3583a Mon Sep 17 00:00:00 2001 From: Volodymyr Kolesnykov Date: Wed, 30 Oct 2024 23:26:36 +0200 Subject: [PATCH] fix: pass the extra argument to event methods --- src/dispatcher.cpp | 10 +++--- src/dispatcher.h | 34 ++++++++++++-------- src/dispatcher_p.cpp | 20 ++++++------ test/test_instrumentation.cpp | 58 ++++++++++++++++++++++------------- 4 files changed, 74 insertions(+), 48 deletions(-) diff --git a/src/dispatcher.cpp b/src/dispatcher.cpp index 7c0c14e..5c4d616 100644 --- a/src/dispatcher.cpp +++ b/src/dispatcher.cpp @@ -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(); } @@ -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 } diff --git a/src/dispatcher.h b/src/dispatcher.h index bc05bbf..2404d98 100644 --- a/src/dispatcher.h +++ b/src/dispatcher.h @@ -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) * { @@ -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 - void add(std::string_view method, F&& f) + inline void add(std::string_view method, F&& f) { this->add(method, std::forward(f), nullptr); } @@ -251,7 +251,7 @@ class WWA_JSONRPC_EXPORT dispatcher { * @see add() */ template - 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), nullptr); } @@ -274,7 +274,7 @@ class WWA_JSONRPC_EXPORT dispatcher { * @see add() */ template - 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>; using ArgsTuple = typename traits::args_tuple; @@ -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. * @@ -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. * @@ -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. @@ -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: /** @@ -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 - 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 && std::is_class_v>) || std::is_null_pointer_v); return [func = std::forward(f), inst](const nlohmann::json& extra, const nlohmann::json& params) { diff --git a/src/dispatcher_p.cpp b/src/dispatcher_p.cpp index 10a46f6..26a21d1 100644 --- a/src/dispatcher_p.cpp +++ b/src/dispatcher_p.cpp @@ -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) ); @@ -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); @@ -135,13 +135,14 @@ 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); @@ -149,7 +150,6 @@ nlohmann::json dispatcher_private::process_request(const nlohmann::json& request method = req.method; request_id = req.id; - nlohmann::json extra_data = extra; if (extra.is_object() && !extra_fields.empty()) { extra_data["extra"] = extra_fields; } @@ -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 @@ -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; } diff --git a/test/test_instrumentation.cpp b/test/test_instrumentation.cpp index 1064764..b10a324 100644 --- a/test/test_instrumentation.cpp +++ b/test/test_instrumentation.cpp @@ -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 { @@ -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); @@ -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); }