Fix Transaction::copy_to() for nested collection

Author: jedelbo

CHANGELOG.md

@@ -52,6 +52,7 @@
 * Catch2 is no longer required as a submodule if the `REALM_NO_TESTS` flag is set.
 * Sha-2 is no longer required as a submodule on Windows if linking with OpenSSL.
 * The Catch2 submodule has moved to `test/external/catch`.
+* Fix possible file corruption if using Transaction::copy_to if nested collections are present.
 
 ----------------------------------------------
 

src/realm/impl/copy_replication.cpp

@@ -130,12 +130,15 @@ void CopyReplication::list_clear(const CollectionBase& coll)
     m_current.obj_in_destination.get_listbase_ptr(dest_col_key)->clear();
 }
 
-void CopyReplication::list_insert(const CollectionBase& coll, size_t idx, Mixed value, size_t)
+void CopyReplication::list_insert(const CollectionBase& source_coll, size_t idx, Mixed value, size_t)
 {
-    ColKey col_key = coll.get_col_key();
-    sync(coll);
+    ColKey col_key = source_coll.get_col_key();
+    sync(source_coll);
     auto dest_col_key = get_colkey_in_destination_realm(col_key);
-    auto list = m_current.obj_in_destination.get_listbase_ptr(dest_col_key);
+    auto path = source_coll.get_short_path();
+    path[0] = dest_col_key;
+    auto coll = m_current.obj_in_destination.get_collection_ptr(path);
+    auto list = dynamic_cast<LstBase*>(coll.get());
     if (value.is_type(type_Link, type_TypedLink)) {
         value = handle_link(col_key, value, [&](TableRef) {
             auto link_list = m_current.obj_in_destination.get_linklist(dest_col_key);
@@ -162,31 +165,34 @@ void CopyReplication::set_insert(const CollectionBase& coll, size_t, Mixed value
     set->insert_any(value);
 }
 
-void CopyReplication::dictionary_insert(const CollectionBase& coll, size_t, Mixed key, Mixed value)
+void CopyReplication::dictionary_insert(const CollectionBase& source_coll, size_t, Mixed key, Mixed value)
 {
-    ColKey col_key = coll.get_col_key();
-    sync(coll);
+    ColKey col_key = source_coll.get_col_key();
+    sync(source_coll);
     auto dest_col_key = get_colkey_in_destination_realm(col_key);
-    auto dict = m_current.obj_in_destination.get_dictionary(dest_col_key);
+    auto path = source_coll.get_short_path();
+    path[0] = dest_col_key;
+    auto coll = m_current.obj_in_destination.get_collection_ptr(path);
+    auto dict = dynamic_cast<Dictionary*>(coll.get());
     if (value.is_type(type_Link, type_TypedLink)) {
         value = handle_link(col_key, value, [&](TableRef dest_target_table) {
             // Check if dictionary obj has embedded obj already
-            size_t ndx = dict.find_any_key(key);
+            size_t ndx = dict->find_any_key(key);
             if (ndx != realm::not_found) {
-                auto val = dict.get_any(ndx);
+                auto val = dict->get_any(ndx);
                 if (val.is_type(type_Link)) {
                     ObjKey key = val.get<ObjKey>();
                     m_current.obj_in_destination = dest_target_table->get_object(key);
                     return;
                 }
             }
             // If not, create one
-            m_current.obj_in_destination = dict.create_and_insert_linked_object(key);
+            m_current.obj_in_destination = dict->create_and_insert_linked_object(key);
         });
         if (value.is_null())
             return;
     }
-    dict.insert(key, value);
+    dict->insert(key, value);
 }
 
 void CopyReplication::sync(const Table* t, ObjKey obj_key)

src/realm/obj.cpp

@@ -240,12 +240,12 @@ bool Obj::compare_list_in_mixed(Lst<Mixed>& val1, Lst<Mixed>& val2, ColKey ck, O
         auto m2 = val2.get_any(i);
 
         if (m1.is_type(type_List) && m2.is_type(type_List)) {
-            DummyParent parent(get_table(), m2.get_ref());
+            DummyParent parent(other.get_table(), m2.get_ref());
             Lst<Mixed> list(parent, 0);
             return compare_list_in_mixed(*val1.get_list(i), list, ck, other, col_name);
         }
         else if (m1.is_type(type_Dictionary) && m2.is_type(type_Dictionary)) {
-            DummyParent parent(get_table(), m2.get_ref());
+            DummyParent parent(other.get_table(), m2.get_ref());
             Dictionary dict(parent, 0);
             return compare_dict_in_mixed(*val1.get_dictionary(i), dict, ck, other, col_name);
         }
@@ -269,12 +269,12 @@ bool Obj::compare_dict_in_mixed(Dictionary& val1, Dictionary& val2, ColKey ck, O
             return false;
 
         if (m1.is_type(type_List) && m2.is_type(type_List)) {
-            DummyParent parent(get_table(), m2.get_ref());
+            DummyParent parent(other.get_table(), m2.get_ref());
             Lst<Mixed> list(parent, 0);
             return compare_list_in_mixed(*val1.get_list(k1.get_string()), list, ck, other, col_name);
         }
         else if (m1.is_type(type_Dictionary) && m2.is_type(type_Dictionary)) {
-            DummyParent parent(get_table(), m2.get_ref());
+            DummyParent parent(other.get_table(), m2.get_ref());
             Dictionary dict(parent, 0);
             return compare_dict_in_mixed(*val1.get_dictionary(k1.get_string()), dict, ck, other, col_name);
         }

src/realm/transaction.cpp

@@ -46,23 +46,27 @@ ColInfo get_col_info(const Table* table)
 }
 
 void add_list_to_repl(CollectionBase& list, Replication& repl, util::UniqueFunction<void(Mixed)> update_embedded);
+
 void add_dictionary_to_repl(Dictionary& dict, Replication& repl, util::UniqueFunction<void(Mixed)> update_embedded)
 {
     size_t sz = dict.size();
     for (size_t n = 0; n < sz; ++n) {
         const auto& [key, val] = dict.get_pair(n);
-        repl.dictionary_insert(dict, n, key, val);
         if (val.is_type(type_List)) {
+            repl.dictionary_insert(dict, n, key, Mixed{0, CollectionType::List});
             auto n_list = dict.get_list({key.get_string()});
-            add_list_to_repl(*n_list, *dict.get_table()->get_repl(), nullptr);
+            add_list_to_repl(*n_list, repl, nullptr);
         }
         else if (val.is_type(type_Dictionary)) {
-            repl.dictionary_insert(dict, n, key, val);
+            repl.dictionary_insert(dict, n, key, Mixed{0, CollectionType::Dictionary});
             auto n_dict = dict.get_dictionary({key.get_string()});
-            add_dictionary_to_repl(*n_dict, *dict.get_table()->get_repl(), nullptr);
+            add_dictionary_to_repl(*n_dict, repl, nullptr);
         }
-        else if (update_embedded) {
-            update_embedded(val);
+        else {
+            repl.dictionary_insert(dict, n, key, val);
+            if (update_embedded) {
+                update_embedded(val);
+            }
         }
     }
 }
@@ -72,17 +76,21 @@ void add_list_to_repl(CollectionBase& list, Replication& repl, util::UniqueFunct
     auto sz = list.size();
     for (size_t n = 0; n < sz; n++) {
         auto val = list.get_any(n);
-        repl.list_insert(list, n, val, n);
         if (val.is_type(type_List)) {
+            repl.list_insert(list, n, Mixed{0, CollectionType::List}, n);
             auto n_list = list.get_list({n});
-            add_list_to_repl(*n_list, *list.get_table()->get_repl(), nullptr);
+            add_list_to_repl(*n_list, repl, nullptr);
         }
         else if (val.is_type(type_Dictionary)) {
+            repl.list_insert(list, n, Mixed{0, CollectionType::Dictionary}, n);
             auto n_dict = list.get_dictionary({n});
-            add_dictionary_to_repl(*n_dict, *list.get_table()->get_repl(), nullptr);
+            add_dictionary_to_repl(*n_dict, repl, nullptr);
         }
-        else if (update_embedded) {
-            update_embedded(val);
+        else {
+            repl.list_insert(list, n, val, n);
+            if (update_embedded) {
+                update_embedded(val);
+            }
         }
     }
 }

test/test_shared.cpp

@@ -4278,7 +4278,6 @@ TEST(Shared_WriteTo)
     tr->commit_and_continue_as_read();
     // tr->to_json(std::cout);
 
-
     // Create remote db
     DBRef db2 = DB::create(make_in_realm_history(), path2);
     {
@@ -4310,7 +4309,9 @@ TEST(Shared_WriteTo)
     // Copy local object over
     auto dest = db2->start_write();
     tr->copy_to(dest);
+    dest->verify();
     dest->commit_and_continue_as_read();
+    // dest->to_json(std::cout);
 
     // The difference between the two realms should now be that the remote db has an
     // extra baa object with pk 333.