From 2278694f4f83efda81f3d49b49d7c8f0439d214b Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Thu, 4 Sep 2025 16:36:10 -0400 Subject: [PATCH] Separate out api url parser --- CMakeLists.txt | 12 +++ src/api_url_parser.cpp | 115 +++++++++++++++++++++ src/api_url_parser.hpp | 89 ++++++++++++++++ src/http_handler.cpp | 188 ++++++++++++---------------------- src/http_handler.hpp | 53 ++++------ tests/test_api_url_parser.cpp | 117 +++++++++++++++++++++ tests/test_http_handler.cpp | 62 ----------- 7 files changed, 417 insertions(+), 219 deletions(-) create mode 100644 src/api_url_parser.cpp create mode 100644 src/api_url_parser.hpp create mode 100644 tests/test_api_url_parser.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 59c1223..0132500 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -111,6 +111,7 @@ set(SOURCES src/server.cpp src/json_commit_request_parser.cpp src/http_handler.cpp + src/api_url_parser.cpp src/arena_allocator.cpp src/format.cpp src/metric.cpp @@ -156,6 +157,7 @@ add_executable( test_http_handler tests/test_http_handler.cpp src/http_handler.cpp + src/api_url_parser.cpp src/server.cpp src/config.cpp src/json_commit_request_parser.cpp @@ -189,6 +191,7 @@ add_executable( src/arena_allocator.cpp src/config.cpp src/http_handler.cpp + src/api_url_parser.cpp src/json_commit_request_parser.cpp src/format.cpp src/metric.cpp @@ -287,3 +290,12 @@ add_test(NAME commit_request_benchmarks COMMAND bench_commit_request) add_test(NAME parser_comparison_benchmarks COMMAND bench_parser_comparison) add_test(NAME thread_pipeline_benchmarks COMMAND bench_thread_pipeline) add_test(NAME format_comparison_benchmarks COMMAND bench_format_comparison) + +add_executable(test_api_url_parser tests/test_api_url_parser.cpp + src/api_url_parser.cpp) +target_link_libraries(test_api_url_parser doctest::doctest) +target_include_directories(test_api_url_parser PRIVATE src) +target_compile_definitions(test_api_url_parser + PRIVATE DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN) +target_compile_options(test_api_url_parser PRIVATE -UNDEBUG) +add_test(NAME api_url_parser_tests COMMAND test_api_url_parser) diff --git a/src/api_url_parser.cpp b/src/api_url_parser.cpp new file mode 100644 index 0000000..217683f --- /dev/null +++ b/src/api_url_parser.cpp @@ -0,0 +1,115 @@ +#include "api_url_parser.hpp" + +#include + +namespace { + +// A simplified helper to split a string_view, similar to std::string::find. +// Returns a pair of views, the part before the delimiter and the part after. +std::pair split_view(std::string_view view, + char delimiter) { + size_t pos = view.find(delimiter); + if (pos == std::string_view::npos) { + return {view, ""}; + } + return {view.substr(0, pos), view.substr(pos + 1)}; +} + +// Maps a string parameter key to its corresponding enum value. +// Unrecognized keys are ignored as per the design. +[[nodiscard]] std::optional +to_api_parameter_key(std::string_view key) { + if (key == "request_id") + return ApiParameterKey::RequestId; + if (key == "min_version") + return ApiParameterKey::MinVersion; + if (key == "wait") + return ApiParameterKey::Wait; + if (key == "timeout") + return ApiParameterKey::Timeout; + if (key == "verbose") + return ApiParameterKey::Verbose; + return std::nullopt; +} + +// Parses the query string part of a URL. +void parse_query_string(std::string_view query_string, RouteMatch &match) { + while (!query_string.empty()) { + auto [key_value_pair, rest] = split_view(query_string, '&'); + auto [key, value] = split_view(key_value_pair, '='); + + if (auto key_enum = to_api_parameter_key(key)) { + match.params[static_cast(*key_enum)] = value; + } + + query_string = rest; + } +} + +} // namespace + +RouteMatch ApiUrlParser::parse(std::string_view method, std::string_view url) { + RouteMatch result; + + auto [path, query] = split_view(url, '?'); + parse_query_string(query, result); + + if (method == "GET") { + if (path == "/v1/version") { + result.route = HttpRoute::GetVersion; + return result; + } + if (path == "/v1/subscribe") { + result.route = HttpRoute::GetSubscribe; + return result; + } + if (path == "/v1/status") { + result.route = HttpRoute::GetStatus; + return result; + } + if (path.starts_with("/v1/retention")) { + result.route = HttpRoute::GetRetention; + // Note: This matches both /v1/retention and /v1/retention/{id} + // The handler will need to check for the presence of the PolicyId param. + if (path.length() > 13) { // length of "/v1/retention" + std::string_view policy_id = + path.substr(14); // length of "/v1/retention/" + if (!policy_id.empty()) { + result.params[static_cast(ApiParameterKey::PolicyId)] = + policy_id; + } + } + return result; + } + if (path == "/metrics") { + result.route = HttpRoute::GetMetrics; + return result; + } + if (path == "/ok") { + result.route = HttpRoute::GetOk; + return result; + } + } else if (method == "POST") { + if (path == "/v1/commit") { + result.route = HttpRoute::PostCommit; + return result; + } + } else if (method == "PUT") { + if (path.starts_with("/v1/retention/")) { + result.route = HttpRoute::PutRetention; + std::string_view policy_id = path.substr(14); + result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; + return result; + } + } else if (method == "DELETE") { + if (path.starts_with("/v1/retention/")) { + result.route = HttpRoute::DeleteRetention; + std::string_view policy_id = path.substr(14); + result.params[static_cast(ApiParameterKey::PolicyId)] = policy_id; + return result; + } + } + + result.route = HttpRoute::NotFound; + return result; +} diff --git a/src/api_url_parser.hpp b/src/api_url_parser.hpp new file mode 100644 index 0000000..62a9565 --- /dev/null +++ b/src/api_url_parser.hpp @@ -0,0 +1,89 @@ +#pragma once + +#include +#include +#include + +/** + * @brief Defines all HTTP routes supported by the WeaselDB server. + */ +enum class HttpRoute { + GetVersion, + PostCommit, + GetSubscribe, + GetStatus, + PutRetention, + GetRetention, + DeleteRetention, + GetMetrics, + GetOk, + NotFound +}; + +/** + * @brief Defines unique keys for all known URL and query parameters in the API. + * @note This allows for O(1) lookup of parameter values in a fixed-size array. + */ +enum class ApiParameterKey { + // --- Query Parameters --- + RequestId, + MinVersion, + Wait, + Timeout, + Verbose, + + // --- URL Parameters --- + PolicyId, + + // --- Sentinel for array size --- + Count +}; + +/** + * @brief A fixed-size array for storing parsed parameter values from a URL. + * + * It is indexed by the ApiParameterKey enum. The value is a string_view into + * the original URL string, making lookups allocation-free. + */ +using ApiParameters = std::array, + static_cast(ApiParameterKey::Count)>; + +/** + * @brief Contains the complete, structured result of a successful URL parse. + * + * This struct is the output of the ApiUrlParser and contains everything + * a handler needs to process a request, with no further parsing required. + */ +struct RouteMatch { + /** + * @brief The specific API endpoint that was matched. + */ + HttpRoute route; + + /** + * @brief A fixed-size array containing all parsed URL and query parameters. + */ + ApiParameters params; +}; + +/** + * @brief A parser that matches a URL against the fixed WeaselDB API. + * + * This class provides a single static method to parse a URL and method + * into a structured RouteMatch object. It is designed to be a high-performance, + * allocation-free parser with a simple interface. + */ +class ApiUrlParser { +public: + /** + * @brief Parses a URL and HTTP method against the known WeaselDB API + * endpoints. + * + * @param method The HTTP method of the request. + * @param url The full URL (including query string) of the request. + * @return A RouteMatch struct. If no route is matched, the `route` member + * of the returned struct will be `HttpRoute::NotFound`. + */ + [[nodiscard]] static RouteMatch parse(std::string_view method, + std::string_view url); +}; diff --git a/src/http_handler.cpp b/src/http_handler.cpp index 8b96724..eea461e 100644 --- a/src/http_handler.cpp +++ b/src/http_handler.cpp @@ -5,6 +5,7 @@ #include #include +#include "api_url_parser.hpp" #include "arena_allocator.hpp" #include "format.hpp" #include "json_commit_request_parser.hpp" @@ -34,7 +35,7 @@ auto banned_request_ids_memory_gauge = // HttpConnectionState implementation HttpConnectionState::HttpConnectionState(ArenaAllocator &arena) - : current_header_field_buf(ArenaStlAllocator(&arena)), + : arena(arena), current_header_field_buf(ArenaStlAllocator(&arena)), current_header_value_buf(ArenaStlAllocator(&arena)) { llhttp_settings_init(&settings); @@ -65,7 +66,10 @@ void HttpHandler::on_connection_established(Connection &conn) { void HttpHandler::on_connection_closed(Connection &conn) { // Arena cleanup happens automatically when connection is destroyed auto *state = static_cast(conn.user_data); - state->~HttpConnectionState(); + if (state) { + // ArenaAllocator::Ptr automatically calls destructors + state->~HttpConnectionState(); + } conn.user_data = nullptr; } @@ -88,12 +92,12 @@ void HttpHandler::on_batch_complete( auto *state = static_cast(conn->user_data); // Count commit requests that passed basic validation - if (state->route == HttpRoute::POST_commit && state->commit_request && + if (state->route == HttpRoute::PostCommit && state->commit_request && state->parsing_commit && state->basic_validation_passed) { pipeline_count++; } // Count status requests - else if (state->route == HttpRoute::GET_status && + else if (state->route == HttpRoute::GetStatus && // Error message not already queued conn->outgoing_bytes_queued() == 0) { pipeline_count++; @@ -111,12 +115,12 @@ void HttpHandler::on_batch_complete( auto *state = static_cast(conn->user_data); // Create CommitEntry for commit requests - if (state->route == HttpRoute::POST_commit && state->commit_request && + if (state->route == HttpRoute::PostCommit && state->commit_request && state->parsing_commit && state->basic_validation_passed) { *out_iter++ = CommitEntry{std::move(conn)}; } // Create StatusEntry for status requests - else if (state->route == HttpRoute::GET_status) { + else if (state->route == HttpRoute::GetStatus) { *out_iter++ = StatusEntry{std::move(conn)}; } } @@ -148,87 +152,49 @@ void HttpHandler::on_data_arrived(std::string_view data, // If message is complete, route and handle the request if (state->message_complete) { - // Parse route from method and URL - state->route = parseRoute(state->method, state->url); + auto route_match = ApiUrlParser::parse(state->method, state->url); + state->route = route_match.route; // Route to appropriate handler switch (state->route) { - case HttpRoute::GET_version: - handleGetVersion(*conn_ptr, *state); + case HttpRoute::GetVersion: + handle_get_version(*conn_ptr, *state); break; - case HttpRoute::POST_commit: - handlePostCommit(*conn_ptr, *state); + case HttpRoute::PostCommit: + handle_post_commit(*conn_ptr, *state); break; - case HttpRoute::GET_subscribe: - handleGetSubscribe(*conn_ptr, *state); + case HttpRoute::GetSubscribe: + handle_get_subscribe(*conn_ptr, *state); break; - case HttpRoute::GET_status: - handleGetStatus(*conn_ptr, *state); + case HttpRoute::GetStatus: + handle_get_status(*conn_ptr, *state, route_match); break; - case HttpRoute::PUT_retention: - handlePutRetention(*conn_ptr, *state); + case HttpRoute::PutRetention: + handle_put_retention(*conn_ptr, *state, route_match); break; - case HttpRoute::GET_retention: - handleGetRetention(*conn_ptr, *state); + case HttpRoute::GetRetention: + handle_get_retention(*conn_ptr, *state, route_match); break; - case HttpRoute::DELETE_retention: - handleDeleteRetention(*conn_ptr, *state); + case HttpRoute::DeleteRetention: + handle_delete_retention(*conn_ptr, *state, route_match); break; - case HttpRoute::GET_metrics: - handleGetMetrics(*conn_ptr, *state); + case HttpRoute::GetMetrics: + handle_get_metrics(*conn_ptr, *state); break; - case HttpRoute::GET_ok: - handleGetOk(*conn_ptr, *state); + case HttpRoute::GetOk: + handle_get_ok(*conn_ptr, *state); break; case HttpRoute::NotFound: default: - handleNotFound(*conn_ptr, *state); + handle_not_found(*conn_ptr, *state); break; } } } -HttpRoute HttpHandler::parseRoute(std::string_view method, - std::string_view url) { - // Strip query parameters if present - size_t query_pos = url.find('?'); - if (query_pos != std::string_view::npos) { - url = url.substr(0, query_pos); - } - - // Route based on method and path - if (method == "GET") { - if (url == "/v1/version") - return HttpRoute::GET_version; - if (url == "/v1/subscribe") - return HttpRoute::GET_subscribe; - if (url.starts_with("/v1/status")) - return HttpRoute::GET_status; - if (url.starts_with("/v1/retention")) { - // Check if it's a specific retention policy or list all - return HttpRoute::GET_retention; - } - if (url == "/metrics") - return HttpRoute::GET_metrics; - if (url == "/ok") - return HttpRoute::GET_ok; - } else if (method == "POST") { - if (url == "/v1/commit") - return HttpRoute::POST_commit; - } else if (method == "PUT") { - if (url.starts_with("/v1/retention/")) - return HttpRoute::PUT_retention; - } else if (method == "DELETE") { - if (url.starts_with("/v1/retention/")) - return HttpRoute::DELETE_retention; - } - - return HttpRoute::NotFound; -} - // Route handlers (basic implementations) -void HttpHandler::handleGetVersion(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_get_version(Connection &conn, + const HttpConnectionState &state) { version_counter.inc(); send_json_response( conn, 200, @@ -237,8 +203,8 @@ void HttpHandler::handleGetVersion(Connection &conn, state.connection_close); } -void HttpHandler::handlePostCommit(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_post_commit(Connection &conn, + const HttpConnectionState &state) { commit_counter.inc(); // Check if streaming parse was successful if (!state.commit_request || !state.parsing_commit) { @@ -297,8 +263,8 @@ void HttpHandler::handlePostCommit(Connection &conn, // Response will be sent after 4-stage pipeline processing is complete } -void HttpHandler::handleGetSubscribe(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_get_subscribe(Connection &conn, + const HttpConnectionState &state) { // TODO: Implement subscription streaming send_json_response( conn, 200, @@ -306,88 +272,61 @@ void HttpHandler::handleGetSubscribe(Connection &conn, state.connection_close); } -void HttpHandler::handleGetStatus(Connection &conn, - HttpConnectionState &state) { +void HttpHandler::handle_get_status(Connection &conn, + HttpConnectionState &state, + const RouteMatch &route_match) { status_counter.inc(); // Status requests are processed through the pipeline // Response will be generated in the sequence stage // This handler extracts request_id from query parameters and prepares for // pipeline processing - // Extract request_id from query parameters: - // /v1/status?request_id=&min_version= - std::string_view url = state.url; - - // Find query parameters - size_t query_pos = url.find('?'); - if (query_pos == std::string_view::npos) { - // No query parameters + const auto &request_id = + route_match.params[static_cast(ApiParameterKey::RequestId)]; + if (!request_id) { send_error_response(conn, 400, "Missing required query parameter: request_id", state.connection_close); return; } - std::string_view query_string = url.substr(query_pos + 1); - - // Simple query parameter parsing for request_id - // Look for "request_id=" in the query string - size_t request_id_pos = query_string.find("request_id="); - if (request_id_pos == std::string_view::npos) { - send_error_response(conn, 400, - "Missing required query parameter: request_id", - state.connection_close); - return; - } - - // Extract the request_id value - size_t value_start = request_id_pos + 11; // length of "request_id=" - if (value_start >= query_string.length()) { + if (request_id->empty()) { send_error_response(conn, 400, "Empty request_id parameter", state.connection_close); return; } - // Find the end of the request_id value (next & or end of string) - size_t value_end = query_string.find('&', value_start); - if (value_end == std::string_view::npos) { - value_end = query_string.length(); - } - - state.status_request_id = - query_string.substr(value_start, value_end - value_start); - - if (state.status_request_id.empty()) { - send_error_response(conn, 400, "Empty request_id parameter", - state.connection_close); - return; - } + // Store the request_id in the state for the pipeline + state.status_request_id = *request_id; // Ready for pipeline processing } -void HttpHandler::handlePutRetention(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_put_retention(Connection &conn, + const HttpConnectionState &state, + const RouteMatch &route_match) { // TODO: Parse retention policy from body and store send_json_response(conn, 200, R"({"policy_id":"example","status":"created"})", state.connection_close); } -void HttpHandler::handleGetRetention(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_get_retention(Connection &conn, + const HttpConnectionState &state, + const RouteMatch &route_match) { // TODO: Extract policy_id from URL or return all policies send_json_response(conn, 200, R"({"policies":[]})", state.connection_close); } -void HttpHandler::handleDeleteRetention(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_delete_retention(Connection &conn, + const HttpConnectionState &state, + const RouteMatch &route_match) { // TODO: Extract policy_id from URL and delete send_json_response(conn, 200, R"({"policy_id":"example","status":"deleted"})", state.connection_close); } -void HttpHandler::handleGetMetrics(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_get_metrics(Connection &conn, + const HttpConnectionState &state) { metrics_counter.inc(); ArenaAllocator &arena = conn.get_arena(); auto metrics_span = metric::render(arena); @@ -428,15 +367,15 @@ void HttpHandler::handleGetMetrics(Connection &conn, } } -void HttpHandler::handleGetOk(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_get_ok(Connection &conn, + const HttpConnectionState &state) { TRACE_EVENT("http", "GET /ok", perfetto::Flow::Global(state.http_request_id)); sendResponse(conn, 200, "text/plain", "OK", state.connection_close); } -void HttpHandler::handleNotFound(Connection &conn, - const HttpConnectionState &state) { +void HttpHandler::handle_not_found(Connection &conn, + const HttpConnectionState &state) { send_error_response(conn, 404, "Not found", state.connection_close); } @@ -589,8 +528,9 @@ int HttpHandler::onHeadersComplete(llhttp_t *parser) { // parser if (state->method == "POST" && state->url.find("/v1/commit") == 0) { // Initialize streaming commit request parsing - state->commit_parser = std::make_unique(); - state->commit_request = std::make_unique(); + state->commit_parser = state->arena.construct(); + state->commit_request = state->arena.construct(); + state->parsing_commit = state->commit_parser->begin_streaming_parse(*state->commit_request); diff --git a/src/http_handler.hpp b/src/http_handler.hpp index b2ac75b..6a33a7a 100644 --- a/src/http_handler.hpp +++ b/src/http_handler.hpp @@ -8,6 +8,7 @@ #include +#include "api_url_parser.hpp" #include "arena_allocator.hpp" #include "connection.hpp" #include "connection_handler.hpp" @@ -20,29 +21,14 @@ // Forward declarations struct CommitRequest; struct JsonCommitRequestParser; - -/** - * HTTP routes supported by WeaselDB server. - * Using enum for efficient switch-based routing. - */ -enum class HttpRoute { - GET_version, - POST_commit, - GET_subscribe, - GET_status, - PUT_retention, - GET_retention, - DELETE_retention, - GET_metrics, - GET_ok, - NotFound -}; +struct RouteMatch; /** * HTTP connection state stored in Connection::user_data. * Manages llhttp parser state and request data. */ struct HttpConnectionState { + ArenaAllocator &arena; llhttp_t parser; llhttp_settings_t settings; @@ -70,8 +56,8 @@ struct HttpConnectionState { 0; // X-Request-Id header value (for tracing/logging) // Streaming parser for POST requests - std::unique_ptr commit_parser; - std::unique_ptr commit_request; + ArenaAllocator::Ptr commit_parser; + ArenaAllocator::Ptr commit_request; bool parsing_commit = false; bool basic_validation_passed = false; // Set to true if basic validation passes @@ -154,9 +140,6 @@ struct HttpHandler : ConnectionHandler { void on_batch_complete( std::span> /*batch*/) override; - // Route parsing (public for testing) - static HttpRoute parseRoute(std::string_view method, std::string_view url); - // llhttp callbacks (public for HttpConnectionState access) static int onUrl(llhttp_t *parser, const char *at, size_t length); static int onHeaderField(llhttp_t *parser, const char *at, size_t length); @@ -208,17 +191,21 @@ private: bool process_release_batch(BatchType &batch); // Route handlers - void handleGetVersion(Connection &conn, const HttpConnectionState &state); - void handlePostCommit(Connection &conn, const HttpConnectionState &state); - void handleGetSubscribe(Connection &conn, const HttpConnectionState &state); - void handleGetStatus(Connection &conn, HttpConnectionState &state); - void handlePutRetention(Connection &conn, const HttpConnectionState &state); - void handleGetRetention(Connection &conn, const HttpConnectionState &state); - void handleDeleteRetention(Connection &conn, - const HttpConnectionState &state); - void handleGetMetrics(Connection &conn, const HttpConnectionState &state); - void handleGetOk(Connection &conn, const HttpConnectionState &state); - void handleNotFound(Connection &conn, const HttpConnectionState &state); + void handle_get_version(Connection &conn, const HttpConnectionState &state); + void handle_post_commit(Connection &conn, const HttpConnectionState &state); + void handle_get_subscribe(Connection &conn, const HttpConnectionState &state); + void handle_get_status(Connection &conn, HttpConnectionState &state, + const RouteMatch &route_match); + void handle_put_retention(Connection &conn, const HttpConnectionState &state, + const RouteMatch &route_match); + void handle_get_retention(Connection &conn, const HttpConnectionState &state, + const RouteMatch &route_match); + void handle_delete_retention(Connection &conn, + const HttpConnectionState &state, + const RouteMatch &route_match); + void handle_get_metrics(Connection &conn, const HttpConnectionState &state); + void handle_get_ok(Connection &conn, const HttpConnectionState &state); + void handle_not_found(Connection &conn, const HttpConnectionState &state); // HTTP utilities static void sendResponse(Connection &conn, int status_code, diff --git a/tests/test_api_url_parser.cpp b/tests/test_api_url_parser.cpp new file mode 100644 index 0000000..ce5dab9 --- /dev/null +++ b/tests/test_api_url_parser.cpp @@ -0,0 +1,117 @@ +#include + +#include "api_url_parser.hpp" + +TEST_CASE("ApiUrlParser routing") { + SUBCASE("Static GET routes") { + auto match = ApiUrlParser::parse("GET", "/v1/version"); + CHECK(match.route == HttpRoute::GetVersion); + + match = ApiUrlParser::parse("GET", "/v1/subscribe"); + CHECK(match.route == HttpRoute::GetSubscribe); + + match = ApiUrlParser::parse("GET", "/metrics"); + CHECK(match.route == HttpRoute::GetMetrics); + + match = ApiUrlParser::parse("GET", "/ok"); + CHECK(match.route == HttpRoute::GetOk); + } + + SUBCASE("Static POST routes") { + auto match = ApiUrlParser::parse("POST", "/v1/commit"); + CHECK(match.route == HttpRoute::PostCommit); + } + + SUBCASE("Not found") { + auto match = ApiUrlParser::parse("GET", "/unknown/route"); + CHECK(match.route == HttpRoute::NotFound); + + match = ApiUrlParser::parse("DELETE", "/v1/version"); + CHECK(match.route == HttpRoute::NotFound); + } +} + +TEST_CASE("ApiUrlParser with query strings") { + SUBCASE("Simple query string") { + auto match = ApiUrlParser::parse("GET", "/v1/status?request_id=123"); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE(match.params[static_cast(ApiParameterKey::RequestId)] + .has_value()); + CHECK( + match.params[static_cast(ApiParameterKey::RequestId)].value() == + "123"); + } + + SUBCASE("Multiple query parameters") { + auto match = + ApiUrlParser::parse("GET", "/v1/status?request_id=abc&min_version=42"); + CHECK(match.route == HttpRoute::GetStatus); + REQUIRE(match.params[static_cast(ApiParameterKey::RequestId)] + .has_value()); + CHECK( + match.params[static_cast(ApiParameterKey::RequestId)].value() == + "abc"); + REQUIRE(match.params[static_cast(ApiParameterKey::MinVersion)] + .has_value()); + CHECK(match.params[static_cast(ApiParameterKey::MinVersion)] + .value() == "42"); + } + + SUBCASE("Unknown parameters are ignored") { + auto match = ApiUrlParser::parse("GET", "/v1/version?foo=bar&baz=quux"); + CHECK(match.route == HttpRoute::GetVersion); + CHECK_FALSE(match.params[static_cast(ApiParameterKey::RequestId)] + .has_value()); + } +} + +TEST_CASE("ApiUrlParser with URL parameters") { + SUBCASE("PUT retention policy") { + auto match = ApiUrlParser::parse("PUT", "/v1/retention/my-policy"); + CHECK(match.route == HttpRoute::PutRetention); + REQUIRE(match.params[static_cast(ApiParameterKey::PolicyId)] + .has_value()); + CHECK( + match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "my-policy"); + } + + SUBCASE("DELETE retention policy") { + auto match = ApiUrlParser::parse("DELETE", "/v1/retention/another-policy"); + CHECK(match.route == HttpRoute::DeleteRetention); + REQUIRE(match.params[static_cast(ApiParameterKey::PolicyId)] + .has_value()); + CHECK( + match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "another-policy"); + } + + SUBCASE("GET retention policy") { + auto match = ApiUrlParser::parse("GET", "/v1/retention/get-this"); + CHECK(match.route == HttpRoute::GetRetention); + REQUIRE(match.params[static_cast(ApiParameterKey::PolicyId)] + .has_value()); + CHECK( + match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "get-this"); + } + + SUBCASE("GET all retention policies (no ID)") { + auto match = ApiUrlParser::parse("GET", "/v1/retention"); + CHECK(match.route == HttpRoute::GetRetention); + CHECK_FALSE(match.params[static_cast(ApiParameterKey::PolicyId)] + .has_value()); + } +} + +TEST_CASE("ApiUrlParser with URL and query parameters") { + auto match = ApiUrlParser::parse("DELETE", "/v1/retention/p1?wait=true"); + CHECK(match.route == HttpRoute::DeleteRetention); + REQUIRE( + match.params[static_cast(ApiParameterKey::PolicyId)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::PolicyId)].value() == + "p1"); + REQUIRE(match.params[static_cast(ApiParameterKey::Wait)].has_value()); + CHECK(match.params[static_cast(ApiParameterKey::Wait)].value() == + "true"); +} diff --git a/tests/test_http_handler.cpp b/tests/test_http_handler.cpp index 530c0d8..b80dae4 100644 --- a/tests/test_http_handler.cpp +++ b/tests/test_http_handler.cpp @@ -27,68 +27,6 @@ struct TestConnectionData { } }; -TEST_CASE("HttpHandler route parsing") { - SUBCASE("GET routes") { - CHECK(HttpHandler::parseRoute("GET", "/v1/version") == - HttpRoute::GET_version); - CHECK(HttpHandler::parseRoute("GET", "/v1/subscribe") == - HttpRoute::GET_subscribe); - CHECK(HttpHandler::parseRoute("GET", "/v1/status") == - HttpRoute::GET_status); - CHECK(HttpHandler::parseRoute("GET", "/v1/retention") == - HttpRoute::GET_retention); - CHECK(HttpHandler::parseRoute("GET", "/metrics") == HttpRoute::GET_metrics); - CHECK(HttpHandler::parseRoute("GET", "/ok") == HttpRoute::GET_ok); - } - - SUBCASE("POST routes") { - CHECK(HttpHandler::parseRoute("POST", "/v1/commit") == - HttpRoute::POST_commit); - } - - SUBCASE("PUT routes") { - CHECK(HttpHandler::parseRoute("PUT", "/v1/retention/policy1") == - HttpRoute::PUT_retention); - } - - SUBCASE("DELETE routes") { - CHECK(HttpHandler::parseRoute("DELETE", "/v1/retention/policy1") == - HttpRoute::DELETE_retention); - } - - SUBCASE("Unknown routes") { - CHECK(HttpHandler::parseRoute("GET", "/unknown") == HttpRoute::NotFound); - CHECK(HttpHandler::parseRoute("PATCH", "/v1/version") == - HttpRoute::NotFound); - } - - SUBCASE("Query parameters stripped") { - CHECK(HttpHandler::parseRoute("GET", "/v1/version?foo=bar") == - HttpRoute::GET_version); - } -} - -TEST_CASE("HttpHandler route parsing edge cases") { - // Test just the static route parsing method since full integration testing - // would require complex Connection setup with server dependencies - - SUBCASE("Route parsing with query parameters") { - CHECK(HttpHandler::parseRoute("GET", "/v1/version?param=value") == - HttpRoute::GET_version); - CHECK(HttpHandler::parseRoute("GET", "/v1/subscribe?stream=true") == - HttpRoute::GET_subscribe); - } - - SUBCASE("Retention policy routes") { - CHECK(HttpHandler::parseRoute("PUT", "/v1/retention/policy123") == - HttpRoute::PUT_retention); - CHECK(HttpHandler::parseRoute("DELETE", "/v1/retention/policy456") == - HttpRoute::DELETE_retention); - CHECK(HttpHandler::parseRoute("GET", "/v1/retention/policy789") == - HttpRoute::GET_retention); - } -} - // Test helper to verify the new hook functionality struct MockConnectionHandler : public ConnectionHandler { bool write_progress_called = false;