From 49573d0311377888f16e9d1effdf5f3b59dd4386 Mon Sep 17 00:00:00 2001
From: Timm Fitschen <t.fitschen@indiscale.com>
Date: Fri, 3 Sep 2021 16:51:49 +0200
Subject: [PATCH] EHN: implement copy constructors and release methods
---
CMakeLists.txt | 2 +-
conanfile.py | 2 +-
include/caosdb/entity.h | 68 ++++++++++++++++++--------
include/caosdb/message_code.h | 2 +-
include/caosdb/status_code.h | 1 +
include/caosdb/transaction.h | 20 +++++++-
include/caosdb/transaction_status.h | 8 +++
src/caosdb/entity.cpp | 11 -----
src/caosdb/status_code_description.cpp | 3 ++
src/ccaosdb.cpp | 42 ++++++++++++++++
test/test_transaction.cpp | 22 +++++++++
11 files changed, 145 insertions(+), 36 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index 104a195..e931347 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 1b50a45..3748da6 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/include/caosdb/entity.h b/include/caosdb/entity.h
index d66c377..147b4ab 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;
@@ -323,7 +327,6 @@ public:
// friend class Parent;
// friend class Property;
-private:
inline Messages() : RepeatedPtrFieldWrapper(){};
};
@@ -588,25 +591,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 +622,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 +751,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/message_code.h b/include/caosdb/message_code.h
index faf04d3..a277656 100644
--- a/include/caosdb/message_code.h
+++ b/include/caosdb/message_code.h
@@ -47,7 +47,7 @@ enum MessageCode {
INTEGER_VALUE_OUT_OF_RANGE =
caosdb::entity::v1alpha1::MessageCode::MESSAGE_CODE_INTEGER_VALUE_OUT_OF_RANGE,
ENTITY_HAS_BEEN_DELETED_SUCCESSFULLY =
- MessageCode::MESSAGE_CODE_ENTITY_HAS_BEEN_DELETED_SUCCESSFULLY,
+ caosdb::entity::v1alpha1::MessageCode::MESSAGE_CODE_ENTITY_HAS_BEEN_DELETED_SUCCESSFULLY,
};
[[nodiscard]] inline auto get_message_code(int code) noexcept -> MessageCode {
diff --git a/include/caosdb/status_code.h b/include/caosdb/status_code.h
index 8a21ea3..e458b17 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 6fd9d02..569f033 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,7 @@ 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;
+ [[nodiscard]] virtual auto release_at(int index) -> Entity * = 0;
auto begin() const -> iterator;
auto end() const -> iterator;
@@ -215,6 +217,11 @@ private:
class AbstractMultiResultSet : public ResultSet {
public:
+ 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 +232,10 @@ public:
[[nodiscard]] inline auto mutable_at(int index) const -> Entity * override {
return this->items.at(index).get();
}
+ [[nodiscard]] inline auto release_at(int index) -> Entity * override {
+ return this->items.at(index).release();
+ }
+ inline auto clear() noexcept -> void { this->items.clear(); }
protected:
std::vector<std::unique_ptr<Entity>> items;
@@ -366,12 +377,17 @@ public:
[[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>>()));
+ // TODO(tf) Discuss. This condition implies a programming error. Should we handle it?
+ this->result_set = std::make_unique<MultiResultSet>(std::vector<std::unique_ptr<Entity>>());
}
return *(this->result_set.get());
}
+ [[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 218ac86..bdfd655 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/src/caosdb/entity.cpp b/src/caosdb/entity.cpp
index 293368b..167e3ae 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 d434d9f..d5e08e7 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 8145d92..8e68dbf 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 db2d76a..7c93ac5 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
--
GitLab