From f38dbd12b86676ad7b665925cfafcf6de2c14606 Mon Sep 17 00:00:00 2001
From: Timm Fitschen <t.fitschen@indiscale.com>
Date: Thu, 12 Aug 2021 13:43:08 +0200
Subject: [PATCH] API: remove UniqueResult, lower-case at, size for ResultSet
---
include/caosdb/transaction.h | 43 ++--------
proto | 2 +-
src/caosdb/transaction.cpp | 155 ++++++++++++++++++++---------------
test/test_transaction.cpp | 60 +++++++-------
4 files changed, 127 insertions(+), 133 deletions(-)
diff --git a/include/caosdb/transaction.h b/include/caosdb/transaction.h
index e2a2ebc..eed8f1c 100644
--- a/include/caosdb/transaction.h
+++ b/include/caosdb/transaction.h
@@ -34,7 +34,6 @@
#include <iterator>
// IWYU pragma: no_include <ext/alloc_traits.h>
#include <memory> // for shared_ptr, unique_ptr
-#include <stdexcept>
#include <string> // for string
#include <vector> // for vector
@@ -179,8 +178,9 @@ class ResultSet {
public:
virtual ~ResultSet() = default;
- [[nodiscard]] virtual auto Size() const noexcept -> int = 0;
- [[nodiscard]] virtual auto At(const int index) const -> const Entity & = 0;
+ [[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;
auto begin() const -> iterator;
auto end() const -> iterator;
@@ -201,49 +201,22 @@ private:
/**
* Container with results of a transaction.
- *
- * In contrast to UniqueResult, this one can also hold multiple entities or zero
- * entities.
*/
class MultiResultSet : public ResultSet {
public:
~MultiResultSet() = default;
explicit MultiResultSet(std::vector<std::unique_ptr<Entity>> result_set);
- [[nodiscard]] inline auto Size() const noexcept -> int override {
+ [[nodiscard]] inline auto size() const noexcept -> int override {
return this->entities.size();
}
- [[nodiscard]] inline auto At(const int index) const
+ [[nodiscard]] inline auto at(const int index) const
-> const Entity & override {
return *(this->entities.at(index));
}
- std::vector<std::unique_ptr<Entity>> entities;
-};
-
-/**
- * Container with the single result of a transaction.
- *
- * In contrast to MultiResultSet, this one guarantees to hold exactly one
- * entity.
- */
-class UniqueResult : public ResultSet {
-public:
- ~UniqueResult() = default;
- explicit inline UniqueResult(ProtoEntity *protoEntity)
- : entity(new Entity(protoEntity)){};
- explicit inline UniqueResult(IdResponse *idResponse)
- : entity(new Entity(idResponse)){};
- [[nodiscard]] auto GetEntity() const -> const Entity &;
- [[nodiscard]] inline auto Size() const noexcept -> int override { return 1; }
- [[nodiscard]] inline auto At(const int index) const
- -> const Entity & override {
- if (index != 0) {
- throw std::out_of_range("Index out of range. Length is 1.");
- }
- return *(this->entity);
+ [[nodiscard]] inline auto mutable_at(int index) const -> Entity * override {
+ return this->entities.at(index).get();
}
-
-private:
- std::unique_ptr<Entity> entity;
+ std::vector<std::unique_ptr<Entity>> entities;
};
/**
diff --git a/proto b/proto
index 36d7956..45d120f 160000
--- a/proto
+++ b/proto
@@ -1 +1 @@
-Subproject commit 36d7956b6eca506fb87096d8d50b6f4b820778b8
+Subproject commit 45d120f78f17986ba67c10b6a4a130e7fa60b34c
diff --git a/src/caosdb/transaction.cpp b/src/caosdb/transaction.cpp
index 61f5278..23f1331 100644
--- a/src/caosdb/transaction.cpp
+++ b/src/caosdb/transaction.cpp
@@ -107,7 +107,7 @@ ResultSet::iterator::iterator(const ResultSet *result_set_param, int index)
: current_index(index), result_set(result_set_param) {}
auto ResultSet::iterator::operator*() const -> const Entity & {
- return this->result_set->At(current_index);
+ return this->result_set->at(current_index);
}
auto ResultSet::iterator::operator++() -> ResultSet::iterator & {
@@ -130,17 +130,12 @@ auto ResultSet::begin() const -> ResultSet::iterator {
}
auto ResultSet::end() const -> ResultSet::iterator {
- return ResultSet::iterator(this, Size());
+ return ResultSet::iterator(this, size());
}
MultiResultSet::MultiResultSet(std::vector<std::unique_ptr<Entity>> result_set)
: entities(std::move(result_set)) {}
-[[nodiscard]] auto UniqueResult::GetEntity() const -> const Entity & {
- const Entity *result = this->entity.get();
- return *result;
-}
-
Transaction::Transaction(
std::shared_ptr<EntityTransactionService::Stub> service_stub)
: request(google::protobuf::Arena::CreateMessage<MultiTransactionRequest>(
@@ -279,86 +274,114 @@ auto Transaction::ExecuteAsynchronously() noexcept -> StatusCode {
}
auto Transaction::WaitForIt() const noexcept -> TransactionStatus {
- if (this->response->responses_size() == 1) {
- auto *responses = this->response->mutable_responses(0);
- switch (responses->wrapped_response_case()) {
+ // if (this->response->responses_size() == 1) {
+ // auto *responses = this->response->mutable_responses(0);
+ // switch (responses->wrapped_response_case()) {
+ // case WrappedResponseCase::kRetrieveResponse: {
+ // auto *retrieve_response = responses->mutable_retrieve_response();
+ // switch (retrieve_response->query_response_case()) {
+ // case QueryResponseCase::kEntity: {
+ // auto *entity = retrieve_response->release_entity();
+ // if (!entity->errors().empty()) {
+ // this->status = TransactionStatus::TRANSACTION_ERROR(
+ //"The request returned with errors.");
+ //}
+ // this->result_set = std::make_unique<UniqueResult>(entity);
+ //} break;
+ // case QueryResponseCase::kSelectResult: {
+ //// TODO(tf) Select queries
+ //} break;
+ // case QueryResponseCase::kCountResult: {
+ // this->query_count = retrieve_response->count_result();
+ // std::vector<std::unique_ptr<Entity>> entities;
+ // this->result_set =
+ // std::make_unique<MultiResultSet>(std::move(entities));
+ //} break;
+ // default:
+ //// TODO(tf) Error
+ // break;
+ //}
+ //} break;
+ // case WrappedResponseCase::kUpdateResponse: {
+ // auto *updatedIdResponse = responses->mutable_update_response();
+ // if (!updatedIdResponse->entity_errors().empty()) {
+ // this->status = TransactionStatus::TRANSACTION_ERROR(
+ //"The request returned with errors.");
+ //}
+ // this->result_set = std::make_unique<UniqueResult>(updatedIdResponse);
+ //} break;
+ // case WrappedResponseCase::kInsertResponse: {
+ // auto *insertedIdResponse = responses->mutable_insert_response();
+ // if (!insertedIdResponse->entity_errors().empty()) {
+ // this->status = TransactionStatus::TRANSACTION_ERROR(
+ //"The request returned with errors.");
+ //}
+ // this->result_set = std::make_unique<UniqueResult>(insertedIdResponse);
+ //} break;
+ // default:
+ //// TODO(tf) Error and Update
+ // break;
+ //}
+ //} else {
+ auto *responses = this->response->mutable_responses();
+ std::vector<std::unique_ptr<Entity>> entities;
+ for (auto sub_response : *responses) {
+ std::unique_ptr<Entity> result;
+ switch (sub_response.wrapped_response_case()) {
+
case WrappedResponseCase::kRetrieveResponse: {
- auto *retrieve_response = responses->mutable_retrieve_response();
+ auto *retrieve_response = sub_response.mutable_retrieve_response();
+
switch (retrieve_response->query_response_case()) {
case QueryResponseCase::kEntity: {
- auto *entity = retrieve_response->release_entity();
- if (!entity->errors().empty()) {
- this->status = TransactionStatus::TRANSACTION_ERROR(
- "The request returned with errors.");
- }
- this->result_set = std::make_unique<UniqueResult>(entity);
+ auto *retrieve_entity_response = retrieve_response->release_entity();
+ result = std::make_unique<Entity>(retrieve_entity_response);
} break;
case QueryResponseCase::kSelectResult: {
// TODO(tf) Select queries
} break;
case QueryResponseCase::kCountResult: {
this->query_count = retrieve_response->count_result();
- std::vector<std::unique_ptr<Entity>> entities;
- this->result_set =
- std::make_unique<MultiResultSet>(std::move(entities));
} break;
default:
// TODO(tf) Error
break;
}
- } break;
- case WrappedResponseCase::kUpdateResponse: {
- auto *updatedIdResponse = responses->mutable_update_response();
- if (!updatedIdResponse->entity_errors().empty()) {
- this->status = TransactionStatus::TRANSACTION_ERROR(
- "The request returned with errors.");
- }
- this->result_set = std::make_unique<UniqueResult>(updatedIdResponse);
- } break;
+
+ break;
+ }
+
+ // case WrappedResponseCase::kRetrieveResponse:
+ // auto *retrieve_entity_response =
+ // sub_response.mutable_retrieve_response()->release_entity(); result =
+ // std::make_unique<Entity>(retrieve_entity_response); break;
case WrappedResponseCase::kInsertResponse: {
- auto *insertedIdResponse = responses->mutable_insert_response();
- if (!insertedIdResponse->entity_errors().empty()) {
- this->status = TransactionStatus::TRANSACTION_ERROR(
- "The request returned with errors.");
- }
- this->result_set = std::make_unique<UniqueResult>(insertedIdResponse);
- } break;
+ auto *inserted_id_response = sub_response.release_insert_response();
+ result = std::make_unique<Entity>(inserted_id_response);
+ break;
+ }
case WrappedResponseCase::kDeleteResponse: {
- auto *deletedIdResponse = responses->mutable_delete_response();
- if (!deletedIdResponse->entity_errors().empty()) {
- this->status = TransactionStatus::TRANSACTION_ERROR(
- "The request returned with errors.");
- }
- this->result_set = std::make_unique<UniqueResult>(deletedIdResponse);
- } break;
+ auto *deleted_id_response = sub_response.release_delete_response();
+ result = std::make_unique<Entity>(deleted_id_response);
+ break;
+ }
+ case WrappedResponseCase::kUpdateResponse: {
+ auto *updated_id_response = sub_response.release_update_response();
+ result = std::make_unique<Entity>(updated_id_response);
+ break;
+ }
default:
- // TODO(tf) Error and Update
+ // TODO(tf) Error
break;
}
- } else {
- auto *responses = this->response->mutable_responses();
- std::vector<std::unique_ptr<Entity>> entities;
- for (auto sub_response : *responses) {
- switch (sub_response.wrapped_response_case()) {
- case WrappedResponseCase::kRetrieveResponse:
- entities.push_back(std::make_unique<Entity>(
- sub_response.mutable_retrieve_response()->release_entity()));
- break;
- case WrappedResponseCase::kInsertResponse:
- entities.push_back(
- std::make_unique<Entity>(sub_response.release_insert_response()));
- break;
- case WrappedResponseCase::kDeleteResponse:
- entities.push_back(
- std::make_unique<Entity>(sub_response.release_insert_response()));
- break;
- default:
- // TODO(tf) Updates
- break;
- }
+ if (result->HasErrors()) {
+ this->status = TransactionStatus::TRANSACTION_ERROR(
+ "The request terminated with errors.");
}
- this->result_set = std::make_unique<MultiResultSet>(std::move(entities));
+ entities.push_back(std::move(result));
}
+ this->result_set = std::make_unique<MultiResultSet>(std::move(entities));
+ //}
return this->status;
}
diff --git a/test/test_transaction.cpp b/test/test_transaction.cpp
index b062cb8..3abe7c1 100644
--- a/test/test_transaction.cpp
+++ b/test/test_transaction.cpp
@@ -23,7 +23,7 @@
#include "caosdb/entity/v1alpha1/main.pb.h" // for Entity
#include "caosdb/exceptions.h" // for ConnectionError
#include "caosdb/status_code.h"
-#include "caosdb/transaction.h" // for Transaction, UniqueResult
+#include "caosdb/transaction.h" // for Transaction
#include "caosdb/transaction_status.h" // for ConnectionError
#include "caosdb_test_utility.h" // for EXPECT_THROW_MESSAGE
#include "gtest/gtest-message.h" // for Message
@@ -38,10 +38,31 @@ namespace caosdb::transaction {
using caosdb::configuration::InsecureConnectionConfiguration;
using caosdb::connection::Connection;
using caosdb::exceptions::ConnectionError;
-using caosdb::transaction::UniqueResult;
+using caosdb::entity::Entity;
using ProtoEntity = caosdb::entity::v1alpha1::Entity;
using caosdb::entity::v1alpha1::RetrieveResponse;
+TEST(test_transaction, test_multi_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));
+
+ EXPECT_EQ(result_set.size(), 5);
+ EXPECT_EQ(result_set.mutable_at(3)->GetName(), "E2");
+ EXPECT_EQ(result_set.at(4)->GetName(), "E3");
+ EXPECT_EQ(result_set.at(4)->GetName(), "E3");
+ EXPECT_THROW(result_set.at(15), std::out_of_range);
+
+ int counter = 0;
+ for(const auto &entity : result_set) {
+ EXPECT_EQ(entity.GetName(), "E" + std::to_string(counter++));
+ }
+ EXPECT_EQ(counter, 5);
+}
+
TEST(test_transaction, create_transaction) {
const auto *host = "localhost";
auto configuration = InsecureConnectionConfiguration(host, 8000);
@@ -55,18 +76,6 @@ TEST(test_transaction, create_transaction) {
"connection to the server could not be established.");
}
-TEST(test_transaction, unique_result) {
- auto *entity = new ProtoEntity();
- entity->set_id("test");
- UniqueResult result(entity);
-
- EXPECT_EQ("test", result.GetEntity().GetId());
-
- // DON'T DELETE! The caosdb::entity::Entity takes care of that
- // Try it yourself:
- // delete entity;
-}
-
TEST(test_transaction, test_unavailable) {
const auto *host = "localhost";
auto configuration = InsecureConnectionConfiguration(host, 8000);
@@ -96,7 +105,7 @@ TEST(test_transaction, test_retrieve_by_ids) {
TEST(test_transaction, test_multi_result_set_empty) {
std::vector<std::unique_ptr<Entity>> empty;
MultiResultSet rs(std::move(empty));
- EXPECT_EQ(rs.Size(), 0);
+ EXPECT_EQ(rs.size(), 0);
}
TEST(test_transaction, test_multi_result_iterator) {
@@ -106,19 +115,8 @@ TEST(test_transaction, test_multi_result_iterator) {
one_elem.push_back(std::make_unique<Entity>(response.release_entity()));
MultiResultSet rs(std::move(one_elem));
- EXPECT_EQ(rs.Size(), 1);
-
- for (const Entity &entity : rs) {
- EXPECT_EQ(entity.GetId(), "100");
- }
-}
-
-TEST(test_transaction, test_unique_result_iterator) {
- caosdb::entity::v1alpha1::Entity response;
- response.set_id("100");
+ EXPECT_EQ(rs.size(), 1);
- UniqueResult rs(&response);
- EXPECT_EQ(rs.Size(), 1);
for (const Entity &entity : rs) {
EXPECT_EQ(entity.GetId(), "100");
}
@@ -131,8 +129,8 @@ TEST(test_transaction, test_multi_result_set_one) {
one_elem.push_back(std::make_unique<Entity>(response.release_entity()));
MultiResultSet rs(std::move(one_elem));
- EXPECT_EQ(rs.Size(), 1);
- EXPECT_EQ(rs.At(0).GetId(), "100");
+ EXPECT_EQ(rs.size(), 1);
+ EXPECT_EQ(rs.at(0).GetId(), "100");
}
TEST(test_transaction, test_multi_result_set_three) {
@@ -160,8 +158,8 @@ TEST(test_transaction, test_multi_result_set_three) {
}
MultiResultSet rs(std::move(three_elem));
- EXPECT_EQ(rs.Size(), 3);
- EXPECT_TRUE(rs.At(1).HasErrors());
+ EXPECT_EQ(rs.size(), 3);
+ EXPECT_TRUE(rs.at(1).HasErrors());
}
TEST(test_transaction, test_update_entity) {
--
GitLab