ENH: Add retrieval and queries to Extern C interface

Summary

See https://gitlab.indiscale.com/caosdb/customers/lfpb/management/-/issues/411. Extends the Extern C interface by retrieving single and multiple entities and executing queries.

Focus

There is a lot of repetitive new code for create, delete, set, and get functions in ccaosdb.cpp. Some aspects of the c++ objects of Entities, Parents, and Propeties had to be changed in order to comply with the C interface, mainly because of memory management.

The most important new parts is that entities, transactions, properties and parents can now be created and deleted via the C interface, and that their properties can be set and gotten.

Test Environment

In principle, unit tests should be sufficient. Check whether you are content with the extent of the tests. Queries and retrievals are checked in the integration tests.

Check List for the Author

Please, prepare your MR for a review. Be sure to write a summary and a focus and create gitlab comments for the reviewer. They should guide the reviewer through the changes, explain your changes and also point out open questions. For further good practices have a look at our review guidelines

• All automated tests pass
• Reference related Issues
• [ ] Up-to-date CHANGELOG.md
• Annotations in code (Gitlab comments)
  • Intent of new code
  • Problems with old code
  • Why this implementation?

Check List for the Reviewer

• I understand the intent of this MR
• All automated tests pass
• [ ] Up-to-date CHANGELOG.md
• The test environment setup works and the intended behavior is reproducible in the test environment
• In-code documentation and comments are up-to-date.
• Check: Are there specifications? Are they satisfied?

For further good practices have a look at our review guidelines.

include/caosdb/entity.h

@@ -43,9 +43,11 @@
 #include <google/protobuf/message.h>                   // for RepeatedPtrField
 #include <google/protobuf/util/json_util.h>            // for MessageToJson...
 #include <iosfwd>                                      // for streamsize
