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.

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

Add automatic resolution of embedded object constraints during migration #1473

Merged

Conversation

rorbech
Copy link
Contributor

@rorbech rorbech commented Aug 1, 2023

This adds configuration option to enable automatic handling of embedded object constraints during automatic migration through:

class RealmConfiguration {

    class Builder {
        /**
         * Sets the migration to handle schema updates with automatic migration of data.
         *
         * @param migration the [AutomaticSchemaMigration] instance to handle schema and data
         * migration in the event of a schema update.
         * @param resolveEmbeddedObjectConstraints a flag to indicate whether realm should resolve
         * embedded object constraints after migration. If this is `true` then all embedded objects
         * without a parent will be deleted and every embedded object with multiple references to it
         * will be duplicated so that every referencing object will hold its own copy of the
         * embedded object.
         *
         * @see RealmMigration
         * @see AutomaticSchemaMigration
         */
        public fun migration(
            migration: AutomaticSchemaMigration,
            resolveEmbeddedObjectConstraints: Boolean = false
        ): Builder =
            apply {
                this.migration = migration
                this.automaticEmbeddedObjectConstraintsResolution = resolveEmbeddedObjectConstraints
            }
    }
}

The above API was chosen in favor of adding additional properties/methods to the AutomaticSchemaMigration interface, as this would require users to instantiate it as anynomous instances and that seems very tedious compared to the standalone functional interface. Alternative it could also have been wrapped in a completely new type, but that just bloats the name space.

Closes #1464

@rorbech rorbech marked this pull request as ready for review August 9, 2023 08:18
@sipersso
Copy link

sipersso commented Aug 9, 2023

An observation here. Coming from the SwiftSDK and Java SDK. It seems as if for those SDK:s the resolveEmbeddedObjectConstraints defaults to true. Is there any reason why this shouldn't default to true for the Kotlin SDK?

@rorbech
Copy link
Contributor Author

rorbech commented Aug 9, 2023

@sipersso The feature wasn't available in the C-API layer for core when launching and we didn't caught that as a required use case for the initial release. Now we cannot just update the default behavior. I will rethink if we can introduce this in a way that makes it the new default, e.g. new method or something like that.

@sipersso
Copy link

sipersso commented Aug 9, 2023

Ok... well as long as I can enable it I guess it isn't an issue. But people migrating from Java to Kotlin might get a nasty surprise if they are not aware of this difference in implementations.

Copy link
Contributor

@cmelchior cmelchior left a comment

Choose a reason for hiding this comment

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

Nice 🚀 .. I was a bit surprised to see the duplicated method, but I assume it is because we need to constrain it to the AutomaticRealmMigration interface rather than the RealmMigration interface?

@rorbech
Copy link
Contributor Author

rorbech commented Aug 14, 2023

Nice 🚀 .. I was a bit surprised to see the duplicated method, but I assume it is because we need to constrain it to the AutomaticRealmMigration interface rather than the RealmMigration interface?

Yes. The option will probably only make sense for an automatic migration, so didn't want to add it to the base method.

@rorbech rorbech merged commit 64f1096 into main Aug 15, 2023
@rorbech rorbech deleted the cr/automatic-handling-of-embedded-constraints-on-migrations branch August 15, 2023 09:47
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Orphaned embedded objects are not deleted when migrating from Object to Embedded Object
3 participants