diff --git a/src/commit_request.hpp b/src/commit_request.hpp index 5ad51d1..d050565 100644 --- a/src/commit_request.hpp +++ b/src/commit_request.hpp @@ -17,7 +17,7 @@ struct Precondition { Type type; std::optional version; - std::string_view key; + std::optional key; std::optional begin; std::optional end; }; @@ -29,7 +29,7 @@ struct Operation { enum class Type { Write, Delete, RangeDelete }; Type type; - std::string_view key; + std::optional key; std::optional value; std::optional begin; std::optional end; @@ -207,9 +207,61 @@ public: * @return true if all required fields are present */ bool is_valid() const { - return !leader_id_.empty() && has_read_version_been_set_; + return !leader_id_.empty() && has_read_version_been_set_ && + validate_preconditions() && validate_operations(); } +private: + /** + * @brief Validate that all preconditions have required fields. + * @return true if all preconditions are valid + */ + bool validate_preconditions() const { + for (const auto &precondition : preconditions_) { + switch (precondition.type) { + case Precondition::Type::PointRead: + if (!precondition.key.has_value()) { + return false; // key is required for point_read + } + break; + case Precondition::Type::RangeRead: + if (!precondition.begin.has_value() || !precondition.end.has_value()) { + return false; // begin and end are required for range_read + } + break; + } + } + return true; + } + + /** + * @brief Validate that all operations have required fields. + * @return true if all operations are valid + */ + bool validate_operations() const { + for (const auto &operation : operations_) { + switch (operation.type) { + case Operation::Type::Write: + if (!operation.key.has_value() || !operation.value.has_value()) { + return false; // key and value are required for write + } + break; + case Operation::Type::Delete: + if (!operation.key.has_value()) { + return false; // key is required for delete + } + break; + case Operation::Type::RangeDelete: + if (!operation.begin.has_value() || !operation.end.has_value()) { + return false; // begin and end are required for range_delete + } + break; + } + } + return true; + } + +public: /** * @brief Check if there was a parse error. * @return true if there was a parse error diff --git a/tests/test_commit_request.cpp b/tests/test_commit_request.cpp index df5fd6a..12a05d5 100644 --- a/tests/test_commit_request.cpp +++ b/tests/test_commit_request.cpp @@ -132,6 +132,289 @@ TEST_CASE("CommitRequest basic parsing") { } } +TEST_CASE("CommitRequest precondition and operation validation") { + CommitRequest request; + + SUBCASE("Valid point_read precondition") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "point_read", + "key": "dGVzdA==" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Invalid point_read precondition - missing key") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "point_read" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Valid point_read precondition - empty key") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "point_read", + "key": "" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Valid range_read precondition") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "range_read", + "begin": "dGVzdA==", + "end": "dGVzdFo=" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Valid range_read precondition - empty begin/end") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "range_read", + "begin": "", + "end": "" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Invalid range_read precondition - missing begin") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "range_read", + "end": "dGVzdFo=" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Invalid range_read precondition - missing end") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "preconditions": [ + { + "type": "range_read", + "begin": "dGVzdA==" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Valid write operation") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "write", + "key": "dGVzdA==", + "value": "dmFsdWU=" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Valid write operation - empty key and value") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "write", + "key": "", + "value": "" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Invalid write operation - missing key") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "write", + "value": "dmFsdWU=" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Invalid write operation - missing value") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "write", + "key": "dGVzdA==" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Valid delete operation") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "delete", + "key": "dGVzdA==" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Invalid delete operation - missing key") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "delete" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Valid range_delete operation") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "range_delete", + "begin": "dGVzdA==", + "end": "dGVzdFo=" + } + ] + })"; + + REQUIRE(request.parse_json(json)); + REQUIRE(request.is_parse_complete()); + } + + SUBCASE("Invalid range_delete operation - missing begin") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "range_delete", + "end": "dGVzdFo=" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Invalid range_delete operation - missing end") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "range_delete", + "begin": "dGVzdA==" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } + + SUBCASE("Mixed valid and invalid operations") { + std::string json = R"({ + "leader_id": "leader456", + "read_version": 12345, + "operations": [ + { + "type": "write", + "key": "dGVzdA==", + "value": "dmFsdWU=" + }, + { + "type": "delete" + } + ] + })"; + + REQUIRE_FALSE(request.parse_json(json)); + REQUIRE_FALSE(request.is_parse_complete()); + } +} + TEST_CASE("CommitRequest memory management") { CommitRequest request;