RCORE-2168: Replicate clear instruction unconditionally for nested co…

…llections (#7821)
realm · Jun 28, 2024 · 3ccd33a · 3ccd33a
1 parent 68f79ae
commit 3ccd33a
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,7 @@
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * `DB::compact()` on an encrypted Realm without explicitly specifying a new encryption key would only work if the old key happened to be a valid nul-terminated string ([#7842](https://github.com/realm/realm-core/issues/7842), since v14.10.0).
+* 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
@@ -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
@@ -503,10 +503,12 @@ void Lst<Mixed>::remove(size_t from, size_t to)
 
 void Lst<Mixed>::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
@@ -6207,6 +6207,118 @@ 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");
+
+    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);
+    auto parent_list = foo.get_list<Mixed>(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();
+
+    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<Mixed>("any");
+        list.clear();
+        list.add("Hello");
+
+        list = foo.get_list<Mixed>("any_list");
+        auto sub_list = list.get_list(0);
+        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();
+        dict.insert("age", 42);
+
+        auto list_int = foo.get_list<Mixed>("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<Mixed>("any");
+        list.clear();
+        list.add("Godbye");
+
+        list = bar.get_list<Mixed>("any_list");
+        auto sub_list = list.get_list(0);
+        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");
+        dict.clear();
+        dict.insert("weight", 70);
+
+        auto list_int = bar.get_list<Mixed>("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<Mixed>("any");
+    CHECK_EQUAL(list.size(), 1);
+
+    list = foo.get_list<Mixed>("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);
+
+    auto list_int = foo.get_list<Mixed>("ints");
+    CHECK_EQUAL(list_int.size(), 4); // We should still have odd behavior for normal lists
+
+    CHECK(compare_groups(*tr_1, *tr_2));
+}
+
 TEST(Sync_Dictionary_Links)
 {
     TEST_CLIENT_DB(db_1);