-#include <random>                                      // for mt19937, rand...
-#include <stdexcept>                                   // for out_of_range
-#include <string>                                      // for string, basic...
+#include <iterator>  // for iterator, output_iterato...
+#include <map>       // for map
+#include <random>    // for mt19937, rand...
+#include <stdexcept> // for out_of_range
+#include <string>    // for string, basic...
 
 namespace caosdb::entity {
 using boost::filesystem::exists;
@@ -59,10 +61,10 @@ using ProtoMessage = caosdb::entity::v1alpha1::Message;
 using caosdb::StatusCode;
 using caosdb::entity::v1alpha1::EntityResponse;
 using caosdb::entity::v1alpha1::FileTransmissionId;
+using ::google::protobuf::RepeatedPtrField;
 using google::protobuf::RepeatedPtrField;
-using ProtoMessage = caosdb::entity::v1alpha1::Message;
 
-const std::string logger_name = "caosdb::entity";
+static const std::string logger_name = "caosdb::entity";
 
 struct FileDescriptor {
   FileTransmissionId *file_transmission_id;
@@ -71,16 +73,197 @@ struct FileDescriptor {
 };
 
 /**
- * Messages convey information about the state and result of transactions.
+ * 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 container.
+   */
+  [[nodiscard]] inline auto size() const -> int { return wrapped->size(); }
+  /**
+   * Return a const reference to the element at the given index.
+   */
+  [[nodiscard]] inline auto at(int index) const -> const T & {
+    return *mutable_at(index);
+  }
+
+  /**
+   * 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;
+
+  virtual ~RepeatedPtrFieldWrapper(){};
+
+protected:
+  RepeatedPtrFieldWrapper(){};
+  explicit inline RepeatedPtrFieldWrapper(
+    ::google::protobuf::RepeatedPtrField<P> *wrapped)
+    : wrapped(wrapped){};
+
+  /**
+   * 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).
+    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 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();
   }
@@ -91,6 +274,7 @@ public:
   // friend class Parent;
   // friend class Property;
   friend class Messages;
+  friend class RepeatedPtrFieldWrapper<Message, ProtoMessage>;
 
 private:
   explicit inline Message(ProtoMessage *wrapped) : wrapped(wrapped){};
@@ -101,20 +285,9 @@ private:
 /**
  * Container for Messages.
  */
-class Messages {
+class Messages : public RepeatedPtrFieldWrapper<Message, ProtoMessage> {
 public:
-  [[nodiscard]] inline auto size() const -> int {
-    if (wrapped == nullptr) {
-      return 0;
-    }
-    return wrapped->size();
-  }
-  [[nodiscard]] inline auto at(int index) const -> const Message {
-    if (wrapped == nullptr) {
-      throw std::out_of_range("Number of messages: 0");
-    }
-    return Message(&(wrapped->at(index)));
-  }
+  ~Messages();
 
   friend class Entity;
   // TODO(fspreck) Same here.
@@ -122,9 +295,7 @@ public:
   // friend class Property;
 
 private:
-  inline Messages() : wrapped(nullptr){};
-
-  RepeatedPtrField<ProtoMessage> *wrapped;
+  inline Messages() : RepeatedPtrFieldWrapper(){};
 };
 
 /**
@@ -203,6 +374,7 @@ public:
 
   friend class Entity;
   friend class Parents;
+  friend class RepeatedPtrFieldWrapper<Parent, ProtoParent>;
 
 private:
   /**
@@ -231,40 +403,17 @@ private:
  *
  * Should only be instantiated and write-accessed by the owning entity.
  */
-class Parents {
+class Parents : public RepeatedPtrFieldWrapper<Parent, ProtoParent> {
 public:
-  /**
-   * 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)));
-  }
-
+  ~Parents() = default;
   friend class Entity;
 
 private:
-  inline Parents(){};
+  inline Parents() : RepeatedPtrFieldWrapper(){};
   explicit inline Parents(
-    RepeatedPtrField<caosdb::entity::v1alpha1::Parent> *wrapped)
-    : wrapped(wrapped){};
-
-  /**
-   * Append a parent.
-   *
-   * This increases the size() by one.
-   */
-  auto Append(const Parent &parent) -> void;
-  /**
-   * The collection of parent messages which serves as a backend for this
-   * class.
-   */
-  RepeatedPtrField<caosdb::entity::v1alpha1::Parent> *wrapped;
+    ::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Parent>
+      *wrapped)
+    : RepeatedPtrFieldWrapper(wrapped){};
 };
 
 /**
@@ -357,6 +506,7 @@ public:
 
   friend class Entity;
   friend class Properties;
+  friend class RepeatedPtrFieldWrapper<Property, ProtoProperty>;
 
 private:
   static auto CreateProtoProperty() -> ProtoProperty *;
@@ -369,37 +519,20 @@ private:
  *
  * Should only be instantiated and write-accessed by the owning entity.
  */
-class Properties {
+class Properties
+  : public RepeatedPtrFieldWrapper<Property,
+                                   caosdb::entity::v1alpha1::Property> {
 public:
-  /**
-   * Return the current size of the properties container.
-   *
-   * This is also the number of properties the owningn 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 Property(&(wrapped->at(index)));
-  }
-
+  ~Properties() = default;
   friend class Entity;
 
 private:
   inline Properties(){};
   explicit inline Properties(
-    RepeatedPtrField<caosdb::entity::v1alpha1::Property> *wrapped)
-    : wrapped(wrapped){};
-
-  /**
-   * Append a property
-   *
-   * This increases the size() by one.
-   */
-  auto Append(const Property &property) -> void;
-
-  RepeatedPtrField<caosdb::entity::v1alpha1::Property> *wrapped;
+    ::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Property>
+      *wrapped)
+    : RepeatedPtrFieldWrapper<Property, caosdb::entity::v1alpha1::Property>(
+        wrapped){};
 };
 
 /**
@@ -491,9 +624,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.
    */

[Florian Spreckelsen]

Implements RemoveParent and RemoveProperty