Add support for fine-grained notifications from backlink TVs

src/impl/collection_change_builder.cpp

@@ -550,9 +550,9 @@ void calculate_moves_sorted(std::vector<RowInfo>& rows, CollectionChangeSet& cha
 CollectionChangeBuilder CollectionChangeBuilder::calculate(std::vector<size_t> const& prev_rows,
                                                            std::vector<size_t> const& next_rows,
                                                            std::function<bool (size_t)> row_did_change,
-                                                           bool sort)
+                                                           bool rows_are_in_table_order)
 {
-    REALM_ASSERT_DEBUG(sort || std::is_sorted(begin(next_rows), end(next_rows)));
+    REALM_ASSERT_DEBUG(!rows_are_in_table_order || std::is_sorted(begin(next_rows), end(next_rows)));
 
     CollectionChangeBuilder ret;
 
@@ -627,7 +627,7 @@ CollectionChangeBuilder CollectionChangeBuilder::calculate(std::vector<size_t> c
         }
     }
 
-    if (sort) {
+    if (!rows_are_in_table_order) {
         calculate_moves_sorted(new_rows, ret);
     }
     else {

src/impl/results_notifier.cpp

@@ -27,7 +27,7 @@ ResultsNotifier::ResultsNotifier(Results& target)
 : CollectionNotifier(target.get_realm())
 , m_target_results(&target)
 , m_sort(target.get_sort())
-, m_from_linkview(target.get_linkview().get() != nullptr)
+, m_target_is_in_table_order(target.is_in_table_order())
 {
     Query q = target.get_query();
     set_table(*q.get_table());
@@ -87,12 +87,8 @@ bool ResultsNotifier::need_to_run()
     }
 
     // If we've run previously, check if we need to rerun
-    if (m_initial_run_complete) {
+    if (m_initial_run_complete && m_query->sync_view_if_needed() == m_last_seen_version) {
-        // Make an empty tableview from the query to get the table version, since
+        return false;
-        // Query doesn't expose it
-        if (m_query->find_all(0, 0, 0).sync_if_needed() == m_last_seen_version) {
-            return false;
-        }
     }
 
     return true;
@@ -125,7 +121,7 @@ void ResultsNotifier::calculate_changes()
 
         m_changes = CollectionChangeBuilder::calculate(m_previous_rows, next_rows,
                                                        get_modification_checker(*m_info, *m_query->get_table()),
-                                                       m_sort || m_from_linkview);
+                                                       m_target_is_in_table_order && !m_sort);
 
         m_previous_rows = std::move(next_rows);
     }
@@ -141,6 +137,7 @@ void ResultsNotifier::run()
     if (!need_to_run())
         return;
 
+    m_query->sync_view_if_needed();
     m_tv = m_query->find_all();
     if (m_sort) {
         m_tv.sort(m_sort.column_indices, m_sort.ascending);

src/impl/results_notifier.hpp

@@ -36,7 +36,7 @@ private:
     Results* m_target_results;
 
     const SortOrder m_sort;
-    bool m_from_linkview;
+    bool m_target_is_in_table_order;
 
     // The source Query, in handover form iff m_sg is null
     std::unique_ptr<SharedGroup::Handover<Query>> m_query_handover;

src/results.cpp

@@ -72,6 +72,17 @@ Results::Results(SharedRealm r, const ObjectSchema& o, LinkViewRef lv, util::Opt
     }
 }
 
+Results::Results(SharedRealm r, const ObjectSchema& o, TableView tv, SortOrder s)
+: m_realm(std::move(r))
+, m_object_schema(&o)
+, m_table_view(std::move(tv))
+, m_table(&m_table_view.get_parent())
+, m_sort(std::move(s))
+, m_mode(Mode::TableView)
+{
+    REALM_ASSERT(m_sort.column_indices.size() == m_sort.ascending.size());
+}
+
 Results::~Results()
 {
     if (m_notifier) {
@@ -85,7 +96,7 @@ void Results::validate_read() const
         m_realm->verify_thread();
     if (m_table && !m_table->is_attached())
         throw InvalidatedException();
-    if (m_mode == Mode::TableView && !m_table_view.is_attached())
+    if (m_mode == Mode::TableView && (!m_table_view.is_attached() || m_table_view.depends_on_deleted_object()))
         throw InvalidatedException();
     if (m_mode == Mode::LinkView && !m_link_view->is_attached())
         throw InvalidatedException();
@@ -115,8 +126,10 @@ size_t Results::size()
     switch (m_mode) {
         case Mode::Empty:    return 0;
         case Mode::Table:    return m_table->size();
-        case Mode::Query:    return m_query.count();
         case Mode::LinkView: return m_link_view->size();
+        case Mode::Query:
+            m_query.sync_view_if_needed();
+            return m_query.count();
         case Mode::TableView:
             update_tableview();
             return m_table_view.size();
@@ -218,6 +231,7 @@ void Results::update_tableview()
         case Mode::LinkView:
             return;
         case Mode::Query:
+            m_query.sync_view_if_needed();
             m_table_view = m_query.find_all();
             if (m_sort) {
                 m_table_view.sort(m_sort.column_indices, m_sort.ascending);
@@ -384,8 +398,19 @@ Query Results::get_query() const
         case Mode::Empty:
         case Mode::Query:
             return m_query;
-        case Mode::TableView:
+        case Mode::TableView: {
-            return m_table_view.get_query();
+            // A TableView has an associated Query if it was produced by Query::find_all. This is indicated
+            // by TableView::get_query returning a Query with a non-null table.
+            Query query = m_table_view.get_query();
+            if (query.get_table()) {
+                return query;
+            }
+
+            // The TableView has no associated query so create one with no conditions that is restricted
+            // to the rows in the TableView.
+            m_table_view.sync_if_needed();
+            return Query(*m_table, std::make_unique<TableView>(m_table_view));
+        }
         case Mode::LinkView:
             return m_table->where(m_link_view);
         case Mode::Table:
@@ -417,15 +442,11 @@ TableView Results::get_tableview()
 Results Results::sort(realm::SortOrder&& sort) const
 {
     REALM_ASSERT(sort.column_indices.size() == sort.ascending.size());
-    if (m_link_view)
-        return Results(m_realm, *m_object_schema, m_link_view, m_query, std::move(sort));
     return Results(m_realm, *m_object_schema, get_query(), std::move(sort));
 }
 
 Results Results::filter(Query&& q) const
 {
-    if (m_link_view)
-        return Results(m_realm, *m_object_schema, m_link_view, get_query().and_query(std::move(q)), m_sort);
     return Results(m_realm, *m_object_schema, get_query().and_query(std::move(q)), m_sort);
 }
 
@@ -457,6 +478,21 @@ NotificationToken Results::add_notification_callback(CollectionChangeCallback cb
     return {m_notifier, m_notifier->add_callback(std::move(cb))};
 }
 
+bool Results::is_in_table_order() const
+{
+    switch (m_mode) {
+        case Mode::Empty:
+        case Mode::Table:
+            return true;
+        case Mode::LinkView:
+            return false;
+        case Mode::Query:
+            return m_query.produces_results_in_table_order() && !m_sort;
+        case Mode::TableView:
+            return m_table_view.is_in_table_order();
+    }
+}
+
 void Results::Internal::set_table_view(Results& results, realm::TableView &&tv)
 {
     // If the previous TableView was never actually used, then stop generating

src/results.hpp

@@ -51,6 +51,7 @@ public:
     Results() = default;
     Results(SharedRealm r, const ObjectSchema& o, Table& table);
     Results(SharedRealm r, const ObjectSchema& o, Query q, SortOrder s = {});
+    Results(SharedRealm r, const ObjectSchema& o, TableView tv, SortOrder s);
     Results(SharedRealm r, const ObjectSchema& o, LinkViewRef lv, util::Optional<Query> q = {}, SortOrder s = {});
     ~Results();
 
@@ -182,6 +183,9 @@ public:
 
     bool wants_background_updates() const { return m_wants_background_updates; }
 
+    // Returns whether the rows are guaranteed to be in table order.
+    bool is_in_table_order() const;
+
     // Helper type to let ResultsNotifier update the tableview without giving access
     // to any other privates or letting anyone else do so
     class Internal {

tests/collection_change_indices.cpp

@@ -338,97 +338,6 @@ TEST_CASE("[collection_change] calculate() unsorted") {
     auto none_modified = [](size_t) { return false; };
     const auto npos = size_t(-1);
 
-    SECTION("returns an empty set when input and output are identical") {
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, none_modified, false);
-        REQUIRE(c.empty());
-    }
-
-    SECTION("marks all as inserted when prev is empty") {
-        c = _impl::CollectionChangeBuilder::calculate({}, {1, 2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.insertions, 0, 1, 2);
-    }
-
-    SECTION("marks all as deleted when new is empty") {
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {}, all_modified, false);
-        REQUIRE_INDICES(c.deletions, 0, 1, 2);
-    }
-
-    SECTION("marks npos rows in prev as deleted") {
-        c = _impl::CollectionChangeBuilder::calculate({npos, 1, 2, 3, npos}, {1, 2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.deletions, 0, 4);
-    }
-
-    SECTION("marks modified rows which do not move as modified") {
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.modifications, 0, 1, 2);
-    }
-
-    SECTION("does not mark unmodified rows as modified") {
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, none_modified, false);
-        REQUIRE(c.modifications.empty());
-    }
-
-    SECTION("marks newly added rows as insertions") {
-        c = _impl::CollectionChangeBuilder::calculate({2, 3}, {1, 2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.insertions, 0);
-
-        c = _impl::CollectionChangeBuilder::calculate({1, 3}, {1, 2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.insertions, 1);
-
-        c = _impl::CollectionChangeBuilder::calculate({1, 2}, {1, 2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.insertions, 2);
-    }
-
-    SECTION("marks removed rows as deleted") {
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2}, all_modified, false);
-        REQUIRE_INDICES(c.deletions, 2);
-
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3}, all_modified, false);
-        REQUIRE_INDICES(c.deletions, 1);
-
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {2, 3}, all_modified, false);
-        REQUIRE_INDICES(c.deletions, 0);
-    }
-
-    SECTION("marks rows as both inserted and deleted") {
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3, 4}, all_modified, false);
-        REQUIRE_INDICES(c.deletions, 1);
-        REQUIRE_INDICES(c.insertions, 2);
-        REQUIRE(c.moves.empty());
-    }
-
-    SECTION("marks rows as modified even if they moved") {
-        c = _impl::CollectionChangeBuilder::calculate({5, 3}, {3, 5}, all_modified, false);
-        REQUIRE_MOVES(c, {1, 0});
-        REQUIRE_INDICES(c.modifications, 0, 1);
-    }
-
-    SECTION("does not mark rows as modified if they are new") {
-        c = _impl::CollectionChangeBuilder::calculate({3}, {3, 5}, all_modified, false);
-        REQUIRE_INDICES(c.modifications, 0);
-    }
-
-    SECTION("reports moves which can be produced by move_last_over()") {
-        auto calc = [&](std::vector<size_t> values) {
-            return _impl::CollectionChangeBuilder::calculate(values, {1, 2, 3}, none_modified, false);
-        };
-
-        REQUIRE(calc({1, 2, 3}).empty());
-        REQUIRE_MOVES(calc({1, 3, 2}), {2, 1});
-        REQUIRE_MOVES(calc({2, 1, 3}), {1, 0});
-        REQUIRE_MOVES(calc({2, 3, 1}), {2, 0});
-        REQUIRE_MOVES(calc({3, 1, 2}), {1, 0}, {2, 1});
-        REQUIRE_MOVES(calc({3, 2, 1}), {2, 0}, {1, 1});
-    }
-}
-
-TEST_CASE("[collection_change] calculate() sorted") {
-    _impl::CollectionChangeBuilder c;
-
-    auto all_modified = [](size_t) { return true; };
-    auto none_modified = [](size_t) { return false; };
-    const auto npos = size_t(-1);
-
     SECTION("returns an empty set when input and output are identical") {
         c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, none_modified, true);
         REQUIRE(c.empty());
@@ -489,9 +398,8 @@ TEST_CASE("[collection_change] calculate() sorted") {
     }
 
     SECTION("marks rows as modified even if they moved") {
-        c = _impl::CollectionChangeBuilder::calculate({3, 5}, {5, 3}, all_modified, true);
+        c = _impl::CollectionChangeBuilder::calculate({5, 3}, {3, 5}, all_modified, true);
-        REQUIRE_INDICES(c.deletions, 1);
+        REQUIRE_MOVES(c, {1, 0});
-        REQUIRE_INDICES(c.insertions, 0);
         REQUIRE_INDICES(c.modifications, 0, 1);
     }
 
@@ -500,9 +408,101 @@ TEST_CASE("[collection_change] calculate() sorted") {
         REQUIRE_INDICES(c.modifications, 0);
     }
 
+    SECTION("reports moves which can be produced by move_last_over()") {
+        auto calc = [&](std::vector<size_t> values) {
+            return _impl::CollectionChangeBuilder::calculate(values, {1, 2, 3}, none_modified, true);
+        };
+
+        REQUIRE(calc({1, 2, 3}).empty());
+        REQUIRE_MOVES(calc({1, 3, 2}), {2, 1});
+        REQUIRE_MOVES(calc({2, 1, 3}), {1, 0});
+        REQUIRE_MOVES(calc({2, 3, 1}), {2, 0});
+        REQUIRE_MOVES(calc({3, 1, 2}), {1, 0}, {2, 1});
+        REQUIRE_MOVES(calc({3, 2, 1}), {2, 0}, {1, 1});
+    }
+}
+
+TEST_CASE("[collection_change] calculate() sorted") {
+    _impl::CollectionChangeBuilder c;
+
+    auto all_modified = [](size_t) { return true; };
+    auto none_modified = [](size_t) { return false; };
+    const auto npos = size_t(-1);
+
+    SECTION("returns an empty set when input and output are identical") {
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, none_modified, false);
+        REQUIRE(c.empty());
+    }
+
+    SECTION("marks all as inserted when prev is empty") {
+        c = _impl::CollectionChangeBuilder::calculate({}, {1, 2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.insertions, 0, 1, 2);
+    }
+
+    SECTION("marks all as deleted when new is empty") {
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 0, 1, 2);
+    }
+
+    SECTION("marks npos rows in prev as deleted") {
+        c = _impl::CollectionChangeBuilder::calculate({npos, 1, 2, 3, npos}, {1, 2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 0, 4);
+    }
+
+    SECTION("marks modified rows which do not move as modified") {
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.modifications, 0, 1, 2);
+    }
+
+    SECTION("does not mark unmodified rows as modified") {
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2, 3}, none_modified, false);
+        REQUIRE(c.modifications.empty());
+    }
+
+    SECTION("marks newly added rows as insertions") {
+        c = _impl::CollectionChangeBuilder::calculate({2, 3}, {1, 2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.insertions, 0);
+
+        c = _impl::CollectionChangeBuilder::calculate({1, 3}, {1, 2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.insertions, 1);
+
+        c = _impl::CollectionChangeBuilder::calculate({1, 2}, {1, 2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.insertions, 2);
+    }
+
+    SECTION("marks removed rows as deleted") {
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 2}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 2);
+
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 1);
+
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {2, 3}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 0);
+    }
+
+    SECTION("marks rows as both inserted and deleted") {
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3, 4}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 1);
+        REQUIRE_INDICES(c.insertions, 2);
+        REQUIRE(c.moves.empty());
+    }
+
+    SECTION("marks rows as modified even if they moved") {
+        c = _impl::CollectionChangeBuilder::calculate({3, 5}, {5, 3}, all_modified, false);
+        REQUIRE_INDICES(c.deletions, 1);
+        REQUIRE_INDICES(c.insertions, 0);
+        REQUIRE_INDICES(c.modifications, 0, 1);
+    }
+
+    SECTION("does not mark rows as modified if they are new") {
+        c = _impl::CollectionChangeBuilder::calculate({3}, {3, 5}, all_modified, false);
+        REQUIRE_INDICES(c.modifications, 0);
+    }
+
     SECTION("reports inserts/deletes for simple reorderings") {
         auto calc = [&](std::vector<size_t> old_rows, std::vector<size_t> new_rows) {
-            return _impl::CollectionChangeBuilder::calculate(old_rows, new_rows, none_modified, true);
+            return _impl::CollectionChangeBuilder::calculate(old_rows, new_rows, none_modified, false);
         };
 
         c = calc({1, 2, 3}, {1, 2, 3});
@@ -652,19 +652,19 @@ TEST_CASE("[collection_change] calculate() sorted") {
 
     SECTION("prefers to produce diffs where modified rows are the ones to move when it is ambiguous") {
         auto two_modified = [](size_t ndx) { return ndx == 2; };
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3, 2}, two_modified, true);
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3, 2}, two_modified, false);
         REQUIRE_INDICES(c.deletions, 1);
         REQUIRE_INDICES(c.insertions, 2);
 
         auto three_modified = [](size_t ndx) { return ndx == 3; };
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3, 2}, three_modified, true);
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {1, 3, 2}, three_modified, false);
         REQUIRE_INDICES(c.deletions, 2);
         REQUIRE_INDICES(c.insertions, 1);
     }
 
     SECTION("prefers smaller diffs over larger diffs moving only modified rows") {
         auto two_modified = [](size_t ndx) { return ndx == 2; };
-        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {2, 3, 1}, two_modified, true);
+        c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, {2, 3, 1}, two_modified, false);
         REQUIRE_INDICES(c.deletions, 0);
         REQUIRE_INDICES(c.insertions, 2);
     }
