From 3dabb1d37067f7036e415bdda49aa470cf2861e2 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 10 Aug 2022 18:59:59 +0100 Subject: [PATCH 01/25] draft resolve migrations automatically --- src/realm/object-store/object_schema.hpp | 1 + src/realm/object-store/object_store.cpp | 39 +++++++++++ src/realm/object-store/shared_realm.hpp | 3 + test/object-store/migrations.cpp | 82 +++++++++++++++++++++++- 4 files changed, 124 insertions(+), 1 deletion(-) diff --git a/src/realm/object-store/object_schema.hpp b/src/realm/object-store/object_schema.hpp index 69bc5783fd1..608ff5739ea 100644 --- a/src/realm/object-store/object_schema.hpp +++ b/src/realm/object-store/object_schema.hpp @@ -64,6 +64,7 @@ class ObjectSchema { TableKey table_key; ObjectType table_type = ObjectType::TopLevel; std::string alias; + bool m_delete_object_if_embedded_and_orphan{false}; Property* property_for_public_name(StringData public_name) noexcept; const Property* property_for_public_name(StringData public_name) const noexcept; diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index 91abfa3f579..fe9c088b798 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -821,6 +821,7 @@ static void apply_post_migration_changes(Group& group, std::vector void operator()(ChangeTableType op) { + post_migration_embedded_objects_cleanup(op.object); table(op.object).set_table_type(static_cast(*op.new_table_type)); } void operator()(RemoveTable) {} @@ -828,6 +829,44 @@ static void apply_post_migration_changes(Group& group, std::vector void operator()(MakePropertyNullable) {} void operator()(MakePropertyRequired) {} void operator()(AddProperty) {} + + void post_migration_embedded_objects_cleanup(const ObjectSchema* object_schema) + { + if(object_schema->table_type == ObjectSchema::ObjectType::Embedded) + { + auto original_object_schema = initial_schema.find(object_schema->name); + if(original_object_schema != initial_schema.end() && original_object_schema->table_type == ObjectSchema::ObjectType::TopLevel) + { + auto table = table_for_object_schema(group, *original_object_schema); + size_t n = table->size(); + for (size_t i = 0; iget_object(i); + //check if we are doing a migration from TopLevel => Embedded + size_t backlink_count = object.get_backlink_count(); + //check back link count. + // 1. if object is an orphan (no backlicks, then delete it if instructed to do so) + // 2. if object has multiple backlicks, then just clone N times the object (for each backlick) and assign to each a different parent + + if (backlink_count == 0 /*&& object_schema->m_delete_object_if_embedded_and_orphan*/) + object.remove(); + + //Migration from Realm Object ==> Embedded Object. By default there can only be one parent per embedded object. So Dup the object for each parent + //and assign only 1 parent to each instance. + else if (backlink_count > 1) { + for(size_t i=0; icreate_object(); + + new_obj.assign(object); + } + object.remove(); + } + } + } + } + } } applier{group, initial_schema, did_reread_schema}; for (auto& change : changes) { diff --git a/src/realm/object-store/shared_realm.hpp b/src/realm/object-store/shared_realm.hpp index 04f9fc42a3e..a08098634f6 100644 --- a/src/realm/object-store/shared_realm.hpp +++ b/src/realm/object-store/shared_realm.hpp @@ -175,6 +175,9 @@ struct RealmConfig { // Disable automatic backup at file format upgrade by setting to false bool backup_at_file_format_change = true; + + //delete embedded orphan objects + bool delete_embedded_ophan_objects = false; }; class Realm : public std::enable_shared_from_this { diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 6fac28f70f6..23829571079 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1050,6 +1050,86 @@ TEST_CASE("migration: Automatic") { REQUIRE(value == 42); } } + //NICO + SECTION("change table to embedded - multiple incoming links - resolved automatically") + { + Schema schema = { + {"child_table", + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto child_object_key = child_object.get_key(); + parent_table->create_object().set_all(child_object_key); + parent_table->create_object().set_all(child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 1); + REQUIRE_FALSE(child_table->is_embedded()); + + REQUIRE_NOTHROW(realm->update_schema( + set_table_type(schema, "child_table", ObjectType::Embedded), 2, [](auto, auto new_realm, auto&) { + Object parent_object1(new_realm, "parent_table", 0); + CppContext context(new_realm); + Object child_object1 = + any_cast(parent_object1.get_property_value(context, "child_property")); + Int value = any_cast(child_object1.get_property_value(context, "value")); + + auto child_table = ObjectStore::table_for_object_type(new_realm->read_group(), "child_table"); + Obj child_object2 = child_table->create_object(); + child_object2.set("value", value); + + Object parent_object2(new_realm, "parent_table", 1); + parent_object2.set_property_value(context, "child_property", util::Any(child_object2)); + })); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 2); + REQUIRE(child_table->is_embedded()); + for (int i = 0; i < 2; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + Object child_object = + any_cast(parent_object.get_property_value(context, "child_property")); + Int value = any_cast(child_object.get_property_value(context, "value")); + REQUIRE(value == 42); + } + } + SECTION("change table from top-level to embedded, delete objects with 0 incoming links") + { + Schema schema = { + {"child_table", + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + REQUIRE_FALSE(child_table->is_embedded()); + + REQUIRE_NOTHROW( + realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, nullptr)); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(child_table->is_embedded()); + } } SECTION("schema correctness during migration") { @@ -2884,4 +2964,4 @@ TEST_CASE("migrations with asymmetric tables") { } } -#endif // REALM_ENABLE_AUTH_TESTS \ No newline at end of file +#endif // REALM_ENABLE_AUTH_TESTS From ab034caed3f8746e8741145fca1876d74a00d198 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 11 Aug 2022 18:02:42 +0100 Subject: [PATCH 02/25] hadle automatically backlinks for migration toplevel => embedded --- src/realm.h | 5 + src/realm/obj.cpp | 76 +++++-- src/realm/obj.hpp | 3 + src/realm/object-store/c_api/config.cpp | 6 + src/realm/object-store/object_schema.hpp | 1 - src/realm/object-store/object_store.cpp | 73 ++++--- src/realm/object-store/object_store.hpp | 2 +- src/realm/object-store/shared_realm.cpp | 5 +- src/realm/object-store/shared_realm.hpp | 2 +- test/object-store/migrations.cpp | 265 +++++++++++++---------- 10 files changed, 262 insertions(+), 176 deletions(-) diff --git a/src/realm.h b/src/realm.h index b8a69095313..f930de16547 100644 --- a/src/realm.h +++ b/src/realm.h @@ -821,6 +821,11 @@ RLM_API void realm_config_set_cached(realm_config_t*, bool cached) RLM_API_NOEXC */ RLM_API bool realm_config_get_cached(realm_config_t*) RLM_API_NOEXCEPT; +/** + * Allow realm to manage automatically embedded objects when a migration from TopLevel to Embedded takes place. + */ +RLM_API void realm_config_set_automatic_backlink_handling(realm_config_t*, bool) RLM_API_NOEXCEPT; + /** * Create a custom scheduler object from callback functions. * diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 133980222eb..020a23e51f0 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1935,6 +1935,60 @@ bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) } void Obj::assign(const Obj& other) +{ + copy_other_object(other); + auto copy_links = [this, &other](ColKey col) { + auto t = m_table->get_opposite_table(col); + auto c = m_table->get_opposite_column(col); + auto backlinks = other.get_all_backlinks(col); + for (auto bl : backlinks) { + auto linking_obj = t->get_object(bl); + if (c.get_type() == col_type_Link) { + // Single link + REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); + linking_obj.set(c, get_key()); + } + else { + auto l = linking_obj.get_linklist(c); + auto n = l.find_first(other.get_key()); + REALM_ASSERT(n != realm::npos); + l.set(n, get_key()); + } + } + return false; + }; + m_table->for_each_backlink_column(copy_links); +} + +void Obj::dup_and_handle_multiple_backlinks() +{ + auto copy_links = [this](ColKey col) { + auto t = m_table->get_opposite_table(col); + auto c = m_table->get_opposite_column(col); + auto backlinks = get_all_backlinks(col); + + for (auto bl : backlinks) { + auto obj = m_table->create_object(); + obj.copy_other_object(*this); + auto linking_obj = t->get_object(bl); + if (c.get_type() == col_type_Link) { + // Single link + REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); + linking_obj.set(c, obj.get_key()); + } + else { + auto l = linking_obj.get_linklist(c); + auto n = l.find_first(get_key()); + REALM_ASSERT(n != realm::npos); + l.set(n, obj.get_key()); + } + } + return false; + }; + m_table->for_each_backlink_column(copy_links); +} + +void Obj::copy_other_object(const Obj& other) { REALM_ASSERT(get_table() == other.get_table()); auto cols = m_table->get_column_keys(); @@ -1974,28 +2028,6 @@ void Obj::assign(const Obj& other) } } } - - auto copy_links = [this, &other](ColKey col) { - auto t = m_table->get_opposite_table(col); - auto c = m_table->get_opposite_column(col); - auto backlinks = other.get_all_backlinks(col); - for (auto bl : backlinks) { - auto linking_obj = t->get_object(bl); - if (c.get_type() == col_type_Link) { - // Single link - REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); - linking_obj.set(c, get_key()); - } - else { - auto l = linking_obj.get_linklist(c); - auto n = l.find_first(other.get_key()); - REALM_ASSERT(n != realm::npos); - l.set(n, get_key()); - } - } - return false; - }; - m_table->for_each_backlink_column(copy_links); } Dictionary Obj::get_dictionary(ColKey col_key) const diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index 4c622fa8af8..8de9cfa0c9c 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -261,6 +261,7 @@ class Obj { Obj& set_all(Head v, Tail... tail); void assign(const Obj& other); + void dup_and_handle_multiple_backlinks(); Obj get_linked_object(ColKey link_col_key) const { @@ -420,6 +421,8 @@ class Obj { bool remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) const; template inline void set_spec(T&, ColKey); + + void copy_other_object(const Obj& other); }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/src/realm/object-store/c_api/config.cpp b/src/realm/object-store/c_api/config.cpp index b86918fca7d..fc3ed7ef31b 100644 --- a/src/realm/object-store/c_api/config.cpp +++ b/src/realm/object-store/c_api/config.cpp @@ -222,3 +222,9 @@ RLM_API bool realm_config_get_cached(realm_config_t* realm_config) noexcept { return realm_config->cache; } + +RLM_API void realm_config_set_automatic_backlink_handling(realm_config_t* realm_config, + bool enable_automatic_handling) noexcept +{ + realm_config->automatic_handle_backlicks_in_migrations = enable_automatic_handling; +} diff --git a/src/realm/object-store/object_schema.hpp b/src/realm/object-store/object_schema.hpp index 608ff5739ea..69bc5783fd1 100644 --- a/src/realm/object-store/object_schema.hpp +++ b/src/realm/object-store/object_schema.hpp @@ -64,7 +64,6 @@ class ObjectSchema { TableKey table_key; ObjectType table_type = ObjectType::TopLevel; std::string alias; - bool m_delete_object_if_embedded_and_orphan{false}; Property* property_for_public_name(StringData public_name) noexcept; const Property* property_for_public_name(StringData public_name) const noexcept; diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index fe9c088b798..e5fd1f15c83 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -762,23 +762,28 @@ static void apply_pre_migration_changes(Group& group, std::vector } enum class DidRereadSchema { Yes, No }; +enum class HandleBackLinksAutomatically { Yes, No }; static void apply_post_migration_changes(Group& group, std::vector const& changes, - Schema const& initial_schema, DidRereadSchema did_reread_schema) + Schema const& initial_schema, DidRereadSchema did_reread_schema, + HandleBackLinksAutomatically handle_backlinks_automatically) { using namespace schema_change; struct Applier { - Applier(Group& group, Schema const& initial_schema, DidRereadSchema did_reread_schema) + Applier(Group& group, Schema const& initial_schema, DidRereadSchema did_reread_schema, + HandleBackLinksAutomatically handle_backlinks_automatically) : group{group} , initial_schema(initial_schema) , table(group) , did_reread_schema(did_reread_schema == DidRereadSchema::Yes) + , handle_backlinks_automatically(handle_backlinks_automatically == HandleBackLinksAutomatically::Yes) { } Group& group; Schema const& initial_schema; TableHelper table; bool did_reread_schema; + bool handle_backlinks_automatically; void operator()(RemoveProperty op) { @@ -821,7 +826,8 @@ static void apply_post_migration_changes(Group& group, std::vector void operator()(ChangeTableType op) { - post_migration_embedded_objects_cleanup(op.object); + if (handle_backlinks_automatically) + post_migration_embedded_objects_backlinks_handling(op.object); table(op.object).set_table_type(static_cast(*op.new_table_type)); } void operator()(RemoveTable) {} @@ -829,45 +835,43 @@ static void apply_post_migration_changes(Group& group, std::vector void operator()(MakePropertyNullable) {} void operator()(MakePropertyRequired) {} void operator()(AddProperty) {} - - void post_migration_embedded_objects_cleanup(const ObjectSchema* object_schema) + + void post_migration_embedded_objects_backlinks_handling(const ObjectSchema* object_schema) { + // check if we are doing a migration from TopLevel Object to Embedded. + // check back link count. + // 1. if object is an orphan (no backlicks, then delete it if instructed to do so) + // 2. if object has multiple backlicks, then just clone N times the object (for each backlick) and assign + // to each a different parent + // object_schema->handle_automatically_backlinks_for_embedded_object = true; + if(object_schema->table_type == ObjectSchema::ObjectType::Embedded) { auto original_object_schema = initial_schema.find(object_schema->name); if(original_object_schema != initial_schema.end() && original_object_schema->table_type == ObjectSchema::ObjectType::TopLevel) { auto table = table_for_object_schema(group, *original_object_schema); - size_t n = table->size(); - for (size_t i = 0; iget_object(i); - //check if we are doing a migration from TopLevel => Embedded + std::vector objects_to_erase; + std::vector objects_to_fix_backlinks; + for (auto& object : *table) { size_t backlink_count = object.get_backlink_count(); - //check back link count. - // 1. if object is an orphan (no backlicks, then delete it if instructed to do so) - // 2. if object has multiple backlicks, then just clone N times the object (for each backlick) and assign to each a different parent - - if (backlink_count == 0 /*&& object_schema->m_delete_object_if_embedded_and_orphan*/) - object.remove(); - - //Migration from Realm Object ==> Embedded Object. By default there can only be one parent per embedded object. So Dup the object for each parent - //and assign only 1 parent to each instance. - else if (backlink_count > 1) { - for(size_t i=0; icreate_object(); - - new_obj.assign(object); - } - object.remove(); - } + if (backlink_count == 0) + objects_to_erase.push_back(object); + else if (backlink_count > 1) + objects_to_fix_backlinks.push_back(object); + } + + for (auto& object : objects_to_erase) + object.remove(); + + for (auto& object : objects_to_fix_backlinks) { + object.dup_and_handle_multiple_backlinks(); + object.remove(); } } } } - } applier{group, initial_schema, did_reread_schema}; + } applier{group, initial_schema, did_reread_schema, handle_backlinks_automatically}; for (auto& change : changes) { change.visit(applier); @@ -877,7 +881,7 @@ static void apply_post_migration_changes(Group& group, std::vector void ObjectStore::apply_schema_changes(Transaction& group, uint64_t schema_version, Schema& target_schema, uint64_t target_schema_version, SchemaMode mode, - std::vector const& changes, + std::vector const& changes, bool handle_automatically_backlinks, std::function migration_function) { create_metadata_tables(group); @@ -925,17 +929,20 @@ void ObjectStore::apply_schema_changes(Transaction& group, uint64_t schema_versi auto old_schema = schema_from_group(group); apply_pre_migration_changes(group, changes); + HandleBackLinksAutomatically handle_backlinks = + handle_automatically_backlinks ? HandleBackLinksAutomatically::Yes : HandleBackLinksAutomatically::No; if (migration_function) { set_schema_keys(group, target_schema); migration_function(); // Migration function may have changed the schema, so we need to re-read it auto schema = schema_from_group(group); - apply_post_migration_changes(group, schema.compare(target_schema, mode), old_schema, DidRereadSchema::Yes); + apply_post_migration_changes(group, schema.compare(target_schema, mode), old_schema, DidRereadSchema::Yes, + handle_backlinks); group.validate_primary_columns(); } else { - apply_post_migration_changes(group, changes, {}, DidRereadSchema::No); + apply_post_migration_changes(group, changes, {}, DidRereadSchema::No, handle_backlinks); } set_schema_version(group, target_schema_version); diff --git a/src/realm/object-store/object_store.hpp b/src/realm/object-store/object_store.hpp index ba69f2eeda7..f0e071a26b4 100644 --- a/src/realm/object-store/object_store.hpp +++ b/src/realm/object-store/object_store.hpp @@ -84,7 +84,7 @@ class ObjectStore { // NOTE: must be performed within a write transaction static void apply_schema_changes(Transaction& group, uint64_t schema_version, Schema& target_schema, uint64_t target_schema_version, SchemaMode mode, - std::vector const& changes, + std::vector const& changes, bool handle_automatically_backlinks, std::function migration_function = {}); static void apply_additive_changes(Group&, std::vector const&, bool update_indexes); diff --git a/src/realm/object-store/shared_realm.cpp b/src/realm/object-store/shared_realm.cpp index d7b57d32231..01cc6a762e4 100644 --- a/src/realm/object-store/shared_realm.cpp +++ b/src/realm/object-store/shared_realm.cpp @@ -459,11 +459,12 @@ void Realm::update_schema(Schema schema, uint64_t version, MigrationFunction mig }); ObjectStore::apply_schema_changes(transaction(), version, m_schema, m_schema_version, m_config.schema_mode, - required_changes, wrapper); + required_changes, m_config.automatic_handle_backlicks_in_migrations, + wrapper); } else { ObjectStore::apply_schema_changes(transaction(), m_schema_version, schema, version, m_config.schema_mode, - required_changes); + required_changes, m_config.automatic_handle_backlicks_in_migrations); REALM_ASSERT_DEBUG(additive || (required_changes = ObjectStore::schema_from_group(read_group()).compare(schema)).empty()); } diff --git a/src/realm/object-store/shared_realm.hpp b/src/realm/object-store/shared_realm.hpp index a08098634f6..88556309caf 100644 --- a/src/realm/object-store/shared_realm.hpp +++ b/src/realm/object-store/shared_realm.hpp @@ -177,7 +177,7 @@ struct RealmConfig { bool backup_at_file_format_change = true; //delete embedded orphan objects - bool delete_embedded_ophan_objects = false; + bool automatic_handle_backlicks_in_migrations = false; }; class Realm : public std::enable_shared_from_this { diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 23829571079..b9e6478ed96 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -631,7 +631,6 @@ TEST_CASE("migration: Automatic") { REQUIRE_THROWS(realm->update_schema(set_table_type(schema, "object", ObjectType::Embedded), 2, [](auto, auto, auto&) {})); } - SECTION("change table to embedded - multiple incoming link per object") { Schema schema = { {"child_table", @@ -1050,86 +1049,120 @@ TEST_CASE("migration: Automatic") { REQUIRE(value == 42); } } - //NICO - SECTION("change table to embedded - multiple incoming links - resolved automatically") - { - Schema schema = { - {"child_table", - { - {"value", PropertyType::Int}, - }}, - {"parent_table", - { - {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, - }}, - }; - auto realm = Realm::get_shared_realm(config); - realm->update_schema(schema, 1); - realm->begin_transaction(); - auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); - Obj child_object = child_table->create_object(); - child_object.set("value", 42); - auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); - auto child_object_key = child_object.get_key(); - parent_table->create_object().set_all(child_object_key); - parent_table->create_object().set_all(child_object_key); - realm->commit_transaction(); - REQUIRE(parent_table->size() == 2); - REQUIRE(child_table->size() == 1); - REQUIRE_FALSE(child_table->is_embedded()); - - REQUIRE_NOTHROW(realm->update_schema( - set_table_type(schema, "child_table", ObjectType::Embedded), 2, [](auto, auto new_realm, auto&) { - Object parent_object1(new_realm, "parent_table", 0); - CppContext context(new_realm); - Object child_object1 = - any_cast(parent_object1.get_property_value(context, "child_property")); - Int value = any_cast(child_object1.get_property_value(context, "value")); - - auto child_table = ObjectStore::table_for_object_type(new_realm->read_group(), "child_table"); - Obj child_object2 = child_table->create_object(); - child_object2.set("value", value); - - Object parent_object2(new_realm, "parent_table", 1); - parent_object2.set_property_value(context, "child_property", util::Any(child_object2)); - })); - - REQUIRE(realm->schema_version() == 2); - REQUIRE(parent_table->size() == 2); - REQUIRE(child_table->size() == 2); - REQUIRE(child_table->is_embedded()); - for (int i = 0; i < 2; i++) { - Object parent_object(realm, "parent_table", i); - CppContext context(realm); - Object child_object = - any_cast(parent_object.get_property_value(context, "child_property")); - Int value = any_cast(child_object.get_property_value(context, "value")); - REQUIRE(value == 42); - } - } - SECTION("change table from top-level to embedded, delete objects with 0 incoming links") - { - Schema schema = { - {"child_table", - { - {"value", PropertyType::Int}, - }}, - {"parent_table", - { - {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, - }}, - }; - auto realm = Realm::get_shared_realm(config); - realm->update_schema(schema, 1); - auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); - REQUIRE_FALSE(child_table->is_embedded()); - - REQUIRE_NOTHROW( - realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, nullptr)); - - REQUIRE(realm->schema_version() == 2); - REQUIRE(child_table->is_embedded()); - } + SECTION("change table to embedded - multiple incoming links - resolved automatically") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + {"child_table", + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto child_object_key = child_object.get_key(); + parent_table->create_object().set_all(child_object_key); + parent_table->create_object().set_all(child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 1); + REQUIRE_FALSE(child_table->is_embedded()); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 2); + REQUIRE(child_table->is_embedded()); + for (int i = 0; i < 2; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + Object child_object = + any_cast(parent_object.get_property_value(context, "child_property")); + Int value = any_cast(child_object.get_property_value(context, "value")); + REQUIRE(value == 42); + } + } + SECTION( + "change table from top-level to embedded, delete objects with 0 incoming links, resolved automatically") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + {"child_table", + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + realm->begin_transaction(); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + realm->commit_transaction(); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE(child_table->size() == 1); + REQUIRE(parent_table->size() == 0); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(child_table->is_embedded()); + REQUIRE(child_table->size() == 0); + REQUIRE(parent_table->size() == 0); + } + SECTION("change table from top-level to embedded, delete objects with 1 incoming links. Resolve automatic " + "should not trigger") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + {"child_table", + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + realm->begin_transaction(); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + auto child_object_key = child_object.get_key(); + parent_table->create_object().set_all(child_object_key); + realm->commit_transaction(); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE(child_table->size() == 1); + REQUIRE(parent_table->size() == 1); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(child_table->is_embedded()); + REQUIRE(child_table->size() == 1); + REQUIRE(parent_table->size() == 1); + } } SECTION("schema correctness during migration") { @@ -1283,7 +1316,7 @@ TEST_CASE("migration: Automatic") { // ObjectStore::delete_data_for_object(realm->read_group(), "DetailStudentStatus"); Object old_obj(old_realm, "EmpDetails", 0); Object new_obj(new_realm, "EmpDetails", 0); - + CppContext ctx1(old_realm); CppContext ctx2(new_realm); auto val = old_obj.get_property_value(ctx1, "EmployeeId"); @@ -1368,7 +1401,7 @@ TEST_CASE("migration: Automatic") { CppContext ctx(old_realm); Object obj = Object::get_for_primary_key(ctx, old_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); - + REQUIRE(any_cast(obj.get_property_value(ctx, "bool")) == true); REQUIRE(any_cast(obj.get_property_value(ctx, "int")) == 5); REQUIRE(any_cast(obj.get_property_value(ctx, "float")) == 2.2f); @@ -1380,19 +1413,19 @@ TEST_CASE("migration: Automatic") { ObjectId("000000000000000000000001")); REQUIRE(any_cast(obj.get_property_value(ctx, "decimal")) == Decimal128("123.45e6")); - + auto link = any_cast(obj.get_property_value(ctx, "object")); REQUIRE(link.is_valid()); REQUIRE(any_cast(link.get_property_value(ctx, "value")) == 10); - + auto list = any_cast(obj.get_property_value(ctx, "array")); REQUIRE(list.size() == 1); - + CppContext list_ctx(ctx, obj.obj(), *obj.get_object_schema().property_for_name("array")); link = any_cast(list.get(list_ctx, 0)); REQUIRE(link.is_valid()); REQUIRE(any_cast(link.get_property_value(list_ctx, "value")) == 20); - + CppContext ctx2(new_realm); obj = Object::get_for_primary_key(ctx, new_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); @@ -1432,15 +1465,15 @@ TEST_CASE("migration: Automatic") { CppContext ctx(new_realm); Object obj = Object::get_for_primary_key(ctx, new_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); - - + + auto link = any_cast(obj.get_property_value(ctx, "object")); REQUIRE(link.is_valid()); REQUIRE(any_cast(link.get_property_value(ctx, "value")) == 10); - + auto list = any_cast(obj.get_property_value(ctx, "array")); REQUIRE(list.size() == 1); - + CppContext list_ctx(ctx, obj.obj(), *obj.get_object_schema().property_for_name("array")); link = any_cast(list.get(list_ctx, 0)); REQUIRE(link.is_valid()); @@ -1453,59 +1486,59 @@ TEST_CASE("migration: Automatic") { CppContext ctx(new_realm); Object obj = Object::get_for_primary_key(ctx, new_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); - + REQUIRE(any_cast(obj.get_property_value(ctx, "bool")) == true); obj.set_property_value(ctx, "bool", util::Any(false)); REQUIRE(any_cast(obj.get_property_value(ctx, "bool")) == false); - + REQUIRE(any_cast(obj.get_property_value(ctx, "int")) == 5); obj.set_property_value(ctx, "int", util::Any(INT64_C(6))); REQUIRE(any_cast(obj.get_property_value(ctx, "int")) == 6); - + REQUIRE(any_cast(obj.get_property_value(ctx, "float")) == 2.2f); obj.set_property_value(ctx, "float", util::Any(1.23f)); REQUIRE(any_cast(obj.get_property_value(ctx, "float")) == 1.23f); - + REQUIRE(any_cast(obj.get_property_value(ctx, "double")) == 3.3); obj.set_property_value(ctx, "double", util::Any(1.23)); REQUIRE(any_cast(obj.get_property_value(ctx, "double")) == 1.23); - + REQUIRE(any_cast(obj.get_property_value(ctx, "string")) == "hello"); obj.set_property_value(ctx, "string", util::Any("abc"s)); REQUIRE(any_cast(obj.get_property_value(ctx, "string")) == "abc"); - + REQUIRE(any_cast(obj.get_property_value(ctx, "data")) == "olleh"); obj.set_property_value(ctx, "data", util::Any("abc"s)); REQUIRE(any_cast(obj.get_property_value(ctx, "data")) == "abc"); - + REQUIRE(any_cast(obj.get_property_value(ctx, "date")) == Timestamp(10, 20)); obj.set_property_value(ctx, "date", util::Any(Timestamp(1, 2))); REQUIRE(any_cast(obj.get_property_value(ctx, "date")) == Timestamp(1, 2)); - + REQUIRE(any_cast(obj.get_property_value(ctx, "object id")) == ObjectId("000000000000000000000001")); ObjectId generated = ObjectId::gen(); obj.set_property_value(ctx, "object id", util::Any(generated)); REQUIRE(any_cast(obj.get_property_value(ctx, "object id")) == generated); - + REQUIRE(any_cast(obj.get_property_value(ctx, "decimal")) == Decimal128("123.45e6")); obj.set_property_value(ctx, "decimal", util::Any(Decimal128("77.88E-99"))); REQUIRE(any_cast(obj.get_property_value(ctx, "decimal")) == Decimal128("77.88E-99")); - + Object linked_obj(new_realm, "link target", 0); Object new_obj(new_realm, get_table(new_realm, "link target")->create_object()); - + auto linking = any_cast(linked_obj.get_property_value(ctx, "origin")); REQUIRE(linking.size() == 1); - + REQUIRE(any_cast(obj.get_property_value(ctx, "object")).obj().get_key() == linked_obj.obj().get_key()); obj.set_property_value(ctx, "object", util::Any(new_obj)); REQUIRE(any_cast(obj.get_property_value(ctx, "object")).obj().get_key() == new_obj.obj().get_key()); - + REQUIRE(linking.size() == 0); }); } @@ -1513,11 +1546,11 @@ TEST_CASE("migration: Automatic") { SECTION("create object in new realm") { realm->update_schema(schema, 2, [&values](auto, auto new_realm, Schema&) { REQUIRE(new_realm->is_in_transaction()); - + CppContext ctx(new_realm); any_cast(values)["pk"] = INT64_C(2); Object obj = Object::create(ctx, new_realm, "all types", values); - + REQUIRE(get_table(new_realm, "all types")->size() == 2); REQUIRE(get_table(new_realm, "link target")->size() == 2); REQUIRE(get_table(new_realm, "array target")->size() == 2); @@ -1556,7 +1589,7 @@ TEST_CASE("migration: Automatic") { schema = set_type(schema, "all types", "pk", PropertyType::String); realm->update_schema(schema, 2, [](auto, auto new_realm, auto&) { Object obj(new_realm, "all types", 0); - + CppContext ctx(new_realm); obj.set_property_value(ctx, "pk", util::Any("1"s)); }); @@ -1575,7 +1608,7 @@ TEST_CASE("migration: Automatic") { Object old_obj(new_realm, "all types", 0); CppContext ctx(new_realm); old_obj.set_property_value(ctx, "pk", util::Any(INT64_C(5))); - + REQUIRE_NOTHROW(Object::create(ctx, new_realm, "all types", values)); }; REQUIRE_NOTHROW(realm->update_schema(schema, 2, good_migration)); @@ -1890,7 +1923,7 @@ TEST_CASE("migration: Automatic") { table->create_object_with_primary_key(10); else table->create_object().set_all(10); - + realm->commit_transaction(); }; @@ -1958,7 +1991,7 @@ TEST_CASE("migration: Automatic") { init(schema); REQUIRE_NOTHROW(realm->update_schema(new_schema, 2, [](auto, auto realm, Schema& schema) { ObjectStore::rename_property(realm->read_group(), schema, "object", "value", "new"); - + CppContext ctx(realm); util::Any values = AnyDict{{"new", INT64_C(11)}}; Object::create(ctx, realm, "object", values); @@ -2265,9 +2298,9 @@ TEST_CASE("migration: SoftResetFile") { }}, }; -// To verify that the file has actually be deleted and recreated, on -// non-Windows we need to hold an open file handle to the old file to force -// using a new inode, but on Windows we *can't* + // To verify that the file has actually be deleted and recreated, on + // non-Windows we need to hold an open file handle to the old file to force + // using a new inode, but on Windows we *can't* #ifdef _WIN32 auto get_fileid = [&] { // this is wrong for non-ascii but it's what core does @@ -2362,9 +2395,9 @@ TEST_CASE("migration: HardResetFile") { }}, }; -// To verify that the file has actually be deleted and recreated, on -// non-Windows we need to hold an open file handle to the old file to force -// using a new inode, but on Windows we *can't* + // To verify that the file has actually be deleted and recreated, on + // non-Windows we need to hold an open file handle to the old file to force + // using a new inode, but on Windows we *can't* #ifdef _WIN32 auto get_fileid = [&] { // this is wrong for non-ascii but it's what core does From c0f60171eec331789eda745bcebbc4828a762ad5 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 11 Aug 2022 18:10:09 +0100 Subject: [PATCH 03/25] appease format checks --- src/realm.h | 2 +- src/realm/object-store/object_store.cpp | 7 +++---- src/realm/object-store/shared_realm.hpp | 4 ++-- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/realm.h b/src/realm.h index f930de16547..88a1dfdf38e 100644 --- a/src/realm.h +++ b/src/realm.h @@ -825,7 +825,7 @@ RLM_API bool realm_config_get_cached(realm_config_t*) RLM_API_NOEXCEPT; * Allow realm to manage automatically embedded objects when a migration from TopLevel to Embedded takes place. */ RLM_API void realm_config_set_automatic_backlink_handling(realm_config_t*, bool) RLM_API_NOEXCEPT; - + /** * Create a custom scheduler object from callback functions. * diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index e5fd1f15c83..ca10c5a6e0e 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -845,11 +845,10 @@ static void apply_post_migration_changes(Group& group, std::vector // to each a different parent // object_schema->handle_automatically_backlinks_for_embedded_object = true; - if(object_schema->table_type == ObjectSchema::ObjectType::Embedded) - { + if (object_schema->table_type == ObjectSchema::ObjectType::Embedded) { auto original_object_schema = initial_schema.find(object_schema->name); - if(original_object_schema != initial_schema.end() && original_object_schema->table_type == ObjectSchema::ObjectType::TopLevel) - { + if (original_object_schema != initial_schema.end() && + original_object_schema->table_type == ObjectSchema::ObjectType::TopLevel) { auto table = table_for_object_schema(group, *original_object_schema); std::vector objects_to_erase; std::vector objects_to_fix_backlinks; diff --git a/src/realm/object-store/shared_realm.hpp b/src/realm/object-store/shared_realm.hpp index 88556309caf..6a4972100e6 100644 --- a/src/realm/object-store/shared_realm.hpp +++ b/src/realm/object-store/shared_realm.hpp @@ -175,8 +175,8 @@ struct RealmConfig { // Disable automatic backup at file format upgrade by setting to false bool backup_at_file_format_change = true; - - //delete embedded orphan objects + + // delete embedded orphan objects bool automatic_handle_backlicks_in_migrations = false; }; From 29da61351546facdce87c9422659b2f3b360d165 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 12 Aug 2022 11:08:26 +0100 Subject: [PATCH 04/25] code review minor fixes --- test/object-store/migrations.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index b9e6478ed96..96aaeaf5e3d 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1127,8 +1127,9 @@ TEST_CASE("migration: Automatic") { REQUIRE(child_table->size() == 0); REQUIRE(parent_table->size() == 0); } - SECTION("change table from top-level to embedded, delete objects with 1 incoming links. Resolve automatic " - "should not trigger") { + SECTION("change table from top-level to embedded, migration allowed, embedded object with 1 incoming link. " + "Resolve automatic " + "should not be triggered") { InMemoryTestFile config; config.automatic_handle_backlicks_in_migrations = true; Schema schema = { From a2eae07709c808f6ee21f46335e5ae10973ca466 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 12 Aug 2022 11:23:00 +0100 Subject: [PATCH 05/25] fix space problems --- test/object-store/migrations.cpp | 71 ++++++++++++++++---------------- 1 file changed, 36 insertions(+), 35 deletions(-) diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 96aaeaf5e3d..b40c2e106bb 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -631,6 +631,7 @@ TEST_CASE("migration: Automatic") { REQUIRE_THROWS(realm->update_schema(set_table_type(schema, "object", ObjectType::Embedded), 2, [](auto, auto, auto&) {})); } + SECTION("change table to embedded - multiple incoming link per object") { Schema schema = { {"child_table", @@ -1317,7 +1318,7 @@ TEST_CASE("migration: Automatic") { // ObjectStore::delete_data_for_object(realm->read_group(), "DetailStudentStatus"); Object old_obj(old_realm, "EmpDetails", 0); Object new_obj(new_realm, "EmpDetails", 0); - + CppContext ctx1(old_realm); CppContext ctx2(new_realm); auto val = old_obj.get_property_value(ctx1, "EmployeeId"); @@ -1402,7 +1403,7 @@ TEST_CASE("migration: Automatic") { CppContext ctx(old_realm); Object obj = Object::get_for_primary_key(ctx, old_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); - + REQUIRE(any_cast(obj.get_property_value(ctx, "bool")) == true); REQUIRE(any_cast(obj.get_property_value(ctx, "int")) == 5); REQUIRE(any_cast(obj.get_property_value(ctx, "float")) == 2.2f); @@ -1414,19 +1415,19 @@ TEST_CASE("migration: Automatic") { ObjectId("000000000000000000000001")); REQUIRE(any_cast(obj.get_property_value(ctx, "decimal")) == Decimal128("123.45e6")); - + auto link = any_cast(obj.get_property_value(ctx, "object")); REQUIRE(link.is_valid()); REQUIRE(any_cast(link.get_property_value(ctx, "value")) == 10); - + auto list = any_cast(obj.get_property_value(ctx, "array")); REQUIRE(list.size() == 1); - + CppContext list_ctx(ctx, obj.obj(), *obj.get_object_schema().property_for_name("array")); link = any_cast(list.get(list_ctx, 0)); REQUIRE(link.is_valid()); REQUIRE(any_cast(link.get_property_value(list_ctx, "value")) == 20); - + CppContext ctx2(new_realm); obj = Object::get_for_primary_key(ctx, new_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); @@ -1466,15 +1467,15 @@ TEST_CASE("migration: Automatic") { CppContext ctx(new_realm); Object obj = Object::get_for_primary_key(ctx, new_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); - - + + auto link = any_cast(obj.get_property_value(ctx, "object")); REQUIRE(link.is_valid()); REQUIRE(any_cast(link.get_property_value(ctx, "value")) == 10); - + auto list = any_cast(obj.get_property_value(ctx, "array")); REQUIRE(list.size() == 1); - + CppContext list_ctx(ctx, obj.obj(), *obj.get_object_schema().property_for_name("array")); link = any_cast(list.get(list_ctx, 0)); REQUIRE(link.is_valid()); @@ -1487,59 +1488,59 @@ TEST_CASE("migration: Automatic") { CppContext ctx(new_realm); Object obj = Object::get_for_primary_key(ctx, new_realm, "all types", util::Any(INT64_C(1))); REQUIRE(obj.is_valid()); - + REQUIRE(any_cast(obj.get_property_value(ctx, "bool")) == true); obj.set_property_value(ctx, "bool", util::Any(false)); REQUIRE(any_cast(obj.get_property_value(ctx, "bool")) == false); - + REQUIRE(any_cast(obj.get_property_value(ctx, "int")) == 5); obj.set_property_value(ctx, "int", util::Any(INT64_C(6))); REQUIRE(any_cast(obj.get_property_value(ctx, "int")) == 6); - + REQUIRE(any_cast(obj.get_property_value(ctx, "float")) == 2.2f); obj.set_property_value(ctx, "float", util::Any(1.23f)); REQUIRE(any_cast(obj.get_property_value(ctx, "float")) == 1.23f); - + REQUIRE(any_cast(obj.get_property_value(ctx, "double")) == 3.3); obj.set_property_value(ctx, "double", util::Any(1.23)); REQUIRE(any_cast(obj.get_property_value(ctx, "double")) == 1.23); - + REQUIRE(any_cast(obj.get_property_value(ctx, "string")) == "hello"); obj.set_property_value(ctx, "string", util::Any("abc"s)); REQUIRE(any_cast(obj.get_property_value(ctx, "string")) == "abc"); - + REQUIRE(any_cast(obj.get_property_value(ctx, "data")) == "olleh"); obj.set_property_value(ctx, "data", util::Any("abc"s)); REQUIRE(any_cast(obj.get_property_value(ctx, "data")) == "abc"); - + REQUIRE(any_cast(obj.get_property_value(ctx, "date")) == Timestamp(10, 20)); obj.set_property_value(ctx, "date", util::Any(Timestamp(1, 2))); REQUIRE(any_cast(obj.get_property_value(ctx, "date")) == Timestamp(1, 2)); - + REQUIRE(any_cast(obj.get_property_value(ctx, "object id")) == ObjectId("000000000000000000000001")); ObjectId generated = ObjectId::gen(); obj.set_property_value(ctx, "object id", util::Any(generated)); REQUIRE(any_cast(obj.get_property_value(ctx, "object id")) == generated); - + REQUIRE(any_cast(obj.get_property_value(ctx, "decimal")) == Decimal128("123.45e6")); obj.set_property_value(ctx, "decimal", util::Any(Decimal128("77.88E-99"))); REQUIRE(any_cast(obj.get_property_value(ctx, "decimal")) == Decimal128("77.88E-99")); - + Object linked_obj(new_realm, "link target", 0); Object new_obj(new_realm, get_table(new_realm, "link target")->create_object()); - + auto linking = any_cast(linked_obj.get_property_value(ctx, "origin")); REQUIRE(linking.size() == 1); - + REQUIRE(any_cast(obj.get_property_value(ctx, "object")).obj().get_key() == linked_obj.obj().get_key()); obj.set_property_value(ctx, "object", util::Any(new_obj)); REQUIRE(any_cast(obj.get_property_value(ctx, "object")).obj().get_key() == new_obj.obj().get_key()); - + REQUIRE(linking.size() == 0); }); } @@ -1547,11 +1548,11 @@ TEST_CASE("migration: Automatic") { SECTION("create object in new realm") { realm->update_schema(schema, 2, [&values](auto, auto new_realm, Schema&) { REQUIRE(new_realm->is_in_transaction()); - + CppContext ctx(new_realm); any_cast(values)["pk"] = INT64_C(2); Object obj = Object::create(ctx, new_realm, "all types", values); - + REQUIRE(get_table(new_realm, "all types")->size() == 2); REQUIRE(get_table(new_realm, "link target")->size() == 2); REQUIRE(get_table(new_realm, "array target")->size() == 2); @@ -1590,7 +1591,7 @@ TEST_CASE("migration: Automatic") { schema = set_type(schema, "all types", "pk", PropertyType::String); realm->update_schema(schema, 2, [](auto, auto new_realm, auto&) { Object obj(new_realm, "all types", 0); - + CppContext ctx(new_realm); obj.set_property_value(ctx, "pk", util::Any("1"s)); }); @@ -1609,7 +1610,7 @@ TEST_CASE("migration: Automatic") { Object old_obj(new_realm, "all types", 0); CppContext ctx(new_realm); old_obj.set_property_value(ctx, "pk", util::Any(INT64_C(5))); - + REQUIRE_NOTHROW(Object::create(ctx, new_realm, "all types", values)); }; REQUIRE_NOTHROW(realm->update_schema(schema, 2, good_migration)); @@ -1924,7 +1925,7 @@ TEST_CASE("migration: Automatic") { table->create_object_with_primary_key(10); else table->create_object().set_all(10); - + realm->commit_transaction(); }; @@ -1992,7 +1993,7 @@ TEST_CASE("migration: Automatic") { init(schema); REQUIRE_NOTHROW(realm->update_schema(new_schema, 2, [](auto, auto realm, Schema& schema) { ObjectStore::rename_property(realm->read_group(), schema, "object", "value", "new"); - + CppContext ctx(realm); util::Any values = AnyDict{{"new", INT64_C(11)}}; Object::create(ctx, realm, "object", values); @@ -2299,9 +2300,9 @@ TEST_CASE("migration: SoftResetFile") { }}, }; - // To verify that the file has actually be deleted and recreated, on - // non-Windows we need to hold an open file handle to the old file to force - // using a new inode, but on Windows we *can't* +// To verify that the file has actually be deleted and recreated, on +// non-Windows we need to hold an open file handle to the old file to force +// using a new inode, but on Windows we *can't* #ifdef _WIN32 auto get_fileid = [&] { // this is wrong for non-ascii but it's what core does @@ -2396,9 +2397,9 @@ TEST_CASE("migration: HardResetFile") { }}, }; - // To verify that the file has actually be deleted and recreated, on - // non-Windows we need to hold an open file handle to the old file to force - // using a new inode, but on Windows we *can't* +// To verify that the file has actually be deleted and recreated, on +// non-Windows we need to hold an open file handle to the old file to force +// using a new inode, but on Windows we *can't* #ifdef _WIN32 auto get_fileid = [&] { // this is wrong for non-ascii but it's what core does From 41343578d4cb3c7037fb2b17ef2ff6712131737c Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Tue, 16 Aug 2022 18:47:20 +0100 Subject: [PATCH 06/25] extend copy to dictionary and set for backlinks and object properties --- src/realm/obj.cpp | 65 +++++++++++++++++++++---- test/object-store/migrations.cpp | 82 ++++++++++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 8 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 020a23e51f0..fc923bc70ff 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1962,26 +1962,55 @@ void Obj::assign(const Obj& other) void Obj::dup_and_handle_multiple_backlinks() { + REALM_ASSERT(!m_table->get_primary_key_column()); + auto copy_links = [this](ColKey col) { auto t = m_table->get_opposite_table(col); auto c = m_table->get_opposite_column(col); auto backlinks = get_all_backlinks(col); - for (auto bl : backlinks) { auto obj = m_table->create_object(); obj.copy_other_object(*this); + auto linking_obj = t->get_object(bl); - if (c.get_type() == col_type_Link) { + + // the central piece that is missing is how we handle if there are multiple + // embedded objects that are parent of the one just created. + + + // backlink from list + if (c.get_type() == col_type_LinkList) { + auto linking_obj_list = linking_obj.get_linklist(c); + auto n = linking_obj_list.find_first(get_key()); + REALM_ASSERT(n != realm::npos); + linking_obj_list.set(n, obj.get_key()); + } + // backlink from set + else if (c.get_attrs().test(col_attr_Set)) { + auto linking_obj_set = linking_obj.get_setbase_ptr(c); + REALM_ASSERT(linking_obj_set); + linking_obj_set->insert_any(obj.get_key()); + } + // backlink from dictionary + else if (c.get_attrs().test(col_attr_Dictionary)) { + auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(c); + REALM_ASSERT(linking_obj_dictionary); + auto pos = linking_obj_dictionary->find_any(get_key()); + REALM_ASSERT(pos != realm::npos); + Mixed key = linking_obj_dictionary->get_key(pos); + linking_obj_dictionary->insert(key, obj.get_key()); + } + // backlink from mixed + else if (c.get_type() == col_type_Mixed && linking_obj.get_any(c).get_type() == type_TypedLink) { + REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); + linking_obj.set_any(c, obj.get_key()); + } + // normal backlink + else if (c.get_type() == col_type_Link) { // Single link REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); linking_obj.set(c, obj.get_key()); } - else { - auto l = linking_obj.get_linklist(c); - auto n = l.find_first(get_key()); - REALM_ASSERT(n != realm::npos); - l.set(n, obj.get_key()); - } } return false; }; @@ -2003,6 +2032,26 @@ void Obj::copy_other_object(const Obj& other) dst_list->insert_any(i, val); } } + else if (col.get_attrs().test(col_attr_Set)) { + auto src_set = other.get_setbase_ptr(col); + auto dst_set = get_setbase_ptr(col); + auto sz = src_set->size(); + dst_set->clear(); + for (size_t i = 0; i < sz; ++i) { + Mixed val = src_set->get_any(i); + dst_set->insert_any(val); + } + } + else if (col.get_attrs().test(col_attr_Dictionary)) { + auto src_dictionary = other.get_dictionary_ptr(col); + auto dst_dictionary = get_dictionary_ptr(col); + auto sz = src_dictionary->size(); + dst_dictionary->clear(); + for (size_t i = 0; i < sz; ++i) { + const auto& [key, value] = src_dictionary->get_pair(i); + dst_dictionary->insert(key, value); + } + } else { Mixed val = other.get_any(col); if (val.is_null()) { diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index b40c2e106bb..f6da1a88e43 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1165,6 +1165,88 @@ TEST_CASE("migration: Automatic") { REQUIRE(child_table->size() == 1); REQUIRE(parent_table->size() == 1); } + SECTION("change table to embedded - multiple incoming links - resolved automatically + copy set, dictionary " + "verification") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + {"child_table", + {{"value", PropertyType::Int}, + {"value_dict", PropertyType::Dictionary | PropertyType::Int}, + {"links_dict", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, + "target"}}}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + {"target", + { + {"value", PropertyType::Int}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + ColKey col_dict_value = child_table->get_column_key("value_dict"); + ColKey col_dict_links = child_table->get_column_key("links_dict"); + object_store::Dictionary dict_vals(realm, child_object, col_dict_value); + dict_vals.insert("test", 10); + + auto target_table = ObjectStore::table_for_object_type(realm->read_group(), "target"); + Obj target_object = target_table->create_object(); + target_object.set("value", 10); + object_store::Dictionary dict_links(realm, child_object, col_dict_links); + dict_links.insert("link", target_object.get_key()); + + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto child_object_key = child_object.get_key(); + parent_table->create_object().set_all(child_object_key); + parent_table->create_object().set_all(child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 1); + REQUIRE(target_table->size() == 1); + REQUIRE(dict_vals.size() == 1); + REQUIRE(dict_links.size() == 1); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + REQUIRE_FALSE(target_table->is_embedded()); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 2); + REQUIRE(target_table->size() == 1); + REQUIRE(child_table->is_embedded()); + REQUIRE_FALSE(target_table->is_embedded()); + + for (int i = 0; i < 2; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + Object child_object = + any_cast(parent_object.get_property_value(context, "child_property")); + Int value = any_cast(child_object.get_property_value(context, "value")); + REQUIRE(value == 42); + auto value_dictionary = util::any_cast( + child_object.get_property_value(context, "value_dict")); + REQUIRE(value_dictionary.size() == 1); + auto pair_val = value_dictionary.get_pair(0); + REQUIRE(pair_val.first == "test"); + REQUIRE(pair_val.second == 10); + auto links_dictionary = util::any_cast( + child_object.get_property_value(context, "links_dict")); + REQUIRE(links_dictionary.size() == 1); + auto pair_link = links_dictionary.get_pair(0); + REQUIRE(pair_link.first == "link"); + REQUIRE_FALSE(pair_link.second.is_unresolved_link()); + REQUIRE(pair_link.second.get() == target_object.get_key()); + } + } } SECTION("schema correctness during migration") { From a2d541c0b501aae3663b3dd0cdc3a8dc7441efa9 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 17 Aug 2022 11:51:10 +0100 Subject: [PATCH 07/25] added entry to the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ff2d4afd071..50ba7bf9388 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) * Notify when read transaction version is advanced. ([PR #5704](https://github.com/realm/realm-core/pull/5704)). +* Automatic handling backlinks migration. ([PR #5737](https://github.com/realm/realm-core/pull/5737)). ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) From c0e90c284ac74c91e7ec8809cb6f43cc2cf11512 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 17 Aug 2022 14:45:41 +0100 Subject: [PATCH 08/25] test in order to copy sets and dictionaries with values and links --- test/object-store/migrations.cpp | 36 +++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index f6da1a88e43..02de0d21b8a 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1170,11 +1170,15 @@ TEST_CASE("migration: Automatic") { InMemoryTestFile config; config.automatic_handle_backlicks_in_migrations = true; Schema schema = { - {"child_table", - {{"value", PropertyType::Int}, - {"value_dict", PropertyType::Dictionary | PropertyType::Int}, - {"links_dict", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, - "target"}}}, + { + "child_table", + {{"value", PropertyType::Int}, + {"value_dict", PropertyType::Dictionary | PropertyType::Int}, + {"links_dict", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, + "target"}, + {"value_set", PropertyType::Set | PropertyType::Int}, + {"links_set", PropertyType::Set | PropertyType::Object, "target"}}, + }, {"parent_table", { {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, @@ -1192,14 +1196,22 @@ TEST_CASE("migration: Automatic") { child_object.set("value", 42); ColKey col_dict_value = child_table->get_column_key("value_dict"); ColKey col_dict_links = child_table->get_column_key("links_dict"); + ColKey col_set_value = child_table->get_column_key("value_set"); + ColKey col_set_links = child_table->get_column_key("links_set"); object_store::Dictionary dict_vals(realm, child_object, col_dict_value); dict_vals.insert("test", 10); + object_store::Set set_vals(realm, child_object, col_set_value); + set_vals.insert(10); + set_vals.insert(11); + set_vals.insert(9); auto target_table = ObjectStore::table_for_object_type(realm->read_group(), "target"); Obj target_object = target_table->create_object(); target_object.set("value", 10); object_store::Dictionary dict_links(realm, child_object, col_dict_links); dict_links.insert("link", target_object.get_key()); + object_store::Set set_links(realm, child_object, col_set_links); + set_links.insert(target_object.get_key()); auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); auto child_object_key = child_object.get_key(); @@ -1211,6 +1223,8 @@ TEST_CASE("migration: Automatic") { REQUIRE(target_table->size() == 1); REQUIRE(dict_vals.size() == 1); REQUIRE(dict_links.size() == 1); + REQUIRE(set_vals.size() == 3); + REQUIRE(set_links.size() == 1); REQUIRE_FALSE(child_table->is_embedded()); REQUIRE_FALSE(parent_table->is_embedded()); REQUIRE_FALSE(target_table->is_embedded()); @@ -1245,6 +1259,18 @@ TEST_CASE("migration: Automatic") { REQUIRE(pair_link.first == "link"); REQUIRE_FALSE(pair_link.second.is_unresolved_link()); REQUIRE(pair_link.second.get() == target_object.get_key()); + + + auto value_set = util::any_cast( + child_object.get_property_value(context, "value_set")); + REQUIRE(value_set.size() == 3); + REQUIRE(value_set.get_any(0) == 9); + REQUIRE(value_set.get_any(1) == 10); + REQUIRE(value_set.get_any(2) == 11); + auto links_set = util::any_cast( + child_object.get_property_value(context, "links_set")); + REQUIRE(links_set.size() == 1); + REQUIRE(links_set.get_any(0).get() == target_object.get_key()); } } } From 1ea939012bb9c87958b279f12e262dd0724bd1db Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 17 Aug 2022 17:22:53 +0100 Subject: [PATCH 09/25] test backlick copy after migration --- src/realm/obj.cpp | 10 ++- test/object-store/migrations.cpp | 139 +++++++++++++++++++++++++++++++ 2 files changed, 145 insertions(+), 4 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index fc923bc70ff..660b5b89396 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1977,7 +1977,6 @@ void Obj::dup_and_handle_multiple_backlinks() // the central piece that is missing is how we handle if there are multiple // embedded objects that are parent of the one just created. - // backlink from list if (c.get_type() == col_type_LinkList) { auto linking_obj_list = linking_obj.get_linklist(c); @@ -1987,21 +1986,24 @@ void Obj::dup_and_handle_multiple_backlinks() } // backlink from set else if (c.get_attrs().test(col_attr_Set)) { + // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store + // multiple times. update_schema should throw longer before to perform this copy. auto linking_obj_set = linking_obj.get_setbase_ptr(c); - REALM_ASSERT(linking_obj_set); + auto pos = linking_obj_set->find_any(Mixed{get_key()}); + REALM_ASSERT(pos != realm::npos); linking_obj_set->insert_any(obj.get_key()); } // backlink from dictionary else if (c.get_attrs().test(col_attr_Dictionary)) { auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(c); - REALM_ASSERT(linking_obj_dictionary); - auto pos = linking_obj_dictionary->find_any(get_key()); + auto pos = linking_obj_dictionary->find_any(ObjLink{m_table->get_key(), get_key()}); REALM_ASSERT(pos != realm::npos); Mixed key = linking_obj_dictionary->get_key(pos); linking_obj_dictionary->insert(key, obj.get_key()); } // backlink from mixed else if (c.get_type() == col_type_Mixed && linking_obj.get_any(c).get_type() == type_TypedLink) { + // this is not supported yet. REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); linking_obj.set_any(c, obj.get_key()); } diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 02de0d21b8a..cb6d33dd296 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1273,6 +1273,145 @@ TEST_CASE("migration: Automatic") { REQUIRE(links_set.get_any(0).get() == target_object.get_key()); } } + SECTION("change table to embedded - multiple links stored in a dictionary") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + { + "child_table", + {{"value", PropertyType::Int}}, + }, + {"parent_table", + { + {"child_property", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, + "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto parent_object = parent_table->create_object(); + ColKey col_links = parent_table->get_column_key("child_property"); + auto child_object_key = child_object.get_key(); + object_store::Dictionary dict_links(realm, parent_object, col_links); + dict_links.insert("ref", child_object_key); + dict_links.insert("ref1", child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 1); + REQUIRE(child_table->size() == 1); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(parent_table->size() == 1); + REQUIRE(child_table->size() == 2); + REQUIRE(child_table->is_embedded()); + + for (int i = 0; i < 1; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + object_store::Dictionary links_dictionary = any_cast( + parent_object.get_property_value(context, "child_property")); + REQUIRE(links_dictionary.size() == dict_links.size()); + for (size_t i = 0; i < 2; ++i) { + const auto& [key, value] = links_dictionary.get_pair(i); + const auto& [key1, value1] = dict_links.get_pair(i); + REQUIRE(key == key1); + REQUIRE(value == value1); + } + } + } + SECTION("change table to embedded - incoming links stored in a set") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + { + "child_table", + {{"value", PropertyType::Int}}, + }, + {"parent_table", + { + {"child_property", PropertyType::Set | PropertyType::Object, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto parent_object = parent_table->create_object(); + ColKey col_links = parent_table->get_column_key("child_property"); + auto child_object_key = child_object.get_key(); + object_store::Set set_links(realm, parent_object, col_links); + set_links.insert(child_object_key); + // this should not create a new ref (set does not allow dups) + set_links.insert(child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 1); + REQUIRE(child_table->size() == 1); + REQUIRE(set_links.size() == 1); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + + REQUIRE_THROWS(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + } + SECTION("change table to embedded - multiple links stored in linked list") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + { + "child_table", + {{"value", PropertyType::Int}}, + }, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Array, "child_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto child_object_key = child_object.get_key(); + auto parent_object = parent_table->create_object(); + auto list = parent_object.get_linklist("child_property"); + list.insert(0, child_object_key); + list.insert(1, child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 1); + REQUIRE(child_table->size() == 1); + REQUIRE(list.size() == 2); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + REQUIRE(realm->schema_version() == 2); + REQUIRE(parent_table->size() == 1); + REQUIRE(child_table->size() == 2); + REQUIRE(child_table->is_embedded()); + auto linklist = parent_object.get_linklist("child_property"); + REQUIRE(linklist.size() == 2); + for (size_t i = 1; i < linklist.size(); ++i) { + REQUIRE(linklist.get(i - 1) != linklist.get(i)); + } + } } SECTION("schema correctness during migration") { From f1676a4e41ce85b21f274c284c9b66f3ee424da7 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 18 Aug 2022 17:49:26 +0100 Subject: [PATCH 10/25] code review + better handling ref to other objects for Mixed --- CHANGELOG.md | 2 +- src/realm/obj.cpp | 9 +++---- test/object-store/migrations.cpp | 40 +++++++++++++++++++++++--------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ab074685903..d03f093005b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ### Enhancements * (PR [#????](https://github.com/realm/realm-core/pull/????)) * Notify when read transaction version is advanced. ([PR #5704](https://github.com/realm/realm-core/pull/5704)). -* Automatic handling backlinks migration. ([PR #5737](https://github.com/realm/realm-core/pull/5737)). +* Automatic handling backlinks for schema migrations where a class changes to being embedded. ([PR #5737](https://github.com/realm/realm-core/pull/5737)). ### Fixed * ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 660b5b89396..3f26f26984a 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1988,10 +1988,7 @@ void Obj::dup_and_handle_multiple_backlinks() else if (c.get_attrs().test(col_attr_Set)) { // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store // multiple times. update_schema should throw longer before to perform this copy. - auto linking_obj_set = linking_obj.get_setbase_ptr(c); - auto pos = linking_obj_set->find_any(Mixed{get_key()}); - REALM_ASSERT(pos != realm::npos); - linking_obj_set->insert_any(obj.get_key()); + REALM_UNREACHABLE(); } // backlink from dictionary else if (c.get_attrs().test(col_attr_Dictionary)) { @@ -1999,13 +1996,13 @@ void Obj::dup_and_handle_multiple_backlinks() auto pos = linking_obj_dictionary->find_any(ObjLink{m_table->get_key(), get_key()}); REALM_ASSERT(pos != realm::npos); Mixed key = linking_obj_dictionary->get_key(pos); - linking_obj_dictionary->insert(key, obj.get_key()); + linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), obj.get_key()}); } // backlink from mixed else if (c.get_type() == col_type_Mixed && linking_obj.get_any(c).get_type() == type_TypedLink) { // this is not supported yet. REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); - linking_obj.set_any(c, obj.get_key()); + linking_obj.set_any(c, ObjLink{m_table->get_key(), obj.get_key()}); } // normal backlink else if (c.get_type() == col_type_Link) { diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index cb6d33dd296..f8702b5c3ce 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1172,17 +1172,18 @@ TEST_CASE("migration: Automatic") { Schema schema = { { "child_table", - {{"value", PropertyType::Int}, - {"value_dict", PropertyType::Dictionary | PropertyType::Int}, - {"links_dict", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, - "target"}, - {"value_set", PropertyType::Set | PropertyType::Int}, - {"links_set", PropertyType::Set | PropertyType::Object, "target"}}, + { + {"value", PropertyType::Int}, + {"value_dict", PropertyType::Dictionary | PropertyType::Int}, + {"links_dict", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, + "target"}, + {"value_set", PropertyType::Set | PropertyType::Int}, + {"links_set", PropertyType::Set | PropertyType::Object, "target"}, + }, }, {"parent_table", - { - {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, - }}, + {{"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + {"mixed_links", PropertyType::Dictionary | PropertyType::Mixed | PropertyType::Nullable}}}, {"target", { {"value", PropertyType::Int}, @@ -1215,8 +1216,15 @@ TEST_CASE("migration: Automatic") { auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); auto child_object_key = child_object.get_key(); - parent_table->create_object().set_all(child_object_key); - parent_table->create_object().set_all(child_object_key); + auto o1 = parent_table->create_object(); + auto o2 = parent_table->create_object(); + ColKey col_mixed_links = parent_table->get_column_key("mixed_links"); + object_store::Dictionary mixed_links_o1(realm, o1, col_mixed_links); + mixed_links_o1.insert("ref_mixed_link", ObjLink{target_table->get_key(), target_object.get_key()}); + object_store::Dictionary mixed_links_o2(realm, o2, col_mixed_links); + mixed_links_o2.insert("ref_mixed_link", ObjLink{target_table->get_key(), target_object.get_key()}); + o1.set_all(child_object_key); + o2.set_all(child_object_key); realm->commit_transaction(); REQUIRE(parent_table->size() == 2); REQUIRE(child_table->size() == 1); @@ -1225,6 +1233,8 @@ TEST_CASE("migration: Automatic") { REQUIRE(dict_links.size() == 1); REQUIRE(set_vals.size() == 3); REQUIRE(set_links.size() == 1); + REQUIRE(mixed_links_o1.size() == 1); + REQUIRE(mixed_links_o2.size() == 1); REQUIRE_FALSE(child_table->is_embedded()); REQUIRE_FALSE(parent_table->is_embedded()); REQUIRE_FALSE(target_table->is_embedded()); @@ -1260,6 +1270,14 @@ TEST_CASE("migration: Automatic") { REQUIRE_FALSE(pair_link.second.is_unresolved_link()); REQUIRE(pair_link.second.get() == target_object.get_key()); + auto mixed_links = util::any_cast( + parent_object.get_property_value(context, "mixed_links")); + REQUIRE(mixed_links.size() == 1); + auto pair_mixed_link = mixed_links.get_pair(0); + REQUIRE(pair_mixed_link.first == "ref_mixed_link"); + REQUIRE_FALSE(pair_mixed_link.second.is_unresolved_link()); + REQUIRE(pair_mixed_link.second.get() == target_object.get_key()); + auto value_set = util::any_cast( child_object.get_property_value(context, "value_set")); From 89f51a15ce09c0ac53df63023a70d32e66cc7600 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 19 Aug 2022 15:24:36 +0100 Subject: [PATCH 11/25] adding breaking test in order to exercise condition in which outgoing link points to some embedded object --- src/realm/obj.cpp | 100 ++++++------ src/realm/obj.hpp | 5 +- src/realm/object-store/object_store.cpp | 4 +- test/object-store/migrations.cpp | 196 ++++++++++++++++++------ 4 files changed, 209 insertions(+), 96 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 3f26f26984a..69bc865bfb0 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1443,6 +1443,7 @@ Obj& Obj::set(ColKey col_key, ObjKey target_key, bool is_default) if (type != ColumnTypeTraits::column_id) throw LogicError(LogicError::illegal_type); TableRef target_table = get_target_table(col_key); + auto n = target_table->get_name(); TableKey target_table_key = target_table->get_key(); if (target_key) { ClusterTree* ct = target_key.is_unresolved() ? target_table->m_tombstones.get() : &target_table->m_clusters; @@ -1936,7 +1937,7 @@ bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) void Obj::assign(const Obj& other) { - copy_other_object(other); + dup(other); auto copy_links = [this, &other](ColKey col) { auto t = m_table->get_opposite_table(col); auto c = m_table->get_opposite_column(col); @@ -1960,67 +1961,28 @@ void Obj::assign(const Obj& other) m_table->for_each_backlink_column(copy_links); } -void Obj::dup_and_handle_multiple_backlinks() +bool Obj::handle_multiple_backlinks_during_schema_migration(std::vector& embedded_objects_to_fix) { REALM_ASSERT(!m_table->get_primary_key_column()); - auto copy_links = [this](ColKey col) { - auto t = m_table->get_opposite_table(col); - auto c = m_table->get_opposite_column(col); + auto opposite_table = m_table->get_opposite_table(col); + auto opposite_column = m_table->get_opposite_column(col); auto backlinks = get_all_backlinks(col); - for (auto bl : backlinks) { - auto obj = m_table->create_object(); - obj.copy_other_object(*this); - - auto linking_obj = t->get_object(bl); - - // the central piece that is missing is how we handle if there are multiple - // embedded objects that are parent of the one just created. - - // backlink from list - if (c.get_type() == col_type_LinkList) { - auto linking_obj_list = linking_obj.get_linklist(c); - auto n = linking_obj_list.find_first(get_key()); - REALM_ASSERT(n != realm::npos); - linking_obj_list.set(n, obj.get_key()); - } - // backlink from set - else if (c.get_attrs().test(col_attr_Set)) { - // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store - // multiple times. update_schema should throw longer before to perform this copy. - REALM_UNREACHABLE(); - } - // backlink from dictionary - else if (c.get_attrs().test(col_attr_Dictionary)) { - auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(c); - auto pos = linking_obj_dictionary->find_any(ObjLink{m_table->get_key(), get_key()}); - REALM_ASSERT(pos != realm::npos); - Mixed key = linking_obj_dictionary->get_key(pos); - linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), obj.get_key()}); - } - // backlink from mixed - else if (c.get_type() == col_type_Mixed && linking_obj.get_any(c).get_type() == type_TypedLink) { - // this is not supported yet. - REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); - linking_obj.set_any(c, ObjLink{m_table->get_key(), obj.get_key()}); - } - // normal backlink - else if (c.get_type() == col_type_Link) { - // Single link - REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == get_key()); - linking_obj.set(c, obj.get_key()); - } + for (auto backlink : backlinks) { + clone_object_during_migration(opposite_table, opposite_column, backlink); } return false; }; m_table->for_each_backlink_column(copy_links); + return true; // everything got cloned, so the original object can now be deleted } -void Obj::copy_other_object(const Obj& other) +void Obj::dup(const Obj& other) { REALM_ASSERT(get_table() == other.get_table()); auto cols = m_table->get_column_keys(); for (auto col : cols) { + if (col.get_attrs().test(col_attr_List)) { auto src_list = other.get_listbase_ptr(col); auto dst_list = get_listbase_ptr(col); @@ -2078,6 +2040,48 @@ void Obj::copy_other_object(const Obj& other) } } +void Obj::clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink) +{ + // Algorithm: duplicate the object once for each backlick. + auto obj = m_table->create_object(); + obj.dup(*this); + auto linking_obj = opposite_table->get_object(backlink); + // fix_links(linking_obj, opposite_col_key, obj) + + // fix linking obj links in order to point to the new object just created + if (opposite_col_key.get_type() == col_type_LinkList) { + auto linking_obj_list = linking_obj.get_linklist(opposite_col_key); + auto n = linking_obj_list.find_first(get_key()); + REALM_ASSERT(n != realm::npos); + linking_obj_list.set(n, obj.get_key()); + } + else if (opposite_col_key.get_attrs().test(col_attr_Set)) { + // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store + // multiple times. update_schema should throw longer before to perform this copy. + REALM_UNREACHABLE(); + } + else if (opposite_col_key.get_attrs().test(col_attr_Dictionary)) { + auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(opposite_col_key); + auto pos = linking_obj_dictionary->find_any(ObjLink{m_table->get_key(), get_key()}); + REALM_ASSERT(pos != realm::npos); + Mixed key = linking_obj_dictionary->get_key(pos); + linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), obj.get_key()}); + } + else if (opposite_col_key.get_type() == col_type_Mixed && + linking_obj.get_any(opposite_col_key).get_type() == type_TypedLink) { + REALM_ASSERT(!linking_obj.get(opposite_col_key) || + linking_obj.get(opposite_col_key) == get_key()); + linking_obj.set_any(opposite_col_key, ObjLink{m_table->get_key(), obj.get_key()}); + } + else if (opposite_col_key.get_type() == col_type_Link) { + // Single link + REALM_ASSERT(!linking_obj.get(opposite_col_key) || + linking_obj.get(opposite_col_key) == get_key()); + linking_obj.set(opposite_col_key, obj.get_key()); + } +} + + Dictionary Obj::get_dictionary(ColKey col_key) const { REALM_ASSERT(col_key.is_dictionary()); diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index 8de9cfa0c9c..c1022cef9c8 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -261,7 +261,7 @@ class Obj { Obj& set_all(Head v, Tail... tail); void assign(const Obj& other); - void dup_and_handle_multiple_backlinks(); + bool handle_multiple_backlinks_during_schema_migration(std::vector& embedded_objects_to_fix); Obj get_linked_object(ColKey link_col_key) const { @@ -422,7 +422,8 @@ class Obj { template inline void set_spec(T&, ColKey); - void copy_other_object(const Obj& other); + void dup(const Obj& other); + void clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink); }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index 40b5e0f81d8..118ecb2616a 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -864,8 +864,8 @@ static void apply_post_migration_changes(Group& group, std::vector object.remove(); for (auto& object : objects_to_fix_backlinks) { - object.dup_and_handle_multiple_backlinks(); - object.remove(); + if (object.handle_multiple_backlinks_during_schema_migration(objects_to_fix_backlinks)) + object.remove(); } } } diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index f8702b5c3ce..465a9ac340c 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1050,50 +1050,6 @@ TEST_CASE("migration: Automatic") { REQUIRE(value == 42); } } - SECTION("change table to embedded - multiple incoming links - resolved automatically") { - InMemoryTestFile config; - config.automatic_handle_backlicks_in_migrations = true; - Schema schema = { - {"child_table", - { - {"value", PropertyType::Int}, - }}, - {"parent_table", - { - {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, - }}, - }; - auto realm = Realm::get_shared_realm(config); - realm->update_schema(schema, 1); - realm->begin_transaction(); - auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); - Obj child_object = child_table->create_object(); - child_object.set("value", 42); - auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); - auto child_object_key = child_object.get_key(); - parent_table->create_object().set_all(child_object_key); - parent_table->create_object().set_all(child_object_key); - realm->commit_transaction(); - REQUIRE(parent_table->size() == 2); - REQUIRE(child_table->size() == 1); - REQUIRE_FALSE(child_table->is_embedded()); - - REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, - [](auto, auto, auto&) {})); - - REQUIRE(realm->schema_version() == 2); - REQUIRE(parent_table->size() == 2); - REQUIRE(child_table->size() == 2); - REQUIRE(child_table->is_embedded()); - for (int i = 0; i < 2; i++) { - Object parent_object(realm, "parent_table", i); - CppContext context(realm); - Object child_object = - any_cast(parent_object.get_property_value(context, "child_property")); - Int value = any_cast(child_object.get_property_value(context, "value")); - REQUIRE(value == 42); - } - } SECTION( "change table from top-level to embedded, delete objects with 0 incoming links, resolved automatically") { InMemoryTestFile config; @@ -1430,6 +1386,158 @@ TEST_CASE("migration: Automatic") { REQUIRE(linklist.get(i - 1) != linklist.get(i)); } } + SECTION("change table to embedded - convert the whole list of linking embedded objects") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + {"child_table", + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + {"origin_table", + { + {"parent_property", PropertyType::Object | PropertyType::Nullable, "parent_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + child_object.set("value", 42); + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto child_object_key = child_object.get_key(); + auto p1 = parent_table->create_object(); + auto p2 = parent_table->create_object(); + p1.set_all(child_object_key); + p2.set_all(child_object_key); + auto origin_table = ObjectStore::table_for_object_type(realm->read_group(), "origin_table"); + origin_table->create_object().set_all(p1.get_key()); + origin_table->create_object().set_all(p2.get_key()); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 1); + REQUIRE(origin_table->size() == 2); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + REQUIRE_FALSE(origin_table->is_embedded()); + + for (auto& obj : *child_table) { + REQUIRE(obj.get_backlink_count() == 2); + } + for (auto& obj : *parent_table) { + REQUIRE(obj.get_backlink_count() == 1); + } + for (auto& obj : *origin_table) { + REQUIRE(obj.get_backlink_count() == 0); + } + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "parent_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + REQUIRE(realm->schema_version() == 2); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE(parent_table->is_embedded()); + REQUIRE_FALSE(origin_table->is_embedded()); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 1); + REQUIRE(origin_table->size() == 2); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 3, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 3); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 2); + REQUIRE(origin_table->size() == 2); + + for (auto& obj : *child_table) { + REQUIRE(obj.get_backlink_count() == 1); + } + for (auto& obj : *parent_table) { + REQUIRE(obj.get_backlink_count() == 1); + } + for (auto& obj : *origin_table) { + REQUIRE(obj.get_backlink_count() == 0); + } + + std::vector obj_children; + for (auto& child_obj : *child_table) { + obj_children.push_back(child_obj.get_key()); + } + for (int i = 0; i < 2; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + Object child_object = + any_cast(parent_object.get_property_value(context, "child_property")); + REQUIRE(child_object.obj().get_key() == obj_children[i]); + } + } + SECTION("change table to embedded - violate embedded object constraints") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + + Schema schema = { + {"child_embedded_table", + ObjectType::Embedded, + { + {"value", PropertyType::Int}, + }}, + {"parent_table", + { + {"child_property", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, + "child_embedded_table"}, + }}, + {"origin_table", + { + {"parent_property", PropertyType::Object | PropertyType::Nullable, "parent_table"}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_embedded_table"); + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + Obj parent_object = parent_table->create_object(); + ColKey col_link = parent_table->get_column_key("child_property"); + object_store::Dictionary dict_link(realm, parent_object, col_link); + dict_link.insert_embedded("Ref"); + + auto origin_table = ObjectStore::table_for_object_type(realm->read_group(), "origin_table"); + origin_table->create_object().set_all(parent_object.get_key()); + origin_table->create_object().set_all(parent_object.get_key()); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 1); + REQUIRE(child_table->size() == 1); + REQUIRE(origin_table->size() == 2); + REQUIRE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + REQUIRE_FALSE(origin_table->is_embedded()); + + for (auto& obj : *child_table) { + REQUIRE(obj.get_backlink_count() == 1); + } + for (auto& obj : *parent_table) { + REQUIRE(obj.get_backlink_count() == 2); + } + for (auto& obj : *origin_table) { + REQUIRE(obj.get_backlink_count() == 0); + } + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "parent_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + REQUIRE(realm->schema_version() == 2); + REQUIRE(child_table->is_embedded()); + REQUIRE(parent_table->is_embedded()); + REQUIRE_FALSE(origin_table->is_embedded()); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 2); + REQUIRE(origin_table->size() == 2); + } } SECTION("schema correctness during migration") { From 1dae2928174dbb5e0aa44c11920b9e53fe930ca7 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 19 Aug 2022 20:10:11 +0100 Subject: [PATCH 12/25] save work.. code need to be polished and fixed --- src/realm/obj.cpp | 208 ++++++++++++++++++++++-- src/realm/obj.hpp | 8 +- src/realm/object-store/object_store.cpp | 50 +++++- 3 files changed, 247 insertions(+), 19 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 69bc865bfb0..f021bf4effa 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1961,20 +1961,76 @@ void Obj::assign(const Obj& other) m_table->for_each_backlink_column(copy_links); } -bool Obj::handle_multiple_backlinks_during_schema_migration(std::vector& embedded_objects_to_fix) -{ - REALM_ASSERT(!m_table->get_primary_key_column()); - auto copy_links = [this](ColKey col) { - auto opposite_table = m_table->get_opposite_table(col); - auto opposite_column = m_table->get_opposite_column(col); - auto backlinks = get_all_backlinks(col); - for (auto backlink : backlinks) { - clone_object_during_migration(opposite_table, opposite_column, backlink); +bool Obj::verify_ongoing_links_to_embedded_objects(std::vector>& embedded_objects_to_fix) +{ + std::vector> embedded_forward_links; + auto compute_embedded_forward_link = [&embedded_forward_links, this](Mixed value) { + if (value.is_type(type_Link, type_TypedLink)) { + auto new_link = value.get(); + if (auto tk = new_link.get_table_key()) { + auto target_table = get_table()->get_parent_group()->get_table(tk); + if (target_table->is_embedded()) { + auto target_key = new_link.get_obj_key(); + auto obj = target_table->get_object(target_key); + embedded_forward_links.push_back({obj, target_table}); + } + } } - return false; }; - m_table->for_each_backlink_column(copy_links); - return true; // everything got cloned, so the original object can now be deleted + + auto cols = m_table->get_column_keys(); + for (auto col : cols) { + if (col.get_attrs().test(col_attr_List)) { + const auto& list = get_listbase_ptr(col); + auto sz = list->size(); + for (size_t i = 0; i < sz; i++) { + Mixed val = list->get_any(i); + compute_embedded_forward_link(val); + } + } + else if (col.get_attrs().test(col_attr_Set)) { + const auto& set = get_setbase_ptr(col); + auto sz = set->size(); + for (size_t i = 0; i < sz; ++i) { + Mixed val = set->get_any(i); + compute_embedded_forward_link(val); + } + } + else if (col.get_attrs().test(col_attr_Dictionary)) { + const auto& dictionary = get_dictionary_ptr(col); + auto sz = dictionary->size(); + for (size_t i = 0; i < sz; ++i) { + const auto& [key, val] = dictionary->get_pair(i); + compute_embedded_forward_link(val); + } + } + else { + compute_embedded_forward_link(get_any(col)); + } + } + embedded_objects_to_fix.insert(embedded_objects_to_fix.end(), embedded_forward_links.begin(), + embedded_forward_links.end()); + return embedded_forward_links.empty(); +} + +bool Obj::handle_multiple_backlinks_during_schema_migration( + std::vector>& embedded_objects_to_fix) +{ + REALM_ASSERT(!m_table->get_primary_key_column()); + if (verify_ongoing_links_to_embedded_objects(embedded_objects_to_fix)) { + auto copy_links = [this](ColKey col) { + auto opposite_table = m_table->get_opposite_table(col); + auto opposite_column = m_table->get_opposite_column(col); + auto backlinks = get_all_backlinks(col); + for (auto backlink : backlinks) { + clone_object_during_migration(opposite_table, opposite_column, backlink); + } + return false; + }; + m_table->for_each_backlink_column(copy_links); + return true; + } + return false; } void Obj::dup(const Obj& other) @@ -1982,7 +2038,6 @@ void Obj::dup(const Obj& other) REALM_ASSERT(get_table() == other.get_table()); auto cols = m_table->get_column_keys(); for (auto col : cols) { - if (col.get_attrs().test(col_attr_List)) { auto src_list = other.get_listbase_ptr(col); auto dst_list = get_listbase_ptr(col); @@ -2040,14 +2095,133 @@ void Obj::dup(const Obj& other) } } -void Obj::clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink) +Obj Obj::clone() +{ + auto obj = m_table->is_embedded() ? m_table->create_linked_object() : m_table->create_object(); + obj.dup(*this); + return obj; +} + +Obj Obj::clone_with_link() { - // Algorithm: duplicate the object once for each backlick. auto obj = m_table->create_object(); + auto& other = *this; + + // COPY FORWARD LINKS + REALM_ASSERT(get_table() == other.get_table()); + auto cols = m_table->get_column_keys(); + for (auto col : cols) { + if (col.get_attrs().test(col_attr_List)) { + auto src_list = other.get_listbase_ptr(col); + auto dst_list = obj.get_listbase_ptr(col); + auto sz = src_list->size(); + dst_list->clear(); + for (size_t i = 0; i < sz; i++) { + Mixed val = src_list->get_any(i); + // dst_list->insert_any(i, link); //TODO it seems an object link is needed + } + } + else if (col.get_attrs().test(col_attr_Set)) { + auto src_set = other.get_setbase_ptr(col); + auto dst_set = obj.get_setbase_ptr(col); + auto sz = src_set->size(); + dst_set->clear(); + for (size_t i = 0; i < sz; ++i) { + Mixed val = src_set->get_any(i); + // dst_set->insert_any(link); //TODO it seems an object link is needed + } + } + else if (col.get_attrs().test(col_attr_Dictionary)) { + auto src_dictionary = other.get_dictionary_ptr(col); + auto dst_dictionary = obj.get_dictionary_ptr(col); + auto sz = src_dictionary->size(); + dst_dictionary->clear(); + for (size_t i = 0; i < sz; ++i) { + const auto& [key, value] = src_dictionary->get_pair(i); + dst_dictionary->create_and_insert_linked_object(key); + } + } + else { + Mixed val = other.get_any(col); + if (val.is_null()) { + obj.set_null(col); + continue; + } + switch (val.get_type()) { + case type_String: { + // Need to take copy. Values might be in same cluster + std::string str{val.get_string()}; + obj.set(col, str); + break; + } + case type_Binary: { + // Need to take copy. Values might be in same cluster + std::string str{val.get_binary()}; + obj.set(col, BinaryData(str)); + break; + } + case DataType::Type::TypedLink: + case DataType::Type::Link: + // obj.set_any(col, link); //TODO it seems an object link is needed + default: + obj.set_any(col, val); + break; + } + } + } + + auto t = obj.get_table(); + auto copy_links = [this, &obj](ColKey col) { + auto opposite_table = m_table->get_opposite_table(col); + auto opposite_col_key = m_table->get_opposite_column(col); + auto backlinks = get_all_backlinks(col); + for (auto backlink : backlinks) { + + auto linking_obj = opposite_table->get_object(backlink); + // fix linking obj links in order to point to the new object just created + if (opposite_col_key.get_type() == col_type_LinkList) { + auto linking_obj_list = linking_obj.get_linklist(opposite_col_key); + auto n = linking_obj_list.find_first(get_key()); + REALM_ASSERT(n != realm::npos); + linking_obj_list.set(n, obj.get_key()); + } + else if (opposite_col_key.get_attrs().test(col_attr_Set)) { + // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store + // multiple times. update_schema should throw longer before to perform this copy. + REALM_UNREACHABLE(); + } + else if (opposite_col_key.get_attrs().test(col_attr_Dictionary)) { + auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(opposite_col_key); + auto pos = linking_obj_dictionary->find_any(ObjLink{m_table->get_key(), get_key()}); + REALM_ASSERT(pos != realm::npos); + Mixed key = linking_obj_dictionary->get_key(pos); + linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), obj.get_key()}); + } + else if (opposite_col_key.get_type() == col_type_Mixed && + linking_obj.get_any(opposite_col_key).get_type() == type_TypedLink) { + REALM_ASSERT(!linking_obj.get(opposite_col_key) || + linking_obj.get(opposite_col_key) == get_key()); + linking_obj.set_any(opposite_col_key, ObjLink{m_table->get_key(), obj.get_key()}); + } + else if (opposite_col_key.get_type() == col_type_Link) { + // Single link + REALM_ASSERT(!linking_obj.get(opposite_col_key) || + linking_obj.get(opposite_col_key) == get_key()); + linking_obj.set(opposite_col_key, obj.get_key()); + } + } + return false; + }; + t->for_each_backlink_column(copy_links); + return obj; +} + + +void Obj::clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink) +{ + auto obj = m_table->is_embedded() ? m_table->create_linked_object() : m_table->create_object(); obj.dup(*this); auto linking_obj = opposite_table->get_object(backlink); - // fix_links(linking_obj, opposite_col_key, obj) - // fix linking obj links in order to point to the new object just created if (opposite_col_key.get_type() == col_type_LinkList) { auto linking_obj_list = linking_obj.get_linklist(opposite_col_key); diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index c1022cef9c8..bf627433e69 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -261,7 +261,8 @@ class Obj { Obj& set_all(Head v, Tail... tail); void assign(const Obj& other); - bool handle_multiple_backlinks_during_schema_migration(std::vector& embedded_objects_to_fix); + bool + handle_multiple_backlinks_during_schema_migration(std::vector>& embedded_objects_to_fix); Obj get_linked_object(ColKey link_col_key) const { @@ -424,6 +425,11 @@ class Obj { void dup(const Obj& other); void clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink); + +public: + Obj clone(); + Obj clone_with_link(); + bool verify_ongoing_links_to_embedded_objects(std::vector>& embedded_objects_to_fix); }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index 118ecb2616a..13b495874d1 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -863,9 +863,57 @@ static void apply_post_migration_changes(Group& group, std::vector for (auto& object : objects_to_erase) object.remove(); + // to be moved + // + // + + struct RelObj { + Obj source, target; + TableRef table_source, table_target; + size_t N; + }; + using OutgoingLinksToEmbeddedTargets = std::vector; + using EmbeddedObjects = std::vector>; + OutgoingLinksToEmbeddedTargets embedded_links_to_fix; + EmbeddedObjects outgoing_embedded_links; + + auto populate_outgoing_embedded_links = [&embedded_links_to_fix](Obj object, TableRef table, + const EmbeddedObjects& links) { + for (auto& [target_object, target_table] : links) { + embedded_links_to_fix.push_back( + {object, target_object, table, target_table, object.get_backlink_count()}); + } + }; + for (auto& object : objects_to_fix_backlinks) { - if (object.handle_multiple_backlinks_during_schema_migration(objects_to_fix_backlinks)) + // handle schema migration from TopLevel to Embedded for objects that have multiple backlinks. + // delete the current object if the operation is succesful + if (object.handle_multiple_backlinks_during_schema_migration(outgoing_embedded_links)) { object.remove(); + } + // if there are outgoing embedded links these will have to be processed later. + populate_outgoing_embedded_links(object, table, outgoing_embedded_links); + } + + for (size_t i = 0; i < embedded_links_to_fix.size(); i++) { + auto& [source, target, source_table, target_table, n] = embedded_links_to_fix[i]; + EmbeddedObjects other_embedded_targets; + target.verify_ongoing_links_to_embedded_objects(other_embedded_targets); + if (other_embedded_targets.empty()) { + for (size_t j = 0; j < n; ++j) { + auto s = source.clone_with_link(); + + // EmbeddedObjects tmp; + // s.handle_multiple_backlinks_during_schema_migration(tmp); + } + // target.remove(); + source.remove(); + REALM_ASSERT(source_table->size() == 2); + REALM_ASSERT(target_table->size() == 2); + } + else { + populate_outgoing_embedded_links(target, target_table, other_embedded_targets); + } } } } From 8af495d31a6825c6ed55cac5e025d577ca0774a2 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Mon, 22 Aug 2022 14:03:51 +0100 Subject: [PATCH 13/25] fix logic for handling outgoing links to embedded tables --- src/realm/obj.cpp | 299 +++++++----------------- src/realm/obj.hpp | 13 +- src/realm/object-store/object_store.cpp | 52 +---- test/object-store/migrations.cpp | 16 +- 4 files changed, 107 insertions(+), 273 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index f021bf4effa..4fc3e838ba7 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1937,104 +1937,84 @@ bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) void Obj::assign(const Obj& other) { - dup(other); + copy(other); auto copy_links = [this, &other](ColKey col) { auto t = m_table->get_opposite_table(col); auto c = m_table->get_opposite_column(col); auto backlinks = other.get_all_backlinks(col); for (auto bl : backlinks) { auto linking_obj = t->get_object(bl); - if (c.get_type() == col_type_Link) { + if (c.get_type() == col_type_LinkList) { + auto linking_obj_list = linking_obj.get_linklist(c); + auto n = linking_obj_list.find_first(other.get_key()); + REALM_ASSERT(n != realm::npos); + linking_obj_list.set(n, get_key()); + } + else if (c.get_attrs().test(col_attr_Set)) { + REALM_UNREACHABLE(); + } + else if (c.get_attrs().test(col_attr_Dictionary)) { + auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(c); + auto pos = linking_obj_dictionary->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); + REALM_ASSERT(pos != realm::npos); + Mixed key = linking_obj_dictionary->get_key(pos); + linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), get_key()}); + } + else if (c.get_type() == col_type_Mixed && linking_obj.get_any(c).get_type() == type_TypedLink) { + REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); + linking_obj.set_any(c, ObjLink{m_table->get_key(), get_key()}); + } + else if (c.get_type() == col_type_Link) { // Single link REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); linking_obj.set(c, get_key()); } - else { - auto l = linking_obj.get_linklist(c); - auto n = l.find_first(other.get_key()); - REALM_ASSERT(n != realm::npos); - l.set(n, get_key()); - } } return false; }; m_table->for_each_backlink_column(copy_links); } -bool Obj::verify_ongoing_links_to_embedded_objects(std::vector>& embedded_objects_to_fix) +void Obj::handle_multiple_backlinks_during_schema_migration() { - std::vector> embedded_forward_links; - auto compute_embedded_forward_link = [&embedded_forward_links, this](Mixed value) { - if (value.is_type(type_Link, type_TypedLink)) { - auto new_link = value.get(); - if (auto tk = new_link.get_table_key()) { - auto target_table = get_table()->get_parent_group()->get_table(tk); - if (target_table->is_embedded()) { - auto target_key = new_link.get_obj_key(); - auto obj = target_table->get_object(target_key); - embedded_forward_links.push_back({obj, target_table}); - } - } + REALM_ASSERT(!m_table->get_primary_key_column()); + auto copy_links = [this](ColKey col) { + auto opposite_table = m_table->get_opposite_table(col); + auto opposite_column = m_table->get_opposite_column(col); + auto backlinks = get_all_backlinks(col); + for (auto backlink : backlinks) { + auto obj = m_table->create_object(); + obj.copy(*this); + auto linking_obj = opposite_table->get_object(backlink); + fix_linking_object_during_schema_migration(linking_obj, obj, opposite_column); } + return false; }; - - auto cols = m_table->get_column_keys(); - for (auto col : cols) { - if (col.get_attrs().test(col_attr_List)) { - const auto& list = get_listbase_ptr(col); - auto sz = list->size(); - for (size_t i = 0; i < sz; i++) { - Mixed val = list->get_any(i); - compute_embedded_forward_link(val); - } - } - else if (col.get_attrs().test(col_attr_Set)) { - const auto& set = get_setbase_ptr(col); - auto sz = set->size(); - for (size_t i = 0; i < sz; ++i) { - Mixed val = set->get_any(i); - compute_embedded_forward_link(val); - } - } - else if (col.get_attrs().test(col_attr_Dictionary)) { - const auto& dictionary = get_dictionary_ptr(col); - auto sz = dictionary->size(); - for (size_t i = 0; i < sz; ++i) { - const auto& [key, val] = dictionary->get_pair(i); - compute_embedded_forward_link(val); - } - } - else { - compute_embedded_forward_link(get_any(col)); - } - } - embedded_objects_to_fix.insert(embedded_objects_to_fix.end(), embedded_forward_links.begin(), - embedded_forward_links.end()); - return embedded_forward_links.empty(); + m_table->for_each_backlink_column(copy_links); } -bool Obj::handle_multiple_backlinks_during_schema_migration( - std::vector>& embedded_objects_to_fix) +void Obj::copy(const Obj& other) { - REALM_ASSERT(!m_table->get_primary_key_column()); - if (verify_ongoing_links_to_embedded_objects(embedded_objects_to_fix)) { - auto copy_links = [this](ColKey col) { - auto opposite_table = m_table->get_opposite_table(col); - auto opposite_column = m_table->get_opposite_column(col); - auto backlinks = get_all_backlinks(col); - for (auto backlink : backlinks) { - clone_object_during_migration(opposite_table, opposite_column, backlink); - } - return false; - }; - m_table->for_each_backlink_column(copy_links); - return true; - } - return false; -} + // To be used carefully since it could potentially duplicate user data. + // since Obj::copy is for now basically only used for handling schema migration when there multiple + // incoming links to some embedded table, this method is basically harmless, but if Obj::assign get's exposed, + // we will have to have 2 versions of it, one designed for migrations (in which links to embedded objects are dup) + // and one for handling normal copies (where an expecption is thrown if an outgoing link to an embedded table is + // found). + + // handle correctly the case in which the value to copy is an outgoing link to an embedded table. + auto handle_object_to_copy = [this](Mixed value) -> Mixed { + TableRef target_table; + ObjKey target_obj_key; + if (is_outgoing_embedded_link(value, target_table, target_obj_key)) { + auto copy_target_object = target_table->create_linked_object(); + auto target_obj = target_table->get_object(target_obj_key); + copy_target_object.copy(target_obj); + return ObjLink{target_table->get_key(), copy_target_object.get_key()}; + } + return value; + }; -void Obj::dup(const Obj& other) -{ REALM_ASSERT(get_table() == other.get_table()); auto cols = m_table->get_column_keys(); for (auto col : cols) { @@ -2044,8 +2024,8 @@ void Obj::dup(const Obj& other) auto sz = src_list->size(); dst_list->clear(); for (size_t i = 0; i < sz; i++) { - Mixed val = src_list->get_any(i); - dst_list->insert_any(i, val); + Mixed value = src_list->get_any(i); + dst_list->insert_any(i, handle_object_to_copy(value)); } } else if (col.get_attrs().test(col_attr_Set)) { @@ -2054,8 +2034,8 @@ void Obj::dup(const Obj& other) auto sz = src_set->size(); dst_set->clear(); for (size_t i = 0; i < sz; ++i) { - Mixed val = src_set->get_any(i); - dst_set->insert_any(val); + Mixed value = src_set->get_any(i); + dst_set->insert_any(handle_object_to_copy(value)); } } else if (col.get_attrs().test(col_attr_Dictionary)) { @@ -2065,163 +2045,46 @@ void Obj::dup(const Obj& other) dst_dictionary->clear(); for (size_t i = 0; i < sz; ++i) { const auto& [key, value] = src_dictionary->get_pair(i); - dst_dictionary->insert(key, value); + TableRef target_table; + ObjKey target_obj_key; + if (is_outgoing_embedded_link(value, target_table, target_obj_key)) { + auto obj = dst_dictionary->create_and_insert_linked_object(key); + obj.copy(target_table->get_object(target_obj_key)); + } + else { + dst_dictionary->insert(key, value); + } } } else { - Mixed val = other.get_any(col); - if (val.is_null()) { + Mixed value = other.get_any(col); + if (value.is_null()) { this->set_null(col); continue; } - switch (val.get_type()) { + switch (value.get_type()) { case type_String: { // Need to take copy. Values might be in same cluster - std::string str{val.get_string()}; + std::string str{value.get_string()}; this->set(col, str); break; } case type_Binary: { // Need to take copy. Values might be in same cluster - std::string str{val.get_binary()}; + std::string str{value.get_binary()}; this->set(col, BinaryData(str)); break; } default: - this->set_any(col, val); + this->set_any(col, handle_object_to_copy(value)); break; } } } } -Obj Obj::clone() -{ - auto obj = m_table->is_embedded() ? m_table->create_linked_object() : m_table->create_object(); - obj.dup(*this); - return obj; -} - -Obj Obj::clone_with_link() +void Obj::fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const { - auto obj = m_table->create_object(); - auto& other = *this; - - // COPY FORWARD LINKS - REALM_ASSERT(get_table() == other.get_table()); - auto cols = m_table->get_column_keys(); - for (auto col : cols) { - if (col.get_attrs().test(col_attr_List)) { - auto src_list = other.get_listbase_ptr(col); - auto dst_list = obj.get_listbase_ptr(col); - auto sz = src_list->size(); - dst_list->clear(); - for (size_t i = 0; i < sz; i++) { - Mixed val = src_list->get_any(i); - // dst_list->insert_any(i, link); //TODO it seems an object link is needed - } - } - else if (col.get_attrs().test(col_attr_Set)) { - auto src_set = other.get_setbase_ptr(col); - auto dst_set = obj.get_setbase_ptr(col); - auto sz = src_set->size(); - dst_set->clear(); - for (size_t i = 0; i < sz; ++i) { - Mixed val = src_set->get_any(i); - // dst_set->insert_any(link); //TODO it seems an object link is needed - } - } - else if (col.get_attrs().test(col_attr_Dictionary)) { - auto src_dictionary = other.get_dictionary_ptr(col); - auto dst_dictionary = obj.get_dictionary_ptr(col); - auto sz = src_dictionary->size(); - dst_dictionary->clear(); - for (size_t i = 0; i < sz; ++i) { - const auto& [key, value] = src_dictionary->get_pair(i); - dst_dictionary->create_and_insert_linked_object(key); - } - } - else { - Mixed val = other.get_any(col); - if (val.is_null()) { - obj.set_null(col); - continue; - } - switch (val.get_type()) { - case type_String: { - // Need to take copy. Values might be in same cluster - std::string str{val.get_string()}; - obj.set(col, str); - break; - } - case type_Binary: { - // Need to take copy. Values might be in same cluster - std::string str{val.get_binary()}; - obj.set(col, BinaryData(str)); - break; - } - case DataType::Type::TypedLink: - case DataType::Type::Link: - // obj.set_any(col, link); //TODO it seems an object link is needed - default: - obj.set_any(col, val); - break; - } - } - } - - auto t = obj.get_table(); - auto copy_links = [this, &obj](ColKey col) { - auto opposite_table = m_table->get_opposite_table(col); - auto opposite_col_key = m_table->get_opposite_column(col); - auto backlinks = get_all_backlinks(col); - for (auto backlink : backlinks) { - - auto linking_obj = opposite_table->get_object(backlink); - // fix linking obj links in order to point to the new object just created - if (opposite_col_key.get_type() == col_type_LinkList) { - auto linking_obj_list = linking_obj.get_linklist(opposite_col_key); - auto n = linking_obj_list.find_first(get_key()); - REALM_ASSERT(n != realm::npos); - linking_obj_list.set(n, obj.get_key()); - } - else if (opposite_col_key.get_attrs().test(col_attr_Set)) { - // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store - // multiple times. update_schema should throw longer before to perform this copy. - REALM_UNREACHABLE(); - } - else if (opposite_col_key.get_attrs().test(col_attr_Dictionary)) { - auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(opposite_col_key); - auto pos = linking_obj_dictionary->find_any(ObjLink{m_table->get_key(), get_key()}); - REALM_ASSERT(pos != realm::npos); - Mixed key = linking_obj_dictionary->get_key(pos); - linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), obj.get_key()}); - } - else if (opposite_col_key.get_type() == col_type_Mixed && - linking_obj.get_any(opposite_col_key).get_type() == type_TypedLink) { - REALM_ASSERT(!linking_obj.get(opposite_col_key) || - linking_obj.get(opposite_col_key) == get_key()); - linking_obj.set_any(opposite_col_key, ObjLink{m_table->get_key(), obj.get_key()}); - } - else if (opposite_col_key.get_type() == col_type_Link) { - // Single link - REALM_ASSERT(!linking_obj.get(opposite_col_key) || - linking_obj.get(opposite_col_key) == get_key()); - linking_obj.set(opposite_col_key, obj.get_key()); - } - } - return false; - }; - t->for_each_backlink_column(copy_links); - return obj; -} - - -void Obj::clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink) -{ - auto obj = m_table->is_embedded() ? m_table->create_linked_object() : m_table->create_object(); - obj.dup(*this); - auto linking_obj = opposite_table->get_object(backlink); // fix linking obj links in order to point to the new object just created if (opposite_col_key.get_type() == col_type_LinkList) { auto linking_obj_list = linking_obj.get_linklist(opposite_col_key); @@ -2255,6 +2118,18 @@ void Obj::clone_object_during_migration(TableRef opposite_table, ColKey opposite } } +bool Obj::is_outgoing_embedded_link(Mixed value, TableRef& target_table, ObjKey& target_obj_key) const +{ + if (value.is_type(type_Link, type_TypedLink)) { + auto link = value.get(); + if (auto tk = link.get_table_key()) { + target_table = get_table()->get_parent_group()->get_table(tk); + target_obj_key = link.get_obj_key(); + return target_table->is_embedded(); + } + } + return false; +} Dictionary Obj::get_dictionary(ColKey col_key) const { diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index bf627433e69..b4e9a1a780d 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -261,8 +261,7 @@ class Obj { Obj& set_all(Head v, Tail... tail); void assign(const Obj& other); - bool - handle_multiple_backlinks_during_schema_migration(std::vector>& embedded_objects_to_fix); + void handle_multiple_backlinks_during_schema_migration(); Obj get_linked_object(ColKey link_col_key) const { @@ -423,13 +422,9 @@ class Obj { template inline void set_spec(T&, ColKey); - void dup(const Obj& other); - void clone_object_during_migration(TableRef opposite_table, ColKey opposite_col_key, ObjKey backlink); - -public: - Obj clone(); - Obj clone_with_link(); - bool verify_ongoing_links_to_embedded_objects(std::vector>& embedded_objects_to_fix); + void copy(const Obj& other); + void fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const; + bool is_outgoing_embedded_link(Mixed value, TableRef& target_table, ObjKey& target_obj_key) const; }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index 13b495874d1..a99e31a85c2 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -863,57 +863,9 @@ static void apply_post_migration_changes(Group& group, std::vector for (auto& object : objects_to_erase) object.remove(); - // to be moved - // - // - - struct RelObj { - Obj source, target; - TableRef table_source, table_target; - size_t N; - }; - using OutgoingLinksToEmbeddedTargets = std::vector; - using EmbeddedObjects = std::vector>; - OutgoingLinksToEmbeddedTargets embedded_links_to_fix; - EmbeddedObjects outgoing_embedded_links; - - auto populate_outgoing_embedded_links = [&embedded_links_to_fix](Obj object, TableRef table, - const EmbeddedObjects& links) { - for (auto& [target_object, target_table] : links) { - embedded_links_to_fix.push_back( - {object, target_object, table, target_table, object.get_backlink_count()}); - } - }; - for (auto& object : objects_to_fix_backlinks) { - // handle schema migration from TopLevel to Embedded for objects that have multiple backlinks. - // delete the current object if the operation is succesful - if (object.handle_multiple_backlinks_during_schema_migration(outgoing_embedded_links)) { - object.remove(); - } - // if there are outgoing embedded links these will have to be processed later. - populate_outgoing_embedded_links(object, table, outgoing_embedded_links); - } - - for (size_t i = 0; i < embedded_links_to_fix.size(); i++) { - auto& [source, target, source_table, target_table, n] = embedded_links_to_fix[i]; - EmbeddedObjects other_embedded_targets; - target.verify_ongoing_links_to_embedded_objects(other_embedded_targets); - if (other_embedded_targets.empty()) { - for (size_t j = 0; j < n; ++j) { - auto s = source.clone_with_link(); - - // EmbeddedObjects tmp; - // s.handle_multiple_backlinks_during_schema_migration(tmp); - } - // target.remove(); - source.remove(); - REALM_ASSERT(source_table->size() == 2); - REALM_ASSERT(target_table->size() == 2); - } - else { - populate_outgoing_embedded_links(target, target_table, other_embedded_targets); - } + object.handle_multiple_backlinks_during_schema_migration(); + object.remove(); } } } diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 465a9ac340c..5bd335bd0c7 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1488,7 +1488,7 @@ TEST_CASE("migration: Automatic") { }}, {"parent_table", { - {"child_property", PropertyType::Dictionary | PropertyType::Object | PropertyType::Nullable, + {"child_property", PropertyType::Object | PropertyType::Nullable | PropertyType::Dictionary, "child_embedded_table"}, }}, {"origin_table", @@ -1505,7 +1505,8 @@ TEST_CASE("migration: Automatic") { Obj parent_object = parent_table->create_object(); ColKey col_link = parent_table->get_column_key("child_property"); object_store::Dictionary dict_link(realm, parent_object, col_link); - dict_link.insert_embedded("Ref"); + auto child_obj = dict_link.insert_embedded("Ref"); + child_obj.set("value", 42); auto origin_table = ObjectStore::table_for_object_type(realm->read_group(), "origin_table"); origin_table->create_object().set_all(parent_object.get_key()); @@ -1537,6 +1538,17 @@ TEST_CASE("migration: Automatic") { REQUIRE(parent_table->size() == 2); REQUIRE(child_table->size() == 2); REQUIRE(origin_table->size() == 2); + + for (int i = 0; i < 2; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + object_store::Dictionary dictionary_to_embedded_object = any_cast( + parent_object.get_property_value(context, "child_property")); + auto child = dictionary_to_embedded_object.get_any("Ref"); + ObjLink link = child.get_link(); + Object child_value(realm, link); + REQUIRE(child_value.get_column_value("value") == 42); + } } } From 62b52acf07bafaf82befc4838a72c8ee0d05de45 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Mon, 22 Aug 2022 14:53:39 +0100 Subject: [PATCH 14/25] fix comment --- src/realm/obj.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 4fc3e838ba7..26bd08bda02 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1996,11 +1996,11 @@ void Obj::handle_multiple_backlinks_during_schema_migration() void Obj::copy(const Obj& other) { // To be used carefully since it could potentially duplicate user data. - // since Obj::copy is for now basically only used for handling schema migration when there multiple + // since Obj::copy is for now basically only used for handling schema migration when there are multiple // incoming links to some embedded table, this method is basically harmless, but if Obj::assign get's exposed, - // we will have to have 2 versions of it, one designed for migrations (in which links to embedded objects are dup) - // and one for handling normal copies (where an expecption is thrown if an outgoing link to an embedded table is - // found). + // we will have to have 2 versions of it, one designed for migrations (in which links to embedded objects are + // duplicated) and one for handling normal copies (where an expecption is thrown if an outgoing link to an + // embedded table is found). // handle correctly the case in which the value to copy is an outgoing link to an embedded table. auto handle_object_to_copy = [this](Mixed value) -> Mixed { From 63e48580cf6cc9d698f2e1bd53228f525812c5e6 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Mon, 22 Aug 2022 17:59:42 +0100 Subject: [PATCH 15/25] remove stale code --- src/realm/obj.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 26bd08bda02..183b398cebf 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1443,7 +1443,6 @@ Obj& Obj::set(ColKey col_key, ObjKey target_key, bool is_default) if (type != ColumnTypeTraits::column_id) throw LogicError(LogicError::illegal_type); TableRef target_table = get_target_table(col_key); - auto n = target_table->get_name(); TableKey target_table_key = target_table->get_key(); if (target_key) { ClusterTree* ct = target_key.is_unresolved() ? target_table->m_tombstones.get() : &target_table->m_clusters; From a7d94abfa77a7f0b09aaf0a741c699c2f142f83e Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Tue, 23 Aug 2022 19:39:22 +0100 Subject: [PATCH 16/25] partial code review fixes --- src/realm/obj.cpp | 43 ++++++++++++------------- src/realm/obj.hpp | 12 ++++++- src/realm/object-store/object_store.cpp | 3 +- test/object-store/migrations.cpp | 10 +++--- 4 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 183b398cebf..962d7809538 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1950,7 +1950,10 @@ void Obj::assign(const Obj& other) linking_obj_list.set(n, get_key()); } else if (c.get_attrs().test(col_attr_Set)) { - REALM_UNREACHABLE(); + auto linking_obj_set = linking_obj.get_setbase_ptr(c); + auto pos = linking_obj_set->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); + REALM_ASSERT(pos != realm::npos); + linking_obj_set->insert_any(ObjLink{m_table->get_key(), get_key()}); } else if (c.get_attrs().test(col_attr_Dictionary)) { auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(c); @@ -1994,18 +1997,11 @@ void Obj::handle_multiple_backlinks_during_schema_migration() void Obj::copy(const Obj& other) { - // To be used carefully since it could potentially duplicate user data. - // since Obj::copy is for now basically only used for handling schema migration when there are multiple - // incoming links to some embedded table, this method is basically harmless, but if Obj::assign get's exposed, - // we will have to have 2 versions of it, one designed for migrations (in which links to embedded objects are - // duplicated) and one for handling normal copies (where an expecption is thrown if an outgoing link to an - // embedded table is found). - // handle correctly the case in which the value to copy is an outgoing link to an embedded table. - auto handle_object_to_copy = [this](Mixed value) -> Mixed { + auto handle_object_to_copy = [this](ColKey col, Mixed value) -> Mixed { TableRef target_table; ObjKey target_obj_key; - if (is_outgoing_embedded_link(value, target_table, target_obj_key)) { + if (is_outgoing_embedded_link(col, value, target_table, target_obj_key)) { auto copy_target_object = target_table->create_linked_object(); auto target_obj = target_table->get_object(target_obj_key); copy_target_object.copy(target_obj); @@ -2024,7 +2020,7 @@ void Obj::copy(const Obj& other) dst_list->clear(); for (size_t i = 0; i < sz; i++) { Mixed value = src_list->get_any(i); - dst_list->insert_any(i, handle_object_to_copy(value)); + dst_list->insert_any(i, handle_object_to_copy(col, value)); } } else if (col.get_attrs().test(col_attr_Set)) { @@ -2034,7 +2030,7 @@ void Obj::copy(const Obj& other) dst_set->clear(); for (size_t i = 0; i < sz; ++i) { Mixed value = src_set->get_any(i); - dst_set->insert_any(handle_object_to_copy(value)); + dst_set->insert_any(handle_object_to_copy(col, value)); } } else if (col.get_attrs().test(col_attr_Dictionary)) { @@ -2046,7 +2042,7 @@ void Obj::copy(const Obj& other) const auto& [key, value] = src_dictionary->get_pair(i); TableRef target_table; ObjKey target_obj_key; - if (is_outgoing_embedded_link(value, target_table, target_obj_key)) { + if (is_outgoing_embedded_link(col, value, target_table, target_obj_key)) { auto obj = dst_dictionary->create_and_insert_linked_object(key); obj.copy(target_table->get_object(target_obj_key)); } @@ -2075,7 +2071,7 @@ void Obj::copy(const Obj& other) break; } default: - this->set_any(col, handle_object_to_copy(value)); + this->set_any(col, handle_object_to_copy(col, value)); break; } } @@ -2117,15 +2113,18 @@ void Obj::fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, C } } -bool Obj::is_outgoing_embedded_link(Mixed value, TableRef& target_table, ObjKey& target_obj_key) const +bool Obj::is_outgoing_embedded_link(ColKey col_key, Mixed value, TableRef& target_table, ObjKey& target_obj_key) const { - if (value.is_type(type_Link, type_TypedLink)) { - auto link = value.get(); - if (auto tk = link.get_table_key()) { - target_table = get_table()->get_parent_group()->get_table(tk); - target_obj_key = link.get_obj_key(); - return target_table->is_embedded(); - } + if (value.is_type(type_Link)) { + target_table = m_table->get_opposite_table(col_key); + target_obj_key = value.get(); + return target_table->is_embedded(); + } + if (value.is_type(type_TypedLink)) { + target_table = m_table->get_opposite_table(col_key); + auto link = value.get_link(); + target_obj_key = link.get_obj_key(); + return target_table->is_embedded(); } return false; } diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index b4e9a1a780d..2013752640a 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -260,6 +260,11 @@ class Obj { template Obj& set_all(Head v, Tail... tail); + // Assign the object passed as argument to the this object. + // This method performs a deep copy of all the properties of + // the other object passed. + // So this could potentially create a lot of user data duplication + // if not used correctly. void assign(const Obj& other); void handle_multiple_backlinks_during_schema_migration(); @@ -422,9 +427,14 @@ class Obj { template inline void set_spec(T&, ColKey); + // To be used carefully since it could potentially duplicate user data. + // since Obj::copy is for now basically only used for handling schema migration when there are multiple + // incoming links to some embedded table, this method is basically harmless, but Obj::assign uses it, so + // in case of copies outside a migration, probably we should throw an excpetion if we attempt to copy. + // an object that contains a link to an embedded table. void copy(const Obj& other); void fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const; - bool is_outgoing_embedded_link(Mixed value, TableRef& target_table, ObjKey& target_obj_key) const; + bool is_outgoing_embedded_link(ColKey col_key, Mixed value, TableRef& target_table, ObjKey& target_obj_key) const; }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp index a99e31a85c2..acf9d628451 100644 --- a/src/realm/object-store/object_store.cpp +++ b/src/realm/object-store/object_store.cpp @@ -848,7 +848,8 @@ static void apply_post_migration_changes(Group& group, std::vector if (object_schema->table_type == ObjectSchema::ObjectType::Embedded) { auto original_object_schema = initial_schema.find(object_schema->name); if (original_object_schema != initial_schema.end() && - original_object_schema->table_type == ObjectSchema::ObjectType::TopLevel) { + (original_object_schema->table_type == ObjectSchema::ObjectType::TopLevel || + original_object_schema->table_type == ObjectSchema::ObjectType::TopLevelAsymmetric)) { auto table = table_for_object_schema(group, *original_object_schema); std::vector objects_to_erase; std::vector objects_to_fix_backlinks; diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 9cd1b950843..b3b43f2e627 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1209,8 +1209,8 @@ TEST_CASE("migration: Automatic") { Object parent_object(realm, "parent_table", i); CppContext context(realm); Object child_object = - any_cast(parent_object.get_property_value(context, "child_property")); - Int value = any_cast(child_object.get_property_value(context, "value")); + util::any_cast(parent_object.get_property_value(context, "child_property")); + Int value = util::any_cast(child_object.get_property_value(context, "value")); REQUIRE(value == 42); auto value_dictionary = util::any_cast( child_object.get_property_value(context, "value_dict")); @@ -1292,7 +1292,7 @@ TEST_CASE("migration: Automatic") { for (int i = 0; i < 1; i++) { Object parent_object(realm, "parent_table", i); CppContext context(realm); - object_store::Dictionary links_dictionary = any_cast( + object_store::Dictionary links_dictionary = util::any_cast( parent_object.get_property_value(context, "child_property")); REQUIRE(links_dictionary.size() == dict_links.size()); for (size_t i = 0; i < 2; ++i) { @@ -1472,7 +1472,7 @@ TEST_CASE("migration: Automatic") { Object parent_object(realm, "parent_table", i); CppContext context(realm); Object child_object = - any_cast(parent_object.get_property_value(context, "child_property")); + util::any_cast(parent_object.get_property_value(context, "child_property")); REQUIRE(child_object.obj().get_key() == obj_children[i]); } } @@ -1542,7 +1542,7 @@ TEST_CASE("migration: Automatic") { for (int i = 0; i < 2; i++) { Object parent_object(realm, "parent_table", i); CppContext context(realm); - object_store::Dictionary dictionary_to_embedded_object = any_cast( + object_store::Dictionary dictionary_to_embedded_object = util::any_cast( parent_object.get_property_value(context, "child_property")); auto child = dictionary_to_embedded_object.get_any("Ref"); ObjLink link = child.get_link(); From f049720bd6edb54484fce3b75ca643686955fd11 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 24 Aug 2022 15:53:36 +0100 Subject: [PATCH 17/25] fix array of mixed handling --- src/realm/obj.cpp | 41 ++++++++++------- src/realm/obj.hpp | 3 +- test/object-store/migrations.cpp | 76 +++++++++++++++++++++++++++++++- 3 files changed, 103 insertions(+), 17 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 962d7809538..516f4d3e5e3 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1998,10 +1998,10 @@ void Obj::handle_multiple_backlinks_during_schema_migration() void Obj::copy(const Obj& other) { // handle correctly the case in which the value to copy is an outgoing link to an embedded table. - auto handle_object_to_copy = [this](ColKey col, Mixed value) -> Mixed { + auto handle_object_to_copy = [this](ConstTableRef table, ColKey col, Mixed value) -> Mixed { TableRef target_table; ObjKey target_obj_key; - if (is_outgoing_embedded_link(col, value, target_table, target_obj_key)) { + if (is_outgoing_embedded_link(table, col, value, target_table, target_obj_key)) { auto copy_target_object = target_table->create_linked_object(); auto target_obj = target_table->get_object(target_obj_key); copy_target_object.copy(target_obj); @@ -2020,7 +2020,7 @@ void Obj::copy(const Obj& other) dst_list->clear(); for (size_t i = 0; i < sz; i++) { Mixed value = src_list->get_any(i); - dst_list->insert_any(i, handle_object_to_copy(col, value)); + dst_list->insert_any(i, handle_object_to_copy(m_table, col, value)); } } else if (col.get_attrs().test(col_attr_Set)) { @@ -2030,7 +2030,7 @@ void Obj::copy(const Obj& other) dst_set->clear(); for (size_t i = 0; i < sz; ++i) { Mixed value = src_set->get_any(i); - dst_set->insert_any(handle_object_to_copy(col, value)); + dst_set->insert_any(handle_object_to_copy(m_table, col, value)); } } else if (col.get_attrs().test(col_attr_Dictionary)) { @@ -2042,7 +2042,7 @@ void Obj::copy(const Obj& other) const auto& [key, value] = src_dictionary->get_pair(i); TableRef target_table; ObjKey target_obj_key; - if (is_outgoing_embedded_link(col, value, target_table, target_obj_key)) { + if (is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { auto obj = dst_dictionary->create_and_insert_linked_object(key); obj.copy(target_table->get_object(target_obj_key)); } @@ -2071,7 +2071,7 @@ void Obj::copy(const Obj& other) break; } default: - this->set_any(col, handle_object_to_copy(col, value)); + this->set_any(col, handle_object_to_copy(other.get_table(), col, value)); break; } } @@ -2087,9 +2087,15 @@ void Obj::fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, C REALM_ASSERT(n != realm::npos); linking_obj_list.set(n, obj.get_key()); } + else if (opposite_col_key.get_attrs().test(col_attr_List)) { + auto linking_obj_list = linking_obj.get_listbase_ptr(opposite_col_key); + auto n = linking_obj_list->find_any(ObjLink{m_table->get_key(), get_key()}); + REALM_ASSERT(n != realm::npos); + linking_obj_list->insert_any(n, ObjLink{m_table->get_key(), obj.get_key()}); + } else if (opposite_col_key.get_attrs().test(col_attr_Set)) { // this SHOULD NEVER HAPPEN, since SET sematincs forbids to have a backlink to the same object store - // multiple times. update_schema should throw longer before to perform this copy. + // multiple times. update_schema would have thrown way before to reach this point. REALM_UNREACHABLE(); } else if (opposite_col_key.get_attrs().test(col_attr_Dictionary)) { @@ -2113,18 +2119,23 @@ void Obj::fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, C } } -bool Obj::is_outgoing_embedded_link(ColKey col_key, Mixed value, TableRef& target_table, ObjKey& target_obj_key) const +bool Obj::is_outgoing_embedded_link(ConstTableRef source_table, ColKey col_key, Mixed value, TableRef& target_table, + ObjKey& target_obj_key) const { if (value.is_type(type_Link)) { - target_table = m_table->get_opposite_table(col_key); - target_obj_key = value.get(); - return target_table->is_embedded(); + target_table = source_table->get_opposite_table(col_key); + if (target_table) { + target_obj_key = value.get(); + return target_table->is_embedded(); + } } if (value.is_type(type_TypedLink)) { - target_table = m_table->get_opposite_table(col_key); - auto link = value.get_link(); - target_obj_key = link.get_obj_key(); - return target_table->is_embedded(); + target_table = source_table->get_opposite_table(col_key); + if (target_table) { + auto link = value.get_link(); + target_obj_key = link.get_obj_key(); + return target_table->is_embedded(); + } } return false; } diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index 2013752640a..09c14f237ca 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -434,7 +434,8 @@ class Obj { // an object that contains a link to an embedded table. void copy(const Obj& other); void fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const; - bool is_outgoing_embedded_link(ColKey col_key, Mixed value, TableRef& target_table, ObjKey& target_obj_key) const; + bool is_outgoing_embedded_link(ConstTableRef source_table, ColKey col_key, Mixed value, TableRef& target_table, + ObjKey& target_obj_key) const; }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index b3b43f2e627..ca43da6bd5c 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1121,7 +1121,81 @@ TEST_CASE("migration: Automatic") { REQUIRE(child_table->size() == 1); REQUIRE(parent_table->size() == 1); } - SECTION("change table to embedded - multiple incoming links - resolved automatically + copy set, dictionary " + SECTION("change table to embedded - multiple incoming links - resolved automatically + copy array of mixed " + "verification") { + InMemoryTestFile config; + config.automatic_handle_backlicks_in_migrations = true; + Schema schema = { + { + "child_table", + { + {"mixed_array", PropertyType::Mixed | PropertyType::Array | PropertyType::Nullable}, + }, + }, + {"parent_table", + { + {"child_property", PropertyType::Object | PropertyType::Nullable, "child_table"}, + }}, + {"target", + { + {"value", PropertyType::Int}, + }}, + }; + auto realm = Realm::get_shared_realm(config); + realm->update_schema(schema, 1); + realm->begin_transaction(); + auto child_table = ObjectStore::table_for_object_type(realm->read_group(), "child_table"); + Obj child_object = child_table->create_object(); + ColKey col_mixed_array = child_table->get_column_key("mixed_array"); + auto target_table = ObjectStore::table_for_object_type(realm->read_group(), "target"); + Obj target_object = target_table->create_object(); + target_object.set("value", 10); + List list(realm, child_object, col_mixed_array); + list.insert(0, Mixed{10}); + list.insert(1, Mixed{10.10}); + list.insert(2, Mixed{ObjLink{target_table->get_key(),target_object.get_key()}});\ + list.insert(3, Mixed{target_object.get_key()}); + + auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); + auto child_object_key = child_object.get_key(); + auto o1 = parent_table->create_object(); + auto o2 = parent_table->create_object(); + o1.set_all(child_object_key); + o2.set_all(child_object_key); + realm->commit_transaction(); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 1); + REQUIRE(target_table->size() == 1); + REQUIRE_FALSE(child_table->is_embedded()); + REQUIRE_FALSE(parent_table->is_embedded()); + REQUIRE_FALSE(target_table->is_embedded()); + + REQUIRE_NOTHROW(realm->update_schema(set_table_type(schema, "child_table", ObjectType::Embedded), 2, + [](auto, auto, auto&) {})); + + REQUIRE(realm->schema_version() == 2); + REQUIRE(parent_table->size() == 2); + REQUIRE(child_table->size() == 2); + REQUIRE(target_table->size() == 1); + REQUIRE(child_table->is_embedded()); + REQUIRE_FALSE(target_table->is_embedded()); + + for (int i = 0; i < 2; i++) { + Object parent_object(realm, "parent_table", i); + CppContext context(realm); + Object child_object = + util::any_cast(parent_object.get_property_value(context, "child_property")); + auto mixed_array = util::any_cast( + child_object.get_property_value(context, "mixed_array")); + REQUIRE(mixed_array.size() == 4); + REQUIRE(mixed_array.get_any(0).get() == 10); + REQUIRE(mixed_array.get_any(1).get() == 10.10); + REQUIRE(mixed_array.get_any(2).get().get_table_key() == target_object.get_table()->get_key()); + REQUIRE(mixed_array.get_any(2).get().get_obj_key() == target_object.get_key()); + REQUIRE(mixed_array.get_any(3).get() == target_object.get_key()); + } + } + SECTION("change table to embedded - multiple incoming links - resolved automatically + copy set, dictionary, any array " "verification") { InMemoryTestFile config; config.automatic_handle_backlicks_in_migrations = true; From f074b7fb7e8e9a442d35557227eff23a1eb42a7b Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 24 Aug 2022 15:53:52 +0100 Subject: [PATCH 18/25] testing for array of mixed --- test/object-store/migrations.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index ca43da6bd5c..7889b1c048b 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1153,7 +1153,7 @@ TEST_CASE("migration: Automatic") { List list(realm, child_object, col_mixed_array); list.insert(0, Mixed{10}); list.insert(1, Mixed{10.10}); - list.insert(2, Mixed{ObjLink{target_table->get_key(),target_object.get_key()}});\ + list.insert(2, Mixed{ObjLink{target_table->get_key(), target_object.get_key()}}); list.insert(3, Mixed{target_object.get_key()}); auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); @@ -1185,17 +1185,19 @@ TEST_CASE("migration: Automatic") { CppContext context(realm); Object child_object = util::any_cast(parent_object.get_property_value(context, "child_property")); - auto mixed_array = util::any_cast( - child_object.get_property_value(context, "mixed_array")); + auto mixed_array = + util::any_cast(child_object.get_property_value(context, "mixed_array")); REQUIRE(mixed_array.size() == 4); REQUIRE(mixed_array.get_any(0).get() == 10); REQUIRE(mixed_array.get_any(1).get() == 10.10); - REQUIRE(mixed_array.get_any(2).get().get_table_key() == target_object.get_table()->get_key()); + REQUIRE(mixed_array.get_any(2).get().get_table_key() == + target_object.get_table()->get_key()); REQUIRE(mixed_array.get_any(2).get().get_obj_key() == target_object.get_key()); REQUIRE(mixed_array.get_any(3).get() == target_object.get_key()); } } - SECTION("change table to embedded - multiple incoming links - resolved automatically + copy set, dictionary, any array " + SECTION("change table to embedded - multiple incoming links - resolved automatically + copy set, dictionary, " + "any array " "verification") { InMemoryTestFile config; config.automatic_handle_backlicks_in_migrations = true; From 650512001630b2d41cb84e925dc9022356ec186b Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Wed, 24 Aug 2022 19:21:39 +0100 Subject: [PATCH 19/25] iterative algorithm for deep copying all the fw links to embedded objects --- src/realm/obj.cpp | 113 +++++++++++++++++++++++++++++++++++----------- src/realm/obj.hpp | 11 ++++- 2 files changed, 97 insertions(+), 27 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 516f4d3e5e3..0ad29f4ab4e 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1936,7 +1936,9 @@ bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) void Obj::assign(const Obj& other) { - copy(other); + Obj::CopyEmbeddedLinkTracker tracker; + copy(other, tracker); + auto copy_links = [this, &other](ColKey col) { auto t = m_table->get_opposite_table(col); auto c = m_table->get_opposite_column(col); @@ -1985,8 +1987,14 @@ void Obj::handle_multiple_backlinks_during_schema_migration() auto opposite_column = m_table->get_opposite_column(col); auto backlinks = get_all_backlinks(col); for (auto backlink : backlinks) { + // create a new obj auto obj = m_table->create_object(); - obj.copy(*this); + // try to deep copy all the properties in the new object + Obj::CopyEmbeddedLinkTracker tracker; + obj.copy(*this, tracker); + // if there are embedded links, then copy them separately + handle_copy_for_embedded_links(tracker); + // fix all the backlinks auto linking_obj = opposite_table->get_object(backlink); fix_linking_object_during_schema_migration(linking_obj, obj, opposite_column); } @@ -1995,24 +2003,16 @@ void Obj::handle_multiple_backlinks_during_schema_migration() m_table->for_each_backlink_column(copy_links); } -void Obj::copy(const Obj& other) +void Obj::copy(const Obj& other, Obj::CopyEmbeddedLinkTracker& embedded_links) { // handle correctly the case in which the value to copy is an outgoing link to an embedded table. - auto handle_object_to_copy = [this](ConstTableRef table, ColKey col, Mixed value) -> Mixed { - TableRef target_table; - ObjKey target_obj_key; - if (is_outgoing_embedded_link(table, col, value, target_table, target_obj_key)) { - auto copy_target_object = target_table->create_linked_object(); - auto target_obj = target_table->get_object(target_obj_key); - copy_target_object.copy(target_obj); - return ObjLink{target_table->get_key(), copy_target_object.get_key()}; - } - return value; - }; - REALM_ASSERT(get_table() == other.get_table()); auto cols = m_table->get_column_keys(); for (auto col : cols) { + + TableRef target_table; + ObjKey target_obj_key; + if (col.get_attrs().test(col_attr_List)) { auto src_list = other.get_listbase_ptr(col); auto dst_list = get_listbase_ptr(col); @@ -2020,7 +2020,14 @@ void Obj::copy(const Obj& other) dst_list->clear(); for (size_t i = 0; i < sz; i++) { Mixed value = src_list->get_any(i); - dst_list->insert_any(i, handle_object_to_copy(m_table, col, value)); + if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { + dst_list->insert_any(i, value); + } + else { + auto target = target_table->create_linked_object(); + auto src = target_table->get_object(target_obj_key); + embedded_links.push_back({target_table, col, target, src, i}); + } } } else if (col.get_attrs().test(col_attr_Set)) { @@ -2030,7 +2037,14 @@ void Obj::copy(const Obj& other) dst_set->clear(); for (size_t i = 0; i < sz; ++i) { Mixed value = src_set->get_any(i); - dst_set->insert_any(handle_object_to_copy(m_table, col, value)); + if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { + dst_set->insert_any(value); + } + else { + auto target = target_table->create_linked_object(); + auto src = target_table->get_object(target_obj_key); + embedded_links.push_back({target_table, col, target, src, 0}); + } } } else if (col.get_attrs().test(col_attr_Dictionary)) { @@ -2040,14 +2054,12 @@ void Obj::copy(const Obj& other) dst_dictionary->clear(); for (size_t i = 0; i < sz; ++i) { const auto& [key, value] = src_dictionary->get_pair(i); - TableRef target_table; - ObjKey target_obj_key; - if (is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { - auto obj = dst_dictionary->create_and_insert_linked_object(key); - obj.copy(target_table->get_object(target_obj_key)); - } - else { + if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) dst_dictionary->insert(key, value); + else { + auto target = dst_dictionary->create_and_insert_linked_object(key); + auto src = target_table->get_object(target_obj_key); + embedded_links.push_back({target_table, col, target, src, 0}); } } } @@ -2070,9 +2082,17 @@ void Obj::copy(const Obj& other) this->set(col, BinaryData(str)); break; } - default: - this->set_any(col, handle_object_to_copy(other.get_table(), col, value)); + default: { + if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { + this->set_any(col, value); + } + else { + auto target = target_table->create_linked_object(); + auto src = target_table->get_object(target_obj_key); + embedded_links.push_back({target_table, col, target, src, 0}); + } break; + } } } } @@ -2140,6 +2160,47 @@ bool Obj::is_outgoing_embedded_link(ConstTableRef source_table, ColKey col_key, return false; } +void Obj::handle_copy_for_embedded_links(CopyEmbeddedLinkTracker& tracker) const +{ + for (size_t i = 0; i < tracker.size(); ++i) { + auto [table, col, target, src, pos] = tracker[i]; + Obj::CopyEmbeddedLinkTracker local_tracker; + if (col.get_attrs().test(col_attr_List)) { + target.copy(src, local_tracker); + if (local_tracker.empty()) { + src.get_listbase_ptr(col)->set_any(pos, ObjLink{table->get_key(), target.get_key()}); + } + else { + tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); + } + } + else if (col.get_attrs().test(col_attr_Set)) { + target.copy(src, local_tracker); + if (local_tracker.empty()) { + src.get_setbase_ptr(col)->insert_any(ObjLink{table->get_key(), target.get_key()}); + } + else { + tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); + } + } + else if (col.get_attrs().test(col_attr_Dictionary)) { + target.copy(src, local_tracker); + if (!local_tracker.empty()) { + tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); + } + } + else { + target.copy(src, local_tracker); + if (local_tracker.empty()) { + src.set_any(col, ObjLink{table->get_key(), target.get_key()}); + } + else { + tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); + } + } + } +} + Dictionary Obj::get_dictionary(ColKey col_key) const { REALM_ASSERT(col_key.is_dictionary()); diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index 09c14f237ca..a442d9e5a26 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -266,6 +266,12 @@ class Obj { // So this could potentially create a lot of user data duplication // if not used correctly. void assign(const Obj& other); + // The main algorithm for handling schema migrations if we try to convert + // from TopLevel* to Embedded, in this case all the orphan objects are deleted + // and all the objects with multiple backlinks are cloned in order to avoid to + // get schema violations during the migration. + // By default this alogirithm is disabled. RealmConfig contains a boolean flag + // to enable it. void handle_multiple_backlinks_during_schema_migration(); Obj get_linked_object(ColKey link_col_key) const @@ -432,10 +438,13 @@ class Obj { // incoming links to some embedded table, this method is basically harmless, but Obj::assign uses it, so // in case of copies outside a migration, probably we should throw an excpetion if we attempt to copy. // an object that contains a link to an embedded table. - void copy(const Obj& other); + using CopyEmbeddedLinkTracker = std::vector>; + void copy(const Obj& other, CopyEmbeddedLinkTracker&); + void fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const; bool is_outgoing_embedded_link(ConstTableRef source_table, ColKey col_key, Mixed value, TableRef& target_table, ObjKey& target_obj_key) const; + void handle_copy_for_embedded_links(CopyEmbeddedLinkTracker&) const; }; std::ostream& operator<<(std::ostream&, const Obj& obj); From 9fd97b6b2915207ce3ce47d8bb6ee1583c29f46c Mon Sep 17 00:00:00 2001 From: James Stone Date: Thu, 25 Aug 2022 01:54:40 -0700 Subject: [PATCH 20/25] Reuse object converters in migration (#5773) * move converters to core level * use object_converters in migration * handle mixed non-pk objects in converter --- Package.swift | 1 + src/realm/CMakeLists.txt | 2 + src/realm/obj.cpp | 182 +------ src/realm/obj.hpp | 11 - src/realm/object_converter.cpp | 468 ++++++++++++++++++ src/realm/object_converter.hpp | 87 ++++ src/realm/sync/noinst/client_reset.cpp | 420 +--------------- src/realm/sync/noinst/client_reset.hpp | 59 --- .../sync/noinst/client_reset_recovery.cpp | 3 +- test/object-store/migrations.cpp | 2 +- 10 files changed, 574 insertions(+), 661 deletions(-) create mode 100644 src/realm/object_converter.cpp create mode 100644 src/realm/object_converter.hpp diff --git a/Package.swift b/Package.swift index 62b09451305..4855e76dcaa 100644 --- a/Package.swift +++ b/Package.swift @@ -80,6 +80,7 @@ let notSyncServerSources: [String] = [ "realm/node.cpp", "realm/obj.cpp", "realm/obj_list.cpp", + "realm/object_converter.cpp", "realm/object_id.cpp", "realm/query.cpp", "realm/query_engine.cpp", diff --git a/src/realm/CMakeLists.txt b/src/realm/CMakeLists.txt index 39297d96b5d..9f09a78892c 100644 --- a/src/realm/CMakeLists.txt +++ b/src/realm/CMakeLists.txt @@ -45,6 +45,7 @@ set(REALM_SOURCES node.cpp mixed.cpp obj.cpp + object_converter.cpp global_key.cpp query_engine.cpp query_expression.cpp @@ -169,6 +170,7 @@ set(REALM_INSTALL_HEADERS null.hpp obj.hpp obj_list.hpp + object_converter.hpp object_id.hpp owned_data.hpp query.hpp diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 0ad29f4ab4e..4349fbd1b5c 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -17,7 +17,6 @@ **************************************************************************/ #include "realm/obj.hpp" -#include "realm/cluster_tree.hpp" #include "realm/array_basic.hpp" #include "realm/array_integer.hpp" #include "realm/array_bool.hpp" @@ -30,14 +29,15 @@ #include "realm/array_fixed_bytes.hpp" #include "realm/array_backlink.hpp" #include "realm/array_typed_link.hpp" +#include "realm/cluster_tree.hpp" #include "realm/column_type_traits.hpp" +#include "realm/dictionary.hpp" #include "realm/index_string.hpp" -#include "realm/cluster_tree.hpp" -#include "realm/spec.hpp" +#include "realm/object_converter.hpp" +#include "realm/replication.hpp" #include "realm/set.hpp" -#include "realm/dictionary.hpp" +#include "realm/spec.hpp" #include "realm/table_view.hpp" -#include "realm/replication.hpp" #include "realm/util/base64.hpp" namespace realm { @@ -1936,8 +1936,10 @@ bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) void Obj::assign(const Obj& other) { - Obj::CopyEmbeddedLinkTracker tracker; - copy(other, tracker); + auto embedded_obj_tracker = std::make_shared(); + converters::InterRealmObjectConverter converter(this->get_table(), this->get_table(), embedded_obj_tracker); + converter.copy(other, *this, nullptr); + embedded_obj_tracker->process_pending(); auto copy_links = [this, &other](ColKey col) { auto t = m_table->get_opposite_table(col); @@ -1983,121 +1985,23 @@ void Obj::handle_multiple_backlinks_during_schema_migration() { REALM_ASSERT(!m_table->get_primary_key_column()); auto copy_links = [this](ColKey col) { + auto embedded_obj_tracker = std::make_shared(); auto opposite_table = m_table->get_opposite_table(col); auto opposite_column = m_table->get_opposite_column(col); auto backlinks = get_all_backlinks(col); for (auto backlink : backlinks) { // create a new obj auto obj = m_table->create_object(); - // try to deep copy all the properties in the new object - Obj::CopyEmbeddedLinkTracker tracker; - obj.copy(*this, tracker); - // if there are embedded links, then copy them separately - handle_copy_for_embedded_links(tracker); - // fix all the backlinks + embedded_obj_tracker->track(*this, obj); auto linking_obj = opposite_table->get_object(backlink); fix_linking_object_during_schema_migration(linking_obj, obj, opposite_column); } + embedded_obj_tracker->process_pending(); return false; }; m_table->for_each_backlink_column(copy_links); } -void Obj::copy(const Obj& other, Obj::CopyEmbeddedLinkTracker& embedded_links) -{ - // handle correctly the case in which the value to copy is an outgoing link to an embedded table. - REALM_ASSERT(get_table() == other.get_table()); - auto cols = m_table->get_column_keys(); - for (auto col : cols) { - - TableRef target_table; - ObjKey target_obj_key; - - if (col.get_attrs().test(col_attr_List)) { - auto src_list = other.get_listbase_ptr(col); - auto dst_list = get_listbase_ptr(col); - auto sz = src_list->size(); - dst_list->clear(); - for (size_t i = 0; i < sz; i++) { - Mixed value = src_list->get_any(i); - if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { - dst_list->insert_any(i, value); - } - else { - auto target = target_table->create_linked_object(); - auto src = target_table->get_object(target_obj_key); - embedded_links.push_back({target_table, col, target, src, i}); - } - } - } - else if (col.get_attrs().test(col_attr_Set)) { - auto src_set = other.get_setbase_ptr(col); - auto dst_set = get_setbase_ptr(col); - auto sz = src_set->size(); - dst_set->clear(); - for (size_t i = 0; i < sz; ++i) { - Mixed value = src_set->get_any(i); - if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { - dst_set->insert_any(value); - } - else { - auto target = target_table->create_linked_object(); - auto src = target_table->get_object(target_obj_key); - embedded_links.push_back({target_table, col, target, src, 0}); - } - } - } - else if (col.get_attrs().test(col_attr_Dictionary)) { - auto src_dictionary = other.get_dictionary_ptr(col); - auto dst_dictionary = get_dictionary_ptr(col); - auto sz = src_dictionary->size(); - dst_dictionary->clear(); - for (size_t i = 0; i < sz; ++i) { - const auto& [key, value] = src_dictionary->get_pair(i); - if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) - dst_dictionary->insert(key, value); - else { - auto target = dst_dictionary->create_and_insert_linked_object(key); - auto src = target_table->get_object(target_obj_key); - embedded_links.push_back({target_table, col, target, src, 0}); - } - } - } - else { - Mixed value = other.get_any(col); - if (value.is_null()) { - this->set_null(col); - continue; - } - switch (value.get_type()) { - case type_String: { - // Need to take copy. Values might be in same cluster - std::string str{value.get_string()}; - this->set(col, str); - break; - } - case type_Binary: { - // Need to take copy. Values might be in same cluster - std::string str{value.get_binary()}; - this->set(col, BinaryData(str)); - break; - } - default: { - if (!is_outgoing_embedded_link(m_table, col, value, target_table, target_obj_key)) { - this->set_any(col, value); - } - else { - auto target = target_table->create_linked_object(); - auto src = target_table->get_object(target_obj_key); - embedded_links.push_back({target_table, col, target, src, 0}); - } - break; - } - } - } - } -} - void Obj::fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const { // fix linking obj links in order to point to the new object just created @@ -2139,68 +2043,6 @@ void Obj::fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, C } } -bool Obj::is_outgoing_embedded_link(ConstTableRef source_table, ColKey col_key, Mixed value, TableRef& target_table, - ObjKey& target_obj_key) const -{ - if (value.is_type(type_Link)) { - target_table = source_table->get_opposite_table(col_key); - if (target_table) { - target_obj_key = value.get(); - return target_table->is_embedded(); - } - } - if (value.is_type(type_TypedLink)) { - target_table = source_table->get_opposite_table(col_key); - if (target_table) { - auto link = value.get_link(); - target_obj_key = link.get_obj_key(); - return target_table->is_embedded(); - } - } - return false; -} - -void Obj::handle_copy_for_embedded_links(CopyEmbeddedLinkTracker& tracker) const -{ - for (size_t i = 0; i < tracker.size(); ++i) { - auto [table, col, target, src, pos] = tracker[i]; - Obj::CopyEmbeddedLinkTracker local_tracker; - if (col.get_attrs().test(col_attr_List)) { - target.copy(src, local_tracker); - if (local_tracker.empty()) { - src.get_listbase_ptr(col)->set_any(pos, ObjLink{table->get_key(), target.get_key()}); - } - else { - tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); - } - } - else if (col.get_attrs().test(col_attr_Set)) { - target.copy(src, local_tracker); - if (local_tracker.empty()) { - src.get_setbase_ptr(col)->insert_any(ObjLink{table->get_key(), target.get_key()}); - } - else { - tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); - } - } - else if (col.get_attrs().test(col_attr_Dictionary)) { - target.copy(src, local_tracker); - if (!local_tracker.empty()) { - tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); - } - } - else { - target.copy(src, local_tracker); - if (local_tracker.empty()) { - src.set_any(col, ObjLink{table->get_key(), target.get_key()}); - } - else { - tracker.insert(tracker.end(), local_tracker.begin(), local_tracker.end()); - } - } - } -} - Dictionary Obj::get_dictionary(ColKey col_key) const { REALM_ASSERT(col_key.is_dictionary()); diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index a442d9e5a26..c4dfb252ab8 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -433,18 +433,7 @@ class Obj { template inline void set_spec(T&, ColKey); - // To be used carefully since it could potentially duplicate user data. - // since Obj::copy is for now basically only used for handling schema migration when there are multiple - // incoming links to some embedded table, this method is basically harmless, but Obj::assign uses it, so - // in case of copies outside a migration, probably we should throw an excpetion if we attempt to copy. - // an object that contains a link to an embedded table. - using CopyEmbeddedLinkTracker = std::vector>; - void copy(const Obj& other, CopyEmbeddedLinkTracker&); - void fix_linking_object_during_schema_migration(Obj linking_obj, Obj obj, ColKey opposite_col_key) const; - bool is_outgoing_embedded_link(ConstTableRef source_table, ColKey col_key, Mixed value, TableRef& target_table, - ObjKey& target_obj_key) const; - void handle_copy_for_embedded_links(CopyEmbeddedLinkTracker&) const; }; std::ostream& operator<<(std::ostream&, const Obj& obj); diff --git a/src/realm/object_converter.cpp b/src/realm/object_converter.cpp new file mode 100644 index 00000000000..b3902a535d7 --- /dev/null +++ b/src/realm/object_converter.cpp @@ -0,0 +1,468 @@ +/////////////////////////////////////////////////////////////////////////// +// +// Copyright 2022 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#include + +#include +#include +#include + +#include + +namespace realm::converters { + +// Takes two lists, src and dst, and makes dst equal src. src is unchanged. +void InterRealmValueConverter::copy_list(const Obj& src_obj, Obj& dst_obj, bool* update_out) +{ + // The two arrays are compared by finding the longest common prefix and + // suffix. The middle section differs between them and is made equal by + // updating the middle section of dst. + // + // Example: + // src = abcdefghi + // dst = abcxyhi + // The common prefix is abc. The common suffix is hi. xy is replaced by defg. + LstBasePtr src = src_obj.get_listbase_ptr(m_src_col); + LstBasePtr dst = dst_obj.get_listbase_ptr(m_dst_col); + + bool updated = false; + size_t len_src = src->size(); + size_t len_dst = dst->size(); + size_t len_min = std::min(len_src, len_dst); + + size_t ndx = 0; + size_t suffix_len = 0; + + while (ndx < len_min && cmp_src_to_dst(src->get_any(ndx), dst->get_any(ndx), nullptr, update_out) == 0) { + ndx++; + } + + size_t suffix_len_max = len_min - ndx; + + while (suffix_len < suffix_len_max && + cmp_src_to_dst(src->get_any(len_src - 1 - suffix_len), dst->get_any(len_dst - 1 - suffix_len), nullptr, + update_out) == 0) { + suffix_len++; + } + + len_min -= (ndx + suffix_len); + + for (size_t i = 0; i < len_min; i++) { + InterRealmValueConverter::ConversionResult converted_src; + if (cmp_src_to_dst(src->get_any(ndx), dst->get_any(ndx), &converted_src, update_out)) { + if (converted_src.requires_new_embedded_object) { + auto lnklist = dynamic_cast(dst.get()); + REALM_ASSERT(lnklist); // this is the only type of list that supports embedded objects + Obj embedded = lnklist->create_and_set_linked_object(ndx); + track_new_embedded(converted_src.src_embedded_to_check, embedded); + } + else { + dst->set_any(ndx, converted_src.converted_value); + } + updated = true; + } + ndx++; + } + + // New elements must be inserted in dst. + while (len_dst < len_src) { + InterRealmValueConverter::ConversionResult converted_src; + cmp_src_to_dst(src->get_any(ndx), Mixed{}, &converted_src, update_out); + if (converted_src.requires_new_embedded_object) { + auto lnklist = dynamic_cast(dst.get()); + REALM_ASSERT(lnklist); // this is the only type of list that supports embedded objects + Obj embedded = lnklist->create_and_insert_linked_object(ndx); + track_new_embedded(converted_src.src_embedded_to_check, embedded); + } + else { + dst->insert_any(ndx, converted_src.converted_value); + } + len_dst++; + ndx++; + updated = true; + } + // Excess elements must be removed from ll_dst. + if (len_dst > len_src) { + dst->remove(len_src - suffix_len, len_dst - suffix_len); + updated = true; + } + + REALM_ASSERT(dst->size() == len_src); + if (updated && update_out) { + *update_out = updated; + } +} + +void InterRealmValueConverter::copy_set(const Obj& src_obj, Obj& dst_obj, bool* update_out) +{ + SetBasePtr src = src_obj.get_setbase_ptr(m_src_col); + SetBasePtr dst = dst_obj.get_setbase_ptr(m_dst_col); + + std::vector sorted_src, sorted_dst, to_insert, to_delete; + constexpr bool ascending = true; + // the implementation could be storing elements in sorted order, but + // we don't assume that here. + src->sort(sorted_src, ascending); + dst->sort(sorted_dst, ascending); + + size_t dst_ndx = 0; + size_t src_ndx = 0; + while (src_ndx < sorted_src.size()) { + if (dst_ndx == sorted_dst.size()) { + // if we have reached the end of the dst items, all remaining + // src items should be added + while (src_ndx < sorted_src.size()) { + to_insert.push_back(sorted_src[src_ndx++]); + } + break; + } + size_t ndx_in_src = sorted_src[src_ndx]; + Mixed src_val = src->get_any(ndx_in_src); + while (dst_ndx < sorted_dst.size()) { + size_t ndx_in_dst = sorted_dst[dst_ndx]; + + int cmp = cmp_src_to_dst(src_val, dst->get_any(ndx_in_dst), nullptr, update_out); + if (cmp == 0) { + // equal: advance both src and dst + ++dst_ndx; + ++src_ndx; + break; + } + else if (cmp < 0) { + // src < dst: insert src, advance src only + to_insert.push_back(ndx_in_src); + ++src_ndx; + break; + } + else { + // src > dst: delete dst, advance only dst + to_delete.push_back(ndx_in_dst); + ++dst_ndx; + continue; + } + } + } + while (dst_ndx < sorted_dst.size()) { + to_delete.push_back(sorted_dst[dst_ndx++]); + } + + std::sort(to_delete.begin(), to_delete.end()); + for (auto it = to_delete.rbegin(); it != to_delete.rend(); ++it) { + dst->erase_any(dst->get_any(*it)); + } + for (auto ndx : to_insert) { + InterRealmValueConverter::ConversionResult converted_src; + cmp_src_to_dst(src->get_any(ndx), Mixed{}, &converted_src, update_out); + // we do not support a set of embedded objects + REALM_ASSERT(!converted_src.requires_new_embedded_object); + dst->insert_any(converted_src.converted_value); + } + + if (update_out && (to_delete.size() || to_insert.size())) { + *update_out = true; + } +} + +void InterRealmValueConverter::copy_dictionary(const Obj& src_obj, Obj& dst_obj, bool* update_out) +{ + Dictionary src = src_obj.get_dictionary(m_src_col); + Dictionary dst = dst_obj.get_dictionary(m_dst_col); + + std::vector sorted_src, sorted_dst, to_insert, to_delete; + constexpr bool ascending = true; + src.sort_keys(sorted_src, ascending); + dst.sort_keys(sorted_dst, ascending); + + size_t dst_ndx = 0; + size_t src_ndx = 0; + while (src_ndx < sorted_src.size()) { + if (dst_ndx == sorted_dst.size()) { + // if we have reached the end of the dst items, all remaining + // src items should be added + while (src_ndx < sorted_src.size()) { + to_insert.push_back(sorted_src[src_ndx++]); + } + break; + } + + auto src_val = src.get_pair(sorted_src[src_ndx]); + while (dst_ndx < sorted_dst.size()) { + auto dst_val = dst.get_pair(sorted_dst[dst_ndx]); + int cmp = src_val.first.compare(dst_val.first); + if (cmp == 0) { + // Check if the values differ + if (cmp_src_to_dst(src_val.second, dst_val.second, nullptr, update_out)) { + // values are different - modify destination, advance both + to_insert.push_back(sorted_src[src_ndx]); + } + // keys and values equal: advance both src and dst + ++dst_ndx; + ++src_ndx; + break; + } + else if (cmp < 0) { + // src < dst: insert src, advance src only + to_insert.push_back(sorted_src[src_ndx++]); + break; + } + else { + // src > dst: delete dst, advance only dst + to_delete.push_back(sorted_dst[dst_ndx++]); + continue; + } + } + } + // at this point, we've gone through all src items but still have dst items + // oustanding; these should all be deleted because they are not in src + while (dst_ndx < sorted_dst.size()) { + to_delete.push_back(sorted_dst[dst_ndx++]); + } + + std::sort(to_delete.begin(), to_delete.end()); + for (auto it = to_delete.rbegin(); it != to_delete.rend(); ++it) { + dst.erase(dst.begin() + *it); + } + for (auto ndx : to_insert) { + auto pair = src.get_pair(ndx); + InterRealmValueConverter::ConversionResult converted_val; + cmp_src_to_dst(pair.second, Mixed{}, &converted_val, update_out); + if (converted_val.requires_new_embedded_object) { + Obj new_embedded = dst.create_and_insert_linked_object(pair.first); + track_new_embedded(converted_val.src_embedded_to_check, new_embedded); + } + else { + dst.insert(pair.first, converted_val.converted_value); + } + } + if (update_out && (to_delete.size() || to_insert.size())) { + *update_out = true; + } +} + +void InterRealmValueConverter::copy_value(const Obj& src_obj, Obj& dst_obj, bool* update_out) +{ + if (m_src_col.is_list()) { + copy_list(src_obj, dst_obj, update_out); + } + else if (m_src_col.is_dictionary()) { + copy_dictionary(src_obj, dst_obj, update_out); + } + else if (m_src_col.is_set()) { + copy_set(src_obj, dst_obj, update_out); + } + else { + REALM_ASSERT(!m_src_col.is_collection()); + InterRealmValueConverter::ConversionResult converted_src; + if (cmp_src_to_dst(src_obj.get_any(m_src_col), dst_obj.get_any(m_dst_col), &converted_src, update_out)) { + if (converted_src.requires_new_embedded_object) { + Obj new_embedded = dst_obj.create_and_set_linked_object(m_dst_col); + track_new_embedded(converted_src.src_embedded_to_check, new_embedded); + } + else { + dst_obj.set_any(m_dst_col, converted_src.converted_value); + } + } + } +} + + +// If an embedded object is encountered, add it to a list of embedded objects to process. +// This relies on the property that embedded objects only have one incoming link +// otherwise there could be an infinite loop while discovering embedded objects. +void EmbeddedObjectConverter::track(Obj e_src, Obj e_dst) +{ + embedded_pending.push_back({e_src, e_dst}); +} + +void EmbeddedObjectConverter::process_pending() +{ + // Conceptually this is a map, but doing a linear search through a vector is known + // to be faster for small number of elements. Since the number of tables expected + // to be processed here is assumed to be small < 20, use linear search instead of + // hashing. N is the depth to which embedded objects are connected and the upper + // bound is the total number of tables which is finite, and is usually small. + util::FlatMap converters; + + while (!embedded_pending.empty()) { + EmbeddedToCheck pending = embedded_pending.back(); + embedded_pending.pop_back(); + TableRef src_table = pending.embedded_in_src.get_table(); + TableRef dst_table = pending.embedded_in_dst.get_table(); + TableKey dst_table_key = dst_table->get_key(); + auto it_with_did_insert = + converters.insert({dst_table_key, InterRealmObjectConverter{src_table, dst_table, shared_from_this()}}); + InterRealmObjectConverter& converter = it_with_did_insert.first->second; + converter.copy(pending.embedded_in_src, pending.embedded_in_dst, nullptr); + } +} + +InterRealmValueConverter::InterRealmValueConverter(ConstTableRef src_table, ColKey src_col, ConstTableRef dst_table, + ColKey dst_col, std::shared_ptr ec) + : m_src_table(src_table) + , m_dst_table(dst_table) + , m_src_col(src_col) + , m_dst_col(dst_col) + , m_embedded_converter(ec) + , m_is_embedded_link(false) + , m_primitive_types_only(!(src_col.get_type() == col_type_TypedLink || src_col.get_type() == col_type_Link || + src_col.get_type() == col_type_LinkList || src_col.get_type() == col_type_Mixed)) +{ + if (!m_primitive_types_only) { + REALM_ASSERT(src_table); + m_opposite_of_src = src_table->get_opposite_table(src_col); + m_opposite_of_dst = dst_table->get_opposite_table(dst_col); + REALM_ASSERT(bool(m_opposite_of_src) == bool(m_opposite_of_dst)); + if (m_opposite_of_src) { + m_is_embedded_link = m_opposite_of_src->is_embedded(); + } + } +} + +void InterRealmValueConverter::track_new_embedded(Obj src, Obj dst) +{ + m_embedded_converter->track(src, dst); +} + +// convert `src` to the destination Realm and compare that value with `dst` +// If `converted_src_out` is provided, it will be set to the converted src value +int InterRealmValueConverter::cmp_src_to_dst(Mixed src, Mixed dst, ConversionResult* converted_src_out, + bool* did_update_out) +{ + int cmp = 0; + Mixed converted_src; + if (m_primitive_types_only || !src.is_type(type_Link, type_TypedLink)) { + converted_src = src; + cmp = src.compare(dst); + } + else if (m_opposite_of_src) { + ObjKey src_link_key = src.get(); + if (m_is_embedded_link) { + Obj src_embedded = m_opposite_of_src->get_object(src_link_key); + REALM_ASSERT_DEBUG(src_embedded.is_valid()); + if (dst.is_type(type_Link, type_TypedLink)) { + cmp = 0; // no need to set this link, there is already an embedded object here + Obj dst_embedded = m_opposite_of_dst->get_object(dst.get()); + REALM_ASSERT_DEBUG(dst_embedded.is_valid()); + converted_src = dst_embedded.get_key(); + track_new_embedded(src_embedded, dst_embedded); + } + else { + cmp = src.compare(dst); + if (converted_src_out) { + converted_src_out->requires_new_embedded_object = true; + converted_src_out->src_embedded_to_check = src_embedded; + } + } + } + else { + Obj dst_link; + if (m_opposite_of_src->get_primary_key_column()) { + Mixed src_link_pk = m_opposite_of_src->get_primary_key(src_link_key); + dst_link = m_opposite_of_dst->create_object_with_primary_key(src_link_pk, did_update_out); + } + else { + if (m_opposite_of_dst == m_opposite_of_src) { + // if this is the same Realm, we can use the ObjKey + dst_link = m_opposite_of_dst->get_object(src_link_key); + } + else { + // in different Realms we create a new object + dst_link = m_opposite_of_dst->create_object(); + } + } + converted_src = dst_link.get_key(); + if (dst.is_type(type_TypedLink)) { + cmp = converted_src.compare(dst.get()); + } + else { + cmp = converted_src.compare(dst); + } + } + } + else { + ObjLink src_link = src.get(); + if (src_link.is_unresolved()) { + converted_src = Mixed{}; // no need to transfer over unresolved links + cmp = converted_src.compare(dst); + } + else { + TableRef src_link_table = m_src_table->get_parent_group()->get_table(src_link.get_table_key()); + REALM_ASSERT_EX(src_link_table, src_link.get_table_key()); + TableRef dst_link_table = m_dst_table->get_parent_group()->get_table(src_link_table->get_name()); + REALM_ASSERT_EX(dst_link_table, src_link_table->get_name()); + // embedded tables should always be covered by the m_opposite_of_src case above. + REALM_ASSERT_EX(!src_link_table->is_embedded(), src_link_table->get_name()); + // regular table, convert by pk + if (src_link_table->get_primary_key_column()) { + Mixed src_pk = src_link_table->get_primary_key(src_link.get_obj_key()); + Obj dst_link = dst_link_table->create_object_with_primary_key(src_pk, did_update_out); + converted_src = ObjLink{dst_link_table->get_key(), dst_link.get_key()}; + } + else if (src_link_table == dst_link_table) { + // no pk, but this is the same Realm, so convert by ObjKey + Obj dst_link = dst_link_table->get_object(src_link.get_obj_key()); + converted_src = ObjLink{dst_link_table->get_key(), dst_link.get_key()}; + } + else { + // no pk, and different Realm, create an object + Obj dst_link = dst_link_table->create_object(); + converted_src = ObjLink{dst_link_table->get_key(), dst_link.get_key()}; + } + cmp = converted_src.compare(dst); + } + } + if (converted_src_out) { + converted_src_out->converted_value = converted_src; + } + if (did_update_out && cmp) { + *did_update_out = true; + } + return cmp; +} + +InterRealmObjectConverter::InterRealmObjectConverter(ConstTableRef table_src, TableRef table_dst, + std::shared_ptr embedded_tracker) + : m_embedded_tracker(embedded_tracker) +{ + populate_columns_from_table(table_src, table_dst); +} + +void InterRealmObjectConverter::copy(const Obj& src, Obj& dst, bool* update_out) +{ + for (auto& column : m_columns_cache) { + column.copy_value(src, dst, update_out); + } +} + +void InterRealmObjectConverter::populate_columns_from_table(ConstTableRef table_src, ConstTableRef table_dst) +{ + m_columns_cache.clear(); + m_columns_cache.reserve(table_src->get_column_count()); + ColKey pk_col = table_src->get_primary_key_column(); + for (ColKey col_key_src : table_src->get_column_keys()) { + if (col_key_src == pk_col) + continue; + StringData col_name = table_src->get_column_name(col_key_src); + ColKey col_key_dst = table_dst->get_column_key(col_name); + REALM_ASSERT(col_key_dst); + m_columns_cache.emplace_back( + InterRealmValueConverter(table_src, col_key_src, table_dst, col_key_dst, m_embedded_tracker)); + } +} + +} // namespace realm::converters diff --git a/src/realm/object_converter.hpp b/src/realm/object_converter.hpp new file mode 100644 index 00000000000..9b95841ebcc --- /dev/null +++ b/src/realm/object_converter.hpp @@ -0,0 +1,87 @@ +/////////////////////////////////////////////////////////////////////////// +// +// Copyright 2022 Realm Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////// + +#ifndef REALM_OBJECT_CONVERTER_HPP +#define REALM_OBJECT_CONVERTER_HPP + +#include +#include + +#include + +namespace realm::converters { + +struct EmbeddedObjectConverter : std::enable_shared_from_this { + void track(Obj e_src, Obj e_dst); + void process_pending(); + +private: + struct EmbeddedToCheck { + Obj embedded_in_src; + Obj embedded_in_dst; + }; + std::vector embedded_pending; +}; + +struct InterRealmValueConverter { + InterRealmValueConverter(ConstTableRef src_table, ColKey src_col, ConstTableRef dst_table, ColKey dst_col, + std::shared_ptr ec); + void track_new_embedded(Obj src, Obj dst); + struct ConversionResult { + Mixed converted_value; + bool requires_new_embedded_object = false; + Obj src_embedded_to_check; + }; + + // convert `src` to the destination Realm and compare that value with `dst` + // If `converted_src_out` is provided, it will be set to the converted src value + int cmp_src_to_dst(Mixed src, Mixed dst, ConversionResult* converted_src_out = nullptr, + bool* did_update_out = nullptr); + void copy_value(const Obj& src_obj, Obj& dst_obj, bool* update_out); + +private: + void copy_list(const Obj& src_obj, Obj& dst_obj, bool* update_out); + void copy_set(const Obj& src_obj, Obj& dst_obj, bool* update_out); + void copy_dictionary(const Obj& src_obj, Obj& dst_obj, bool* update_out); + + TableRef m_dst_link_table; + ConstTableRef m_src_table; + ConstTableRef m_dst_table; + ColKey m_src_col; + ColKey m_dst_col; + TableRef m_opposite_of_src; + TableRef m_opposite_of_dst; + std::shared_ptr m_embedded_converter; + bool m_is_embedded_link; + const bool m_primitive_types_only; +}; + +struct InterRealmObjectConverter { + InterRealmObjectConverter(ConstTableRef table_src, TableRef table_dst, + std::shared_ptr embedded_tracker); + void copy(const Obj& src, Obj& dst, bool* update_out); + +private: + void populate_columns_from_table(ConstTableRef table_src, ConstTableRef table_dst); + std::shared_ptr m_embedded_tracker; + std::vector m_columns_cache; +}; + +} // namespace realm::converters + +#endif // REALM_OBJECT_CONVERTER_HPP diff --git a/src/realm/sync/noinst/client_reset.cpp b/src/realm/sync/noinst/client_reset.cpp index c8a6d9a71c1..9e527b33985 100644 --- a/src/realm/sync/noinst/client_reset.cpp +++ b/src/realm/sync/noinst/client_reset.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -64,425 +65,6 @@ std::ostream& operator<<(std::ostream& os, const ClientResyncMode& mode) } // namespace realm -namespace realm::_impl::client_reset::converters { - -// Takes two lists, src and dst, and makes dst equal src. src is unchanged. -void InterRealmValueConverter::copy_list(const Obj& src_obj, Obj& dst_obj, bool* update_out) -{ - // The two arrays are compared by finding the longest common prefix and - // suffix. The middle section differs between them and is made equal by - // updating the middle section of dst. - // - // Example: - // src = abcdefghi - // dst = abcxyhi - // The common prefix is abc. The common suffix is hi. xy is replaced by defg. - LstBasePtr src = src_obj.get_listbase_ptr(m_src_col); - LstBasePtr dst = dst_obj.get_listbase_ptr(m_dst_col); - - bool updated = false; - size_t len_src = src->size(); - size_t len_dst = dst->size(); - size_t len_min = std::min(len_src, len_dst); - - size_t ndx = 0; - size_t suffix_len = 0; - - while (ndx < len_min && cmp_src_to_dst(src->get_any(ndx), dst->get_any(ndx), nullptr, update_out) == 0) { - ndx++; - } - - size_t suffix_len_max = len_min - ndx; - - while (suffix_len < suffix_len_max && - cmp_src_to_dst(src->get_any(len_src - 1 - suffix_len), dst->get_any(len_dst - 1 - suffix_len), nullptr, - update_out) == 0) { - suffix_len++; - } - - len_min -= (ndx + suffix_len); - - for (size_t i = 0; i < len_min; i++) { - InterRealmValueConverter::ConversionResult converted_src; - if (cmp_src_to_dst(src->get_any(ndx), dst->get_any(ndx), &converted_src, update_out)) { - if (converted_src.requires_new_embedded_object) { - auto lnklist = dynamic_cast(dst.get()); - REALM_ASSERT(lnklist); // this is the only type of list that supports embedded objects - Obj embedded = lnklist->create_and_set_linked_object(ndx); - track_new_embedded(converted_src.src_embedded_to_check, embedded); - } - else { - dst->set_any(ndx, converted_src.converted_value); - } - updated = true; - } - ndx++; - } - - // New elements must be inserted in dst. - while (len_dst < len_src) { - InterRealmValueConverter::ConversionResult converted_src; - cmp_src_to_dst(src->get_any(ndx), Mixed{}, &converted_src, update_out); - if (converted_src.requires_new_embedded_object) { - auto lnklist = dynamic_cast(dst.get()); - REALM_ASSERT(lnklist); // this is the only type of list that supports embedded objects - Obj embedded = lnklist->create_and_insert_linked_object(ndx); - track_new_embedded(converted_src.src_embedded_to_check, embedded); - } - else { - dst->insert_any(ndx, converted_src.converted_value); - } - len_dst++; - ndx++; - updated = true; - } - // Excess elements must be removed from ll_dst. - if (len_dst > len_src) { - dst->remove(len_src - suffix_len, len_dst - suffix_len); - updated = true; - } - - REALM_ASSERT(dst->size() == len_src); - if (updated && update_out) { - *update_out = updated; - } -} - -void InterRealmValueConverter::copy_set(const Obj& src_obj, Obj& dst_obj, bool* update_out) -{ - SetBasePtr src = src_obj.get_setbase_ptr(m_src_col); - SetBasePtr dst = dst_obj.get_setbase_ptr(m_dst_col); - - std::vector sorted_src, sorted_dst, to_insert, to_delete; - constexpr bool ascending = true; - // the implementation could be storing elements in sorted order, but - // we don't assume that here. - src->sort(sorted_src, ascending); - dst->sort(sorted_dst, ascending); - - size_t dst_ndx = 0; - size_t src_ndx = 0; - while (src_ndx < sorted_src.size()) { - if (dst_ndx == sorted_dst.size()) { - // if we have reached the end of the dst items, all remaining - // src items should be added - while (src_ndx < sorted_src.size()) { - to_insert.push_back(sorted_src[src_ndx++]); - } - break; - } - size_t ndx_in_src = sorted_src[src_ndx]; - Mixed src_val = src->get_any(ndx_in_src); - while (dst_ndx < sorted_dst.size()) { - size_t ndx_in_dst = sorted_dst[dst_ndx]; - - int cmp = cmp_src_to_dst(src_val, dst->get_any(ndx_in_dst), nullptr, update_out); - if (cmp == 0) { - // equal: advance both src and dst - ++dst_ndx; - ++src_ndx; - break; - } - else if (cmp < 0) { - // src < dst: insert src, advance src only - to_insert.push_back(ndx_in_src); - ++src_ndx; - break; - } - else { - // src > dst: delete dst, advance only dst - to_delete.push_back(ndx_in_dst); - ++dst_ndx; - continue; - } - } - } - while (dst_ndx < sorted_dst.size()) { - to_delete.push_back(sorted_dst[dst_ndx++]); - } - - std::sort(to_delete.begin(), to_delete.end()); - for (auto it = to_delete.rbegin(); it != to_delete.rend(); ++it) { - dst->erase_any(dst->get_any(*it)); - } - for (auto ndx : to_insert) { - InterRealmValueConverter::ConversionResult converted_src; - cmp_src_to_dst(src->get_any(ndx), Mixed{}, &converted_src, update_out); - // we do not support a set of embedded objects - REALM_ASSERT(!converted_src.requires_new_embedded_object); - dst->insert_any(converted_src.converted_value); - } - - if (update_out && (to_delete.size() || to_insert.size())) { - *update_out = true; - } -} - -void InterRealmValueConverter::copy_dictionary(const Obj& src_obj, Obj& dst_obj, bool* update_out) -{ - Dictionary src = src_obj.get_dictionary(m_src_col); - Dictionary dst = dst_obj.get_dictionary(m_dst_col); - - std::vector sorted_src, sorted_dst, to_insert, to_delete; - constexpr bool ascending = true; - src.sort_keys(sorted_src, ascending); - dst.sort_keys(sorted_dst, ascending); - - size_t dst_ndx = 0; - size_t src_ndx = 0; - while (src_ndx < sorted_src.size()) { - if (dst_ndx == sorted_dst.size()) { - // if we have reached the end of the dst items, all remaining - // src items should be added - while (src_ndx < sorted_src.size()) { - to_insert.push_back(sorted_src[src_ndx++]); - } - break; - } - - auto src_val = src.get_pair(sorted_src[src_ndx]); - while (dst_ndx < sorted_dst.size()) { - auto dst_val = dst.get_pair(sorted_dst[dst_ndx]); - int cmp = src_val.first.compare(dst_val.first); - if (cmp == 0) { - // Check if the values differ - if (cmp_src_to_dst(src_val.second, dst_val.second, nullptr, update_out)) { - // values are different - modify destination, advance both - to_insert.push_back(sorted_src[src_ndx]); - } - // keys and values equal: advance both src and dst - ++dst_ndx; - ++src_ndx; - break; - } - else if (cmp < 0) { - // src < dst: insert src, advance src only - to_insert.push_back(sorted_src[src_ndx++]); - break; - } - else { - // src > dst: delete dst, advance only dst - to_delete.push_back(sorted_dst[dst_ndx++]); - continue; - } - } - } - // at this point, we've gone through all src items but still have dst items - // oustanding; these should all be deleted because they are not in src - while (dst_ndx < sorted_dst.size()) { - to_delete.push_back(sorted_dst[dst_ndx++]); - } - - std::sort(to_delete.begin(), to_delete.end()); - for (auto it = to_delete.rbegin(); it != to_delete.rend(); ++it) { - dst.erase(dst.begin() + *it); - } - for (auto ndx : to_insert) { - auto pair = src.get_pair(ndx); - InterRealmValueConverter::ConversionResult converted_val; - cmp_src_to_dst(pair.second, Mixed{}, &converted_val, update_out); - if (converted_val.requires_new_embedded_object) { - Obj new_embedded = dst.create_and_insert_linked_object(pair.first); - track_new_embedded(converted_val.src_embedded_to_check, new_embedded); - } - else { - dst.insert(pair.first, converted_val.converted_value); - } - } - if (update_out && (to_delete.size() || to_insert.size())) { - *update_out = true; - } -} - -void InterRealmValueConverter::copy_value(const Obj& src_obj, Obj& dst_obj, bool* update_out) -{ - if (m_src_col.is_list()) { - copy_list(src_obj, dst_obj, update_out); - } - else if (m_src_col.is_dictionary()) { - copy_dictionary(src_obj, dst_obj, update_out); - } - else if (m_src_col.is_set()) { - copy_set(src_obj, dst_obj, update_out); - } - else { - REALM_ASSERT(!m_src_col.is_collection()); - InterRealmValueConverter::ConversionResult converted_src; - if (cmp_src_to_dst(src_obj.get_any(m_src_col), dst_obj.get_any(m_dst_col), &converted_src, update_out)) { - if (converted_src.requires_new_embedded_object) { - Obj new_embedded = dst_obj.create_and_set_linked_object(m_dst_col); - track_new_embedded(converted_src.src_embedded_to_check, new_embedded); - } - else { - dst_obj.set_any(m_dst_col, converted_src.converted_value); - } - } - } -} - - -// If an embedded object is encountered, add it to a list of embedded objects to process. -// This relies on the property that embedded objects only have one incoming link -// otherwise there could be an infinite loop while discovering embedded objects. -void EmbeddedObjectConverter::track(Obj e_src, Obj e_dst) -{ - embedded_pending.push_back({e_src, e_dst}); -} - -void EmbeddedObjectConverter::process_pending() -{ - // Conceptually this is a map, but doing a linear search through a vector is known - // to be faster for small number of elements. Since the number of tables expected - // to be processed here is assumed to be small < 20, use linear search instead of - // hashing. N is the depth to which embedded objects are connected and the upper - // bound is the total number of tables which is finite, and is usually small. - util::FlatMap converters; - - while (!embedded_pending.empty()) { - EmbeddedToCheck pending = embedded_pending.back(); - embedded_pending.pop_back(); - TableRef src_table = pending.embedded_in_src.get_table(); - TableRef dst_table = pending.embedded_in_dst.get_table(); - TableKey dst_table_key = dst_table->get_key(); - auto it_with_did_insert = - converters.insert({dst_table_key, InterRealmObjectConverter{src_table, dst_table, shared_from_this()}}); - InterRealmObjectConverter& converter = it_with_did_insert.first->second; - converter.copy(pending.embedded_in_src, pending.embedded_in_dst, nullptr); - } -} - -InterRealmValueConverter::InterRealmValueConverter(ConstTableRef src_table, ColKey src_col, ConstTableRef dst_table, - ColKey dst_col, std::shared_ptr ec) - : m_src_table(src_table) - , m_dst_table(dst_table) - , m_src_col(src_col) - , m_dst_col(dst_col) - , m_embedded_converter(ec) - , m_is_embedded_link(false) - , m_primitive_types_only(!(src_col.get_type() == col_type_TypedLink || src_col.get_type() == col_type_Link || - src_col.get_type() == col_type_LinkList || src_col.get_type() == col_type_Mixed)) -{ - if (!m_primitive_types_only) { - REALM_ASSERT(src_table); - m_opposite_of_src = src_table->get_opposite_table(src_col); - m_opposite_of_dst = dst_table->get_opposite_table(dst_col); - REALM_ASSERT(bool(m_opposite_of_src) == bool(m_opposite_of_dst)); - if (m_opposite_of_src) { - m_is_embedded_link = m_opposite_of_src->is_embedded(); - } - } -} - -void InterRealmValueConverter::track_new_embedded(Obj src, Obj dst) -{ - m_embedded_converter->track(src, dst); -} - -// convert `src` to the destination Realm and compare that value with `dst` -// If `converted_src_out` is provided, it will be set to the converted src value -int InterRealmValueConverter::cmp_src_to_dst(Mixed src, Mixed dst, ConversionResult* converted_src_out, - bool* did_update_out) -{ - int cmp = 0; - Mixed converted_src; - if (m_primitive_types_only || !src.is_type(type_Link, type_TypedLink)) { - converted_src = src; - cmp = src.compare(dst); - } - else if (m_opposite_of_src) { - ObjKey src_link_key = src.get(); - if (m_is_embedded_link) { - Obj src_embedded = m_opposite_of_src->get_object(src_link_key); - REALM_ASSERT_DEBUG(src_embedded.is_valid()); - if (dst.is_type(type_Link, type_TypedLink)) { - cmp = 0; // no need to set this link, there is already an embedded object here - Obj dst_embedded = m_opposite_of_dst->get_object(dst.get()); - REALM_ASSERT_DEBUG(dst_embedded.is_valid()); - converted_src = dst_embedded.get_key(); - track_new_embedded(src_embedded, dst_embedded); - } - else { - cmp = src.compare(dst); - if (converted_src_out) { - converted_src_out->requires_new_embedded_object = true; - converted_src_out->src_embedded_to_check = src_embedded; - } - } - } - else { - Mixed src_link_pk = m_opposite_of_src->get_primary_key(src_link_key); - Obj dst_link = m_opposite_of_dst->create_object_with_primary_key(src_link_pk, did_update_out); - converted_src = dst_link.get_key(); - if (dst.is_type(type_TypedLink)) { - cmp = converted_src.compare(dst.get()); - } - else { - cmp = converted_src.compare(dst); - } - } - } - else { - ObjLink src_link = src.get(); - if (src_link.is_unresolved()) { - converted_src = Mixed{}; // no need to transfer over unresolved links - cmp = converted_src.compare(dst); - } - else { - TableRef src_link_table = m_src_table->get_parent_group()->get_table(src_link.get_table_key()); - REALM_ASSERT_EX(src_link_table, src_link.get_table_key()); - TableRef dst_link_table = m_dst_table->get_parent_group()->get_table(src_link_table->get_name()); - REALM_ASSERT_EX(dst_link_table, src_link_table->get_name()); - // embedded tables should always be covered by the m_opposite_of_src case above. - REALM_ASSERT_EX(!src_link_table->is_embedded(), src_link_table->get_name()); - // regular table, convert by pk - Mixed src_pk = src_link_table->get_primary_key(src_link.get_obj_key()); - Obj dst_link = dst_link_table->create_object_with_primary_key(src_pk, did_update_out); - converted_src = ObjLink{dst_link_table->get_key(), dst_link.get_key()}; - cmp = converted_src.compare(dst); - } - } - if (converted_src_out) { - converted_src_out->converted_value = converted_src; - } - if (did_update_out && cmp) { - *did_update_out = true; - } - return cmp; -} - -InterRealmObjectConverter::InterRealmObjectConverter(ConstTableRef table_src, TableRef table_dst, - std::shared_ptr embedded_tracker) - : m_embedded_tracker(embedded_tracker) -{ - populate_columns_from_table(table_src, table_dst); -} - -void InterRealmObjectConverter::copy(const Obj& src, Obj& dst, bool* update_out) -{ - for (auto& column : m_columns_cache) { - column.copy_value(src, dst, update_out); - } -} - -void InterRealmObjectConverter::populate_columns_from_table(ConstTableRef table_src, ConstTableRef table_dst) -{ - m_columns_cache.clear(); - m_columns_cache.reserve(table_src->get_column_count()); - ColKey pk_col = table_src->get_primary_key_column(); - for (ColKey col_key_src : table_src->get_column_keys()) { - if (col_key_src == pk_col) - continue; - StringData col_name = table_src->get_column_name(col_key_src); - ColKey col_key_dst = table_dst->get_column_key(col_name); - REALM_ASSERT(col_key_dst); - m_columns_cache.emplace_back( - InterRealmValueConverter(table_src, col_key_src, table_dst, col_key_dst, m_embedded_tracker)); - } -} - -} // namespace realm::_impl::client_reset::converters - - namespace realm::_impl::client_reset { static inline bool should_skip_table(const Transaction& group, TableKey key) diff --git a/src/realm/sync/noinst/client_reset.hpp b/src/realm/sync/noinst/client_reset.hpp index b7b7c0783a5..8c5158ab8c0 100644 --- a/src/realm/sync/noinst/client_reset.hpp +++ b/src/realm/sync/noinst/client_reset.hpp @@ -90,65 +90,6 @@ LocalVersionIDs perform_client_reset_diff(DBRef db, DBRef db_remote, sync::Salte bool* did_recover_out, sync::SubscriptionStore* sub_store, util::UniqueFunction on_flx_version_complete); -namespace converters { - -struct EmbeddedObjectConverter : std::enable_shared_from_this { - void track(Obj e_src, Obj e_dst); - void process_pending(); - -private: - struct EmbeddedToCheck { - Obj embedded_in_src; - Obj embedded_in_dst; - }; - std::vector embedded_pending; -}; - -struct InterRealmValueConverter { - InterRealmValueConverter(ConstTableRef src_table, ColKey src_col, ConstTableRef dst_table, ColKey dst_col, - std::shared_ptr ec); - void track_new_embedded(Obj src, Obj dst); - struct ConversionResult { - Mixed converted_value; - bool requires_new_embedded_object = false; - Obj src_embedded_to_check; - }; - - // convert `src` to the destination Realm and compare that value with `dst` - // If `converted_src_out` is provided, it will be set to the converted src value - int cmp_src_to_dst(Mixed src, Mixed dst, ConversionResult* converted_src_out = nullptr, - bool* did_update_out = nullptr); - void copy_value(const Obj& src_obj, Obj& dst_obj, bool* update_out); - -private: - void copy_list(const Obj& src_obj, Obj& dst_obj, bool* update_out); - void copy_set(const Obj& src_obj, Obj& dst_obj, bool* update_out); - void copy_dictionary(const Obj& src_obj, Obj& dst_obj, bool* update_out); - - TableRef m_dst_link_table; - ConstTableRef m_src_table; - ConstTableRef m_dst_table; - ColKey m_src_col; - ColKey m_dst_col; - TableRef m_opposite_of_src; - TableRef m_opposite_of_dst; - std::shared_ptr m_embedded_converter; - bool m_is_embedded_link; - const bool m_primitive_types_only; -}; - -struct InterRealmObjectConverter { - InterRealmObjectConverter(ConstTableRef table_src, TableRef table_dst, - std::shared_ptr embedded_tracker); - void copy(const Obj& src, Obj& dst, bool* update_out); - -private: - void populate_columns_from_table(ConstTableRef table_src, ConstTableRef table_dst); - std::shared_ptr m_embedded_tracker; - std::vector m_columns_cache; -}; - -} // namespace converters } // namespace _impl::client_reset } // namespace realm diff --git a/src/realm/sync/noinst/client_reset_recovery.cpp b/src/realm/sync/noinst/client_reset_recovery.cpp index 8a6d8458d10..17386d53507 100644 --- a/src/realm/sync/noinst/client_reset_recovery.cpp +++ b/src/realm/sync/noinst/client_reset_recovery.cpp @@ -18,6 +18,7 @@ #include #include +#include #include #include @@ -454,7 +455,7 @@ void RecoverLocalChangesetsHandler::copy_lists_with_unrecoverable_changes() // final state which would be [B]. // IDEA: if a unique id were associated with each list element, we could recover lists correctly because // we would know where list elements ended up or if they were deleted by the server. - using namespace realm::_impl::client_reset::converters; + using namespace realm::converters; std::shared_ptr embedded_object_tracker = std::make_shared(); for (auto& it : m_lists) { if (!it.second.requires_manual_copy()) diff --git a/test/object-store/migrations.cpp b/test/object-store/migrations.cpp index 7889b1c048b..57e60ebe19c 100644 --- a/test/object-store/migrations.cpp +++ b/test/object-store/migrations.cpp @@ -1154,7 +1154,7 @@ TEST_CASE("migration: Automatic") { list.insert(0, Mixed{10}); list.insert(1, Mixed{10.10}); list.insert(2, Mixed{ObjLink{target_table->get_key(), target_object.get_key()}}); - list.insert(3, Mixed{target_object.get_key()}); + list.insert(3, Mixed{ObjLink{target_table->get_key(), target_object.get_key()}}); auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table"); auto child_object_key = child_object.get_key(); From f9169a9c7053d6f7bc5b77cdf4a79e860d13397b Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 25 Aug 2022 19:04:42 +0100 Subject: [PATCH 21/25] added support for mixedl links + basic test at core level for Obj::assign --- src/realm/obj.cpp | 6 ++ test/test_table.cpp | 182 +++++++++----------------------------------- 2 files changed, 42 insertions(+), 146 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 4349fbd1b5c..2094ec7a836 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1953,6 +1953,12 @@ void Obj::assign(const Obj& other) REALM_ASSERT(n != realm::npos); linking_obj_list.set(n, get_key()); } + else if (c.get_attrs().test(col_attr_List)) { + auto linking_obj_mixed_links = linking_obj.get_listbase_ptr(c); + auto n = linking_obj_mixed_links->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); + REALM_ASSERT(n != realm::npos); + linking_obj_mixed_links->set_any(n, ObjLink{other.get_table()->get_key(), other.get_key()}); + } else if (c.get_attrs().test(col_attr_Set)) { auto linking_obj_set = linking_obj.get_setbase_ptr(c); auto pos = linking_obj_set->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); diff --git a/test/test_table.cpp b/test/test_table.cpp index 9117fae0f64..649f5935a04 100644 --- a/test/test_table.cpp +++ b/test/test_table.cpp @@ -4104,152 +4104,6 @@ TEST(Table_4) } // Very basic sanity check of search index when you add, remove and set objects -TEST(Table_SearchIndexFindFirst) -{ - Table table; - - auto c1 = table.add_column(type_Int, "a"); - auto c2 = table.add_column(type_Int, "b", true); - auto c3 = table.add_column(type_String, "c"); - auto c4 = table.add_column(type_String, "d", true); - auto c5 = table.add_column(type_Bool, "e"); - auto c6 = table.add_column(type_Bool, "f", true); - auto c7 = table.add_column(type_Timestamp, "g"); - auto c8 = table.add_column(type_Timestamp, "h", true); - - Obj o0 = table.create_object(); - Obj o1 = table.create_object(); - Obj o2 = table.create_object(); - Obj o3 = table.create_object(); - - o0.set_all(100, 100, "100", "100", false, false, Timestamp(100, 100), Timestamp(100, 100)); - o1.set_all(200, 200, "200", "200", true, true, Timestamp(200, 200), Timestamp(200, 200)); - o2.set_all(200, 200, "200", "200", true, true, Timestamp(200, 200), Timestamp(200, 200)); - CHECK(o3.is_null(c2)); - CHECK(o3.is_null(c4)); - CHECK(o3.is_null(c6)); - CHECK(o3.is_null(c8)); - - table.add_search_index(c1); - table.add_search_index(c2); - table.add_search_index(c3); - table.add_search_index(c4); - table.add_search_index(c5); - table.add_search_index(c6); - table.add_search_index(c7); - table.add_search_index(c8); - - // Non-nullable integers - CHECK_EQUAL(table.find_first_int(c1, 100), o0.get_key()); - CHECK_EQUAL(table.find_first_int(c1, 200), o1.get_key()); - // Uninitialized non-nullable integers equal 0 - CHECK_EQUAL(table.find_first_int(c1, 0), o3.get_key()); - - // Nullable integers - CHECK_EQUAL(table.find_first_int(c2, 100), o0.get_key()); - CHECK_EQUAL(table.find_first_int(c2, 200), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(1), o3.get_key()); - - // Non-nullable strings - CHECK_EQUAL(table.find_first_string(c3, "100"), o0.get_key()); - CHECK_EQUAL(table.find_first_string(c3, "200"), o1.get_key()); - // Uninitialized non-nullable strings equal "" - CHECK_EQUAL(table.find_first_string(c3, ""), o3.get_key()); - - // Nullable strings - CHECK_EQUAL(table.find_first_string(c4, "100"), o0.get_key()); - CHECK_EQUAL(table.find_first_string(c4, "200"), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(3), o3.get_key()); - - // Non-nullable bools - CHECK_EQUAL(table.find_first_bool(c5, false), o0.get_key()); - CHECK_EQUAL(table.find_first_bool(c5, true), o1.get_key()); - - // Nullable bools - CHECK_EQUAL(table.find_first_bool(c6, false), o0.get_key()); - CHECK_EQUAL(table.find_first_bool(c6, true), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(5), o3.get_key()); - - // Non-nullable Timestamp - CHECK_EQUAL(table.find_first_timestamp(c7, Timestamp(100, 100)), o0.get_key()); - CHECK_EQUAL(table.find_first_timestamp(c7, Timestamp(200, 200)), o1.get_key()); - - // Nullable Timestamp - CHECK_EQUAL(table.find_first_timestamp(c8, Timestamp(100, 100)), o0.get_key()); - CHECK_EQUAL(table.find_first_timestamp(c8, Timestamp(200, 200)), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(7), o3.get_key()); - - // Remove object and see if things still work - // ******************************************************************************* - table.remove_object(o0.get_key()); - - // Integers - CHECK_EQUAL(table.find_first_int(c1, 100), null_key); - CHECK_EQUAL(table.find_first_int(c1, 200), o1.get_key()); - // Uninitialized non-nullable integers equal 0 - CHECK_EQUAL(table.find_first_int(c1, 0), o3.get_key()); - - CHECK_EQUAL(table.find_first_int(c2, 200), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(1), o3.get_key()); - - // Non-nullable strings - CHECK_EQUAL(table.find_first_string(c3, "100"), null_key); - CHECK_EQUAL(table.find_first_string(c3, "200"), o1.get_key()); - // Uninitialized non-nullable strings equal "" - CHECK_EQUAL(table.find_first_string(c3, ""), o3.get_key()); - - // Nullable strings - CHECK_EQUAL(table.find_first_string(c4, "100"), null_key); - CHECK_EQUAL(table.find_first_string(c4, "200"), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(3), o3.get_key()); - - // Non-nullable bools - // default value for non-nullable bool is false, so o3 is a match - CHECK_EQUAL(table.find_first_bool(c5, false), o3.get_key()); - CHECK_EQUAL(table.find_first_bool(c5, true), o1.get_key()); - - // Nullable bools - CHECK_EQUAL(table.find_first_bool(c6, false), null_key); - CHECK_EQUAL(table.find_first_bool(c6, true), o1.get_key()); - - // Call "set" and see if things still work - // ******************************************************************************* - o1.set_all(500, 500, "500", "500"); - o2.set_all(600, 600, "600", "600"); - - CHECK_EQUAL(table.find_first_int(c1, 500), o1.get_key()); - CHECK_EQUAL(table.find_first_int(c1, 600), o2.get_key()); - // Uninitialized non-nullable integers equal 0 - CHECK_EQUAL(table.find_first_int(c1, 0), o3.get_key()); - CHECK_EQUAL(table.find_first_int(c2, 500), o1.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(1), o3.get_key()); - - // Non-nullable strings - CHECK_EQUAL(table.find_first_string(c3, "500"), o1.get_key()); - CHECK_EQUAL(table.find_first_string(c3, "600"), o2.get_key()); - // Uninitialized non-nullable strings equal "" - CHECK_EQUAL(table.find_first_string(c3, ""), o3.get_key()); - - // Nullable strings - CHECK_EQUAL(table.find_first_string(c4, "500"), o1.get_key()); - CHECK_EQUAL(table.find_first_string(c4, "600"), o2.get_key()); - // FIXME: Waiting for fix outside scope of search index PR - // CHECK_EQUAL(table.find_first_null(3), o3.get_key()); - - // Remove four of the indexes through remove_search_index() call. Let other four remain to see - // if they leak memory when Table goes out of scope (needs leak detector) - table.remove_search_index(c1); - table.remove_search_index(c2); - table.remove_search_index(c3); - table.remove_search_index(c4); -} TEST(Table_SearchIndexFindAll) { @@ -5904,4 +5758,40 @@ TEST(Table_AsymmetricObjects) tr->commit(); } +TEST(Table_Assign) +{ + + Timestamp now{std::chrono::system_clock::now()}; + Group g; + + auto test_table = g.add_table("test"); + auto other_table = g.add_table("other"); + + auto col_test_int = test_table->add_column(type_Int, "int"); + auto col_test_string = test_table->add_column(type_String, "string"); + auto col_test_double = test_table->add_column(type_Double, "double"); + auto col_test_mixed = test_table->add_column(type_Mixed, "any"); + auto col_test_link = test_table->add_column(*other_table, "link"); + + auto col_other_int = other_table->add_column(type_Int, "other_int"); + + auto other_obj = other_table->create_object(); + other_obj.set(col_other_int, 5); + + auto test_obj1 = test_table->create_object(); + test_obj1.set(col_test_int, 10); + test_obj1.set(col_test_string, "string"); + test_obj1.set(col_test_double, 10.10); + test_obj1.set(col_test_mixed, Mixed("Hello")); + test_obj1.set(col_test_link, other_obj.get_key()); + CHECK(other_obj.get_backlink_count() == 1); + + auto test_obj2 = test_table->create_object(); + test_obj2.assign(test_obj1); + CHECK(other_obj.get_backlink_count() == 2); + for (auto col : test_table->get_column_keys()) { + CHECK(test_obj2.get_any(col) == test_obj1.get_any(col)); + } +} + #endif // TEST_TABLE From a12d3b39e9cf52ee48b4a6374bd5f1d77d6eb38f Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 25 Aug 2022 19:06:51 +0100 Subject: [PATCH 22/25] Revert "added support for mixedl links + basic test at core level for Obj::assign" This reverts commit f9169a9c7053d6f7bc5b77cdf4a79e860d13397b. --- src/realm/obj.cpp | 6 -- test/test_table.cpp | 182 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 146 insertions(+), 42 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 2094ec7a836..4349fbd1b5c 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1953,12 +1953,6 @@ void Obj::assign(const Obj& other) REALM_ASSERT(n != realm::npos); linking_obj_list.set(n, get_key()); } - else if (c.get_attrs().test(col_attr_List)) { - auto linking_obj_mixed_links = linking_obj.get_listbase_ptr(c); - auto n = linking_obj_mixed_links->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); - REALM_ASSERT(n != realm::npos); - linking_obj_mixed_links->set_any(n, ObjLink{other.get_table()->get_key(), other.get_key()}); - } else if (c.get_attrs().test(col_attr_Set)) { auto linking_obj_set = linking_obj.get_setbase_ptr(c); auto pos = linking_obj_set->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); diff --git a/test/test_table.cpp b/test/test_table.cpp index 649f5935a04..9117fae0f64 100644 --- a/test/test_table.cpp +++ b/test/test_table.cpp @@ -4104,6 +4104,152 @@ TEST(Table_4) } // Very basic sanity check of search index when you add, remove and set objects +TEST(Table_SearchIndexFindFirst) +{ + Table table; + + auto c1 = table.add_column(type_Int, "a"); + auto c2 = table.add_column(type_Int, "b", true); + auto c3 = table.add_column(type_String, "c"); + auto c4 = table.add_column(type_String, "d", true); + auto c5 = table.add_column(type_Bool, "e"); + auto c6 = table.add_column(type_Bool, "f", true); + auto c7 = table.add_column(type_Timestamp, "g"); + auto c8 = table.add_column(type_Timestamp, "h", true); + + Obj o0 = table.create_object(); + Obj o1 = table.create_object(); + Obj o2 = table.create_object(); + Obj o3 = table.create_object(); + + o0.set_all(100, 100, "100", "100", false, false, Timestamp(100, 100), Timestamp(100, 100)); + o1.set_all(200, 200, "200", "200", true, true, Timestamp(200, 200), Timestamp(200, 200)); + o2.set_all(200, 200, "200", "200", true, true, Timestamp(200, 200), Timestamp(200, 200)); + CHECK(o3.is_null(c2)); + CHECK(o3.is_null(c4)); + CHECK(o3.is_null(c6)); + CHECK(o3.is_null(c8)); + + table.add_search_index(c1); + table.add_search_index(c2); + table.add_search_index(c3); + table.add_search_index(c4); + table.add_search_index(c5); + table.add_search_index(c6); + table.add_search_index(c7); + table.add_search_index(c8); + + // Non-nullable integers + CHECK_EQUAL(table.find_first_int(c1, 100), o0.get_key()); + CHECK_EQUAL(table.find_first_int(c1, 200), o1.get_key()); + // Uninitialized non-nullable integers equal 0 + CHECK_EQUAL(table.find_first_int(c1, 0), o3.get_key()); + + // Nullable integers + CHECK_EQUAL(table.find_first_int(c2, 100), o0.get_key()); + CHECK_EQUAL(table.find_first_int(c2, 200), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(1), o3.get_key()); + + // Non-nullable strings + CHECK_EQUAL(table.find_first_string(c3, "100"), o0.get_key()); + CHECK_EQUAL(table.find_first_string(c3, "200"), o1.get_key()); + // Uninitialized non-nullable strings equal "" + CHECK_EQUAL(table.find_first_string(c3, ""), o3.get_key()); + + // Nullable strings + CHECK_EQUAL(table.find_first_string(c4, "100"), o0.get_key()); + CHECK_EQUAL(table.find_first_string(c4, "200"), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(3), o3.get_key()); + + // Non-nullable bools + CHECK_EQUAL(table.find_first_bool(c5, false), o0.get_key()); + CHECK_EQUAL(table.find_first_bool(c5, true), o1.get_key()); + + // Nullable bools + CHECK_EQUAL(table.find_first_bool(c6, false), o0.get_key()); + CHECK_EQUAL(table.find_first_bool(c6, true), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(5), o3.get_key()); + + // Non-nullable Timestamp + CHECK_EQUAL(table.find_first_timestamp(c7, Timestamp(100, 100)), o0.get_key()); + CHECK_EQUAL(table.find_first_timestamp(c7, Timestamp(200, 200)), o1.get_key()); + + // Nullable Timestamp + CHECK_EQUAL(table.find_first_timestamp(c8, Timestamp(100, 100)), o0.get_key()); + CHECK_EQUAL(table.find_first_timestamp(c8, Timestamp(200, 200)), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(7), o3.get_key()); + + // Remove object and see if things still work + // ******************************************************************************* + table.remove_object(o0.get_key()); + + // Integers + CHECK_EQUAL(table.find_first_int(c1, 100), null_key); + CHECK_EQUAL(table.find_first_int(c1, 200), o1.get_key()); + // Uninitialized non-nullable integers equal 0 + CHECK_EQUAL(table.find_first_int(c1, 0), o3.get_key()); + + CHECK_EQUAL(table.find_first_int(c2, 200), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(1), o3.get_key()); + + // Non-nullable strings + CHECK_EQUAL(table.find_first_string(c3, "100"), null_key); + CHECK_EQUAL(table.find_first_string(c3, "200"), o1.get_key()); + // Uninitialized non-nullable strings equal "" + CHECK_EQUAL(table.find_first_string(c3, ""), o3.get_key()); + + // Nullable strings + CHECK_EQUAL(table.find_first_string(c4, "100"), null_key); + CHECK_EQUAL(table.find_first_string(c4, "200"), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(3), o3.get_key()); + + // Non-nullable bools + // default value for non-nullable bool is false, so o3 is a match + CHECK_EQUAL(table.find_first_bool(c5, false), o3.get_key()); + CHECK_EQUAL(table.find_first_bool(c5, true), o1.get_key()); + + // Nullable bools + CHECK_EQUAL(table.find_first_bool(c6, false), null_key); + CHECK_EQUAL(table.find_first_bool(c6, true), o1.get_key()); + + // Call "set" and see if things still work + // ******************************************************************************* + o1.set_all(500, 500, "500", "500"); + o2.set_all(600, 600, "600", "600"); + + CHECK_EQUAL(table.find_first_int(c1, 500), o1.get_key()); + CHECK_EQUAL(table.find_first_int(c1, 600), o2.get_key()); + // Uninitialized non-nullable integers equal 0 + CHECK_EQUAL(table.find_first_int(c1, 0), o3.get_key()); + CHECK_EQUAL(table.find_first_int(c2, 500), o1.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(1), o3.get_key()); + + // Non-nullable strings + CHECK_EQUAL(table.find_first_string(c3, "500"), o1.get_key()); + CHECK_EQUAL(table.find_first_string(c3, "600"), o2.get_key()); + // Uninitialized non-nullable strings equal "" + CHECK_EQUAL(table.find_first_string(c3, ""), o3.get_key()); + + // Nullable strings + CHECK_EQUAL(table.find_first_string(c4, "500"), o1.get_key()); + CHECK_EQUAL(table.find_first_string(c4, "600"), o2.get_key()); + // FIXME: Waiting for fix outside scope of search index PR + // CHECK_EQUAL(table.find_first_null(3), o3.get_key()); + + // Remove four of the indexes through remove_search_index() call. Let other four remain to see + // if they leak memory when Table goes out of scope (needs leak detector) + table.remove_search_index(c1); + table.remove_search_index(c2); + table.remove_search_index(c3); + table.remove_search_index(c4); +} TEST(Table_SearchIndexFindAll) { @@ -5758,40 +5904,4 @@ TEST(Table_AsymmetricObjects) tr->commit(); } -TEST(Table_Assign) -{ - - Timestamp now{std::chrono::system_clock::now()}; - Group g; - - auto test_table = g.add_table("test"); - auto other_table = g.add_table("other"); - - auto col_test_int = test_table->add_column(type_Int, "int"); - auto col_test_string = test_table->add_column(type_String, "string"); - auto col_test_double = test_table->add_column(type_Double, "double"); - auto col_test_mixed = test_table->add_column(type_Mixed, "any"); - auto col_test_link = test_table->add_column(*other_table, "link"); - - auto col_other_int = other_table->add_column(type_Int, "other_int"); - - auto other_obj = other_table->create_object(); - other_obj.set(col_other_int, 5); - - auto test_obj1 = test_table->create_object(); - test_obj1.set(col_test_int, 10); - test_obj1.set(col_test_string, "string"); - test_obj1.set(col_test_double, 10.10); - test_obj1.set(col_test_mixed, Mixed("Hello")); - test_obj1.set(col_test_link, other_obj.get_key()); - CHECK(other_obj.get_backlink_count() == 1); - - auto test_obj2 = test_table->create_object(); - test_obj2.assign(test_obj1); - CHECK(other_obj.get_backlink_count() == 2); - for (auto col : test_table->get_column_keys()) { - CHECK(test_obj2.get_any(col) == test_obj1.get_any(col)); - } -} - #endif // TEST_TABLE From 7bdf666e86dea98d0d08f0148547958ab511f0a4 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Thu, 25 Aug 2022 19:10:24 +0100 Subject: [PATCH 23/25] added support for mixed links + test at core level for Obj::assign --- src/realm/obj.cpp | 6 ++++++ test/test_table.cpp | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 4349fbd1b5c..41c7e1a51cd 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1953,6 +1953,12 @@ void Obj::assign(const Obj& other) REALM_ASSERT(n != realm::npos); linking_obj_list.set(n, get_key()); } + else if (c.get_attrs().test(col_attr_List)) { + auto linking_obj_list = linking_obj.get_listbase_ptr(c); + auto pos = linking_obj_list->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); + REALM_ASSERT(pos != realm::npos); + linking_obj_list->set_any(pos, ObjLink{m_table->get_key(), get_key()}); + } else if (c.get_attrs().test(col_attr_Set)) { auto linking_obj_set = linking_obj.get_setbase_ptr(c); auto pos = linking_obj_set->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); diff --git a/test/test_table.cpp b/test/test_table.cpp index 9117fae0f64..160c8bc9b64 100644 --- a/test/test_table.cpp +++ b/test/test_table.cpp @@ -5904,4 +5904,39 @@ TEST(Table_AsymmetricObjects) tr->commit(); } +TEST(Table_Assign) +{ + Timestamp now{std::chrono::system_clock::now()}; + Group g; + + auto test_table = g.add_table("test"); + auto other_table = g.add_table("other"); + + auto col_test_int = test_table->add_column(type_Int, "int"); + auto col_test_string = test_table->add_column(type_String, "string"); + auto col_test_double = test_table->add_column(type_Double, "double"); + auto col_test_mixed = test_table->add_column(type_Mixed, "any"); + auto col_test_link = test_table->add_column(*other_table, "link"); + + auto col_other_int = other_table->add_column(type_Int, "other_int"); + + auto other_obj = other_table->create_object(); + other_obj.set(col_other_int, 5); + + auto test_obj1 = test_table->create_object(); + test_obj1.set(col_test_int, 10); + test_obj1.set(col_test_string, "string"); + test_obj1.set(col_test_double, 10.10); + test_obj1.set(col_test_mixed, Mixed("Hello")); + test_obj1.set(col_test_link, other_obj.get_key()); + CHECK(other_obj.get_backlink_count() == 1); + + auto test_obj2 = test_table->create_object(); + test_obj2.assign(test_obj1); + CHECK(other_obj.get_backlink_count() == 2); + for (auto col : test_table->get_column_keys()) { + CHECK(test_obj2.get_any(col) == test_obj1.get_any(col)); + } +} + #endif // TEST_TABLE From 809f4f75efcef65d4c8fec06680b724e69e45404 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 26 Aug 2022 15:16:13 +0100 Subject: [PATCH 24/25] remove Obj::assign --- src/realm/obj.cpp | 53 --------------------------------------------- src/realm/obj.hpp | 6 ----- test/test_table.cpp | 35 ------------------------------ 3 files changed, 94 deletions(-) diff --git a/src/realm/obj.cpp b/src/realm/obj.cpp index 41c7e1a51cd..517e31beb9e 100644 --- a/src/realm/obj.cpp +++ b/src/realm/obj.cpp @@ -1934,59 +1934,6 @@ bool Obj::remove_backlink(ColKey col_key, ObjLink old_link, CascadeState& state) return false; } -void Obj::assign(const Obj& other) -{ - auto embedded_obj_tracker = std::make_shared(); - converters::InterRealmObjectConverter converter(this->get_table(), this->get_table(), embedded_obj_tracker); - converter.copy(other, *this, nullptr); - embedded_obj_tracker->process_pending(); - - auto copy_links = [this, &other](ColKey col) { - auto t = m_table->get_opposite_table(col); - auto c = m_table->get_opposite_column(col); - auto backlinks = other.get_all_backlinks(col); - for (auto bl : backlinks) { - auto linking_obj = t->get_object(bl); - if (c.get_type() == col_type_LinkList) { - auto linking_obj_list = linking_obj.get_linklist(c); - auto n = linking_obj_list.find_first(other.get_key()); - REALM_ASSERT(n != realm::npos); - linking_obj_list.set(n, get_key()); - } - else if (c.get_attrs().test(col_attr_List)) { - auto linking_obj_list = linking_obj.get_listbase_ptr(c); - auto pos = linking_obj_list->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); - REALM_ASSERT(pos != realm::npos); - linking_obj_list->set_any(pos, ObjLink{m_table->get_key(), get_key()}); - } - else if (c.get_attrs().test(col_attr_Set)) { - auto linking_obj_set = linking_obj.get_setbase_ptr(c); - auto pos = linking_obj_set->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); - REALM_ASSERT(pos != realm::npos); - linking_obj_set->insert_any(ObjLink{m_table->get_key(), get_key()}); - } - else if (c.get_attrs().test(col_attr_Dictionary)) { - auto linking_obj_dictionary = linking_obj.get_dictionary_ptr(c); - auto pos = linking_obj_dictionary->find_any(ObjLink{other.get_table()->get_key(), other.get_key()}); - REALM_ASSERT(pos != realm::npos); - Mixed key = linking_obj_dictionary->get_key(pos); - linking_obj_dictionary->insert(key, ObjLink{m_table->get_key(), get_key()}); - } - else if (c.get_type() == col_type_Mixed && linking_obj.get_any(c).get_type() == type_TypedLink) { - REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); - linking_obj.set_any(c, ObjLink{m_table->get_key(), get_key()}); - } - else if (c.get_type() == col_type_Link) { - // Single link - REALM_ASSERT(!linking_obj.get(c) || linking_obj.get(c) == other.get_key()); - linking_obj.set(c, get_key()); - } - } - return false; - }; - m_table->for_each_backlink_column(copy_links); -} - void Obj::handle_multiple_backlinks_during_schema_migration() { REALM_ASSERT(!m_table->get_primary_key_column()); diff --git a/src/realm/obj.hpp b/src/realm/obj.hpp index c4dfb252ab8..8839c80a138 100644 --- a/src/realm/obj.hpp +++ b/src/realm/obj.hpp @@ -260,12 +260,6 @@ class Obj { template Obj& set_all(Head v, Tail... tail); - // Assign the object passed as argument to the this object. - // This method performs a deep copy of all the properties of - // the other object passed. - // So this could potentially create a lot of user data duplication - // if not used correctly. - void assign(const Obj& other); // The main algorithm for handling schema migrations if we try to convert // from TopLevel* to Embedded, in this case all the orphan objects are deleted // and all the objects with multiple backlinks are cloned in order to avoid to diff --git a/test/test_table.cpp b/test/test_table.cpp index 160c8bc9b64..9117fae0f64 100644 --- a/test/test_table.cpp +++ b/test/test_table.cpp @@ -5904,39 +5904,4 @@ TEST(Table_AsymmetricObjects) tr->commit(); } -TEST(Table_Assign) -{ - Timestamp now{std::chrono::system_clock::now()}; - Group g; - - auto test_table = g.add_table("test"); - auto other_table = g.add_table("other"); - - auto col_test_int = test_table->add_column(type_Int, "int"); - auto col_test_string = test_table->add_column(type_String, "string"); - auto col_test_double = test_table->add_column(type_Double, "double"); - auto col_test_mixed = test_table->add_column(type_Mixed, "any"); - auto col_test_link = test_table->add_column(*other_table, "link"); - - auto col_other_int = other_table->add_column(type_Int, "other_int"); - - auto other_obj = other_table->create_object(); - other_obj.set(col_other_int, 5); - - auto test_obj1 = test_table->create_object(); - test_obj1.set(col_test_int, 10); - test_obj1.set(col_test_string, "string"); - test_obj1.set(col_test_double, 10.10); - test_obj1.set(col_test_mixed, Mixed("Hello")); - test_obj1.set(col_test_link, other_obj.get_key()); - CHECK(other_obj.get_backlink_count() == 1); - - auto test_obj2 = test_table->create_object(); - test_obj2.assign(test_obj1); - CHECK(other_obj.get_backlink_count() == 2); - for (auto col : test_table->get_column_keys()) { - CHECK(test_obj2.get_any(col) == test_obj1.get_any(col)); - } -} - #endif // TEST_TABLE From f26fd3edb2c1484fd0cbd4c430c009884b737e78 Mon Sep 17 00:00:00 2001 From: Nicola Cabiddu Date: Fri, 26 Aug 2022 15:25:15 +0100 Subject: [PATCH 25/25] remove installed header --- src/realm/CMakeLists.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/src/realm/CMakeLists.txt b/src/realm/CMakeLists.txt index 9f09a78892c..993ca38fc08 100644 --- a/src/realm/CMakeLists.txt +++ b/src/realm/CMakeLists.txt @@ -170,7 +170,6 @@ set(REALM_INSTALL_HEADERS null.hpp obj.hpp obj_list.hpp - object_converter.hpp object_id.hpp owned_data.hpp query.hpp