Fix sync replication (#7343)

src/realm/collection.hpp

@@ -35,6 +35,10 @@ class DummyParent : public CollectionParent {
     {
         return {};
     }
+    ColKey get_col_key() const noexcept final
+    {
+        return {};
+    }
     void add_index(Path&, const Index&) const noexcept final {}
     size_t find_index(const Index&) const noexcept final
     {
@@ -478,7 +482,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
     }
 
     // Overriding members of CollectionBase:
-    ColKey get_col_key() const noexcept final
+    ColKey get_col_key() const noexcept override
     {
         return m_col_key;
     }
@@ -576,6 +580,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
     Obj m_obj_mem;
     std::shared_ptr<CollectionParent> m_col_parent;
     CollectionParent::Index m_index;
+    mutable size_t m_my_version = 0;
     ColKey m_col_key;
     bool m_nullable = false;
 
@@ -618,6 +623,7 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
     CollectionBaseImpl(CollectionParent& parent, CollectionParent::Index index) noexcept
         : m_obj_mem(parent.get_object())
         , m_index(index)
+        , m_col_key(parent.get_col_key())
         , m_parent(&parent)
         , m_alloc(&m_obj_mem.get_alloc())
     {
@@ -655,8 +661,9 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
 
         if (status != UpdateStatus::Detached) {
             auto content_version = m_alloc->get_content_version();
-            if (content_version != m_content_version) {
+            if (content_version != m_content_version || m_my_version != m_parent->m_parent_version) {
                 m_content_version = content_version;
+                m_my_version = m_parent->m_parent_version;
                 status = UpdateStatus::Updated;
             }
         }
@@ -673,8 +680,9 @@ class CollectionBaseImpl : public Interface, protected ArrayParent {
         bool changed = m_parent->update_if_needed(); // Throws if the object does not exist.
         auto content_version = m_alloc->get_content_version();
 
-        if (changed || content_version != m_content_version) {
+        if (changed || content_version != m_content_version || m_my_version != m_parent->m_parent_version) {
             m_content_version = content_version;
+            m_my_version = m_parent->m_parent_version;
             return true;
         }
         return false;

src/realm/collection_parent.hpp

@@ -84,6 +84,8 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
     virtual FullPath get_path() const = 0;
     // Return path from owning object
     virtual Path get_short_path() const = 0;
+    // Return column of owning property
+    virtual ColKey get_col_key() const noexcept = 0;
     // Return path from owning object
     virtual StablePath get_stable_path() const = 0;
     // Add a translation of Index to PathElement
@@ -105,6 +107,7 @@ class CollectionParent : public std::enable_shared_from_this<CollectionParent> {
     static constexpr size_t s_max_level = 100;
 #endif
     size_t m_level = 0;
+    mutable size_t m_parent_version = 0;
 
     constexpr CollectionParent(size_t level = 0)
         : m_level(level)

src/realm/dictionary.cpp

@@ -436,13 +436,17 @@ void Dictionary::insert_collection(const PathElement& path_elem, CollectionType
 
     check_level();
     ensure_created();
-    m_values->ensure_keys();
-    auto [it, inserted] = insert(path_elem.get_key(), Mixed(0, dict_or_list));
-    int64_t key = generate_key(size());
-    while (m_values->find_key(key) != realm::not_found) {
-        key++;
+    Mixed new_val(0, dict_or_list);
+    auto old_val = try_get(path_elem.get_key());
+    if (!old_val || *old_val != new_val) {
+        m_values->ensure_keys();
+        auto [it, inserted] = insert(path_elem.get_key(), new_val);
+        int64_t key = generate_key(size());
+        while (m_values->find_key(key) != realm::not_found) {
+            key++;
+        }
+        m_values->set_key(it.index(), key);
     }
-    m_values->set_key(it.index(), key);
 }
 
 DictionaryPtr Dictionary::get_dictionary(const PathElement& path_elem) const
@@ -668,6 +672,7 @@ UpdateStatus Dictionary::update_if_needed_with_status() const
             // the function will return false;
             bool attached = init_from_parent(false);
             Base::update_content_version();
+            CollectionParent::m_parent_version++;
             return attached ? UpdateStatus::Updated : UpdateStatus::Detached;
         }
     }
@@ -681,6 +686,7 @@ void Dictionary::ensure_created()
         // In case of errors, an exception is thrown.
         constexpr bool allow_create = true;
         init_from_parent(allow_create); // Throws
+        CollectionParent::m_parent_version++;
         Base::update_content_version();
     }
 }

src/realm/dictionary.hpp

@@ -196,6 +196,11 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle
         return Base::get_short_path();
     }
 
+    ColKey get_col_key() const noexcept override
+    {
+        return Base::get_col_key();
+    }
+
     StablePath get_stable_path() const override
     {
         return Base::get_stable_path();

src/realm/list.cpp

@@ -396,6 +396,7 @@ UpdateStatus Lst<Mixed>::update_if_needed_with_status() const
         case UpdateStatus::Updated: {
             bool attached = init_from_parent(false);
             Base::update_content_version();
+            CollectionParent::m_parent_version++;
             return attached ? UpdateStatus::Updated : UpdateStatus::Detached;
         }
     }

src/realm/list.hpp

@@ -458,6 +458,7 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa
             constexpr bool allow_create = true;
             init_from_parent(allow_create); // Throws
             Base::update_content_version();
+            CollectionParent::m_parent_version++;
         }
     }
 
@@ -484,6 +485,11 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa
         return Base::get_stable_path();
     }
 
