refactor: let ChangeCollection/ChangeIterator keep a copy of the Session

...so that we can (finally) implement some better locking.

Change-Id: I9417abba054a4b4ec21b0873a4ec090cf7251d18

Author: jktjkt

include/sysrepo-cpp/Changes.hpp

@@ -10,9 +10,9 @@
 #include <memory>
 #include <optional>
 #include <sysrepo-cpp/Enum.hpp>
+#include <sysrepo-cpp/Session.hpp>
 #include <string>
 
-struct sr_session_ctx_s;
 struct sr_change_iter_s;
 
 namespace sysrepo {
@@ -99,14 +99,15 @@ class ChangeIterator {
     struct iterator_end_tag{
     };
 
-    ChangeIterator(sr_change_iter_s* iter, std::shared_ptr<sr_session_ctx_s> sess);
+    ChangeIterator(sr_change_iter_s* iter, Session sess);
     ChangeIterator(const iterator_end_tag);
     friend ChangeCollection;
 
     std::optional<Change> m_current;
 
     std::shared_ptr<sr_change_iter_s> m_iter;
-    std::shared_ptr<sr_session_ctx_s> m_sess;
+    // FIXME: get rid of the optional<> when we have a sentinel instead of an end iterator
+    std::optional<Session> m_sess;
 };
 
 /**
@@ -128,9 +129,9 @@ class ChangeCollection {
     ChangeIterator end() const;
 
 private:
-    ChangeCollection(const std::string& xpath, std::shared_ptr<sr_session_ctx_s> sess);
+    ChangeCollection(const std::string& xpath, Session sess);
     friend Session;
     std::string m_xpath;
-    std::shared_ptr<sr_session_ctx_s> m_sess;
+    Session m_sess;
 };
 }

include/sysrepo-cpp/Session.hpp

@@ -20,6 +20,7 @@ struct sr_session_ctx_s;
 namespace sysrepo {
 class Connection;
 class ChangeCollection;
+class ChangeIterator;
 class DynamicSubscription;
 class Session;
 class Subscription;
@@ -178,6 +179,8 @@ class Session {
 
 private:
     friend Connection;
+    friend ChangeCollection;
+    friend ChangeIterator;
     friend Session wrapUnmanagedSession(sr_session_ctx_s* session);
     friend Subscription;
     friend sr_session_ctx_s* getRawSession(Session sess);

src/Session.cpp

@@ -695,7 +695,7 @@ DynamicSubscription Session::subscribeNotifications(
  */
 ChangeCollection Session::getChanges(const std::string& xpath)
 {
-    return ChangeCollection{xpath, m_sess};
+    return ChangeCollection{xpath, *this};
 }
 
 /**

src/Subscription.cpp

@@ -293,9 +293,9 @@ Subscription::Subscription(Subscription&& other) noexcept = default;
 
 Subscription& Subscription::operator=(Subscription&& other) noexcept = default;
 
-ChangeCollection::ChangeCollection(const std::string& xpath, std::shared_ptr<sr_session_ctx_s> sess)
+ChangeCollection::ChangeCollection(const std::string& xpath, Session sess)
     : m_xpath(xpath)
-    , m_sess(sess)
+    , m_sess(std::move(sess))
 {
 }
 
@@ -305,9 +305,9 @@ ChangeCollection::ChangeCollection(const std::string& xpath, std::shared_ptr<sr_
 ChangeIterator ChangeCollection::begin() const
 {
     sr_change_iter_t* iter;
-    auto res = sr_get_changes_iter(m_sess.get(), m_xpath.c_str(), &iter);
+    auto res = sr_get_changes_iter(m_sess.m_sess.get(), m_xpath.c_str(), &iter);
 
-    throwIfError(res, "Couldn't create an iterator for changes", m_sess.get());
+    throwIfError(res, "Couldn't create an iterator for changes", m_sess.m_sess.get());
 
     return ChangeIterator{iter, m_sess};
 }
@@ -323,17 +323,17 @@ ChangeIterator ChangeCollection::end() const
 /**
  * Wraps `sr_change_iter_s`.
  */
-ChangeIterator::ChangeIterator(sr_change_iter_s* iter, std::shared_ptr<sr_session_ctx_s> sess)
+ChangeIterator::ChangeIterator(sr_change_iter_s* iter, Session sess)
     : m_iter(iter, sr_free_change_iter)
-    , m_sess(sess)
+    , m_sess(std::move(sess))
 {
     operator++();
 }
 
 ChangeIterator::ChangeIterator(const iterator_end_tag)
     : m_current(std::nullopt)
     , m_iter(nullptr)
-    , m_sess(nullptr)
+    , m_sess(std::nullopt)
 {
 }
 
@@ -347,14 +347,14 @@ ChangeIterator& ChangeIterator::operator++()
     const char* prevValue;
     const char* prevList;
     int prevDefault;
-    auto ret = sr_get_change_tree_next(m_sess.get(), m_iter.get(), &operation, &node, &prevValue, &prevList, &prevDefault);
+    auto ret = sr_get_change_tree_next(m_sess->m_sess.get(), m_iter.get(), &operation, &node, &prevValue, &prevList, &prevDefault);
 
     if (ret == SR_ERR_NOT_FOUND) {
         m_current = std::nullopt;
         return *this;
     }
 
-    throwIfError(ret, "Could not iterate to the next change", m_sess.get());
+    throwIfError(ret, "Could not iterate to the next change", m_sess->m_sess.get());
 
     // I can safely "dereference" the change here, because last change is handled by the condition above.
     m_current.emplace(Change{