RCORE-1980 Add unit test and changelog entry for replication bug (#7537)

* Add unit test and changelog entry for replication bug

* Update changelog

* Revert replication of clear instruction on empty lists

* Revert tests too

Author: danieltabacaru

CHANGELOG.md

@@ -6,6 +6,7 @@
 ### Fixed
 * <How do the end-user experience this issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * Fix an assertion failure "m_lock_info && m_lock_info->m_file.get_path() == m_filename" that appears to be related to opening a Realm while the file is in the process of being closed on another thread ([Swift #8507](https://github.com/realm/realm-swift/issues/8507)).
+* Fixed diverging history due to a bug in the replication code when setting default null values (embedded objects included) ([#7536](https://github.com/realm/realm-core/issues/7536)).
 
 ### Breaking changes
 * Updated default base URL to be `https://services.cloud.mongodb.com` to support the new domains (was `https://realm.mongodb.com`). ([PR #7534](https://github.com/realm/realm-core/pull/7534))

src/realm/list.hpp

@@ -928,11 +928,10 @@ inline LnkLst Obj::get_linklist(StringData col_name) const
 template <class T>
 void Lst<T>::clear()
 {
-    auto sz = size();
-    if (Replication* repl = Base::get_replication()) {
-        repl->list_clear(*this);
-    }
-    if (sz > 0) {
+    if (size() > 0) {
+        if (Replication* repl = Base::get_replication()) {
+            repl->list_clear(*this);
+        }
         do_clear();
         bump_content_version();
     }

test/object-store/object.cpp

@@ -2023,10 +2023,10 @@ TEST_CASE("object") {
 
         Obj obj = object1.get_obj();
         REQUIRE(obj.get<Int>("_id") == 7); // pk
-        REQUIRE(obj.get_linklist("array 1").size() == 1);
+        REQUIRE(obj.get_linklist("array 1").size() == 2);
         REQUIRE(obj.get<Int>("int 1") == 1); // non-default from r1
         REQUIRE(obj.get<Int>("int 2") == 2); // non-default from r2
-        REQUIRE(obj.get_linklist("array 2").size() == 1);
+        REQUIRE(obj.get_linklist("array 2").size() == 2);
     }
 #endif
 }

test/test_transform.cpp

@@ -3011,4 +3011,67 @@ TEST(Transform_ClearArrayVsUpdateInt)
     CHECK_EQUAL(table->get_object_with_primary_key(1).get_any(col_any), 42);
 }
 
+TEST(Transform_SetNullDefaultVsSetDefault)
+{
+    auto changeset_dump_dir_gen = get_changeset_dump_dir_generator(test_context);
+    auto server = Peer::create_server(test_context, changeset_dump_dir_gen.get());
+    auto client_1 = Peer::create_client(test_context, 2, changeset_dump_dir_gen.get());
+    auto client_2 = Peer::create_client(test_context, 3, changeset_dump_dir_gen.get());
+
+    // Baseline: no data
+    client_1->transaction([](Peer& c) {
+        auto& tr = *c.group;
+        TableRef table = tr.add_table_with_primary_key("class_Table", type_Int, "id", true);
+        bool is_default = true;
+        table->add_column(type_Mixed, "mixed", is_default);
+        table->add_column(type_Int, "int");
+    });
+
+    synchronize(server.get(), {client_1.get(), client_2.get()});
+
+    client_1->history.set_time(1);
+    client_2->history.set_time(2);
+
+    // Client 1 creates object with primary key '1' and sets property 'mixed' to a default value of 'null' and
+    // property 'int' to '42'
+    // {id: 1, mixed: null, int: 42}
+    client_1->transaction([](Peer& p) {
+        auto obj = p.table("class_Table")->create_object_with_primary_key(1);
+        auto col_mixed = p.table("class_Table")->get_column_key("mixed");
+        auto col_int = p.table("class_Table")->get_column_key("int");
+        bool is_default = true;
+        obj.set_null(col_mixed, is_default);
+        obj.set(col_int, 42, is_default);
+    });
+
+    // Client 2 creates object with primary key '1' and sets property 'mixed' to a default value of '-6' and property
+    // 'int' to '6'
+    // {id: 1, mixed: -6, int: 6}
+    client_2->transaction([](Peer& p) {
+        auto obj = p.table("class_Table")->create_object_with_primary_key(1);
+        auto col_mixed = p.table("class_Table")->get_column_key("mixed");
+        auto col_int = p.table("class_Table")->get_column_key("int");
+        bool is_default = true;
+        obj.set(col_mixed, Mixed(-6), is_default);
+        obj.set(col_int, 6, is_default);
+    });
+
+    synchronize(server.get(), {client_1.get(), client_2.get()});
+
+    // Result: Client 2's changes win because they happen after Client 1's changes (and is_default is true for all
+    // instructions)
+    // {id: 1, mixed: -6, int: 6}
+    ReadTransaction read_server(server->shared_group);
+    ReadTransaction read_client_1(client_1->shared_group);
+    ReadTransaction read_client_2(client_2->shared_group);
+    CHECK(compare_groups(read_server, read_client_1));
+    CHECK(compare_groups(read_server, read_client_2, *test_context.logger));
+    auto table = read_server.get_table("class_Table");
+    auto col_mixed = table->get_column_key("mixed");
+    auto col_int = table->get_column_key("int");
+    auto obj = table->get_object_with_primary_key(1);
+    CHECK_EQUAL(obj.get_any(col_mixed), Mixed(-6));
+    CHECK_EQUAL(obj.get_any(col_int), Mixed(6));
+}
+
 } // unnamed namespace