fix collection notifications reporting modifications when sort/distin…

…ct has been applied (#4573)

* fix collection notifications with only modifications

* updated tests for delete and modify

CHANGELOG.md

@@ -15,6 +15,7 @@
 * Potential/unconfirmed fix for crashes associated with failure to memory map (low on memory, low on virtual address space). For example ([#4514](https://github.com/realm/realm-core/issues/4514)).
 * Fixed name aliasing not working in sort/distinct clauses of the query parser. ([#4550](https://github.com/realm/realm-core/issues/4550), never before working).
 * Fix assertion failures such as "!m_notifier_skip_version.version" or "m_notifier_sg->get_version() + 1 == new_version.version" when performing writes inside change notification callbacks. Previously refreshing the Realm by beginning a write transaction would skip delivering notifications, leaving things in an inconsistent state. Notifications are now delivered recursively when needed instead. ([Cocoa #7165](https://github.com/realm/realm-cocoa/issues/7165)).
+* Fix collection notification reporting for modifications. This could be observed by receiving the wrong indices of modifications on sorted or distinct results, or notification blocks sometimes not being called when only modifications have occured. ([#4573](https://github.com/realm/realm-core/pull/4573) since v6).
 
 ### Breaking changes
 * None.

src/realm/object-store/impl/results_notifier.cpp

@@ -310,7 +310,7 @@ void ListResultsNotifier::calculate_changes()
         }
 
         m_change = CollectionChangeBuilder::calculate(m_previous_indices, *m_run_indices, [=](int64_t key) {
-            return m_change.modifications_new.contains(static_cast<size_t>(key));
+            return m_change.modifications.contains(static_cast<size_t>(key));
         });
     }
 

test/object-store/primitive_list.cpp

@@ -990,6 +990,101 @@ TEMPLATE_TEST_CASE("primitive list", "[primitives]", ::Int, ::Bool, ::Float, ::D
             REQUIRE_INDICES(srchange.deletions, TestType::is_optional);
         }
 
+        SECTION("modify value in place") {
+            REQUIRE(calls == 0);
+            advance_and_notify(*r);
+            REQUIRE(calls == 3);
+            // Remove the existing copy of this value so that the sorted list
+            // doesn't have dupes resulting in an unstable order
+            r->begin_transaction();
+            list.remove(0);
+            r->commit_transaction();
+            advance_and_notify(*r);
+            REQUIRE(calls == 6);
+
+            REQUIRE(list.size() > 0);
+            REQUIRE(list.get<T>(0) == static_cast<T>(values[1]));
+
+            size_t sorted_ndx_pre_modification = sorted.index_of<T>(static_cast<T>(values[1]));
+            r->begin_transaction();
+            list.set(0, static_cast<T>(values[0]));
+            r->commit_transaction();
+            advance_and_notify(*r);
+            REQUIRE(calls == 9);
+            size_t sorted_ndx_post_modification = sorted.index_of<T>(static_cast<T>(values[0]));
+
+            REQUIRE_INDICES(change.insertions);
+            REQUIRE_INDICES(change.deletions);
+            REQUIRE_INDICES(change.modifications, 0);
+            REQUIRE_INDICES(change.modifications_new, 0);
+            REQUIRE_INDICES(rchange.insertions);
+            REQUIRE_INDICES(rchange.deletions);
+            REQUIRE_INDICES(rchange.modifications, 0);
+            REQUIRE_INDICES(rchange.modifications_new, 0);
+            if (sorted_ndx_pre_modification == sorted_ndx_post_modification) {
+                REQUIRE_INDICES(srchange.insertions);
+                REQUIRE_INDICES(srchange.deletions);
+                REQUIRE_INDICES(srchange.modifications, sorted_ndx_post_modification);
+                REQUIRE_INDICES(srchange.modifications_new, sorted_ndx_post_modification);
+            }
+            else {
+                REQUIRE_INDICES(srchange.insertions, sorted_ndx_post_modification);
+                REQUIRE_INDICES(srchange.deletions, sorted_ndx_pre_modification);
+                REQUIRE_INDICES(srchange.modifications);
+                REQUIRE_INDICES(srchange.modifications_new);
+            }
+        }
+
+        SECTION("delete and modify") {
+            auto distinct = results.distinct({{"self"}});
+            CollectionChangeSet drchange;
+            auto drtoken = distinct.add_notification_callback([&](CollectionChangeSet c, std::exception_ptr) {
+                drchange = c;
+                ++calls;
+            });
+
+            REQUIRE(calls == 0);
+            advance_and_notify(*r);
+            REQUIRE(calls == 4);
+            size_t sorted_ndx_pre_modification = sorted.index_of<T>(static_cast<T>(values[1]));
+            size_t sorted_ndx_pre_delete = sorted.index_of<T>(static_cast<T>(values[0]));
+            r->begin_transaction();
+            list.remove(0); // remove values[0]
+            REQUIRE(list.size() > 0);
+            REQUIRE(list.get<T>(0) == static_cast<T>(values[1]));
+            list.set(0, static_cast<T>(values[0]));
+            r->commit_transaction();
+            advance_and_notify(*r);
+            REQUIRE(calls == 8);
+            size_t sorted_ndx_post_modification = sorted.index_of<T>(static_cast<T>(values[0]));
+
+            REQUIRE_INDICES(change.insertions);
+            REQUIRE_INDICES(change.deletions, 0);
+            REQUIRE_INDICES(change.modifications, 1);
+            REQUIRE_INDICES(change.modifications_new, 0);
+            REQUIRE_INDICES(rchange.insertions);
+            REQUIRE_INDICES(rchange.deletions, 0);
+            REQUIRE_INDICES(rchange.modifications, 1);
+            REQUIRE_INDICES(rchange.modifications_new, 0);
+            REQUIRE_INDICES(drchange.insertions);
+            REQUIRE_INDICES(drchange.deletions, 0);
+            REQUIRE_INDICES(drchange.modifications, 1);
+            REQUIRE_INDICES(drchange.modifications_new, 0);
+
+            if (sorted_ndx_pre_modification == sorted_ndx_post_modification) {
+                REQUIRE_INDICES(srchange.insertions);
+                REQUIRE_INDICES(srchange.deletions, sorted_ndx_pre_delete);
+                REQUIRE_INDICES(srchange.modifications, sorted_ndx_post_modification);
+                REQUIRE_INDICES(srchange.modifications_new, sorted_ndx_post_modification);
+            }
+            else {
+                REQUIRE_INDICES(srchange.insertions, sorted_ndx_post_modification);
+                REQUIRE_INDICES(srchange.deletions, sorted_ndx_pre_modification, sorted_ndx_pre_delete);
+                REQUIRE_INDICES(srchange.modifications);
+                REQUIRE_INDICES(srchange.modifications_new);
+            }
+        }
+
         SECTION("clear list") {
             advance_and_notify(*r);