Scheme version/migration configuration clarifications

realm/realm-library/src/androidTest/kotlin/io/realm/RealmMigrationTestsKotlin.kt

@@ -0,0 +1,163 @@
+/*
+ * Copyright 2020 Realm Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.realm
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import io.realm.entities.StringAndInt
+import io.realm.entities.StringOnly
+import io.realm.exceptions.RealmMigrationNeededException
+import io.realm.rule.TestRealmConfigurationFactory
+import junit.framework.Assert.assertEquals
+import junit.framework.Assert.assertFalse
+import org.junit.After
+import org.junit.Before
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+import kotlin.test.assertFailsWith
+import kotlin.test.assertTrue
+
+@RunWith(AndroidJUnit4::class)
+class RealmMigrationTestsKotlin {
+
+    @get:Rule
+    val configFactory = TestRealmConfigurationFactory()
+
+    @Before
+    fun setup() {
+    }
+
+    @After
+    fun tearDown() {
+    }
+
+    private open class TestRealmMigration : RealmMigration {
+        var triggered: Boolean = false
+        var oldVersion: Long = 0
+        var newVersion: Long = 0
+        override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
+            this.triggered = true
+            this.oldVersion = oldVersion
+            this.newVersion = newVersion
+        }
+    }
+
+    // Test that
+    // - supplying updated version and migration for schema change we actually trigger migration
+    //   with out errors
+    @Test
+    fun standardMigration() {
+        val configV1: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .schema(StringAndInt::class.java)
+                .schemaVersion(1).build()
+        Realm.getInstance(configV1).close()
+
+        val migration = object : TestRealmMigration() {
+            override fun migrate(realm: DynamicRealm, oldVersion: Long, newVersion: Long) {
+                super.migrate(realm, oldVersion, newVersion)
+                realm.schema.create("StringOnly")
+                        .addField("chars", String::class.java);
+            }
+        }
+        val configV2: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .schemaVersion(2)
+                .schema(StringAndInt::class.java, StringOnly::class.java)
+                .migration(migration)
+                .build()
+        Realm.getInstance(configV2).close()
+
+        assertTrue(migration.triggered)
+    }
+
+    // Test that
+    // - we never reach migration f version is not specified, not even when triggering
+    //   RealmMigrationNeededException by scheme updates
+    @Test
+    fun noMigrationWithoutVersion() {
+        val migration = TestRealmMigration()
+
+        val configDefault: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .schema(StringAndInt::class.java)
+                .migration(migration)
+                .build()
+        // No migration on initial default realm
+        Realm.getInstance(configDefault).close()
+        assertFalse(migration.triggered)
+
+        val configV0: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .migration(migration)
+                .schema(StringAndInt::class.java, StringOnly::class.java)
+                .build()
+        // No migration on actual scheme changes if we did not specify version
+        assertFailsWith<RealmMigrationNeededException> {
+            Realm.getInstance(configV0).close()
+        }
+        assertFalse(migration.triggered)
+
+        // Bump version silently
+        val realmConfigV1: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .schema(StringAndInt::class.java)
+                .schemaVersion(1)
+                .build()
+        Realm.getInstance(realmConfigV1).close()
+
+        val updatingDefaultConfig: RealmConfiguration = configFactory.createConfigurationBuilder()
+            .migration(migration)
+            .schema(StringAndInt::class.java, StringOnly::class.java)
+            .build()
+        // No migration if not specifying version on previously non-default version realm
+        assertFailsWith<IllegalArgumentException> {
+            Realm.getInstance(updatingDefaultConfig).close()
+        }
+        assertFalse(migration.triggered)
+    }
+
+    // Test that
+    // - updating version number without migration and schema change will silently accept new
+    //   scheme version
+    @Test
+    fun silentVersionUpdate() {
+        val realmConfigV0: RealmConfiguration = configFactory.createConfigurationBuilder().build()
+        Realm.getInstance(realmConfigV0).close()
+        val newVersion = 2L
+        val realmConfigV1: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .schemaVersion(newVersion)
+                .build()
+        Realm.getInstance(realmConfigV1).use {
+            assertEquals(newVersion, it.version)
+        }
+    }
+
+    // Test that
+    // - migration is invoked when updating version, even though scheme has not been updated
+    @Test
+    fun migrationOnSchemaVersionOnlyUpdate() {
+        val realmConfigV0: RealmConfiguration = configFactory.createConfigurationBuilder().build()
+        Realm.getInstance(realmConfigV0).close()
+        val newVersion = 2L
+        val migration = TestRealmMigration()
+        val realmConfigV1: RealmConfiguration = configFactory.createConfigurationBuilder()
+                .schemaVersion(newVersion)
+                .migration(migration)
+                .build()
+        Realm.getInstance(realmConfigV1).use {
+            assertEquals(newVersion, it.version)
+        }
+        assertTrue(migration.triggered)
+    }
+
+}

realm/realm-library/src/main/java/io/realm/RealmConfiguration.java

@@ -468,6 +468,8 @@ boolean isSyncConfiguration() {
      * RealmConfiguration.Builder used to construct instances of a RealmConfiguration in a fluent manner.
      */
     public static class Builder {
+        private static long DEFAULT_SCHEMA_VERSION = 0;
+
         // IMPORTANT: When adding any new methods to this class also add them to SyncConfiguration.
         private File directory;
         private String fileName;
@@ -510,7 +512,7 @@ private void initializeBuilder(Context context) {
             this.directory = context.getFilesDir();
             this.fileName = Realm.DEFAULT_REALM_NAME;
             this.key = null;
-            this.schemaVersion = 0;
+            this.schemaVersion = DEFAULT_SCHEMA_VERSION;
             this.migration = null;
             this.deleteRealmIfMigrationNeeded = false;
             this.durability = OsRealmConfig.Durability.FULL;
@@ -578,14 +580,28 @@ public Builder encryptionKey(byte[] key) {
         }
 
         /**
-         * Sets the schema version of the Realm. This must be equal to or higher than the schema version of the existing
-         * Realm file, if any. If the schema version is higher than the already existing Realm, a migration is needed.
+         * Sets the schema version of the Realm.
+         * <p>
+         * This must be equal to or higher than the schema version of the existing Realm file, if any.
          * <p>
-         * If no migration code is provided, Realm will throw a
+         * If the schema version is higher than the already existing Realm it will be updated given
+         * that any actual schema changes are handled properly by the {@link RealmMigration} supplied
+         * through {@link #migration(RealmMigration)}. If the schema has not changed, the version
+         * is still updated and {@link RealmMigration#migrate(DynamicRealm, long, long)} is notified
+         * if set through {@link #migration(RealmMigration)}.
+         * <p>
+         * If the schema has changed without increasing the version or failing to resolve schema
+         * migration through the supplied {@link RealmMigration}, Realm will throw a
          * {@link io.realm.exceptions.RealmMigrationNeededException}.
          *
+         * @throws IllegalArgumentException If {@code schemaVersion} is negative.
+         *
+         * @see #schemaVersion(long, RealmMigration)
          * @see #migration(RealmMigration)
+         *
+         * @deprecated Use {@link #schemaVersion(long, RealmMigration)} instead.
          */
+        @Deprecated
         public Builder schemaVersion(long schemaVersion) {
             if (schemaVersion < 0) {
                 throw new IllegalArgumentException("Realm schema version numbers must be 0 (zero) or higher. Yours was: " + schemaVersion);
@@ -594,11 +610,46 @@ public Builder schemaVersion(long schemaVersion) {
             return this;
         }
 
+        /**
+         * Update the schema version of the Realm and apply the provided migration.
+         * <p>
+         * The schema version must be equal to or higher than the schema version of the existing
+         * Realm file, if any, and the supplied {@link RealmMigration} must handled any schema changes.
+         * For <i>version only</i> updates the supplied
+         * {@link RealmMigration#migrate(DynamicRealm, long, long)} can be a <i>no operation</i>.
+         * <p>
+         * If the schema has changed without increasing the version or failing to resolve schema
+         * migration through the supplied {@link RealmMigration},, Realm will throw a
+         * {@link io.realm.exceptions.RealmMigrationNeededException}.
+         *
+         * @throws IllegalArgumentException If {@code schemaVersion} is negative or {@code migration} is null.
+         *
+         * @see #schemaVersion(long)
+         * @see #migration(RealmMigration)
+         */
+        public Builder schemaVersion(long schemaVersion, RealmMigration migration) {
+            if (schemaVersion < 0) {
+                throw new IllegalArgumentException("Realm schema version numbers must be 0 (zero) or higher. Yours was: " + schemaVersion);
+            }
+            if (migration == null) {
+                throw new IllegalArgumentException("Migration cannot be null");
+            }
+            this.schemaVersion = schemaVersion;
+            this.migration = migration;
+            return this;
+        }
+
         /**
          * Sets the {@link io.realm.RealmMigration} to be run if a migration is needed. If this migration fails to
          * upgrade the on-disc schema to the runtime schema, a {@link io.realm.exceptions.RealmMigrationNeededException}
          * will be thrown.
+         * <p>
+         * FIXME Also triggered on version update even though no schema change
+         * FIXME Not triggered if no version number is specified.
+         *
+         * @deprecated Use {@link #schemaVersion(long, RealmMigration)} instead.
          */
+        @Deprecated
         public Builder migration(RealmMigration migration) {
             //noinspection ConstantConditions
             if (migration == null) {
@@ -832,6 +883,11 @@ final Builder schema(Class<? extends RealmModel> firstClass, Class<? extends Rea
          * @return the created {@link RealmConfiguration}.
          */
         public RealmConfiguration build() {
+            // FIXME Do we require that a migration is present when updating version...which would
+            //  be a breaking change
+            //if (schemaVersion != DEFAULT_SCHEMA_VERSION && migration == null) {
+            //    throw new IllegalStateException("Providing a scheme version also requires a migration");
+            //}
             // Check that readOnly() was applied to legal configuration. Right now it should only be allowed if
             // an assetFile is configured
             if (readOnly) {