From 752ec1669caf5b3edbbb38fcc7cfb889cce61460 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= <jorgen.edelbo@mongodb.com>
Date: Tue, 15 Aug 2023 15:40:55 +0200
Subject: [PATCH] Make exceptions thrown by nested collections more consistent
 (#6875)

---
 src/realm/dictionary.cpp | 22 +++++++++++++++++-----
 src/realm/dictionary.hpp |  1 +
 src/realm/list.hpp       | 12 +++++++++---
 test/test_list.cpp       | 13 +++++++++++--
 4 files changed, 38 insertions(+), 10 deletions(-)

diff --git a/src/realm/dictionary.cpp b/src/realm/dictionary.cpp
index 533dd2c0ec5..998826d57eb 100644
--- a/src/realm/dictionary.cpp
+++ b/src/realm/dictionary.cpp
@@ -114,14 +114,18 @@ bool Dictionary::is_null(size_t ndx) const
 Mixed Dictionary::get_any(size_t ndx) const
 {
     // Note: `size()` calls `update_if_needed()`.
-    CollectionBase::validate_index("get_any()", ndx, size());
+    auto current_size = size();
+    ensure_attached();
+    CollectionBase::validate_index("get_any()", ndx, current_size);
     return do_get(ndx);
 }
 
 std::pair<Mixed, Mixed> Dictionary::get_pair(size_t ndx) const
 {
     // Note: `size()` calls `update_if_needed()`.
-    CollectionBase::validate_index("get_pair()", ndx, size());
+    auto current_size = size();
+    ensure_attached();
+    CollectionBase::validate_index("get_pair()", ndx, current_size);
     return do_get_pair(ndx);
 }
 
@@ -467,6 +471,7 @@ Mixed Dictionary::get(Mixed key) const
     if (auto opt_val = try_get(key)) {
         return *opt_val;
     }
+    ensure_attached();
     throw KeyNotFound("Dictionary::get");
 }
 
@@ -664,9 +669,15 @@ UpdateStatus Dictionary::update_if_needed_with_status() const noexcept
 void Dictionary::ensure_created()
 {
     if (Base::should_update() || !(m_dictionary_top && m_dictionary_top->is_attached())) {
-        if (!init_from_parent(true)) {
-            throw IllegalOperation("This is an ex-dictionary");
-        }
+        init_from_parent(true);
+        ensure_attached();
+    }
+}
+
+void Dictionary::ensure_attached() const
+{
+    if (!m_dictionary_top) {
+        throw IllegalOperation("This is an ex-dictionary");
     }
 }
 
@@ -690,6 +701,7 @@ bool Dictionary::try_erase(Mixed key)
 void Dictionary::erase(Mixed key)
 {
     if (!try_erase(key)) {
+        ensure_attached();
         throw KeyNotFound(util::format("Cannot remove key %1 from dictionary: key not found", key));
     }
 }
diff --git a/src/realm/dictionary.hpp b/src/realm/dictionary.hpp
index 6a25478b9e6..33563961024 100644
--- a/src/realm/dictionary.hpp
+++ b/src/realm/dictionary.hpp
@@ -250,6 +250,7 @@ class Dictionary final : public CollectionBaseImpl<DictionaryBase>, public Colle
     void do_accumulate(size_t* return_ndx, AggregateType& agg) const;
 
     void ensure_created();
+    void ensure_attached() const;
     inline bool update() const
     {
         return update_if_needed_with_status() != UpdateStatus::Detached;
diff --git a/src/realm/list.hpp b/src/realm/list.hpp
index bea5704f2ca..d41b444dcf3 100644
--- a/src/realm/list.hpp
+++ b/src/realm/list.hpp
@@ -480,9 +480,8 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa
     void ensure_created()
     {
         if (Base::should_update() || !(m_tree && m_tree->is_attached())) {
-            if (!init_from_parent(true)) {
-                throw IllegalOperation("This is an ex-list");
-            }
+            init_from_parent(true);
+            ensure_attached();
             Base::update_content_version();
         }
     }
@@ -623,9 +622,16 @@ class Lst<Mixed> final : public CollectionBaseImpl<LstBase>, public CollectionPa
             return Mixed{};
         return value;
     }
+    void ensure_attached() const
+    {
+        if (!m_tree->is_attached()) {
+            throw IllegalOperation("This is an ex-list");
+        }
+    }
     Mixed do_get(size_t ndx, const char* msg) const
     {
         const auto current_size = size();
+        ensure_attached();
         CollectionBase::validate_index(msg, ndx, current_size);
 
         return unresolved_to_null(m_tree->get(ndx));
diff --git a/test/test_list.cpp b/test/test_list.cpp
index 35b5db1219d..61797326ca6 100644
--- a/test/test_list.cpp
+++ b/test/test_list.cpp
@@ -883,20 +883,29 @@ TEST(List_Nested_InMixed)
       ]
     }
     */
+    std::string message;
     CHECK_EQUAL(list2->get(1), Mixed("Hello"));
     tr->promote_to_write();
     list2->remove(1);
     CHECK_EQUAL(dict2->get("Hello"), Mixed("World"));
     obj.set(col_any, Mixed());
     CHECK_EQUAL(dict->size(), 0);
-    CHECK_THROW_ANY(dict->insert("Five", 5)); // This dictionary ceased to be
+    CHECK_THROW_ANY_GET_MESSAGE(dict->insert("Five", 5), message); // This dictionary ceased to be
+    CHECK_EQUAL(message, "This is an ex-dictionary");
+    CHECK_THROW_ANY_GET_MESSAGE(dict->get("Five"), message);
+    CHECK_EQUAL(message, "This is an ex-dictionary");
 
     obj.set_collection(col_any, CollectionType::List);
     auto list3 = obj.get_list_ptr<Mixed>(col_any);
     list3->add(5);
     obj.set(col_any, Mixed());
     CHECK_EQUAL(list3->size(), 0);
-    CHECK_THROW_ANY(list3->add(42));
+    CHECK_THROW_ANY_GET_MESSAGE(list3->add(42), message);
+    CHECK_EQUAL(message, "This is an ex-list");
+    CHECK_THROW_ANY_GET_MESSAGE(list3->insert(5, 42), message);
+    CHECK_EQUAL(message, "This is an ex-list");
+    CHECK_THROW_ANY_GET_MESSAGE(list3->get(5), message);
+    CHECK_EQUAL(message, "This is an ex-list");
     tr->verify();
     obj.set_json(col_any,
                  "[{\"Seven\":7, \"Six\":6}, \"Hello\", {\"Points\": [1.25, 4.5, 6.75], \"Hello\": \"World\"}]");