Realm-java: Add hint to documentation that RealmMigration implementation should override equals() and hashCode()

[djuelg]

Problem

I don't understand the reason, but under some circumstances concerning multithreading the following exception will not be printed:

java.lang.IllegalArgumentException: Configurations cannot be different if used to open the same file. The most likely cause is that equals() and hashCode() are not overridden in the migration class: package.MigrationClass
                                                       at io.realm.RealmCache.validateConfiguration(RealmCache.java:450)
                                                       at io.realm.RealmCache.doCreateRealmOrGetFromCache(RealmCache.java:339)
                                                       at io.realm.RealmCache.createRealmOrGetFromCache(RealmCache.java:281)
                                                       at io.realm.Realm.getInstance(Realm.java:346)

Instead the background thread will get terminated without any feedback. I produced this stacktrace by accident, while making a call to my realm database on the main thread instead of on a background thread. Even if you can't figure out the reason for this either it would be awesome to have an hint in the documentation that an implementation of "RealmMigration" should override equals and hashcode. This would have saved me a couple of hours :)

Greets,
djuelg

Version of Realm and tooling

Realm version(s): 3.7.2

Realm sync feature enabled: no

Android Studio version: 2.3.3

Which Android version and device: OnePlus 3T (API 25), Emulator (API26)

[cmelchior]

The reason is that you are creating multiple instances of the same RealmConfiguration most likely doing something like:

// This is done twice
RealmConfiguration config = new RealmConfiguration.Builder()
  .migration(new MyMigration())
  .schemaVersion(2)
  .build();

Internally we compare the configuration with any existing ones used to open the same Realm and new MyMigration().equals(new MyMigration()) is false unless you override equals and hashcode, so theoretically it could be two totally different implementations.

It is really an anti-pattern to generate the configuration twice since it is immutable, so you should just store a reference to the first one you generate.

I do agree that would could probably document this better.

[beeender]

@cmelchior do we really have to disallow different migration blocks for the "same" Realm config? it feels harmless to allow that.

[cmelchior]

It is often a source of confusion, I agree, which is also why the exception is so explicit about it, but consider the following case:

// This is done twice
RealmConfiguration config1 = new RealmConfiguration.Builder()
  .migration(new MyMigration(true))
  .schemaVersion(2)
  .build();

RealmConfiguration config2 = new RealmConfiguration.Builder()
  .migration(new MyMigration(false))
  .schemaVersion(2)
  .build();

public class MyMigration implements RealmMigration {

   private final boolean ignore;

   public MyMigration(boolean ignore) {
     this.ignore = ignore;
   }

  @Override
  public void migrate(DynamicRealm, long oldVersion, long newVersion) {
    if (!ignore) {
       // Run migration
    }

  }

Realm realm1 = Realm.getInstance(config1);
Realm realm2 = Realm.getInstance(config2); // throws currently
}

This is of course constructed but shows that you might parse in two different implementations of a migration if we just compare e.g. the class. Some really subtle bugs could happen because of this.

I don't recall ever seeing a RealmMigration class with state like that though, so one could argue that we should just check for the class and document the corner case.

On the other hand, people should not be creating multiple RealmConfigurations in the first place.

[djuelg]

Thank's for your help. I'm storing the RealmConfigurations in my database class. And I don't use multiple RealmConfigurations. What I do is calling Realm realm = Realm.getInstance(this.config); multiple times. Is this an anit-pattern, too?

Further information about concrete implementation here: https://github.com/djuelg/Neuronizer/blob/master/app/src/main/java/de/djuelg/neuronizer/storage/PreviewRepositoryImpl.java

[Zhuinden]

https://github.com/djuelg/Neuronizer/blob/master/app/src/main/java/de/djuelg/neuronizer/storage/PreviewRepositoryImpl.java

[Zhuinden]

Migration is an interface, and the RealmConfiguration checks against equality of two RealmConfigurations.

If one wants to make two migrations equal, it makes sense to override hashCode and equals accordingly.