Fix backlink check in migration.

CHANGELOG.md

@@ -6,6 +6,7 @@
 ### Fixed
 * <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
 * Fixed property aliases not working in the parsed queries which use the `@links.Class.property` syntax. ([#4398](https://github.com/realm/realm-core/issues/4398), this never previously worked)
+* Fixed a bug that prevented an ObjectSchema with incoming links from being marked as embedded during migrations. ([#4414](https://github.com/realm/realm-core/pull/4414))
 
 ### Breaking changes
 * None.

src/realm/object-store/object_store.cpp

@@ -737,8 +737,7 @@ static void apply_pre_migration_changes(Group& group, std::vector<SchemaChange>
         }
         void operator()(RemoveTable) {}
         void operator()(ChangeTableType op)
-        {
-            set_embedded(table(op.object), op.object->is_embedded);
+        { /* delayed until after the migration */
         }
         void operator()(AddInitialProperties op)
         {
@@ -840,7 +839,10 @@ static void apply_post_migration_changes(Group& group, std::vector<SchemaChange>
             table(op.object).remove_search_index(op.property->column_key);
         }
 
-        void operator()(ChangeTableType) {}
+        void operator()(ChangeTableType op)
+        {
+            set_embedded(table(op.object), op.object->is_embedded);
+        }
         void operator()(RemoveTable) {}
         void operator()(ChangePropertyType) {}
         void operator()(MakePropertyNullable) {}

src/realm/table.cpp

@@ -1053,13 +1053,23 @@ bool Table::set_embedded(bool embedded)
         });
 
         if (has_backlink_columns) {
-            for (auto o : *this) {
-                // each object should be owned by one and only one parent
-                if (o.get_backlink_count() != 1) {
+            for (auto object : *this) {
+                if (object.get_backlink_count() == 0) {
+                    // If the object does not have any backlinks it can be deleted.
+                    object.remove();
+                }
+                else if (object.get_backlink_count() > 1) {
+                    // Each object should be owned by one parent at most.
                     return false;
                 }
             }
         }
+        else {
+            // If the table does not have any backlink columns the objects can be deleted.
+            for (auto object : *this) {
+                object.remove();
+            }
+        }
     }
 
     do_set_embedded(embedded);

test/object-store/migrations.cpp

@@ -434,7 +434,7 @@ TEST_CASE("migration: Automatic") {
             REQUIRE_UPDATE_SUCCEEDS(*realm, schema2, 1);
         }
 
-        SECTION("change table from embedded to top-level") {
+        SECTION("change table from embedded to top-level without version bump") {
             auto realm = Realm::get_shared_realm(config);
 
             Schema schema = {
@@ -448,7 +448,7 @@ TEST_CASE("migration: Automatic") {
             REQUIRE_MIGRATION_NEEDED(*realm, schema, set_embedded(schema, "object", false));
         }
 
-        SECTION("change table from top-level to embedded") {
+        SECTION("change table from top-level to embedded without version bump") {
             auto realm = Realm::get_shared_realm(config);
 
             Schema schema = {
@@ -601,6 +601,238 @@ TEST_CASE("migration: Automatic") {
 
             REQUIRE_THROWS(realm->update_schema(set_embedded(schema, "target", true), 2, nullptr));
         }
+
+        SECTION("change empty table from top-level to embedded") {
+            Schema schema = {
+                {"object",
+                 {
+                     {"value", PropertyType::Int},
+                 }},
+            };
+            auto realm = Realm::get_shared_realm(config);
+
+            REQUIRE_NOTHROW(realm->update_schema(set_embedded(schema, "object", true), 1, nullptr));
+
+            REQUIRE(realm->schema_version() == 1);
+        }
+
+        SECTION("change empty table from embedded to top-level") {
+            Schema schema = {
+                {"object",
+                 ObjectSchema::IsEmbedded{true},
+                 {
+                     {"value", PropertyType::Int},
+                 }},
+            };
+            auto realm = Realm::get_shared_realm(config);
+
+            REQUIRE_NOTHROW(realm->update_schema(set_embedded(schema, "object", false), 1, nullptr));
+
+            REQUIRE(realm->schema_version() == 1);
+        }
+
+        SECTION("change table to embedded - no migration block") {
+            Schema schema = {
+                {"object",
+                 {
+                     {"value", PropertyType::Int},
+                 }},
+            };
+            auto realm = Realm::get_shared_realm(config);
+            realm->update_schema(schema, 1);
+            realm->begin_transaction();
+            auto table = ObjectStore::table_for_object_type(realm->read_group(), "object");
+            table->create_object();
+            realm->commit_transaction();
+            REQUIRE(table->size() == 1);
+
+            REQUIRE_NOTHROW(realm->update_schema(set_embedded(schema, "object", true), 2, nullptr));
+
+            REQUIRE(realm->schema_version() == 2);
+            REQUIRE(table->size() == 0);
+        }
+
+        SECTION("change table to embedded - objects without incoming links get deleted") {
+            Schema schema = {
+                {"object",
+                 {
+                     {"value", PropertyType::Int},
+                 }},
+            };
+            auto realm = Realm::get_shared_realm(config);
+            realm->update_schema(schema, 1);
+            realm->begin_transaction();
+            auto table = ObjectStore::table_for_object_type(realm->read_group(), "object");
+            create_objects(*table, 10);
+            realm->commit_transaction();
+            REQUIRE(table->size() == 10);
+            auto migration_called = false;
+
+            REQUIRE_NOTHROW(realm->update_schema(set_embedded(schema, "object", true), 2,
+                                                 [&migration_called](auto old_realm, auto new_realm, auto&) {
+                                                     migration_called = true;
+                                                 }));
+
+            REQUIRE(migration_called);
+            REQUIRE(realm->schema_version() == 2);
+            REQUIRE(table->size() == 0);
+        }
+
+        SECTION("change table to embedded - one incoming link per object") {
+            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_object1 = child_table->create_object();
+            child_object1.set("value", 42);
+            Obj child_object2 = child_table->create_object();
+            child_object2.set("value", 43);
+            auto parent_table = ObjectStore::table_for_object_type(realm->read_group(), "parent_table");
+            auto child_object_key1 = child_object1.get_key();
+            auto child_object_key2 = child_object2.get_key();
+            parent_table->create_object().set_all(child_object_key1);
+            parent_table->create_object().set_all(child_object_key2);
+            realm->commit_transaction();
+            REQUIRE(parent_table->size() == 2);
+            REQUIRE(child_table->size() == 2);
+
+            REQUIRE_NOTHROW(realm->update_schema(set_embedded(schema, "child_table", true), 2, nullptr));
+
+            REQUIRE(realm->schema_version() == 2);
+            REQUIRE(parent_table->size() == 2);
+            REQUIRE(child_table->size() == 2);
+            for (int i = 0; i < 2; i++) {
+                Object parent_object(realm, "parent_table", i);
+                CppContext context(realm);
+                Object child_object =
+                    any_cast<Object>(parent_object.get_property_value<util::Any>(context, "child_property"));
+                int value = any_cast<Int>(child_object.get_property_value<util::Any>(context, "value"));
+                REQUIRE(value == 42 + i);
+            }
+        }
+
+        SECTION("change table to embedded - multiple incoming link per object") {
+            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_THROWS(realm->update_schema(set_embedded(schema, "child_table", true), 2, nullptr));
+        }
+
+        SECTION("change table to embedded - multiple incoming links - resolved in migration block") {
+            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_NOTHROW(realm->update_schema(
+                set_embedded(schema, "child_table", true), 2, [](auto old_realm, auto new_realm, auto&) {
+                    for (int i = 0; i < 2; i++) {
+                        Obj child_obj = get_table(new_realm, "child_table")->create_object();
+                        child_obj.set("value", 43);
+                        Object child_object(new_realm, child_obj);
+                        REQUIRE(child_object.is_valid());
+                        Object parent_object(new_realm, "parent_table", i);
+                        CppContext context(new_realm);
+                        parent_object.set_property_value(context, "child_property", util::Any(child_object));
+                    }
+                }));
+
+            REQUIRE(realm->schema_version() == 2);
+            REQUIRE(parent_table->size() == 2);
+            REQUIRE(child_table->size() == 2);
+            for (int i = 0; i < 2; i++) {
+                Object parent_object(realm, "parent_table", i);
+                CppContext context(realm);
+                Object child_object =
+                    any_cast<Object>(parent_object.get_property_value<util::Any>(context, "child_property"));
+                int value = any_cast<Int>(child_object.get_property_value<util::Any>(context, "value"));
+                REQUIRE(value == 43);
+            }
+        }
+
+        SECTION("change table to embedded - adding more links in migration block") {
+            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();
+            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);
+            realm->commit_transaction();
+            REQUIRE(parent_table->size() == 1);
+            REQUIRE(child_table->size() == 1);
+
+            REQUIRE_THROWS(realm->update_schema(
+                set_embedded(schema, "child_table", true), 2, [](auto old_realm, auto new_realm, auto&) {
+                    Object child_object(new_realm, "child_table", 0);
+                    auto parent_table = ObjectStore::table_for_object_type(new_realm->read_group(), "parent_table");
+                    Obj parent_obj = parent_table->create_object();
+                    Object parent_object(new_realm, parent_obj);
+                    CppContext context(new_realm);
+                    parent_object.set_property_value(context, "child_property", util::Any(child_object));
+                }));
+        }
     }
 
     SECTION("valid migrations") {