From ab1e994e8698a5716c7a287cf17177bcf5ba5c52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Wed, 19 Jun 2024 11:06:10 +0200 Subject: [PATCH 1/5] Replicate clear instruction unconditionally for nested collections --- CHANGELOG.md | 2 +- src/realm/dictionary.cpp | 10 +++-- src/realm/list.cpp | 10 +++-- test/test_sync.cpp | 89 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6354652ec0..2f61614e1b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) -* None. +* You could get unexpected merge results when assigning to a nested collection ([#7809](https://github.com/realm/realm-core/issues/7809), since v14.0.0) ### Breaking changes * None. diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp index cf3fee23d24..147d0b95c25 100644 --- a/src/realm/dictionary.cpp +++ b/src/realm/dictionary.cpp @@ -823,10 +823,12 @@ size_t Dictionary::find_first(Mixed value) const void Dictionary::clear() { - if (size() > 0) { - if (Replication* repl = get_replication()) { - repl->dictionary_clear(*this); - } + auto sz = size(); + Replication* repl = Base::get_replication(); + if (repl && (sz > 0 || !m_col_key.is_collection() || m_level > 1)) { + repl->dictionary_clear(*this); + } + if (sz > 0) { CascadeState cascade_state(CascadeState::Mode::Strong); bool recurse = remove_backlinks(cascade_state); diff --git a/src/realm/list.cpp b/src/realm/list.cpp index d3f94bd895a..bdb38f0b622 100644 --- a/src/realm/list.cpp +++ b/src/realm/list.cpp @@ -503,10 +503,12 @@ void Lst::remove(size_t from, size_t to) void Lst::clear() { - if (size() > 0) { - if (Replication* repl = Base::get_replication()) { - repl->list_clear(*this); - } + auto sz = size(); + Replication* repl = Base::get_replication(); + if (repl && (sz > 0 || !m_col_key.is_collection() || m_level > 1)) { + repl->list_clear(*this); + } + if (sz > 0) { CascadeState state; bool recurse = remove_backlinks(state); diff --git a/test/test_sync.cpp b/test/test_sync.cpp index 746364246f9..708b9d2f61c 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -6204,6 +6204,95 @@ TEST(Sync_DeleteCollectionInCollection) } } +TEST(Sync_NestedCollectionClear) +{ + TEST_CLIENT_DB(db_1); + TEST_CLIENT_DB(db_2); + + TEST_DIR(dir); + fixtures::ClientServerFixture fixture{dir, test_context}; + fixture.start(); + + Session session_1 = fixture.make_session(db_1, "/test"); + Session session_2 = fixture.make_session(db_2, "/test"); + + Timestamp now{std::chrono::system_clock::now()}; + + auto tr_1 = db_1->start_write(); + auto tr_2 = db_2->start_read(); + auto table_1 = tr_1->add_table_with_primary_key("class_Table", type_Int, "id"); + auto col = table_1->add_column(type_Mixed, "any"); + table_1->add_column_list(type_Mixed, "ints"); + auto col_list = table_1->add_column_list(type_Mixed, "any_list"); + + auto foo = table_1->create_object_with_primary_key(123); + foo.set_collection(col, CollectionType::List); + foo.get_list(col_list).insert_collection(0, CollectionType::List); + tr_1->commit_and_continue_as_read(); + + session_1.wait_for_upload_complete_or_client_stopped(); + session_2.wait_for_download_complete_or_client_stopped(); + + { + tr_1->promote_to_write(); + auto list = foo.get_list("any"); + list.clear(); + list.add("Hello"); + + list = foo.get_list("any_list"); + auto sub_list = list.get_list(0); + sub_list->clear(); + sub_list->add(1); + sub_list->add(2); + + auto list_int = foo.get_list("ints"); + list_int.clear(); + list_int.add(1); + list_int.add(2); + tr_1->commit_and_continue_as_read(); + } + + { + tr_2->promote_to_write(); + auto table_2 = tr_2->get_table("class_Table"); + + auto bar = table_2->get_object_with_primary_key(123); + auto list = bar.get_list("any"); + list.clear(); + list.add("Godbye"); + + list = bar.get_list("any_list"); + auto sub_list = list.get_list(0); + sub_list->clear(); + sub_list->add(3); + sub_list->add(4); + + auto list_int = bar.get_list("ints"); + list_int.clear(); + list_int.add(3); + list_int.add(4); + tr_2->commit_and_continue_as_read(); + } + + session_1.wait_for_upload_complete_or_client_stopped(); + session_2.wait_for_upload_complete_or_client_stopped(); + session_1.wait_for_download_complete_or_client_stopped(); + session_2.wait_for_download_complete_or_client_stopped(); + + tr_1->advance_read(); + tr_2->advance_read(); + auto list = foo.get_list("any"); + CHECK_EQUAL(list.size(), 1); + + list = foo.get_list("any_list"); + CHECK_EQUAL(list.get_list(0)->size(), 2); + + auto list_int = foo.get_list("ints"); + CHECK_EQUAL(list_int.size(), 4); // We should stille have odd behavior for normal lists + + CHECK(compare_groups(*tr_1, *tr_2)); +} + TEST(Sync_Dictionary_Links) { TEST_CLIENT_DB(db_1); From 35ab1dee3725ac129909774d2563de0b90902411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Thu, 27 Jun 2024 09:31:34 +0200 Subject: [PATCH 2/5] Add test for dictionary --- test/test_sync.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/test_sync.cpp b/test/test_sync.cpp index 708b9d2f61c..b9c79218002 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -6228,6 +6228,8 @@ TEST(Sync_NestedCollectionClear) auto foo = table_1->create_object_with_primary_key(123); foo.set_collection(col, CollectionType::List); foo.get_list(col_list).insert_collection(0, CollectionType::List); + auto foo1 = table_1->create_object_with_primary_key(456); + foo1.set_collection(col, CollectionType::Dictionary); tr_1->commit_and_continue_as_read(); session_1.wait_for_upload_complete_or_client_stopped(); @@ -6245,6 +6247,10 @@ TEST(Sync_NestedCollectionClear) sub_list->add(1); sub_list->add(2); + auto dict = foo1.get_dictionary("any"); + dict.clear(); + dict.insert("age", 42); + auto list_int = foo.get_list("ints"); list_int.clear(); list_int.add(1); @@ -6267,6 +6273,11 @@ TEST(Sync_NestedCollectionClear) sub_list->add(3); sub_list->add(4); + auto bar1 = table_2->get_object_with_primary_key(456); + auto dict = bar1.get_dictionary("any"); + dict.clear(); + dict.insert("weight", 70); + auto list_int = bar.get_list("ints"); list_int.clear(); list_int.add(3); @@ -6287,6 +6298,9 @@ TEST(Sync_NestedCollectionClear) list = foo.get_list("any_list"); CHECK_EQUAL(list.get_list(0)->size(), 2); + auto dict = foo1.get_dictionary("any"); + CHECK_EQUAL(dict.size(), 1); + auto list_int = foo.get_list("ints"); CHECK_EQUAL(list_int.size(), 4); // We should stille have odd behavior for normal lists From 87655e02fc49b0d9644f8fec2b6d712faa8e7652 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Thu, 27 Jun 2024 13:35:51 +0200 Subject: [PATCH 3/5] Typo --- test/test_sync.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_sync.cpp b/test/test_sync.cpp index b9c79218002..a59ca01b556 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -6302,7 +6302,7 @@ TEST(Sync_NestedCollectionClear) CHECK_EQUAL(dict.size(), 1); auto list_int = foo.get_list("ints"); - CHECK_EQUAL(list_int.size(), 4); // We should stille have odd behavior for normal lists + CHECK_EQUAL(list_int.size(), 4); // We should still have odd behavior for normal lists CHECK(compare_groups(*tr_1, *tr_2)); } From 57d23074d89382a78c0684516d76ccfa086133a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 28 Jun 2024 11:28:04 +0200 Subject: [PATCH 4/5] Remove unused code --- test/test_sync.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/test_sync.cpp b/test/test_sync.cpp index a59ca01b556..986bda35526 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -6216,8 +6216,6 @@ TEST(Sync_NestedCollectionClear) Session session_1 = fixture.make_session(db_1, "/test"); Session session_2 = fixture.make_session(db_2, "/test"); - Timestamp now{std::chrono::system_clock::now()}; - auto tr_1 = db_1->start_write(); auto tr_2 = db_2->start_read(); auto table_1 = tr_1->add_table_with_primary_key("class_Table", type_Int, "id"); From d8d95f74a72f2349cc8fbef8a3538cf5feee78bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 28 Jun 2024 15:41:50 +0200 Subject: [PATCH 5/5] More test --- test/test_sync.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/test/test_sync.cpp b/test/test_sync.cpp index 7e53f550eb5..638195ab528 100644 --- a/test/test_sync.cpp +++ b/test/test_sync.cpp @@ -6228,7 +6228,9 @@ TEST(Sync_NestedCollectionClear) auto foo = table_1->create_object_with_primary_key(123); foo.set_collection(col, CollectionType::List); - foo.get_list(col_list).insert_collection(0, CollectionType::List); + auto parent_list = foo.get_list(col_list); + parent_list.insert_collection(0, CollectionType::List); + parent_list.insert_collection(1, CollectionType::Dictionary); auto foo1 = table_1->create_object_with_primary_key(456); foo1.set_collection(col, CollectionType::Dictionary); tr_1->commit_and_continue_as_read(); @@ -6247,6 +6249,10 @@ TEST(Sync_NestedCollectionClear) sub_list->clear(); sub_list->add(1); sub_list->add(2); + auto sub_dict = list.get_dictionary(1); + sub_dict->clear(); + sub_dict->insert("one", 1); + sub_dict->insert("two", 2); auto dict = foo1.get_dictionary("any"); dict.clear(); @@ -6273,6 +6279,10 @@ TEST(Sync_NestedCollectionClear) sub_list->clear(); sub_list->add(3); sub_list->add(4); + auto sub_dict = list.get_dictionary(1); + sub_dict->clear(); + sub_dict->insert("three", 3); + sub_dict->insert("four", 4); auto bar1 = table_2->get_object_with_primary_key(456); auto dict = bar1.get_dictionary("any"); @@ -6298,6 +6308,7 @@ TEST(Sync_NestedCollectionClear) list = foo.get_list("any_list"); CHECK_EQUAL(list.get_list(0)->size(), 2); + CHECK_EQUAL(list.get_dictionary(1)->size(), 2); auto dict = foo1.get_dictionary("any"); CHECK_EQUAL(dict.size(), 1);