From ca1eeb69a48926977abcd7f151577d4af558b7ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Thu, 14 Mar 2024 10:38:48 +0100 Subject: [PATCH 01/12] Small optimization of CollectionBase::do_init_from_parent --- src/realm/collection.cpp | 4 ---- src/realm/list.hpp | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/src/realm/collection.cpp b/src/realm/collection.cpp index 99553ec53ee..03e9bd4325b 100644 --- a/src/realm/collection.cpp +++ b/src/realm/collection.cpp @@ -243,10 +243,6 @@ UpdateStatus CollectionBase::do_init_from_parent(BPlusTreeBase* tree, ref_type r tree->init_from_ref(ref); } else { - if (tree->init_from_parent()) { - // All is well - return UpdateStatus::Updated; - } if (!allow_create) { tree->detach(); return UpdateStatus::Detached; diff --git a/src/realm/list.hpp b/src/realm/list.hpp index fec6505c030..989c7597e53 100644 --- a/src/realm/list.hpp +++ b/src/realm/list.hpp @@ -260,7 +260,7 @@ class Lst final : public CollectionBaseImpl { m_tree->set_parent(const_cast(parent), 0); } Base::update_content_version(); - return do_init_from_parent(m_tree.get(), 0, allow_create); + return do_init_from_parent(m_tree.get(), Base::get_collection_ref(), allow_create); } template From 9eaf6c348960095fc1c42d8a163aa22a19fcc42c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Thu, 14 Mar 2024 13:57:51 +0100 Subject: [PATCH 02/12] 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 + +namespace realm::object_store { template auto Dictionary::dispatch(Fn&& fn) const @@ -191,9 +197,27 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po ctx.template unbox(value, policy, obj_key); return; } - dispatch([&](auto t) { - this->insert(key, ctx.template unbox>(value, policy)); - }); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox(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>(value, policy)); + }); + } } template @@ -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 old_value = dict().try_get(key); - auto new_value = ctx.template unbox(value); - if (!old_value || *old_value != new_value) { - dict().insert(key, new_value); + Mixed new_value = ctx.template unbox(value); + if (!new_value.is_type(type_Dictionary, type_List)) { + util::Optional 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(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; }; +} // namespace realm + +#include +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 void List::add(Context& ctx, T&& value, CreatePolicy policy) { - if (m_is_embedded) { - validate_embedded(ctx, value, policy); - auto key = as().create_and_insert_linked_object(size()).get_key(); - ctx.template unbox(value, policy, key); - return; - } - dispatch([&](auto t) { - this->add(ctx.template unbox>(value, policy)); - }); + this->insert(ctx, size(), std::move(value), policy); } template @@ -203,9 +199,28 @@ void List::insert(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) ctx.template unbox(value, policy, key); return; } - dispatch([&](auto t) { - this->insert(list_ndx, ctx.template unbox>(value, policy)); - }); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox(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>(value, policy)); + }); + } } template @@ -219,35 +234,75 @@ void List::set(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) ctx.template unbox(value, policy, key); return; } - dispatch([&](auto t) { - this->set(list_ndx, ctx.template unbox>(value, policy)); - }); + if (m_type == PropertyType::Mixed) { + Mixed new_val = ctx.template unbox(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>(value, policy)); + }); + } } template -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(row_ndx) : as().create_and_set_linked_object(row_ndx); + auto key = policy.diff ? this->get(list_ndx) : as().create_and_set_linked_object(list_ndx); ctx.template unbox(value, policy, key.get_key()); return; } - dispatch([&](auto t) { - using U = std::decay_t; - if constexpr (std::is_same_v) { - auto old_value = this->get(row_ndx); - auto new_value = ctx.template unbox(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(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(row_ndx); - auto new_value = ctx.template unbox(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(list_ndx); + Mixed new_value = ctx.template unbox(value, policy); + if (old_value != new_value) + this->set(list_ndx, new_value); + } + else { + dispatch([&](auto t) { + using U = std::decay_t; + if constexpr (std::is_same_v) { + auto old_value = this->get(list_ndx); + auto new_value = ctx.template unbox(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(list_ndx); + auto new_value = ctx.template unbox(value, policy); + if (old_value != new_value) + this->set(list_ndx, new_value); + } + }); + } } template @@ -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(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 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{ From 768ab8aa7f15a2569149392340c637fc60302fc9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 15 Mar 2024 13:20:05 +0100 Subject: [PATCH 03/12] Support getting nested collections via templated API --- src/realm/object-store/collection.hpp | 7 +++++++ src/realm/object-store/dictionary.hpp | 18 +++++++++++++++--- src/realm/object-store/list.hpp | 18 +++++++++++++++--- src/realm/object-store/object_accessor.hpp | 12 ++++++++++-- test/object-store/dictionary.cpp | 13 +++++++------ test/object-store/primitive_list.cpp | 16 ++++++++++++---- 6 files changed, 66 insertions(+), 18 deletions(-) diff --git a/src/realm/object-store/collection.hpp b/src/realm/object-store/collection.hpp index e809ee2bab1..5c0765f7c1b 100644 --- a/src/realm/object-store/collection.hpp +++ b/src/realm/object-store/collection.hpp @@ -156,6 +156,13 @@ void Collection::validate_embedded(Context& ctx, T&& value, CreatePolicy policy) throw IllegalOperation(util::format("Cannot add an existing managed embedded object to a %1.", type_name())); } +// Dummy implementation to satisfy StringifyingContext +inline std::ostream& operator<<(std::ostream& out, const Collection&) +{ + return out; +} + + } // namespace object_store } // namespace realm diff --git a/src/realm/object-store/dictionary.hpp b/src/realm/object-store/dictionary.hpp index 6ead9d7f6bc..717501d6531 100644 --- a/src/realm/object-store/dictionary.hpp +++ b/src/realm/object-store/dictionary.hpp @@ -223,9 +223,21 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po template auto Dictionary::get(Context& ctx, StringData key) const { - return dispatch([&](auto t) { - return ctx.box(this->get>(key)); - }); + if (m_type == PropertyType::Mixed) { + Mixed value = dict().get(key); + if (value.is_type(type_Dictionary)) { + return ctx.box(get_dictionary(key)); + } + if (value.is_type(type_List)) { + return ctx.box(get_list(key)); + } + return ctx.box(value); + } + else { + return dispatch([&](auto t) { + return ctx.box(this->get>(key)); + }); + } } template diff --git a/src/realm/object-store/list.hpp b/src/realm/object-store/list.hpp index c7724c8d40f..821fc62bc4e 100644 --- a/src/realm/object-store/list.hpp +++ b/src/realm/object-store/list.hpp @@ -171,9 +171,21 @@ auto List::dispatch(Fn&& fn) const template auto List::get(Context& ctx, size_t row_ndx) const { - return dispatch([&](auto t) { - return ctx.box(this->get>(row_ndx)); - }); + if (m_type == PropertyType::Mixed) { + Mixed value = get(row_ndx); + if (value.is_type(type_Dictionary)) { + return ctx.box(get_dictionary(row_ndx)); + } + if (value.is_type(type_List)) { + return ctx.box(get_list(row_ndx)); + } + return ctx.box(value); + } + else { + return dispatch([&](auto t) { + return ctx.box(this->get>(row_ndx)); + }); + } } template diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index 2435e29f529..6f13ea55a6b 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -234,8 +234,16 @@ ValueType Object::get_property_value_impl(ContextType& ctx, const Property& prop case PropertyType::UUID: return is_nullable(property.type) ? ctx.box(m_obj.get>(column)) : ctx.box(m_obj.get(column)); - case PropertyType::Mixed: - return ctx.box(m_obj.get(column)); + case PropertyType::Mixed: { + Mixed value = m_obj.get(column); + if (value.is_type(type_Dictionary)) { + return ctx.box(object_store::Dictionary(m_realm, m_obj, column)); + } + if (value.is_type(type_List)) { + return ctx.box(List(m_realm, m_obj, column)); + } + return ctx.box(value); + } case PropertyType::Object: { auto linkObjectSchema = m_realm->schema().find(property.object_type); auto linked = const_cast(m_obj).get_linked_object(column); diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index ca096d4e140..58a146e88f8 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -1369,9 +1369,10 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { 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 dict = util::any_cast(obj.get_property_value(ctx, "any")); + + auto dict0 = util::any_cast(dict.get(ctx, "0")); + auto list1 = util::any_cast(dict.get(ctx, "1")); auto dict2 = dict.get_dictionary("2"); CHECK(dict0.get_any("zero") == Mixed(0)); CHECK(list1.get_any(0) == Mixed("one")); @@ -1422,8 +1423,8 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { 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); + auto list = util::any_cast(obj.get_property_value(ctx, "any")); + dict0 = util::any_cast(list.get(ctx, 0)); CHECK(dict0.get_any("zero") == Mixed(0)); SECTION("modify dictionary only") { @@ -1443,7 +1444,7 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { r->begin_transaction(); Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); r->commit_transaction(); - auto list0 = list.get_list(0); + auto list0 = util::any_cast(list.get(ctx, 0)); CHECK(list0.get_any(0) == Mixed("seven")); CHECK(list0.get_any(1) == Mixed(7)); } diff --git a/test/object-store/primitive_list.cpp b/test/object-store/primitive_list.cpp index 9bab1b1deea..4bb893cf087 100644 --- a/test/object-store/primitive_list.cpp +++ b/test/object-store/primitive_list.cpp @@ -808,24 +808,32 @@ TEMPLATE_TEST_CASE("primitive list", "[primitives]", cf::MixedVal, cf::Int, cf:: if (!util::EventLoop::has_implementation()) return; - SyncServer server; - SyncTestFile sync_config(server, "shared"); - sync_config.schema = config.schema; + TestSyncManager init_sync_manager({}, {false}); + auto& server = init_sync_manager.sync_server(); + SyncTestFile sync_config(init_sync_manager, "shared"); + sync_config.schema = Schema{ + {"object", + {{"value", PropertyType::Array | TestType::property_type}, + {"_id", PropertyType::Int, Property::IsPrimary{true}}}}, + }; sync_config.schema_version = 0; + server.start(); { auto r = Realm::get_shared_realm(sync_config); r->begin_transaction(); CppContext ctx(r); - auto obj = Object::create(ctx, r, *r->schema().find("object"), std::any(AnyDict{})); + auto obj = Object::create(ctx, r, *r->schema().find("object"), std::any(AnyDict{{"_id", INT64_C(5)}})); auto list = util::any_cast(obj.get_property_value(ctx, "value")); list.add(static_cast(values[0])); r->commit_transaction(); wait_for_upload(*r); + wait_for_download(*r); } + std::this_thread::sleep_for(std::chrono::milliseconds(5)); util::File::remove(sync_config.path); { From 05d453ebd46c4411c16e351eab0fd19627836dcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Mon, 18 Mar 2024 10:21:43 +0100 Subject: [PATCH 04/12] Update release notes --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41c0f0b36ff..3cb2408de7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) -* None. +* Support assigning nested collections via templated API (PR [#7478](https://github.com/realm/realm-core/pull/7478)) ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) From 0dd6887ffe7d288c2b4d959c9e6761f8eb31a021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 5 Apr 2024 14:20:34 +0200 Subject: [PATCH 05/12] Assigning unmanaged object to Mixed property --- external/catch | 2 +- src/realm/object-store/dictionary.hpp | 60 ++++++++++++++----- .../impl/object_accessor_impl.hpp | 18 +++++- src/realm/object-store/list.hpp | 9 ++- src/realm/object-store/object_accessor.hpp | 16 ++++- test/object-store/dictionary.cpp | 36 ++++++++--- 6 files changed, 113 insertions(+), 28 deletions(-) diff --git a/external/catch b/external/catch index 05e10dfccc2..8ac8190e494 160000 --- a/external/catch +++ b/external/catch @@ -1 +1 @@ -Subproject commit 05e10dfccc28c7f973727c54f850237d07d5e10f +Subproject commit 8ac8190e494a381072c89f5e161b92a08d98b37b diff --git a/src/realm/object-store/dictionary.hpp b/src/realm/object-store/dictionary.hpp index 717501d6531..24b57ea4594 100644 --- a/src/realm/object-store/dictionary.hpp +++ b/src/realm/object-store/dictionary.hpp @@ -184,6 +184,41 @@ inline Obj Dictionary::get(StringData key) const return get_object(key); } +namespace { +template +struct ValueUpdater { + ContextType& ctx; + ValueType& value; + Dictionary& dict; + StringData key; + CreatePolicy policy; + + template + void operator()(T*) + { + bool attr_changed = !policy.diff; + auto new_val = ctx.template unbox(value, policy); + + if (!attr_changed) { + util::Optional old_val = dict.try_get_any(key); + if (old_val) { + if constexpr (std::is_same::value) { + attr_changed = !new_val.is_same_type(*old_val); + } + + attr_changed = attr_changed || new_val != *old_val; + } + else { + attr_changed = true; + } + } + + if (attr_changed) + dict.insert(key, new_val); + } +}; +} // namespace + template void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy policy) { @@ -211,12 +246,19 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po list.assign(ctx, value, policy); return; } - this->insert(key, new_val); + + bool attr_changed = !policy.diff; + if (!attr_changed) { + util::Optional old_value = dict().try_get(key); + attr_changed = !old_value || !new_val.is_same_type(*old_value) || new_val != *old_value; + } + + if (attr_changed) + this->insert(key, new_val); } else { - dispatch([&](auto t) { - this->insert(key, ctx.template unbox>(value, policy)); - }); + ValueUpdater updater{ctx, value, *this, key, policy}; + dispatch(updater); } } @@ -255,16 +297,6 @@ void Dictionary::assign(Context& ctx, T&& values, CreatePolicy policy) remove_all(); ctx.enumerate_dictionary(values, [&](StringData key, auto&& value) { - if (policy.diff) { - Mixed new_value = ctx.template unbox(value); - if (!new_value.is_type(type_Dictionary, type_List)) { - util::Optional old_value = dict().try_get(key); - if (!old_value || *old_value != new_value) { - dict().insert(key, new_value); - } - return; - } - } this->insert(ctx, key, value, policy); }); } diff --git a/src/realm/object-store/impl/object_accessor_impl.hpp b/src/realm/object-store/impl/object_accessor_impl.hpp index 4ea5e3a8f0b..c7371589335 100644 --- a/src/realm/object-store/impl/object_accessor_impl.hpp +++ b/src/realm/object-store/impl/object_accessor_impl.hpp @@ -31,6 +31,11 @@ namespace realm { using AnyDict = std::map; using AnyVector = std::vector; +struct UnmanagedObject { + std::string object_type; + std::any properties; +}; + // An object accessor context which can be used to create and access objects // using std::any as the type-erased value type. In addition, this serves as // the reference implementation of an accessor context that must be implemented @@ -370,7 +375,7 @@ inline util::Optional CppContext::unbox(std::any& v, CreatePolicy, ObjKey) } template <> -inline Mixed CppContext::unbox(std::any& v, CreatePolicy, ObjKey) const +inline Mixed CppContext::unbox(std::any& v, CreatePolicy policy, ObjKey) const { if (v.has_value()) { const std::type_info& this_type{v.type()}; @@ -414,6 +419,17 @@ inline Mixed CppContext::unbox(std::any& v, CreatePolicy, ObjKey) const else if (this_type == typeid(AnyVector)) { return Mixed(0, CollectionType::List); } + else if (this_type == typeid(UnmanagedObject)) { + UnmanagedObject unmanaged_obj = util::any_cast(v); + auto os = realm->schema().find(unmanaged_obj.object_type); + CppContext child_ctx(realm, &*os); + auto obj = child_ctx.unbox(unmanaged_obj.properties, policy, ObjKey()); + return Mixed(obj); + } + else if (this_type == typeid(Obj)) { + Obj obj = util::any_cast(v); + return Mixed(obj); + } } return Mixed{}; } diff --git a/src/realm/object-store/list.hpp b/src/realm/object-store/list.hpp index 821fc62bc4e..08de6ed1f82 100644 --- a/src/realm/object-store/list.hpp +++ b/src/realm/object-store/list.hpp @@ -295,7 +295,7 @@ void List::set_if_different(Context& ctx, size_t list_ndx, T&& value, CreatePoli } Mixed old_value = this->get(list_ndx); Mixed new_value = ctx.template unbox(value, policy); - if (old_value != new_value) + if (!new_value.is_same_type(old_value) || old_value != new_value) this->set(list_ndx, new_value); } else { @@ -308,9 +308,14 @@ void List::set_if_different(Context& ctx, size_t list_ndx, T&& value, CreatePoli this->set(list_ndx, new_value); } else { + bool attr_changed = false; auto old_value = this->get(list_ndx); auto new_value = ctx.template unbox(value, policy); - if (old_value != new_value) + if constexpr (std::is_same_v) { + attr_changed = !new_value.is_same_type(old_value); + } + attr_changed = attr_changed || old_value != new_value; + if (attr_changed) this->set(list_ndx, new_value); } }); diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index 6f13ea55a6b..277f58bb43f 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -188,10 +188,20 @@ void Object::set_property_value_impl(ContextType& ctx, const Property& property, ctx.did_change(); return; } - } + bool attr_changed = !policy.diff; + + if (!attr_changed) { + auto old_val = m_obj.get(col); + attr_changed = !new_val.is_same_type(old_val) || new_val != old_val; + } - ValueUpdater updater{ctx, property, value, m_obj, col, policy, is_default}; - switch_on_type(property.type, updater); + if (attr_changed) + m_obj.set(col, new_val, is_default); + } + else { + ValueUpdater 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 58a146e88f8..cc6be73beb4 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -1350,13 +1350,16 @@ TEST_CASE("dictionary nullify", "[dictionary]") { 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}, - }}, - }; + config.schema = Schema{{"DictionaryObject", + { + {"_id", PropertyType::Int, Property::IsPrimary{true}}, + {"any", PropertyType::Mixed | PropertyType::Nullable}, + }}, + {"StringObject", + { + {"_id", PropertyType::Int, Property::IsPrimary{true}}, + {"string", PropertyType::String}, + }}}; auto r = Realm::get_shared_realm(config); CppContext ctx(r); @@ -1365,8 +1368,13 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { {"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)}})}}}}}; + Any value2{AnyDict{ + {"_id", INT64_C(6)}, + {"any", UnmanagedObject{"StringObject", AnyDict{{"_id", INT64_C(2)}, {"string", std::string("hello")}}}}}}; + r->begin_transaction(); auto obj = Object::create(ctx, r, *r->schema().find("DictionaryObject"), value); + Obj obj1 = Object::create(ctx, r, *r->schema().find("DictionaryObject"), value2).get_obj(); r->commit_transaction(); auto dict = util::any_cast(obj.get_property_value(ctx, "any")); @@ -1391,6 +1399,20 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { CHECK(list1.get_any(1) == Mixed(7)); } + SECTION("assign managed object") { + r->begin_transaction(); + Any godbye{AnyDict{{"_id", INT64_C(3)}, {"string", std::string("godbye")}}}; + Obj new_obj = Object::create(ctx, r, *r->schema().find("StringObject"), godbye).get_obj(); + + Any new_value{AnyDict{{"_id", INT64_C(6)}, {"any", new_obj}}}; + + Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); + r->commit_transaction(); + auto link = obj1.get_any("any"); + auto linked_obj = r->read_group().get_object(link.get_link()); + CHECK(linked_obj.get("string") == "godbye"); + } + SECTION("update with less data") { Any new_value{ AnyDict{{"_id", INT64_C(5)}, {"any", AnyDict{{"1", Any(AnyVector{std::string("seven"), INT64_C(7)})}}}}}; From 0039efcae46026dd8cfa5149796a42dc15df8b1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 5 Apr 2024 15:03:03 +0200 Subject: [PATCH 06/12] Move test --- test/object-store/dictionary.cpp | 36 +++++----------------- test/object-store/object.cpp | 51 ++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+), 29 deletions(-) diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index cc6be73beb4..58a146e88f8 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -1350,16 +1350,13 @@ TEST_CASE("dictionary nullify", "[dictionary]") { 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}, - }}, - {"StringObject", - { - {"_id", PropertyType::Int, Property::IsPrimary{true}}, - {"string", PropertyType::String}, - }}}; + 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); @@ -1368,13 +1365,8 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { {"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)}})}}}}}; - Any value2{AnyDict{ - {"_id", INT64_C(6)}, - {"any", UnmanagedObject{"StringObject", AnyDict{{"_id", INT64_C(2)}, {"string", std::string("hello")}}}}}}; - r->begin_transaction(); auto obj = Object::create(ctx, r, *r->schema().find("DictionaryObject"), value); - Obj obj1 = Object::create(ctx, r, *r->schema().find("DictionaryObject"), value2).get_obj(); r->commit_transaction(); auto dict = util::any_cast(obj.get_property_value(ctx, "any")); @@ -1399,20 +1391,6 @@ TEST_CASE("nested collection set by Object::create", "[dictionary]") { CHECK(list1.get_any(1) == Mixed(7)); } - SECTION("assign managed object") { - r->begin_transaction(); - Any godbye{AnyDict{{"_id", INT64_C(3)}, {"string", std::string("godbye")}}}; - Obj new_obj = Object::create(ctx, r, *r->schema().find("StringObject"), godbye).get_obj(); - - Any new_value{AnyDict{{"_id", INT64_C(6)}, {"any", new_obj}}}; - - Object::create(ctx, r, *r->schema().find("DictionaryObject"), new_value, CreatePolicy::UpdateModified); - r->commit_transaction(); - auto link = obj1.get_any("any"); - auto linked_obj = r->read_group().get_object(link.get_link()); - CHECK(linked_obj.get("string") == "godbye"); - } - SECTION("update with less data") { Any new_value{ AnyDict{{"_id", INT64_C(5)}, {"any", AnyDict{{"1", Any(AnyVector{std::string("seven"), INT64_C(7)})}}}}}; diff --git a/test/object-store/object.cpp b/test/object-store/object.cpp index ea779201724..5929ce51d10 100644 --- a/test/object-store/object.cpp +++ b/test/object-store/object.cpp @@ -2322,6 +2322,57 @@ TEST_CASE("Embedded Object") { } } +TEST_CASE("Typed Object") { + InMemoryTestFile config; + config.schema = Schema{{"MixedObject", + { + {"_id", PropertyType::Int, Property::IsPrimary{true}}, + {"any", PropertyType::Mixed | PropertyType::Nullable}, + }}, + {"StringObject", + { + {"_id", PropertyType::Int, Property::IsPrimary{true}}, + {"string", PropertyType::String}, + }}}; + auto r = Realm::get_shared_realm(config); + CppContext ctx(r); + + UnmanagedObject unmanaged{"StringObject", AnyDict{{"_id", INT64_C(2)}, {"string", std::string("hello")}}}; + util::Any value1{AnyDict{{"_id", INT64_C(6)}, {"any", unmanaged}}}; + util::Any value2{AnyDict{{"_id", INT64_C(3)}, {"string", std::string("godbye")}}}; + + r->begin_transaction(); + Obj mixed_obj = Object::create(ctx, r, *r->schema().find("MixedObject"), value1).get_obj(); + Obj string_obj = Object::create(ctx, r, *r->schema().find("StringObject"), value2).get_obj(); + r->commit_transaction(); + + auto link = mixed_obj.get_any("any"); + auto linked_obj = r->read_group().get_object(link.get_link()); + CHECK(linked_obj.get("string") == "hello"); + + SECTION("assign managed object") { + r->begin_transaction(); + util::Any new_value{AnyDict{{"_id", INT64_C(6)}, {"any", string_obj}}}; + Object::create(ctx, r, *r->schema().find("MixedObject"), new_value, CreatePolicy::UpdateModified); + r->commit_transaction(); + auto link = mixed_obj.get_any("any"); + auto linked_obj = r->read_group().get_object(link.get_link()); + CHECK(linked_obj.get("string") == "godbye"); + } + + SECTION("assign unmanaged object again") { + util::Any new_value{AnyDict{{"_id", INT64_C(8)}, {"any", unmanaged}}}; + r->begin_transaction(); + auto obj = Object::create(ctx, r, *r->schema().find("MixedObject"), new_value, CreatePolicy::UpdateModified) + .get_obj(); + r->commit_transaction(); + CHECK(r->read_group().get_table("class_StringObject")->size() == 2); + auto link = obj.get_any("any"); + auto linked_obj = r->read_group().get_object(link.get_link()); + CHECK(linked_obj.get("string") == "hello"); + } +} + #if REALM_ENABLE_SYNC TEST_CASE("Asymmetric Object") { From 0f798cba32fe6c71378d15c3474d879ad0ce9f01 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Tue, 9 Apr 2024 15:52:59 +0200 Subject: [PATCH 07/12] Update after review --- src/realm/object-store/collection.hpp | 7 --- src/realm/object-store/dictionary.hpp | 88 ++++++++++----------------- src/realm/object-store/list.hpp | 37 ++++++----- test/object-store/primitive_list.cpp | 6 ++ 4 files changed, 54 insertions(+), 84 deletions(-) diff --git a/src/realm/object-store/collection.hpp b/src/realm/object-store/collection.hpp index 5c0765f7c1b..e809ee2bab1 100644 --- a/src/realm/object-store/collection.hpp +++ b/src/realm/object-store/collection.hpp @@ -156,13 +156,6 @@ void Collection::validate_embedded(Context& ctx, T&& value, CreatePolicy policy) throw IllegalOperation(util::format("Cannot add an existing managed embedded object to a %1.", type_name())); } -// Dummy implementation to satisfy StringifyingContext -inline std::ostream& operator<<(std::ostream& out, const Collection&) -{ - return out; -} - - } // namespace object_store } // namespace realm diff --git a/src/realm/object-store/dictionary.hpp b/src/realm/object-store/dictionary.hpp index 24b57ea4594..9a522808577 100644 --- a/src/realm/object-store/dictionary.hpp +++ b/src/realm/object-store/dictionary.hpp @@ -184,41 +184,6 @@ inline Obj Dictionary::get(StringData key) const return get_object(key); } -namespace { -template -struct ValueUpdater { - ContextType& ctx; - ValueType& value; - Dictionary& dict; - StringData key; - CreatePolicy policy; - - template - void operator()(T*) - { - bool attr_changed = !policy.diff; - auto new_val = ctx.template unbox(value, policy); - - if (!attr_changed) { - util::Optional old_val = dict.try_get_any(key); - if (old_val) { - if constexpr (std::is_same::value) { - attr_changed = !new_val.is_same_type(*old_val); - } - - attr_changed = attr_changed || new_val != *old_val; - } - else { - attr_changed = true; - } - } - - if (attr_changed) - dict.insert(key, new_val); - } -}; -} // namespace - template void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy policy) { @@ -232,34 +197,43 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po ctx.template unbox(value, policy, obj_key); return; } - if (m_type == PropertyType::Mixed) { - Mixed new_val = ctx.template unbox(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; + dispatch([&](auto t) { + using U = std::decay_t; + bool attr_changed = !policy.diff; + auto new_val = ctx.template unbox(value, policy); + + if constexpr (std::is_same_v) { + 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; + } } - bool attr_changed = !policy.diff; if (!attr_changed) { - util::Optional old_value = dict().try_get(key); - attr_changed = !old_value || !new_val.is_same_type(*old_value) || new_val != *old_value; + util::Optional old_val = dict().try_get(key); + if (old_val) { + if constexpr (std::is_same_v) { + attr_changed = !new_val.is_same_type(*old_val); + } + + attr_changed = attr_changed || new_val != *old_val; + } + else { + attr_changed = true; + } } if (attr_changed) - this->insert(key, new_val); - } - else { - ValueUpdater updater{ctx, value, *this, key, policy}; - dispatch(updater); - } + dict().insert(key, new_val); + }); } template diff --git a/src/realm/object-store/list.hpp b/src/realm/object-store/list.hpp index 08de6ed1f82..fa9d56a31b1 100644 --- a/src/realm/object-store/list.hpp +++ b/src/realm/object-store/list.hpp @@ -246,28 +246,25 @@ void List::set(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) ctx.template unbox(value, policy, key); return; } - if (m_type == PropertyType::Mixed) { - Mixed new_val = ctx.template unbox(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; + dispatch([&](auto t) { + using U = std::decay_t; + auto new_val = ctx.template unbox(value, policy); + if constexpr (std::is_same_v) { + 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>(value, policy)); - }); - } + }); } template diff --git a/test/object-store/primitive_list.cpp b/test/object-store/primitive_list.cpp index 4bb893cf087..c84e63d5e18 100644 --- a/test/object-store/primitive_list.cpp +++ b/test/object-store/primitive_list.cpp @@ -46,6 +46,12 @@ using namespace realm; using namespace realm::util; namespace cf = realm::collection_fixtures; +// Dummy implementation to satisfy StringifyingContext +inline std::ostream& operator<<(std::ostream& out, const realm::object_store::Collection&) +{ + return out; +} + struct StringifyingContext { template std::string box(T value) From 03b47213a917bcac9c79c69813d370e6aca44165 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Fri, 12 Apr 2024 14:49:10 +0200 Subject: [PATCH 08/12] Refactor and fix --- src/realm/object-store/dictionary.hpp | 101 +------- src/realm/object-store/list.hpp | 189 --------------- src/realm/object-store/object_accessor.hpp | 264 +++++++++++++++++++++ test/object-store/dictionary.cpp | 9 + test/object-store/object.cpp | 5 + 5 files changed, 280 insertions(+), 288 deletions(-) diff --git a/src/realm/object-store/dictionary.hpp b/src/realm/object-store/dictionary.hpp index 9a522808577..6af4fcc5ac9 100644 --- a/src/realm/object-store/dictionary.hpp +++ b/src/realm/object-store/dictionary.hpp @@ -142,13 +142,6 @@ class Dictionary : public object_store::Collection { Obj get_object(StringData key) const; }; -} // namespace object_store -} // namespace realm - -#include - -namespace realm::object_store { - template auto Dictionary::dispatch(Fn&& fn) const { @@ -184,97 +177,7 @@ inline Obj Dictionary::get(StringData key) const return get_object(key); } -template -void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy policy) -{ - if (ctx.is_null(value)) { - this->insert(key, Mixed()); - return; - } - if (m_is_embedded) { - validate_embedded(ctx, value, policy); - auto obj_key = dict().create_and_insert_linked_object(key).get_key(); - ctx.template unbox(value, policy, obj_key); - return; - } - dispatch([&](auto t) { - using U = std::decay_t; - bool attr_changed = !policy.diff; - auto new_val = ctx.template unbox(value, policy); - - if constexpr (std::is_same_v) { - 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; - } - } - - if (!attr_changed) { - util::Optional old_val = dict().try_get(key); - if (old_val) { - if constexpr (std::is_same_v) { - attr_changed = !new_val.is_same_type(*old_val); - } - - attr_changed = attr_changed || new_val != *old_val; - } - else { - attr_changed = true; - } - } - - if (attr_changed) - dict().insert(key, new_val); - }); -} - -template -auto Dictionary::get(Context& ctx, StringData key) const -{ - if (m_type == PropertyType::Mixed) { - Mixed value = dict().get(key); - if (value.is_type(type_Dictionary)) { - return ctx.box(get_dictionary(key)); - } - if (value.is_type(type_List)) { - return ctx.box(get_list(key)); - } - return ctx.box(value); - } - else { - return dispatch([&](auto t) { - return ctx.box(this->get>(key)); - }); - } -} - -template -void Dictionary::assign(Context& ctx, T&& values, CreatePolicy policy) -{ - if (ctx.is_same_dictionary(*this, values)) - return; - - if (ctx.is_null(values)) { - remove_all(); - return; - } - - if (!policy.diff) - remove_all(); - - ctx.enumerate_dictionary(values, [&](StringData key, auto&& value) { - this->insert(ctx, key, value, policy); - }); -} - -} // namespace realm::object_store +} // namespace object_store +} // namespace realm #endif /* REALM_OS_DICTIONARY_HPP */ diff --git a/src/realm/object-store/list.hpp b/src/realm/object-store/list.hpp index fa9d56a31b1..ca1c1efbd60 100644 --- a/src/realm/object-store/list.hpp +++ b/src/realm/object-store/list.hpp @@ -127,16 +127,9 @@ class List : public object_store::Collection { template auto& as() const; - template - void set_if_different(Context&, size_t row_ndx, T&& value, CreatePolicy); - friend struct std::hash; }; -} // namespace realm -#include - -namespace realm { template <> Obj List::get(size_t row_ndx) const; @@ -168,188 +161,6 @@ auto List::dispatch(Fn&& fn) const return switch_on_type(get_type(), std::forward(fn)); } -template -auto List::get(Context& ctx, size_t row_ndx) const -{ - if (m_type == PropertyType::Mixed) { - Mixed value = get(row_ndx); - if (value.is_type(type_Dictionary)) { - return ctx.box(get_dictionary(row_ndx)); - } - if (value.is_type(type_List)) { - return ctx.box(get_list(row_ndx)); - } - return ctx.box(value); - } - else { - return dispatch([&](auto t) { - return ctx.box(this->get>(row_ndx)); - }); - } -} - -template -size_t List::find(Context& ctx, T&& value) const -{ - return dispatch([&](auto t) { - return this->find(ctx.template unbox>(value, CreatePolicy::Skip)); - }); -} - -template -void List::add(Context& ctx, T&& value, CreatePolicy policy) -{ - this->insert(ctx, size(), std::move(value), policy); -} - -template -void List::insert(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) -{ - if (m_is_embedded) { - validate_embedded(ctx, value, policy); - auto key = as().create_and_insert_linked_object(list_ndx).get_key(); - ctx.template unbox(value, policy, key); - return; - } - if (m_type == PropertyType::Mixed) { - Mixed new_val = ctx.template unbox(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>(value, policy)); - }); - } -} - -template -void List::set(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) -{ - if (m_is_embedded) { - validate_embedded(ctx, value, policy); - - auto& list = as(); - auto key = policy.diff ? list.get(list_ndx) : list.create_and_set_linked_object(list_ndx).get_key(); - ctx.template unbox(value, policy, key); - return; - } - dispatch([&](auto t) { - using U = std::decay_t; - auto new_val = ctx.template unbox(value, policy); - if constexpr (std::is_same_v) { - 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); - }); -} - -template -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(list_ndx) : as().create_and_set_linked_object(list_ndx); - ctx.template unbox(value, policy, key.get_key()); - return; - } - if (m_type == PropertyType::Mixed) { - Mixed new_val = ctx.template unbox(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; - } - Mixed old_value = this->get(list_ndx); - Mixed new_value = ctx.template unbox(value, policy); - if (!new_value.is_same_type(old_value) || old_value != new_value) - this->set(list_ndx, new_value); - } - else { - dispatch([&](auto t) { - using U = std::decay_t; - if constexpr (std::is_same_v) { - auto old_value = this->get(list_ndx); - auto new_value = ctx.template unbox(value, policy, old_value.get_key()); - if (new_value.get_key() != old_value.get_key()) - this->set(list_ndx, new_value); - } - else { - bool attr_changed = false; - auto old_value = this->get(list_ndx); - auto new_value = ctx.template unbox(value, policy); - if constexpr (std::is_same_v) { - attr_changed = !new_value.is_same_type(old_value); - } - attr_changed = attr_changed || old_value != new_value; - if (attr_changed) - this->set(list_ndx, new_value); - } - }); - } -} - -template -void List::assign(Context& ctx, T&& values, CreatePolicy policy) -{ - if (ctx.is_same_list(*this, values)) - return; - - if (ctx.is_null(values)) { - remove_all(); - return; - } - - if (!policy.diff) - remove_all(); - - size_t sz = size(); - size_t index = 0; - ctx.enumerate_collection(values, [&](auto&& element) { - if (index >= sz) { - this->add(ctx, element, policy); - } - 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); - } - index++; - }); - while (index < sz) - remove(--sz); -} } // namespace realm namespace std { diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index 277f58bb43f..bcba3cedd9d 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -466,6 +466,270 @@ ObjKey Object::get_for_primary_key_in_migration(ContextType& ctx, Table const& t return table.find_first(primary_prop.column_key, ctx.template unbox(primary_value)); } +namespace _impl { + +template +auto box_mixed(Context& ctx, const object_store::Collection& collection, const PathElement& path, Mixed value) +{ + if (value.is_type(type_Dictionary)) { + return ctx.box(collection.get_dictionary(path)); + } + if (value.is_type(type_List)) { + return ctx.box(collection.get_list(path)); + } + return ctx.box(value); +} + +template +void assign_collection(Context& ctx, object_store::Collection& collection, const PathElement& path, + CollectionType type, T&& value, CreatePolicy policy) +{ + switch (type) { + case CollectionType::List: + collection.get_list(path).assign(ctx, value, policy); + return; + case CollectionType::Dictionary: + collection.get_dictionary(path).assign(ctx, value, policy); + return; + default: + REALM_UNREACHABLE(); + } +} + +template +inline std::optional collection_type(T) +{ + return {}; +} + +inline std::optional collection_type(Mixed m) +{ + static_assert(int(CollectionType::Dictionary) == int(type_Dictionary)); + static_assert(int(CollectionType::List) == int(type_List)); + if (m.is_type(type_Dictionary, type_List)) { + return static_cast(static_cast(m.get_type())); + } + return {}; +} + +template +bool compare(const T& a, const T& b) +{ + return a == b; +} +inline bool compare(const Obj& a, const Obj& b) +{ + return a.get_key() == b.get_key(); +} +inline bool compare(Mixed a, Mixed b) +{ + return a.is_same_type(b) && a == b; +} + +} // namespace _impl + +template +auto List::get(Context& ctx, size_t row_ndx) const +{ + if (m_type == PropertyType::Mixed) { + return _impl::box_mixed(ctx, *this, row_ndx, get(row_ndx)); + } + + return dispatch([&](auto t) { + return ctx.box(this->get>(row_ndx)); + }); +} + +template +size_t List::find(Context& ctx, T&& value) const +{ + return dispatch([&](auto t) { + return this->find(ctx.template unbox>(value, CreatePolicy::Skip)); + }); +} + +template +void List::add(Context& ctx, T&& value, CreatePolicy policy) +{ + this->insert(ctx, size(), std::move(value), policy); +} + +template +void List::insert(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) +{ + if (m_is_embedded) { + validate_embedded(ctx, value, policy); + auto key = as().create_and_insert_linked_object(list_ndx).get_key(); + ctx.template unbox(value, policy, key); + return; + } + dispatch([&](auto t) { + using U = std::decay_t; + auto new_val = ctx.template unbox(value, policy); + if (auto type = _impl::collection_type(new_val)) { + insert_collection(list_ndx, *type); + _impl::assign_collection(ctx, *this, list_ndx, *type, value, policy); + return; + } + this->insert(list_ndx, new_val); + }); +} + +template +void List::set(Context& ctx, size_t list_ndx, T&& value, CreatePolicy policy) +{ + if (m_is_embedded) { + validate_embedded(ctx, value, policy); + auto& list = as(); + auto key = policy.diff ? list.get(list_ndx) : list.create_and_set_linked_object(list_ndx).get_key(); + ctx.template unbox(value, policy, key); + return; + } + + dispatch([&](auto t) { + using U = std::decay_t; + U new_val; + if constexpr (std::is_same_v) { + auto old_key = this->get(list_ndx); + new_val = ctx.template unbox(value, policy, old_key); + } + else { + new_val = ctx.template unbox(value, policy); + } + + if constexpr (std::is_same_v) { + if (auto type = _impl::collection_type(new_val)) { + set_collection(list_ndx, *type); + _impl::assign_collection(ctx, *this, list_ndx, *type, value, policy); + return; + } + } + + if (policy.diff) { + auto old_value = this->get(list_ndx); + if (_impl::compare(old_value, new_val)) + return; + } + + this->set(list_ndx, new_val); + }); +} + +template +void List::assign(Context& ctx, T&& values, CreatePolicy policy) +{ + if (ctx.is_same_list(*this, values)) + return; + + if (ctx.is_null(values)) { + remove_all(); + return; + } + + if (!policy.diff) + remove_all(); + + size_t sz = size(); + size_t index = 0; + ctx.enumerate_collection(values, [&](auto&& element) { + if (index >= sz) { + this->add(ctx, element, policy); + } + 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(ctx, index, element, policy); + } + index++; + }); + while (index < sz) + remove(--sz); +} + +namespace object_store { + +template +void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy policy) +{ + if (ctx.is_null(value)) { + this->insert(key, Mixed()); + return; + } + if (m_is_embedded) { + validate_embedded(ctx, value, policy); + auto old_value = dict().try_get(key); + auto obj_key = (policy.diff && old_value) ? old_value->get() + : dict().create_and_insert_linked_object(key).get_key(); + ctx.template unbox(value, policy, obj_key); + return; + }; + + dispatch([&](auto t) { + using U = std::decay_t; + U new_val; + if constexpr (std::is_same_v) { + ObjKey old_key; + if (auto old_value = dict().try_get(key)) { + old_key = old_value->get_link().get_obj_key(); + } + new_val = ctx.template unbox(value, policy, old_key); + } + else { + new_val = ctx.template unbox(value, policy); + } + + if constexpr (std::is_same_v) { + if (auto type = _impl::collection_type(new_val)) { + insert_collection(key, *type); + _impl::assign_collection(ctx, *this, key, *type, value, policy); + return; + } + } + + if (policy.diff) { + if (auto old_value = dict().try_get(key)) { + if (_impl::compare(*old_value, new_val)) { + return; + } + } + } + + dict().insert(key, new_val); + }); +} + +template +auto Dictionary::get(Context& ctx, StringData key) const +{ + if (m_type == PropertyType::Mixed) { + return _impl::box_mixed(ctx, *this, key, get(key)); + } + return dispatch([&](auto t) { + return ctx.box(this->get>(key)); + }); +} + +template +void Dictionary::assign(Context& ctx, T&& values, CreatePolicy policy) +{ + if (ctx.is_same_dictionary(*this, values)) + return; + + if (ctx.is_null(values)) { + remove_all(); + return; + } + + if (!policy.diff) + remove_all(); + + ctx.enumerate_dictionary(values, [&](StringData key, auto&& value) { + this->insert(ctx, key, value, policy); + }); +} + +} // namespace object_store } // namespace realm #endif // REALM_OS_OBJECT_ACCESSOR_HPP diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index 58a146e88f8..a381eb485ce 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -1064,6 +1064,15 @@ TEST_CASE("embedded dictionary", "[dictionary]") { REQUIRE(dict.get_object("foo").get(col_value) == 20); } + SECTION("mutates the existing object for update mode Modified") { + auto old_object = dict.get("0"); + dict.insert(ctx, "0", std::any(AnyDict{{"value", INT64_C(20)}}), CreatePolicy::UpdateModified); + REQUIRE(dict.size() == 10); + REQUIRE(target->size() == initial_target_size); + REQUIRE(dict.get_object("0").get(col_value) == 20); + REQUIRE(old_object.is_valid()); + } + r->cancel_transaction(); } } diff --git a/test/object-store/object.cpp b/test/object-store/object.cpp index 5929ce51d10..37ebc08ec72 100644 --- a/test/object-store/object.cpp +++ b/test/object-store/object.cpp @@ -2340,11 +2340,16 @@ TEST_CASE("Typed Object") { UnmanagedObject unmanaged{"StringObject", AnyDict{{"_id", INT64_C(2)}, {"string", std::string("hello")}}}; util::Any value1{AnyDict{{"_id", INT64_C(6)}, {"any", unmanaged}}}; util::Any value2{AnyDict{{"_id", INT64_C(3)}, {"string", std::string("godbye")}}}; + util::Any value3{AnyDict{{"_id", INT64_C(7)}, {"any", AnyDict{{"obj", INT64_C(5)}}}}}; + util::Any value4{AnyDict{{"_id", INT64_C(7)}, {"any", AnyDict{{"obj", unmanaged}}}}}; r->begin_transaction(); Obj mixed_obj = Object::create(ctx, r, *r->schema().find("MixedObject"), value1).get_obj(); Obj string_obj = Object::create(ctx, r, *r->schema().find("StringObject"), value2).get_obj(); + Object::create(ctx, r, *r->schema().find("MixedObject"), value3); + Object::create(ctx, r, *r->schema().find("MixedObject"), value4, CreatePolicy::UpdateModified); r->commit_transaction(); + CHECK(r->read_group().get_table("class_StringObject")->size() == 2); auto link = mixed_obj.get_any("any"); auto linked_obj = r->read_group().get_object(link.get_link()); From e71f412236443ca183b1a22be5e95e6aa3977e25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Mon, 15 Apr 2024 13:58:27 +0200 Subject: [PATCH 09/12] Make Results::get return proper nested collections --- src/realm/object-store/results.hpp | 15 +++++++++++++-- test/object-store/dictionary.cpp | 4 ++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/realm/object-store/results.hpp b/src/realm/object-store/results.hpp index 88c1c2b5a4f..78d334c41b3 100644 --- a/src/realm/object-store/results.hpp +++ b/src/realm/object-store/results.hpp @@ -411,10 +411,21 @@ auto Results::dispatch(Fn&& fn) const } template -auto Results::get(Context& ctx, size_t row_ndx) +auto Results::get(Context& ctx, size_t ndx) { return dispatch([&](auto t) { - return ctx.box(this->get>(row_ndx)); + using U = std::decay_t; + auto value = this->get(ndx); + if constexpr (std::is_same_v) { + if (value.is_type(type_Dictionary)) { + return ctx.box(get_dictionary(ndx)); + } + if (value.is_type(type_List)) { + return ctx.box(get_list(ndx)); + } + } + + return ctx.box(value); }); } diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index a381eb485ce..7c6ab5df1d0 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -190,6 +190,10 @@ TEST_CASE("nested dictionary in mixed", "[dictionary]") { REQUIRE(val.is_type(type_List)); auto list = results.get_list(1); REQUIRE(list.is_valid()); + + CppContext ctx(r); + CHECK(util::any_cast(results.get(ctx, 0)).is_valid()); + CHECK(util::any_cast(results.get(ctx, 1)).is_valid()); } } From 3c5a1b453c910d4baaee6fe872a713e90c0f2170 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Mon, 22 Apr 2024 16:54:31 +0200 Subject: [PATCH 10/12] Rearrange a bit --- src/realm/object-store/results.hpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/realm/object-store/results.hpp b/src/realm/object-store/results.hpp index 78d334c41b3..d2e419869a0 100644 --- a/src/realm/object-store/results.hpp +++ b/src/realm/object-store/results.hpp @@ -415,17 +415,19 @@ auto Results::get(Context& ctx, size_t ndx) { return dispatch([&](auto t) { using U = std::decay_t; - auto value = this->get(ndx); if constexpr (std::is_same_v) { + Mixed value = this->get(ndx); if (value.is_type(type_Dictionary)) { return ctx.box(get_dictionary(ndx)); } if (value.is_type(type_List)) { return ctx.box(get_list(ndx)); } + return ctx.box(value); + } + else { + return ctx.box(this->get(ndx)); } - - return ctx.box(value); }); } From 06e71c4d29bfe1c792dff3a1adc85be56a978c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Mon, 22 Apr 2024 17:29:34 +0200 Subject: [PATCH 11/12] Add check for NULL in Dictionary::insert<>() --- src/realm/object-store/object_accessor.hpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index bcba3cedd9d..7726f4bce34 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -670,7 +670,8 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po U new_val; if constexpr (std::is_same_v) { ObjKey old_key; - if (auto old_value = dict().try_get(key)) { + if (auto old_value = dict().try_get(key); old_value && !old_value->is_null()) { + // Value is either null or an ObjKey old_key = old_value->get_link().get_obj_key(); } new_val = ctx.template unbox(value, policy, old_key); From 8e3d77dc78f7d5fad5b4d0ce1ef039cec52e949e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B8rgen=20Edelbo?= Date: Tue, 23 Apr 2024 10:42:43 +0200 Subject: [PATCH 12/12] Add some tests --- src/realm/object-store/object_accessor.hpp | 5 ++-- test/object-store/dictionary.cpp | 32 ++++++++++++++++++---- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/src/realm/object-store/object_accessor.hpp b/src/realm/object-store/object_accessor.hpp index 7726f4bce34..824ee437204 100644 --- a/src/realm/object-store/object_accessor.hpp +++ b/src/realm/object-store/object_accessor.hpp @@ -659,8 +659,9 @@ void Dictionary::insert(Context& ctx, StringData key, T&& value, CreatePolicy po if (m_is_embedded) { validate_embedded(ctx, value, policy); auto old_value = dict().try_get(key); - auto obj_key = (policy.diff && old_value) ? old_value->get() - : dict().create_and_insert_linked_object(key).get_key(); + auto obj_key = (policy.diff && old_value && !old_value->is_null()) + ? old_value->get() + : dict().create_and_insert_linked_object(key).get_key(); ctx.template unbox(value, policy, obj_key); return; }; diff --git a/test/object-store/dictionary.cpp b/test/object-store/dictionary.cpp index 7c6ab5df1d0..8da2f0a0a59 100644 --- a/test/object-store/dictionary.cpp +++ b/test/object-store/dictionary.cpp @@ -1044,6 +1044,7 @@ TEST_CASE("embedded dictionary", "[dictionary]") { object_store::Dictionary dict(r, obj, col_links); for (int i = 0; i < 10; ++i) dict.insert_embedded(util::to_string(i)); + dict.insert("null", ObjKey()); r->commit_transaction(); @@ -1063,15 +1064,22 @@ TEST_CASE("embedded dictionary", "[dictionary]") { SECTION("creates new object for dictionary") { dict.insert(ctx, "foo", std::any(AnyDict{{"value", INT64_C(20)}})); - REQUIRE(dict.size() == 11); + REQUIRE(dict.size() == 12); REQUIRE(target->size() == initial_target_size + 1); REQUIRE(dict.get_object("foo").get(col_value) == 20); } + SECTION("overwrite null value") { + dict.insert(ctx, "null", std::any(AnyDict{{"value", INT64_C(17)}}), CreatePolicy::UpdateModified); + REQUIRE(dict.size() == 11); + REQUIRE(target->size() == initial_target_size + 1); + REQUIRE(dict.get_object("null").get(col_value) == 17); + } + SECTION("mutates the existing object for update mode Modified") { auto old_object = dict.get("0"); dict.insert(ctx, "0", std::any(AnyDict{{"value", INT64_C(20)}}), CreatePolicy::UpdateModified); - REQUIRE(dict.size() == 10); + REQUIRE(dict.size() == 11); REQUIRE(target->size() == initial_target_size); REQUIRE(dict.get_object("0").get(col_value) == 20); REQUIRE(old_object.is_valid()); @@ -1121,8 +1129,10 @@ TEMPLATE_TEST_CASE("dictionary of objects", "[dictionary][links]", cf::MixedVal, Obj target_obj = target->create_object().set(col_target_value, T(values[i])); dict.insert(keys[i], target_obj); } + r->commit_transaction(); r->begin_transaction(); + SECTION("min()") { if constexpr (!TestType::can_minmax) { REQUIRE_EXCEPTION( @@ -1352,13 +1362,25 @@ TEST_CASE("dictionary nullify", "[dictionary]") { Any{AnyDict{{"intDictionary", AnyDict{{"0", Any(AnyDict{{"intCol", INT64_C(0)}})}, {"1", Any(AnyDict{{"intCol", INT64_C(1)}})}, {"2", Any(AnyDict{{"intCol", INT64_C(2)}})}}}}}); + auto obj1 = Object::create(ctx, r, *r->schema().find("DictionaryObject"), + Any{AnyDict{{"intDictionary", AnyDict{{"null", Any()}}}}}); r->commit_transaction(); r->begin_transaction(); - // Before fix, we would crash here - r->read_group().get_table("class_IntObject")->clear(); + SECTION("clear dictionary") { + // Before fix, we would crash here + r->read_group().get_table("class_IntObject")->clear(); + // r->read_group().to_json(std::cout); + } + + SECTION("overwrite null value") { + obj1.set_property_value(ctx, "intDictionary", Any(AnyDict{{"null", Any(AnyDict{{"intCol", INT64_C(3)}})}}), + CreatePolicy::UpdateModified); + auto dict = + util::any_cast(obj1.get_property_value(ctx, "intDictionary")); + REQUIRE(dict.get_object("null").get("intCol") == 3); + } r->commit_transaction(); - // r->read_group().to_json(std::cout); } TEST_CASE("nested collection set by Object::create", "[dictionary]") {