fix(cpp): remove UB when reading unset builder vertex id

cpp/src/graphar/high-level/vertices_builder.h

@@ -23,6 +23,7 @@
 #include <cassert>
 #include <cstddef>
 #include <memory>
+#include <optional>
 #include <string>
 #include <unordered_map>
 #include <unordered_set>
@@ -51,21 +52,30 @@ namespace graphar::builder {
  */
 class Vertex {
  public:
-  Vertex() : empty_(true) {}
+  Vertex() = default;
 
   /**
    * @brief Initialize the vertex with a given id.
    *
    * @param id The id of the vertex.
    */
-  explicit Vertex(IdType id) : id_(id), empty_(false) {}
+  explicit Vertex(IdType id) : id_(id) {}
 
   /**
    * @brief Get id of the vertex.
    *
+   * The id is absent until explicitly set or assigned by VerticesBuilder.
+   *
    * @return The id of the vertex.
    */
-  IdType GetId() const noexcept { return id_; }
+  IdType GetId() const { return id_.value(); }
+
+  /**
+   * @brief Check if the vertex id has been initialized.
+   *
+   * @return true/false.
+   */
+  bool HasId() const noexcept { return id_.has_value(); }
 
   /**
    * @brief Set id of the vertex.
@@ -75,11 +85,11 @@ class Vertex {
   void SetId(IdType id) { id_ = id; }
 
   /**
-   * @brief Check if the vertex is empty.
+   * @brief Check if the vertex contains no property payload.
    *
    * @return true/false.
    */
-  bool Empty() const noexcept { return empty_; }
+  bool Empty() const noexcept { return properties_.empty(); }
 
   /**
    * @brief Add a property to the vertex.
@@ -89,7 +99,6 @@ class Vertex {
    */
   // TODO(@acezen): Enable the property to be a vector(list).
   void AddProperty(const std::string& name, const std::any& val) {
-    empty_ = false;
     properties_[name] = val;
   }
 
@@ -100,7 +109,6 @@ class Vertex {
       AddProperty(name, val);
       return;
     }
-    empty_ = false;
     if (cardinalities_.find(name) != cardinalities_.end()) {
       if (cardinalities_[name] != cardinality) {
         throw std::runtime_error("Cardinality mismatch for property: " + name);
@@ -211,8 +219,7 @@ class Vertex {
   }
 
  private:
-  IdType id_;
-  bool empty_;
+  std::optional<IdType> id_;
   std::unordered_map<std::string, std::any> properties_;
   std::unordered_map<std::string, Cardinality> cardinalities_;
 };

cpp/test/test_builder.cc

@@ -64,6 +64,18 @@ TEST_CASE_METHOD(GlobalFixture, "Test_vertices_builder") {
   builder->SetValidateLevel(ValidateLevel::strong_validate);
   REQUIRE(builder->GetValidateLevel() == ValidateLevel::strong_validate);
 
+  // vertex id and payload state are tracked independently
+  builder::Vertex empty_vertex;
+  REQUIRE_FALSE(empty_vertex.HasId());
+  REQUIRE(empty_vertex.Empty());
+  REQUIRE_THROWS_AS(empty_vertex.GetId(), std::bad_optional_access);
+  empty_vertex.SetId(42);
+  REQUIRE(empty_vertex.HasId());
+  REQUIRE(empty_vertex.GetId() == 42);
+  REQUIRE(empty_vertex.Empty());
+  empty_vertex.AddProperty("id", int64_t{42});
+  REQUIRE_FALSE(empty_vertex.Empty());
+
   // check different validate levels
   builder::Vertex v;
   v.AddProperty("id", "id_of_string");
@@ -81,6 +93,11 @@ TEST_CASE_METHOD(GlobalFixture, "Test_vertices_builder") {
   builder->Clear();
   REQUIRE(builder->GetNum() == 0);
 
+  builder::Vertex indexed_vertex(7);
+  REQUIRE(indexed_vertex.HasId());
+  REQUIRE(indexed_vertex.GetId() == 7);
+  REQUIRE(indexed_vertex.Empty());
+
   // add vertices
   std::ifstream fp(test_data_dir + "/ldbc_sample/person_0_0.csv");
   std::string line;

python/src/bindings/high_level_binding.cc

@@ -415,4 +415,4 @@ extern "C" void bind_high_level_api(pybind11::module_& m) {
       py::arg("dst_type"), py::arg("adj_list_type"), py::arg("num_vertices"),
       py::arg("writer_options") = nullptr,
       py::arg("validate_level") = graphar::ValidateLevel::no_validate);
-}  // namespace graphar
+}  // namespace graphar

python/test/test_high_level_api.py

@@ -111,6 +111,16 @@ def test_vertices_builder(sample_graph_vertex):
         # Set validate level
         builder.SetValidateLevel(ValidateLevel.strong_validate)
 
+        empty_vertex = BuilderVertex()
+        assert empty_vertex.Empty()
+        with pytest.raises(Exception):
+            empty_vertex.GetId()
+        empty_vertex.SetId(42)
+        assert empty_vertex.GetId() == 42
+        assert empty_vertex.Empty()
+        empty_vertex.AddProperty("id", 42)
+        assert not empty_vertex.Empty()
+
         # Prepare vertex data
         vertex_count = 3
         property_names = ["id", "firstName", "lastName", "gender"]