@@ -672,7 +672,7 @@ TEST_CASE("[collection_change] calculate() sorted") {
     SECTION("supports duplicate indices") {
         c = _impl::CollectionChangeBuilder::calculate({1, 1, 2, 2, 3, 3},
                                                       {1, 2, 3, 1, 2, 3},
-                                                      all_modified, true);
+                                                      all_modified, false);
         REQUIRE_INDICES(c.deletions, 3, 5);
         REQUIRE_INDICES(c.insertions, 1, 2);
     }
@@ -680,7 +680,7 @@ TEST_CASE("[collection_change] calculate() sorted") {
     SECTION("deletes and inserts the last option when any in a range could be deleted") {
         c = _impl::CollectionChangeBuilder::calculate({3, 2, 1, 1, 2, 3},
                                                       {1, 1, 2, 2, 3, 3},
-                                                      all_modified, true);
+                                                      all_modified, false);
         REQUIRE_INDICES(c.deletions, 0, 1);
         REQUIRE_INDICES(c.insertions, 3, 5);
     }
@@ -688,13 +688,13 @@ TEST_CASE("[collection_change] calculate() sorted") {
     SECTION("reports insertions/deletions when the number of duplicate entries changes") {
         c = _impl::CollectionChangeBuilder::calculate({1, 1, 1, 1, 2, 3},
                                                       {1, 2, 3, 1},
-                                                      all_modified, true);
+                                                      all_modified, false);
         REQUIRE_INDICES(c.deletions, 1, 2, 3);
         REQUIRE_INDICES(c.insertions, 3);
 
         c = _impl::CollectionChangeBuilder::calculate({1, 2, 3, 1},
                                                       {1, 1, 1, 1, 2, 3},
-                                                      all_modified, true);
+                                                      all_modified, false);
         REQUIRE_INDICES(c.deletions, 3);
         REQUIRE_INDICES(c.insertions, 1, 2, 3);
     }
