Fix collection mismatch for Set in Mixed. (#6802)

realm · Jul 19, 2023 · c1e4126 · c1e4126
1 parent 78ffbbc
commit c1e4126
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 19 deletions.
diff --git a/src/realm/bplustree.hpp b/src/realm/bplustree.hpp
@@ -190,7 +190,6 @@ class BPlusTreeBase {
 
     bool init_from_parent()
     {
-        ;
         if (ref_type ref = m_parent->get_child_ref(m_ndx_in_parent)) {
             init_from_ref(ref);
             return true;

diff --git a/src/realm/list.cpp b/src/realm/list.cpp
@@ -349,7 +349,6 @@ void Lst<Mixed>::insert(size_t ndx, Mixed value)
     ensure_created();
     auto sz = size();
     CollectionBase::validate_index("insert()", ndx, sz + 1);
-
     if (Replication* repl = Base::get_replication()) {
         repl->list_insert(*this, ndx, value, sz);
     }
@@ -464,7 +463,7 @@ void Lst<Mixed>::set_collection(const PathElement& path_elem, CollectionType typ
 {
     auto ndx = path_elem.get_ndx();
     // get will check for ndx out of bounds
-    Mixed old_val = do_get(ndx, "set()");
+    Mixed old_val = do_get(ndx, "set_collection()");
     Mixed new_val(0, type);
 
     check_level();
@@ -544,6 +543,7 @@ void Lst<Mixed>::do_insert(size_t ndx, Mixed value)
     if (value.is_type(type_TypedLink)) {
         Base::set_backlink(m_col_key, value.get<ObjLink>());
     }
+
     m_tree->insert(ndx, value);
 }
 

diff --git a/src/realm/list.hpp b/src/realm/list.hpp
@@ -212,8 +212,8 @@ class Lst final : public CollectionBaseImpl<LstBase> {
     {
         if (Base::should_update() || !(m_tree && m_tree->is_attached())) {
             bool attached = init_from_parent(true);
-            Base::update_content_version();
             REALM_ASSERT(attached);
+            Base::update_content_version();
         }
     }
 
@@ -1215,9 +1215,7 @@ void Lst<T>::insert(size_t ndx, T value)
 
     auto sz = size();
     CollectionBase::validate_index("insert()", ndx, sz + 1);
-
     ensure_created();
-
     if (Replication* repl = Base::get_replication()) {
         repl->list_insert(*this, ndx, value, sz);
     }

diff --git a/src/realm/set.hpp b/src/realm/set.hpp
@@ -195,8 +195,10 @@ class Set final : public CollectionBaseImpl<SetBase> {
     {
         if (Base::should_update() || !(m_tree && m_tree->is_attached())) {
             bool attached = init_from_parent(true);
+            if (!attached) {
+                throw IllegalOperation("This is an ex-set");
+            }
             Base::update_content_version();
-            REALM_ASSERT(attached);
         }
     }
 
@@ -223,19 +225,28 @@ class Set final : public CollectionBaseImpl<SetBase> {
             const ArrayParent* parent = this;
             m_tree->set_parent(const_cast<ArrayParent*>(parent), 0);
         }
-
-        if (m_tree->init_from_parent()) {
-            // All is well
-            return true;
+        try {
+            auto ref = Base::get_collection_ref();
+            if (ref) {
+                m_tree->init_from_ref(ref);
+            }
+            else {
+                if (m_tree->init_from_parent()) {
+                    // All is well
+                    return true;
+                }
+                if (!allow_create) {
+                    return false;
+                }
+                // The ref in the column was NULL, create the tree in place.
+                m_tree->create();
+                REALM_ASSERT(m_tree->is_attached());
+            }
         }
-
-        if (!allow_create) {
+        catch (...) {
+            m_tree->detach();
             return false;
         }
-
-        // The ref in the column was NULL, create the tree in place.
-        m_tree->create();
-        REALM_ASSERT(m_tree->is_attached());
         return true;
     }
 
@@ -665,13 +676,12 @@ REALM_NOINLINE auto Set<T>::find_impl(const T& value) const -> iterator
 template <class T>
 std::pair<size_t, bool> Set<T>::insert(T value)
 {
-    update();
+    ensure_created();
 
     if (value_is_null(value) && !m_nullable)
         throw InvalidArgument(ErrorCodes::PropertyNotNullable,
                               util::format("Set: %1", CollectionBase::get_property_name()));
 
-    ensure_created();
     auto it = find_impl(value);
 
     if (it != this->end() && SetElementEquals<T>{}(*it, value)) {

diff --git a/test/object-store/c_api/c_api.cpp b/test/object-store/c_api/c_api.cpp
@@ -5088,6 +5088,54 @@ TEST_CASE("C API: nested collections", "[c_api]") {
         REQUIRE(size == 5);
     }
 
+    SECTION("set list for collection in mixed, verify that previous reference is invalid") {
+        REQUIRE(realm_set_collection(obj1.get(), foo_any_col_key, RLM_COLLECTION_TYPE_LIST));
+        realm_value_t value;
+        realm_get_value(obj1.get(), foo_any_col_key, &value);
+        REQUIRE(value.type == RLM_TYPE_LIST);
+        auto list = cptr_checked(realm_get_list(obj1.get(), foo_any_col_key));
+        realm_list_insert_collection(list.get(), 0, RLM_COLLECTION_TYPE_LIST);
+        auto n_list = cptr_checked(realm_list_get_list(list.get(), 0));
+        size_t size;
+        checked(realm_list_size(list.get(), &size));
+        REQUIRE(size == 1);
+        realm_list_insert(n_list.get(), 0, rlm_str_val("Test1"));
+        realm_list_set_collection(list.get(), 0, RLM_COLLECTION_TYPE_DICTIONARY);
+        // accessor has become invalid
+        REQUIRE(!realm_list_insert(n_list.get(), 1, rlm_str_val("Test2")));
+        CHECK_ERR(RLM_ERR_ILLEGAL_OPERATION);
+        // try to get a dictionary should work
+        auto n_dict = cptr_checked(realm_list_get_dictionary(list.get(), 0));
+        bool inserted = false;
+        size_t ndx;
+        realm_value_t key = rlm_str_val("key");
+        realm_value_t val = rlm_str_val("value");
+        REQUIRE(realm_dictionary_insert(n_dict.get(), key, val, &ndx, &inserted));
+        REQUIRE(ndx == 0);
+        REQUIRE(inserted);
+        realm_list_set_collection(list.get(), 0, RLM_COLLECTION_TYPE_SET);
+        // accessor invalid
+        REQUIRE(!realm_dictionary_insert(n_dict.get(), key, val, &ndx, &inserted));
+        CHECK_ERR(RLM_ERR_ILLEGAL_OPERATION);
+        auto n_set = cptr_checked(realm_list_get_set(list.get(), 0));
+        REQUIRE(realm_set_insert(n_set.get(), val, &ndx, &inserted));
+        REQUIRE(ndx == 0);
+        REQUIRE(inserted);
+        realm_list_set_collection(list.get(), 0, RLM_COLLECTION_TYPE_LIST);
+        // accessor invalid
+        REQUIRE(!realm_set_insert(n_set.get(), val, &ndx, &inserted));
+        CHECK_ERR(RLM_ERR_ILLEGAL_OPERATION);
+        // get a list should work
+        n_list = cptr_checked(realm_list_get_list(list.get(), 0));
+        REQUIRE(realm_list_insert(n_list.get(), 0, rlm_str_val("Test1")));
+        // reset the collection type to the same type (nop)
+        realm_list_set_collection(list.get(), 0, RLM_COLLECTION_TYPE_LIST);
+        // accessor is still valid
+        REQUIRE(realm_list_insert(n_list.get(), 0, rlm_str_val("Test2")));
+        checked(realm_list_size(n_list.get(), &size));
+        REQUIRE(size == 2);
+    }
+
     SECTION("set") {
         REQUIRE(realm_set_collection(obj1.get(), foo_any_col_key, RLM_COLLECTION_TYPE_SET));
         realm_value_t value;