+    ColKey get_col_key() const noexcept override
+    {
+        return Base::get_col_key();
+    }
+
     void add_index(Path& path, const Index& ndx) const final;
     size_t find_index(const Index& ndx) const final;
 

src/realm/obj.cpp

@@ -385,6 +385,7 @@ bool Obj::update() const
     if (changes) {
         m_mem = new_obj.m_mem;
         m_row_ndx = new_obj.m_row_ndx;
+        CollectionParent::m_parent_version++;
     }
     // Always update versions
     m_storage_version = new_obj.m_storage_version;
@@ -422,6 +423,7 @@ UpdateStatus Obj::update_if_needed_with_status() const
         if ((m_mem.get_addr() != state.mem.get_addr()) || (m_row_ndx != state.index)) {
             m_mem = state.mem;
             m_row_ndx = state.index;
+            CollectionParent::m_parent_version++;
             return UpdateStatus::Updated;
         }
     }
@@ -1087,6 +1089,11 @@ Path Obj::get_short_path() const noexcept
     return {};
 }
 
+ColKey Obj::get_col_key() const noexcept
+{
+    return {};
+}
+
 StablePath Obj::get_stable_path() const noexcept
 {
     return {};

src/realm/obj.hpp

@@ -76,6 +76,7 @@ class Obj : public CollectionParent {
     FullPath get_path() const final;
     std::string get_id() const;
     Path get_short_path() const noexcept final;
+    ColKey get_col_key() const noexcept final;
     StablePath get_stable_path() const noexcept final;
     void add_index(Path& path, const Index& ndx) const final;
     size_t find_index(const Index&) const final

test/test_list.cpp

@@ -693,10 +693,13 @@ TEST(List_Nested_InMixed)
     */
 
     tr->promote_to_write();
-    dict2->insert_collection("List", CollectionType::List);
+    dict->insert_collection("Dict", CollectionType::Dictionary); // Idempotent, but updates dict accessor
+    dict2->insert_collection("List", CollectionType::List);      // dict2 should update
     {
         auto list = dict2->get_list("List");
+        CHECK_EQUAL(dict2->get_col_key(), col_any);
         CHECK(list->is_empty());
+        CHECK_EQUAL(list->get_col_key(), col_any);
         list->add(8);
         list->add(9);
     }
@@ -1060,7 +1063,10 @@ TEST(List_Nested_Replication)
         StablePath expected_path;
     } parser(test_context);
 
+    auto dict2_index = dict->build_index("level1");
     parser.expected_path.push_back(StableIndex());
-    parser.expected_path.push_back(dict->build_index("level1"));
+    parser.expected_path.push_back(dict2_index);
     tr->advance_read(&parser);
+    Dictionary dict3(*dict, dict2_index);
+    CHECK_EQUAL(dict3.get_col_key(), col_any);
 }

test/test_sync.cpp

@@ -6140,6 +6140,8 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         dict->insert_collection("list", CollectionType::List);
         auto l = dict->get_list("list");
         l->add(5);
+        l->insert_collection(1, CollectionType::List);
+        l->get_list(1)->add(7);
 
         auto bar = table->create_object_with_primary_key(456);
 
