refactored object_proxy and object_ptr and fixed their bugs

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)
+  : resolver_(std::move(resolver))
-  , pk_(std::move(id)) {
+  , 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)
+  : obj_(std::move(obj))
-  , resolver_(resolver)
+  , resolver_(std::move(resolver))
-  , pk_(primary_key_resolver::resolve_object(*obj).pk)
+  , pk_(obj_ ? primary_key_resolver::resolve_object(*obj_).pk : utils::identifier{})
-  , state_(object_state::Persistent){
+  , state_(obj_ ? object_state::Persistent : object_state::Detached) {
   }
 
   // Transient
   explicit object_proxy(std::shared_ptr<Type> obj)
-  : obj_(obj)
+  : obj_(std::move(obj))
-  , pk_(primary_key_resolver::resolve_object(*obj).pk) {
+  , 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_) {
+    if (!obj_) {
-      pk_ = primary_key_resolver::resolve_object(*obj_).pk;
+      pk_.clear();
-      state_.store(object_state::Persistent, std::memory_order_release);
+      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_) {
+    return resolve_object();
-      std::ignore = resolve();
-    }
-    return obj_;
   }
 
   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->() {
-  Type &operator*() { return *pointer(); }
+    auto *ptr = pointer();
-  const Type &operator*() const { return *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_; }
+  [[nodiscard]] bool has_primary_key() const {
-  void primary_key(const utils::identifier &pk) { pk_ = pk; }
+    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);
+    std::lock_guard lock(mutex_);
+    state_ = state;
   }
 private:
-  Type* resolve() const {
+  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_.get();
+        return obj_;
       }
 
-    std::lock_guard lock(mutex_);
+      resolver = resolver_.lock();
-    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)");
       }
 
-    const_cast<std::shared_ptr<Type>&>(obj_) = resolver->resolve(pk_);
+      pk = pk_;
-
-    return obj_.get();
     }
 
+    current = resolver->resolve(pk);
+
+    {
+      std::lock_guard lock(mutex_);
+      if (!obj_) {
+        obj_ = std::move(current);
+      }
+      return obj_;
+    }
+  }
 private:
-  std::shared_ptr<Type> obj_{};
+  mutable std::shared_ptr<Type> obj_{};
-  std::weak_ptr<object_resolver<Type>> resolver_{};
+  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->() const {
-  Type &operator*() { return *get(); }
+    return checked_get();
-  const Type &operator*() const { return *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(); }
+  [[nodiscard]] std::shared_ptr<Type> object() const {
-  void reset(const std::shared_ptr<object_proxy<Type>>& proxy) { proxy_ = proxy; }
+    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(); }
+  void reset(std::shared_ptr<object_proxy<Type>> proxy) {
-  [[nodiscard]] bool valid() const { return proxy_ != nullptr && !proxy_->empty(); }
+    proxy_ = std::move(proxy);
+  }
 
-  [[nodiscard]] bool has_primary_key() const { return proxy_->has_primary_key(); }
+  [[nodiscard]] std::shared_ptr<object_proxy<Type>> proxy() const {
-  [[nodiscard]] const utils::identifier &primary_key() const { return proxy_->primary_key(); }
+    return proxy_;
-  void primary_key(const utils::identifier &pk) { proxy_->primary_key(pk); }
+  }
 
-  [[nodiscard]] bool is_persistent() const { return proxy_->is_persistent(); }
+  explicit operator bool() const {
-  [[nodiscard]] bool is_transient() const { return proxy_->is_transient(); }
+    return valid();
-  [[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); }
+  [[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>