@@ -702,7 +702,7 @@ TEST_CASE("[collection_change] calculate() sorted") {
     SECTION("properly recurses into smaller subblocks") {
         std::vector<size_t> prev = {10, 1, 2, 11, 3, 4, 5, 12, 6, 7, 13};
         std::vector<size_t> next = {13, 1, 2, 12, 3, 4, 5, 11, 6, 7, 10};
-        c = _impl::CollectionChangeBuilder::calculate(prev, next, all_modified, true);
+        c = _impl::CollectionChangeBuilder::calculate(prev, next, all_modified, false);
         REQUIRE_INDICES(c.deletions, 0, 3, 7, 10);
         REQUIRE_INDICES(c.insertions, 0, 3, 7, 10);
     }
@@ -718,13 +718,13 @@ TEST_CASE("[collection_change] calculate() sorted") {
 
                 std::vector<size_t> after_insert = {1, 2, 3};
                 after_insert.insert(after_insert.begin() + insert_pos, 4);
-                c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, after_insert, four_modified, true);
+                c = _impl::CollectionChangeBuilder::calculate({1, 2, 3}, after_insert, four_modified, false);
 
                 std::vector<size_t> after_move = {1, 2, 3};
                 after_move.insert(after_move.begin() + move_to_pos, 4);
-                c.merge(_impl::CollectionChangeBuilder::calculate(after_insert, after_move, four_modified, true));
+                c.merge(_impl::CollectionChangeBuilder::calculate(after_insert, after_move, four_modified, false));
 
-                c.merge(_impl::CollectionChangeBuilder::calculate(after_move, {1, 2, 3}, four_modified, true));
+                c.merge(_impl::CollectionChangeBuilder::calculate(after_move, {1, 2, 3}, four_modified, false));
                 REQUIRE(c.empty());
             }
         }