diff --git a/src/commit_request.cpp b/src/commit_request.cpp index 3940f09..344122b 100644 --- a/src/commit_request.cpp +++ b/src/commit_request.cpp @@ -332,6 +332,7 @@ void CommitRequest::handle_completed_number() { ctx.current_number.data() + ctx.current_number.size(), version); if (result.ec == std::errc{}) { read_version_ = version; + has_read_version_been_set_ = true; } else { ctx.parse_error = true; } @@ -379,7 +380,8 @@ bool CommitRequest::parse_json(std::string_view json_str) { status = WeaselJsonParser_parse(parser, nullptr, 0); } - bool success = (status == WeaselJson_OK) && !parser_context_.parse_error; + bool success = + (status == WeaselJson_OK) && !parser_context_.parse_error && is_valid(); WeaselJsonParser_destroy(parser); diff --git a/src/commit_request.hpp b/src/commit_request.hpp index 3ff4491..5ad51d1 100644 --- a/src/commit_request.hpp +++ b/src/commit_request.hpp @@ -100,6 +100,7 @@ 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_; ParserContext parser_context_; @@ -131,6 +132,7 @@ 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_)), parser_context_(std::move(other.parser_context_)), @@ -149,6 +151,7 @@ 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_); parser_context_ = std::move(other.parser_context_); @@ -195,7 +198,16 @@ public: * @return true if parsing is complete and successful */ bool is_parse_complete() const { - return parser_context_.parse_complete && !parser_context_.parse_error; + return parser_context_.parse_complete && !parser_context_.parse_error && + is_valid(); + } + + /** + * @brief Validate that all required fields are present. + * @return true if all required fields are present + */ + bool is_valid() const { + return !leader_id_.empty() && has_read_version_been_set_; } /** @@ -256,6 +268,7 @@ public: request_id_.reset(); leader_id_ = {}; read_version_ = 0; + has_read_version_been_set_ = false; preconditions_.clear(); operations_.clear(); diff --git a/tests/test_commit_request.cpp b/tests/test_commit_request.cpp index d3987b2..df5fd6a 100644 --- a/tests/test_commit_request.cpp +++ b/tests/test_commit_request.cpp @@ -77,6 +77,59 @@ TEST_CASE("CommitRequest basic parsing") { REQUIRE_FALSE(request.parse_json(json)); } + + SUBCASE("Missing required leader_id") { + std::string json = R"({ + "request_id": "test123", + "read_version": 12345 + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Missing required read_version") { + std::string json = R"({ + "request_id": "test123", + "leader_id": "leader456" + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Empty leader_id") { + std::string json = R"({ + "request_id": "test123", + "leader_id": "", + "read_version": 12345 + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Missing both leader_id and read_version") { + std::string json = R"({ + "request_id": "test123" + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("request_id is optional") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345 + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + REQUIRE_FALSE(request.request_id().has_value()); + REQUIRE(request.leader_id() == "leader456"); + REQUIRE(request.read_version() == 12345); + } } TEST_CASE("CommitRequest memory management") { @@ -254,4 +307,38 @@ TEST_CASE("CommitRequest streaming parsing") { REQUIRE(request.leader_id() == "test"); REQUIRE(request.read_version() == 123); } + + SUBCASE("Streaming parse missing required leader_id") { + std::string json = R"({"request_id": "test", "read_version": 123})"; + + REQUIRE(request.begin_streaming_parse()); + + std::string mutable_json = json; + CommitRequest::ParseStatus status = + request.parse_chunk(mutable_json.data(), mutable_json.size()); + + if (status == CommitRequest::ParseStatus::Incomplete) { + status = request.finish_streaming_parse(); + } + + REQUIRE(status == CommitRequest::ParseStatus::Complete); + REQUIRE_FALSE(request.is_parse_complete()); // Should fail validation + } + + SUBCASE("Streaming parse missing required read_version") { + std::string json = R"({"request_id": "test", "leader_id": "leader123"})"; + + REQUIRE(request.begin_streaming_parse()); + + std::string mutable_json = json; + CommitRequest::ParseStatus status = + request.parse_chunk(mutable_json.data(), mutable_json.size()); + + if (status == CommitRequest::ParseStatus::Incomplete) { + status = request.finish_streaming_parse(); + } + + REQUIRE(status == CommitRequest::ParseStatus::Complete); + REQUIRE_FALSE(request.is_parse_complete()); // Should fail validation + } } \ No newline at end of file