@@ -6148,15 +6150,6 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         auto list = bar.get_list_ptr<Mixed>(col_any);
         list->add("John");
         list->insert(0, 5);
-
-        auto foobar = table->create_object_with_primary_key(789);
-
-        // Create set in Mixed property
-        foobar.set_collection(col_any, CollectionType::Set);
-        auto set = foobar.get_set_ptr<Mixed>(col_any);
-        set->insert(1);
-        set->insert(2);
-        set->insert(5);
     });
 
     session_1.wait_for_upload_complete_or_client_stopped();
@@ -6165,7 +6158,7 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
     write_transaction(db_2, [&](WriteTransaction& tr) {
         auto table = tr.get_table("class_Table");
         auto col_any = table->get_column_key("any");
-        CHECK_EQUAL(table->size(), 3);
+        CHECK_EQUAL(table->size(), 2);
 
         auto obj = table->get_object_with_primary_key(123);
         auto dict = obj.get_dictionary_ptr(col_any);
@@ -6195,15 +6188,6 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         list->set(1, "Paul");
         // Erase list element
         list->remove(0);
-
-        obj = table->get_object_with_primary_key(789);
-        auto set = obj.get_set_ptr<Mixed>(col_any);
-        // Check that values are replicated
-        CHECK_NOT_EQUAL(set->find(1), realm::npos);
-        CHECK_NOT_EQUAL(set->find(2), realm::npos);
-        CHECK_NOT_EQUAL(set->find(5), realm::npos);
-        // Erase set element
-        set->erase(2);
     });
 
     session_2.wait_for_upload_complete_or_client_stopped();
@@ -6212,7 +6196,7 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
     write_transaction(db_1, [&](WriteTransaction& tr) {
         auto table = tr.get_table("class_Table");
         auto col_any = table->get_column_key("any");
-        CHECK_EQUAL(table->size(), 3);
+        CHECK_EQUAL(table->size(), 2);
 
         auto obj = table->get_object_with_primary_key(123);
         auto dict = obj.get_dictionary(col_any);
@@ -6232,11 +6216,6 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         CHECK_EQUAL(list->get(0).get_string(), "Paul");
         // List clear
         list->clear();
-
-        obj = table->get_object_with_primary_key(789);
-        auto set = obj.get_set_ptr<Mixed>(col_any);
-        CHECK_EQUAL(set->size(), 2);
-        set->clear();
     });
 
     session_1.wait_for_upload_complete_or_client_stopped();
@@ -6246,7 +6225,7 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         auto table = tr.get_table("class_Table");
         auto col_any = table->get_column_key("any");
 
-        CHECK_EQUAL(table->size(), 3);
+        CHECK_EQUAL(table->size(), 2);
 
         auto obj = table->get_object_with_primary_key(123);
         auto dict = obj.get_dictionary(col_any);
@@ -6258,14 +6237,9 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         obj = table->get_object_with_primary_key(456);
         auto list = obj.get_list<Mixed>(col_any);
         CHECK_EQUAL(list.size(), 0);
-        // Replace list with set on property
-        obj.set_collection(col_any, CollectionType::Set);
-
-        obj = table->get_object_with_primary_key(789);
-        auto set = obj.get_set<Mixed>(col_any);
-        CHECK_EQUAL(set.size(), 0);
-        // Replace set with dictionary on property
+        // Replace list with Dictionary on property
         obj.set_collection(col_any, CollectionType::Dictionary);
+
     });
 
     session_2.wait_for_upload_complete_or_client_stopped();
@@ -6278,17 +6252,13 @@ TEST_IF(Sync_CollectionInMixed, sync::SYNC_SUPPORTS_NESTED_COLLECTIONS)
         auto table = read_2.get_table("class_Table");
         auto col_any = table->get_column_key("any");
 
-        CHECK_EQUAL(table->size(), 3);
+        CHECK_EQUAL(table->size(), 2);
 
         auto obj = table->get_object_with_primary_key(123);
         auto list = obj.get_list<Mixed>(col_any);
         CHECK_EQUAL(list.size(), 0);
 
         obj = table->get_object_with_primary_key(456);
-        auto set = obj.get_set<Mixed>(col_any);
-        CHECK_EQUAL(set.size(), 0);
-
-        obj = table->get_object_with_primary_key(789);
         auto dict = obj.get_dictionary(col_any);
         CHECK_EQUAL(dict.size(), 0);