Merge remote-tracking branch 'origin/feature/matador-ng' into feature/matador-ng

include/matador/object/collection.hpp

@@ -21,7 +21,7 @@ public:
   collection(const collection& other) = default;
 
   void push_back(const value_type& value) {
-    proxy_->items().push_back(value);
+    proxy_->push_back(value);
   }
 
   iterator begin() {
@@ -41,11 +41,11 @@ public:
   }
 
   [[nodiscard]] size_t size() const {
-    return proxy_->items().size();
+    return proxy_->size();
   }
 
   [[nodiscard]] bool empty() const {
-    return proxy_->items().empty();
+    return proxy_->empty();
   }
 
   void reset(std::shared_ptr<collection_proxy<Type>> proxy) {

include/matador/object/collection_proxy.hpp

@@ -20,11 +20,17 @@ namespace matador::object {
 // relation<OwnerType, ForeignType>
 
 template < class RelationType >
-struct relation_iterator_traits;
+struct relation_iterator_traits {
+  static RelationType& value(RelationType& item) {
+    return item;
+  }
+};
 
 template < typename Type, typename OwnerType >
 struct relation_iterator_traits<relation<OwnerType, Type>> {
-
+  static Type& value(relation<OwnerType, Type>& re) {
+    return re.relation().value();
+  }
 };
 
 template<typename Type, typename RelationType = Type>
@@ -33,14 +39,21 @@ public:
   using value_type = Type;
   using reference = Type&;
   using pointer = Type*;
+  using relation_type = RelationType;
   using iterator_category = std::forward_iterator_tag;
 
   collection_proxy_iterator() = default;
-  explicit collection_proxy_iterator(typename std::vector<RelationType>::iterator it)
+  collection_proxy_iterator(typename std::vector<RelationType>::iterator it)
   : it_(it) {}
 
-  reference operator*() { return *it_; }
-  pointer operator->() { return &*it_; }
+  reference operator*() {
+    return relation_iterator_traits<relation_type>::value(*it_);
+  }
+
+  pointer operator->() {
+    return &relation_iterator_traits<relation_type>::value(*it_);
+  }
+
   collection_proxy_iterator& operator++() {
     ++it_;
     return *this;
@@ -52,6 +65,10 @@ public:
     return !operator==(other);
   }
 
+  // relation_type& relation() {
+
+    // return *it_;
+  // }
 private:
   typename std::vector<RelationType>::iterator it_;
 };
@@ -61,8 +78,10 @@ class abstract_collection_proxy {
 public:
   using value_type = Type;
   using relation_type = RelationType;
-  using iterator = typename std::vector<value_type>::iterator;
-  using const_iterator = typename std::vector<value_type>::const_iterator;
+  // using iterator = typename std::vector<value_type>::iterator;
+  // using const_iterator = typename std::vector<value_type>::const_iterator;
+  using iterator = collection_proxy_iterator<value_type, relation_type>;
+  using const_iterator = collection_proxy_iterator<value_type, relation_type>;
 
   virtual ~abstract_collection_proxy() = default;
 
@@ -70,13 +89,10 @@ public:
 
   virtual iterator begin() = 0;
   virtual iterator end() = 0;
-  virtual const_iterator begin() const = 0;
-  virtual const_iterator end() const = 0;
 
-  [[nodiscard]] virtual size_t size() const = 0;
-  [[nodiscard]] virtual bool empty() const = 0;
-//  virtual collection_proxy_iterator<Type> begin() = 0;
-//  virtual collection_proxy_iterator<Type> end() = 0;
+  [[nodiscard]] virtual size_t size() = 0;
+  [[nodiscard]] virtual bool empty() = 0;
+
   [[nodiscard]] virtual const utils::identifier& owner_id() const = 0;
 
 protected:
@@ -117,23 +133,19 @@ public:
     return items_.end();
   }
 
-  const_iterator begin() const override {
+  void push_back(const value_type& value) override {
     resolve();
-    return items_.begin();
+    items_.push_back(value);
+  }
+  [[nodiscard]] size_t size() override {
+    resolve();
+    return items_.size();
+  }
+  [[nodiscard]] bool empty() override {
+    resolve();
+    return items_.empty();
   }
 
-  const_iterator end() const override {
-    resolve();
-    return items_.end();
-  }
-  // const std::vector<Type>& items() const {
-  //   resolve();
-  //   return items_;
-  // }
-  // std::vector<Type>& items() {
-  //   resolve();
-  //   return items_;
-  // }
 private:
   void resolve() {
     if (loaded_) {

include/matador/object/object_proxy.hpp

@@ -26,31 +26,37 @@ public:
 
   // Lazy
   object_proxy(std::weak_ptr<object_resolver<Type>> resolver, utils::identifier id)
-  : resolver_(resolver)
-  , pk_(std::move(id)) {
+  : resolver_(std::move(resolver))
+  , pk_(std::move(id))
+  , state_(object_state::Persistent) {
   }
 
   // Eager
   object_proxy(std::weak_ptr<object_resolver<Type>> resolver, std::shared_ptr<Type> obj)
-  : obj_(obj)
-  , resolver_(resolver)
-  , pk_(primary_key_resolver::resolve_object(*obj).pk)
-  , state_(object_state::Persistent){
+  : obj_(std::move(obj))
+  , resolver_(std::move(resolver))
+  , pk_(obj_ ? primary_key_resolver::resolve_object(*obj_).pk : utils::identifier{})
+  , state_(obj_ ? object_state::Persistent : object_state::Detached) {
   }
 
   // Transient
   explicit object_proxy(std::shared_ptr<Type> obj)
-  : obj_(obj)
-  , pk_(primary_key_resolver::resolve_object(*obj).pk) {
+  : obj_(std::move(obj))
+  , pk_(obj_ ? primary_key_resolver::resolve_object(*obj_).pk : utils::identifier{}) {
   }
 
   void attach(std::shared_ptr<Type> obj) {
     std::lock_guard lock(mutex_);
+
     obj_ = std::move(obj);
-    if (obj_) {
-      pk_ = primary_key_resolver::resolve_object(*obj_).pk;
-      state_.store(object_state::Persistent, std::memory_order_release);
+    if (!obj_) {
+      pk_.clear();
+      state_ = object_state::Detached;
+      return;
     }
+
+    pk_ = primary_key_resolver::resolve_object(*obj_).pk;
+    state_ = object_state::Persistent;
   }
 
   void resolver(std::weak_ptr<object_resolver<Type>> resolver) {
@@ -59,66 +65,125 @@ public:
   }
 
   [[nodiscard]] std::shared_ptr<Type> object() const {
-    if (!obj_) {
-      std::ignore = resolve();
-    }
-    return obj_;
+    return resolve_object();
   }
 
   void invalidate() {
     std::lock_guard lock(mutex_);
     obj_.reset();
     resolver_.reset();
-    state_.store(object_state::Detached, std::memory_order_release);
+    state_ = object_state::Detached;
   }
 
   [[nodiscard]] void *raw_pointer() const { return static_cast<void *>(pointer()); }
 
-  Type *operator->() { return pointer(); }
-  Type &operator*() { return *pointer(); }
-  const Type &operator*() const { return *pointer(); }
+  Type *operator->() {
+    auto *ptr = pointer();
+    if (!ptr) {
+      throw std::runtime_error("Cannot dereference empty object proxy");
+    }
+    return ptr;
+  }
 
-  Type *pointer() const { return resolve(); }
+  const Type *operator->() const {
+    auto *ptr = pointer();
+    if (!ptr) {
+      throw std::runtime_error("Cannot dereference empty object proxy");
+    }
+    return ptr;
+  }
+
+  Type &operator*() {
+    auto *ptr = pointer();
+    if (!ptr) {
+      throw std::runtime_error("Cannot dereference empty object proxy");
+    }
+    return *ptr;
+  }
+
+  const Type &operator*() const {
+    auto *ptr = pointer();
+    if (!ptr) {
+      throw std::runtime_error("Cannot dereference empty object proxy");
+    }
+    return *ptr;
+  }
+
+  Type *pointer() const {
+    return resolve_object().get();
+  }
+
+  [[nodiscard]] bool empty() const {
+    std::lock_guard lock(mutex_);
+    return !obj_ && resolver_.expired();
+  }
 
-  [[nodiscard]] bool empty() const { return !obj_ && resolver_.expired(); }
   [[nodiscard]] bool valid() const { return !empty(); }
-  [[nodiscard]] bool has_primary_key() const { return !pk_.is_null(); }
-  [[nodiscard]] const utils::identifier &primary_key() const { return pk_; }
-  void primary_key(const utils::identifier &pk) { pk_ = pk; }
+
+  [[nodiscard]] bool has_primary_key() const {
+    std::lock_guard lock(mutex_);
+    return !pk_.is_null();
+  }
+
+  [[nodiscard]] utils::identifier primary_key() const {
+    std::lock_guard lock(mutex_);
+    return pk_;
+  }
+
+  void primary_key(const utils::identifier &pk) {
+    std::lock_guard lock(mutex_);
+    pk_ = pk;
+  }
 
   bool is_persistent() const { return is_state(object_state::Persistent); }
   bool is_transient() const { return is_state(object_state::Transient); }
   bool is_detached() const { return is_state(object_state::Detached); }
   bool is_removed() const { return is_state(object_state::Removed); }
-  bool is_state(const object_state state) const { return state_ == state; }
+
+  bool is_state(const object_state state) const {
+    std::lock_guard lock(mutex_);
+    return state_ == state;
+  }
 
   void change_state(const object_state state) {
-    state_.store(state, std::memory_order_release);
-  }
-private:
-  Type* resolve() const {
-    if (obj_) {
-      return obj_.get();
-    }
-
     std::lock_guard lock(mutex_);
-    auto resolver = resolver_.lock();
-    if (!resolver) {
-      return nullptr;
-      // Todo: Add states (Detached, Attached, Transient) - if attached an no resolver is available throw runtime exception
-      // throw std::runtime_error("Detached proxy (session expired)");
+    state_ = state;
+  }
+private:
+  std::shared_ptr<Type> resolve_object() const {
+    std::shared_ptr<Type> current;
+    std::shared_ptr<object_resolver<Type>> resolver;
+    utils::identifier pk;
+
+    {
+      std::lock_guard lock(mutex_);
+      if (obj_) {
+        return obj_;
+      }
+
+      resolver = resolver_.lock();
+      if (!resolver) {
+        return nullptr;
+      }
+
+      pk = pk_;
     }
 
-    const_cast<std::shared_ptr<Type>&>(obj_) = resolver->resolve(pk_);
+    current = resolver->resolve(pk);
 
-    return obj_.get();
+    {
+      std::lock_guard lock(mutex_);
+      if (!obj_) {
+        obj_ = std::move(current);
+      }
+      return obj_;
+    }
   }
-
 private:
-  std::shared_ptr<Type> obj_{};
-  std::weak_ptr<object_resolver<Type>> resolver_{};
+  mutable std::shared_ptr<Type> obj_{};
+  mutable std::weak_ptr<object_resolver<Type>> resolver_{};
   utils::identifier pk_{};
-  std::atomic<object_state> state_{object_state::Transient};
+  object_state state_{object_state::Transient};
   mutable std::mutex mutex_{};
 };
 }

include/matador/object/object_ptr.hpp

@@ -17,68 +17,152 @@ inline constexpr null_object_ptr_t nullobj{};
 template <typename Type>
 class object_ptr {
 public:
-  object_ptr()
+object_ptr()
   : proxy_(std::make_shared<object_proxy<Type>>()) {}
+
   object_ptr(null_object_ptr_t) {}
+
   explicit object_ptr(std::shared_ptr<Type> obj)
-  : proxy_(std::make_shared<object_proxy<Type>>(obj)) {}
+  : proxy_(std::make_shared<object_proxy<Type>>(std::move(obj))) {}
+
   explicit object_ptr(std::shared_ptr<object_proxy<Type>> obj)
   : proxy_(std::move(obj)) {}
+
   object_ptr(const object_ptr &other) = default;
   object_ptr(object_ptr &&other) noexcept = default;
   object_ptr& operator=(const object_ptr &other) = default;
-  object_ptr& operator=(object_ptr &&other) = default;
+  object_ptr& operator=(object_ptr &&other) noexcept = default;
+
   object_ptr& operator=(null_object_ptr_t) {
     proxy_.reset();
     return *this;
   }
 
   bool operator==(const object_ptr &other) const {
-    return get() == other.get();
+    if (proxy_ == other.proxy_) {
+      return true;
+    }
+
+    if (!proxy_ || !other.proxy_) {
+      return false;
+    }
+
+    if (has_primary_key() && other.has_primary_key()) {
+      return primary_key() == other.primary_key();
+    }
+
+    return false;
   }
+
   bool operator==(null_object_ptr_t) const {
     return empty();
   }
+
   bool operator!=(const object_ptr &other) const { return !operator==(other); }
   bool operator!=(null_object_ptr_t) const { return !empty(); }
 
   using value_type = Type;
 
-  Type *operator->() const { return get(); }
-  Type &operator*() { return *get(); }
-  const Type &operator*() const { return *get(); }
+  Type *operator->() const {
+    return checked_get();
+  }
 
-  [[nodiscard]] bool empty() const { return get() == nullptr; }
+  Type &operator*() {
+    return *checked_get();
+  }
+
+  const Type &operator*() const {
+    return *checked_get();
+  }
+
+  [[nodiscard]] bool empty() const {
+    return proxy_ == nullptr || proxy_->empty();
+  }
 
   Type *get() const {
     return proxy_ ? proxy_->pointer() : nullptr;
   }
 
-  void reset() { proxy_.reset(); }
-  void reset(const std::shared_ptr<object_proxy<Type>>& proxy) { proxy_ = proxy; }
+  [[nodiscard]] std::shared_ptr<Type> object() const {
+    return proxy_ ? proxy_->object() : nullptr;
+  }
 
-  [[nodiscard]] std::shared_ptr<object_proxy<Type>> proxy() const { return proxy_; }
+  void reset() {
+    proxy_.reset();
+  }
 
-  operator bool() const { return valid(); }
-  [[nodiscard]] bool valid() const { return proxy_ != nullptr && !proxy_->empty(); }
+  void reset(std::shared_ptr<object_proxy<Type>> proxy) {
+    proxy_ = std::move(proxy);
+  }
 
-  [[nodiscard]] bool has_primary_key() const { return proxy_->has_primary_key(); }
-  [[nodiscard]] const utils::identifier &primary_key() const { return proxy_->primary_key(); }
-  void primary_key(const utils::identifier &pk) { proxy_->primary_key(pk); }
+  [[nodiscard]] std::shared_ptr<object_proxy<Type>> proxy() const {
+    return proxy_;
+  }
 
-  [[nodiscard]] bool is_persistent() const { return proxy_->is_persistent(); }
-  [[nodiscard]] bool is_transient() const { return proxy_->is_transient(); }
-  [[nodiscard]] bool is_detached() const { return proxy_->is_detached(); }
-  [[nodiscard]] bool is_removed() const { return proxy_->is_removed(); }
-  [[nodiscard]] bool is_state(const object_state state) const { return proxy_->is_state(state); }
+  explicit operator bool() const {
+    return valid();
+  }
+
+  [[nodiscard]] bool valid() const {
+    return proxy_ != nullptr && !proxy_->empty();
+  }
+
+  [[nodiscard]] bool has_primary_key() const {
+    return proxy_ != nullptr && proxy_->has_primary_key();
+  }
+
+  [[nodiscard]] utils::identifier primary_key() const {
+    return proxy_ ? proxy_->primary_key() : utils::identifier{};
+  }
+
+  void primary_key(const utils::identifier &pk) {
+    ensure_proxy();
+    proxy_->primary_key(pk);
+  }
+
+  [[nodiscard]] bool is_persistent() const {
+    return proxy_ != nullptr && proxy_->is_persistent();
+  }
+
+  [[nodiscard]] bool is_transient() const {
+    return proxy_ != nullptr && proxy_->is_transient();
+  }
+
+  [[nodiscard]] bool is_detached() const {
+    return proxy_ != nullptr && proxy_->is_detached();
+  }
+
+  [[nodiscard]] bool is_removed() const {
+    return proxy_ != nullptr && proxy_->is_removed();
+  }
+
+  [[nodiscard]] bool is_state(const object_state state) const {
+    return proxy_ != nullptr && proxy_->is_state(state);
+  }
 
   void change_state(object_state s) const {
     if (proxy_) {
       proxy_->change_state(s);
     }
   }
+
 private:
-  std::shared_ptr<object_proxy<Type> > proxy_{};
+  Type *checked_get() const {
+    auto *ptr = get();
+    if (!ptr) {
+      throw std::runtime_error("Cannot dereference empty object_ptr");
+    }
+    return ptr;
+  }
+
+  void ensure_proxy() {
+    if (!proxy_) {
+      proxy_ = std::make_shared<object_proxy<Type>>();
+    }
+  }
+
+private:
+  std::shared_ptr<object_proxy<Type>> proxy_{};
 };
 
 template<typename>

include/matador/query/expression_evaluator.hpp

@@ -19,7 +19,7 @@ public:
     void visit(const value_expression& node) override;
     void visit(const placeholder_expression& node) override;
 
-    const std::string& result() const;
+    [[nodiscard]] const std::string& result() const;
 
 private:
     const sql::dialect &dialect_;

include/matador/sql/statement.hpp

@@ -171,7 +171,8 @@ template<class Type>
 utils::result<query_result<Type>, utils::error> statement::fetch() {
   std::cout << statement_proxy_->sql() << std::endl;
   statement_proxy_->statement_->query_.result_type = typeid(Type);
-  return statement_proxy_->fetch(*bindings_).and_then([this](std::unique_ptr<query_result_impl> &&value) {
+  return statement_proxy_->fetch(*bindings_)
+  .and_then([this](std::unique_ptr<query_result_impl> &&value) -> utils::result<query_result<Type>, utils::error> {
     auto resolver = statement_proxy_->statement_->query_.resolver->object_resolver<Type>();
     const auto prototype = value->prototype();
     return utils::ok(query_result<Type>(std::forward<decltype(value)>(value), resolver, [prototype] {

include/matador/utils/result.hpp

@@ -2,7 +2,6 @@
 #define QUERY_RESULT_HPP
 
 #include <variant>
-#include <optional>
 #include <functional>
 #include <type_traits>
 
@@ -21,6 +20,7 @@ template < typename ValueType >
 class ok {
 public:
   using value_type = ValueType;
+  constexpr ok() = default;
   explicit constexpr ok(const ValueType &value) : value_(value) {}
   explicit constexpr ok(ValueType &&value) : value_(std::move(value)) {}
 
@@ -49,21 +49,49 @@ public:
 
   constexpr ErrorType&& release() { return std::move(error_); }
   const ErrorType& value() const { return error_; }
-  ErrorType value() { return error_; }
+  ErrorType& value() { return error_; }
 
 private:
   ErrorType error_;
 };
 
+namespace detail {
+template <typename ValueType, typename Func, bool IsVoidValue = std::is_void_v<ValueType>>
+struct map_result_value_type;
+
+template <typename ValueType, typename Func>
+struct map_result_value_type<ValueType, Func, false> {
+  using type = std::invoke_result_t<Func, ValueType&&>;
+};
+
+template <typename ValueType, typename Func>
+struct map_result_value_type<ValueType, Func, true> {
+  using type = std::invoke_result_t<Func>;
+};
+
+template <typename ValueType, typename Func, bool IsVoidValue = std::is_void_v<ValueType>>
+struct and_then_result_type;
+
+template <typename ValueType, typename Func>
+struct and_then_result_type<ValueType, Func, false> {
+  using type = std::invoke_result_t<Func, ValueType&&>;
+};
+
+template <typename ValueType, typename Func>
+struct and_then_result_type<ValueType, Func, true> {
+  using type = std::invoke_result_t<Func>;
+};
+}
+
 template < typename ValueType, typename ErrorType >
 class result {
 public:
   using value_type = ValueType;
   using error_type = ErrorType;
 
-  result() : result_(ValueType{}) {}
-  result(ok<value_type> value) : result_(std::move(value.release())) {} // NOLINT(*-explicit-constructor)
-  result(failure<error_type> error) : result_(std::move(error.release())) {} // NOLINT(*-explicit-constructor)
+  result() : result_(ok<value_type>{}) {}
+  result(ok<value_type> value) : result_(std::move(value)) {} // NOLINT(*-explicit-constructor)
+  result(failure<error_type> error) : result_(std::move(error)) {} // NOLINT(*-explicit-constructor)
   result(const result &x) = default;
   result& operator=(const result &x) = default;
   result(result &&x) = default;
@@ -71,123 +99,128 @@ public:
 
   operator bool() const { return is_ok(); } // NOLINT(*-explicit-constructor)
 
-  [[nodiscard]] bool is_ok() const { return std::holds_alternative<value_type>(result_); }
-  [[nodiscard]] bool is_error() const { return std::holds_alternative<error_type>(result_); }
+  [[nodiscard]] bool is_ok() const {
+    return std::holds_alternative<ok<value_type>>(result_);
+  }
+  [[nodiscard]] bool is_error() const {
+    return std::holds_alternative<failure<error_type>>(result_);
+  }
 
-  ValueType&& release() { return std::move(std::get<value_type>(result_)); }
-  ErrorType&& release_error() { return std::move(std::get<error_type>(result_)); }
+  template <typename T = ValueType>
+  std::enable_if_t<!std::is_void_v<T>, T&&> release() {
+    return std::move(std::get<ok<value_type>>(result_).release());
+  }
+  ErrorType&& release_error() {
+    return std::move(std::get<failure<error_type>>(result_).release());
+  }
 
-  const ValueType& value() const { return std::get<value_type>(result_); }
-  ValueType& value() { return std::get<value_type>(result_); }
-  const ErrorType& err() const { return std::get<error_type>(result_); }
-  ErrorType err() { return std::get<error_type>(result_); }
+  template <typename T = ValueType>
+  std::enable_if_t<!std::is_void_v<T>, const T&> value() const {
+    return std::get<ok<value_type>>(result_).value();
+  }
+  template <typename T = ValueType>
+  std::enable_if_t<!std::is_void_v<T>, T&> value() {
+    return std::get<ok<value_type>>(result_).value();
+  }
+  const ErrorType& err() const {
+    return std::get<failure<error_type>>(result_).value();
+  }
+  ErrorType& err() {
+    return std::get<failure<error_type>>(result_).value();
+  }
 
-  constexpr const ValueType* operator->() const { return &value(); }
-  constexpr ValueType* operator->() { return &std::get<value_type>(result_); }
+  template <typename T = ValueType>
+  constexpr std::enable_if_t<!std::is_void_v<T>, const T*> operator->() const {
+    return &value();
+  }
+  template <typename T = ValueType>
+  constexpr std::enable_if_t<!std::is_void_v<T>, T*> operator->() {
+    return &value();
+  }
 
-  constexpr const ValueType& operator*() const& noexcept { return value(); }
-  constexpr ValueType& operator*() & noexcept { return value(); }
+  template <typename T = ValueType>
+  constexpr std::enable_if_t<!std::is_void_v<T>, const T&> operator*() const& noexcept {
+    return value();
+  }
+  template <typename T = ValueType>
+  constexpr std::enable_if_t<!std::is_void_v<T>, T&> operator*() & noexcept {
+    return value();
+  }
 
   template<typename Func,
-           typename SecondValueType = std::invoke_result_t<Func, ValueType >>
+           typename SecondValueType = typename detail::map_result_value_type<ValueType, Func>::type>
   result<SecondValueType, ErrorType> map(Func &&f) {
-    if (is_ok()) {
-      return result<SecondValueType, ErrorType>(ok(f(release())));
+    if (is_error()) {
+      return failure<ErrorType>(release_error());
     }
 
-    return result<SecondValueType, ErrorType>(failure(release_error()));
+    if constexpr (std::is_void_v<ValueType>) {
+      if constexpr (std::is_void_v<SecondValueType>) {
+        std::invoke(std::forward<Func>(f));
+        return ok<void>{};
+      } else {
+        return ok<SecondValueType>(std::invoke(std::forward<Func>(f)));
+      }
+    } else {
+      if constexpr (std::is_void_v<SecondValueType>) {
+        std::invoke(std::forward<Func>(f), release());
+        return ok<void>{};
+      } else {
+        return ok<SecondValueType>(std::invoke(std::forward<Func>(f), release()));
+      }
+    }
   }
 
   template<typename Func,
            typename SecondErrorType = typename std::invoke_result_t<Func, ErrorType >::value_type>
   result<SecondErrorType, ErrorType> map_error(Func &&f) {
-    if (!is_ok()) {
-      return result<SecondErrorType, ErrorType>(ok(release()));
+    if (!is_error()) {
+      return failure<SecondErrorType>{std::invoke(std::forward<Func>(f), release_error())};
     }
 
-    return result<SecondErrorType, ErrorType>(error(release_error()));
+    if constexpr (std::is_void_v<ValueType>) {
+      return ok<void>{};
+    } else {
+      return ok<ValueType>(release());
+    }
   }
 
-  template<typename Func,
-           typename SecondValueType = typename std::invoke_result_t<Func, ValueType>::value_type>
-  result<SecondValueType, ErrorType> and_then(Func &&f) {
+  template <typename Func,
+            typename ReturnResult = typename detail::and_then_result_type<ValueType, Func>::type>
+  ReturnResult and_then(Func &&f) {
+    static_assert(is_result<ReturnResult>::value, "and_then() callback must return matador::utils::result");
+
     if (is_ok()) {
-      return f(release());
+      if constexpr (std::is_void_v<ValueType>) {
+        return std::invoke(std::forward<Func>(f));
+      } else {
+        return std::invoke(std::forward<Func>(f), release());
+      }
     }
 
-    return result<SecondValueType, ErrorType>(failure(release_error()));
+    return ReturnResult(failure<ErrorType>(release_error()));
   }
 
-  template<typename Func,
-           typename SecondErrorType = typename std::invoke_result_t<Func, ErrorType&& >::value_type>
+  template <typename Func,
+            typename FailureType = std::invoke_result_t<Func, ErrorType&&>,
+            typename SecondErrorType = typename FailureType::value_type>
   result<ValueType, SecondErrorType> or_else(Func &&f) {
     if (is_error()) {
-      return f(release_error());
+      return result<ValueType, SecondErrorType>(
+        std::invoke(std::forward<Func>(f), release_error())
+      );
     }
 
-    return result<ValueType, SecondErrorType>(ok(release()));
+    if constexpr (std::is_void_v<ValueType>) {
+      return ok<void>{};
+    } else {
+      return ok<ValueType>(release());
+    }
   }
 
 private:
-  std::variant<value_type, error_type> result_;
+  std::variant<ok<value_type>, failure<error_type>> result_;
 };
-
-template < typename ErrorType >
-class result<void, ErrorType>
-{
-public:
-  using value_type = void;
-  using error_type = ErrorType;
-
-  result() = default;
-  result(ok<void> /*value*/) {}
-  result(failure<error_type> error) : result_(std::move(error.release())) {} // NOLINT(*-explicit-constructor)
-  result(const result &x) = default;
-  result& operator=(const result &x) = default;
-  result(result &&x) = default;
-  result& operator=(result &&x) = default;
-
-  operator bool() const { return is_ok(); } // NOLINT(*-explicit-constructor)
-
-  [[nodiscard]] bool is_ok() const { return !result_.has_value(); }
-  [[nodiscard]] bool is_error() const { return result_.has_value(); }
-
-  ErrorType&& release_error() { return std::move(*result_); }
-
-  const ErrorType& err() const { return result_.value(); }
-  ErrorType err() { return result_.value(); }
-
-  template<typename Func, typename SecondValueType = std::invoke_result_t<Func>>
-  result<SecondValueType, ErrorType> map(Func &&f) {
-    if (is_ok()) {
-      return result<SecondValueType, ErrorType>(ok(f()));
-    }
-
-    return result<SecondValueType, ErrorType>(failure(release_error()));
-  }
-
-  template<typename Func>
-  result and_then(Func &&f) {
-    if (is_ok()) {
-      return f();
-    }
-
-    return result(failure(release_error()));
-  }
-
-  template<typename Func, typename SecondErrorType = typename std::invoke_result_t<Func, ErrorType&& >::value_type>
-  result<void, SecondErrorType> or_else(Func &&f) {
-    if (is_error()) {
-      return f(release_error());
-    }
-
-    return result<void, SecondErrorType>(ok<void>());
-  }
-
-private:
-  std::optional<error_type> result_;
-};
-
 }
 
 #endif //QUERY_RESULT_HPP

include/matador/utils/value.hpp

@@ -56,7 +56,7 @@ public:
     if (!res.is_ok()) {
       return std::nullopt;
     }
-    return *res;
+    return res.value();
   }
 
   template<class Type>

include/matador/utils/version.hpp

@@ -3,16 +3,16 @@
 
 #include "matador/utils/result.hpp"
 #include "matador/utils/error.hpp"
+#include "matador/utils/export.hpp"
 
 #include <string>
-#include <ostream>
 
 namespace matador::utils {
 
-class version {
+class MATADOR_UTILS_API version final {
 public:
   version() = default;
-  ~version() =default;
+  ~version() = default;
   version(unsigned int major, unsigned int minor, unsigned int patch);
   version(const version& x) = default;
   version& operator=(const version& x) = default;

source/core/utils/version.cpp

@@ -3,98 +3,122 @@
 #include <matador/utils/errors.hpp>
 
 namespace matador::utils {
+namespace {
 
-version::version(unsigned int major, unsigned int minor, unsigned int patch)
-: major_(major)
-, minor_(minor)
-, patch_(patch)
-{}
+bool parse_uint_component(const std::string &text, std::size_t &pos, unsigned int &value) {
+  if (pos >= text.size() || text[pos] < '0' || text[pos] > '9') {
+    return false;
+  }
 
-bool version::operator==(const version &x) const
-{
-  return major_ == x.major_ &&
-         minor_ == x.minor_ &&
-         patch_ == x.patch_;
+  unsigned int result{};
+
+  while (pos < text.size() && text[pos] >= '0' && text[pos] <= '9') {
+    const auto digit = static_cast<unsigned int>(text[pos] - '0');
+
+    if (result > (std::numeric_limits<unsigned int>::max() - digit) / 10) {
+      return false;
+    }
+
+    result = result * 10 + digit;
+    ++pos;
+  }
+
+  value = result;
+  return true;
 }
 
-bool version::operator!=(const version &x) const
-{
+bool parse_dot(const std::string &text, std::size_t &pos) {
+  if (pos >= text.size() || text[pos] != '.') {
+    return false;
+  }
+
+  ++pos;
+  return true;
+}
+
+} // namespace
+version::version(const unsigned int major, const unsigned int minor, const unsigned int patch)
+: major_(major)
+, minor_(minor)
+, patch_(patch) {
+}
+
+bool version::operator==(const version& x) const {
+  return major_ == x.major_ &&
+    minor_ == x.minor_ &&
+    patch_ == x.patch_;
+}
+
+bool version::operator!=(const version& x) const {
   return !(*this == x);
 }
 
-bool version::operator>(const version &x) const
-{
+bool version::operator>(const version& x) const {
   return !(*this <= x);
 }
 
-bool version::operator>=(const version &x) const
-{
+bool version::operator>=(const version& x) const {
   return !(*this < x);
 }
 
-bool version::operator<(const version &x) const
-{
+bool version::operator<(const version& x) const {
   return (major_ < x.major_) ||
-         (major_ == x.major_ && minor_ < x.minor_) ||
-         (major_ == x.major_ && minor_ == x.minor_ && patch_ < x.patch_);
+    (major_ == x.major_ && minor_ < x.minor_) ||
+    (major_ == x.major_ && minor_ == x.minor_ && patch_ < x.patch_);
 }
 
-bool version::operator<=(const version &x) const
-{
+bool version::operator<=(const version& x) const {
   return *this < x || *this == x;
 }
 
-std::string version::str() const
-{
-  char buf[32];
-  sprintf(buf, "%d.%d.%d", major_, minor_, patch_);
-  return buf;
+std::string version::str() const {
+  return std::to_string(major_) + "." +
+    std::to_string(minor_) + "." +
+    std::to_string(patch_);
 }
 
-std::ostream &operator<<(std::ostream &out, const version &v)
-{
+std::ostream& operator<<(std::ostream& out, const version& v) {
   out << v.str();
   return out;
 }
 
-result<version, error> version::from_string(const std::string &version_string)
-{
-  version result;
-  if (const auto ret = sscanf(version_string.c_str(), "%u.%u.%u", &result.major_, &result.minor_, &result.patch_); ret != 3) {
+result<version, error> version::from_string(const std::string& version_string) {
+  unsigned int major{};
+  unsigned int minor{};
+  unsigned int patch{};
+
+  if (std::size_t pos{}; !parse_uint_component(version_string, pos, major) ||
+      !parse_dot(version_string, pos) ||
+      !parse_uint_component(version_string, pos, minor) ||
+      !parse_dot(version_string, pos) ||
+      !parse_uint_component(version_string, pos, patch) ||
+      pos != version_string.size()) {
     return failure(error(utils_error::InvalidVersionString, version_string));
-  }
+      }
 
-  return ok(result);
-}
+  return ok(version{major, minor, patch});}
 
-unsigned int version::major() const
-{
+unsigned int version::major() const {
   return major_;
 }
 
-unsigned int version::minor() const
-{
+unsigned int version::minor() const {
   return minor_;
 }
 
-unsigned int version::patch() const
-{
+unsigned int version::patch() const {
   return patch_;
 }
 
-void version::major(unsigned int m)
-{
+void version::major(unsigned int m) {
   major_ = m;
 }
 
-void version::minor(unsigned int m)
-{
+void version::minor(unsigned int m) {
   minor_ = m;
 }
 
-void version::patch(unsigned int p)
-{
+void version::patch(unsigned int p) {
   patch_ = p;
 }
-
 }

source/orm/CMakeLists.txt

@@ -71,7 +71,6 @@ add_library(matador-orm STATIC
   ../../include/matador/query/key_value_generator.hpp
   ../../include/matador/query/manual_pk_generator.hpp
   ../../include/matador/query/meta_table_macro.hpp
-  ../../include/matador/query/meta_table_macro.hpp
   ../../include/matador/query/query.hpp
   ../../include/matador/query/query_builder.hpp
   ../../include/matador/query/query_builder_utils.hpp

source/orm/query/intermediates/fetchable_query.cpp

@@ -30,7 +30,7 @@ utils::result<sql::query_result<sql::record>, utils::error> fetchable_query::fet
   auto ctx = compiler.build(*context_, exec.dialect(), std::nullopt);
   ctx.resolver = exec.resolver();
   return exec.fetch(ctx)
-    .and_then([](auto &&res) {
+    .and_then([](auto &&res) -> utils::result<sql::query_result<sql::record>, utils::error> {
       const auto prototype = res->prototype();
       return utils::ok(sql::query_result<sql::record>(std::forward<decltype(res)>(res), prototype));
     });

source/orm/query/table.cpp

@@ -42,7 +42,9 @@ table::table(const table &other)
 , alias_(other.alias_)
 , schema_name_(other.schema_name_)
 , columns_(other.columns_)
-, pk_column_index_(other.pk_column_index_){
+, pk_column_index_(other.pk_column_index_)
+, join_column_index_(other.join_column_index_)
+, inverse_join_column_index_(other.inverse_join_column_index_) {
   for (auto &col : columns_) {
     col.table(this);
   }
@@ -64,6 +66,8 @@ table & table::operator=(table &&other) noexcept {
   alias_ = std::move(other.alias_);
   columns_ = std::move(other.columns_);
   pk_column_index_ = other.pk_column_index_;
+  join_column_index_ = other.join_column_index_;
+  inverse_join_column_index_ = other.inverse_join_column_index_;
   for (auto &col : columns_) {
     col.table(this);
   }

test/backends/QueryFixture.cpp

@@ -37,7 +37,8 @@ void QueryFixture::check_table_not_exists(const std::string &table_name) const {
 }
 
 void QueryFixture::drop_table_if_exists(const std::string &table_name) const {
-  const auto result = db.exists(table_name).and_then([&table_name, this](const bool exists) {
+  const auto result = db.exists(table_name)
+  .and_then([&table_name, this](const bool exists) -> utils::result<bool, utils::error> {
     if (exists) {
       auto res = query::drop()
         .table(table_name)

test/backends/SequenceFixture.cpp

@@ -35,7 +35,8 @@ void SequenceFixture::check_sequence_not_exists(const std::string& sequence_name
 }
 
 void SequenceFixture::drop_sequence_if_exists(const std::string& sequence_name) const {
-  const auto result = db.sequence_exists(sequence_name).and_then([&sequence_name, this](const bool exists) {
+  const auto result = db.sequence_exists(sequence_name).
+  and_then([&sequence_name, this](const bool exists) -> utils::result<bool, utils::error> {
     if (exists) {
       auto res = query::drop()
         .sequence(sequence_name)

test/core/CMakeLists.txt

@@ -5,6 +5,7 @@ list(APPEND CMAKE_MODULE_PATH ${catch2_SOURCE_DIR}/extras)
 add_executable(CoreTests
   ../backends/SchemaFixture.hpp
   logger/LoggerTest.cpp
+  object/CollectionTest.cpp
   object/ObjectCacheTest.cpp
   object/ObjectTest.cpp
   object/PrimaryKeyResolverTest.cpp

test/core/object/CollectionTest.cpp

@@ -0,0 +1,16 @@
+#include <catch2/catch_test_macros.hpp>
+
+#include "matador/object/collection.hpp"
+#include "matador/object/object.hpp"
+
+#include <iostream>
+
+using namespace matador::object;
+
+TEST_CASE("Test collection", "[collection]") {
+    collection<int> ints{{1,2,3,4}};
+
+    for (auto const& i : ints) {
+        std::cout << i << std::endl ;
+    }
+}

test/core/object/ObjectCacheTest.cpp

@@ -59,7 +59,7 @@ private:
 };
 } // namespace
 
-TEST_CASE("object_cache: acquire_proxy returns the same proxy instance across threads", "[object][cache][threadsafe]") {
+TEST_CASE("ObjectCache: acquire_proxy returns the same proxy instance across threads", "[object][cache][threadsafe]") {
   matador::utils::message_bus bus;
   matador::object::object_cache cache(bus);
   const matador::utils::identifier id{123};
@@ -91,7 +91,7 @@ TEST_CASE("object_cache: acquire_proxy returns the same proxy instance across th
   }
 }
 
-TEST_CASE("object_cache: attach_entity makes is_loaded/get_entity reflect presence", "[object][cache]") {
+TEST_CASE("ObjectCache: attach_entity makes is_loaded/get_entity reflect presence", "[object][cache]") {
   matador::utils::message_bus bus;
   matador::object::object_cache cache(bus);
   const matador::utils::identifier id{42};
@@ -110,7 +110,7 @@ TEST_CASE("object_cache: attach_entity makes is_loaded/get_entity reflect presen
   REQUIRE(got->name == "hans");
 }
 
-TEST_CASE("object_cache: erase invalidates existing proxies", "[object][cache]") {
+TEST_CASE("ObjectCache: erase invalidates existing proxies", "[object][cache]") {
   matador::utils::message_bus bus;
   matador::object::object_cache cache(bus);
   const matador::utils::identifier id{9};

test/core/object/ObjectTest.cpp

@@ -7,7 +7,7 @@
 #include "../test/models/author.hpp"
 #include "../test/models/book.hpp"
 
-TEST_CASE("Generate object from type", "[object][generate]") {
+TEST_CASE("Object: Generate object from type", "[object][generate]") {
   using namespace matador;
   object::repository repo;
   auto result = repo.attach<test::author>("authors");

test/core/utils/IdentifierTest.cpp

@@ -6,7 +6,7 @@
 
 using namespace matador::utils;
 
-TEST_CASE("Test create identifier", "[identifier][create]") {
+TEST_CASE("Identifier: Test create identifier", "[identifier][create]") {
   const identifier id;
 
   REQUIRE(id.is_null());
@@ -16,7 +16,7 @@ TEST_CASE("Test create identifier", "[identifier][create]") {
   REQUIRE(id.str() == "null");
 }
 
-TEST_CASE("Test assign value to identifier", "[identifier][assign]") {
+TEST_CASE("Identifier: Test assign value to identifier", "[identifier][assign]") {
   identifier id;
 
   REQUIRE(id.is_null());
@@ -48,7 +48,7 @@ TEST_CASE("Test assign value to identifier", "[identifier][assign]") {
   // REQUIRE(id == identifier{"UniqueId"});
 }
 
-TEST_CASE("Test compare identifier", "[identifier][compare]") {
+TEST_CASE("Identifier: Test compare identifier", "[identifier][compare]") {
   identifier id1{6}, id2{7};
 
   REQUIRE(id1 != id2);
@@ -68,7 +68,7 @@ identifier create(const int id) {
   return identifier{id};
 }
 
-TEST_CASE("Test copy identifier" "[identifier][copy]") {
+TEST_CASE("Identifier: Test copy identifier" "[identifier][copy]") {
   identifier id1{"Unique"};
   REQUIRE(id1.is_valid());
   REQUIRE(id1.str() == "Unique");
@@ -92,7 +92,7 @@ TEST_CASE("Test copy identifier" "[identifier][copy]") {
   id3 = id1;
 }
 
-TEST_CASE("Test move identifier", "[identifier][move]") {
+TEST_CASE("Identifier: Test move identifier", "[identifier][move]") {
   identifier id1{6};
 
   REQUIRE(id1.is_integer());
@@ -103,7 +103,7 @@ TEST_CASE("Test move identifier", "[identifier][move]") {
   REQUIRE(id2.is_integer());
 }
 
-TEST_CASE("identifier assignment from integer types", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier assignment from integer types", "[utils][identifier][assign]") {
   identifier id;
 
   id = int8_t{-5};
@@ -125,7 +125,7 @@ TEST_CASE("identifier assignment from integer types", "[utils][identifier][assig
   REQUIRE(id.is_valid());
 }
 
-TEST_CASE("identifier assignment from string type", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier assignment from string type", "[utils][identifier][assign]") {
   identifier id;
 
   id = std::string{"hello"};
@@ -140,7 +140,7 @@ TEST_CASE("identifier assignment from string type", "[utils][identifier][assign]
   REQUIRE_FALSE(id.is_valid());
 }
 
-TEST_CASE("identifier assignment from const char*", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier assignment from const char*", "[utils][identifier][assign]") {
   identifier id;
 
   id = "world";
@@ -155,7 +155,7 @@ TEST_CASE("identifier assignment from const char*", "[utils][identifier][assign]
   REQUIRE_FALSE(id.is_valid());
 }
 
-TEST_CASE("identifier assignment from nullptr", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier assignment from nullptr", "[utils][identifier][assign]") {
   identifier id{42};
 
   REQUIRE(id.is_valid());
@@ -168,7 +168,7 @@ TEST_CASE("identifier assignment from nullptr", "[utils][identifier][assign]") {
   REQUIRE_FALSE(id.is_valid());
 }
 
-TEST_CASE("identifier reassignment between types", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier reassignment between types", "[utils][identifier][assign]") {
   identifier id{uint64_t{7}};
 
   REQUIRE(id.is_integer());
@@ -189,7 +189,7 @@ TEST_CASE("identifier reassignment between types", "[utils][identifier][assign]"
   REQUIRE(id.str() == "null");
 }
 
-TEST_CASE("identifier as() returns exact integer type", "[utils][identifier][as]") {
+TEST_CASE("Identifier: Identifier as() returns exact integer type", "[utils][identifier][as]") {
   identifier id{int32_t{42}};
 
   const auto as_i32 = id.as<int32_t>();
@@ -200,7 +200,7 @@ TEST_CASE("identifier as() returns exact integer type", "[utils][identifier][as]
   REQUIRE(as_i64.is_error());
 }
 
-TEST_CASE("identifier as() returns exact string type", "[utils][identifier][as]") {
+TEST_CASE("Identifier: Identifier as() returns exact string type", "[utils][identifier][as]") {
   identifier id{std::string{"hello"}};
 
   const auto as_string = id.as<std::string>();
@@ -211,7 +211,7 @@ TEST_CASE("identifier as() returns exact string type", "[utils][identifier][as]"
   REQUIRE(as_i32.is_error());
 }
 
-TEST_CASE("identifier as() returns null type failure for null identifier", "[utils][identifier][as]") {
+TEST_CASE("Identifier: Identifier as() returns null type failure for null identifier", "[utils][identifier][as]") {
   identifier id{nullptr};
 
   const auto as_i32 = id.as<int32_t>();
@@ -221,7 +221,7 @@ TEST_CASE("identifier as() returns null type failure for null identifier", "[uti
   REQUIRE(as_string.is_error());
 }
 
-TEST_CASE("identifier convert() converts between integer types", "[utils][identifier][convert]") {
+TEST_CASE("Identifier: Identifier convert() converts between integer types", "[utils][identifier][convert]") {
   identifier id{int32_t{123}};
 
   const auto as_i64 = id.convert<int64_t>();
@@ -233,21 +233,21 @@ TEST_CASE("identifier convert() converts between integer types", "[utils][identi
   REQUIRE(*as_u8 == 123);
 }
 
-TEST_CASE("identifier convert() rejects out of range integer conversion", "[utils][identifier][convert]") {
+TEST_CASE("Identifier: Identifier convert() rejects out of range integer conversion", "[utils][identifier][convert]") {
   identifier id{int32_t{300}};
 
   const auto as_u8 = id.convert<uint8_t>();
   REQUIRE(as_u8.is_error());
 }
 
-TEST_CASE("identifier convert() rejects negative to unsigned conversion", "[utils][identifier][convert]") {
+TEST_CASE("Identifier: Identifier convert() rejects negative to unsigned conversion", "[utils][identifier][convert]") {
   identifier id{int32_t{-1}};
 
   const auto as_u32 = id.convert<uint32_t>();
   REQUIRE(as_u32.is_error());
 }
 
-TEST_CASE("identifier convert() rejects non-integer source types", "[utils][identifier][convert]") {
+TEST_CASE("Identifier: Identifier convert() rejects non-integer source types", "[utils][identifier][convert]") {
   identifier id{std::string{"123"}};
 
   const auto as_i32 = id.convert<int32_t>();
@@ -258,7 +258,7 @@ TEST_CASE("identifier convert() rejects non-integer source types", "[utils][iden
   REQUIRE(null_to_i64.is_error());
 }
 
-TEST_CASE("identifier convert() works with unsigned integer sources", "[utils][identifier][convert]") {
+TEST_CASE("Identifier: Identifier convert() works with unsigned integer sources", "[utils][identifier][convert]") {
   identifier id{uint16_t{500}};
 
   const auto as_i32 = id.convert<int32_t>();
@@ -269,7 +269,7 @@ TEST_CASE("identifier convert() works with unsigned integer sources", "[utils][i
   REQUIRE(as_u8.is_error());
 }
 
-TEST_CASE("identifier as() and convert() behave consistently for same integer type", "[utils][identifier][as][convert]") {
+TEST_CASE("Identifier: Identifier as() and convert() behave consistently", "[utils][identifier][as][convert]") {
   identifier id{uint64_t{77}};
 
   const auto as_u64 = id.as<uint64_t>();
@@ -281,7 +281,7 @@ TEST_CASE("identifier as() and convert() behave consistently for same integer ty
   REQUIRE(*conv_u64 == 77);
 }
 
-TEST_CASE("identifier assign from value", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier assign from value", "[utils][identifier][assign]") {
   identifier id;
 
   value v1{int32_t{17}};
@@ -301,7 +301,7 @@ TEST_CASE("identifier assign from value", "[utils][identifier][assign]") {
   REQUIRE(r3.is_error());
 }
 
-TEST_CASE("identifier assign from value with exact type mapping", "[utils][identifier][assign]") {
+TEST_CASE("Identifier: Identifier assign from value with exact type mapping", "[utils][identifier][assign]") {
   identifier id;
 
   SECTION("integer types") {
@@ -328,7 +328,7 @@ TEST_CASE("identifier assign from value with exact type mapping", "[utils][ident
   }
 }
 
-TEST_CASE("identifier::to_database_type converts internal value to database_type", "[identifier]") {
+TEST_CASE("Identifier: Identifier::to_database_type converts internal value", "[identifier][to_database_type]") {
   SECTION("null identifier becomes nullptr") {
     identifier id{nullptr};
 
@@ -337,7 +337,7 @@ TEST_CASE("identifier::to_database_type converts internal value to database_type
     REQUIRE(std::holds_alternative<std::nullptr_t>(db_value));
   }
 
-  SECTION("signed integral identifier becomes same signed database type") {
+  SECTION("Identifier: Signed integral identifier becomes same signed database type") {
     identifier id{int32_t{42}};
 
     const auto db_value = id.to_database_type();
@@ -346,7 +346,7 @@ TEST_CASE("identifier::to_database_type converts internal value to database_type
     REQUIRE(std::get<int32_t>(db_value) == 42);
   }
 
-  SECTION("unsigned integral identifier becomes same unsigned database type") {
+  SECTION("Identifier: Unsigned integral identifier becomes same unsigned database type") {
     identifier id{uint64_t{123456789ULL}};
 
     const auto db_value = id.to_database_type();
@@ -355,7 +355,7 @@ TEST_CASE("identifier::to_database_type converts internal value to database_type
     REQUIRE(std::get<uint64_t>(db_value) == 123456789ULL);
   }
 
-  SECTION("string identifier becomes std::string database type") {
+  SECTION("Identifier: String identifier becomes std::string database type") {
     identifier id{std::string{"customer_1"}};
 
     const auto db_value = id.to_database_type();

test/core/utils/MessageBusTest.cpp

@@ -18,7 +18,7 @@ public:
 
 using namespace matador::utils;
 
-TEST_CASE("Basic publish/subscribe works", "[MessageBus]") {
+TEST_CASE("MessageBus: Basic publish/subscribe works", "[MessageBus]") {
     message_bus bus;
     int counter = 0;
 
@@ -32,7 +32,7 @@ TEST_CASE("Basic publish/subscribe works", "[MessageBus]") {
     REQUIRE(counter == 3);
 }
 
-TEST_CASE("Filtering works", "[MessageBus]") {
+TEST_CASE("MessageBus: Filtering works", "[MessageBus]") {
     message_bus bus;
     int counter = 0;
 
@@ -49,7 +49,7 @@ TEST_CASE("Filtering works", "[MessageBus]") {
     REQUIRE(counter == 6); // only 2 + 4
 }
 
-TEST_CASE("Member function subscription works", "[MessageBus]") {
+TEST_CASE("MessageBus: Member function subscription works", "[MessageBus]") {
     message_bus bus;
     Receiver r;
 
@@ -62,7 +62,7 @@ TEST_CASE("Member function subscription works", "[MessageBus]") {
     REQUIRE(r.received[1] == "world");
 }
 
-TEST_CASE("Shared_ptr instance subscription works", "[MessageBus]") {
+TEST_CASE("MessageBus: SharedPtr instance subscription works", "[MessageBus]") {
     message_bus bus;
     const auto r = std::make_shared<Receiver>();
 
@@ -73,7 +73,7 @@ TEST_CASE("Shared_ptr instance subscription works", "[MessageBus]") {
     REQUIRE(r->received[0] == "foo");
 }
 
-TEST_CASE("RAII unsubscription works", "[MessageBus]") {
+TEST_CASE("MessageBus: RAII unsubscription works", "[MessageBus]") {
     message_bus bus;
     int counter = 0;
 
@@ -90,7 +90,7 @@ TEST_CASE("RAII unsubscription works", "[MessageBus]") {
     REQUIRE(counter == 5);
 }
 
-TEST_CASE("Type-erased AnyMessage publish works", "[MessageBus]") {
+TEST_CASE("MessageBus: Type-erased AnyMessage publish works", "[MessageBus]") {
     message_bus bus;
     int counter = 0;
 
@@ -104,7 +104,7 @@ TEST_CASE("Type-erased AnyMessage publish works", "[MessageBus]") {
     REQUIRE(counter == 10);
 }
 
-TEST_CASE("Multiple subscribers all receive messages", "[MessageBus]") {
+TEST_CASE("MessageBus: Multiple subscribers all receive messages", "[MessageBus]") {
     message_bus bus;
     int a = 0, b = 0;
 
@@ -116,7 +116,7 @@ TEST_CASE("Multiple subscribers all receive messages", "[MessageBus]") {
     REQUIRE(b == 3);
 }
 
-TEST_CASE("Stress test with multi-threaded publish and subscribe", "[MessageBus][stress]") {
+TEST_CASE("MessageBus: Stress test with multi-threaded publish and subscribe", "[MessageBus][stress]") {
     message_bus bus;
     std::atomic<int> counter{0};
     constexpr int numThreads = 8;

test/core/utils/ResultTest.cpp

@@ -5,13 +5,15 @@
 #include "matador/utils/result.hpp"
 
 namespace matador::test {
+namespace {
 enum class math_error : int32_t {
   OK = 0,
   DIVISION_BY_ZERO = 1,
   FAILURE = 2
 };
+}
 
-utils::result<float, math_error>divide(const float x, const float y) {
+static utils::result<float, math_error>divide(const float x, const float y) {
   if (y == 0) {
     return utils::failure(math_error::DIVISION_BY_ZERO);
   }
@@ -19,15 +21,15 @@ utils::result<float, math_error>divide(const float x, const float y) {
   return utils::ok(x / y);
 }
 
-utils::result<float, math_error>multiply(const float x, const float y) {
+static utils::result<float, math_error>multiply(const float x, const float y) {
   return utils::ok(x * y);
 }
 
-utils::result<float, math_error>plus(const float x, const float y) {
+static utils::result<float, math_error>plus(const float x, const float y) {
   return utils::ok(x + y);
 }
 
-utils::result<void, math_error>action_on_greater_42(const float i) {
+static utils::result<void, math_error>action_on_greater_42(const float i) {
   if (i > 42) {
     return utils::ok<void>();
   }
@@ -88,7 +90,7 @@ TEST_CASE("Test result", "[result]") {
   REQUIRE(!res2.is_ok());
   REQUIRE(res2.is_error());
   const auto e = res2.err();
-//  REQUIRE(res2.err() == "division by zero error");
+  REQUIRE(res2.err() == "division by zero error");
 
   res = test::divide(4, 2)
     .and_then([](const auto &val) { return test::multiply(val, 5); })

test/core/utils/ThreadPoolTest.cpp

@@ -4,21 +4,21 @@
 
 using namespace matador::utils;
 
-TEST_CASE("thread_pool creates and rejects zero-sized construction") {
+TEST_CASE("ThreadPool: thread_pool creates and rejects zero-sized construction") {
     REQUIRE_THROWS(thread_pool(0));
 
     const thread_pool tp(2);
     REQUIRE(tp.size() == 2);
 }
 
-TEST_CASE("thread_pool schedules and runs simple tasks") {
+TEST_CASE("ThreadPool: thread_pool schedules and runs simple tasks") {
     thread_pool pool(3);
     auto fut = pool.schedule([](cancel_token&, const int x) { return x * 10; }, 7);
     REQUIRE(fut);
     REQUIRE(fut.value().get() == 70);
 }
 
-TEST_CASE("thread_pool parallel computation and results") {
+TEST_CASE("ThreadPool: thread_pool parallel computation and results") {
     thread_pool pool(4);
     std::vector<std::future<int>> futs;
     for (int i = 1; i <= 20; ++i) {
@@ -33,7 +33,7 @@ TEST_CASE("thread_pool parallel computation and results") {
     REQUIRE(sum == 230); // (2+3+...+21) = (21*22/2)-(1) = 231-1
 }
 
-TEST_CASE("thread_pool rejects scheduling after shutdown") {
+TEST_CASE("ThreadPool: thread_pool rejects scheduling after shutdown") {
     thread_pool pool(2);
     pool.shutdown();
     auto result = pool.schedule([](cancel_token&) { return 1; });
@@ -41,7 +41,7 @@ TEST_CASE("thread_pool rejects scheduling after shutdown") {
     REQUIRE(result.err().find("shut down") != std::string::npos);
 }
 
-TEST_CASE("thread_pool supports cancel_token for not-yet-run task") {
+TEST_CASE("ThreadPool: thread_pool supports cancel_token for not-yet-run task") {
     thread_pool pool(1);
     std::atomic ran{false};
 
@@ -73,7 +73,7 @@ TEST_CASE("thread_pool supports cancel_token for not-yet-run task") {
     // If started before cancel, may be 100 (rare but possible)
 }
 
-TEST_CASE("thread_pool: shutdown finishes all running tasks") {
+TEST_CASE("ThreadPool: thread_pool: shutdown finishes all running tasks") {
     thread_pool pool(2);
     std::atomic counter{0};
     std::vector<std::future<void>> futs;

test/core/utils/VersionTest.cpp

@@ -4,7 +4,26 @@
 
 using namespace matador::utils;
 
-TEST_CASE("Test version interface", "[version][interface]") {
+TEST_CASE("Version: Test form string", "[version][from_string]") {
+  REQUIRE(version::from_string("1.2.3").is_ok());
+  REQUIRE(version::from_string("0.0.0").is_ok());
+
+  REQUIRE(version::from_string("1.2").is_error());
+  REQUIRE(version::from_string("1.2.3.4").is_error());
+  REQUIRE(version::from_string("1.2.3abc").is_error());
+  REQUIRE(version::from_string("abc").is_error());
+  REQUIRE(version::from_string("-1.2.3").is_error());
+}
+
+TEST_CASE("Version: Test comparison", "[version][comparison]") {
+  REQUIRE(version(1, 2, 3) == version(1, 2, 3));
+  REQUIRE(version(1, 2, 3) < version(1, 2, 4));
+  REQUIRE(version(1, 2, 3) < version(1, 3, 0));
+  REQUIRE(version(1, 2, 3) < version(2, 0, 0));
+  REQUIRE(version(2, 0, 0) > version(1, 9, 9));
+}
+
+TEST_CASE("Version: Test interface", "[version][interface]") {
   version v0;
 
   REQUIRE(v0.major() == 0);
@@ -38,8 +57,8 @@ TEST_CASE("Test version interface", "[version][interface]") {
   REQUIRE(v2 == v1);
 }
 
-TEST_CASE("Test version parsing", "[version][parse]") {
-  const auto version_str{"13.67.34"};
+TEST_CASE("Version: Test parsing", "[version][parse]") {
+  constexpr auto version_str{"13.67.34"};
   const auto v1 = version::from_string(version_str);
   REQUIRE(v1.is_ok());
 

test/orm/query/DeleteQueryBuilderTest.cpp

@@ -72,7 +72,7 @@ TEST_CASE_METHOD(QueryFixture, "Test delete builder has many to many", "[query][
     .and_then( [&scm] { return scm.attach<ingredient>("ingredients"); } );
   REQUIRE(result.is_ok());
 
-  const std::vector ingredients {
+  std::vector ingredients {
     make_object<ingredient>(1, "Apple"),
     make_object<ingredient>(2, "Strawberry"),
     make_object<ingredient>(3, "Pineapple"),
@@ -82,12 +82,20 @@ TEST_CASE_METHOD(QueryFixture, "Test delete builder has many to many", "[query][
     make_object<ingredient>(7, "Beans")
   };
 
+  for (auto &i : ingredients) {
+    i.change_state(object_state::Persistent);
+  }
+
   std::vector recipes {
     make_object<recipe>(1, "Apple Pie", std::vector{ingredients[0], ingredients[3], ingredients[4]}),
     make_object<recipe>(2, "Strawberry Cake", std::vector{ingredients[5], ingredients[6]}),
     make_object<recipe>(3, "Pineapple Pie", std::vector{ingredients[0], ingredients[1], ingredients[2]})
   };
 
+  for (auto &r : recipes) {
+    r.change_state(object_state::Persistent);
+  }
+
   const auto contexts_by_type = to_contexts_by_name(scm, db->dialect());
   delete_query_builder<recipe> dqb(scm, contexts_by_type);