Fix bug on (vertex|edge) properties in C++ API (#732)

include/_mgp.hpp

@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
 //
 // Use of this software is governed by the Business Source License
 // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -378,6 +378,10 @@ inline mgp_value *vertex_get_property(mgp_vertex *v, const char *property_name,
   return MgInvoke<mgp_value *>(mgp_vertex_get_property, v, property_name, memory);
 }
 
+inline void vertex_set_property(mgp_vertex *v, const char *property_name, mgp_value *property_value) {
+  MgInvokeVoid(mgp_vertex_set_property, v, property_name, property_value);
+}
+
 inline mgp_properties_iterator *vertex_iter_properties(mgp_vertex *v, mgp_memory *memory) {
   return MgInvoke<mgp_properties_iterator *>(mgp_vertex_iter_properties, v, memory);
 }
@@ -410,6 +414,10 @@ inline mgp_value *edge_get_property(mgp_edge *e, const char *property_name, mgp_
   return MgInvoke<mgp_value *>(mgp_edge_get_property, e, property_name, memory);
 }
 
+inline void edge_set_property(mgp_edge *e, const char *property_name, mgp_value *property_value) {
+  MgInvokeVoid(mgp_edge_set_property, e, property_name, property_value);
+}
+
 inline mgp_properties_iterator *edge_iter_properties(mgp_edge *e, mgp_memory *memory) {
   return MgInvoke<mgp_properties_iterator *>(mgp_edge_iter_properties, e, memory);
 }

include/mgp.hpp

@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
 //
 // Use of this software is governed by the Business Source License
 // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -66,9 +66,12 @@ struct MapItem;
 class Duration;
 class Value;
 
+struct StealType {};
+inline constexpr StealType steal{};
+
 inline mgp_memory *memory{nullptr};
 
