Implementation CollectionList::remove() (#6381)

* init commit for implementing CollectionList::remove() * Revert "init commit for implementing CollectionList::remove()" This reverts commit 57a074e. * Revert "Revert "init commit for implementing CollectionList::remove()"" This reverts commit 43e102a. * fix internal logic for deleting a nested collection * more testing * Add verify() to List_NestedList_Remove * code review * typo * fix comment * code review --------- Co-authored-by: Jørgen Edelbo <[email protected]>
realm · Mar 20, 2023 · 1b41c6f · 1b41c6f
1 parent 3412c84
commit 1b41c6f
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 5 deletions.
diff --git a/src/realm/collection.hpp b/src/realm/collection.hpp
@@ -419,8 +419,8 @@ class CollectionList : public CollectionParent,
     CollectionListPtr insert_collection_list(StringData key);
     CollectionListPtr get_collection_list(size_t ndx) const;
 
-    void remove(size_t ndx);     // TODO
-    void remove(StringData key); // TODO
+    void remove(size_t ndx);
+    void remove(StringData key);
 
     ref_type get_child_ref(size_t child_ndx) const noexcept final;
     void update_child_ref(size_t child_ndx, ref_type new_ref) final;

diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp
@@ -222,6 +222,36 @@ CollectionListPtr CollectionList::get_collection_list(size_t ndx) const
     return CollectionList::create(const_cast<CollectionList*>(this)->shared_from_this(), m_col_key, index, coll_type);
 }
 
+void CollectionList::remove(size_t ndx)
+{
+    REALM_ASSERT(m_key_type == type_Int);
+    auto int_keys = static_cast<BPlusTree<Int>*>(m_keys.get());
+    const auto sz = int_keys->size();
+    if (ndx >= sz) {
+        throw OutOfBounds("CollectionList::remove", ndx, sz);
+    }
+    int_keys->erase(ndx);
+    auto ref = m_refs.get(ndx);
+    Array::destroy_deep(ref, *m_alloc);
+    m_refs.erase(ndx);
+}
+
+void CollectionList::remove(StringData key)
+{
+    REALM_ASSERT(m_key_type == type_String);
+    auto string_keys = static_cast<BPlusTree<String>*>(m_keys.get());
+    IteratorAdapter help(string_keys);
+    auto it = std::lower_bound(help.begin(), help.end(), key);
+    if (it.index() >= string_keys->size()) {
+        throw KeyNotFound("CollectionList::remove");
+    }
+    const auto index = it.index();
+    string_keys->erase(index);
+    auto ref = m_refs.get(index);
+    Array::destroy_deep(ref, *m_alloc);
+    m_refs.erase(index);
+}
+
 ref_type CollectionList::get_collection_ref(Index index) const noexcept
 {
     size_t ndx;

diff --git a/test/test_list.cpp b/test/test_list.cpp
@@ -33,8 +33,8 @@ using namespace std::chrono;
 #include "test.hpp"
 #include "test_types_helper.hpp"
 
-//#include <valgrind/callgrind.h>
-//#define PERFORMACE_TESTING
+// #include <valgrind/callgrind.h>
+// #define PERFORMACE_TESTING
 
 using namespace realm;
 using namespace realm::util;
@@ -627,7 +627,7 @@ TEST(List_AggOps)
     test_lists_numeric_agg<Decimal128>(test_context, sg, type_Decimal, Decimal128(realm::null()), true);
 }
 
-TEST(List_NestedList)
+TEST(List_NestedList_Insert)
 {
     SHARED_GROUP_TEST_PATH(path);
     DBRef db = DB::create(make_in_realm_history(), path);
@@ -676,3 +676,68 @@ TEST(List_NestedList)
     CHECK_EQUAL(collection->size(), 0);
     CHECK_EQUAL(collection2->size(), 0);
 }
+
+TEST(List_NestedList_Remove)
+{
+    SHARED_GROUP_TEST_PATH(path);
+    DBRef db = DB::create(make_in_realm_history(), path);
+    auto tr = db->start_write();
+    auto table = tr->add_table("table");
+    auto list_col = table->add_column(type_Int, "int_list_list", false, {CollectionType::List, CollectionType::List});
+    auto list_col2 = table->add_column(type_Int, "int_dict_list_list", false,
+                                       {CollectionType::Dictionary, CollectionType::List, CollectionType::List});
+
+    CHECK_EQUAL(table->get_nesting_levels(list_col), 1);
+    CHECK_EQUAL(table->get_nesting_levels(list_col2), 2);
+
+    Obj obj = table->create_object();
+
+    auto list = obj.get_collection_list(list_col);
+    CHECK(list->is_empty());
+    auto collection = list->insert_collection(0);
+    dynamic_cast<Lst<Int>*>(collection.get())->add(5);
+
+    auto dict = obj.get_collection_list(list_col2);
+    auto list2 = dict->insert_collection_list("Foo");
+    auto collection2 = list2->insert_collection(0);
+    dynamic_cast<Lst<Int>*>(collection2.get())->add(5);
+
+    tr->commit_and_continue_as_read();
+    CHECK_NOT(list->is_empty());
+    CHECK_EQUAL(obj.get_collection_list(list_col)->get_collection_ptr(0)->get_any(0).get_int(), 5);
+    CHECK_EQUAL(dynamic_cast<Lst<Int>*>(collection2.get())->get(0), 5);
+    // transaction
+    {
+        tr->promote_to_write();
+
+        auto lst = list->get_collection_ptr(0);
+        dynamic_cast<Lst<Int>*>(lst.get())->add(47);
+
+        lst = obj.get_collection_list(list_col2)->insert_collection_list("Foo")->get_collection_ptr(0);
+        dynamic_cast<Lst<Int>*>(collection2.get())->set(0, 100);
+
+        tr->commit_and_continue_as_read();
+    }
+    CHECK_EQUAL(dynamic_cast<Lst<Int>*>(collection.get())->get(0), 5);
+    CHECK_EQUAL(dynamic_cast<Lst<Int>*>(collection.get())->get(1), 47);
+    CHECK_EQUAL(dynamic_cast<Lst<Int>*>(collection2.get())->get(0), 100);
+
+    CHECK(list->size() == 1);
+    CHECK(dict->size() == 1);
+    CHECK(list2->size() == 1);
+    CHECK(collection->size() == 2);
+    CHECK(collection2->size() == 1);
+
+    tr->promote_to_write();
+    list->remove(0);
+    dict->remove("Foo");
+    tr->verify();
+    tr->commit_and_continue_as_read();
+
+    CHECK_EQUAL(list->size(), 0);
+    CHECK_EQUAL(dict->size(), 0);
+    CHECK_EQUAL(collection->size(), 0);
+    tr->promote_to_write();
+    obj.remove();
+    tr->commit_and_continue_as_read();
+}