FIX get_{parent,property,error,warning,info}

include/caosdb/entity.h

@@ -29,16 +29,63 @@
 #ifndef CAOSDB_ENTITY_H
 #define CAOSDB_ENTITY_H
 
-#include <string>                           // for string
-#include "caosdb/entity/v1alpha1/main.pb.h" // for RepeatedPtrField, Message
+#include "caosdb/entity/v1alpha1/main.pb.h" // for ProtoEntity, ProtoParent...
 #include "caosdb/message_code.h"            // for get_message_code, Messag...
-#include "google/protobuf/util/json_util.h" // for MessageToJsonString, Jso...
+#include <google/protobuf/util/json_util.h> // for MessageToJsonString, Jso...
+#include <google/protobuf/message.h>        // for RepeatedPtrField, Message
+#include <map>                              // for map
+#include <string>                           // for string
 
 namespace caosdb::entity {
 using caosdb::entity::v1alpha1::IdResponse;
 using ProtoParent = caosdb::entity::v1alpha1::Parent;
 using ProtoProperty = caosdb::entity::v1alpha1::Property;
 using ProtoEntity = caosdb::entity::v1alpha1::Entity;
+using ProtoMessage = caosdb::entity::v1alpha1::Message;
+using ::google::protobuf::RepeatedPtrField;
+
+static const std::string logger_name = "caosdb::entity";
+
+template <typename T, typename P> class RepeatedPtrFieldWrapper {
+public:
+  /**
+   * 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 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 (cache.count(index) == 0) {
+      cache.emplace(index, T(&(wrapped->at(index))));
+    }
+    return &(cache.at(index));
+  }
+
+  friend class Entity;
+
+  virtual ~RepeatedPtrFieldWrapper(){};
+
+protected:
+  RepeatedPtrFieldWrapper(){};
+  explicit inline RepeatedPtrFieldWrapper(
+    ::google::protobuf::RepeatedPtrField<P> *wrapped)
+    : wrapped(wrapped){};
+
+  virtual auto Append(const T &element) -> void = 0;
+
+  ::google::protobuf::RepeatedPtrField<P> *wrapped;
+  mutable std::map<int, T> cache;
+};
 
 /**
  * Messages convey information about the state and result of transactions.
@@ -61,23 +108,24 @@ public:
   // friend class Parent;
   // friend class Property;
   friend class Messages;
+  friend class RepeatedPtrFieldWrapper<Message, ProtoMessage>;
 
 private:
-  explicit inline Message(caosdb::entity::v1alpha1::Message *wrapped)
-    : wrapped(wrapped){};
+  explicit inline Message(ProtoMessage *wrapped) : wrapped(wrapped){};
 
-  caosdb::entity::v1alpha1::Message *wrapped;
+  ProtoMessage *wrapped;
 };
 
 /**
  * Container for Messages.
  */
-class Messages {
+class Messages : public RepeatedPtrFieldWrapper<Message, ProtoMessage> {
 public:
-  [[nodiscard]] inline auto Size() const -> int { return wrapped->size(); }
-  [[nodiscard]] inline auto At(int index) const -> const Message {
-    return Message(&(wrapped->at(index)));
-  }
+  ~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.
@@ -85,10 +133,11 @@ public:
   // friend class Property;
 
 private:
-  inline Messages() : wrapped(nullptr){};
+  inline Messages() : RepeatedPtrFieldWrapper(){};
+  auto Append(const Message &message) -> void override;
 
-  ::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Message>
-    *wrapped;
+  //::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Message>
+  //*wrapped;
 };
 
 /**
@@ -168,6 +217,7 @@ public:
 
   friend class Entity;
   friend class Parents;
+  friend class RepeatedPtrFieldWrapper<Parent, ProtoParent>;
 
 private:
   /**
@@ -196,42 +246,44 @@ private:
  *
  * Should only be instantiated and write-accessed by the owning entity.
  */
-class Parents {
+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(); }
+  //[[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)));
-  }
+  //[[nodiscard]] inline auto At(int index) const -> const Parent {
+  // return Parent(&(wrapped->at(index)));
+  //}
 
   friend class Entity;
 
 private:
-  inline Parents(){};
+  inline Parents() : RepeatedPtrFieldWrapper(){};
   explicit inline Parents(
     ::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Parent>
       *wrapped)
-    : wrapped(wrapped){};
+    : RepeatedPtrFieldWrapper(wrapped){};
+  //: wrapped(wrapped){};
 
   /**
    * Append a parent.
    *
    * This increases the Size() by one.
    */
-  auto Append(const Parent &parent) -> void;
+  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;
+  //::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Parent>
+  //*wrapped;
 };
 
 /**
@@ -321,6 +373,7 @@ public:
 
   friend class Entity;
   friend class Properties;
+  friend class RepeatedPtrFieldWrapper<Property, ProtoProperty>;
 
 private:
   static auto CreateProtoProperty() -> ProtoProperty *;
@@ -333,20 +386,30 @@ private:
  *
  * Should only be instantiated and write-accessed by the owning entity.
  */
-class Properties {
+class Properties
+  : public RepeatedPtrFieldWrapper<Property,
+                                   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(); }
+  //[[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)));
-  }
+  //[[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;
 
