diff --git a/CHANGELOG.md b/CHANGELOG.md index f017326d6d6..6f06f197ba1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,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. diff --git a/src/realm/object-store/impl/results_notifier.cpp b/src/realm/object-store/impl/results_notifier.cpp index 08c57e45267..cf0843c7d92 100644 --- a/src/realm/object-store/impl/results_notifier.cpp +++ b/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(key)); + return m_change.modifications.contains(static_cast(key)); }); } diff --git a/test/object-store/primitive_list.cpp b/test/object-store/primitive_list.cpp index 31a79531e1c..a7a20a54811 100644 --- a/test/object-store/primitive_list.cpp +++ b/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(0) == static_cast(values[1])); + + size_t sorted_ndx_pre_modification = sorted.index_of(static_cast(values[1])); + r->begin_transaction(); + list.set(0, static_cast(values[0])); + r->commit_transaction(); + advance_and_notify(*r); + REQUIRE(calls == 9); + size_t sorted_ndx_post_modification = sorted.index_of(static_cast(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(static_cast(values[1])); + size_t sorted_ndx_pre_delete = sorted.index_of(static_cast(values[0])); + r->begin_transaction(); + list.remove(0); // remove values[0] + REQUIRE(list.size() > 0); + REQUIRE(list.get(0) == static_cast(values[1])); + list.set(0, static_cast(values[0])); + r->commit_transaction(); + advance_and_notify(*r); + REQUIRE(calls == 8); + size_t sorted_ndx_post_modification = sorted.index_of(static_cast(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);