EHN: add iterator to RepeatedPtrFieldWrapper

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.
    */

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());
 }

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