@@ -355,17 +418,20 @@ private:
   explicit inline Properties(
     ::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Property>
       *wrapped)
-    : wrapped(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;
+  auto Append(const Property &property) -> void override;
 
-  ::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Property>
-    *wrapped;
+  //::google::protobuf::RepeatedPtrField<caosdb::entity::v1alpha1::Property>
+  //*wrapped;
+  // mutable std::map<int, Property> cache;
 };
 
 /**

include/ccaosdb.h

@@ -279,7 +279,7 @@ int caosdb_transaction_delete_transaction(
 int caosdb_transaction_transaction_retrieve_by_id(
   caosdb_transaction_transaction *transaction, const char *id);
 int caosdb_transaction_transaction_retrieve_by_ids(
-  caosdb_transaction_transaction *transaction, const char *ids[]);
+  caosdb_transaction_transaction *transaction, const char *ids[], int length);
 int caosdb_transaction_transaction_query(
   caosdb_transaction_transaction *transaction, const char *query);
 int caosdb_transaction_transaction_execute(

src/caosdb/entity.cpp

@@ -20,7 +20,12 @@
  *
  */
 #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
 
@@ -31,6 +36,13 @@ using ProtoProperty = caosdb::entity::v1alpha1::Property;
 using ProtoEntity = caosdb::entity::v1alpha1::Entity;
 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.";
