diff --git a/CMakeLists.txt b/CMakeLists.txt index 6b1488a..d8ef79c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -78,8 +78,10 @@ find_package(weaseljson REQUIRED) find_program(GPERF_EXECUTABLE gperf REQUIRED) add_custom_command( OUTPUT ${CMAKE_BINARY_DIR}/json_tokens.cpp - COMMAND ${GPERF_EXECUTABLE} ${CMAKE_SOURCE_DIR}/src/json_tokens.gperf > - ${CMAKE_BINARY_DIR}/json_tokens.cpp + COMMAND + ${GPERF_EXECUTABLE} --class-name=PerfectHash + ${CMAKE_SOURCE_DIR}/src/json_tokens.gperf > + ${CMAKE_BINARY_DIR}/json_tokens.cpp DEPENDS ${CMAKE_SOURCE_DIR}/src/json_tokens.gperf COMMENT "Generating JSON token hash table with gperf") add_custom_target(generate_json_tokens diff --git a/benchmarks/bench_commit_request.cpp b/benchmarks/bench_commit_request.cpp index 4321ea7..c5cd013 100644 --- a/benchmarks/bench_commit_request.cpp +++ b/benchmarks/bench_commit_request.cpp @@ -22,7 +22,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = SIMPLE_JSON; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); @@ -34,7 +34,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = MEDIUM_JSON; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); @@ -46,7 +46,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = COMPLEX_JSON; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); @@ -60,7 +60,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = large_json; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); diff --git a/benchmarks/bench_parser_comparison.cpp b/benchmarks/bench_parser_comparison.cpp index 99c3027..33ee308 100644 --- a/benchmarks/bench_parser_comparison.cpp +++ b/benchmarks/bench_parser_comparison.cpp @@ -469,7 +469,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = SIMPLE_JSON; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); @@ -550,7 +550,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = MEDIUM_JSON; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); @@ -631,7 +631,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = COMPLEX_JSON; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); @@ -715,7 +715,7 @@ int main() { CommitRequest request; JsonCommitRequestParser parser; std::string mutable_json = large_json; - bool result = + auto result = parser.parse(request, mutable_json.data(), mutable_json.size()); ankerl::nanobench::doNotOptimizeAway(result); ankerl::nanobench::doNotOptimizeAway(request.leader_id()); diff --git a/src/json_commit_request_parser.cpp b/src/json_commit_request_parser.cpp index 3e376b0..c8dc960 100644 --- a/src/json_commit_request_parser.cpp +++ b/src/json_commit_request_parser.cpp @@ -449,16 +449,33 @@ void JsonCommitRequestParser::handle_completed_number(std::string_view s) { } } -bool JsonCommitRequestParser::parse(CommitRequest &request, char *data, - size_t len) { +CommitRequestParser::ParseResult +JsonCommitRequestParser::parse(CommitRequest &request, char *data, size_t len) { if (!begin_streaming_parse(request)) { - return false; + return ParseResult::OutOfMemory; } - parse_chunk(data, len); - finish_streaming_parse(); - return !has_parse_error() && !request.leader_id().empty() && - parser_context_->has_read_version_been_set; + ParseStatus status = parse_chunk(data, len); + if (status == ParseStatus::Error) { + return has_parse_error() ? ParseResult::InvalidJson + : ParseResult::InvalidField; + } + + status = finish_streaming_parse(); + if (status == ParseStatus::Error) { + return has_parse_error() ? ParseResult::InvalidJson + : ParseResult::InvalidField; + } + + if (request.leader_id().empty()) { + return ParseResult::MissingField; + } + + if (!parser_context_->has_read_version_been_set) { + return ParseResult::MissingField; + } + + return ParseResult::Success; } bool JsonCommitRequestParser::begin_streaming_parse(CommitRequest &request) { diff --git a/src/json_commit_request_parser.hpp b/src/json_commit_request_parser.hpp index 77ae255..ab31d39 100644 --- a/src/json_commit_request_parser.hpp +++ b/src/json_commit_request_parser.hpp @@ -112,7 +112,7 @@ public: JsonCommitRequestParser &operator=(JsonCommitRequestParser &&other) noexcept; // CommitRequestParser interface implementation - bool parse(CommitRequest &request, char *data, size_t len) override; + ParseResult parse(CommitRequest &request, char *data, size_t len) override; bool begin_streaming_parse(CommitRequest &request) override; ParseStatus parse_chunk(char *data, size_t len) override; ParseStatus finish_streaming_parse() override; diff --git a/src/json_token_enum.hpp b/src/json_token_enum.hpp index fb2d5bb..c587f90 100644 --- a/src/json_token_enum.hpp +++ b/src/json_token_enum.hpp @@ -56,7 +56,7 @@ enum class JsonTokenType { */ inline JsonTokenType get_json_token_type(std::string_view str) { const JsonToken *token = - Perfect_Hash::lookup_json_token(str.data(), str.size()); + PerfectHash::lookup_json_token(str.data(), str.size()); if (token) { return static_cast(token->token_id); } diff --git a/src/json_tokens.hpp b/src/json_tokens.hpp index 158214c..2a2752d 100644 --- a/src/json_tokens.hpp +++ b/src/json_tokens.hpp @@ -26,14 +26,14 @@ struct JsonToken { * * @example * ```cpp - * const JsonToken* token = Perfect_Hash::lookup_json_token("request_id", 10); + * const JsonToken* token = PerfectHash::lookup_json_token("request_id", 10); * if (token) { * JsonTokenType type = static_cast(token->token_id); * // Handle known token... * } * ``` */ -class Perfect_Hash { +class PerfectHash { public: /** * @brief Look up a JSON token by name using perfect hash. diff --git a/src/main.cpp b/src/main.cpp index 624202c..aacf6dd 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -87,10 +87,28 @@ int main(int argc, char *argv[]) { })"; auto copy = sample_json; - if (parser.parse(request, copy.data(), copy.size())) { + auto parse_result = parser.parse(request, copy.data(), copy.size()); + if (parse_result == CommitRequestParser::ParseResult::Success) { print_stats(request); } else { - std::cout << "✗ Failed to parse commit request" << std::endl; + std::cout << "✗ Failed to parse commit request: "; + switch (parse_result) { + case CommitRequestParser::ParseResult::InvalidJson: + std::cout << "Invalid JSON format" << std::endl; + break; + case CommitRequestParser::ParseResult::MissingField: + std::cout << "Missing required field" << std::endl; + break; + case CommitRequestParser::ParseResult::InvalidField: + std::cout << "Invalid field value" << std::endl; + break; + case CommitRequestParser::ParseResult::OutOfMemory: + std::cout << "Out of memory" << std::endl; + break; + default: + std::cout << "Unknown error" << std::endl; + break; + } } // Demonstrate streaming parsing diff --git a/src/parser_interface.hpp b/src/parser_interface.hpp index 8ce5d4f..d97ecbe 100644 --- a/src/parser_interface.hpp +++ b/src/parser_interface.hpp @@ -24,6 +24,17 @@ public: Error ///< Parse error occurred (check get_parse_error() for details) }; + /** + * @brief Result type for one-shot parsing operations. + */ + enum class ParseResult { + Success, ///< Parsing completed successfully + InvalidJson, ///< Invalid JSON format or structure + MissingField, ///< Required field missing from input + InvalidField, ///< Field value is invalid or malformed + OutOfMemory ///< Arena allocation failed + }; + virtual ~CommitRequestParser() = default; /** @@ -31,9 +42,9 @@ public: * @param request The CommitRequest object to populate * @param data Pointer to the data buffer * @param len Length of the data in bytes - * @return true if parsing succeeded, false otherwise + * @return ParseResult indicating success or specific error type */ - virtual bool parse(CommitRequest &request, char *data, size_t len) = 0; + virtual ParseResult parse(CommitRequest &request, char *data, size_t len) = 0; /** * @brief Initialize streaming parsing. diff --git a/tests/test_commit_request.cpp b/tests/test_commit_request.cpp index c1f06eb..dc20007 100644 --- a/tests/test_commit_request.cpp +++ b/tests/test_commit_request.cpp @@ -17,7 +17,8 @@ TEST_CASE("CommitRequest basic parsing") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); REQUIRE(request.request_id().has_value()); REQUIRE(request.request_id().value() == "test123"); REQUIRE(request.leader_id() == "leader456"); @@ -38,7 +39,8 @@ TEST_CASE("CommitRequest basic parsing") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); REQUIRE(request.preconditions().size() == 1); REQUIRE(request.preconditions()[0].type == Precondition::Type::PointRead); REQUIRE(request.preconditions()[0].version == 12340); @@ -64,7 +66,8 @@ TEST_CASE("CommitRequest basic parsing") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); REQUIRE(request.operations().size() == 2); REQUIRE(request.operations()[0].type == Operation::Type::Write); @@ -82,8 +85,9 @@ TEST_CASE("CommitRequest basic parsing") { "read_version": "not_a_number" })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Missing required leader_id") { @@ -92,8 +96,9 @@ TEST_CASE("CommitRequest basic parsing") { "read_version": 12345 })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); // Check completion based on required fields REQUIRE(request.leader_id().empty()); } @@ -104,8 +109,9 @@ TEST_CASE("CommitRequest basic parsing") { "leader_id": "leader456" })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); // Check completion based on required fields REQUIRE(!parser.has_read_version_been_set()); } @@ -117,8 +123,9 @@ TEST_CASE("CommitRequest basic parsing") { "read_version": 12345 })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); // Check completion based on required fields REQUIRE(request.leader_id().empty()); } @@ -128,8 +135,9 @@ TEST_CASE("CommitRequest basic parsing") { "request_id": "test123" })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); // Check completion based on required fields bool missing_leader = request.leader_id().empty(); bool missing_version = !parser.has_read_version_been_set(); @@ -143,7 +151,8 @@ TEST_CASE("CommitRequest basic parsing") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); REQUIRE_FALSE(request.request_id().has_value()); REQUIRE(request.leader_id() == "leader456"); REQUIRE(request.read_version() == 12345); @@ -167,7 +176,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid point_read precondition - missing key") { @@ -181,8 +191,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Valid point_read precondition - empty key") { @@ -198,7 +209,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Valid range_read precondition") { @@ -215,9 +227,10 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - bool parse_result = + auto parse_result = parser.parse(request, const_cast(json.data()), json.size()); - INFO("Parse result: " << parse_result); + INFO("Parse result: " << (parse_result == + CommitRequestParser::ParseResult::Success)); INFO("Parse error: " << parser.has_parse_error()); const char *error_msg = parser.get_parse_error(); INFO("Parse error message: " << (error_msg ? std::string(error_msg) @@ -225,7 +238,7 @@ TEST_CASE("CommitRequest precondition and operation validation") { INFO("Leader ID: '" << request.leader_id() << "'"); INFO("Read version: " << request.read_version()); - REQUIRE(parse_result); + REQUIRE(parse_result == CommitRequestParser::ParseResult::Success); } SUBCASE("Valid range_read precondition - empty begin/end") { @@ -242,7 +255,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid range_read precondition - missing begin") { @@ -257,8 +271,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid range_read precondition - missing end") { @@ -273,8 +288,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Valid write operation") { @@ -291,7 +307,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Valid write operation - empty key and value") { @@ -308,7 +325,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid write operation - missing key") { @@ -323,8 +341,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid write operation - missing value") { @@ -339,8 +358,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Valid delete operation") { @@ -356,7 +376,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid delete operation - missing key") { @@ -370,8 +391,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Valid range_delete operation") { @@ -388,7 +410,8 @@ TEST_CASE("CommitRequest precondition and operation validation") { })"; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid range_delete operation - missing begin") { @@ -403,8 +426,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Invalid range_delete operation - missing end") { @@ -419,8 +443,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } SUBCASE("Mixed valid and invalid operations") { @@ -439,8 +464,9 @@ TEST_CASE("CommitRequest precondition and operation validation") { ] })"; - REQUIRE_FALSE( - parser.parse(request, const_cast(json.data()), json.size())); + REQUIRE( + parser.parse(request, const_cast(json.data()), json.size()) != + CommitRequestParser::ParseResult::Success); } } @@ -461,7 +487,8 @@ TEST_CASE("CommitRequest memory management") { ] })"; - REQUIRE(parser.parse(request, json.data(), json.size())); + REQUIRE(parser.parse(request, json.data(), json.size()) == + CommitRequestParser::ParseResult::Success); // Check that arena allocation worked REQUIRE(request.total_allocated() > 0); @@ -669,7 +696,8 @@ TEST_CASE("CommitRequest arena debug dump") { std::string json = weaseldb::test_data::COMPLEX_JSON; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); // Verify the request was parsed correctly REQUIRE(request.request_id().has_value()); @@ -725,7 +753,8 @@ TEST_CASE("CommitRequest arena debug dump") { // Parse complex JSON std::string json = weaseldb::test_data::COMPLEX_JSON; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); // Debug dump after parsing std::ostringstream used_output; @@ -746,7 +775,8 @@ TEST_CASE("CommitRequest arena debug dump") { // Parse complex JSON first std::string json = weaseldb::test_data::COMPLEX_JSON; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); size_t allocated_before_reset = request.total_allocated(); size_t used_before_reset = request.used_bytes(); @@ -789,7 +819,8 @@ TEST_CASE("CommitRequest arena debug dump") { // Parse COMPLEX_JSON to get diverse content in memory std::string json = weaseldb::test_data::COMPLEX_JSON; REQUIRE( - parser.parse(request, const_cast(json.data()), json.size())); + parser.parse(request, const_cast(json.data()), json.size()) == + CommitRequestParser::ParseResult::Success); // Test different content visualization options std::ostringstream no_content; diff --git a/tools/debug_arena.cpp b/tools/debug_arena.cpp index 031ed38..8f572a8 100644 --- a/tools/debug_arena.cpp +++ b/tools/debug_arena.cpp @@ -265,10 +265,10 @@ int main(int argc, char *argv[]) { std::vector mutable_json(json_content.begin(), json_content.end()); mutable_json.push_back('\0'); // Null terminate for safety - bool parse_success = parser.parse(commit_request, mutable_json.data(), - mutable_json.size() - 1); + auto parse_result = parser.parse(commit_request, mutable_json.data(), + mutable_json.size() - 1); - if (!parse_success) { + if (parse_result != CommitRequestParser::ParseResult::Success) { std::cerr << "Error: Failed to parse JSON" << std::endl; if (parser.has_parse_error()) { std::cerr << "Parse error: " << parser.get_parse_error() << std::endl;