Fix backlink check in migration. (#4414)

Setting tables to embedded had to be moved to post migration changes.

CHANGELOG.md

@@ -5,7 +5,7 @@
 
 ### Fixed
 * <How to hit and notice issue? what was the impact?> ([#????](https://github.com/realm/realm-core/issues/????), since v?.?.?)
-* None.
+* 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

@@ -563,16 +563,6 @@ static void apply_non_migration_changes(Group& group, std::vector<SchemaChange>
     verify_no_errors<SchemaMismatchException>(applier, changes);
 }
 
-static void set_embedded(Table& table, ObjectSchema::IsEmbedded is_embedded)
-{
-    if (!table.set_embedded(is_embedded) && is_embedded) {
-        auto msg =
-            util::format("Cannot convert object type '%1' to embedded because objects have multiple incoming links.",
-                         ObjectStore::object_type_for_table_name(table.get_name()));
-        throw std::logic_error(std::move(msg));
-    }
-}
-
 static void set_primary_key(Table& table, const Property* property)
 {
     ColKey col;
@@ -612,7 +602,7 @@ static void create_initial_tables(Group& group, std::vector<SchemaChange> const&
         // downside.
         void operator()(ChangeTableType op)
         {
-            set_embedded(table(op.object), op.object->is_embedded);
+            table(op.object).set_embedded(op.object->is_embedded);
         }
         void operator()(AddProperty op)
         {
@@ -736,9 +726,8 @@ static void apply_pre_migration_changes(Group& group, std::vector<SchemaChange>
             create_table(group, *op.object);
         }
         void operator()(RemoveTable) {}
-        void operator()(ChangeTableType op)
-        {
-            set_embedded(table(op.object), op.object->is_embedded);
+        void operator()(ChangeTableType)
+        { /* delayed until after the migration */
         }
         void operator()(AddInitialProperties op)
         {
@@ -840,7 +829,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)
+        {
+            table(op.object).set_embedded(op.object->is_embedded);
+        }
         void operator()(RemoveTable) {}
         void operator()(ChangePropertyType) {}
         void operator()(MakePropertyNullable) {}

src/realm/table.cpp

@@ -1029,42 +1029,51 @@ void Table::do_erase_root_column(ColKey col_key)
     bump_storage_version();
 }
 
-bool Table::set_embedded(bool embedded)
+void Table::set_embedded(bool embedded)
 {
-    if (embedded == m_is_embedded)
-        return true;
+    if (embedded == m_is_embedded) {
+        return;
+    }
 
     if (Replication* repl = get_repl()) {
         if (repl->get_history_type() == Replication::HistoryType::hist_SyncClient) {
-            throw std::logic_error("Cannot change embedded property in sync client");
+            throw std::logic_error("Cannot change table to embedded when using Sync.");
         }
     }
 
+    if (embedded == false) {
+        do_set_embedded(false);
+        return;
+    }
+
+    // Embedded objects cannot have a primary key.
     if (get_primary_key_column()) {
-        return false;
+        throw std::logic_error("Cannot change table to embedded when using a primary key.");
     }
-    if (size() > 0) {
-        // Check if the table has any backlink columns. If not, it is not required
-        // to check all objects for backlinks.
-        bool has_backlink_columns = false;
-        for_each_backlink_column([&has_backlink_columns](ColKey) {
-            has_backlink_columns = true;
-            return true; // Done
-        });
 
-        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) {
-                    return false;
-                }
+    // `has_backlink_columns` indicates if the table is embedded in any other table.
+    bool has_backlink_columns = false;
+    for_each_backlink_column([&has_backlink_columns](ColKey) {
+        has_backlink_columns = true;
+        return true;
+    });
+    if (!has_backlink_columns) {
+        throw std::logic_error("Cannot change table to embedded without backlink columns. Table must be embedded in "
+                               "at least one other table.");
+    }
+    else if (size() > 0) {
+        for (auto object : *this) {
+            size_t backlink_count = object.get_backlink_count();
+            if (backlink_count == 0) {
+                throw std::logic_error("At least one object does not have a backlink (data would get lost).");
+            }
+            else if (backlink_count > 1) {
+                throw std::logic_error("At least one object does have multiple backlinks.");
             }
         }
     }
 
     do_set_embedded(embedded);
-
-    return true;
 }
 
 void Table::do_set_embedded(bool embedded)

src/realm/table.hpp

@@ -161,9 +161,8 @@ class Table {
     bool valid_column(ColKey col_key) const noexcept;
     void check_column(ColKey col_key) const;
     // Change the embedded property of a table. If switching to being embedded, the table must
-    // not have a primary key and all objects must have exactly 1 backlink. Return value
-    // indicates if the conversion was done
-    bool set_embedded(bool embedded);
+    // not have a primary key and all objects must have exactly 1 backlink.
+    void set_embedded(bool embedded);
     //@}
 
     /// True for `col_type_Link` and `col_type_LinkList`.