Skip to content
This repository was archived by the owner on Nov 8, 2021. It is now read-only.

Add support for Weak/Strong links aka Cascading deletes #622

Closed
cmelchior wants to merge 14 commits into from
Closed

Add support for Weak/Strong links aka Cascading deletes #622

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
f3a10e3
Add support for link types in properties and migrations
cmelchior Jan 11, 2018
2c871e1
Fix compiler issues
cmelchior Jan 11, 2018
5012339
Fix unit test + cleanup
cmelchior Jan 11, 2018
f5d05c2
Add migration unit tests
cmelchior Jan 11, 2018
6bdbfdb
Add unit tests
cmelchior Jan 11, 2018
96f89f1
Disallow setting LinkType on Backlinks.
cmelchior Jan 11, 2018
8a2a2fc
Merge branch 'master' into cm/support-link-type
cmelchior Jan 11, 2018
5616f81
Support GCC
cmelchior Jan 12, 2018
c78dd01
PR feedback
cmelchior Jan 12, 2018
dabdcb7
Merge branch 'master' into cm/support-link-type
cmelchior Feb 16, 2018
8cdbb59
Remove redundant constructor
cmelchior Feb 16, 2018
0aa34df
Merge branch 'master' into cm/support-link-type
cmelchior Apr 30, 2018
8a7fb06
Use core helper function
cmelchior Apr 30, 2018
ccf2d9b
Fix test
cmelchior Apr 30, 2018
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
34 changes: 29 additions & 5 deletions src/object_store.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 %3.", op.object->name, op.property->name, type);
}
};

class TableHelper {
Expand Down Expand Up @@ -443,6 +458,7 @@ bool ObjectStore::needs_migration(std::vector<SchemaChange> 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),
Expand Down Expand Up @@ -565,11 +581,9 @@ static void create_initial_tables(Group& group, std::vector<SchemaChange> 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) {
Expand Down Expand Up @@ -599,6 +613,7 @@ void ObjectStore::apply_additive_changes(Group& group, std::vector<SchemaChange>
void operator()(ChangePropertyType) { }
void operator()(MakePropertyNullable) { }
void operator()(MakePropertyRequired) { }
void operator()(ChangeRelationshipType) { }
} applier{group, update_indexes};

for (auto& change : changes) {
Expand All @@ -624,6 +639,8 @@ static void apply_pre_migration_changes(Group& group, std::vector<SchemaChange>
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) {
Expand Down Expand Up @@ -680,6 +697,7 @@ static void apply_post_migration_changes(Group& group, std::vector<SchemaChange>
void operator()(MakePropertyNullable) { }
void operator()(MakePropertyRequired) { }
void operator()(AddProperty) { }
void operator()(ChangeRelationshipType) { }
} applier{group, initial_schema, did_reread_schema};

for (auto& change : changes) {
Expand Down Expand Up @@ -793,6 +811,12 @@ util::Optional<Property> 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;
}
Expand Down
48 changes: 43 additions & 5 deletions src/property.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,11 @@

#include "util/tagged_bool.hpp"

#include <string>
Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This include was previously in the correct place.

Copy link
Copy Markdown
Contributor Author

@cmelchior cmelchior Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure why, but If this is not here I get:

In file included from /Users/cm/Realm/realm-object-store/src/property.hpp:24:
/Users/cm/Realm/realm-object-store/CMakeFiles/realm-core-5.0.1/include/realm/data_type.hpp:27:9: error: unknown type name 'int64_t'
typedef int64_t Int;

Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data_types.hpp needs to include <cstdint>


#include <realm/data_type.hpp>
#include <realm/util/features.h>

#include <string>

namespace realm {
namespace util {
Expand Down Expand Up @@ -58,14 +60,21 @@ enum class PropertyType : unsigned char {
Flags = Nullable | Array
};

enum class Relationship {
Strong,
Weak
};
struct Property {
using IsPrimary = util::TaggedBool<class IsPrimaryTag>;
using IsIndexed = util::TaggedBool<class IsIndexedTag>;

std::string name;
PropertyType type = PropertyType::Int;
std::string object_type;
std::string link_origin_property_name;
// 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;

Expand All @@ -74,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 = "");
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;
Expand All @@ -85,6 +94,17 @@ 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"
Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Giving Relationship a backing type of bool eliminates the need for this.

LinkType link_type() const {
switch(relationship) {
Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space between keyword and (

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;

Expand Down Expand Up @@ -204,6 +224,14 @@ inline Property::Property(std::string name, PropertyType type,
{
}

inline Property::Property(std::string name, PropertyType type,
Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This constructor is redundant.

Copy link
Copy Markdown
Contributor Author

@cmelchior cmelchior Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is being used by tests and not having it results in:

Undefined symbols for architecture x86_64:
  "realm::Property::Property(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >, realm::PropertyType, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> >)", referenced from:
      ____C_A_T_C_H____T_E_S_T____0() in list.cpp.o
      ____C_A_T_C_H____T_E_S_T____3() in migrations.cpp.o
      ____C_A_T_C_H____T_E_S_T____111() in migrations.cpp.o
      ____C_A_T_C_H____T_E_S_T____134() in migrations.cpp.o
      ____C_A_T_C_H____T_E_S_T____0() in object.cpp.o
      ____C_A_T_C_H____T_E_S_T____3() in object_store.cpp.o
      ____C_A_T_C_H____T_E_S_T____102() in results.cpp.o

Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is functionally identical to the three-argument version with a default value for the third argument (i.e. what previously existed).

Copy link
Copy Markdown
Contributor Author

@cmelchior cmelchior Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those was changed as well, these constructors have no default values anymore as having two 3-argument constructors with different defaults is ambiguous. So there is now a constructor for creating backlinks (with a string default value) and one for creating normal links (with a relationship default value)

Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having Property(std::string name, PropertyType type, std::string object_type, std::string link_origin_property_name=""); and Property(std::string name, PropertyType type, std::string object_type, Relationship relationship); is not ambiguous.

Copy link
Copy Markdown
Contributor Author

@cmelchior cmelchior Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, like that. No, your right.

Copy link
Copy Markdown
Contributor Author

@cmelchior cmelchior Feb 16, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2-arg constructor has been removed

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)
Expand All @@ -214,6 +242,16 @@ inline Property::Property(std::string name, PropertyType type,
{
}

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)
{
}

inline bool Property::type_is_indexable() const
{
return type == PropertyType::Int
Expand Down
4 changes: 4 additions & 0 deletions src/schema.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, &current_prop});
}
if(target_prop->relationship != current_prop.relationship) {
Copy link
Copy Markdown
Member

@tgoyne tgoyne Jan 12, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space between keyword and (

changes.emplace_back(schema_change::ChangeRelationshipType{&existing_schema, &current_prop});
}
}

if (existing_schema.primary_key != target_schema.primary_key) {
Expand Down Expand Up @@ -246,6 +249,7 @@ 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)
Expand Down
6 changes: 6 additions & 0 deletions src/schema.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) \
Expand All @@ -139,6 +144,7 @@ struct ChangePrimaryKey {
macro(AddIndex) \
macro(RemoveIndex) \
macro(ChangePrimaryKey) \
macro(ChangeRelationshipType) \

class SchemaChange {
public:
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ set(HEADERS
set(SOURCES
collection_change_indices.cpp
index_set.cpp
link_types.cpp
list.cpp
main.cpp
migrations.cpp
Expand Down
Loading