DataConnectCacheDatabaseMigrator.kt, DataConnectCacheDatabaseMigratorUnitTest.kt: Improve schema version error handling

Improves the developer experience by providing more detailed and helpful
error messages when the database schema version is invalid or not set.
This change helps prevent potential data corruption by aborting migrations
on databases with unknown schema versions.

### Changes
* DataConnectCacheDatabaseMigrator.kt:
    * Updated the exception messages for invalid or null schema versions to be more descriptive.
* DataConnectCacheDatabaseMigratorUnitTest.kt:
    * Added unit tests to verify that an `InvalidSchemaVersionException` is thrown for unknown schema versions.
    * Added a unit test to verify that an `InvalidSchemaVersionException` is thrown for a null schema version.

Author: dconeybe

firebase-dataconnect/src/main/kotlin/com/google/firebase/dataconnect/sqlite2/DataConnectCacheDatabaseMigrator.kt

@@ -183,7 +183,11 @@ private constructor(private val sqliteDatabase: SQLiteDatabase, private val logg
     // The PATCH version changes for all other non-schema-affecting changes, such as adding an
     // index to an existing table.
     return if (schemaVersion === null) {
-      throw InvalidSchemaVersionException("schema_version is null")
+      throw InvalidSchemaVersionException(
+        "schema_version is null or not set;" +
+          " expected a value that starts with \"1.\";" +
+          " aborting to avoid corrupting the contents of the database"
+      )
     } else if (schemaVersion == "1.0.0") {
       RunMigrationStepResult.StepExecuted("1.1.0").apply {
         logger.debug { "migrating to schema version $newSchemaVersion from $schemaVersion" }
@@ -192,7 +196,11 @@ private constructor(private val sqliteDatabase: SQLiteDatabase, private val logg
     } else if (schemaVersion.startsWith("1.")) {
       RunMigrationStepResult.NoMore
     } else {
-      throw InvalidSchemaVersionException("unsupported schema_version: $schemaVersion")
+      throw InvalidSchemaVersionException(
+        "schema_version $schemaVersion is unknown;" +
+          " expected a value that starts with \"1.\";" +
+          " aborting to avoid corrupting the contents of the database"
+      )
     }
   }
 

firebase-dataconnect/src/test/kotlin/com/google/firebase/dataconnect/sqlite2/DataConnectCacheDatabaseMigratorUnitTest.kt

@@ -16,24 +16,36 @@
 
 package com.google.firebase.dataconnect.sqlite2
 
+import android.database.sqlite.SQLiteDatabase
 import com.google.firebase.dataconnect.core.Logger
 import com.google.firebase.dataconnect.sqlite2.DataConnectCacheDatabaseMigrator.InvalidApplicationIdException
+import com.google.firebase.dataconnect.sqlite2.DataConnectCacheDatabaseMigrator.InvalidSchemaVersionException
 import com.google.firebase.dataconnect.sqlite2.DataConnectCacheDatabaseMigrator.InvalidUserVersionException
 import com.google.firebase.dataconnect.sqlite2.SQLiteDatabaseExts.getApplicationId
 import com.google.firebase.dataconnect.sqlite2.SQLiteDatabaseExts.setApplicationId
 import com.google.firebase.dataconnect.testutil.DataConnectLogLevelRule
 import com.google.firebase.dataconnect.testutil.shouldContainWithNonAbuttingText
 import com.google.firebase.dataconnect.testutil.shouldContainWithNonAbuttingTextIgnoringCase
 import com.google.firebase.dataconnect.util.StringUtil.to0xHexString
+import io.github.z4kn4fein.semver.toVersion
 import io.kotest.assertions.assertSoftly
 import io.kotest.assertions.throwables.shouldThrow
+import io.kotest.assertions.withClue
 import io.kotest.common.ExperimentalKotest
+import io.kotest.matchers.nulls.shouldNotBeNull
 import io.kotest.matchers.shouldBe
 import io.kotest.property.Arb
 import io.kotest.property.EdgeConfig
 import io.kotest.property.PropTestConfig
+import io.kotest.property.arbitrary.Codepoint
+import io.kotest.property.arbitrary.alphanumeric
+import io.kotest.property.arbitrary.bind
+import io.kotest.property.arbitrary.choice
+import io.kotest.property.arbitrary.filterNot
 import io.kotest.property.arbitrary.int
-import io.kotest.property.arbitrary.next
+import io.kotest.property.arbitrary.map
+import io.kotest.property.arbitrary.nonNegativeInt
+import io.kotest.property.arbitrary.string
 import io.kotest.property.assume
 import io.kotest.property.checkAll
 import io.mockk.mockk
@@ -161,6 +173,100 @@ class DataConnectCacheDatabaseMigratorUnitTest {
     }
   }
 
+  @Test
+  fun `migrate() schema_version should set the value in a new database`() {
+    val mockLogger: Logger = mockk(relaxed = true)
+
+    val schemaVersion =
+      DataConnectSQLiteDatabaseOpener.open(dbFile, mockLogger).use { db ->
+        DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+        getSchemaVersion(db)
+      }
+
+    val parsedSchemaVersion =
+      withClue("schemaVersion") { schemaVersion.shouldNotBeNull().toVersion() }
+    withClue("parsedSchemaVersion.major") { parsedSchemaVersion.major shouldBe 1 }
+  }
+
+  @Test
+  fun `migrate() schema_version should leave value alone if already set`() {
+    val mockLogger: Logger = mockk(relaxed = true)
+    DataConnectSQLiteDatabaseOpener.open(dbFile, mockLogger).use { db ->
+      DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+      val schemaVersion1 = withClue("getSchemaVersion1") { getSchemaVersion(db).shouldNotBeNull() }
+      DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+      val schemaVersion2 = withClue("getSchemaVersion2") { getSchemaVersion(db).shouldNotBeNull() }
+      schemaVersion1 shouldBe schemaVersion2
+    }
+  }
+
+  @Test
+  fun `migrate() schema_version should throw if the schema version is invalid`() = runTest {
+    val invalidMajorVersionArb = Arb.int(2..Int.MAX_VALUE).map { "$it.2.3" }
+    val invalidSemanticVersionArb =
+      Arb.string(0..10, Codepoint.alphanumeric()).filterNot { it.startsWith("1.") }
+    val invalidSchemaVersionArb = Arb.choice(invalidMajorVersionArb, invalidSemanticVersionArb)
+    checkAll(propTestConfig, invalidSchemaVersionArb) { schemaVersion ->
+      val mockLogger: Logger = mockk(relaxed = true)
+
+      val exception =
+        DataConnectSQLiteDatabaseOpener.open(null, mockLogger).use { db ->
+          DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+          setSchemaVersion(db, schemaVersion)
+
+          shouldThrow<InvalidSchemaVersionException> {
+            DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+          }
+        }
+
+      assertSoftly {
+        exception.message shouldContainWithNonAbuttingText "schema_version"
+        exception.message shouldContainWithNonAbuttingText schemaVersion
+        exception.message shouldContainWithNonAbuttingTextIgnoringCase "unknown"
+        exception.message shouldContainWithNonAbuttingTextIgnoringCase "aborting"
+      }
+    }
+  }
+
+  @Test
+  fun `migrate() schema_version should throw if the schema version is not set`() {
+    val mockLogger: Logger = mockk(relaxed = true)
+
+    val exception =
+      DataConnectSQLiteDatabaseOpener.open(null, mockLogger).use { db ->
+        DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+        unsetSchemaVersion(db)
+
+        shouldThrow<InvalidSchemaVersionException> {
+          DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+        }
+      }
+
+    assertSoftly {
+      exception.message shouldContainWithNonAbuttingText "schema_version"
+      exception.message shouldContainWithNonAbuttingTextIgnoringCase "null"
+      exception.message shouldContainWithNonAbuttingTextIgnoringCase "aborting"
+    }
+  }
+
+  @Test
+  fun `migrate() schema_version should accept higher minor and-or patch versions`() = runTest {
+    val mockLogger: Logger = mockk(relaxed = true)
+    DataConnectSQLiteDatabaseOpener.open(dbFile, mockLogger).use { db ->
+      DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+      val originalSchemaVersion =
+        withClue("getSchemaVersion") { getSchemaVersion(db).shouldNotBeNull() }
+      checkAll(propTestConfig, higherMinorAndOrPatchVersionArb(originalSchemaVersion)) {
+        newSchemaVersion ->
+        setSchemaVersion(db, newSchemaVersion)
+
+        DataConnectCacheDatabaseMigrator.migrate(db, mockLogger)
+
+        getSchemaVersion(db) shouldBe newSchemaVersion
+      }
+    }
+  }
+
   private companion object {
 
     @OptIn(ExperimentalKotest::class)
@@ -169,5 +275,44 @@ class DataConnectCacheDatabaseMigratorUnitTest {
         iterations = 10,
         edgeConfig = EdgeConfig(edgecasesGenerationProbability = 0.33)
       )
+
+    fun getSchemaVersion(db: SQLiteDatabase): String? {
+      db.rawQuery("SELECT text FROM metadata WHERE name = 'schema_version'", null).use { cursor ->
+        return if (cursor.moveToNext()) cursor.getString(0) else null
+      }
+    }
+
+    fun setSchemaVersion(db: SQLiteDatabase, value: String) {
+      db.execSQL(
+        "INSERT OR REPLACE INTO metadata (name, text) VALUES ('schema_version', ?)",
+        arrayOf(value)
+      )
+    }
+
+    fun unsetSchemaVersion(db: SQLiteDatabase) {
+      db.execSQL("DELETE FROM metadata WHERE name = 'schema_version'")
+    }
+
+    /**
+     * Creates and returns an [Arb] that generates whose values are semantic versions that have the
+     * same major version as the given version, but a higher minor and/or patch version.
+     */
+    fun higherMinorAndOrPatchVersionArb(version: String): Arb<String> {
+      val parsedVersion = version.toVersion(strict = false)
+
+      val higherMinorVersionArb =
+        Arb.bind(Arb.int(parsedVersion.minor + 1..Int.MAX_VALUE), Arb.nonNegativeInt()) {
+          minor,
+          patch ->
+          parsedVersion.copy(minor = minor, patch = patch).toString()
+        }
+
+      val higherPatchVersionArb =
+        Arb.int(parsedVersion.patch + 1..Int.MAX_VALUE).map { patch ->
+          parsedVersion.copy(patch = patch).toString()
+        }
+
+      return Arb.choice(higherMinorVersionArb, higherPatchVersionArb)
+    }
   }
 }