+}
+
 Parent::Parent() : wrapped(Parent::CreateProtoParent()) {
   // TODO(fspreck) Re-enable once we have decided how to attach
   // messages to parents.

src/ccaosdb.cpp

@@ -43,6 +43,7 @@ extern "C" {
  */
 #define ERROR_RETURN_CODE(code, fun, body)                                     \
   fun {                                                                        \
+    CAOSDB_LOG_TRACE(CCAOSDB_LOGGER_NAME) << "Enter " << #fun;                 \
     try {                                                                      \
       body                                                                     \
     } catch (const std::exception &exc) {                                      \
@@ -385,22 +386,16 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                     return wrapped_transaction->RetrieveById(std::string(id));
                   })
 
-ERROR_RETURN_CODE(
-  GENERIC_ERROR,
-  int caosdb_transaction_transaction_retrieve_by_ids(
-    caosdb_transaction_transaction *transaction, const char *ids[]),
-  {
-    auto *wrapped_transaction = static_cast<caosdb::transaction::Transaction *>(
-      transaction->wrapped_transaction);
-    // Fill a string vector with the contents of the array of char arrays.
-    std::vector<std::string> str_ids;
-    std::cout << "Filling vector with ids ..." << std::endl;
-    for (const char **i = ids; *i; ++i) {
-      std::cout << *i << ", empty: " << (strcmp(*i, "") == 0) << std::endl;
-      str_ids.push_back(std::string(*i));
-    }
-    return wrapped_transaction->RetrieveById(str_ids.begin(), str_ids.end());
-  })
+ERROR_RETURN_CODE(GENERIC_ERROR,
+                  int caosdb_transaction_transaction_retrieve_by_ids(
+                    caosdb_transaction_transaction *transaction,
+                    const char *ids[], int length),
+                  {
+                    auto *wrapped_transaction =
+                      static_cast<caosdb::transaction::Transaction *>(
+                        transaction->wrapped_transaction);
+                    return wrapped_transaction->RetrieveById(ids, ids + length);
+                  })
 
 ERROR_RETURN_CODE(GENERIC_ERROR,
                   int caosdb_transaction_transaction_query(
@@ -547,8 +542,7 @@ ERROR_RETURN_CODE(
   {
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
-    auto requested_error = wrapped_entity->GetErrors().At(index);
-    out->wrapped_message = (&requested_error);
+    out->wrapped_message = wrapped_entity->GetErrors().mutable_at(index);
     return 0;
   })
 
@@ -570,8 +564,7 @@ ERROR_RETURN_CODE(
   {
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
-    auto requested_warning = wrapped_entity->GetWarnings().At(index);
-    out->wrapped_message = (&requested_warning);
+    out->wrapped_message = wrapped_entity->GetWarnings().mutable_at(index);
     return 0;
   })
 
@@ -593,8 +586,7 @@ ERROR_RETURN_CODE(
   {
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
-    auto requested_info = wrapped_entity->GetInfos().At(index);
-    out->wrapped_message = (&requested_info);
+    out->wrapped_message = wrapped_entity->GetInfos().mutable_at(index);
     return 0;
   })
 
@@ -616,8 +608,7 @@ ERROR_RETURN_CODE(
   {
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
-    auto requested_property = wrapped_entity->GetProperties().At(index);
-    out->wrapped_property = &requested_property;
+    out->wrapped_property = wrapped_entity->GetProperties().mutable_at(index);
     return 0;
   })
 
@@ -639,8 +630,7 @@ ERROR_RETURN_CODE(
   {
     auto *wrapped_entity =
       static_cast<caosdb::entity::Entity *>(entity->wrapped_entity);
-    auto requested_parent = wrapped_entity->GetParents().At(index);
-    out->wrapped_parent = &requested_parent;
+    out->wrapped_parent = wrapped_entity->GetParents().mutable_at(index);
     return 0;
   })
 

test/test_ccaosdb.cpp

@@ -93,7 +93,7 @@ TEST_F(test_ccaosdb, test_execute_transaction) {
   // linting
   const char *ids[] = {"id1", "id2", "id3"}; // NOLINT
   return_code =
-    caosdb_transaction_transaction_retrieve_by_ids(&multi_transaction, ids);
+    caosdb_transaction_transaction_retrieve_by_ids(&multi_transaction, ids, 3);
   EXPECT_EQ(return_code, caosdb::StatusCode::GO_ON);
 
   return_code = caosdb_transaction_delete_transaction(&multi_transaction);
@@ -116,7 +116,7 @@ TEST_F(test_ccaosdb, test_multi_retrieve) {
   const char *ids[] = {"id1", "id2", "id3"}; // NOLINT
   std::cout << "Adding mutli retrieval ..." << std::endl;
   int return_code(
-    caosdb_transaction_transaction_retrieve_by_ids(&multi_transaction, ids));
+    caosdb_transaction_transaction_retrieve_by_ids(&multi_transaction, ids, 3));
   EXPECT_EQ(return_code, caosdb::StatusCode::GO_ON);
 
   std::cout << "Deleting transaction ..." << std::endl;
@@ -323,8 +323,14 @@ TEST_F(test_ccaosdb, test_entity_with_parent_and_property) {
   EXPECT_EQ(return_code, 0);
   return_code = caosdb_entity_delete_entity(&entity);
   EXPECT_EQ(return_code, 0);
-  return_code = caosdb_entity_delete_parent(&output_parent);
-  EXPECT_EQ(return_code, 0);
-  return_code = caosdb_entity_delete_property(&output_property);
-  EXPECT_EQ(return_code, 0);
+
+  // TODO(fspreck) This fails, because the output_p{arent,roperty} is owned
+  // by the entity (which is already deleted). Also, this shows that it is
+  // dangerous to call the deletion of output_p{arent,roperty} before the
+  // entity is being deleted, because it screws up the RepeatedPtrFieldWrapper.
+  //
+  // return_code = caosdb_entity_delete_parent(&output_parent);
+  // EXPECT_EQ(return_code, 0);
+  // return_code = caosdb_entity_delete_property(&output_property);
+  // EXPECT_EQ(return_code, 0);
 }