diff --git a/CMakeLists.txt b/CMakeLists.txt index 104a19503abee599aea0708c5ef51a8a7a4b06fe..e931347885bc39a64e131d8ae8003a67ebf72847 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -20,7 +20,7 @@ cmake_minimum_required(VERSION 3.13) -set(libcaosdb_VERSION 0.0.15) +set(libcaosdb_VERSION 0.0.16) set(libcaosdb_COMPATIBLE_SERVER_VERSION_MAJOR 0) set(libcaosdb_COMPATIBLE_SERVER_VERSION_MINOR 5) set(libcaosdb_COMPATIBLE_SERVER_VERSION_PATCH 0) diff --git a/conanfile.py b/conanfile.py index 1b50a4511ca456f6f08b58b182e0d2493793a249..3748da681bff0aa278059c0a4e42df0d6432e67a 100644 --- a/conanfile.py +++ b/conanfile.py @@ -3,7 +3,7 @@ from conans import ConanFile, CMake, tools class CaosdbConan(ConanFile): name = "caosdb" - version = "0.0.15" + version = "0.0.16" license = "AGPL-3.0-or-later" author = "Timm C. Fitschen <t.fitschen@indiscale.com>" url = "https://gitlab.indiscale.com/caosdb/src/caosdb-cpplib.git" diff --git a/doc/capi/index.rst.in b/doc/capi/index.rst.in index a15f3b876035058113a61f2325cb739f01a39fbb..dad5482336472aa01355467687b2441b0a282a7b 100644 --- a/doc/capi/index.rst.in +++ b/doc/capi/index.rst.in @@ -29,7 +29,11 @@ C API When working with libcaosdb's C API keep the following in mind. Delete all objects (transactions, entities, properties, parents, ...) that you created using a `caosdb_..._create_...` - function and only those. + function or released using a `caosdb_..._release_...` and only + those. This means, that any objects set by a `caosdb_..._get_...` + function, are managed by another owning object (e.g., a connection + object managed by the connection manager) should not be deleted + manually. The underlying reason is that all C++ objects are realized in the Extern C interface as mutable structs containing a void pointer to diff --git a/include/caosdb/entity.h b/include/caosdb/entity.h index d66c377e57e9e23f61315d58fee647032f39b5a8..fea060d726d67c0cf6f21d42f6b8f3ead0622671 100644 --- a/include/caosdb/entity.h +++ b/include/caosdb/entity.h @@ -33,6 +33,7 @@ #include "caosdb/entity/v1alpha1/main.pb.h" // for RepeatedPtrField #include "caosdb/logging.h" // for CAOSDB_LOG_WARN #include "caosdb/message_code.h" // for get_message_code +#include "caosdb/protobuf_helper.h" // for get_arena #include "caosdb/status_code.h" // for StatusCode #include "caosdb/value.h" // for Value #include <boost/filesystem/operations.hpp> // for exists, is_di... @@ -67,6 +68,7 @@ using ProtoImportance = caosdb::entity::v1alpha1::Importance; using caosdb::StatusCode; using caosdb::entity::v1alpha1::EntityResponse; using caosdb::entity::v1alpha1::FileTransmissionId; +using caosdb::utility::get_arena; using ::google::protobuf::RepeatedPtrField; using google::protobuf::RepeatedPtrField; @@ -202,6 +204,8 @@ protected: } } + inline auto Clear() noexcept -> void { this->wrapped->Clear(); } + ::google::protobuf::RepeatedPtrField<P> *wrapped; mutable std::map<int, T> cache; @@ -588,25 +592,30 @@ private: }; /** - * @brief Wrapper for the Protobuf entity. + * Entity is the central and basic data object of CaosDB. + * + * This class is a wrapper of the Entity class auto-generated by protobuf + * (henceforth "ProtoEntity"). + * + * Overview of the Constructors: + * + * <li> Entity() - Calls Entity(ProtoEntity *) with a fresh ProtoEntity + * <li> Entity(Entity) - Copy constructor, calls Entity(ProtoEntity *) after copying wrapped + * ProtoEntity of the original, then also copies all Messages. <li> Entity(ProtoEntity *) - The + * workhorse of the constructors. Initializes everything and does not call other Entity + * constructors. <li> Entity(EntityResponse *) - Constructor which is used by the Transaction class + * to create an Entity from the server's response, calls Entity(ProtoEntity). <li> Entity(IdResponse + * *) - Constructor which is used by the Transaction class to create an Entity from the servers's + * response. calls Entity(), then moves the data to the wrapped ProtoEntity. + * */ class Entity { public: - Entity(); - inline Entity(const Entity &original) - : wrapped(original.wrapped), value(Value(original.wrapped->mutable_value())), - data_type(DataType(original.wrapped->mutable_data_type())) { - this->wrapped->CopyFrom(*original.wrapped); - data_type.wrapped = this->wrapped->mutable_data_type(); - value.wrapped = this->wrapped->mutable_value(); - properties.wrapped = this->wrapped->mutable_properties(); - parents.wrapped = this->wrapped->mutable_parents(); - // FIXME(dh) copy messages? - errors.wrapped = CreateMessagesField(); - warnings.wrapped = CreateMessagesField(); - infos.wrapped = CreateMessagesField(); + inline Entity(const Entity &original) : Entity(Copy(*original.wrapped)) { + this->errors.wrapped->CopyFrom(*original.errors.wrapped); + this->warnings.wrapped->CopyFrom(*original.warnings.wrapped); + this->infos.wrapped->CopyFrom(*original.infos.wrapped); }; - explicit Entity(IdResponse *id_response); explicit Entity(ProtoEntity *other) : wrapped(other), value(Value(other->mutable_value())), data_type(DataType(other->mutable_data_type())) { @@ -614,17 +623,26 @@ public: value.wrapped = this->wrapped->mutable_value(); properties.wrapped = this->wrapped->mutable_properties(); parents.wrapped = this->wrapped->mutable_parents(); - // FIXME(dh) copy messages? errors.wrapped = CreateMessagesField(); warnings.wrapped = CreateMessagesField(); infos.wrapped = CreateMessagesField(); }; explicit inline Entity(EntityResponse *response) : Entity(response->release_entity()) { - errors.wrapped->Swap(response->mutable_errors()); - warnings.wrapped->Swap(response->mutable_warnings()); - infos.wrapped->Swap(response->mutable_infos()); + this->errors.wrapped->Swap(response->mutable_errors()); + this->warnings.wrapped->Swap(response->mutable_warnings()); + this->infos.wrapped->Swap(response->mutable_infos()); + }; + + explicit inline Entity(IdResponse *id_response) : Entity() { + this->wrapped->set_id(id_response->id()); + this->wrapped->mutable_version()->Swap(id_response->mutable_version()); + this->errors.wrapped->Swap(id_response->mutable_errors()); + this->warnings.wrapped->Swap(id_response->mutable_warnings()); + this->infos.wrapped->Swap(id_response->mutable_infos()); }; + explicit inline Entity() : Entity(Entity::CreateProtoEntity()){}; + [[nodiscard]] inline auto GetId() const noexcept -> const std::string & { return wrapped->id(); }; [[nodiscard]] inline auto HasId() const noexcept -> bool { return !wrapped->id().empty(); } [[nodiscard]] inline auto GetVersionId() const -> const std::string & { @@ -734,7 +752,18 @@ public: return StatusCode::SUCCESS; } + inline auto ClearMessages() noexcept -> void { + errors.Clear(); + warnings.Clear(); + infos.Clear(); + } + private: + static inline auto Copy(const ProtoEntity &from) -> ProtoEntity * { + auto to = from.New(); + to->CopyFrom(from); + return to; + } inline auto GetNextFileId() -> std::string { std::string str = "0123456789abcdef"; std::mt19937 generator(std::random_device{}()); diff --git a/include/caosdb/status_code.h b/include/caosdb/status_code.h index 8a21ea32ef10c28e194b29e0ab91b63a4a62bf65..e458b1727af51720451cbc057d4e40cee35a4a4e 100644 --- a/include/caosdb/status_code.h +++ b/include/caosdb/status_code.h @@ -76,6 +76,7 @@ enum StatusCode { FILE_UPLOAD_ERROR = 35, FILE_DOWNLOAD_ERROR = 36, ENUM_MAPPING_ERROR = 37, + SPOILED = 38, OTHER_CLIENT_ERROR = 9999, }; diff --git a/include/caosdb/transaction.h b/include/caosdb/transaction.h index 6fd9d026a728990710973469887e19799c69fd81..0699e14b28ae6e4b44ff495695c843ffa2e8d01f 100644 --- a/include/caosdb/transaction.h +++ b/include/caosdb/transaction.h @@ -38,6 +38,7 @@ #include <google/protobuf/generated_message_util.h> // for CreateMessage... #include <google/protobuf/util/json_util.h> // for MessageToJsonS... #include <grpcpp/impl/codegen/completion_queue.h> // for CompletionQueue +#include <algorithm> // for max #include <iterator> // for iterator, next #include <map> // for map // IWYU pragma: no_include <ext/alloc_traits.h> @@ -195,6 +196,15 @@ public: [[nodiscard]] virtual auto size() const noexcept -> int = 0; [[nodiscard]] virtual auto at(const int index) const -> const Entity & = 0; [[nodiscard]] virtual auto mutable_at(int index) const -> Entity * = 0; + /** + * Return the Entity at the given index. + * + * This method releases the entity from the underlying collection and thus + * leaves the ResultSet in a corrupted state. + * + * This method can be called only once for each index. + */ + [[nodiscard]] virtual auto release_at(int index) -> Entity * = 0; auto begin() const -> iterator; auto end() const -> iterator; @@ -215,6 +225,16 @@ private: class AbstractMultiResultSet : public ResultSet { public: + /** + * Copy Constructor. + * + * Copies the underlying collection of entities. + */ + inline AbstractMultiResultSet(const AbstractMultiResultSet &original) { + for (const Entity &entity : original) { + this->items.push_back(std::make_unique<Entity>(entity)); + } + } virtual ~AbstractMultiResultSet() = default; inline explicit AbstractMultiResultSet(std::vector<std::unique_ptr<Entity>> result_set) : items(std::move(result_set)) {} @@ -225,6 +245,21 @@ public: [[nodiscard]] inline auto mutable_at(int index) const -> Entity * override { return this->items.at(index).get(); } + /** + * Return the Entity at the given index. + * + * This method releases the entity from the underlying collection and thus + * leaves the ResultSet in a corrupted state. + * + * This method can be called only once for each index. + */ + [[nodiscard]] inline auto release_at(int index) -> Entity * override { + return this->items.at(index).release(); + } + /** + * Remove all entities from this result set. + */ + inline auto clear() noexcept -> void { this->items.clear(); } protected: std::vector<std::unique_ptr<Entity>> items; @@ -364,14 +399,43 @@ public: */ [[nodiscard]] inline auto GetStatus() const noexcept -> TransactionStatus { return this->status; } + /** + * Return the ResultSet of this transaction. + * + * Note: If this method is called before the transaction has terminated + * (GetStatus().GetCode() < 0) this method has undefined behavior. + * + * Instead, do Execute() or WaitForIt() and only call this method afterwards. + */ [[nodiscard]] inline auto GetResultSet() const noexcept -> const ResultSet & { if (!this->result_set) { - this->result_set = - std::make_unique<MultiResultSet>(std::move(std::vector<std::unique_ptr<Entity>>())); + CAOSDB_LOG_ERROR(logger_name) + << "GetResultSet was called before the transaction has terminated. This is a programming " + "error of the code which uses the transaction."; + // TODO(tf) This is a really bad SegFault factory. When the transaction + // terminates and the result_set is being overriden, the unique_ptr + // created here will be deleted and any client of the return ResultSet + // will have a SegFault. + this->result_set = std::make_unique<MultiResultSet>(std::vector<std::unique_ptr<Entity>>()); } return *(this->result_set.get()); } + /** + * Return the ResultSet of this transaction. + * + * This method releases the ResultSet from the transaction leaving it in a + * currupted state. + * + * This method can be called only once and only after the transaction has terminated. + * + * Otherwise, this method has undefined behavior. + */ + [[nodiscard]] inline auto ReleaseResultSet() noexcept -> const ResultSet * { + this->status = TransactionStatus::SPOILED(); + return this->result_set.release(); + } + /** * Return the result of a count query * diff --git a/include/caosdb/transaction_status.h b/include/caosdb/transaction_status.h index 218ac8614ca73ed16d7d15d9793c698a8446c5b9..bdfd65595090acde2b177dd7486492738901b0e0 100644 --- a/include/caosdb/transaction_status.h +++ b/include/caosdb/transaction_status.h @@ -137,6 +137,14 @@ public: */ CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(TRANSACTION_ERROR, StatusCode::GENERIC_TRANSACTION_ERROR) + /** + * Factory for a SPOILED status. + * + * This status means that the transaction's result set has been released and + * GetResultSet() will not return the actual results of the transaction + * anymore. + */ + CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(SPOILED, StatusCode::SPOILED); /** * Another factory for a TRANSACTION_ERROR status with a detailed * description. diff --git a/include/caosdb/value.h b/include/caosdb/value.h index e6ca1f33bcd43ce41acd44e4244c8d3825b43ebe..6031e75dbd69c262cc69079c63323d916cec223d 100644 --- a/include/caosdb/value.h +++ b/include/caosdb/value.h @@ -63,11 +63,9 @@ public: return (this->wrapped->scalar_value_case() == ScalarValueCase::kStringValue) || (this->wrapped->scalar_value_case() == ScalarValueCase::kSpecialValue && this->wrapped->special_value() == ProtoSpecialValue::SPECIAL_VALUE_EMPTY_STRING); - return false; } [[nodiscard]] inline auto AsString() const noexcept -> const std::string & { return this->wrapped->string_value(); - ; } [[nodiscard]] inline auto IsDouble() const noexcept -> bool { diff --git a/include/ccaosdb.h b/include/ccaosdb.h index a60189522dfa4f5493fa201f63c3eb3df0d992d7..788cf0bdaa49bea9fffa787a66d55d35addbd14a 100644 --- a/include/ccaosdb.h +++ b/include/ccaosdb.h @@ -297,17 +297,46 @@ typedef struct caosdb_transaction_result_set { bool _deletable = false; } caosdb_transaction_result_set; +typedef struct caosdb_entity_entity { + void *wrapped_entity; + bool _deletable = false; +} caosdb_entity_entity; + int caosdb_transaction_transaction_get_result_set(caosdb_transaction_transaction *transaction, caosdb_transaction_result_set *out); +/** + * Release the result set from the transaction. + * + * The transactions is spoiled after this action and should not be used anymore. + * + * Note: The result_set has to be deleted via caosdb_transaction_delete_result_set. + * + * EXPERT USE ONLY. Only use it when you know what you are doing. + */ +int caosdb_transaction_transaction_release_result_set(caosdb_transaction_transaction *transaction, + caosdb_transaction_result_set *out); +/** + * Release the entity from the result set. + * + * Each entity (each index) can be released once. The result set is spoiled + * after this action and should not be used for anything else anymore. + * + * Note: The result_set has to be deleted via caosdb_entity_delete_entity. + * + * EXPERT USE ONLY. Only use it when you know what you are doing. + */ +int caosdb_transaction_result_set_release_at(caosdb_transaction_result_set *result_set, + caosdb_entity_entity *entity, int index); +/** + * Destructor for caosdb_transaction_result_set. + * + * EXPERT USE ONLY. Only use it when you know what you are doing. + */ +int caosdb_transaction_delete_result_set(caosdb_transaction_result_set *result_set); int caosdb_transaction_transaction_get_count_result(caosdb_transaction_transaction *transaction, long *out); -typedef struct caosdb_entity_entity { - void *wrapped_entity; - bool _deletable = false; -} caosdb_entity_entity; - int caosdb_transaction_result_set_at(caosdb_transaction_result_set *result_set, caosdb_entity_entity *entity, int index); int caosdb_transaction_result_set_size(caosdb_transaction_result_set *result_set, int *out); diff --git a/src/caosdb/entity.cpp b/src/caosdb/entity.cpp index 293368b268bc44d6db1bbd5e5983b6421fe6fbbb..167e3ae994b3d2b7beaff07f5b09e37eebdb3060 100644 --- a/src/caosdb/entity.cpp +++ b/src/caosdb/entity.cpp @@ -29,7 +29,6 @@ #include <new> // for operator new 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; @@ -177,16 +176,6 @@ auto Entity::CreateProtoEntity() -> ProtoEntity * { return Arena::CreateMessage<ProtoEntity>(get_arena()); } -Entity::Entity(IdResponse *id_response) : Entity() { - this->wrapped->set_id(id_response->id()); - this->wrapped->mutable_version()->Swap(id_response->mutable_version()); - this->errors.wrapped->Swap(id_response->mutable_errors()); - this->warnings.wrapped->Swap(id_response->mutable_warnings()); - this->infos.wrapped->Swap(id_response->mutable_infos()); -} - -Entity::Entity() : Entity(Entity::CreateProtoEntity()) {} - auto Entity::CreateMessagesField() -> RepeatedPtrField<ProtoMessage> * { return Arena::CreateMessage<RepeatedPtrField<ProtoMessage>>(get_arena()); } diff --git a/src/caosdb/status_code_description.cpp b/src/caosdb/status_code_description.cpp index d434d9f913c8b7acc0f8c4bef545898c437f5dfc..d5e08e7820ca0f7380723087570f838465a9c261 100644 --- a/src/caosdb/status_code_description.cpp +++ b/src/caosdb/status_code_description.cpp @@ -156,6 +156,9 @@ auto get_status_description(int code) -> const std::string & { "to delete the old pointee first."}, {StatusCode::ENUM_MAPPING_ERROR, "The role, importance, or datatype you specified does not exist."}, + {StatusCode::SPOILED, + "The object has been used in a way that left it in a corrupted state. This does not indicate " + "any form of misuse. It just indicates that the object is spoiled for further use now."}, {StatusCode::OTHER_CLIENT_ERROR, "This is code is reserved to errors raised by other clients wrapping the " "C++ client (or its Extern C interface). This should never occur when " diff --git a/src/ccaosdb.cpp b/src/ccaosdb.cpp index 61a5ed18908da60beaac09891b7874b55ac75b4d..2e2a452729518ea5fb4acf125dea24337dc4a0e2 100644 --- a/src/ccaosdb.cpp +++ b/src/ccaosdb.cpp @@ -500,6 +500,32 @@ ERROR_RETURN_CODE( return 0; }) +ERROR_RETURN_CODE( + GENERIC_ERROR, + int caosdb_transaction_transaction_release_result_set(caosdb_transaction_transaction *transaction, + caosdb_transaction_result_set *out), + { + if (out->_deletable) { + return caosdb::StatusCode::EXTERN_C_ASSIGNMENT_ERROR; + } + auto *wrapped_transaction = + static_cast<caosdb::transaction::Transaction *>(transaction->wrapped_transaction); + out->wrapped_result_set = (void *)(wrapped_transaction->ReleaseResultSet()); + // out is the owner now, that are the semantics of ReleaseResultSet + out->_deletable = true; + return 0; + }) + +ERROR_RETURN_CODE( + GENERIC_ERROR, + int caosdb_transaction_delete_result_set(caosdb_transaction_result_set *result_set), { + if (result_set->_deletable) { + delete static_cast<caosdb::entity::Entity *>(result_set->wrapped_result_set); + } + result_set->_deletable = false; + return 0; + }) + ERROR_RETURN_CODE(GENERIC_ERROR, int caosdb_transaction_transaction_get_count_result( caosdb_transaction_transaction *transaction, long *out), @@ -521,6 +547,22 @@ ERROR_RETURN_CODE(GENERIC_ERROR, return 0; }) +ERROR_RETURN_CODE( + GENERIC_ERROR, + int caosdb_transaction_result_set_release_at(caosdb_transaction_result_set *result_set, + caosdb_entity_entity *entity, int index), + { + if (entity->_deletable) { + return caosdb::StatusCode::EXTERN_C_ASSIGNMENT_ERROR; + } + auto *wrapped_result_set = + static_cast<caosdb::transaction::MultiResultSet *>(result_set->wrapped_result_set); + entity->wrapped_entity = wrapped_result_set->release_at(index); + // entity is the owner now. That are the semantics of release_at. + entity->_deletable = true; + return 0; + }) + ERROR_RETURN_CODE(GENERIC_ERROR, int caosdb_transaction_result_set_size(caosdb_transaction_result_set *result_set, int *out), diff --git a/test/test_transaction.cpp b/test/test_transaction.cpp index db2d76a845f5548eb68570d183e1a7867a91fe15..7c93ac5d9b081ac4cb2314f34272f54cba0f61c9 100644 --- a/test/test_transaction.cpp +++ b/test/test_transaction.cpp @@ -224,4 +224,26 @@ TEST(test_transaction, test_insert_with_file) { EXPECT_EQ(transaction->GetStatus().GetCode(), StatusCode::FILE_UPLOAD_ERROR); } +TEST(test_transaction, test_copy_result_set) { + std::vector<std::unique_ptr<Entity>> entities; + for (int i = 0; i < 5; i++) { + entities.push_back(std::make_unique<Entity>()); + entities[i]->SetName("E" + std::to_string(i)); + } + MultiResultSet result_set(std::move(entities)); + MultiResultSet copy(result_set); + + EXPECT_EQ(result_set.size(), 5); + EXPECT_EQ(copy.size(), 5); + + result_set.clear(); + + EXPECT_EQ(result_set.size(), 0); + EXPECT_EQ(copy.size(), 5); + + for (int i = 0; i < 5; i++) { + EXPECT_EQ(copy.at(i).GetName(), "E" + std::to_string(i)); + } +} + } // namespace caosdb::transaction