Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix backlink check in migration. #4414

Merged
merged 23 commits into from
Feb 17, 2021
Merged

Fix backlink check in migration. #4414

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
1641562
Fix backlink check in migration.
Feb 10, 2021
41ebbf5
Update CHANGELOG.
Feb 10, 2021
cc4442c
Apply clang-format patch.
Feb 10, 2021
ead8022
Update CHANGELOG.
Feb 10, 2021
76c9653
Rework embeddednes checks based on PR comments.
Feb 12, 2021
eace01d
Fix some table names.
Feb 12, 2021
81462c7
clang-format
Feb 15, 2021
411a76d
clang-format
Feb 15, 2021
7d780a2
Check sync before setting to false.
Feb 15, 2021
8f37ec9
Check if the table is correctly set to embedded after the migration.
Feb 15, 2021
7f06af5
Remove return value since function does only throw or succeed.
Feb 15, 2021
0cc1739
Merge remote-tracking branch 'origin/master' into df/fix-backlink-che…
Feb 15, 2021
a800c07
Fix checks in Group_ChangeEmbeddedness.
Feb 15, 2021
8e6c0cd
Remove unused parameter.
Feb 15, 2021
6ca0566
Use size_t to not loose precision.
Feb 15, 2021
fdedf30
Use REQUIRE_THROWS instead of CHECK_THROWS.
Feb 15, 2021
60d3b05
Remove some unused parameters.
Feb 15, 2021
48de099
Use long instead of int.
Feb 15, 2021
1f031df
Use CHECK_THROW.
Feb 15, 2021
3a1ba7a
Use CHECK_THROW_ANY_GET_MESSAGE.
Feb 15, 2021
f12eea4
Int.
Feb 15, 2021
51f37e7
Int instead of int.
Feb 16, 2021
2dbdf4d
Merge branch 'master' into df/fix-backlink-check-in-migration
jedelbo Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/realm/object-store/object_store.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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) {}
Expand Down
14 changes: 11 additions & 3 deletions src/realm/table.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,12 +1053,20 @@ bool Table::set_embedded(bool embedded)
});

DominicFrei marked this conversation as resolved.
Show resolved Hide resolved
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();
DominicFrei marked this conversation as resolved.
Show resolved Hide resolved
} 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) {
DominicFrei marked this conversation as resolved.
Show resolved Hide resolved
object.remove();
}
}
}

Expand Down
230 changes: 228 additions & 2 deletions test/object-store/migrations.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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 = {
Expand Down Expand Up @@ -601,6 +601,232 @@ 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") {
DominicFrei marked this conversation as resolved.
Show resolved Hide resolved
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") {
Expand Down