From f3a10e3c8607890852f8116f2bb89b7a6cb4879a Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Thu, 11 Jan 2018 09:07:31 +0100 Subject: [PATCH 01/11] Add support for link types in properties and migrations --- src/object_store.cpp | 28 +++++++++++++++++++++++----- src/property.hpp | 19 +++++++++++++++++-- src/schema.cpp | 4 ++++ src/schema.hpp | 6 ++++++ 4 files changed, 50 insertions(+), 7 deletions(-) diff --git a/src/object_store.cpp b/src/object_store.cpp index a98e774ed..7dadc5de8 100644 --- a/src/object_store.cpp +++ b/src/object_store.cpp @@ -114,7 +114,7 @@ void insert_column(Group& group, Table& table, Property const& property, size_t TableRef link_table = group.get_table(target_name); REALM_ASSERT(link_table); table.insert_column_link(col_ndx, is_array(property.type) ? type_LinkList : type_Link, - property.name, *link_table); + property.name, *link_table, property.link_type()); } else if (is_array(property.type)) { DescriptorRef desc; @@ -394,6 +394,21 @@ struct SchemaDifferenceExplainer { { errors.emplace_back("Property '%1.%2' has been made unindexed.", op.object->name, op.property->name); } + void operator()(schema_change::ChangeRelationshipType op) + { + std::string type; + switch (op.property->relationship) { + case Relationship::Strong: + type = "'Strong'"; + break; + + case Relationship::Weak: + type = "'Weak'"; + break; + } + + errors.emplace_back("Property '%1.%2' has changed the type of the relationship to %.", type); + } }; class TableHelper { @@ -443,6 +458,7 @@ bool ObjectStore::needs_migration(std::vector const& changes) bool operator()(MakePropertyRequired) { return true; } bool operator()(RemoveIndex) { return false; } bool operator()(RemoveProperty) { return true; } + bool operator()(ChangeRelationshipType) { return true; } }; return std::any_of(begin(changes), end(changes), @@ -565,11 +581,9 @@ static void create_initial_tables(Group& group, std::vector const& void operator()(ChangePrimaryKey op) { ObjectStore::set_primary_key_for_object(group, op.object->name, op.property ? StringData{op.property->name} : ""); } void operator()(AddIndex op) { table(op.object).add_search_index(op.property->table_column); } void operator()(RemoveIndex op) { table(op.object).remove_search_index(op.property->table_column); } + void operator()(ChangePropertyType op) { replace_column(group, table(op.object), *op.old_property, *op.new_property); } + void operator()(ChangeRelationshipType op) { table(op.object).get_descriptor()->set_link_type(op.property->table_column, op.property->link_type()); } - void operator()(ChangePropertyType op) - { - replace_column(group, table(op.object), *op.old_property, *op.new_property); - } } applier{group}; for (auto& change : changes) { @@ -599,6 +613,7 @@ void ObjectStore::apply_additive_changes(Group& group, std::vector void operator()(ChangePropertyType) { } void operator()(MakePropertyNullable) { } void operator()(MakePropertyRequired) { } + void operator()(ChangeRelationshipType) { } } applier{group, update_indexes}; for (auto& change : changes) { @@ -624,6 +639,8 @@ static void apply_pre_migration_changes(Group& group, std::vector void operator()(ChangePrimaryKey op) { ObjectStore::set_primary_key_for_object(group, op.object->name.c_str(), op.property ? op.property->name.c_str() : ""); } void operator()(AddIndex op) { table(op.object).add_search_index(op.property->table_column); } void operator()(RemoveIndex op) { table(op.object).remove_search_index(op.property->table_column); } + void operator()(ChangeRelationshipType op) { table(op.object).get_descriptor()->set_link_type(op.property->table_column, op.property->link_type()); } + } applier{group}; for (auto& change : changes) { @@ -680,6 +697,7 @@ static void apply_post_migration_changes(Group& group, std::vector void operator()(MakePropertyNullable) { } void operator()(MakePropertyRequired) { } void operator()(AddProperty) { } + void operator()(ChangeRelationshipType) { } } applier{group, initial_schema, did_reread_schema}; for (auto& change : changes) { diff --git a/src/property.hpp b/src/property.hpp index ad76b9d05..cc7bb0652 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -22,6 +22,7 @@ #include "util/tagged_bool.hpp" #include +#include #include @@ -58,6 +59,10 @@ enum class PropertyType : unsigned char { Flags = Nullable | Array }; +enum class Relationship { + Strong, + Weak +}; struct Property { using IsPrimary = util::TaggedBool; using IsIndexed = util::TaggedBool; @@ -66,6 +71,7 @@ struct Property { PropertyType type = PropertyType::Int; std::string object_type; std::string link_origin_property_name; + Relationship relationship; // Only useful if the property is Link or LinkList property IsPrimary is_primary = false; IsIndexed is_indexed = false; @@ -76,7 +82,7 @@ struct Property { Property(std::string name, PropertyType type, IsPrimary primary = false, IsIndexed indexed = false); Property(std::string name, PropertyType type, std::string object_type, - std::string link_origin_property_name = ""); + std::string link_origin_property_name = "", Relationship relationship = Relationship::Weak); Property(Property const&) = default; Property(Property&&) = default; @@ -85,6 +91,13 @@ struct Property { bool requires_index() const { return is_primary || is_indexed; } + // Return the underlying LinkType. Only useful for Object or Array properties. + LinkType link_type() const { + switch(relationship) { + case Relationship::Strong: return LinkType::link_Strong; + case Relationship::Weak: return LinkType::link_Weak; + } + } bool type_is_indexable() const; bool type_is_nullable() const; @@ -206,11 +219,13 @@ inline Property::Property(std::string name, PropertyType type, inline Property::Property(std::string name, PropertyType type, std::string object_type, - std::string link_origin_property_name) + std::string link_origin_property_name, + Relationship relationship) : name(std::move(name)) , type(type) , object_type(std::move(object_type)) , link_origin_property_name(std::move(link_origin_property_name)) +, relationship(relationship) { } diff --git a/src/schema.cpp b/src/schema.cpp index efdc257b9..bab23d32e 100644 --- a/src/schema.cpp +++ b/src/schema.cpp @@ -136,6 +136,9 @@ static void compare(ObjectSchema const& existing_schema, else if (current_prop.requires_index()) { changes.emplace_back(schema_change::RemoveIndex{&existing_schema, ¤t_prop}); } + if(target_prop->relationship != current_prop.relationship) { + changes.emplace_back(schema_change::ChangeRelationshipType{&existing_schema, ¤t_prop}); + } } if (existing_schema.primary_key != target_schema.primary_key) { @@ -250,6 +253,7 @@ bool operator==(SchemaChange const& lft, SchemaChange const& rgt) REALM_SC_COMPARE(MakePropertyRequired, v.object, v.property) REALM_SC_COMPARE(RemoveIndex, v.object, v.property) REALM_SC_COMPARE(RemoveProperty, v.object, v.property) + REALM_SC_COMPARE(ChangeRelationshipType, v.object, v.property) #undef REALM_SC_COMPARE } visitor{lft}; diff --git a/src/schema.hpp b/src/schema.hpp index 3feab40fa..1facfd3ad 100644 --- a/src/schema.hpp +++ b/src/schema.hpp @@ -126,6 +126,11 @@ struct ChangePrimaryKey { const ObjectSchema* object; const Property* property; }; + +struct ChangeRelationshipType { + const ObjectSchema* object; + const Property* property; +}; } #define REALM_FOR_EACH_SCHEMA_CHANGE_TYPE(macro) \ @@ -139,6 +144,7 @@ struct ChangePrimaryKey { macro(AddIndex) \ macro(RemoveIndex) \ macro(ChangePrimaryKey) \ + macro(ChangeRelationshipType) \ class SchemaChange { public: From 2c871e177c5bfe3800a4fd111d592b1b41dce0a8 Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Thu, 11 Jan 2018 09:28:46 +0100 Subject: [PATCH 02/11] Fix compiler issues --- src/property.hpp | 5 +++-- tests/schema.cpp | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/property.hpp b/src/property.hpp index cc7bb0652..a5bd1bb38 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -21,10 +21,11 @@ #include "util/tagged_bool.hpp" -#include +#include + #include +#include -#include namespace realm { namespace util { diff --git a/tests/schema.cpp b/tests/schema.cpp index 7b91a4d71..ced1994dc 100644 --- a/tests/schema.cpp +++ b/tests/schema.cpp @@ -59,6 +59,7 @@ struct SchemaChangePrinter { REALM_SC_PRINT(AddInitialProperties, v.object) REALM_SC_PRINT(ChangePrimaryKey, v.object, v.property) REALM_SC_PRINT(ChangePropertyType, v.object, v.old_property, v.new_property) + REALM_SC_PRINT(ChangeRelationshipType, v.object, v.property) REALM_SC_PRINT(MakePropertyNullable, v.object, v.property) REALM_SC_PRINT(MakePropertyRequired, v.object, v.property) REALM_SC_PRINT(RemoveIndex, v.object, v.property) From 50123392a9c2a10bb3fb83a7bb915bcf71724a8f Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Thu, 11 Jan 2018 10:10:22 +0100 Subject: [PATCH 03/11] Fix unit test + cleanup --- src/property.hpp | 2 +- src/schema.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/property.hpp b/src/property.hpp index a5bd1bb38..328dd248e 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -72,7 +72,7 @@ struct Property { PropertyType type = PropertyType::Int; std::string object_type; std::string link_origin_property_name; - Relationship relationship; // Only useful if the property is Link or LinkList property + Relationship relationship = Relationship::Weak; // Only useful if the property is Link or LinkList property IsPrimary is_primary = false; IsIndexed is_indexed = false; diff --git a/src/schema.cpp b/src/schema.cpp index bab23d32e..7429098d6 100644 --- a/src/schema.cpp +++ b/src/schema.cpp @@ -249,11 +249,11 @@ bool operator==(SchemaChange const& lft, SchemaChange const& rgt) REALM_SC_COMPARE(AddTable, v.object) REALM_SC_COMPARE(ChangePrimaryKey, v.object, v.property) REALM_SC_COMPARE(ChangePropertyType, v.object, v.old_property, v.new_property) + REALM_SC_COMPARE(ChangeRelationshipType, v.object, v.property) REALM_SC_COMPARE(MakePropertyNullable, v.object, v.property) REALM_SC_COMPARE(MakePropertyRequired, v.object, v.property) REALM_SC_COMPARE(RemoveIndex, v.object, v.property) REALM_SC_COMPARE(RemoveProperty, v.object, v.property) - REALM_SC_COMPARE(ChangeRelationshipType, v.object, v.property) #undef REALM_SC_COMPARE } visitor{lft}; From f5d05c2f0a816713871921acf93dbbdd5fb88ef8 Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Thu, 11 Jan 2018 11:47:49 +0100 Subject: [PATCH 04/11] Add migration unit tests --- src/object_store.cpp | 8 +++++++- tests/migrations.cpp | 26 ++++++++++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/src/object_store.cpp b/src/object_store.cpp index 7dadc5de8..14605f3f9 100644 --- a/src/object_store.cpp +++ b/src/object_store.cpp @@ -407,7 +407,7 @@ struct SchemaDifferenceExplainer { break; } - errors.emplace_back("Property '%1.%2' has changed the type of the relationship to %.", type); + errors.emplace_back("Property '%1.%2' has changed the type of the relationship to %3.", op.object->name, op.property->name, type); } }; @@ -811,6 +811,12 @@ util::Optional ObjectStore::property_for_column_index(ConstTableRef& t // set link type for objects and arrays ConstTableRef linkTable = table->get_link_target(column_index); property.object_type = ObjectStore::object_type_for_table_name(linkTable->get_name().data()); + + // TODO Add Table::get_link_type(col_idx) to Core + typedef _impl::TableFriend tf; + ColumnAttr attr = tf::get_spec(*table).get_column_attr(column_index) ; + bool is_strong_link = (attr & ColumnAttr::col_attr_StrongLinks) != 0; + property.relationship = (is_strong_link) ? Relationship::Strong : Relationship::Weak; } return property; } diff --git a/tests/migrations.cpp b/tests/migrations.cpp index 07226cb75..82db1cad8 100644 --- a/tests/migrations.cpp +++ b/tests/migrations.cpp @@ -146,6 +146,11 @@ Schema set_target(Schema schema, StringData object_name, StringData property_nam return schema; } +Schema set_relationship(Schema schema, StringData object_name, StringData property_name, Relationship new_relationship_type) { + schema.find(object_name)->property_for_name(property_name)->relationship = new_relationship_type; + return schema; +} + Schema set_primary_key(Schema schema, StringData object_name, StringData new_primary_property) { auto& object_schema = *schema.find(object_name); @@ -1735,7 +1740,7 @@ TEST_CASE("migration: Manual") { {"link origin", { {"not a pk", PropertyType::Int}, {"object", PropertyType::Object|PropertyType::Nullable, "object"}, - {"array", PropertyType::Array|PropertyType::Object, "object"}, + {"array", PropertyType::Array|PropertyType::Object, "object", "", Relationship::Strong}, }} }; realm->update_schema(schema); @@ -1814,7 +1819,7 @@ TEST_CASE("migration: Manual") { [](SharedRealm, SharedRealm realm, Schema&) { auto table = get_table(realm, "link origin"); table->remove_column(2); - table->add_column_link(type_LinkList, "array", *table); + table->add_column_link(type_LinkList, "array", *table, LinkType::link_Strong); }); } SECTION("make property optional") { @@ -1873,4 +1878,21 @@ TEST_CASE("migration: Manual") { Schema new_schema = remove_property(schema, "object", "value"); REQUIRE_THROWS_AS(realm->update_schema(new_schema, 1, nullptr), SchemaMismatchException); } + + SECTION("change relationship type from Weak to Strong") { + REQUIRE_MIGRATION(set_relationship(schema, "link origin", "object", Relationship::Strong), + [](SharedRealm, SharedRealm realm, Schema&) { + auto table = get_table(realm, "link origin"); + table->get_descriptor()->set_link_type(1, LinkType::link_Strong); + }); + } + + SECTION("change relationship type from Strong to Weak") { + REQUIRE_MIGRATION(set_relationship(schema, "link origin", "array", Relationship::Weak), + [](SharedRealm, SharedRealm realm, Schema&) { + auto table = get_table(realm, "link origin"); + table->get_descriptor()->set_link_type(2, LinkType::link_Weak); + }); + } + } From 6bdbfdba64573037a572bb0bb2533e0640168e12 Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Thu, 11 Jan 2018 16:40:30 +0100 Subject: [PATCH 05/11] Add unit tests --- tests/CMakeLists.txt | 1 + tests/link_types.cpp | 303 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 304 insertions(+) create mode 100644 tests/link_types.cpp diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e90d91b30..bb6fb4f6f 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,6 +10,7 @@ set(HEADERS set(SOURCES collection_change_indices.cpp index_set.cpp + link_types.cpp list.cpp main.cpp migrations.cpp diff --git a/tests/link_types.cpp b/tests/link_types.cpp new file mode 100644 index 000000000..1f7cfa6de --- /dev/null +++ b/tests/link_types.cpp @@ -0,0 +1,303 @@ +//////////////////////////////////////////////////////////////////////////// +// +// Copyright 2018 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 "catch.hpp" +#include "object_accessor.hpp" +#include "impl/realm_coordinator.hpp" +#include "impl/object_accessor_impl.hpp" +#include "util/test_file.hpp" + +// This class contain tests for the interaction between Strong and Weak links. +// See Descriptor::set_link_type() in Core (descriptor.hpp) for an extended explanation of the exact semantics. + +namespace realm { +class TestHelper { +public: + static SharedRealm get_realm() { + TestFile config; + config.schema_version = 1; + Schema schema = { + // DAG Object hierarchy: (A -> Child) and (B -> Child) + {"WeakParentA", { + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", "", Relationship::Weak}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "Child", "", Relationship::Weak}, + }}, + {"StrongParentB", { + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", "", Relationship::Strong}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "Child", "", Relationship::Strong}, + }}, + {"Child", { + {"prop", PropertyType::String}, + }}, + + // Cyclic Object hierarchy: (C -> D) and (D -> C) + {"CycleC", { + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", "", Relationship::Weak}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", "", Relationship::Strong}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", "", Relationship::Weak}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", "", Relationship::Strong}, + }}, + {"CycleD", { + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", "", Relationship::Weak}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", "", Relationship::Strong}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", "", Relationship::Weak}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", "", Relationship::Strong}, + }}, + + // Cyclic Object hierarchy: (E -> E) + {"SingleClassCycleE", { + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", "", Relationship::Weak}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", "", Relationship::Strong}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", "", Relationship::Weak}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", "", Relationship::Strong}, + }}, + + // Island graphs: (F -> (C <-> D)) + {"IslandParentF", { + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", "", Relationship::Strong}, + }}, + }; + config.schema = schema; + + return Realm::get_shared_realm(config); + }; + + static long get_count(SharedRealm realm, ObjectSchema object_class_schema) { + return ObjectStore::table_for_object_type(realm->read_group(), object_class_schema.name)->size(); + } +}; +} + +using namespace realm; +using namespace std::string_literals; + +TEST_CASE("If no more strong links exist, objects are deleted") { + SharedRealm realm = TestHelper::get_realm(); + CppContext context(realm); + auto child_schema = *realm->schema().find("Child"); + auto weak_parent_schema = *realm->schema().find("WeakParentA"); + auto strong_parent_schema = *realm->schema().find("StrongParentB"); + auto cyclic_schema_c = *realm->schema().find("CycleC"); + auto cyclic_schema_d = *realm->schema().find("CycleD"); + auto cyclic_schema_e = *realm->schema().find("SingleClassCycleE"); + auto island_parent_schema = *realm->schema().find("IslandParentF"); + + SECTION("DAG with only strong links and they are all removed") { + // Create data with links + realm->begin_transaction(); + auto child = Object::create(context, realm, child_schema, AnyDict{{ "prop", "childA"s }}, false); + auto parent = Object::create(context, realm, strong_parent_schema, AnyDict{ + { "strongObjectRef", child }, + { "strongArrayRef", AnyVector{ child }} + }); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, child_schema)); + + // Remove first direct Strong link + realm->begin_transaction(); + parent.set_property_value(context, "strongObjectRef", Any(), true); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, child_schema)); + + // Remove second Strong link through a List = no more strong links + realm->begin_transaction(); + parent.set_property_value(context, "strongArrayRef", Any(), true); + realm->commit_transaction(); + REQUIRE(0 == TestHelper::get_count(realm, child_schema)); + } + + SECTION("DAG with mix of weak and strong links and all Strong links are removed") { + // Create mix of links to same object + realm->begin_transaction(); + auto child = Object::create(context, realm, child_schema, AnyDict{{ "prop", "childA"s }}, false); + auto weak_parent = Object::create(context, realm, weak_parent_schema, AnyDict{ {"weakObjectRef", child} }, false); + auto strong_parent = Object::create(context, realm, strong_parent_schema, AnyDict{ {"strongObjectRef", child} }, false); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, child_schema)); + + // Removing Strong link should delete object + realm->begin_transaction(); + strong_parent.set_property_value(context, "strongObjectRef", Any(), true); + realm->commit_transaction(); + REQUIRE(0 == TestHelper::get_count(realm, child_schema)); + } + + // Special case described in descriptor.hpp: (A <-> B) -> (A -> B) = Both objects are deleted. + SECTION("Breaking an isolated strong cycle deletes both objects") { + // Create isolated strong cyclic graph + realm->begin_transaction(); + auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); + auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); + obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); + + // Breaking the strong cycle deletes both objects (see details in descriptor.hpp) + realm->begin_transaction(); + obj_c.set_property_value(context, "strongObjectRef", Any(), true); + realm->commit_transaction(); + REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_d)); + + } + + // (F -> (C <-> D)) -> (F -> C) = D is deleted + SECTION("Breaking a strong cycle with outside links only delete leftover object") { + // Create strong cyclic graph with outside link + realm->begin_transaction(); + auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); + auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); + obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); + Object::create(context, realm, island_parent_schema, AnyDict{ {"strongObjectRef", obj_c} }, false); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); + REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); + + // Breaking the strong cycle only deletes the now single isolated object + realm->begin_transaction(); + obj_c.set_property_value(context, "strongObjectRef", Any(), true); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_d)); + REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); + } + + // (F -> (C <-> D)) -> (C <-> D) = No objects are deleted + SECTION("Removing reference to Strong island of multiple classes do not remove the island") { + // Create strong cyclic graph with outside link + realm->begin_transaction(); + auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); + auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); + obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); + auto obj_f = Object::create(context, realm, island_parent_schema, AnyDict{ {"objectRef", obj_c } }, false); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); + REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); + + // Breaking the reference to the island does not delete any objects. + realm->begin_transaction(); + obj_f.set_property_value(context, "strongObjectRef", Any(), true); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); + REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); + } + + // (E1 -> (E2 <-> E3)) -> (E2 <-> E3) = No objects are deleted + SECTION("Removing reference to Strong island of a single class do not remove the island") { + // Create strong cyclic graph with outside link consisting of only a single object type + realm->begin_transaction(); + auto obj_e2 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any()} }, false); + auto obj_e3 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", obj_e2} }, false); + obj_e2.set_property_value(context, "strongObjectRef", Any(obj_e3), true); + auto obj_e1 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", obj_e2 } }, false); + realm->commit_transaction(); + REQUIRE(3 == TestHelper::get_count(realm, cyclic_schema_e)); + + // Breaking the reference to the island does not delete any objects. + realm->begin_transaction(); + obj_e1.set_property_value(context, "strongObjectRef", Any(), true); + realm->commit_transaction(); + REQUIRE(3 == TestHelper::get_count(realm, cyclic_schema_e)); + } + +} + +TEST_CASE("Removal of children is immediate inside transactions.") { + SharedRealm realm = TestHelper::get_realm(); + CppContext context(realm); + auto child_schema = *realm->schema().find("Child"); + auto weak_parent_schema = *realm->schema().find("WeakParentA"); + auto strong_parent_schema = *realm->schema().find("StrongParentB"); + auto cyclic_schema_c = *realm->schema().find("CycleC"); + auto cyclic_schema_d = *realm->schema().find("CycleD"); + auto cyclic_schema_e = *realm->schema().find("SingleClassCycleE"); + + // (wP -> Child) and (sP -> Child) -> (wP -> Child) = Child is deleted + SECTION("DAG with mix of weak and strong links. Removing references causes immediate deletion.") { + // Create mix of links to same object + realm->begin_transaction(); + auto child = Object::create(context, realm, child_schema, AnyDict{{ "prop", "childA"s }}, false); + auto weak_parent = Object::create(context, realm, weak_parent_schema, AnyDict{ {"weakObjectRef", child} }, false); + auto strong_parent = Object::create(context, realm, strong_parent_schema, AnyDict{ {"strongObjectRef", child} }, false); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, child_schema)); + + // Removing Strong link should delete object + realm->begin_transaction(); + strong_parent.set_property_value(context, "strongObjectRef", Any(), true); + REQUIRE(0 == TestHelper::get_count(realm, child_schema)); + + // Canceling transaction restores objects again + realm->cancel_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, child_schema)); + } + + // Special case described in descriptor.hpp: (A <-> B) -> (A -> B) = Both objects are deleted. + // See descriptor.hpp for details. + SECTION("Breaking an island deletes objects immediately") { + // Create isolated strong cyclic graph + realm->begin_transaction(); + auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); + auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); + obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); + + // Breaking the island deletes both objects. + realm->begin_transaction(); + obj_c.set_property_value(context, "strongObjectRef", Any(), true); + REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_d)); + + // Canceling transaction restores objects again + realm->cancel_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); + } +} + +TEST_CASE("Objects created with no parents are not deleted") { + SharedRealm realm = TestHelper::get_realm(); + CppContext context(realm); + auto cyclic_schema_e = *realm->schema().find("SingleClassCycleE"); + + SECTION("Object with a strong incoming reference is not deleted if no parent exist when it is created") { + realm->begin_transaction(); + auto obj = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any()} }, false); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); + } + + SECTION("Cascading behaviour is only triggered when references are updated") { + realm->begin_transaction(); + auto obj_e1 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any()} }, false); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); + auto obj_e2 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any(obj_e1)} }, false); + REQUIRE(2 == TestHelper::get_count(realm, cyclic_schema_e)); + obj_e2.set_property_value(context, "strongObjectRef", Any(), true); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); + realm->commit_transaction(); + REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); + } +} \ No newline at end of file From 96f89f124abbb2307aa3c5d2edcadd0214d7b991 Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Thu, 11 Jan 2018 16:54:00 +0100 Subject: [PATCH 06/11] Disallow setting LinkType on Backlinks. --- src/property.hpp | 32 +++++++++++++++++++++++++------- tests/link_types.cpp | 36 ++++++++++++++++++------------------ tests/migrations.cpp | 2 +- 3 files changed, 44 insertions(+), 26 deletions(-) diff --git a/src/property.hpp b/src/property.hpp index 328dd248e..74a88cebf 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -71,8 +71,10 @@ struct Property { std::string name; PropertyType type = PropertyType::Int; std::string object_type; - std::string link_origin_property_name; - Relationship relationship = Relationship::Weak; // Only useful if the property is Link or LinkList property + // Only used with LinkingObjects. `relationship` is ignored if this is different than "" + std::string link_origin_property_name = ""; + // Only useful if the property is Link or LinkList property and `link_origin_property_name` is "" + Relationship relationship = Relationship::Weak; IsPrimary is_primary = false; IsIndexed is_indexed = false; @@ -81,9 +83,9 @@ struct Property { Property() = default; Property(std::string name, PropertyType type, IsPrimary primary = false, IsIndexed indexed = false); - - Property(std::string name, PropertyType type, std::string object_type, - std::string link_origin_property_name = "", Relationship relationship = Relationship::Weak); + Property(std::string name, PropertyType type, std::string object_type); + Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name); + Property(std::string name, PropertyType type, std::string object_type, Relationship relationship); Property(Property const&) = default; Property(Property&&) = default; @@ -218,14 +220,30 @@ inline Property::Property(std::string name, PropertyType type, { } +inline Property::Property(std::string name, PropertyType type, + std::string object_type) +: name(std::move(name)) +, type(type) +, object_type(std::move(object_type)) +{ +} + inline Property::Property(std::string name, PropertyType type, std::string object_type, - std::string link_origin_property_name, - Relationship relationship) + std::string link_origin_property_name) : name(std::move(name)) , type(type) , object_type(std::move(object_type)) , link_origin_property_name(std::move(link_origin_property_name)) +{ +} + +inline Property::Property(std::string name, PropertyType type, + std::string object_type, + Relationship relationship) +: name(std::move(name)) +, type(type) +, object_type(std::move(object_type)) , relationship(relationship) { } diff --git a/tests/link_types.cpp b/tests/link_types.cpp index 1f7cfa6de..5246928f8 100644 --- a/tests/link_types.cpp +++ b/tests/link_types.cpp @@ -34,12 +34,12 @@ class TestHelper { Schema schema = { // DAG Object hierarchy: (A -> Child) and (B -> Child) {"WeakParentA", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", "", Relationship::Weak}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "Child", "", Relationship::Weak}, + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", Relationship::Weak}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "Child", Relationship::Weak}, }}, {"StrongParentB", { - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", "", Relationship::Strong}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "Child", "", Relationship::Strong}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", Relationship::Strong}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "Child", Relationship::Strong}, }}, {"Child", { {"prop", PropertyType::String}, @@ -47,29 +47,29 @@ class TestHelper { // Cyclic Object hierarchy: (C -> D) and (D -> C) {"CycleC", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", "", Relationship::Weak}, - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", "", Relationship::Strong}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", "", Relationship::Weak}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", "", Relationship::Strong}, + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", Relationship::Weak}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", Relationship::Strong}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", Relationship::Weak}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", Relationship::Strong}, }}, {"CycleD", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", "", Relationship::Weak}, - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", "", Relationship::Strong}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", "", Relationship::Weak}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", "", Relationship::Strong}, + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", Relationship::Weak}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", Relationship::Strong}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", Relationship::Weak}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", Relationship::Strong}, }}, // Cyclic Object hierarchy: (E -> E) {"SingleClassCycleE", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", "", Relationship::Weak}, - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", "", Relationship::Strong}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", "", Relationship::Weak}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", "", Relationship::Strong}, + {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", Relationship::Weak}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", Relationship::Strong}, + {"weakArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", Relationship::Weak}, + {"strongArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", Relationship::Strong}, }}, // Island graphs: (F -> (C <-> D)) {"IslandParentF", { - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", "", Relationship::Strong}, + {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", Relationship::Strong}, }}, }; config.schema = schema; @@ -300,4 +300,4 @@ TEST_CASE("Objects created with no parents are not deleted") { realm->commit_transaction(); REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); } -} \ No newline at end of file +} diff --git a/tests/migrations.cpp b/tests/migrations.cpp index 82db1cad8..12402f2ae 100644 --- a/tests/migrations.cpp +++ b/tests/migrations.cpp @@ -1740,7 +1740,7 @@ TEST_CASE("migration: Manual") { {"link origin", { {"not a pk", PropertyType::Int}, {"object", PropertyType::Object|PropertyType::Nullable, "object"}, - {"array", PropertyType::Array|PropertyType::Object, "object", "", Relationship::Strong}, + {"array", PropertyType::Array|PropertyType::Object, "object", Relationship::Strong}, }} }; realm->update_schema(schema); From 5616f811d8a46aac01161140ab808b0c499e0de3 Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Fri, 12 Jan 2018 08:59:30 +0100 Subject: [PATCH 07/11] Support GCC --- src/property.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/property.hpp b/src/property.hpp index 74a88cebf..7248b84b9 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -95,12 +95,16 @@ struct Property { bool requires_index() const { return is_primary || is_indexed; } // Return the underlying LinkType. Only useful for Object or Array properties. +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wreturn-type" LinkType link_type() const { switch(relationship) { case Relationship::Strong: return LinkType::link_Strong; case Relationship::Weak: return LinkType::link_Weak; } } +#pragma GCC diagnostic pop + bool type_is_indexable() const; bool type_is_nullable() const; From c78dd014682a6f2ca544a740b824e06db08fcc4c Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Fri, 12 Jan 2018 21:37:27 +0100 Subject: [PATCH 08/11] PR feedback --- src/property.hpp | 12 +- src/schema.cpp | 2 +- tests/CMakeLists.txt | 1 - tests/link_types.cpp | 303 ------------------------------------------- tests/migrations.cpp | 3 +- 5 files changed, 8 insertions(+), 313 deletions(-) delete mode 100644 tests/link_types.cpp diff --git a/src/property.hpp b/src/property.hpp index 7248b84b9..f6a4852cc 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -60,10 +60,11 @@ enum class PropertyType : unsigned char { Flags = Nullable | Array }; -enum class Relationship { - Strong, - Weak +enum class Relationship : bool { + Strong = true, + Weak = false }; + struct Property { using IsPrimary = util::TaggedBool; using IsIndexed = util::TaggedBool; @@ -95,15 +96,12 @@ struct Property { bool requires_index() const { return is_primary || is_indexed; } // Return the underlying LinkType. Only useful for Object or Array properties. -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wreturn-type" LinkType link_type() const { - switch(relationship) { + switch (relationship) { case Relationship::Strong: return LinkType::link_Strong; case Relationship::Weak: return LinkType::link_Weak; } } -#pragma GCC diagnostic pop bool type_is_indexable() const; bool type_is_nullable() const; diff --git a/src/schema.cpp b/src/schema.cpp index 7429098d6..160569967 100644 --- a/src/schema.cpp +++ b/src/schema.cpp @@ -136,7 +136,7 @@ static void compare(ObjectSchema const& existing_schema, else if (current_prop.requires_index()) { changes.emplace_back(schema_change::RemoveIndex{&existing_schema, ¤t_prop}); } - if(target_prop->relationship != current_prop.relationship) { + if (target_prop->relationship != current_prop.relationship) { changes.emplace_back(schema_change::ChangeRelationshipType{&existing_schema, ¤t_prop}); } } diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index bb6fb4f6f..e90d91b30 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -10,7 +10,6 @@ set(HEADERS set(SOURCES collection_change_indices.cpp index_set.cpp - link_types.cpp list.cpp main.cpp migrations.cpp diff --git a/tests/link_types.cpp b/tests/link_types.cpp deleted file mode 100644 index 5246928f8..000000000 --- a/tests/link_types.cpp +++ /dev/null @@ -1,303 +0,0 @@ -//////////////////////////////////////////////////////////////////////////// -// -// Copyright 2018 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 "catch.hpp" -#include "object_accessor.hpp" -#include "impl/realm_coordinator.hpp" -#include "impl/object_accessor_impl.hpp" -#include "util/test_file.hpp" - -// This class contain tests for the interaction between Strong and Weak links. -// See Descriptor::set_link_type() in Core (descriptor.hpp) for an extended explanation of the exact semantics. - -namespace realm { -class TestHelper { -public: - static SharedRealm get_realm() { - TestFile config; - config.schema_version = 1; - Schema schema = { - // DAG Object hierarchy: (A -> Child) and (B -> Child) - {"WeakParentA", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", Relationship::Weak}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "Child", Relationship::Weak}, - }}, - {"StrongParentB", { - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "Child", Relationship::Strong}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "Child", Relationship::Strong}, - }}, - {"Child", { - {"prop", PropertyType::String}, - }}, - - // Cyclic Object hierarchy: (C -> D) and (D -> C) - {"CycleC", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", Relationship::Weak}, - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleD", Relationship::Strong}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", Relationship::Weak}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleD", Relationship::Strong}, - }}, - {"CycleD", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", Relationship::Weak}, - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", Relationship::Strong}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", Relationship::Weak}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "CycleC", Relationship::Strong}, - }}, - - // Cyclic Object hierarchy: (E -> E) - {"SingleClassCycleE", { - {"weakObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", Relationship::Weak}, - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "SingleClassCycleE", Relationship::Strong}, - {"weakArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", Relationship::Weak}, - {"strongArrayRef", PropertyType::Array|PropertyType::Object, "SingleClassCycleE", Relationship::Strong}, - }}, - - // Island graphs: (F -> (C <-> D)) - {"IslandParentF", { - {"strongObjectRef", PropertyType::Object|PropertyType::Nullable, "CycleC", Relationship::Strong}, - }}, - }; - config.schema = schema; - - return Realm::get_shared_realm(config); - }; - - static long get_count(SharedRealm realm, ObjectSchema object_class_schema) { - return ObjectStore::table_for_object_type(realm->read_group(), object_class_schema.name)->size(); - } -}; -} - -using namespace realm; -using namespace std::string_literals; - -TEST_CASE("If no more strong links exist, objects are deleted") { - SharedRealm realm = TestHelper::get_realm(); - CppContext context(realm); - auto child_schema = *realm->schema().find("Child"); - auto weak_parent_schema = *realm->schema().find("WeakParentA"); - auto strong_parent_schema = *realm->schema().find("StrongParentB"); - auto cyclic_schema_c = *realm->schema().find("CycleC"); - auto cyclic_schema_d = *realm->schema().find("CycleD"); - auto cyclic_schema_e = *realm->schema().find("SingleClassCycleE"); - auto island_parent_schema = *realm->schema().find("IslandParentF"); - - SECTION("DAG with only strong links and they are all removed") { - // Create data with links - realm->begin_transaction(); - auto child = Object::create(context, realm, child_schema, AnyDict{{ "prop", "childA"s }}, false); - auto parent = Object::create(context, realm, strong_parent_schema, AnyDict{ - { "strongObjectRef", child }, - { "strongArrayRef", AnyVector{ child }} - }); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, child_schema)); - - // Remove first direct Strong link - realm->begin_transaction(); - parent.set_property_value(context, "strongObjectRef", Any(), true); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, child_schema)); - - // Remove second Strong link through a List = no more strong links - realm->begin_transaction(); - parent.set_property_value(context, "strongArrayRef", Any(), true); - realm->commit_transaction(); - REQUIRE(0 == TestHelper::get_count(realm, child_schema)); - } - - SECTION("DAG with mix of weak and strong links and all Strong links are removed") { - // Create mix of links to same object - realm->begin_transaction(); - auto child = Object::create(context, realm, child_schema, AnyDict{{ "prop", "childA"s }}, false); - auto weak_parent = Object::create(context, realm, weak_parent_schema, AnyDict{ {"weakObjectRef", child} }, false); - auto strong_parent = Object::create(context, realm, strong_parent_schema, AnyDict{ {"strongObjectRef", child} }, false); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, child_schema)); - - // Removing Strong link should delete object - realm->begin_transaction(); - strong_parent.set_property_value(context, "strongObjectRef", Any(), true); - realm->commit_transaction(); - REQUIRE(0 == TestHelper::get_count(realm, child_schema)); - } - - // Special case described in descriptor.hpp: (A <-> B) -> (A -> B) = Both objects are deleted. - SECTION("Breaking an isolated strong cycle deletes both objects") { - // Create isolated strong cyclic graph - realm->begin_transaction(); - auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); - auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); - obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); - - // Breaking the strong cycle deletes both objects (see details in descriptor.hpp) - realm->begin_transaction(); - obj_c.set_property_value(context, "strongObjectRef", Any(), true); - realm->commit_transaction(); - REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_d)); - - } - - // (F -> (C <-> D)) -> (F -> C) = D is deleted - SECTION("Breaking a strong cycle with outside links only delete leftover object") { - // Create strong cyclic graph with outside link - realm->begin_transaction(); - auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); - auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); - obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); - Object::create(context, realm, island_parent_schema, AnyDict{ {"strongObjectRef", obj_c} }, false); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); - REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); - - // Breaking the strong cycle only deletes the now single isolated object - realm->begin_transaction(); - obj_c.set_property_value(context, "strongObjectRef", Any(), true); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_d)); - REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); - } - - // (F -> (C <-> D)) -> (C <-> D) = No objects are deleted - SECTION("Removing reference to Strong island of multiple classes do not remove the island") { - // Create strong cyclic graph with outside link - realm->begin_transaction(); - auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); - auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); - obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); - auto obj_f = Object::create(context, realm, island_parent_schema, AnyDict{ {"objectRef", obj_c } }, false); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); - REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); - - // Breaking the reference to the island does not delete any objects. - realm->begin_transaction(); - obj_f.set_property_value(context, "strongObjectRef", Any(), true); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); - REQUIRE(1 == TestHelper::get_count(realm, island_parent_schema)); - } - - // (E1 -> (E2 <-> E3)) -> (E2 <-> E3) = No objects are deleted - SECTION("Removing reference to Strong island of a single class do not remove the island") { - // Create strong cyclic graph with outside link consisting of only a single object type - realm->begin_transaction(); - auto obj_e2 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any()} }, false); - auto obj_e3 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", obj_e2} }, false); - obj_e2.set_property_value(context, "strongObjectRef", Any(obj_e3), true); - auto obj_e1 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", obj_e2 } }, false); - realm->commit_transaction(); - REQUIRE(3 == TestHelper::get_count(realm, cyclic_schema_e)); - - // Breaking the reference to the island does not delete any objects. - realm->begin_transaction(); - obj_e1.set_property_value(context, "strongObjectRef", Any(), true); - realm->commit_transaction(); - REQUIRE(3 == TestHelper::get_count(realm, cyclic_schema_e)); - } - -} - -TEST_CASE("Removal of children is immediate inside transactions.") { - SharedRealm realm = TestHelper::get_realm(); - CppContext context(realm); - auto child_schema = *realm->schema().find("Child"); - auto weak_parent_schema = *realm->schema().find("WeakParentA"); - auto strong_parent_schema = *realm->schema().find("StrongParentB"); - auto cyclic_schema_c = *realm->schema().find("CycleC"); - auto cyclic_schema_d = *realm->schema().find("CycleD"); - auto cyclic_schema_e = *realm->schema().find("SingleClassCycleE"); - - // (wP -> Child) and (sP -> Child) -> (wP -> Child) = Child is deleted - SECTION("DAG with mix of weak and strong links. Removing references causes immediate deletion.") { - // Create mix of links to same object - realm->begin_transaction(); - auto child = Object::create(context, realm, child_schema, AnyDict{{ "prop", "childA"s }}, false); - auto weak_parent = Object::create(context, realm, weak_parent_schema, AnyDict{ {"weakObjectRef", child} }, false); - auto strong_parent = Object::create(context, realm, strong_parent_schema, AnyDict{ {"strongObjectRef", child} }, false); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, child_schema)); - - // Removing Strong link should delete object - realm->begin_transaction(); - strong_parent.set_property_value(context, "strongObjectRef", Any(), true); - REQUIRE(0 == TestHelper::get_count(realm, child_schema)); - - // Canceling transaction restores objects again - realm->cancel_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, child_schema)); - } - - // Special case described in descriptor.hpp: (A <-> B) -> (A -> B) = Both objects are deleted. - // See descriptor.hpp for details. - SECTION("Breaking an island deletes objects immediately") { - // Create isolated strong cyclic graph - realm->begin_transaction(); - auto obj_c = Object::create(context, realm, cyclic_schema_c, AnyDict{ {"strongObjectRef", Any()} }, false); - auto obj_d = Object::create(context, realm, cyclic_schema_d, AnyDict{ {"strongObjectRef", obj_c} }, false); - obj_c.set_property_value(context, "strongObjectRef", Any(obj_d), true); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); - - // Breaking the island deletes both objects. - realm->begin_transaction(); - obj_c.set_property_value(context, "strongObjectRef", Any(), true); - REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(0 == TestHelper::get_count(realm, cyclic_schema_d)); - - // Canceling transaction restores objects again - realm->cancel_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_c)); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_d)); - } -} - -TEST_CASE("Objects created with no parents are not deleted") { - SharedRealm realm = TestHelper::get_realm(); - CppContext context(realm); - auto cyclic_schema_e = *realm->schema().find("SingleClassCycleE"); - - SECTION("Object with a strong incoming reference is not deleted if no parent exist when it is created") { - realm->begin_transaction(); - auto obj = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any()} }, false); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); - } - - SECTION("Cascading behaviour is only triggered when references are updated") { - realm->begin_transaction(); - auto obj_e1 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any()} }, false); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); - auto obj_e2 = Object::create(context, realm, cyclic_schema_e, AnyDict{ {"strongObjectRef", Any(obj_e1)} }, false); - REQUIRE(2 == TestHelper::get_count(realm, cyclic_schema_e)); - obj_e2.set_property_value(context, "strongObjectRef", Any(), true); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); - realm->commit_transaction(); - REQUIRE(1 == TestHelper::get_count(realm, cyclic_schema_e)); - } -} diff --git a/tests/migrations.cpp b/tests/migrations.cpp index 12402f2ae..7e52799e3 100644 --- a/tests/migrations.cpp +++ b/tests/migrations.cpp @@ -146,7 +146,8 @@ Schema set_target(Schema schema, StringData object_name, StringData property_nam return schema; } -Schema set_relationship(Schema schema, StringData object_name, StringData property_name, Relationship new_relationship_type) { +Schema set_relationship(Schema schema, StringData object_name, StringData property_name, Relationship new_relationship_type) +{ schema.find(object_name)->property_for_name(property_name)->relationship = new_relationship_type; return schema; } From 8cdbb599ba72e9c9ad760fac3ec89f03b9ba076e Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Fri, 16 Feb 2018 12:11:11 +0100 Subject: [PATCH 09/11] Remove redundant constructor --- src/property.hpp | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/src/property.hpp b/src/property.hpp index f6a4852cc..07a9cae9c 100644 --- a/src/property.hpp +++ b/src/property.hpp @@ -21,11 +21,10 @@ #include "util/tagged_bool.hpp" -#include - #include #include +#include namespace realm { namespace util { @@ -84,8 +83,7 @@ struct Property { Property() = default; Property(std::string name, PropertyType type, IsPrimary primary = false, IsIndexed indexed = false); - Property(std::string name, PropertyType type, std::string object_type); - Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name); + Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name = ""); Property(std::string name, PropertyType type, std::string object_type, Relationship relationship); Property(Property const&) = default; @@ -222,14 +220,6 @@ inline Property::Property(std::string name, PropertyType type, { } -inline Property::Property(std::string name, PropertyType type, - std::string object_type) -: name(std::move(name)) -, type(type) -, object_type(std::move(object_type)) -{ -} - inline Property::Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name) From 8a7fb06842d88d4f27c537cf08fc25e882051223 Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Mon, 30 Apr 2018 11:01:46 +0200 Subject: [PATCH 10/11] Use core helper function --- src/object_store.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/src/object_store.cpp b/src/object_store.cpp index dbdfe07ef..74a9ec607 100644 --- a/src/object_store.cpp +++ b/src/object_store.cpp @@ -908,12 +908,7 @@ util::Optional ObjectStore::property_for_column_index(ConstTableRef& t // set link type for objects and arrays ConstTableRef linkTable = table->get_link_target(column_index); property.object_type = ObjectStore::object_type_for_table_name(linkTable->get_name().data()); - - // TODO Add Table::get_link_type(col_idx) to Core - typedef _impl::TableFriend tf; - ColumnAttr attr = tf::get_spec(*table).get_column_attr(column_index) ; - bool is_strong_link = (attr & ColumnAttr::col_attr_StrongLinks) != 0; - property.relationship = (is_strong_link) ? Relationship::Strong : Relationship::Weak; + property.relationship = (table->get_link_type(column_index) == LinkType::link_Strong) ? Relationship::Strong : Relationship::Weak; } return property; } From ccf2d9b6ff6d8182da2768d487080536b23c95ba Mon Sep 17 00:00:00 2001 From: Christian Melchior Date: Mon, 30 Apr 2018 12:10:00 +0200 Subject: [PATCH 11/11] Fix test --- src/object_store.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/object_store.cpp b/src/object_store.cpp index 74a9ec607..55c8c490f 100644 --- a/src/object_store.cpp +++ b/src/object_store.cpp @@ -767,6 +767,7 @@ static void create_default_permissions(Group& group, std::vector c void operator()(AddIndex) { } void operator()(RemoveIndex) { } void operator()(ChangePropertyType) { } + void operator()(ChangeRelationshipType) { } } applier{group}; for (auto& change : changes) {