From 87b4de92d3ea9f8608327824b05c5877ade956a4 Mon Sep 17 00:00:00 2001
From: Timm Fitschen <t.fitschen@indiscale.com>
Date: Thu, 12 Aug 2021 09:29:23 +0200
Subject: [PATCH] EHN: add iterator to RepeatedPtrFieldWrapper
---
include/caosdb/entity.h | 242 ++++++++++++++++++++++++++++------------
src/caosdb/entity.cpp | 44 +++-----
test/test_entity.cpp | 109 +++++++++++++++++-
3 files changed, 296 insertions(+), 99 deletions(-)
diff --git a/include/caosdb/entity.h b/include/caosdb/entity.h
index 7edee76..e3c0d40 100644
--- a/include/caosdb/entity.h
+++ b/include/caosdb/entity.h
@@ -33,7 +33,9 @@
#include "caosdb/message_code.h" // for get_message_code, Messag...
#include <google/protobuf/util/json_util.h> // for MessageToJsonString, Jso...
#include <google/protobuf/message.h> // for RepeatedPtrField, Message
+#include <iterator> // for iterator, output_iterato...
#include <map> // for map
+#include <stdexcept> // for out_of_range
#include <string> // for string
namespace caosdb::entity {
@@ -46,12 +48,17 @@ using ::google::protobuf::RepeatedPtrField;
static const std::string logger_name = "caosdb::entity";
+/**
+ * Abstract base class for Messages, Properties and Parents container classes.
+ *
+ * This is a list-like class.
+ */
template <typename T, typename P> class RepeatedPtrFieldWrapper {
+ class iterator;
+
public:
/**
- * Return the current size of the properties container.
- *
- * This is also the number of properties the owning entity currently has.
+ * Return the current size of the container.
*/
[[nodiscard]] inline auto Size() const -> int { return wrapped->size(); }
/**
@@ -65,11 +72,30 @@ public:
* Return a mutable pointer to the element at the given index.
*/
[[nodiscard]] inline auto mutable_at(int index) const -> T * {
+ if (index >= Size() || index < 0) {
+ throw std::out_of_range("Container has size " + std::to_string(Size()));
+ }
if (cache.count(index) == 0) {
cache.emplace(index, T(&(wrapped->at(index))));
}
return &(cache.at(index));
}
+ /**
+ * Return iterator positioned at the beginning of the list.
+ */
+ auto begin() -> iterator;
+ /**
+ * Return iterator positioned at the end of the list.
+ */
+ auto end() -> iterator;
+ /**
+ * Return constant iterator positioned at the beginning of the list.
+ */
+ auto begin() const -> const iterator;
+ /**
+ * Return constant iterator positioned at the end of the list.
+ */
+ auto end() const -> const iterator;
friend class Entity;
@@ -81,23 +107,159 @@ protected:
::google::protobuf::RepeatedPtrField<P> *wrapped)
: wrapped(wrapped){};
- virtual auto Append(const T &element) -> void = 0;
+ /**
+ * Append an element. This adds the element to the end of the wrapped list
+ * and increases the size by one.
+ */
+ inline auto Append(const T &element) -> void {
+ auto *destination = this->wrapped->Add();
+ destination->Swap(element.wrapped);
+
+ // Clear the originally wrapped object and return it to the Arena
+ element.wrapped->Clear();
+
+ // set the pointer to the new object which is owned by the RepeatedPtrField
+ element.wrapped = destination;
+ }
+
+ /**
+ * Remove the element at the given index.
+ */
+ inline auto remove(int index) -> void {
+ this->wrapped->DeleteSubrange(index, 1);
+ if (cache.count(index) > 0) {
+ cache.erase(index);
+ }
+
+ // shift all indices in the cache above index (such that the values do not
+ // get deleted/copied because this could destroy pointers (c-interface).
+ // auto next = cache.begin();
+ // while(next != cache.end()) {
+ // auto cached = *next;
+ // if(cached.first > index) {
+ // std::make_pair<int, T>(cached.first-1,
+ //}
+ // cached.first
+ // next = std::next(next);
+ //}
+ // for(auto &cached : cache) {
+ // if(cached.first > index) {
+ // cached.first--;
+ //}
+ //}
+ // for(int i = index + 1; i < Size(); i++) {
+ // if(cache.count(i)>0) {
+
+ // cache.at(i).wrapped = &(this->wrapped->at(i));
+ //}
+ //}
+ for (int i = index + 1; i < Size(); i++) {
+ if (cache.count(i) > 0) {
+ auto handle = cache.extract(i);
+ handle.key()--;
+ cache.insert(std::move(handle));
+ }
+ }
+ }
::google::protobuf::RepeatedPtrField<P> *wrapped;
mutable std::map<int, T> cache;
+
+private:
+ class iterator : public std::iterator<std::output_iterator_tag, T> {
+ public:
+ explicit iterator(const RepeatedPtrFieldWrapper<T, P> *instance,
+ int index = 0);
+ auto operator*() const -> T &;
+ auto operator++() -> iterator &;
+ auto operator++(int) -> iterator;
+ auto operator!=(const iterator &rhs) const -> bool;
+
+ private:
+ int current_index = 0;
+ const RepeatedPtrFieldWrapper<T, P> *instance;
+ };
};
+template <class T, class P>
+RepeatedPtrFieldWrapper<T, P>::iterator::iterator(
+ const RepeatedPtrFieldWrapper<T, P> *instance, int index)
+ : current_index(index), instance(instance) {}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::iterator::operator*() const -> T & {
+ return *(this->instance->mutable_at(current_index));
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::iterator::operator++()
+ -> RepeatedPtrFieldWrapper<T, P>::iterator & {
+ current_index++;
+ return *this;
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::iterator::operator++(int)
+ -> RepeatedPtrFieldWrapper<T, P>::iterator {
+ iterator tmp(*this);
+ operator++();
+ return tmp;
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::iterator::operator!=(
+ const iterator &rhs) const -> bool {
+ return this->current_index != rhs.current_index;
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::begin()
+ -> RepeatedPtrFieldWrapper<T, P>::iterator {
+ return RepeatedPtrFieldWrapper<T, P>::iterator(this, 0);
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::end()
+ -> RepeatedPtrFieldWrapper<T, P>::iterator {
+ return RepeatedPtrFieldWrapper<T, P>::iterator(this, Size());
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::begin() const
+ -> const RepeatedPtrFieldWrapper<T, P>::iterator {
+ return RepeatedPtrFieldWrapper<T, P>::iterator(this, 0);
+}
+
+template <typename T, typename P>
+auto RepeatedPtrFieldWrapper<T, P>::end() const
+ -> const RepeatedPtrFieldWrapper<T, P>::iterator {
+ return RepeatedPtrFieldWrapper<T, P>::iterator(this, Size());
+}
+
/**
- * Messages convey information about the state and result of transactions.
+ * Messages convey information about the state of entities and result of
+ * transactions.
*
* A Message object can be thought of as kinf of a generalized error object in
* other frameworks. Please have a look at MessageCodes for more details.
*/
class Message {
public:
+ /**
+ * Get the code of this message.
+ *
+ * The message code is a unique identifier of the type of message and is
+ * intended to make it easiert to identify messages in a machine-readable
+ * way.
+ */
[[nodiscard]] inline auto GetCode() const -> MessageCode {
return get_message_code(wrapped->code());
}
+ /**
+ * Get the description of this message.
+ *
+ * The description is intended for a human reader.
+ */
[[nodiscard]] inline auto GetDescription() const -> std::string {
return wrapped->description();
}
@@ -122,10 +284,6 @@ private:
class Messages : public RepeatedPtrFieldWrapper<Message, ProtoMessage> {
public:
~Messages();
- //[[nodiscard]] inline auto Size() const -> int { return wrapped->size(); }
- //[[nodiscard]] inline auto At(int index) const -> const Message {
- // return Message(&(wrapped->at(index)));
- //}
friend class Entity;
// TODO(fspreck) Same here.
@@ -134,10 +292,6 @@ public:
private:
inline Messages() : RepeatedPtrFieldWrapper(){};
- auto Append(const Message &message) -> void override;
-
- //::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Message>
- //*wrapped;
};
/**
@@ -249,19 +403,6 @@ private:
class Parents : public RepeatedPtrFieldWrapper<Parent, ProtoParent> {
public:
~Parents() = default;
- /**
- * Return the current size of the parent container.
- *
- * That is also the number of parents the owning entity currently has.
- */
- //[[nodiscard]] inline auto Size() const -> int { return wrapped->size(); }
- /**
- * Return the parent at the given index.
- */
- //[[nodiscard]] inline auto At(int index) const -> const Parent {
- // return Parent(&(wrapped->at(index)));
- //}
-
friend class Entity;
private:
@@ -270,20 +411,6 @@ private:
::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Parent>
*wrapped)
: RepeatedPtrFieldWrapper(wrapped){};
- //: wrapped(wrapped){};
-
- /**
- * Append a parent.
- *
- * This increases the Size() by one.
- */
- auto Append(const Parent &parent) -> void override;
- /**
- * The collection of parent messages which serves as a backend for this
- * class.
- */
- //::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Parent>
- //*wrapped;
};
/**
@@ -391,26 +518,6 @@ class Properties
caosdb::entity::v1alpha1::Property> {
public:
~Properties() = default;
- /**
- * Return the current size of the properties container.
- *
- * This is also the number of properties the owning entity currently has.
- */
- //[[nodiscard]] inline auto Size() const -> int { return wrapped->size(); }
- /**
- * Return the property at the given index.
- */
- //[[nodiscard]] auto At(int index) const -> const Property & {
- // return mutable_at(index);
- //}
-
- //[[nodiscard]] auto mutable_at(int index) const -> Property & {
- // if (cache.count(index) == 0) {
- // cache.emplace(index, Property(&(wrapped->at(index))));
- //}
- // return cache.at(index);
- //}
-
friend class Entity;
private:
@@ -420,18 +527,6 @@ private:
*wrapped)
: RepeatedPtrFieldWrapper<Property, caosdb::entity::v1alpha1::Property>(
wrapped){};
- //: wrapped(wrapped){};
-
- /**
- * Append a property
- *
- * This increases the Size() by one.
- */
- auto Append(const Property &property) -> void override;
-
- //::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Property>
- //*wrapped;
- // mutable std::map<int, Property> cache;
};
/**
@@ -513,9 +608,12 @@ public:
auto SetUnit(const std::string &unit) -> void;
// Currently no references or lists.
auto SetDatatype(const std::string &datatype) -> void;
+
auto AppendProperty(const Property &property) -> void;
+ auto RemoveProperty(int index) -> void;
auto AppendParent(const Parent &parent) -> void;
+ auto RemoveParent(int index) -> void;
/**
* Copy all of this entity's features to the target ProtoEntity.
*/
diff --git a/src/caosdb/entity.cpp b/src/caosdb/entity.cpp
index 7cea00f..26c0438 100644
--- a/src/caosdb/entity.cpp
+++ b/src/caosdb/entity.cpp
@@ -20,12 +20,7 @@
*
*/
#include "caosdb/entity.h"
-#include "boost/log/core/record.hpp" // for record
-#include "boost/log/sources/record_ostream.hpp" // for record_pump<>:...
-#include "boost/preprocessor/seq/limits/enum_256.hpp" // for BOOST_PP_SEQ_E...
-#include "boost/preprocessor/seq/limits/size_256.hpp" // for BOOST_PP_SEQ_S...
#include "caosdb/entity/v1alpha1/main.pb.h" // for Parent, Arena::CreateMay...
-#include "caosdb/logging.h" // for CAOSDB_LOG_WARN
#include "caosdb/protobuf_helper.h" // for get_arena
#include "google/protobuf/arena.h" // for Arena
@@ -38,10 +33,10 @@ using caosdb::utility::get_arena;
Messages::~Messages() = default;
-auto Messages::Append(const Message & /*message*/) -> void {
- CAOSDB_LOG_WARN(logger_name)
- << "The Messages container does not implement the Append function.";
-}
+// auto Messages::Append(const Message & [>message<]) -> void {
+// CAOSDB_LOG_WARN(logger_name)
+//<< "The Messages container does not implement the Append function.";
+//}
Parent::Parent() : wrapped(Parent::CreateProtoParent()) {
// TODO(fspreck) Re-enable once we have decided how to attach
@@ -73,16 +68,16 @@ auto Parent::SetId(const std::string &id) -> void { this->wrapped->set_id(id); }
return this->wrapped->description();
}
-auto Parents::Append(const Parent &parent) -> void {
- auto *destination = this->wrapped->Add();
- destination->Swap(parent.wrapped);
+// auto Parents::Append(const Parent &parent) -> void {
+// auto *destination = this->wrapped->Add();
+// destination->Swap(parent.wrapped);
- // Clear the originally wrapped object and return it to the Arena
- parent.wrapped->Clear();
+//// Clear the originally wrapped object and return it to the Arena
+// parent.wrapped->Clear();
- // set the pointer to the new object which is owned by the RepeatedPtrField
- parent.wrapped = destination;
-}
+//// set the pointer to the new object which is owned by the RepeatedPtrField
+// parent.wrapped = destination;
+//}
Property::Property() : wrapped(Property::CreateProtoProperty()) {}
@@ -142,15 +137,6 @@ auto Property::SetDatatype(const std::string &datatype) -> void {
this->wrapped->set_datatype(datatype);
}
-auto Properties::Append(const Property &property) -> void {
- auto *destination = this->wrapped->Add();
- destination->Swap(property.wrapped);
-
- property.wrapped->Clear();
-
- property.wrapped = destination;
-}
-
[[nodiscard]] auto Entity::GetParents() const -> const Parents & {
return parents;
}
@@ -159,6 +145,8 @@ auto Entity::AppendParent(const Parent &parent) -> void {
this->parents.Append(parent);
}
+auto Entity::RemoveParent(int index) -> void { this->parents.remove(index); }
+
[[nodiscard]] auto Entity::GetProperties() const -> const Properties & {
return properties;
}
@@ -167,6 +155,10 @@ auto Entity::AppendProperty(const Property &property) -> void {
this->properties.Append(property);
}
+auto Entity::RemoveProperty(int index) -> void {
+ this->properties.remove(index);
+}
+
auto Entity::CreateProtoEntity() -> ProtoEntity * {
return google::protobuf::Arena::CreateMessage<ProtoEntity>(get_arena());
}
diff --git a/test/test_entity.cpp b/test/test_entity.cpp
index 3639f8c..5b08480 100644
--- a/test/test_entity.cpp
+++ b/test/test_entity.cpp
@@ -30,7 +30,9 @@
#include "gtest/gtest-message.h" // for Message
#include "gtest/gtest-test-part.h" // for TestPartResult, Sui...
#include "gtest/gtest_pred_impl.h" // for Test, EXPECT_EQ
-#include <memory> // for allocator, shared_ptr
+#include <iostream>
+#include <memory> // for allocator, shared_ptr
+#include <string>
namespace caosdb::entity {
using caosdb::entity::v1alpha1::IdResponse;
@@ -260,4 +262,109 @@ TEST(test_entity, test_from_id_response) {
EXPECT_EQ(other_ent.GetInfos().At(0).GetDescription(), "info_desc");
EXPECT_EQ(other_ent.GetInfos().At(0).GetCode(), MessageCode::UNSPECIFIED);
}
+
+TEST(test_entity, test_remove_property) {
+ Entity entity;
+ for (int i = 0; i < 5; i++) {
+ auto name = "PROPERTY-" + std::to_string(i);
+
+ Property property;
+ property.SetName(name);
+ EXPECT_EQ(property.GetName(), name);
+
+ entity.AppendProperty(property);
+ EXPECT_EQ(property.GetName(), name);
+
+ // not initializing the cache
+ }
+ ASSERT_EQ(entity.GetProperties().Size(), 5);
+ for (int i = 5; i < 10; i++) {
+ auto name = "PROPERTY-" + std::to_string(i);
+
+ Property property;
+ property.SetName(name);
+ EXPECT_EQ(property.GetName(), name);
+
+ entity.AppendProperty(property);
+ EXPECT_EQ(property.GetName(), name);
+
+ // initializing the cache
+ const auto &property_2 = entity.GetProperties().At(i);
+ EXPECT_EQ(property_2.GetName(), name);
+ }
+
+ ASSERT_EQ(entity.GetProperties().Size(), 10);
+
+ for (int i = 5; i < 10; i++) {
+ // double checking the cache
+ auto name = "PROPERTY-" + std::to_string(i);
+ const auto &property = entity.GetProperties().At(i);
+ EXPECT_EQ(property.GetName(), name);
+ }
+
+ // Remove at index 3
+ // P0,P1,P2,P3,P4,P5,P6,P7,P8,P9
+ // ^
+ // P0,P1,P2, P4,P5,P6,P7,P8,P9
+ entity.RemoveProperty(3);
+
+ // Remove at index 6
+ // P0,P1,P2, P4,P5,P6,P7,P8,P9
+ // ^
+ // P0,P1,P2, P4,P5,P6, P8,P9
+ entity.RemoveProperty(6);
+ ASSERT_EQ(entity.GetProperties().Size(), 8);
+
+ // AppendProperty another property
+ // P0,P1,P2, P4,P5,P6, P8,P9
+ // ^
+ // P0,P1,P2, P4,P5,P6, P8,P9,P10
+ Property property10;
+ property10.SetName("PROPERTY-10");
+ entity.AppendProperty(property10);
+ ASSERT_EQ(entity.GetProperties().Size(), 9);
+
+ std::cout << "[" << std::endl;
+ for (int i = 0; i < 9; i++) {
+ std::cout << " " << entity.GetProperties().At(i).GetName() << ",\n";
+ }
+ std::cout << "]" << std::endl;
+
+ for (int i = 0; i < 3; i++) {
+ auto name = "PROPERTY-" + std::to_string(i);
+ const auto &property = entity.GetProperties().At(i);
+ EXPECT_EQ(property.GetName(), name);
+ }
+ for (int i = 3; i < 6; i++) {
+ auto name = "PROPERTY-" + std::to_string(i + 1);
+ const auto &property = entity.GetProperties().At(i);
+ EXPECT_EQ(property.GetName(), name);
+ }
+ for (int i = 6; i < 9; i++) {
+ auto name = "PROPERTY-" + std::to_string(i + 2);
+ const auto &property = entity.GetProperties().At(i);
+ EXPECT_EQ(property.GetName(), name);
+ }
+}
+
+TEST(test_entity, test_property_iterator) {
+ Entity entity;
+ for (int i = 0; i < 5; i++) {
+ auto name = "PROPERTY-" + std::to_string(i);
+
+ Property property;
+ property.SetName(name);
+ EXPECT_EQ(property.GetName(), name);
+
+ entity.AppendProperty(property);
+ }
+ ASSERT_EQ(entity.GetProperties().Size(), 5);
+ int counter = 0;
+ for (auto &property : entity.GetProperties()) {
+ auto name = "PROPERTY-" + std::to_string(counter);
+ EXPECT_EQ(property.GetName(), name);
+ counter++;
+ }
+ EXPECT_EQ(counter, 5);
+}
} // namespace caosdb::entity
--
GitLab