diff --git a/include/caosdb/entity.h b/include/caosdb/entity.h index 0102ac948391f11e762889dab4ef6103b6a26326..7edee7663303261b54f417967c99f8b2cd53eb26 100644 --- a/include/caosdb/entity.h +++ b/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; }; /** diff --git a/include/ccaosdb.h b/include/ccaosdb.h index 42a02d7f259befad2eff50ad83377a70d876c6ac..a0643a5515e002f6c2659f104ef33481077814ec 100644 --- a/include/ccaosdb.h +++ b/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( diff --git a/src/caosdb/entity.cpp b/src/caosdb/entity.cpp index f90f8fbb434a9c1aa607a9c1a4d81209289a661b..7cea00fa41e12c68df89e8bacdb92524e87cd133 100644 --- a/src/caosdb/entity.cpp +++ b/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. diff --git a/src/ccaosdb.cpp b/src/ccaosdb.cpp index 4716970851af786eb0f0d0bd0d5e25bfbc77e8f6..4c9a4f07bd8f11bece7e5babcf0739629a4508e5 100644 --- a/src/ccaosdb.cpp +++ b/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; }) diff --git a/test/test_ccaosdb.cpp b/test/test_ccaosdb.cpp index 6d1a33c7f0956a154669b3d7c4d0d5848371aee8..c957b3c2d46885a36732a79e8cf8c714d7a7a40e 100644 --- a/test/test_ccaosdb.cpp +++ b/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); }