WIP: iterator for ResultSet

include/caosdb/exceptions.h

@@ -73,10 +73,22 @@ public:
     : Exception(StatusCode::GENERIC_TRANSACTION_ERROR, what_arg) {}
 };
 
-class TransactionStatusError : public TransactionError {
+/**
+ * @brief The transaction is in the wrong state for your action.
+ */
+class TransactionStatusError : public Exception {
 public:
   explicit TransactionStatusError(const std::string &what_arg)
-    : TransactionError(StatusCode::TRANSACTION_STATUS_ERROR, what_arg) {}
+    : Exception(StatusCode::TRANSACTION_STATUS_ERROR, what_arg) {}
+};
+
+/**
+ * @brief The transaction has a wrong type for your action.
+ */
+class TransactionTypeError : public Exception {
+public:
+  explicit TransactionTypeError(const std::string &what_arg)
+    : Exception(StatusCode::TRANSACTION_TYPE_ERROR, what_arg) {}
 };
 
 /**

include/caosdb/transaction.h

@@ -31,32 +31,29 @@
 #include "caosdb/transaction_status.h"           // for TransactionStatus
 #include "caosdb/status_code.h"                  // for StatusCode
 #include "google/protobuf/util/json_util.h" // for MessageToJsonString, Jso...
-#include <stdexcept>
+#include <cstddef>                          // for size_t
 #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
 
-/**
- * @brief Creation and execution of transactions.
- * @author Timm Fitschen
- * @date 2021-08-05
- */
-namespace caosdb::transaction {
-
 /**
  * Do all necessary checks and assure that another retrieval (by id or by
  * query) can be added as a sub-request to a transaction.
  */
 #define ASSERT_CAN_ADD_RETRIEVAL                                               \
-  if (!IsStatus(TransactionStatus::INITIAL())) {                               \
+  if (!IsStatus(TransactionStatus::INITIAL()) &&                               \
+      !IsStatus(TransactionStatus::GO_ON())) {                                 \
     return StatusCode::TRANSACTION_STATUS_ERROR;                               \
   }                                                                            \
   switch (this->transaction_type) {                                            \
   case NONE:                                                                   \
     this->transaction_type = TransactionType::READ_ONLY;                       \
+    break;                                                                     \
   case READ_ONLY:                                                              \
+    break;                                                                     \
   case MIXED_READ_AND_WRITE:                                                   \
     break;                                                                     \
   default:                                                                     \
@@ -66,12 +63,26 @@ namespace caosdb::transaction {
       "wrong TransactionType.")                                                \
   }
 
+/**
+ * Do all necessary checks and assure that another retrieval (by id or by
+ * query) can be added as a sub-request to a transaction.
+ */
+#define ASSERT_CAN_ADD_QUERY                                                   \
+  ASSERT_CAN_ADD_RETRIEVAL                                                     \
+  if (this->has_query) {                                                       \
+    CAOSDB_LOG_ERROR_AND_RETURN_STATUS(                                        \
+      logger_name, StatusCode::UNSUPPORTED_FEATURE,                            \
+      "Currently the number of queries which can be processed in a single "    \
+      "transaction is limitted to one.");                                      \
+  }
+
 /**
  * Do all necessary checks and assure that another deletion can be added as a
  * sub-request to a transaction.
  */
 #define ASSERT_CAN_ADD_DELETION                                                \
-  if (!IsStatus(TransactionStatus::INITIAL())) {                               \
+  if (!IsStatus(TransactionStatus::INITIAL()) &&                               \
+      !IsStatus(TransactionStatus::GO_ON())) {                                 \
     return StatusCode::TRANSACTION_STATUS_ERROR;                               \
   }                                                                            \
   switch (this->transaction_type) {                                            \
@@ -93,7 +104,8 @@ namespace caosdb::transaction {
  * sub-request to a transaction.
  */
 #define ASSERT_CAN_ADD_INSERTION                                               \
-  if (!IsStatus(TransactionStatus::INITIAL())) {                               \
+  if (!IsStatus(TransactionStatus::INITIAL()) &&                               \
+      !IsStatus(TransactionStatus::GO_ON())) {                                 \
     return StatusCode::TRANSACTION_STATUS_ERROR;                               \
   }                                                                            \
   switch (this->transaction_type) {                                            \
@@ -110,12 +122,13 @@ namespace caosdb::transaction {
       "wrong TransactionType.")                                                \
   }
 
-/*
+/**
  * Do all necessary checks and assure that another update can be added as a
  * sub-request to a transaction.
  */
 #define ASSERT_CAN_ADD_UPDATE                                                  \
-  if (!IsStatus(TransactionStatus::INITIAL())) {                               \
+  if (!IsStatus(TransactionStatus::INITIAL()) &&                               \
+      !IsStatus(TransactionStatus::GO_ON())) {                                 \
     return StatusCode::TRANSACTION_STATUS_ERROR;                               \
   }                                                                            \
   switch (this->transaction_type) {                                            \
@@ -139,6 +152,11 @@ namespace caosdb::transaction {
       "retrieval of the existing entity which may then be changed.")           \
   }
 
+/**
+ * @brief Creation and execution of transactions.
+ * @author Timm Fitschen
+ * @date 2021-08-05
+ */
 namespace caosdb::transaction {
 using caosdb::entity::Entity;
 using ProtoEntity = caosdb::entity::v1alpha1::Entity;
@@ -158,10 +176,28 @@ static const std::string logger_name = "caosdb::transaction";
  * Abstract base class for the results of a Transaction.
  */
 class ResultSet {
+  class iterator;
+
 public:
   virtual ~ResultSet() = default;
-  [[nodiscard]] virtual auto Size() const noexcept -> int = 0;
+  [[nodiscard]] virtual auto Size() const noexcept -> size_t = 0;
   [[nodiscard]] virtual auto At(const int index) const -> const Entity & = 0;
+  auto begin() const -> iterator;
+  auto end() const -> iterator;
+
+private:
+  class iterator : public std::iterator<std::output_iterator_tag, Entity> {
+  public:
+    explicit iterator(const ResultSet *result_set, size_t index = 0);
+    auto operator*() const -> const Entity &;
+    auto operator++() -> iterator &;
+    auto operator++(int) -> iterator;
+    auto operator!=(const iterator &rhs) const -> bool;
+
+  private:
+    size_t current_index = 0;
+    const ResultSet *result_set;
+  };
 };
 
 /**
@@ -174,7 +210,7 @@ class MultiResultSet : public ResultSet {
 public:
   ~MultiResultSet() = default;
   explicit MultiResultSet(MultiTransactionResponse *response);
-  [[nodiscard]] inline auto Size() const noexcept -> int override {
+  [[nodiscard]] inline auto Size() const noexcept -> size_t override {
     return this->entities.size();
   }
   [[nodiscard]] inline auto At(const int index) const
@@ -198,7 +234,9 @@ public:
   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 Size() const noexcept -> size_t override {
+    return 1;
+  }
   [[nodiscard]] inline auto At(const int index) const
     -> const Entity & override {
     if (index != 0) {
@@ -340,6 +378,7 @@ public:
   }
 
 private:
+  bool has_query = false;
   TransactionType transaction_type = TransactionType::NONE;
   mutable std::unique_ptr<ResultSet> result_set;
   mutable TransactionStatus status = TransactionStatus::INITIAL();
@@ -362,7 +401,8 @@ inline auto Transaction::RetrieveById(InputIterator begin,
     next = std::next(next);
   }
 
-  return StatusCode::INITIAL;
+  this->status = TransactionStatus::GO_ON();
+  return this->status.GetCode();
 }
 
 } // namespace caosdb::transaction

include/caosdb/transaction_status.h

@@ -33,6 +33,18 @@ using caosdb::exceptions::AuthenticationError;
 using caosdb::exceptions::ConnectionError;
 using caosdb::exceptions::Exception;
 using caosdb::exceptions::TransactionError;
+using caosdb::exceptions::TransactionStatusError;
+using caosdb::exceptions::TransactionTypeError;
+
+/**
+ * Define static factory method in the TransactionStatus class.
+ */
+#define CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(_StatusName, _StatusCode)    \
+  inline static auto _StatusName()->const TransactionStatus & {                \
+    static const TransactionStatus instance(                                   \
+      _StatusCode, caosdb::get_status_description(_StatusCode));               \
+    return instance;                                                           \
+  }
 
 /**
  * TransactionStatus indicates the current status of a transaction and, when it
@@ -46,39 +58,62 @@ class TransactionStatus {
 public:
   // REFACTORING NEEDED: When you touch this code again consider writing a
   // macro, because this is a lot of redundant code here...
-  inline static auto INITIAL() -> const TransactionStatus & {
-    static const TransactionStatus initial(
-      StatusCode::INITIAL, caosdb::get_status_description(StatusCode::INITIAL));
-    return initial;
-  }
-  inline static auto EXECUTING() -> const TransactionStatus & {
-    static const TransactionStatus executing(
-      StatusCode::EXECUTING,
-      caosdb::get_status_description(StatusCode::EXECUTING));
-    return executing;
-  }
-  inline static auto SUCCESS() -> const TransactionStatus & {
-    static const TransactionStatus success(
-      StatusCode::SUCCESS, caosdb::get_status_description(StatusCode::SUCCESS));
-    return success;
-  }
-  inline static auto RPC_ERROR(const std::string &details)
-    -> const TransactionStatus {
-    // We use the GENERIC_RPC_ERROR here because we might want to add further
-    // RPC_ERROR states with different error codes (which stem from GRPC) here
-    // in the future.
-    return TransactionStatus(
-      StatusCode::GENERIC_RPC_ERROR,
-      caosdb::get_status_description(StatusCode::GENERIC_RPC_ERROR) +
-        " Original error: " + details);
-  }
-  inline static auto CONNECTION_ERROR() -> const TransactionStatus & {
-    static const TransactionStatus connection_error(
-      StatusCode::CONNECTION_ERROR,
-      caosdb::get_status_description(StatusCode::CONNECTION_ERROR));
-
-    return connection_error;
-  }
+  /**
+   * Factory for an INITIAL status.
+   *
+   * This status means that the transaction has not been executed yet and the
+   * transaction does not contain any sub-transactions yet.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(INITIAL, StatusCode::INITIAL)
+  /**
+   * Factory for a GO_ON status.
+   *
+   * This status means that the transaction has not been executed yet but it
+   * already contains sub-transaction and more subtransactions may be added.
+   *
+   * However, it also can be executed right now.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(GO_ON, StatusCode::GO_ON)
+  /**
+   * Factory for a READY status.
+   *
+   * This status means that the transaction has not been executed yet but it is
+   * ready to be executed and it is not possible anymore to add further
+   * sub-transactions.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(READY, StatusCode::READY)
+  /**
+   * Factory for an EXECUTING status.
+   *
+   * This status means that the transaction is currently being executed.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(EXECUTING, StatusCode::EXECUTING)
+  /**
+   * Factory for a SUCCESS status.
+   *
+   * This status means that the transaction has been executed successfully.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(SUCCESS, StatusCode::SUCCESS)
+  /**
+   * Factory for a CONNECTION_ERROR status.
+   *
+   * This status means that the connection could not be established. This is
+   * possibly due to misconfiguration of the client, errors in the network or
+   * because the server is down.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(CONNECTION_ERROR,
+                                            StatusCode::CONNECTION_ERROR)
+  /**
+   * Factory for an AUTHENTICATION_ERROR status.
+   *
+   * This status means that the RPC layer reported an authentication error.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(AUTHENTICATION_ERROR,
+                                            StatusCode::AUTHENTICATION_ERROR)
+  /**
+   * Another factory for an TRANSACTION_ERROR Status with a detailed
+   * description.
+   */
   inline static auto AUTHENTICATION_ERROR(const std::string &details)
     -> const TransactionStatus {
     return TransactionStatus(
@@ -86,19 +121,18 @@ public:
       caosdb::get_status_description(StatusCode::AUTHENTICATION_ERROR) +
         " Original error: " + details);
   }
-  inline static auto AUTHENTICATION_ERROR() -> const TransactionStatus & {
-    static const TransactionStatus authentication_error(
-      StatusCode::AUTHENTICATION_ERROR,
-      caosdb::get_status_description(StatusCode::AUTHENTICATION_ERROR));
-
-    return authentication_error;
-  }
-  inline static auto TRANSACTION_ERROR() -> const TransactionStatus & {
-    static const TransactionStatus transaction_error(
-      StatusCode::GENERIC_TRANSACTION_ERROR,
-      caosdb::get_status_description(StatusCode::GENERIC_TRANSACTION_ERROR));
-    return transaction_error;
-  }
+  /**
+   * Factory for a TRANSACTION_ERROR status.
+   *
+   * This status means that the transaction failed due to errors thrown by the
+   * server.
+   */
+  CAOSDB_TRANSACTION_STATUS_DEFAULT_FACTORY(
+    TRANSACTION_ERROR, StatusCode::GENERIC_TRANSACTION_ERROR)
+  /**
+   * Another factory for a TRANSACTION_ERROR status with a detailed
+   * description.
+   */
   inline static auto TRANSACTION_ERROR(const std::string &details)
     -> const TransactionStatus {
     return TransactionStatus(
@@ -106,23 +140,53 @@ public:
       caosdb::get_status_description(StatusCode::GENERIC_TRANSACTION_ERROR) +
         " Original error: " + details);
   }
+  /**
+   * Factory for a RPC_ERROR with a detailed description.
+   *
+   * This status is used for any error on the RPC layer.
+   */
+  inline static auto RPC_ERROR(const std::string &details)
+    -> const TransactionStatus {
+    // We use the GENERIC_RPC_ERROR here because we might want to add further
+    // RPC_ERROR states with different error codes (which stem from GRPC) here
+    // in the future.
+    return TransactionStatus(
+      StatusCode::GENERIC_RPC_ERROR,
+      caosdb::get_status_description(StatusCode::GENERIC_RPC_ERROR) +
+        " Original error: " + details);
+  }
 
   inline auto ThrowExceptionIfError() const -> void {
-    if (!IsError()) {
+    TransactionStatus::ThrowExceptionIfError(this->code, this->description);
+  }
+
+  inline static auto ThrowExceptionIfError(StatusCode code,
+                                           const std::string &description)
+    -> void {
+    if (!IsError(code)) {
       return;
     }
-    switch (this->code) {
+    switch (code) {
     case StatusCode::CONNECTION_ERROR:
-      throw ConnectionError(this->description);
+      throw ConnectionError(description);
     case StatusCode::AUTHENTICATION_ERROR:
-      throw AuthenticationError(this->description);
+      throw AuthenticationError(description);
     case StatusCode::GENERIC_TRANSACTION_ERROR:
-      throw TransactionError(this->description);
+      throw TransactionError(description);
+    case StatusCode::TRANSACTION_STATUS_ERROR:
+      throw TransactionStatusError(description);
+    case StatusCode::TRANSACTION_TYPE_ERROR:
+      throw TransactionTypeError(description);
     default:
-      throw Exception(StatusCode::GENERIC_ERROR, this->description);
+      throw Exception(StatusCode::GENERIC_ERROR, description);
     }
   }
 
+  /**
+   * Return true if the given StatusCode represents an erroneous state.
+   */
+  inline static auto IsError(StatusCode code) -> bool { return code > 0; };
+
   /**
    * Return true if this TransactionStatus represents a terminated state.
    */
@@ -131,7 +195,9 @@ public:
   /**
    * Return true if this TransactionStatus represents an erroneous state.
    */
-  inline auto IsError() const -> bool { return this->code > 0; };
+  inline auto IsError() const -> bool {
+    return TransactionStatus::IsError(this->code);
+  };
 
   /**
    * Return a description of the erroneous state.

src/caosdb/transaction.cpp

@@ -100,6 +100,36 @@ using grpc::ClientAsyncResponseReader;
 using ProtoEntity = caosdb::entity::v1alpha1::Entity;
 using grpc::CompletionQueue;
 
+ResultSet::iterator::iterator(const ResultSet *result_set_param, size_t index)
+  : current_index(index), result_set(result_set_param) {}
+
+auto ResultSet::iterator::operator*() const -> const Entity & {
+  return this->result_set->At(current_index);
+}
+
+auto ResultSet::iterator::operator++() -> ResultSet::iterator & {
+  current_index++;
+  return *this;
+}
+
+auto ResultSet::iterator::operator++(int) -> ResultSet::iterator {
+  iterator tmp(*this);
+  operator++();
+  return tmp;
+}
+
+auto ResultSet::iterator::operator!=(const iterator &rhs) const -> bool {
+  return this->current_index != rhs.current_index;
+}
+
+auto ResultSet::begin() const -> ResultSet::iterator {
+  return ResultSet::iterator(this, 0);
+}
+
+auto ResultSet::end() const -> ResultSet::iterator {
+  return ResultSet::iterator(this, Size());
+}
+
 MultiResultSet::MultiResultSet(MultiTransactionResponse *response) {
   auto *responses = response->mutable_responses();
   for (auto sub_response : *responses) {
@@ -143,16 +173,21 @@ auto Transaction::RetrieveById(const std::string &id) noexcept -> StatusCode {
   auto *sub_request = this->request->add_requests();
   sub_request->mutable_retrieve_request()->set_id(id);
 
-  return StatusCode::INITIAL;
+  this->status = TransactionStatus::GO_ON();
+  return this->status.GetCode();
 }
 
 auto Transaction::Query(const std::string &query) noexcept -> StatusCode {
-  ASSERT_CAN_ADD_RETRIEVAL
+  ASSERT_CAN_ADD_QUERY
 
+  // currently, the server can only process one query at a time (but mixed with
+  // other retrievals).
+  this->has_query = true;
   auto *sub_request = this->request->add_requests();
   sub_request->mutable_retrieve_request()->mutable_query()->set_query(query);
 
-  return StatusCode::INITIAL;
+  this->status = TransactionStatus::GO_ON();
+  return this->status.GetCode();
 }
 
 auto Transaction::DeleteById(const std::string &id) noexcept -> StatusCode {
@@ -161,7 +196,8 @@ auto Transaction::DeleteById(const std::string &id) noexcept -> StatusCode {
   auto *sub_request = this->request->add_requests();
   sub_request->mutable_delete_request()->set_id(id);
 
-  return StatusCode::INITIAL;
+  this->status = TransactionStatus::READY();
+  return this->status.GetCode();
 }
 
 auto Transaction::InsertEntity(Entity *entity) noexcept -> StatusCode {
@@ -172,7 +208,8 @@ auto Transaction::InsertEntity(Entity *entity) noexcept -> StatusCode {
 
   // copy the original entity for the transaction
   entity->CopyTo(proto_entity);
-  return StatusCode::INITIAL;
+  this->status = TransactionStatus::READY();
+  return this->status.GetCode();
 }
 
 auto Transaction::UpdateEntity(Entity *entity) noexcept -> StatusCode {
@@ -181,20 +218,23 @@ auto Transaction::UpdateEntity(Entity *entity) noexcept -> StatusCode {
   auto *sub_request = this->request->add_requests();
   auto *proto_entity = sub_request->mutable_update_request();
 
-  entity->Switch(proto_entity);
-
-  return StatusCode::INITIAL;
+  entity->CopyTo(proto_entity);
+  this->status = TransactionStatus::READY();
+  return this->status.GetCode();
 }
 
 auto Transaction::Execute() -> TransactionStatus {
-  ExecuteAsynchronously();
+  auto status_code = ExecuteAsynchronously();
+  TransactionStatus::ThrowExceptionIfError(
+    status_code, caosdb::get_status_description(status_code));
   auto status = WaitForIt();
   status.ThrowExceptionIfError();
   return status;
 }
 
 auto Transaction::ExecuteAsynchronously() noexcept -> StatusCode {
-  if (!IsStatus(TransactionStatus::INITIAL())) {
+  if (!IsStatus(TransactionStatus::READY()) &&
+      !IsStatus(TransactionStatus::GO_ON())) {
     return StatusCode::TRANSACTION_STATUS_ERROR;
   }
   switch (this->transaction_type) {

test/test_transaction.cpp

@@ -29,10 +29,9 @@
 #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 <iostream>
-#include <memory> // for allocator, unique_ptr
-#include <string> // for string, basic_string
-#include <vector> // for vector
+#include <memory>                      // for allocator, unique_ptr
+#include <string>                      // for string, basic_string
+#include <vector>                      // for vector
 
 namespace caosdb::transaction {
 using caosdb::configuration::InsecureConnectionConfiguration;
@@ -47,7 +46,7 @@ TEST(test_transaction, create_transaction) {
   Connection connection(configuration);
   auto transaction = connection.CreateTransaction();
 
-  transaction->RetrieveById("100");
+  ASSERT_EQ(StatusCode::GO_ON, transaction->RetrieveById("100"));
   EXPECT_THROW_MESSAGE(
     transaction->Execute(), ConnectionError,
     "The attempt to execute this transaction was not successful because the "
@@ -99,6 +98,33 @@ TEST(test_transaction, test_multi_result_set_empty) {
   EXPECT_EQ(rs.Size(), 0);
 }
 
+TEST(test_transaction, test_multi_result_iterator) {
+  MultiTransactionResponse response;
+
+  response.add_responses()
+    ->mutable_retrieve_response()
+    ->mutable_entity()
+    ->set_id("100");
+
+  MultiResultSet rs(&response);
+  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");
+
+  UniqueResult rs(&response);
+  EXPECT_EQ(rs.Size(), 1);
+  for (const Entity &entity : rs) {
+    EXPECT_EQ(entity.GetId(), "100");
+  }
+}
+
 TEST(test_transaction, test_multi_result_set_one) {
   MultiTransactionResponse response;
   response.add_responses()
@@ -129,7 +155,6 @@ TEST(test_transaction, test_multi_result_set_three) {
   MultiResultSet rs(&response);
   EXPECT_EQ(rs.Size(), 3);
   EXPECT_TRUE(rs.At(1).HasErrors());
-  std::cout << rs.At(1).ToString();
 }
 
 TEST(test_transaction, test_update_entity) {