From 8be8b51bed4c9ece2dd6f6fe40c1668ba9905b65 Mon Sep 17 00:00:00 2001
From: Dimitris Christodoulou <36637689+DemetrisChr@users.noreply.github.com>
Date: Thu, 22 Aug 2024 15:24:21 +0100
Subject: [PATCH] CXXCBC-531: Transfer ownership of
 range_scan_orchestrator_impl to scan_result (#645)

---
 core/impl/collection.cxx           |  6 +++---
 core/impl/internal_scan_result.hxx |  3 +--
 core/impl/scan_result.cxx          |  6 ++----
 core/range_scan_orchestrator.cxx   |  8 ++++++--
 core/scan_result.cxx               | 25 +++++++------------------
 core/scan_result.hxx               |  2 +-
 6 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/core/impl/collection.cxx b/core/impl/collection.cxx
index 7ad19507f..9e8a2253e 100644
--- a/core/impl/collection.cxx
+++ b/core/impl/collection.cxx
@@ -905,12 +905,12 @@ class collection_impl : public std::enable_shared_from_this<collection_impl>
                                                               core_scan_type,
                                                               orchestrator_opts);
             return orchestrator.scan(
-              [handler = std::move(handler), orchestrator](auto ec, auto core_scan_result) mutable {
+              [handler = std::move(handler)](auto ec, auto core_scan_result) mutable {
                 if (ec) {
                   return handler(error(ec, "Error while starting the range scan"), {});
                 }
-                auto internal_result = std::make_shared<internal_scan_result>(
-                  std::move(orchestrator), std::move(core_scan_result));
+                auto internal_result =
+                  std::make_shared<internal_scan_result>(std::move(core_scan_result));
                 return handler({}, scan_result{ internal_result });
               });
           });
diff --git a/core/impl/internal_scan_result.hxx b/core/impl/internal_scan_result.hxx
index 52a0451d6..60590b1ba 100644
--- a/core/impl/internal_scan_result.hxx
+++ b/core/impl/internal_scan_result.hxx
@@ -27,7 +27,7 @@ class internal_scan_result
 {
 
 public:
-  internal_scan_result(core::range_scan_orchestrator orchestrator, core::scan_result core_result);
+  explicit internal_scan_result(core::scan_result core_result);
   internal_scan_result(const internal_scan_result&) = delete;
   internal_scan_result(internal_scan_result&&) noexcept = default;
   auto operator=(const internal_scan_result&) -> internal_scan_result& = delete;
@@ -38,7 +38,6 @@ public:
   void cancel();
 
 private:
-  core::range_scan_orchestrator orchestrator_;
   core::scan_result core_result_;
 };
 } // namespace couchbase
diff --git a/core/impl/scan_result.cxx b/core/impl/scan_result.cxx
index cc94ad70c..b2b7c38d5 100644
--- a/core/impl/scan_result.cxx
+++ b/core/impl/scan_result.cxx
@@ -50,10 +50,8 @@ to_scan_result_item(core::range_scan_item core_item) -> scan_result_item
 }
 } // namespace
 
-internal_scan_result::internal_scan_result(core::range_scan_orchestrator orchestrator,
-                                           core::scan_result core_result)
-  : orchestrator_{ std::move(orchestrator) }
-  , core_result_{ std::move(core_result) }
+internal_scan_result::internal_scan_result(core::scan_result core_result)
+  : core_result_{ std::move(core_result) }
 {
 }
 
diff --git a/core/range_scan_orchestrator.cxx b/core/range_scan_orchestrator.cxx
index 0e71f3fe5..17c66186e 100644
--- a/core/range_scan_orchestrator.cxx
+++ b/core/range_scan_orchestrator.cxx
@@ -440,7 +440,8 @@ class range_scan_orchestrator_impl
           self->streams_[vbucket] = stream;
         }
         self->start_streams(self->concurrency_);
-        return cb({}, scan_result(self));
+        // Transferring ownership of the range_scan_orchestrator impl to the scan_result
+        return cb({}, scan_result(std::move(self)));
       });
   }
 
@@ -663,6 +664,9 @@ range_scan_orchestrator::scan() -> tl::expected<scan_result, std::error_code>
 void
 range_scan_orchestrator::scan(couchbase::core::scan_callback&& cb)
 {
-  return impl_->scan(std::move(cb));
+  if (impl_) {
+    return impl_->scan(std::move(cb));
+  }
+  cb(errc::common::request_canceled, {});
 }
 } // namespace couchbase::core
diff --git a/core/scan_result.cxx b/core/scan_result.cxx
index 31a16b970..a145e58d1 100644
--- a/core/scan_result.cxx
+++ b/core/scan_result.cxx
@@ -25,47 +25,36 @@ namespace couchbase::core
 class scan_result_impl
 {
 public:
-  explicit scan_result_impl(std::weak_ptr<range_scan_item_iterator> iterator)
+  explicit scan_result_impl(std::shared_ptr<range_scan_item_iterator> iterator)
     : iterator_{ std::move(iterator) }
   {
   }
 
   [[nodiscard]] auto next() const -> tl::expected<range_scan_item, std::error_code>
   {
-    if (auto ptr = iterator_.lock(); ptr != nullptr) {
-      return ptr->next().get();
-    }
-    return tl::unexpected{ errc::common::request_canceled };
+    return iterator_->next().get();
   }
 
   void next(utils::movable_function<void(range_scan_item, std::error_code)> callback) const
   {
-    if (auto ptr = iterator_.lock(); ptr != nullptr) {
-      return ptr->next(std::move(callback));
-    }
-    callback({}, errc::common::request_canceled);
+    return iterator_->next(std::move(callback));
   }
 
   void cancel()
   {
-    if (auto ptr = iterator_.lock(); ptr != nullptr) {
-      return ptr->cancel();
-    }
+    return iterator_->cancel();
   }
 
   [[nodiscard]] auto is_cancelled() -> bool
   {
-    if (auto ptr = iterator_.lock(); ptr != nullptr) {
-      return ptr->is_cancelled();
-    }
-    return false;
+    return iterator_->is_cancelled();
   }
 
 private:
-  std::weak_ptr<range_scan_item_iterator> iterator_;
+  std::shared_ptr<range_scan_item_iterator> iterator_;
 };
 
-scan_result::scan_result(std::weak_ptr<range_scan_item_iterator> iterator)
+scan_result::scan_result(std::shared_ptr<range_scan_item_iterator> iterator)
   : impl_{ std::make_shared<scan_result_impl>(std::move(iterator)) }
 {
 }
diff --git a/core/scan_result.hxx b/core/scan_result.hxx
index 29082aac5..fc24c6117 100644
--- a/core/scan_result.hxx
+++ b/core/scan_result.hxx
@@ -41,7 +41,7 @@ class scan_result
 {
 public:
   scan_result() = default;
-  explicit scan_result(std::weak_ptr<range_scan_item_iterator> iterator);
+  explicit scan_result(std::shared_ptr<range_scan_item_iterator> iterator);
   [[nodiscard]] auto next() const -> tl::expected<range_scan_item, std::error_code>;
   void next(utils::movable_function<void(range_scan_item, std::error_code)> callback) const;
   void cancel();