diff --git a/CHANGELOG.md b/CHANGELOG.md index 8102faa28d3..4260fc872e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,10 @@ * Fixed a segfault in sync compiled by MSVC 2019. ([#4624](https://github.com/realm/realm-core/issues/4624), since v10.4.0) * Removing a change callback from a Results would sometimes block the calling thread while the query for that Results was running on the background worker thread (since v11.1.0). * Object observers did not handle the object being deleted properly, which could result in assertion failures mentioning "m_table" in ObjectNotifier ([#4824](https://github.com/realm/realm-core/issues/4824), since v11.1.0). +* Fixed a crash when delivering notifications over a nested hierarchy of lists of Mixed that contain links. ([#4803](https://github.com/realm/realm-core/issues/4803), since v11.0.0) +* Fixed key path filtered notifications throwing on null links and asserting on unresolved links. ([#4828](https://github.com/realm/realm-core/pull/4828), since v11.1.0) +* Fixed a crash when an object which is linked to by a Mixed is invalidated (sync only). ([#4828](https://github.com/realm/realm-core/pull/4828), since v11.0.0) +* Fixed a rare crash when setting a mixed link for the first time which would trigger if the link was to the same table and adding the backlink column caused a BPNode split. ([#4828](https://github.com/realm/realm-core/pull/4828), since v11.0.0) * Accessing an invalidated dictionary will throw a confusing error message ([#4805](https://github.com/realm/realm-core/issues/4805), since v11.0.0). ### Breaking changes diff --git a/src/realm/cluster.cpp b/src/realm/cluster.cpp index 488f3c3b5c5..99abf079e7a 100644 --- a/src/realm/cluster.cpp +++ b/src/realm/cluster.cpp @@ -279,10 +279,10 @@ inline void Cluster::do_insert_link(size_t ndx, ColKey col_key, Mixed init_val, // Insert backlink if link is not null if (target_link) { Table* origin_table = const_cast(m_tree_top.get_owning_table()); - Obj target_obj = origin_table->get_parent_group()->get_object(target_link); - auto target_table = target_obj.get_table(); + auto target_table = origin_table->get_parent_group()->get_table(target_link.get_table_key()); + ColKey backlink_col_key = target_table->find_or_add_backlink_column(col_key, origin_table->get_key()); - target_obj.add_backlink(backlink_col_key, origin_key); + target_table->get_object(target_link.get_obj_key()).add_backlink(backlink_col_key, origin_key); } } diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 06cd64a09f2..21706c745ab 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1931,15 +1931,22 @@ void Obj::nullify_link(ColKey origin_col_key, ObjLink target_link) void Obj::set_backlink(ColKey col_key, ObjLink new_link) const { if (new_link && new_link.get_obj_key()) { - auto target_obj = m_table->get_parent_group()->get_object(new_link); + auto target_table = m_table->get_parent_group()->get_table(new_link.get_table_key()); ColKey backlink_col_key; auto type = col_key.get_type(); if (type == col_type_TypedLink || type == col_type_Mixed || col_key.is_dictionary()) { - backlink_col_key = target_obj.get_table()->find_or_add_backlink_column(col_key, get_table_key()); + // This may modify the target table + backlink_col_key = target_table->find_or_add_backlink_column(col_key, get_table_key()); + // it is possible that this was a link to the same table and that adding a backlink column has + // caused the need to update this object as well. + update_if_needed(); } else { backlink_col_key = m_table->get_opposite_column(col_key); } + auto obj_key = new_link.get_obj_key(); + auto target_obj = + obj_key.is_unresolved() ? target_table->get_tombstone(obj_key) : target_table->get_object(obj_key); target_obj.add_backlink(backlink_col_key, m_key); } } @@ -2145,11 +2152,6 @@ void Obj::assign_pk_and_backlinks(const Obj& other) set.insert(ObjLink{m_table->get_key(), get_key()}); } } - else if (c.get_type() == col_type_Link) { - // Single link - REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); - linking_obj.set(c, get_key()); - } else if (c.is_list()) { if (c.get_type() == col_type_Mixed) { auto l = linking_obj.get_list(c); @@ -2169,7 +2171,21 @@ void Obj::assign_pk_and_backlinks(const Obj& other) } } else { - REALM_UNREACHABLE(); // missing type handling + REALM_ASSERT(!c.is_collection()); + if (c.get_type() == col_type_Link) { + // Single link + REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); + linking_obj.set(c, get_key()); + } + else if (c.get_type() == col_type_Mixed) { + // Mixed link + REALM_ASSERT(linking_obj.get_any(c).is_null() || + linking_obj.get_any(c).get_link().get_obj_key() == other.get_key()); + linking_obj.set(c, Mixed{ObjLink{m_table->get_key(), get_key()}}); + } + else { + REALM_UNREACHABLE(); // missing type handling + } } } return false; diff --git a/src/realm/object-store/impl/deep_change_checker.cpp b/src/realm/object-store/impl/deep_change_checker.cpp index 4aa4e004236..9df65cb488c 100644 --- a/src/realm/object-store/impl/deep_change_checker.cpp +++ b/src/realm/object-store/impl/deep_change_checker.cpp @@ -275,11 +275,18 @@ bool DeepChangeChecker::check_outgoing_links(Table const& table, ObjKey obj_key, if (outgoing_link_column.is_collection()) { return do_check_for_collection_modifications(obj, outgoing_link_column, filtered_columns, depth); } - + if (outgoing_link_column.get_type() == col_type_Mixed) { + TableRef no_cached; + Mixed value = obj.get(outgoing_link_column); + return do_check_mixed_for_link(*table.get_parent_group(), no_cached, value, filtered_columns, depth); + } + REALM_ASSERT_EX(outgoing_link_column.get_type() == col_type_Link, outgoing_link_column.get_type()); + ConstTableRef dst_table = table.get_link_target(outgoing_link_column); ObjKey dst_key = obj.get(outgoing_link_column); + if (!dst_key) // do not descend into a null or unresolved link return false; - return check_row(*table.get_link_target(outgoing_link_column), dst_key.value, filtered_columns, depth + 1); + return check_row(*dst_table, dst_key.value, filtered_columns, depth + 1); }; // Check the `links` of all `m_related_tables` and return true if any of them has a `linked_object_changed`. @@ -289,6 +296,8 @@ bool DeepChangeChecker::check_outgoing_links(Table const& table, ObjKey obj_key, bool DeepChangeChecker::check_row(Table const& table, ObjKeyType object_key, const std::vector& filtered_columns, size_t depth) { + REALM_ASSERT(!ObjKey(object_key).is_unresolved()); + TableKey table_key = table.get_key(); // First check if the object was modified directly. We skip this if we're @@ -335,6 +344,12 @@ bool DeepChangeChecker::operator()(ObjKeyType key) return true; } + // In production code it shouldn't be possible for a notifier to call this on + // an invalidated object, but we do have tests for it just in case. + if (ObjKey(key).is_unresolved()) { + return false; + } + // The object itself wasn't modified, so move on to check if any of the // objects it links to were modified. return check_row(m_root_table, key, m_filtered_columns, 0); @@ -353,6 +368,12 @@ bool CollectionKeyPathChangeChecker::operator()(ObjKeyType object_key) { std::vector changed_columns; + // In production code it shouldn't be possible for a notifier to call this on + // an invalidated object, but we do have tests for it just in case. + if (ObjKey(object_key).is_unresolved()) { + return false; + } + for (auto& key_path : m_key_path_array) { find_changed_columns(changed_columns, key_path, 0, m_root_table, object_key); } @@ -364,6 +385,7 @@ void CollectionKeyPathChangeChecker::find_changed_columns(std::vector& const KeyPath& key_path, size_t depth, const Table& table, const ObjKeyType& object_key_value) { + REALM_ASSERT(!ObjKey(object_key_value).is_unresolved()); if (depth >= key_path.size()) { // We've reached the end of the key path. @@ -409,6 +431,9 @@ void CollectionKeyPathChangeChecker::find_changed_columns(std::vector& auto check_mixed_object = [&](const Mixed& mixed_object) { if (mixed_object.is_type(type_Link, type_TypedLink)) { auto object_key = mixed_object.get(); + if (object_key.is_unresolved()) { + return; + } auto target_table_key = mixed_object.get_link().get_table_key(); Group* group = table.get_parent_group(); auto target_table = group->get_table(target_table_key); @@ -439,43 +464,35 @@ void CollectionKeyPathChangeChecker::find_changed_columns(std::vector& else if (column_key.is_set()) { if (column_type == col_type_Mixed) { auto set = object.get_set(column_key); - for (size_t i = 0; i < set.size(); i++) { - auto target_object = set.get(i); - check_mixed_object(target_object); + for (auto& mixed_val : set) { + check_mixed_object(mixed_val); } } else { REALM_ASSERT(column_type == col_type_Link || column_type == col_type_LinkList); auto set = object.get_linkset(column_key); auto target_table = table.get_link_target(column_key); - for (size_t i = 0; i < set.size(); i++) { - auto target_object = set.get(i); + for (auto& target_object : set) { find_changed_columns(changed_columns, key_path, depth + 1, *target_table, target_object.value); } } } else if (column_key.is_dictionary()) { - if (column_type == col_type_Mixed) { - auto dictionary = object.get_dictionary(column_key); - for (size_t i = 0; i < dictionary.size(); i++) { - auto target_object = dictionary.get(dictionary.get_key(i)); - check_mixed_object(target_object); - } - } - else { - REALM_ASSERT(column_type == col_type_Link || column_type == col_type_LinkList); - auto dictionary = object.get_dictionary(column_key); - auto linked_dictionary = std::make_unique(dictionary); - auto target_table = table.get_link_target(column_key); - for (size_t i = 0; i < linked_dictionary->size(); i++) { - auto target_object = linked_dictionary->get_key(i); - find_changed_columns(changed_columns, key_path, depth + 1, *target_table, target_object.value); - } - } + // a dictionary always stores mixed values + auto dictionary = object.get_dictionary(column_key); + dictionary.for_all_values([&](Mixed val) { + check_mixed_object(val); + }); + } + else if (column_type == col_type_Mixed) { + check_mixed_object(object.get_any(column_key)); } else if (column_type == col_type_Link) { // A forward link will only have one target object. auto target_object = object.get(column_key); + if (!target_object || target_object.is_unresolved()) { + return; + } auto target_table = table.get_link_target(column_key); find_changed_columns(changed_columns, key_path, depth + 1, *target_table, target_object.value); } diff --git a/test/object-store/transaction_log_parsing.cpp b/test/object-store/transaction_log_parsing.cpp index dda208f1782..d9f2da5ce57 100644 --- a/test/object-store/transaction_log_parsing.cpp +++ b/test/object-store/transaction_log_parsing.cpp @@ -1744,7 +1744,7 @@ struct DictionaryOfMixedLinks { constexpr static bool allows_storing_nulls = true; }; -TEMPLATE_TEST_CASE("DeepChangeChecker", "[notifications]", ListOfObjects, ListOfMixedLinks, SetOfObjects, +TEMPLATE_TEST_CASE("DeepChangeChecker collections", "[notifications]", ListOfObjects, ListOfMixedLinks, SetOfObjects, SetOfMixedLinks, DictionaryOfObjects, DictionaryOfMixedLinks) { TestType test_type; @@ -1802,129 +1802,6 @@ TEMPLATE_TEST_CASE("DeepChangeChecker", "[notifications]", ListOfObjects, ListOf test_type.set_relation_updater(relation_updater); auto cols = table->get_column_keys(); - SECTION("direct changes are tracked") { - auto info = track_changes([&] { - table->get_object(9).set(cols[0], 10); - }); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(8)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(8)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(8)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(8)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(8)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(8)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(8)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(9)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(8)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(9)); - } - - SECTION("changes over links are tracked") { - bool did_run_section = false; - SECTION("first link set") { - did_run_section = true; - r->begin_transaction(); - objects[0].set(cols[1], objects[1].get_key()); - objects[1].set(cols[1], objects[2].get_key()); - objects[2].set(cols[1], objects[4].get_key()); - r->commit_transaction(); - } - SECTION("second link set") { - did_run_section = true; - r->begin_transaction(); - objects[0].set(cols[2], objects[1].get_key()); - objects[1].set(cols[2], objects[2].get_key()); - objects[2].set(cols[2], objects[4].get_key()); - r->commit_transaction(); - } - SECTION("both set") { - did_run_section = true; - r->begin_transaction(); - objects[0].set(cols[1], objects[1].get_key()); - objects[1].set(cols[1], objects[2].get_key()); - objects[2].set(cols[1], objects[4].get_key()); - - objects[0].set(cols[2], objects[1].get_key()); - objects[1].set(cols[2], objects[2].get_key()); - objects[2].set(cols[2], objects[4].get_key()); - r->commit_transaction(); - } - SECTION("circular link") { - did_run_section = true; - r->begin_transaction(); - objects[0].set(cols[1], objects[0].get_key()); - objects[1].set(cols[1], objects[1].get_key()); - objects[2].set(cols[1], objects[2].get_key()); - objects[3].set(cols[1], objects[3].get_key()); - objects[4].set(cols[1], objects[4].get_key()); - - objects[0].set(cols[2], objects[1].get_key()); - objects[1].set(cols[2], objects[2].get_key()); - objects[2].set(cols[2], objects[4].get_key()); - r->commit_transaction(); - } - - catch2_ensure_section_run_workaround(did_run_section, "changes over links are tracked", [&]() { - auto info = track_changes([&] { - objects[4].set(cols[0], 10); - }); - - // link chain should cascade to all but #3 being marked as modified - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(1)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(3)); - - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(1)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(3)); - - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(1)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(3)); - - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(1)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(3)); - - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(1)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(3)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(1)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(3)); - - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(1)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(3)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(1)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(2)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(3)); - }); - } - SECTION("changes over collections are tracked") { r->begin_transaction(); for (int i = 0; i < 3; ++i) { @@ -2033,74 +1910,6 @@ TEMPLATE_TEST_CASE("DeepChangeChecker", "[notifications]", ListOfObjects, ListOf REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(3)); } - SECTION("changes from an object with an unresolved link") { - r->begin_transaction(); - size_t obj_ndx_to_invalidate = 6; - ColKey col_link1 = cols[1]; - objects[0].set(col_link1, objects[obj_ndx_to_invalidate].get_key()); - ObjKey link = objects[0].get(col_link1); - REQUIRE(!link.is_unresolved()); - REQUIRE(link); - - // Object invalidation can only happen if another sync client has deleted the object - // we simulate this by calling it directly here. The consequence is that links to this - // object are changed to the invalidated key, but not removed. This tests that the - // change checker doesn't try to descend down invalidated link paths. - objects[obj_ndx_to_invalidate].invalidate(); - - // the link is actually unresolved, but that is hidden by a null at this level of abstraction - link = objects[0].get(col_link1); - REQUIRE(!link.is_unresolved()); - REQUIRE(!link); - r->commit_transaction(); - - auto info = track_changes([&] { - objects[1].set(cols[0], 10); - }); - // if the change checker iterates over an invalid link, it'll throw an exception - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(0)); - REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(1)); - - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(1)); - } - - SECTION("cycles over links do not loop forever") { - r->begin_transaction(); - objects[0].set(cols[1], objects[0].get_key()); - r->commit_transaction(); - - auto info = track_changes([&] { - objects[9].set(cols[0], 10); - }); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, false)(0)); - REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(0)); - } - SECTION("cycles over collections do not loop forever") { r->begin_transaction(); test_type.add_link(objects[0], cols[3], {dst_table_key, objects[0].get_key()}); @@ -2119,70 +1928,7 @@ TEMPLATE_TEST_CASE("DeepChangeChecker", "[notifications]", ListOfObjects, ListOf REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(0)); } - SECTION("link chains are tracked") { - r->begin_transaction(); - for (int i = 0; i < 10; ++i) - objects.push_back(table->create_object()); - for (int i = 0; i < 19; ++i) - objects[i].set(cols[1], objects[i + 1].get_key()); - r->commit_transaction(); - - auto info = track_changes([&] { - objects[19].set(cols[0], -1); - }); - - SECTION("without filter - up to 4 levels deep") { - _impl::DeepChangeChecker checker(info, *table, related_tables, key_path_array_empty, false); - CHECK(checker(19)); - CHECK(checker(18)); - CHECK(checker(16)); - CHECK_FALSE(checker(15)); - - // Check in other orders to make sure that the caching doesn't effect - // the results - _impl::DeepChangeChecker checker2(info, *table, related_tables, key_path_array_empty, false); - CHECK_FALSE(checker2(15)); - CHECK(checker2(16)); - CHECK(checker2(18)); - CHECK(checker2(19)); - - _impl::DeepChangeChecker checker3(info, *table, related_tables, key_path_array_empty, false); - CHECK(checker3(16)); - CHECK_FALSE(checker3(15)); - CHECK(checker3(18)); - CHECK(checker3(19)); - } - - SECTION("with filter - more than 4 levels deep") { - KeyPath key_path_five_levels = {pair_link, pair_link, pair_link, pair_link, pair_int}; - KeyPathArray key_path_array_five_levels = {key_path_five_levels}; - - _impl::CollectionKeyPathChangeChecker checker(info, *table, related_tables, key_path_array_five_levels, - false); - CHECK_THROWS(checker(19)); - CHECK_THROWS(checker(18)); - CHECK_FALSE(checker(16)); - CHECK(checker(15)); - - // Check in other orders to make sure that the caching doesn't effect - // the results - _impl::CollectionKeyPathChangeChecker checker2(info, *table, related_tables, key_path_array_five_levels, - false); - CHECK(checker2(15)); - CHECK_FALSE(checker2(16)); - CHECK_THROWS(checker2(18)); - CHECK_THROWS(checker2(19)); - - _impl::CollectionKeyPathChangeChecker checker3(info, *table, related_tables, key_path_array_five_levels, - false); - CHECK_FALSE(checker3(16)); - CHECK(checker3(15)); - CHECK_THROWS(checker3(18)); - CHECK_THROWS(checker3(19)); - } - } - - SECTION("changes made in the 3rd elements in the link list") { + SECTION("changes made in the 3rd elements in the collection") { r->begin_transaction(); test_type.add_link(objects[0], cols[3], {dst_table_key, objects[1].get_key()}); test_type.add_link(objects[0], cols[3], {dst_table_key, objects[2].get_key()}); @@ -2228,7 +1974,7 @@ TEMPLATE_TEST_CASE("DeepChangeChecker", "[notifications]", ListOfObjects, ListOf REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(3)); } - SECTION("changes made to lists mark the containing row as modified") { + SECTION("changes made to collections mark the containing row as modified") { auto info = track_changes([&] { test_type.add_link(objects[0], cols[3], {dst_table_key, objects[1].get_key()}); }); @@ -2242,3 +1988,367 @@ TEMPLATE_TEST_CASE("DeepChangeChecker", "[notifications]", ListOfObjects, ListOf REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_test_type, true)(0)); } } + +TEST_CASE("DeepChangeChecker singular links", "[notifications]") { + InMemoryTestFile config; + config.automatic_change_notifications = false; + auto r = Realm::get_shared_realm(config); + r->update_schema({{ + "table", + { + {"int", PropertyType::Int}, + {"link1", PropertyType::Object | PropertyType::Nullable, "table"}, + {"link2", PropertyType::Object | PropertyType::Nullable, "table"}, + {"mixed_link", PropertyType::Mixed | PropertyType::Nullable}, + }, + }}); + auto table = r->read_group().get_table("class_table"); + TableKey dst_table_key = table->get_key(); + + std::vector objects; + r->begin_transaction(); + for (int i = 0; i < 10; ++i) + objects.push_back(table->create_object().set_all(i)); + r->commit_transaction(); + + auto track_changes = [&](auto&& f) { + auto history = make_in_realm_history(config.path); + auto db = DB::create(*history, config.options()); + auto rt = db->start_read(); + + r->begin_transaction(); + f(); + r->commit_transaction(); + + _impl::TransactionChangeInfo info{}; + for (auto key : rt->get_table_keys()) + info.tables[key.value]; + _impl::transaction::advance(*rt, info); + return info; + }; + + std::vector<_impl::DeepChangeChecker::RelatedTable> related_tables; + std::pair pair_int(table->get_key(), table->get_column_key("int")); + std::pair pair_link(table->get_key(), table->get_column_key("link1")); + std::pair pair_mixed_link(table->get_key(), table->get_column_key("mixed_link")); + KeyPathArray key_path_array_int = {{pair_int}}; + KeyPathArray key_path_array_link = {{pair_link}}; + KeyPathArray key_path_mixed_link = {{pair_mixed_link}}; + KeyPathArray key_path_array_empty = {}; + auto relation_updater = [&]() { + related_tables.clear(); + _impl::DeepChangeChecker::find_related_tables(related_tables, *table, key_path_array_empty); + }; + relation_updater(); + + auto set_link = [&](Obj obj, ColKey col, ObjLink link) { + if (col.get_type() == col_type_Mixed) { + obj.set(col, Mixed{link}); + } + else { + obj.set(col, link.get_obj_key()); + } + }; + auto get_link = [&](Obj obj, ColKey col) -> ObjLink { + if (col.get_type() == col_type_Mixed) { + Mixed val = obj.get(col); + if (val.is_type(type_TypedLink)) { + return val.get_link(); + } + return {}; + } + REALM_ASSERT(col.get_type() == col_type_Link); + return ObjLink{dst_table_key, obj.get(col)}; + }; + + auto cols = table->get_column_keys(); + SECTION("direct changes are tracked") { + auto info = track_changes([&] { + table->get_object(9).set(cols[0], 10); + }); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(8)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(9)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(8)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(9)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(8)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(9)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(8)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(9)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(8)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(9)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(8)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(9)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(8)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(9)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(8)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(9)); + } + + SECTION("changes over links are tracked") { + bool did_run_section = false; + + { + size_t col_ndx = GENERATE(1, 2, 3); + ColKey link_column = cols[col_ndx]; + SECTION(util::format("one link set: %1", col_ndx)) { + did_run_section = true; + r->begin_transaction(); + set_link(objects[0], link_column, {dst_table_key, objects[1].get_key()}); + set_link(objects[1], link_column, {dst_table_key, objects[2].get_key()}); + set_link(objects[2], link_column, {dst_table_key, objects[4].get_key()}); + r->commit_transaction(); + REQUIRE(get_link(objects[0], link_column) == ObjLink{dst_table_key, objects[1].get_key()}); + REQUIRE(get_link(objects[1], link_column) == ObjLink{dst_table_key, objects[2].get_key()}); + REQUIRE(get_link(objects[2], link_column) == ObjLink{dst_table_key, objects[4].get_key()}); + } + ColKey next_link_col = cols[1 + ((col_ndx - 1) % 3)]; + SECTION(util::format("two links set: %1", col_ndx)) { + did_run_section = true; + r->begin_transaction(); + set_link(objects[0], link_column, {dst_table_key, objects[1].get_key()}); + set_link(objects[1], link_column, {dst_table_key, objects[2].get_key()}); + set_link(objects[2], link_column, {dst_table_key, objects[4].get_key()}); + + set_link(objects[0], next_link_col, {dst_table_key, objects[1].get_key()}); + set_link(objects[1], next_link_col, {dst_table_key, objects[2].get_key()}); + set_link(objects[2], next_link_col, {dst_table_key, objects[4].get_key()}); + r->commit_transaction(); + } + SECTION(util::format("circular link: %1", col_ndx)) { + did_run_section = true; + r->begin_transaction(); + set_link(objects[0], link_column, {dst_table_key, objects[0].get_key()}); + set_link(objects[1], link_column, {dst_table_key, objects[1].get_key()}); + set_link(objects[2], link_column, {dst_table_key, objects[2].get_key()}); + set_link(objects[3], link_column, {dst_table_key, objects[3].get_key()}); + set_link(objects[4], link_column, {dst_table_key, objects[4].get_key()}); + + set_link(objects[0], next_link_col, {dst_table_key, objects[1].get_key()}); + set_link(objects[1], next_link_col, {dst_table_key, objects[2].get_key()}); + set_link(objects[2], next_link_col, {dst_table_key, objects[4].get_key()}); + r->commit_transaction(); + } + } + SECTION("three links set") { + did_run_section = true; + r->begin_transaction(); + objects[0].set(cols[1], objects[1].get_key()); + objects[1].set(cols[1], objects[2].get_key()); + objects[2].set(cols[1], objects[4].get_key()); + + objects[0].set(cols[2], objects[1].get_key()); + objects[1].set(cols[2], objects[2].get_key()); + objects[2].set(cols[2], objects[4].get_key()); + + objects[0].set(cols[3], Mixed{ObjLink(dst_table_key, objects[1].get_key())}); + objects[1].set(cols[3], Mixed{ObjLink(dst_table_key, objects[2].get_key())}); + objects[2].set(cols[3], Mixed{ObjLink(dst_table_key, objects[4].get_key())}); + r->commit_transaction(); + } + relation_updater(); + + catch2_ensure_section_run_workaround(did_run_section, "changes over links are tracked", [&]() { + auto info = track_changes([&] { + objects[4].set(cols[0], 10); + }); + + // link chain should cascade to all but #3 being marked as modified + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(1)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(3)); + + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(1)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(3)); + + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(1)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(3)); + + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(1)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(3)); + + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(1)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(3)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(1)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(3)); + + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(1)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(3)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(1)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(2)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(3)); + }); + } + + SECTION("for different link types") { + ColKey link_column = GENERATE_REF(cols[1], cols[3]); + std::string column_name = table->get_column_name(link_column); + SECTION("cycles over links do not loop forever for column: " + column_name) { + r->begin_transaction(); + set_link(objects[0], link_column, ObjLink{dst_table_key, objects[0].get_key()}); + r->commit_transaction(); + + auto info = track_changes([&] { + objects[9].set(cols[0], 10); + }); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(0)); + } + + SECTION("changes from an object with an unresolved link for column: " + column_name) { + r->begin_transaction(); + size_t obj_ndx_to_invalidate = 6; + set_link(objects[0], link_column, {dst_table_key, objects[obj_ndx_to_invalidate].get_key()}); + Mixed link = objects[0].get_any(link_column); + REQUIRE(!link.is_null()); + REQUIRE(!link.is_unresolved_link()); + + // Object invalidation can only happen if another sync client has deleted the object + // we simulate this by calling it directly here. The consequence is that links to this + // object are changed to the invalidated key, but not removed. This tests that the + // change checker doesn't try to descend down invalidated link paths. + objects[obj_ndx_to_invalidate].invalidate(); + + // At this point, the link is unresolved. A simple link will hide this + // by faking a null there instead, but Mixed will report the unresolved link. + link = objects[0].get_any(link_column); + REQUIRE((link.is_null() || link.is_unresolved_link())); + r->commit_transaction(); + + auto info = track_changes([&] { + objects[1].set(cols[0], 10); + }); + // if the change checker iterates over an invalid link, it'll throw an exception + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, false)(1)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_empty, true)(1)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, false)(1)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_int, true)(1)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, false)(1)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_array_link, true)(1)); + + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(0)); + REQUIRE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, false)(1)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(0)); + REQUIRE_FALSE(_impl::DeepChangeChecker(info, *table, related_tables, key_path_mixed_link, true)(1)); + } + + SECTION("link chains are tracked for column: " + column_name) { + auto verify_changes_for = [&](auto& checker, std::function pred) { + // Check in random orders to make sure that the caching doesn't affect + // the results + std::vector indices(20); + std::iota(indices.begin(), indices.end(), 0); + std::random_device rd; + std::mt19937 g(rd()); + std::shuffle(indices.begin(), indices.end(), g); + for (auto ndx : indices) { + if (pred(ndx)) { + CHECK(checker(objects[ndx].get_key().value)); + } + else { + CHECK_FALSE(checker(objects[ndx].get_key().value)); + } + } + }; + r->begin_transaction(); + table->clear(); + objects.clear(); + for (int i = 0; i < 20; ++i) + objects.push_back(table->create_object().set_all(i)); + for (int i = 0; i < 19; ++i) + set_link(objects[i], link_column, ObjLink{dst_table_key, objects[i + 1].get_key()}); + r->commit_transaction(); + relation_updater(); + + SECTION("with valid links in a chain") { + auto info = track_changes([&] { + objects[19].set(cols[0], -1); + }); + + SECTION("without filter - up to 4 levels deep") { + auto obj_indexes_with_changes = [](size_t ndx) { + return ndx >= 16; + }; + _impl::DeepChangeChecker checker(info, *table, related_tables, key_path_array_empty, false); + verify_changes_for(checker, obj_indexes_with_changes); + } + + SECTION("with filter - more than 4 levels deep") { + auto obj_indexes_with_changes = [](size_t ndx) { + return ndx == 15; + }; + std::pair pair_link_under_test(table->get_key(), link_column); + KeyPath key_path_five_levels = {pair_link_under_test, pair_link_under_test, pair_link_under_test, + pair_link_under_test, pair_int}; + KeyPathArray key_path_array_five_levels = {key_path_five_levels}; + _impl::CollectionKeyPathChangeChecker checker(info, *table, related_tables, + key_path_array_five_levels, false); + verify_changes_for(checker, obj_indexes_with_changes); + } + } + SECTION("with invalidated link in chain") { + r->begin_transaction(); + objects[17].invalidate(); + r->commit_transaction(); + + auto info = track_changes([&] { + objects[19].set(cols[0], -1); + }); + + SECTION("without filter - post invalid object only") { + auto obj_indexes_with_changes = [](size_t ndx) { + return ndx >= 18; + }; + _impl::DeepChangeChecker checker(info, *table, related_tables, key_path_array_empty, false); + verify_changes_for(checker, obj_indexes_with_changes); + } + + SECTION("with filter - none along complete path") { + auto obj_indexes_with_changes = [](size_t) { + return false; + }; + std::pair pair_link_under_test(table->get_key(), link_column); + KeyPath key_path_five_levels = {pair_link_under_test, pair_link_under_test, pair_link_under_test, + pair_link_under_test, pair_int}; + KeyPathArray key_path_array_five_levels = {key_path_five_levels}; + _impl::CollectionKeyPathChangeChecker checker(info, *table, related_tables, + key_path_array_five_levels, false); + verify_changes_for(checker, obj_indexes_with_changes); + } + } + } + } +}