-/* #region Graph (Id, Graph, Nodes, GraphRelationships, Relationships, Properties & Labels) */
+/* #region Graph (Id, Graph, Nodes, GraphRelationships, Relationships & Labels) */
 
 /// Wrapper for int64_t IDs to prevent dangerous implicit conversions.
 class Id {
@@ -281,40 +284,6 @@ class Relationships {
   mgp_edges_iterator *relationships_iterator_ = nullptr;
 };
 
-/// @brief View of node properties.
-class Properties {
- public:
-  explicit Properties(mgp_properties_iterator *properties_iterator);
-
-  /// @brief Returns the size of the properties map.
-  size_t Size() const;
-  /// @brief Returns whether the properties map is empty.
-  bool Empty() const;
-
-  /// @brief Returns the value associated with the given `key`. If there’s no such value, the behavior is undefined.
-  /// @note Each key-value pair needs to be checked, ensuing O(n) time complexity.
-  Value operator[](const std::string_view key) const;
-
-  std::map<std::string_view, Value>::const_iterator begin() const;
-  std::map<std::string_view, Value>::const_iterator end() const;
-
-  std::map<std::string_view, Value>::const_iterator cbegin() const;
-  std::map<std::string_view, Value>::const_iterator cend() const;
-
-  /// @brief Returns the key-value iterator for the given `key`. If there’s no such pair, returns the end of the
-  /// iterator.
-  /// @note Each key-value pair needs to be checked, ensuing O(n) time complexity.
-  std::map<std::string_view, Value>::const_iterator find(const std::string_view key) const;
-
-  /// @exception std::runtime_error Map contains value(s) of unknown type.
-  bool operator==(const Properties &other) const;
-  /// @exception std::runtime_error Map contains value(s) of unknown type.
-  bool operator!=(const Properties &other) const;
-
- private:
-  std::map<const std::string_view, Value> property_map_;
-};
-
 /// @brief View of node labels.
 class Labels {
  public:
@@ -596,10 +565,11 @@ class Node {
   bool HasLabel(std::string_view label) const;
 
   /// @brief Returns an iterable & indexable structure of the node’s properties.
-  class Properties Properties() const;
+  std::map<std::string, Value> Properties() const;
 
-  /// @brief Returns the value of the node’s `property_name` property.
+  void SetProperty(std::string property, Value value);
-  Value operator[](const std::string_view property_name) const;
+
+  Value GetProperty(const std::string &property) const;
 
   /// @brief Returns an iterable structure of the node’s inbound relationships.
   Relationships InRelationships() const;
@@ -649,11 +619,12 @@ class Relationship {
   /// @brief Returns the relationship’s type.
   std::string_view Type() const;
 
-  /// @brief Returns an iterable & indexable structure of the relationship’s properties.
+  /// @brief Returns an std::map of the relationship’s properties.
-  class Properties Properties() const;
+  std::map<std::string, Value> Properties() const;
 
-  /// @brief Returns the value of the relationship’s `property_name` property.
+  void SetProperty(std::string property, Value value);
-  Value operator[](const std::string_view property_name) const;
+
+  Value GetProperty(const std::string &property) const;
 
   /// @brief Returns the relationship’s source node.
   Node From() const;
@@ -986,6 +957,8 @@ class Value {
 
   explicit Value(mgp_value *ptr);
 
+  explicit Value(StealType /*steal*/, mgp_value *ptr);
+
   // Null constructor:
   explicit Value();
 
@@ -1963,35 +1936,6 @@ inline Relationships::Iterator Relationships::cbegin() const { return Iterator(r
 
 inline Relationships::Iterator Relationships::cend() const { return Iterator(nullptr); }
 
-// Properties:
-
-inline Properties::Properties(mgp_properties_iterator *properties_iterator) {
-  for (auto property = mgp::properties_iterator_get(properties_iterator); property;
-       property = mgp::properties_iterator_next(properties_iterator)) {
-    auto value = Value(property->value);
-    property_map_.emplace(property->name, value);
-  }
-  mgp::properties_iterator_destroy(properties_iterator);
-}
-
-inline size_t Properties::Size() const { return property_map_.size(); }
-
-inline bool Properties::Empty() const { return Size() == 0; }
-
-inline Value Properties::operator[](const std::string_view key) const { return property_map_.at(key); }
-
-inline std::map<std::string_view, Value>::const_iterator Properties::begin() const { return property_map_.begin(); }
-
-inline std::map<std::string_view, Value>::const_iterator Properties::end() const { return property_map_.end(); }
-
-inline std::map<std::string_view, Value>::const_iterator Properties::cbegin() const { return property_map_.cbegin(); }
-
-inline std::map<std::string_view, Value>::const_iterator Properties::cend() const { return property_map_.cend(); }
-
-inline bool Properties::operator==(const Properties &other) const { return property_map_ == other.property_map_; }
-
-inline bool Properties::operator!=(const Properties &other) const { return !(*this == other); }
-
 // Labels:
 
 inline Labels::Labels(mgp_vertex *node_ptr) : node_ptr_(mgp::vertex_copy(node_ptr, memory)) {}
@@ -2306,10 +2250,6 @@ inline void Map::Insert(std::string_view key, Value &&value) {
   value.ptr_ = nullptr;
 }
 
-inline std::map<std::string_view, Value>::const_iterator Properties::find(const std::string_view key) const {
-  return property_map_.find(key);
-}
-
 inline bool Map::operator==(const Map &other) const { return util::MapsEqual(ptr_, other.ptr_); }
 
 inline bool Map::operator!=(const Map &other) const { return !(*this == other); }
@@ -2366,10 +2306,6 @@ inline bool Node::HasLabel(std::string_view label) const {
   return false;
 }
 
-inline class Properties Node::Properties() const { return mgp::Properties(mgp::vertex_iter_properties(ptr_, memory)); }
-
-inline Value Node::operator[](const std::string_view property_name) const { return Properties()[property_name]; }
-
 inline Relationships Node::InRelationships() const {
   auto relationship_iterator = mgp::vertex_iter_in_edges(ptr_, memory);
   if (relationship_iterator == nullptr) {
@@ -2390,6 +2326,26 @@ inline void Node::AddLabel(const std::string_view label) {
   mgp::vertex_add_label(this->ptr_, mgp_label{.name = label.data()});
 }
 
+inline std::map<std::string, Value> Node::Properties() const {
+  mgp_properties_iterator *properties_iterator = mgp::vertex_iter_properties(ptr_, memory);
+  std::map<std::string, Value> property_map;
+  for (auto *property = mgp::properties_iterator_get(properties_iterator); property;
+       property = mgp::properties_iterator_next(properties_iterator)) {
+    property_map.emplace(std::string(property->name), Value(property->value));
+  }
+  mgp::properties_iterator_destroy(properties_iterator);
+  return property_map;
+}
+
+inline void Node::SetProperty(std::string property, Value value) {
+  mgp::vertex_set_property(ptr_, property.data(), value.ptr());
+}
+
+inline Value Node::GetProperty(const std::string &property) const {
+  mgp_value *vertex_prop = mgp::vertex_get_property(ptr_, property.data(), memory);
+  return Value(steal, vertex_prop);
+}
+
 inline bool Node::operator<(const Node &other) const { return Id() < other.Id(); }
 
 inline bool Node::operator==(const Node &other) const { return util::NodesEqual(ptr_, other.ptr_); }
@@ -2436,12 +2392,24 @@ inline mgp::Id Relationship::Id() const { return Id::FromInt(mgp::edge_get_id(pt
 
 inline std::string_view Relationship::Type() const { return mgp::edge_get_type(ptr_).name; }
 
-inline class Properties Relationship::Properties() const {
+inline std::map<std::string, Value> Relationship::Properties() const {
-  return mgp::Properties(mgp::edge_iter_properties(ptr_, memory));
+  mgp_properties_iterator *properties_iterator = mgp::edge_iter_properties(ptr_, memory);
+  std::map<std::string, Value> property_map;
+  for (mgp_property *property = mgp::properties_iterator_get(properties_iterator); property;
+       property = mgp::properties_iterator_next(properties_iterator)) {
+    property_map.emplace(property->name, Value(property->value));
+  }
+  mgp::properties_iterator_destroy(properties_iterator);
+  return property_map;
 }
 
-inline Value Relationship::operator[](const std::string_view property_name) const {
+inline void Relationship::SetProperty(std::string property, Value value) {
-  return Properties()[property_name];
+  mgp::edge_set_property(ptr_, property.data(), value.ptr());
+}
+
+inline Value Relationship::GetProperty(const std::string &property) const {
+  mgp_value *edge_prop = mgp::edge_get_property(ptr_, property.data(), memory);
+  return Value(steal, edge_prop);
 }
 
 inline Node Relationship::From() const { return Node(mgp::edge_get_from(ptr_)); }
@@ -2917,6 +2885,7 @@ inline bool Duration::operator<(const Duration &other) const {
 /* #region Value */
 
 inline Value::Value(mgp_value *ptr) : ptr_(mgp::value_copy(ptr, memory)) {}
+inline Value::Value(StealType /*steal*/, mgp_value *ptr) : ptr_{ptr} {}
 
 inline Value::Value() : ptr_(mgp::value_make_null(memory)) {}
 

tests/unit/cpp_api.cpp

@@ -1,4 +1,4 @@
-// Copyright 2022 Memgraph Ltd.
+// Copyright 2023 Memgraph Ltd.
 //
 // Use of this software is governed by the Business Source License
 // included in the file licenses/BSL.txt; by using this file, you agree to be bound by the terms of the Business Source
@@ -195,7 +195,7 @@ TEST_F(CppApiTestFixture, TestNode) {
   ASSERT_EQ(node_1.HasLabel("L1"), true);
   ASSERT_EQ(node_1.HasLabel("L2"), true);
 
-  ASSERT_EQ(node_1.Properties().Size(), 0);
+  ASSERT_EQ(node_1.Properties().size(), 0);
 
   auto node_2 = graph.GetNodeById(node_1.Id());
 
@@ -264,7 +264,7 @@ TEST_F(CppApiTestFixture, TestRelationship) {
   auto relationship = graph.CreateRelationship(node_1, node_2, "edge_type");
 
   ASSERT_EQ(relationship.Type(), "edge_type");
-  ASSERT_EQ(relationship.Properties().Size(), 0);
+  ASSERT_EQ(relationship.Properties().size(), 0);
   ASSERT_EQ(relationship.From().Id(), node_1.Id());
   ASSERT_EQ(relationship.To().Id(), node_2.Id());
 
@@ -419,3 +419,19 @@ TEST_F(CppApiTestFixture, TestDuration) {
   // Use Value move constructor
   auto value_y = mgp::Value(mgp::Duration("PT2M2.33S"));
 }
+
+TEST_F(CppApiTestFixture, TestNodeProperties) {
+  mgp_graph raw_graph = CreateGraph(memgraph::storage::View::NEW);
+  auto graph = mgp::Graph(&raw_graph);
+
+  auto node_1 = graph.CreateNode();
+
+  ASSERT_EQ(node_1.Properties().size(), 0);
+
+  std::map<std::string, mgp::Value> node1_prop = node_1.Properties();
+  node_1.SetProperty("b", mgp::Value("b"));
+
+  ASSERT_EQ(node_1.Properties().size(), 1);
+  ASSERT_EQ(node_1.Properties()["b"].ValueString(), "b");
+  ASSERT_EQ(node_1.GetProperty("b").ValueString(), "b");
+}