From 67ddcd0fc877a7fe8cda348709745630c0047078 Mon Sep 17 00:00:00 2001 From: Andrew Noyes Date: Sun, 17 Aug 2025 14:09:55 -0400 Subject: [PATCH] Remove has_read_version_been_set_ from CommitRequest --- src/commit_request.hpp | 18 ++---------------- src/json_commit_request_parser.cpp | 8 +++++++- src/json_commit_request_parser.hpp | 11 ++++++++++- tests/test_commit_request.cpp | 6 +++--- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/commit_request.hpp b/src/commit_request.hpp index f827835..f50e1ae 100644 --- a/src/commit_request.hpp +++ b/src/commit_request.hpp @@ -3,7 +3,6 @@ #include "arena_allocator.hpp" #include #include -#include #include #include @@ -43,7 +42,6 @@ private: std::optional request_id_; std::string_view leader_id_; uint64_t read_version_ = 0; - bool has_read_version_been_set_ = false; std::vector> preconditions_; std::vector> operations_; @@ -59,7 +57,6 @@ public: CommitRequest(CommitRequest &&other) noexcept : arena_(std::move(other.arena_)), request_id_(other.request_id_), leader_id_(other.leader_id_), read_version_(other.read_version_), - has_read_version_been_set_(other.has_read_version_been_set_), preconditions_(std::move(other.preconditions_)), operations_(std::move(other.operations_)) {} @@ -70,7 +67,6 @@ public: request_id_ = other.request_id_; leader_id_ = other.leader_id_; read_version_ = other.read_version_; - has_read_version_been_set_ = other.has_read_version_been_set_; preconditions_ = std::move(other.preconditions_); operations_ = std::move(other.operations_); } @@ -101,12 +97,6 @@ public: */ uint64_t read_version() const { return read_version_; } - /** - * @brief Check if read version has been explicitly set. - * @return true if read version was set during parsing - */ - bool has_read_version_been_set() const { return has_read_version_been_set_; } - /** * @brief Get the preconditions. * @return span of preconditions @@ -151,7 +141,6 @@ public: request_id_.reset(); leader_id_ = {}; read_version_ = 0; - has_read_version_been_set_ = false; preconditions_.clear(); operations_.clear(); } @@ -166,10 +155,7 @@ public: leader_id_ = arena_allocated_leader_id; } - void set_read_version(uint64_t read_version) { - read_version_ = read_version; - has_read_version_been_set_ = true; - } + void set_read_version(uint64_t read_version) { read_version_ = read_version; } void add_precondition(Precondition::Type type, uint64_t version, std::string_view arena_allocated_begin, @@ -214,4 +200,4 @@ public: } } } -}; \ No newline at end of file +}; diff --git a/src/json_commit_request_parser.cpp b/src/json_commit_request_parser.cpp index 51c7c08..857851e 100644 --- a/src/json_commit_request_parser.cpp +++ b/src/json_commit_request_parser.cpp @@ -423,6 +423,7 @@ void JsonCommitRequestParser::handle_completed_number(std::string_view s) { auto result = std::from_chars(s.data(), s.data() + s.size(), version); if (result.ec == std::errc{}) { current_request_->set_read_version(version); + ctx.has_read_version_been_set = true; } else { ctx.parse_error = "Invalid number format for read_version field"; } @@ -456,7 +457,7 @@ bool JsonCommitRequestParser::parse(CommitRequest &request, char *data, finish_streaming_parse(request); return !has_parse_error() && !request.leader_id().empty() && - request.has_read_version_been_set(); + parser_context_->has_read_version_been_set; } bool JsonCommitRequestParser::begin_streaming_parse(CommitRequest &request) { @@ -469,6 +470,7 @@ bool JsonCommitRequestParser::begin_streaming_parse(CommitRequest &request) { parser_context_->reset_arena_memory(&request.arena()); parser_context_->parse_error = nullptr; parser_context_->parse_complete = false; + parser_context_->has_read_version_been_set = false; } if (json_parser_) { @@ -544,4 +546,8 @@ bool JsonCommitRequestParser::has_parse_error() const { const char *JsonCommitRequestParser::get_parse_error() const { return parser_context_ ? parser_context_->parse_error : nullptr; +} + +bool JsonCommitRequestParser::has_read_version_been_set() const { + return parser_context_ && parser_context_->has_read_version_been_set; } \ No newline at end of file diff --git a/src/json_commit_request_parser.hpp b/src/json_commit_request_parser.hpp index ae5bcf9..5432a63 100644 --- a/src/json_commit_request_parser.hpp +++ b/src/json_commit_request_parser.hpp @@ -58,6 +58,7 @@ private: bool in_key = false; const char *parse_error = nullptr; bool parse_complete = false; + bool has_read_version_been_set = false; // Current objects being parsed PreconditionParseState current_precondition{}; @@ -73,7 +74,9 @@ private: current_string(ArenaStlAllocator(arena)), current_number(ArenaStlAllocator(arena)), precondition_type(ArenaStlAllocator(arena)), - operation_type(ArenaStlAllocator(arena)) {} + operation_type(ArenaStlAllocator(arena)) { + has_read_version_been_set = false; + } void reset_arena_memory(ArenaAllocator *arena) { current_key = ArenaString{ArenaStlAllocator(arena)}; @@ -119,6 +122,12 @@ public: bool has_parse_error() const override; const char *get_parse_error() const override; + /** + * @brief Check if read version has been explicitly set during parsing. + * @return true if read version was set during parsing + */ + bool has_read_version_been_set() const; + // Weaseljson callbacks (public for global callbacks) static void on_begin_object(void *userdata); static void on_end_object(void *userdata); diff --git a/tests/test_commit_request.cpp b/tests/test_commit_request.cpp index 2189d3c..5cb6862 100644 --- a/tests/test_commit_request.cpp +++ b/tests/test_commit_request.cpp @@ -107,7 +107,7 @@ TEST_CASE("CommitRequest basic parsing") { REQUIRE_FALSE( parser.parse(request, const_cast(json.data()), json.size())); // Check completion based on required fields - REQUIRE(!request.has_read_version_been_set()); + REQUIRE(!parser.has_read_version_been_set()); } SUBCASE("Empty leader_id") { @@ -132,7 +132,7 @@ TEST_CASE("CommitRequest basic parsing") { parser.parse(request, const_cast(json.data()), json.size())); // Check completion based on required fields bool missing_leader = request.leader_id().empty(); - bool missing_version = !request.has_read_version_been_set(); + bool missing_version = !parser.has_read_version_been_set(); REQUIRE((missing_leader || missing_version)); } @@ -656,7 +656,7 @@ TEST_CASE("CommitRequest streaming parsing") { REQUIRE(status == CommitRequestParser::ParseStatus::Complete); // Check that required field is missing - REQUIRE(!request.has_read_version_been_set()); + REQUIRE(!parser.has_read_version_been_set()); } }