Automatic handling backlinks migration #5737
Conversation
There was a problem hiding this comment.
Are we sure this is the API we want to expose to users? Particularly, I'm wondering if it's better to have this as a function that the user explicitly invokes, similarly to rename_property.
Alternatively, if we keep it as a property on the config, should the default be false or true? I can see some value in opting in for true as that would make sure such migrations work automatically, but I can understand the hesitance as it may mean deleting user data.
In relation to add a new function. Yes, I could add a new function in object_store and propagate it in the CAPI. However, its usage is kind of meaningless outside the migration callback itself. I mean, if there are objects that are not respecting the constraints, core will throw an exception before client code could do anything meaningful with this new API. I personally think that a new setting makes more sense in this case (but I am open to whatever we decide to do) ... and yes there is some value in letting core fix things automatically, but deleting user data should not be done without explicitly asking for it. |
|
@ironage I haven't added the logic for constructing the list of embedded backlinks. So the PR is not yet ready to be merged. |
…c_handling_backlinks_migration
… link points to some embedded object
|
I am demoting this to draft, I need to clean up the code, fix a bunch of issues and review the main functions, this is not by any means ready for review/production |
|
this is now ready for review @ironage |
…c_handling_backlinks_migration
* move converters to core level * use object_converters in migration * handle mixed non-pk objects in converter
… Obj::assign" This reverts commit f9169a9.
| dst_link = m_opposite_of_dst->get_object(src_link_key); | ||
| } | ||
| else { | ||
| // in different Realms we create a new object |
There was a problem hiding this comment.
Is it relevant to support a case where we copy between 2 different realms in case the object does not have a primary key? I guess it will be hard to create a test case for this?
There was a problem hiding this comment.
I would not add a test for this right now, I don't think this could happen within the context of a schema migration.
| converted_src = ObjLink{dst_link_table->get_key(), dst_link.get_key()}; | ||
| } | ||
| else { | ||
| // no pk, and different Realm, create an object |
There was a problem hiding this comment.
I would say we could try to add a test covering this case later, I am not sure if this could happen within the context of a schema migration. We are reusing this code mostly because I ended up rewriting the same logic we already have in place for client reset.
|
FIY: Just tried this in the Kotlin SDK and the orphaned embedded objects did not get deleted automatically. I have a unit test for this and the app just crashes. I created a separate issue here: realm/realm-kotlin#1464 |
What, How & Why?
Automatically handle backlinks migration from TopLevel ==> Embedded objects.
In case of 0 backlinks, delete automatically the object
In case of multiple backlinks, duplicate the object and assign 1 parent to each object.
Fixes: #4729
Fixes: #3898
☑️ ToDos