Replace `Results::set_live` with `Results::snapshot` and `List::snapshot`, and add tests.

`snapshot()` functions are a better fit for what realm-js needs. The new
API also makes it clearer that the liveness of a given `Results`
cannot change at arbitrary times. Changing the liveness at arbitrary
times was not safe and could result in incorrect behavior, such as a
non-live `Results` changing.

src/object-store/src/list.cpp

@@ -181,6 +181,12 @@ Results List::filter(Query q)
     return Results(m_realm, *m_object_schema, m_link_view, get_query().and_query(std::move(q)));
 }
 
+Results List::snapshot() const
+{
+    verify_attached();
+    return Results(m_realm, *m_object_schema, m_link_view).snapshot();
+}
+
 // These definitions rely on that LinkViews are interned by core
 bool List::operator==(List const& rgt) const noexcept
 {

src/object-store/src/list.hpp

@@ -75,6 +75,9 @@ public:
     Results sort(SortOrder order);
     Results filter(Query q);
 
+    // Return a Results representing a snapshot of this List.
+    Results snapshot() const;
+
     bool operator==(List const& rgt) const noexcept;
 
     NotificationToken add_notification_callback(CollectionChangeCallback cb);

src/object-store/src/results.cpp

@@ -109,9 +109,9 @@ Results::Results(Results&& other)
 , m_link_view(std::move(other.m_link_view))
 , m_table(other.m_table)
 , m_sort(std::move(other.m_sort))
-, m_live(other.m_live)
 , m_notifier(std::move(other.m_notifier))
 , m_mode(other.m_mode)
+, m_update_policy(other.m_update_policy)
 , m_has_used_table_view(other.m_has_used_table_view)
 , m_wants_background_updates(other.m_wants_background_updates)
 {
@@ -152,19 +152,6 @@ void Results::validate_write() const
         throw InvalidTransactionException("Must be in a write transaction");
 }
 
-void Results::set_live(bool live)
-{
-    validate_read();
-
-    if (!live && (m_mode == Mode::Table || m_mode == Mode::LinkView)) {
-        m_query = get_query();
-        m_mode = Mode::Query;
-    }
-
-    update_tableview();
-    m_live = live;
-}
-
 size_t Results::size()
 {
     validate_read();
@@ -208,7 +195,7 @@ RowExpr Results::get(size_t row_ndx)
             update_tableview();
             if (row_ndx >= m_table_view.size())
                 break;
-            if (!m_live && !m_table_view.is_row_attached(row_ndx))
+            if (m_update_policy == UpdatePolicy::Never && !m_table_view.is_row_attached(row_ndx))
                 return {};
             return m_table_view.get(row_ndx);
     }
@@ -258,6 +245,8 @@ util::Optional<RowExpr> Results::last()
 
 bool Results::update_linkview()
 {
+    REALM_ASSERT(m_update_policy == UpdatePolicy::Auto);
+
     if (m_sort) {
         m_query = get_query();
         m_mode = Mode::Query;
@@ -267,9 +256,13 @@ bool Results::update_linkview()
     return true;
 }
 
-void Results::update_tableview()
+void Results::update_tableview(bool wants_notifications)
 {
-    validate_read();
+    if (m_update_policy == UpdatePolicy::Never) {
+        REALM_ASSERT(m_mode == Mode::TableView);
+        return;
+    }
+
     switch (m_mode) {
         case Mode::Empty:
         case Mode::Table:
@@ -284,10 +277,7 @@ void Results::update_tableview()
             m_mode = Mode::TableView;
             break;
         case Mode::TableView:
-            if (!m_live) {
-                return;
-            }
-            if (!m_notifier && !m_realm->is_in_transaction() && m_realm->can_deliver_notifications()) {
+            if (wants_notifications && !m_notifier && !m_realm->is_in_transaction() && m_realm->can_deliver_notifications()) {
                 m_notifier = std::make_shared<_impl::ResultsNotifier>(*this);
                 _impl::RealmCoordinator::register_notifier(m_notifier);
             }
@@ -429,13 +419,16 @@ void Results::clear()
             validate_write();
             update_tableview();
 
-            if (m_live) {
+            switch (m_update_policy) {
+                case UpdatePolicy::Auto:
                     m_table_view.clear(RemoveMode::unordered);
+                    break;
+                case UpdatePolicy::Never: {
+                    // Copy the TableView because a frozen Results shouldn't let its size() change.
+                    TableView copy(m_table_view);
+                    copy.clear(RemoveMode::unordered);
+                    break;
                 }
-            else {
-                // Copy the TableView because a non-live Results shouldn't have let its size() change.
-                TableView table_view_copy = m_table_view;
-                table_view_copy.clear(RemoveMode::unordered);
             }
             break;
         case Mode::LinkView:
@@ -462,7 +455,9 @@ Query Results::get_query() const
 
             // The TableView has no associated query so create one with no conditions that is restricted
             // to the rows in the TableView.
+            if (m_update_policy == UpdatePolicy::Auto) {
                 m_table_view.sync_if_needed();
+            }
             return Query(*m_table, std::unique_ptr<TableViewBase>(new TableView(m_table_view)));
         }
         case Mode::LinkView:
@@ -504,6 +499,37 @@ Results Results::filter(Query&& q) const
     return Results(m_realm, *m_object_schema, get_query().and_query(std::move(q)), m_sort);
 }
 
+Results Results::snapshot() const &
+{
+    validate_read();
+
+    return Results(*this).snapshot();
+}
+
+Results Results::snapshot() &&
+{
+    validate_read();
+
+    switch (m_mode) {
+        case Mode::Empty:
+            return Results();
+
+        case Mode::Table:
+        case Mode::LinkView:
+            m_query = get_query();
+            m_mode = Mode::Query;
+
+            REALM_FALLTHROUGH;
+        case Mode::Query:
+        case Mode::TableView:
+            update_tableview(false);
+            m_notifier.reset();
+            m_update_policy = UpdatePolicy::Never;
+            return std::move(*this);
+    }
+    REALM_UNREACHABLE();
+}
+
 void Results::prepare_async()
 {
     if (m_realm->config().read_only) {
@@ -512,6 +538,9 @@ void Results::prepare_async()
     if (m_realm->is_in_transaction()) {
         throw InvalidTransactionException("Cannot create asynchronous query while in a write transaction");
     }
+    if (m_update_policy == UpdatePolicy::Never) {
+        throw std::logic_error("Cannot create asynchronous query for snapshotted Results.");
+    }
 
     if (!m_notifier) {
         m_wants_background_updates = true;
@@ -551,6 +580,7 @@ bool Results::is_in_table_order() const
 
 void Results::Internal::set_table_view(Results& results, realm::TableView &&tv)
 {
+    REALM_ASSERT(results.m_update_policy != UpdatePolicy::Never);
     // If the previous TableView was never actually used, then stop generating
     // new ones until the user actually uses the Results object again
     if (results.m_mode == Mode::TableView) {

src/object-store/src/results.hpp

@@ -83,9 +83,6 @@ public:
     // Get the LinkView this Results is derived from, if any
     LinkViewRef get_linkview() const { return m_link_view; }
 
-    // Set whether the TableView should sync if needed before accessing results
-    void set_live(bool live);
-
     // Get the size of this results
     // Can be either O(1) or O(N) depending on the state of things
     size_t size();
@@ -114,6 +111,10 @@ public:
     Results filter(Query&& q) const;
     Results sort(SortOrder&& sort) const;
 
+    // Return a snapshot of this Results that never updates to reflect changes in the underlying data.
+    Results snapshot() const &;
+    Results snapshot() &&;
+
     // Get the min/max/average/sum of the given column
     // All but sum() returns none when there are zero matching rows
     // sum() returns 0, except for when it returns none
@@ -129,7 +130,7 @@ public:
         Table, // Backed directly by a Table
         Query, // Backed by a query that has not yet been turned into a TableView
         LinkView,  // Backed directly by a LinkView
-        TableView // Backed by a TableView created from a Query
+        TableView, // Backed by a TableView created from a Query
     };
     // Get the currrent mode of the Results
     // Ideally this would not be public but it's needed for some KVO stuff
@@ -192,6 +193,11 @@ public:
     };
     
 private:
+    enum class UpdatePolicy {
+        Auto,  // Update automatically to reflect changes in the underlying data.
+        Never, // Never update.
+    };
+
     SharedRealm m_realm;
     const ObjectSchema *m_object_schema;
     Query m_query;
@@ -199,15 +205,15 @@ private:
     LinkViewRef m_link_view;
     Table* m_table = nullptr;
     SortOrder m_sort;
-    bool m_live = true;
 
     _impl::CollectionNotifier::Handle<_impl::ResultsNotifier> m_notifier;
 
     Mode m_mode = Mode::Empty;
+    UpdatePolicy m_update_policy = UpdatePolicy::Auto;
     bool m_has_used_table_view = false;
     bool m_wants_background_updates = true;
 
-    void update_tableview();
+    void update_tableview(bool wants_notifications = true);
     bool update_linkview();
 
     void validate_read() const;

src/object-store/tests/list.cpp

@@ -447,4 +447,35 @@ TEST_CASE("list") {
             REQUIRE(results.get(i).get_index() == i + 6);
         }
     }
+
+    SECTION("snapshot()") {
+        auto objectschema = &*r->config().schema->find("origin");
+        List list(r, *objectschema, lv);
+
+        auto snapshot = list.snapshot();
+        REQUIRE(&snapshot.get_object_schema() == objectschema);
+        REQUIRE(snapshot.get_mode() == Results::Mode::TableView);
+        REQUIRE(snapshot.size() == 10);
+
+        r->begin_transaction();
+        for (size_t i = 0; i < 5; ++i) {
+            list.remove(0);
+        }
+        REQUIRE(snapshot.size() == 10);
+        for (size_t i = 0; i < snapshot.size(); ++i) {
+            REQUIRE(snapshot.get(i).is_attached());
+        }
+        for (size_t i = 0; i < 5; ++i) {
+            target->move_last_over(i);
+        }
+        REQUIRE(snapshot.size() == 10);
+        for (size_t i = 0; i < 5; ++i) {
+            REQUIRE(!snapshot.get(i).is_attached());
+        }
+        for (size_t i = 5; i < 10; ++i) {
+            REQUIRE(snapshot.get(i).is_attached());
+        }
+        list.add(0);
+        REQUIRE(snapshot.size() == 10);
+    }
 }

src/object-store/tests/results.cpp

@@ -12,6 +12,7 @@
 #include <realm/commit_log.hpp>
 #include <realm/group_shared.hpp>
 #include <realm/link_view.hpp>
+#include <realm/query_engine.hpp>
 
 #include <unistd.h>
 
@@ -609,3 +610,307 @@ TEST_CASE("[results] error messages") {
         REQUIRE_THROWS_WITH(results.sum(0), "Cannot sum property 'value': operation not supported for 'string' properties");
     }
 }
+
+TEST_CASE("results: snapshots") {
+    InMemoryTestFile config;
+    config.cache = false;
+    config.automatic_change_notifications = false;
+    config.schema = std::make_unique<Schema>(Schema{
+        {"object", "", {
+            {"value", PropertyType::Int},
+            {"array", PropertyType::Array, "linked to object"}
+        }},
+        {"linked to object", "", {
+            {"value", PropertyType::Int}
+        }}
+    });
+
+    auto r = Realm::get_shared_realm(config);
+
+    auto write = [&](auto&& f) {
+        r->begin_transaction();
+        f();
+        r->commit_transaction();
+        advance_and_notify(*r);
+    };
+
+    SECTION("snapshot of empty Results") {
+        Results results;
+        auto snapshot = results.snapshot();
+        REQUIRE(snapshot.size() == 0);
+    }
+
+    SECTION("snapshot of Results based on Table") {
+        auto table = r->read_group()->get_table("class_object");
+        Results results(r, *config.schema->find("object"), *table);
+
+        {
+            // A newly-added row should not appear in the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 0);
+            write([=]{
+                table->add_empty_row();
+            });
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 0);
+        }
+
+        {
+            // Removing a row present in the snapshot should not affect the size of the snapshot,
+            // but will result in the snapshot returning a detached row accessor.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 1);
+            write([=]{
+                table->move_last_over(0);
+            });
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+
+            // Adding a row at the same index that was formerly present in the snapshot shouldn't
+            // affect the state of the snapshot.
+            write([=]{
+                table->add_empty_row();
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+        }
+    }
+
+    SECTION("snapshot of Results based on LinkView") {
+        auto object = r->read_group()->get_table("class_object");
+        auto linked_to = r->read_group()->get_table("class_linked to object");
+
+        write([=]{
+            object->add_empty_row();
+        });
+
+        LinkViewRef lv = object->get_linklist(1, 0);
+        Results results(r, *config.schema->find("linked to object"), lv);
+
+        {
+            // A newly-added row should not appear in the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 0);
+            write([=]{
+                lv->add(linked_to->add_empty_row());
+            });
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 0);
+        }
+
+        {
+            // Removing a row from the link list should not affect the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 1);
+            write([=]{
+                lv->remove(0);
+            });
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(snapshot.get(0).is_attached());
+
+            // Removing a row present in the snapshot from its table should result in the snapshot
+            // returning a detached row accessor.
+            write([=]{
+                linked_to->remove(0);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+
+            // Adding a new row to the link list shouldn't affect the state of the snapshot.
+            write([=]{
+                lv->add(linked_to->add_empty_row());
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+        }
+    }
+
+    SECTION("snapshot of Results based on Query") {
+        auto table = r->read_group()->get_table("class_object");
+        Query q = table->column<Int>(0) > 0;
+        Results results(r, *config.schema->find("object"), std::move(q));
+
+        {
+            // A newly-added row should not appear in the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 0);
+            write([=]{
+                table->set_int(0, table->add_empty_row(), 1);
+            });
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 0);
+        }
+
+        {
+            // Updating a row to no longer match the query criteria should not affect the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 1);
+            write([=]{
+                table->set_int(0, 0, 0);
+            });
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(snapshot.get(0).is_attached());
+
+            // Removing a row present in the snapshot from its table should result in the snapshot
+            // returning a detached row accessor.
+            write([=]{
+                table->remove(0);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+
+            // Adding a new row that matches the query criteria shouldn't affect the state of the snapshot.
+            write([=]{
+                table->set_int(0, table->add_empty_row(), 1);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+        }
+    }
+
+    SECTION("snapshot of Results based on TableView from query") {
+        auto table = r->read_group()->get_table("class_object");
+        Query q = table->column<Int>(0) > 0;
+        Results results(r, *config.schema->find("object"), q.find_all(), {});
+
+        {
+            // A newly-added row should not appear in the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 0);
+            write([=]{
+                table->set_int(0, table->add_empty_row(), 1);
+            });
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 0);
+        }
+
+        {
+            // Updating a row to no longer match the query criteria should not affect the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 1);
+            write([=]{
+                table->set_int(0, 0, 0);
+            });
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(snapshot.get(0).is_attached());
+
+            // Removing a row present in the snapshot from its table should result in the snapshot
+            // returning a detached row accessor.
+            write([=]{
+                table->remove(0);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+
+            // Adding a new row that matches the query criteria shouldn't affect the state of the snapshot.
+            write([=]{
+                table->set_int(0, table->add_empty_row(), 1);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+        }
+    }
+
+    SECTION("snapshot of Results based on TableView from backlinks") {
+        auto object = r->read_group()->get_table("class_object");
+        auto linked_to = r->read_group()->get_table("class_linked to object");
+
+        write([=]{
+            linked_to->add_empty_row();
+        });
+
+        TableView backlinks = linked_to->get_backlink_view(0, object.get(), 1);
+        Results results(r, *config.schema->find("object"), std::move(backlinks), {});
+
+        auto lv = object->get_linklist(1, object->add_empty_row());
+
+        {
+            // A newly-added row should not appear in the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 0);
+            write([=]{
+                lv->add(0);
+            });
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 0);
+        }
+
+        {
+            // Removing the link should not affect the snapshot.
+            auto snapshot = results.snapshot();
+            REQUIRE(results.size() == 1);
+            REQUIRE(snapshot.size() == 1);
+            write([=]{
+                lv->remove(0);
+            });
+            REQUIRE(results.size() == 0);
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(snapshot.get(0).is_attached());
+
+            // Removing a row present in the snapshot from its table should result in the snapshot
+            // returning a detached row accessor.
+            write([=]{
+                object->remove(0);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+
+            // Adding a new link shouldn't affect the state of the snapshot.
+            write([=]{
+                object->add_empty_row();
+                auto lv = object->get_linklist(1, object->add_empty_row());
+                lv->add(0);
+            });
+            REQUIRE(snapshot.size() == 1);
+            REQUIRE(!snapshot.get(0).is_attached());
+        }
+    }
+
+    SECTION("snapshot of Results with notification callback registered") {
+        auto table = r->read_group()->get_table("class_object");
+        Query q = table->column<Int>(0) > 0;
+        Results results(r, *config.schema->find("object"), q.find_all(), {});
+
+        auto token = results.add_notification_callback([&](CollectionChangeSet, std::exception_ptr err) {
+            REQUIRE_FALSE(err);
+        });
+        advance_and_notify(*r);
+
+        SECTION("snapshot of lvalue") {
+            auto snapshot = results.snapshot();
+            write([=] {
+                table->set_int(0, table->add_empty_row(), 1);
+            });
+            REQUIRE(snapshot.size() == 0);
+        }
+
+        SECTION("snapshot of rvalue") {
+            auto snapshot = std::move(results).snapshot();
+            write([=] {
+                table->set_int(0, table->add_empty_row(), 1);
+            });
+            REQUIRE(snapshot.size() == 0);
+        }
+    }
+
+    SECTION("adding notification callback to snapshot throws") {
+        auto table = r->read_group()->get_table("class_object");
+        Query q = table->column<Int>(0) > 0;
+        Results results(r, *config.schema->find("object"), q.find_all(), {});
+        auto snapshot = results.snapshot();
+        CHECK_THROWS(snapshot.add_notification_callback([](CollectionChangeSet, std::exception_ptr) {}));
+    }
+}