Upgrading to Kotlin 1.8.20 throws Property "x" has been required migration error for private properties

alashow commented 1 year ago

How frequently does the bug occur?

Always

Description

I was upgrading to Kotlin 1.8.20 from 1.8.10 and the app was crashing with RealmMigrationNeededException:

Property "ModelDO.id" has been made required.

I checked the history of our ModelDO.id field and apparently it was changed from val id: String? = null to val id: String = "".

Writing a migration solves this:

schema.get(ModelDO::class.java.simpleName)
                ?.setNullable("id", false)

Stacktrace & log output

No response

Can you reproduce the bug?

Always

Reproduction Steps

I didn't try exactly to reproduce this on a clean setup yet, but simple steps are:

Create database model with val id: String? = null field on Kotlin 1.8.10
Run the app with above database
Change the field to val id: String = "" on Kotlin 1.8.10
Run the app again to see if it's working without migration
Compile with Kotlin 1.8.20
Run the app and expect described migration required error

Version

10.15.1

What Atlas App Services are you using?

Local Database only

Are you using encryption?

No

Platform OS and version(s)

Android 13

Build environment

Android Studio version: 2022.2.1 RC 1 Android Build Tools version: 8.0.0 Gradle version: 8.0.0

cmelchior commented 1 year ago

Hi @alashow

I tried to reproduce this, but it failed. On Kotlin 1.8.10 I correctly got the migration error on step 4. I tried to reproduce it on our Kotlin sample here: https://github.com/realm/realm-java/tree/main/examples/kotlinExample

Make sure that it uses Kotlin 1.8.10
Compile and run the sample on an emulator
Press the red stop button in Android Studio.
Change Person.name to String? instead of String
Press run again
See it crashes on startup with a migration error:

 Caused by: io.realm.exceptions.RealmMigrationNeededException: Migration is required due to the following errors:  
    - Property 'Person.name' has been made optional.

My guess is that you maybe hit a cache when testing this and so accidentally reinstalled something with the same schema?

bmarty commented 1 year ago

I have the issue too. Adding a migration https://github.com/vector-im/element-android/pull/8401/commits/9b240c3f16e5e15ce11e46d2ae511d46eb0d5025 fixes the issue (I need to add some fields to the migration though, the tests are still not passing)

claytongreen commented 1 year ago

I'm running into the same issue upgrading from 1.8.10 to 1.8.21 and created a repo to demonstrate the issue.

Instructions are in the readme: https://github.com/claytongreen/realm-java-7810

rorbech commented 1 year ago

I have been investigating and it seems like Kotlin is not generating the org.jetbrains.annotations.NotNull annotations that we rely on to derive nullability on private fields prior to Kotlin 1.8.20. This means that private properties have erroneously been marked as nullable when built with Kotlin version lower that 1.8.20.

I will have to instigate the details ... and consider the consequences and potential fixes of this behavior, but for now I can't really see a different option than actually doing a migration that updates the nullability for the affected properties. At least there shouldn't be any null-values to clear out and reason about as the compiler would have been enforcing not to set it to null, so it is only a matter of changing the nullability-property of the fields.

rorbech commented 1 year ago

There doesn't seems to be a good way around this, where we would not introduce further headaches along the way or cause potential new issues reasoning about the progression of the schema. So seems safest to leave as is. This means that the only option is to fix the mismatch of the old and new schema by:

Add the actual migration that would change the affected fields to be non nullable with something like

     RealmMigration migration = (realm, oldVersion, newVersion) -> {
        if (oldVersion < NEW_VERSION) {
            RealmSchema schema = realm.getSchema();
            RealmObjectSchema objectSchema = schema.get(CLASS_NAME);
            objectSchema.setRequired(FIELD_NAME, true);
        }
    };

    RealmConfiguration config = RealmConfiguration.Builder()
            .schemaVersion(NEW_VERSION)
            .migration(migration)
            .build();

The alternative would be to update the modelled properties to be nullable in the class definition, but this would unfortunately be reflected in the type system of the classes.

realm / realm-java