More validation of commit request
This commit is contained in:
@@ -332,6 +332,7 @@ void CommitRequest::handle_completed_number() {
|
|||||||
ctx.current_number.data() + ctx.current_number.size(), version);
|
ctx.current_number.data() + ctx.current_number.size(), version);
|
||||||
if (result.ec == std::errc{}) {
|
if (result.ec == std::errc{}) {
|
||||||
read_version_ = version;
|
read_version_ = version;
|
||||||
|
has_read_version_been_set_ = true;
|
||||||
} else {
|
} else {
|
||||||
ctx.parse_error = true;
|
ctx.parse_error = true;
|
||||||
}
|
}
|
||||||
@@ -379,7 +380,8 @@ bool CommitRequest::parse_json(std::string_view json_str) {
|
|||||||
status = WeaselJsonParser_parse(parser, nullptr, 0);
|
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);
|
WeaselJsonParser_destroy(parser);
|
||||||
|
|
||||||
|
|||||||
@@ -100,6 +100,7 @@ private:
|
|||||||
std::optional<std::string_view> request_id_;
|
std::optional<std::string_view> request_id_;
|
||||||
std::string_view leader_id_;
|
std::string_view leader_id_;
|
||||||
uint64_t read_version_ = 0;
|
uint64_t read_version_ = 0;
|
||||||
|
bool has_read_version_been_set_ = false;
|
||||||
std::vector<Precondition, ArenaStlAllocator<Precondition>> preconditions_;
|
std::vector<Precondition, ArenaStlAllocator<Precondition>> preconditions_;
|
||||||
std::vector<Operation, ArenaStlAllocator<Operation>> operations_;
|
std::vector<Operation, ArenaStlAllocator<Operation>> operations_;
|
||||||
ParserContext parser_context_;
|
ParserContext parser_context_;
|
||||||
@@ -131,6 +132,7 @@ public:
|
|||||||
CommitRequest(CommitRequest &&other) noexcept
|
CommitRequest(CommitRequest &&other) noexcept
|
||||||
: arena_(std::move(other.arena_)), request_id_(other.request_id_),
|
: arena_(std::move(other.arena_)), request_id_(other.request_id_),
|
||||||
leader_id_(other.leader_id_), read_version_(other.read_version_),
|
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_)),
|
preconditions_(std::move(other.preconditions_)),
|
||||||
operations_(std::move(other.operations_)),
|
operations_(std::move(other.operations_)),
|
||||||
parser_context_(std::move(other.parser_context_)),
|
parser_context_(std::move(other.parser_context_)),
|
||||||
@@ -149,6 +151,7 @@ public:
|
|||||||
request_id_ = other.request_id_;
|
request_id_ = other.request_id_;
|
||||||
leader_id_ = other.leader_id_;
|
leader_id_ = other.leader_id_;
|
||||||
read_version_ = other.read_version_;
|
read_version_ = other.read_version_;
|
||||||
|
has_read_version_been_set_ = other.has_read_version_been_set_;
|
||||||
preconditions_ = std::move(other.preconditions_);
|
preconditions_ = std::move(other.preconditions_);
|
||||||
operations_ = std::move(other.operations_);
|
operations_ = std::move(other.operations_);
|
||||||
parser_context_ = std::move(other.parser_context_);
|
parser_context_ = std::move(other.parser_context_);
|
||||||
@@ -195,7 +198,16 @@ public:
|
|||||||
* @return true if parsing is complete and successful
|
* @return true if parsing is complete and successful
|
||||||
*/
|
*/
|
||||||
bool is_parse_complete() const {
|
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();
|
request_id_.reset();
|
||||||
leader_id_ = {};
|
leader_id_ = {};
|
||||||
read_version_ = 0;
|
read_version_ = 0;
|
||||||
|
has_read_version_been_set_ = false;
|
||||||
preconditions_.clear();
|
preconditions_.clear();
|
||||||
operations_.clear();
|
operations_.clear();
|
||||||
|
|
||||||
|
|||||||
@@ -77,6 +77,59 @@ TEST_CASE("CommitRequest basic parsing") {
|
|||||||
|
|
||||||
REQUIRE_FALSE(request.parse_json(json));
|
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") {
|
TEST_CASE("CommitRequest memory management") {
|
||||||
@@ -254,4 +307,38 @@ TEST_CASE("CommitRequest streaming parsing") {
|
|||||||
REQUIRE(request.leader_id() == "test");
|
REQUIRE(request.leader_id() == "test");
|
||||||
REQUIRE(request.read_version() == 123);
|
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
|
||||||
|
}
|
||||||
}
|
}
|
||||||
Reference in New Issue
Block a user