From 1cd1da84fd9b5f5d205fac5444376018670c05bb Mon Sep 17 00:00:00 2001 From: Antonio Filipovic <61245998+antoniofilipovic@users.noreply.github.com> Date: Mon, 23 Jan 2023 12:57:17 +0100 Subject: [PATCH] Fix bug on (vertex|edge) properties in C++ API (#732) --- include/_mgp.hpp | 10 ++- include/mgp.hpp | 137 ++++++++++++++++------------------------- tests/unit/cpp_api.cpp | 22 ++++++- 3 files changed, 81 insertions(+), 88 deletions(-) diff --git a/include/_mgp.hpp b/include/_mgp.hpp index 39cbedb4e..aae4a0824 100644 --- a/include/_mgp.hpp +++ b/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_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_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_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_edge_iter_properties, e, memory); } diff --git a/include/mgp.hpp b/include/mgp.hpp index e18f18961..29d6fb25e 100644 --- a/include/mgp.hpp +++ b/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::const_iterator begin() const; - std::map::const_iterator end() const; - - std::map::const_iterator cbegin() const; - std::map::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::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 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 Properties() const; - /// @brief Returns the value of the node’s `property_name` property. - Value operator[](const std::string_view property_name) const; + void SetProperty(std::string property, Value value); + + 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. - class Properties Properties() const; + /// @brief Returns an std::map of the relationship’s properties. + std::map Properties() const; - /// @brief Returns the value of the relationship’s `property_name` property. - Value operator[](const std::string_view property_name) const; + void SetProperty(std::string property, Value value); + + 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::const_iterator Properties::begin() const { return property_map_.begin(); } - -inline std::map::const_iterator Properties::end() const { return property_map_.end(); } - -inline std::map::const_iterator Properties::cbegin() const { return property_map_.cbegin(); } - -inline std::map::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::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 Node::Properties() const { + mgp_properties_iterator *properties_iterator = mgp::vertex_iter_properties(ptr_, memory); + std::map 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 { - return mgp::Properties(mgp::edge_iter_properties(ptr_, memory)); +inline std::map Relationship::Properties() const { + mgp_properties_iterator *properties_iterator = mgp::edge_iter_properties(ptr_, memory); + std::map 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 { - return Properties()[property_name]; +inline void Relationship::SetProperty(std::string property, Value value) { + 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)) {} diff --git a/tests/unit/cpp_api.cpp b/tests/unit/cpp_api.cpp index f1344776b..09673b00c 100644 --- a/tests/unit/cpp_api.cpp +++ b/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 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"); +}