From 9eaf6c348960095fc1c42d8a163aa22a19fcc42c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= <jorgen.edelbo@mongodb.com> Date: Thu, 14 Mar 2024 13:57:51 +0100 Subject: [PATCH] Support assigning nested collections via templated API --- src/realm/collection_parent.cpp | 2 +- src/realm/object-store/dictionary.hpp | 49 +++++-- .../impl/object_accessor_impl.hpp | 6 + src/realm/object-store/list.hpp | 126 +++++++++++++----- src/realm/object-store/object_accessor.hpp | 20 +++ test/object-store/dictionary.cpp | 119 +++++++++++++++++ 6 files changed, 274 insertions(+), 48 deletions(-) diff --git a/src/realm/collection_parent.cpp b/src/realm/collection_parent.cpp index dc3ad806bc3..1677d57b910 100644 --- a/src/realm/collection_parent.cpp +++ b/src/realm/collection_parent.cpp @@ -75,7 +75,7 @@ CollectionParent::~CollectionParent() {} void CollectionParent::check_level() const { - if (m_level + 1 > s_max_level) { + if (size_t(m_level) + 1 > s_max_level) { throw LogicError(ErrorCodes::LimitExceeded, "Max nesting level reached"); } } diff --git a/src/realm/object-store/dictionary.hpp b/src/realm/object-store/dictionary.hpp index 8f0fcf52f1a..6ead9d7f6bc 100644 --- a/src/realm/object-store/dictionary.hpp +++ b/src/realm/object-store/dictionary.hpp @@ -142,6 +142,12 @@ class Dictionary : public object_store::Collection { Obj get_object(StringData key) const; }; +} // namespace object_store +} // namespace realm + +#include <realm/object-store/list.hpp> + +namespace realm::object_store { template <typename Fn> auto Dictionary::dispatch(Fn&& fn) const @@ -191,9 +197,27 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po ctx.template unbox<Obj>(value, policy, obj_key); return; } - dispatch([&](auto t) { - this->insert(key, ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); - }); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox<Mixed>(value, policy); + if (new_val.is_type(type_Dictionary)) { + insert_collection(key, CollectionType::Dictionary); + auto dict = get_dictionary(key); + dict.assign(ctx, value, policy); + return; + } + if (new_val.is_type(type_List)) { + insert_collection(key, CollectionType::List); + auto list = get_list(key); + list.assign(ctx, value, policy); + return; + } + this->insert(key, new_val); + } + else { + dispatch([&](auto t) { + this->insert(key, ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); + }); + } } template <typename Context> @@ -220,20 +244,19 @@ void Dictionary::assign(Context& ctx, T&& values, CreatePolicy policy) ctx.enumerate_dictionary(values, [&](StringData key, auto&& value) { if (policy.diff) { - util::Optional<Mixed> old_value = dict().try_get(key); - auto new_value = ctx.template unbox<Mixed>(value); - if (!old_value || *old_value != new_value) { - dict().insert(key, new_value); + Mixed new_value = ctx.template unbox<Mixed>(value); + if (!new_value.is_type(type_Dictionary, type_List)) { + util::Optional<Mixed> old_value = dict().try_get(key); + if (!old_value || *old_value != new_value) { + dict().insert(key, new_value); + } + return; } } - else { - this->insert(ctx, key, value, policy); - } + this->insert(ctx, key, value, policy); }); } -} // namespace object_store -} // namespace realm - +} // namespace realm::object_store #endif /* REALM_OS_DICTIONARY_HPP */ diff --git a/src/realm/object-store/impl/object_accessor_impl.hpp b/src/realm/object-store/impl/object_accessor_impl.hpp index 8a9eacccf0e..4ea5e3a8f0b 100644 --- a/src/realm/object-store/impl/object_accessor_impl.hpp +++ b/src/realm/object-store/impl/object_accessor_impl.hpp @@ -408,6 +408,12 @@ inline Mixed CppContext::unbox(std::any& v, CreatePolicy, ObjKey) const else if (this_type == typeid(UUID)) { return Mixed(util::any_cast<UUID>(v)); } + else if (this_type == typeid(AnyDict)) { + return Mixed(0, CollectionType::Dictionary); + } + else if (this_type == typeid(AnyVector)) { + return Mixed(0, CollectionType::List); + } } return Mixed{}; } diff --git a/src/realm/object-store/list.hpp b/src/realm/object-store/list.hpp index 020f3414fee..c7724c8d40f 100644 --- a/src/realm/object-store/list.hpp +++ b/src/realm/object-store/list.hpp @@ -132,7 +132,11 @@ class List : public object_store::Collection { friend struct std::hash<List>; }; +} // namespace realm + +#include <realm/object-store/dictionary.hpp> +namespace realm { template <> Obj List::get(size_t row_ndx) const; @@ -183,15 +187,7 @@ size_t List::find(Context& ctx, T&& value) const template <typename T, typename Context> void List::add(Context& ctx, T&& value, CreatePolicy policy) { - if (m_is_embedded) { - validate_embedded(ctx, value, policy); - auto key = as<Obj>().create_and_insert_linked_object(size()).get_key(); - ctx.template unbox<Obj>(value, policy, key); - return; - } - dispatch([&](auto t) { - this->add(ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); - }); + this->insert(ctx, size(), std::move(value), policy); } template <typename T, typename Context> @@ -203,9 +199,28 @@ void List::insert(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) ctx.template unbox<Obj>(value, policy, key); return; } - dispatch([&](auto t) { - this->insert(list_ndx, ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); - }); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox<Mixed>(value, policy); + if (new_val.is_type(type_Dictionary)) { + insert_collection(list_ndx, CollectionType::Dictionary); + auto dict = get_dictionary(list_ndx); + dict.assign(ctx, value, policy); + return; + } + if (new_val.is_type(type_List)) { + insert_collection(list_ndx, CollectionType::List); + auto list = get_list(list_ndx); + list.assign(ctx, value, policy); + return; + } + + this->insert(list_ndx, new_val); + } + else { + dispatch([&](auto t) { + this->insert(list_ndx, ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); + }); + } } template <typename T, typename Context> @@ -219,35 +234,75 @@ void List::set(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) ctx.template unbox<Obj>(value, policy, key); return; } - dispatch([&](auto t) { - this->set(list_ndx, ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); - }); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox<Mixed>(value, policy); + if (new_val.is_type(type_Dictionary)) { + set_collection(list_ndx, CollectionType::Dictionary); + auto dict = get_dictionary(list_ndx); + dict.assign(ctx, value, policy); + return; + } + if (new_val.is_type(type_List)) { + set_collection(list_ndx, CollectionType::List); + auto list = get_list(list_ndx); + list.assign(ctx, value, policy); + return; + } + + this->set(list_ndx, new_val); + } + else { + dispatch([&](auto t) { + this->set(list_ndx, ctx.template unbox<std::decay_t<decltype(*t)>>(value, policy)); + }); + } } template <typename T, typename Context> -void List::set_if_different(Context& ctx, size_t row_ndx, T&& value, CreatePolicy policy) +void List::set_if_different(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) { if (m_is_embedded) { validate_embedded(ctx, value, policy); - auto key = policy.diff ? this->get<Obj>(row_ndx) : as<Obj>().create_and_set_linked_object(row_ndx); + auto key = policy.diff ? this->get<Obj>(list_ndx) : as<Obj>().create_and_set_linked_object(list_ndx); ctx.template unbox<Obj>(value, policy, key.get_key()); return; } - dispatch([&](auto t) { - using U = std::decay_t<decltype(*t)>; - if constexpr (std::is_same_v<U, Obj>) { - auto old_value = this->get<U>(row_ndx); - auto new_value = ctx.template unbox<U>(value, policy, old_value.get_key()); - if (new_value.get_key() != old_value.get_key()) - this->set(row_ndx, new_value); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox<Mixed>(value, policy); + if (new_val.is_type(type_Dictionary)) { + set_collection(list_ndx, CollectionType::Dictionary); + auto dict = get_dictionary(list_ndx); + dict.assign(ctx, value, policy); + return; } - else { - auto old_value = this->get<U>(row_ndx); - auto new_value = ctx.template unbox<U>(value, policy); - if (old_value != new_value) - this->set(row_ndx, new_value); + if (new_val.is_type(type_List)) { + set_collection(list_ndx, CollectionType::List); + auto list = get_list(list_ndx); + list.assign(ctx, value, policy); + return; } - }); + Mixed old_value = this->get<Mixed>(list_ndx); + Mixed new_value = ctx.template unbox<Mixed>(value, policy); + if (old_value != new_value) + this->set(list_ndx, new_value); + } + else { + dispatch([&](auto t) { + using U = std::decay_t<decltype(*t)>; + if constexpr (std::is_same_v<U, Obj>) { + auto old_value = this->get<U>(list_ndx); + auto new_value = ctx.template unbox<U>(value, policy, old_value.get_key()); + if (new_value.get_key() != old_value.get_key()) + this->set(list_ndx, new_value); + } + else { + auto old_value = this->get<U>(list_ndx); + auto new_value = ctx.template unbox<U>(value, policy); + if (old_value != new_value) + this->set(list_ndx, new_value); + } + }); + } } template <typename T, typename Context> @@ -267,12 +322,15 @@ void List::assign(Context& ctx, T&& values, CreatePolicy policy) size_t sz = size(); size_t index = 0; ctx.enumerate_collection(values, [&](auto&& element) { - if (index >= sz) + if (index >= sz) { this->add(ctx, element, policy); - else if (policy.diff) + } + else { + // If index is within legal range, policy.diff must be true - + // otherwise the list would have been cleared + REALM_ASSERT(policy.diff); this->set_if_different(ctx, index, element, policy); - else - this->set(ctx, index, element, policy); + } index++; }); while (index < sz) diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index 5b6688e5fb2..2435e29f529 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -170,6 +170,26 @@ void Object::set_property_value_impl(ContextType& ctx, const Property& property, return; } + if (property.type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox<Mixed>(value, policy); + if (new_val.is_type(type_Dictionary)) { + ContextType child_ctx(ctx, m_obj, property); + m_obj.set_collection(col, CollectionType::Dictionary); + object_store::Dictionary dict(m_realm, m_obj, col); + dict.assign(child_ctx, value, policy); + ctx.did_change(); + return; + } + if (new_val.is_type(type_List)) { + ContextType child_ctx(ctx, m_obj, property); + m_obj.set_collection(col, CollectionType::List); + List list(m_realm, m_obj, col); + list.assign(child_ctx, value, policy); + ctx.did_change(); + return; + } + } + ValueUpdater<ValueType, ContextType> updater{ctx, property, value, m_obj, col, policy, is_default}; switch_on_type(property.type, updater); ctx.did_change(); diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index 0ae18396843..ca096d4e140 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -1348,6 +1348,125 @@ TEST_CASE("dictionary nullify", "[dictionary]") { // r->read_group().to_json(std::cout); } +TEST_CASE("nested collection set by Object::create", "[dictionary]") { + InMemoryTestFile config; + config.schema = Schema{ + {"DictionaryObject", + { + {"_id", PropertyType::Int, Property::IsPrimary{true}}, + {"any", PropertyType::Mixed | PropertyType::Nullable}, + }}, + }; + + auto r = Realm::get_shared_realm(config); + CppContext ctx(r); + + Any value{AnyDict{{"_id", INT64_C(5)}, + {"any", AnyDict{{"0", Any(AnyDict{{"zero", INT64_C(0)}})}, + {"1", Any(AnyVector{std::string("one"), INT64_C(1)})}, + {"2", Any(AnyDict{{"two", INT64_C(2)}, {"three", INT64_C(3)}})}}}}}; + r->begin_transaction(); + auto obj = Object::create(ctx, r, *r->schema().find("DictionaryObject"), value); + r->commit_transaction(); + + object_store::Dictionary dict(obj, r->schema().find("DictionaryObject")->property_for_name("any")); + auto dict0 = dict.get_dictionary("0"); + auto list1 = dict.get_list("1"); + auto dict2 = dict.get_dictionary("2"); + CHECK(dict0.get_any("zero") == Mixed(0)); + CHECK(list1.get_any(0) == Mixed("one")); + CHECK(list1.get_any(1) == Mixed(1)); + CHECK(dict2.get_any("two") == Mixed(2)); + CHECK(dict2.get_any("three") == Mixed(3)); + + SECTION("modify list only") { + Any new_value{ + AnyDict{{"_id", INT64_C(5)}, {"any", AnyDict{{"1", Any(AnyVector{std::string("seven"), INT64_C(7)})}}}}}; + + r->begin_transaction(); + Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); + r->commit_transaction(); + CHECK(list1.get_any(0) == Mixed("seven")); + CHECK(list1.get_any(1) == Mixed(7)); + } + + SECTION("update with less data") { + Any new_value{ + AnyDict{{"_id", INT64_C(5)}, {"any", AnyDict{{"1", Any(AnyVector{std::string("seven"), INT64_C(7)})}}}}}; + + r->begin_transaction(); + Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateAll); + r->commit_transaction(); + CHECK(dict.size() == 1); + list1 = dict.get_list("1"); + CHECK(list1.get_any(0) == Mixed("seven")); + CHECK(list1.get_any(1) == Mixed(7)); + } + + SECTION("replace list with dictionary") { + Any new_value{AnyDict{{"_id", INT64_C(5)}, {"any", AnyDict{{"1", Any(AnyDict{{"seven", INT64_C(7)}})}}}}}; + + r->begin_transaction(); + Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); + r->commit_transaction(); + auto dict1 = dict.get_dictionary("1"); + CHECK(dict1.get_any("seven") == Mixed(7)); + } + + SECTION("replace dictionary with list on top level") { + value = Any{AnyDict{ + {"_id", INT64_C(5)}, + {"any", AnyVector{Any(AnyDict{{"zero", INT64_C(0)}}), Any(AnyVector{std::string("one"), INT64_C(1)}), + Any(AnyDict{{"two", INT64_C(2)}, {"three", INT64_C(3)}})}}}}; + + r->begin_transaction(); + Object::create(ctx, r, *r->schema().find("DictionaryObject"), value, CreatePolicy::UpdateModified); + r->commit_transaction(); + List list(obj, r->schema().find("DictionaryObject")->property_for_name("any")); + dict0 = list.get_dictionary(0); + CHECK(dict0.get_any("zero") == Mixed(0)); + + SECTION("modify dictionary only") { + Any new_value{ + AnyDict{{"_id", INT64_C(5)}, {"any", AnyVector{Any(AnyDict{{std::string("seven"), INT64_C(7)}})}}}}; + + r->begin_transaction(); + Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); + r->commit_transaction(); + CHECK(dict0.get_any("seven") == Mixed(7)); + } + + SECTION("replace dictionary with list") { + Any new_value{ + AnyDict{{"_id", INT64_C(5)}, {"any", AnyVector{Any(AnyVector{std::string("seven"), INT64_C(7)})}}}}; + + r->begin_transaction(); + Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); + r->commit_transaction(); + auto list0 = list.get_list(0); + CHECK(list0.get_any(0) == Mixed("seven")); + CHECK(list0.get_any(1) == Mixed(7)); + } + + SECTION("assign dictionary directly to nested list") { + r->begin_transaction(); + list.set(ctx, 1, Any(AnyDict{{std::string("ten"), INT64_C(10)}})); + r->commit_transaction(); + auto dict0 = list.get_dictionary(1); + CHECK(dict0.get_any("ten") == Mixed(10)); + } + + SECTION("assign list directly to nested list") { + r->begin_transaction(); + list.set(ctx, 0, Any(AnyVector{std::string("ten"), INT64_C(10)})); + r->commit_transaction(); + auto list0 = list.get_list(0); + CHECK(list0.get_any(0) == Mixed("ten")); + CHECK(list0.get_any(1) == Mixed(10)); + } + } +} + TEST_CASE("dictionary assign", "[dictionary]") { InMemoryTestFile config; config.schema = Schema{