WIP making Operation and Precondition smaller

src/commit_request.cpp

@@ -51,6 +51,15 @@ std::string_view CommitRequest::store_string(std::string_view str) {
   return std::string_view(arena_str, str.size());
 }
 
+void CommitRequest::on_complete() {
+  // Fill in default read version
+  for (auto &precondition : preconditions_) {
+    if (precondition.version == 0) {
+      precondition.version = read_version_;
+    }
+  }
+}
+
 std::string_view CommitRequest::decode_base64(std::string_view base64_str) {
   if (base64_str.empty()) {
     return {};
@@ -119,11 +128,11 @@ void CommitRequest::on_begin_object(void *userdata) {
     break;
   case ParseState::PreconditionsArray:
     ctx.state_stack.push(ParseState::PreconditionObject);
-    ctx.current_precondition = Precondition{};
+    ctx.current_precondition = PreconditionParseState{};
     break;
   case ParseState::OperationsArray:
     ctx.state_stack.push(ParseState::OperationObject);
-    ctx.current_operation = Operation{};
+    ctx.current_operation = OperationParseState{};
     break;
   default:
     ctx.parse_error = true;
@@ -147,12 +156,66 @@ void CommitRequest::on_end_object(void *userdata) {
   case ParseState::Root:
     // We're done parsing the root object
     ctx.parse_complete = true;
+    self->on_complete();
     break;
   case ParseState::PreconditionObject:
-    self->preconditions_.push_back(ctx.current_precondition);
+    switch (ctx.current_precondition.type) {
+    case Precondition::Type::PointRead:
+      if (!ctx.current_precondition.key.has_value()) {
+        ctx.parse_error = true;
+      } else {
+        self->preconditions_.push_back(
+            Precondition{ctx.current_precondition.type,
+                         ctx.current_precondition.version.value_or(0),
+                         ctx.current_precondition.key.value(),
+                         {}});
+      }
+      break;
+    case Precondition::Type::RangeRead:
+      if (!ctx.current_precondition.begin.has_value() ||
+          !ctx.current_precondition.end.has_value()) {
+        ctx.parse_error = true;
+      } else {
+        self->preconditions_.push_back(
+            Precondition{ctx.current_precondition.type,
+                         ctx.current_precondition.version.value_or(0),
+                         ctx.current_precondition.begin.value(),
+                         ctx.current_precondition.end.value()});
+      }
+      break;
+    }
     break;
   case ParseState::OperationObject:
-    self->operations_.push_back(ctx.current_operation);
+    switch (ctx.current_operation.type) {
+    case Operation::Type::Write:
+      if (!ctx.current_operation.key.has_value() ||
+          !ctx.current_operation.value.has_value()) {
+        ctx.parse_error = true;
+      } else {
+        self->operations_.push_back(Operation{
+            ctx.current_operation.type, ctx.current_operation.key.value(),
+            ctx.current_operation.value.value()});
+      }
+      break;
+    case Operation::Type::Delete:
+      if (!ctx.current_operation.key.has_value()) {
+        ctx.parse_error = true;
+      } else {
+        self->operations_.push_back(Operation{
+            ctx.current_operation.type, ctx.current_operation.key.value(), {}});
+      }
+      break;
+    case Operation::Type::RangeDelete:
+      if (!ctx.current_operation.begin.has_value() ||
+          !ctx.current_operation.end.has_value()) {
+        ctx.parse_error = true;
+      } else {
+        self->operations_.push_back(Operation{
+            ctx.current_operation.type, ctx.current_operation.begin.value(),
+            ctx.current_operation.end.value()});
+      }
+      break;
+    }
     break;
   default:
     break;
@@ -361,31 +424,13 @@ void CommitRequest::handle_completed_key() {
   // No immediate action needed as we'll use it when processing values
 }
 
-bool CommitRequest::parse_json(std::string_view json_str) {
+bool CommitRequest::parse_json(char *data, size_t len) {
-  reset();
+  if (!begin_streaming_parse()) {
-
-  WeaselJsonParser *parser =
-      WeaselJsonParser_create(64, &json_callbacks, this, 0);
-  if (!parser) {
     return false;
   }
-
+  parse_chunk(data, len);
-  // Make a mutable copy of the JSON string for parsing
+  finish_streaming_parse();
-  std::string mutable_json(json_str);
+  return is_parse_complete();
-
-  WeaselJsonStatus status =
-      WeaselJsonParser_parse(parser, mutable_json.data(), mutable_json.size());
-  if (status == WeaselJson_OK || status == WeaselJson_AGAIN) {
-    // End of input
-    status = WeaselJsonParser_parse(parser, nullptr, 0);
-  }
-
-  bool success =
-      (status == WeaselJson_OK) && !parser_context_.parse_error && is_valid();
-
-  WeaselJsonParser_destroy(parser);
-
-  return success;
 }
 
 bool CommitRequest::begin_streaming_parse() {

src/commit_request.hpp

@@ -16,10 +16,9 @@ struct Precondition {
   enum class Type { PointRead, RangeRead };
 
   Type type;
-  std::optional<uint64_t> version;
+  uint64_t version;
-  std::optional<std::string_view> key;
+  std::string_view begin;
-  std::optional<std::string_view> begin;
+  std::string_view end;
-  std::optional<std::string_view> end;
 };
 
 /**
@@ -29,10 +28,8 @@ struct Operation {
   enum class Type { Write, Delete, RangeDelete };
 
   Type type;
-  std::optional<std::string_view> key;
+  std::string_view param1;
-  std::optional<std::string_view> value;
+  std::string_view param2;
-  std::optional<std::string_view> begin;
-  std::optional<std::string_view> end;
 };
 
 /**
@@ -42,6 +39,25 @@ struct Operation {
  * memory management and ownership.
  */
 class CommitRequest {
+  struct PreconditionParseState {
+    Precondition::Type type;
+    std::optional<uint64_t> version;
+    std::optional<std::string_view> key;
+    std::optional<std::string_view> begin;
+    std::optional<std::string_view> end;
+  };
+
+  /**
+   * @brief Represents an operation in a commit request.
+   */
+  struct OperationParseState {
+    Operation::Type type;
+    std::optional<std::string_view> key;
+    std::optional<std::string_view> value;
+    std::optional<std::string_view> begin;
+    std::optional<std::string_view> end;
+  };
+
 public:
   // Parser state
   enum class ParseState {
@@ -77,8 +93,8 @@ public:
     bool parse_complete = false;
 
     // Current objects being parsed
-    Precondition current_precondition{};
+    PreconditionParseState current_precondition{};
-    Operation current_operation{};
+    OperationParseState current_operation{};
 
     // Parsing state for nested structures
     ArenaString precondition_type;
@@ -115,8 +131,8 @@ public:
    */
   explicit CommitRequest(size_t arena_size = 4096)
       : arena_(arena_size),
-        preconditions_(ArenaStlAllocator<Precondition>(&arena_)),
+        preconditions_(ArenaStlAllocator<PreconditionParseState>(&arena_)),
-        operations_(ArenaStlAllocator<Operation>(&arena_)),
+        operations_(ArenaStlAllocator<OperationParseState>(&arena_)),
         parser_context_(&arena_) {}
 
   /**
@@ -171,7 +187,7 @@ public:
    * @param json_str The JSON string to parse
    * @return true if parsing succeeded, false otherwise
    */
-  bool parse_json(std::string_view json_str);
+  bool parse_json(char *data, size_t len);
 
   /**
    * @brief Initialize streaming JSON parsing.
@@ -199,69 +215,9 @@ public:
    */
   bool is_parse_complete() const {
     return parser_context_.parse_complete && !parser_context_.parse_error &&
-           is_valid();
+           !leader_id_.empty() && has_read_version_been_set_;
   }
 
-  /**
-   * @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_ &&
-           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
@@ -354,6 +310,8 @@ private:
    */
   std::string_view store_string(std::string_view str);
 
+  void on_complete();
+
   /**
    * @brief Decode a base64 string and store it in the arena.
    * @param base64_str The base64 encoded string

src/main.cpp

@@ -20,8 +20,7 @@ void print_stats(const CommitRequest &request) {
     const auto &op = request.operations()[0];
     std::cout << "  First operation: "
               << (op.type == Operation::Type::Write ? "write" : "other")
-              << " key=" << op.key
+              << " param1=" << op.param1 << " param2=" << op.param2
-              << " value=" << (op.value ? op.value.value() : "none")
               << std::endl;
   }
 }
@@ -85,7 +84,7 @@ int main(int argc, char *argv[]) {
   })";
   auto copy = sample_json;
 
-  if (request.parse_json(copy)) {
+  if (request.parse_json(copy.data(), copy.size())) {
     print_stats(request);
   } else {
     std::cout << "✗ Failed to parse commit request" << std::endl;

tests/test_commit_request.cpp

@@ -12,7 +12,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "read_version": 12345
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.request_id().has_value());
     REQUIRE(request.request_id().value() == "test123");
     REQUIRE(request.leader_id() == "leader456");
@@ -32,12 +32,12 @@ TEST_CASE("CommitRequest basic parsing") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.preconditions().size() == 1);
     REQUIRE(request.preconditions()[0].type == Precondition::Type::PointRead);
-    REQUIRE(request.preconditions()[0].version.has_value());
+    REQUIRE(request.preconditions()[0].version == 12340);
-    REQUIRE(request.preconditions()[0].version.value() == 12340);
+    REQUIRE(request.preconditions()[0].begin == "test"); // "dGVzdA==" decoded
-    REQUIRE(request.preconditions()[0].key == "test"); // "dGVzdA==" decoded
+    REQUIRE(request.preconditions()[0].end == "");
   }
 
   SUBCASE("With operations") {
@@ -57,16 +57,16 @@ TEST_CASE("CommitRequest basic parsing") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.operations().size() == 2);
 
     REQUIRE(request.operations()[0].type == Operation::Type::Write);
-    REQUIRE(request.operations()[0].key == "test");
+    REQUIRE(request.operations()[0].param1 == "test");
-    REQUIRE(request.operations()[0].value.has_value());
+    REQUIRE(request.operations()[0].param2 == "value");
-    REQUIRE(request.operations()[0].value.value() == "value");
 
     REQUIRE(request.operations()[1].type == Operation::Type::Delete);
-    REQUIRE(request.operations()[1].key == "test2");
+    REQUIRE(request.operations()[1].param1 == "test2");
+    REQUIRE(request.operations()[1].param2 == "");
   }
 
   SUBCASE("Invalid JSON") {
@@ -75,7 +75,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "read_version": "not_a_number"
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
   }
 
   SUBCASE("Missing required leader_id") {
@@ -84,7 +84,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "read_version": 12345
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -94,7 +94,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "leader_id": "leader456"
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -105,7 +105,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "read_version": 12345
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -114,7 +114,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "request_id": "test123"
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -124,7 +124,7 @@ TEST_CASE("CommitRequest basic parsing") {
             "read_version": 12345
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
     REQUIRE_FALSE(request.request_id().has_value());
     REQUIRE(request.leader_id() == "leader456");
@@ -147,7 +147,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -162,7 +162,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -178,7 +178,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -195,7 +195,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -212,7 +212,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -228,7 +228,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -244,7 +244,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -261,7 +261,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -278,7 +278,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -294,7 +294,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -310,7 +310,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -326,7 +326,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -341,7 +341,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -358,7 +358,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE(request.parse_json(json));
+    REQUIRE(request.parse_json(json.data(), json.size()));
     REQUIRE(request.is_parse_complete());
   }
 
@@ -374,7 +374,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -390,7 +390,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 
@@ -410,7 +410,7 @@ TEST_CASE("CommitRequest precondition and operation validation") {
             ]
         })";
 
-    REQUIRE_FALSE(request.parse_json(json));
+    REQUIRE_FALSE(request.parse_json(json.data(), json.size()));
     REQUIRE_FALSE(request.is_parse_complete());
   }
 }
@@ -431,7 +431,7 @@ TEST_CASE("CommitRequest memory management") {
         ]
     })";
 
-  REQUIRE(request.parse_json(json));
+  REQUIRE(request.parse_json(json.data(), json.size()));
 
   // Check that arena allocation worked
   REQUIRE(request.total_allocated() > 0);
@@ -537,16 +537,18 @@ TEST_CASE("CommitRequest streaming parsing") {
 
     // Verify precondition was parsed correctly
     REQUIRE(request.preconditions()[0].type == Precondition::Type::PointRead);
-    REQUIRE(request.preconditions()[0].version.value() == 98764);
+    REQUIRE(request.preconditions()[0].version == 98764);
-    REQUIRE(request.preconditions()[0].key == "testKey");
+    REQUIRE(request.preconditions()[0].begin == "testKey");
+    REQUIRE(request.preconditions()[0].end == "");
 
     // Verify operations were parsed correctly
     REQUIRE(request.operations()[0].type == Operation::Type::Write);
-    REQUIRE(request.operations()[0].key == "testKey");
+    REQUIRE(request.operations()[0].param1 == "testKey");
-    REQUIRE(request.operations()[0].value.value() == "testValue");
+    REQUIRE(request.operations()[0].param2 == "testValue");
 
     REQUIRE(request.operations()[1].type == Operation::Type::Delete);
-    REQUIRE(request.operations()[1].key == "deleteKey");
+    REQUIRE(request.operations()[1].param1 == "deleteKey");
+    REQUIRE(request.operations()[1].param2 == "");
   }
 
   SUBCASE("Streaming parse error handling") {