Merge branch 'f-remove-unique-result' into 'f-extended-c'

API: remove UniqueResult, lower-case at, size for ResultSet

See merge request !10

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;
 };
 
 /**

proto @ 45d120f7

-Subproject commit 36d7956b6eca506fb87096d8d50b6f4b820778b8
+Subproject commit 45d120f78f17986ba67c10b6a4a130e7fa60b34c

src/caosdb/transaction.cpp

@@ -110,7 +110,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 & {
@@ -133,17 +133,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>(
@@ -282,86 +277,61 @@ 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()) {
+  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::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;
 }

src/ccaosdb.cpp

@@ -512,8 +512,8 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                     auto *wrapped_result_set =
                       static_cast<caosdb::transaction::MultiResultSet *>(
                         result_set->wrapped_result_set);
-                    auto requested_entity = wrapped_result_set->At(index);
-                    entity->wrapped_entity = (&requested_entity);
+                    entity->wrapped_entity =
+                      wrapped_result_set->mutable_at(index);
                     return 0;
                   })
 
@@ -524,7 +524,7 @@ ERROR_RETURN_CODE(GENERIC_ERROR,
                     auto *wrapped_result_set =
                       static_cast<caosdb::transaction::MultiResultSet *>(
                         result_set->wrapped_result_set);
-                    int size(wrapped_result_set->Size());
+                    int size(wrapped_result_set->size());
                     *out = size;
                     return 0;
                   })

test/test_transaction.cpp

@@ -23,13 +23,14 @@
 #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
 #include "gtest/gtest-test-part.h"     // for SuiteApiResolver, TestPa...
 #include "gtest/gtest_pred_impl.h"     // for Test, TestInfo, TEST
 #include <memory>                      // for allocator, unique_ptr
+#include <stdexcept>                   // for out_of_range
 #include <string>                      // for string, basic_string
 #include <utility>                     // for move
 #include <vector>                      // for vector
@@ -37,11 +38,35 @@
 namespace caosdb::transaction {
 using caosdb::configuration::InsecureConnectionConfiguration;
 using caosdb::connection::Connection;
+using caosdb::entity::Entity;
 using caosdb::exceptions::ConnectionError;
-using caosdb::transaction::UniqueResult;
 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(2)->GetName(), "E2");
+  EXPECT_EQ(result_set.at(3).GetName(), "E3");
+  EXPECT_EQ(result_set.at(3).GetName(), "E3");
+
+  // discarding a [[nodiscard]] return value here (ok, we're only
+  // interested in the error anyway), so ignore when linting
+  EXPECT_THROW(result_set.at(15), std::out_of_range); // NOLINT
+
+  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 +80,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 +109,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 +119,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 +133,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 +162,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) {