Assorted fixes for migrations (#112)

* Make AndroidxSqliteConfiguration immutable

* Only apply pragmas to already created reader connections

* Rename ConfigurableDatabase

* Code style update

* Some documentation updates

* Remove unused import

* Disable foreign keys while creating or migrating the database

* Add tests for ensuring connections wait for schema creation or migration

* Access statementsCache through a lock

  - This used to be an LruCache which is synchronized internally
  - Now that it's a HashMap we need to synchronize it ourselves

* Add tests for foreign key behavior during creation/migration

* Fix migrateEmptySchema

Author: eygraber

.idea/codeStyles/Project.xml

@@ -80,10 +80,29 @@ Database(
 
 It will handle calling the `create` and `migrate` functions on your schema for you, and keep track of the database's version.
 
+## Foreign Key Constraints
+
+When using `AndroidxSqliteDriver`, the handling of foreign key constraints during database creation and migration is 
+managed to ensure data integrity.
+
+If you have foreign key constraints enabled in your
+`AndroidxSqliteConfiguration` (i.e. `isForeignKeyConstraintsEnabled = true`),
+the driver will automatically disable them before executing the schema `create` or `migrate` operations.
+This is done to prevent issues with table creation order and data manipulation during the migration process.
+
+After the creation or migration is complete, foreign key constraints are re-enabled.
+
+Furthermore, to verify the integrity of the foreign key relationships after these operations,
+the driver performs an additional check. If `isForeignKeyConstraintsCheckedAfterCreateOrUpdate`
+is `true` (which it is by default), a `PRAGMA foreign_key_check` is executed. If this check finds
+any violations, an `AndroidxSqliteDriver.ForeignKeyConstraintCheckException` is thrown, detailing the 
+specific constraints that have been violated. This helps catch any inconsistencies in your data that might
+have been introduced during the migration.
+
 ## Connection Pooling
 
-By default, one connection will be used for both reading and writing, and only one thread can acquire that connection at a time.
-If you have WAL enabled, you could (and should) set the amount of pooled reader connections that will be used:
+By default, one connection will be used for both reading and writing, and only one thread can acquire that connection 
+at a time. If you have WAL enabled, you could (and should) set the amount of pooled reader connections that will be used:
 
 ```kotlin
 AndroidxSqliteDriver(

README.md

@@ -1,5 +1,3 @@
-import com.android.build.api.dsl.ManagedVirtualDevice
-
 plugins {
   id("com.eygraber.conventions-kotlin-multiplatform")
   id("com.eygraber.conventions-android-library")

library/build.gradle.kts

@@ -42,13 +42,16 @@ actual class CommonDriverTest : AndroidxSqliteDriverTest()
 actual class CommonDriverOpenFlagsTest : AndroidxSqliteDriverOpenFlagsTest()
 
 @RunWith(RobolectricTestRunner::class)
-actual class CommonQueryTest : AndroidxSqliteQueryTest()
+actual class CommonEphemeralTest : AndroidxSqliteEphemeralTest()
 
 @RunWith(RobolectricTestRunner::class)
-actual class CommonTransacterTest : AndroidxSqliteTransacterTest()
+actual class CommonMigrationKeyTest : AndroidxSqliteMigrationKeyTest()
 
 @RunWith(RobolectricTestRunner::class)
-actual class CommonEphemeralTest : AndroidxSqliteEphemeralTest()
+actual class CommonQueryTest : AndroidxSqliteQueryTest()
+
+@RunWith(RobolectricTestRunner::class)
+actual class CommonTransacterTest : AndroidxSqliteTransacterTest()
 
 actual fun androidxSqliteTestDriver(): SQLiteDriver = AndroidSQLiteDriver()
 

library/src/androidUnitTest/kotlin/com/eygraber/sqldelight/androidx/driver/AndroidxSqliteCommonTests.android.kt

@@ -4,7 +4,7 @@ import app.cash.sqldelight.db.QueryResult
 import app.cash.sqldelight.db.SqlCursor
 import app.cash.sqldelight.db.SqlPreparedStatement
 
-public class ConfigurableDatabase(
+public class AndroidxSqliteConfigurableDriver(
   private val driver: AndroidxSqliteDriver,
 ) {
   public fun setForeignKeyConstraintsEnabled(isForeignKeyConstraintsEnabled: Boolean) {

library/src/commonMain/kotlin/com/eygraber/sqldelight/androidx/driver/AndroidxSqliteHelpers.kt renamed to library/src/commonMain/kotlin/com/eygraber/sqldelight/androidx/driver/AndroidxSqliteConfigurableDriver.kt

@@ -8,6 +8,7 @@ public enum class SqliteJournalMode(internal val value: String) {
   Truncate("TRUNCATE"),
   Persist("PERSIST"),
   Memory("MEMORY"),
+
   @Suppress("EnumNaming")
   WAL("WAL"),
   Off("OFF"),
@@ -35,19 +36,32 @@ public class AndroidxSqliteConfiguration(
    *
    * Default is false.
    */
-  public var isForeignKeyConstraintsEnabled: Boolean = false,
+  public val isForeignKeyConstraintsEnabled: Boolean = false,
+  /**
+   * When true, a `PRAGMA foreign_key_check` is performed after the schema is created or migrated.
+   *
+   * This is only useful when [isForeignKeyConstraintsEnabled] is true.
+   *
+   * During schema creation and migration, foreign key constraints are temporarily disabled.
+   * This check ensures that after the schema operations are complete, all foreign key constraints are satisfied.
+   * If any violations are found, a [AndroidxSqliteDriver.ForeignKeyConstraintCheckException]
+   * is thrown with details about the violations.
+   *
+   * Default is true.
+   */
+  public val isForeignKeyConstraintsCheckedAfterCreateOrUpdate: Boolean = true,
   /**
    * Journal mode to use.
    *
    * Default is [SqliteJournalMode.WAL].
    */
-  public var journalMode: SqliteJournalMode = SqliteJournalMode.WAL,
+  public val journalMode: SqliteJournalMode = SqliteJournalMode.WAL,
   /**
    * Synchronous mode to use.
    *
    * Default is [SqliteSync.Full] unless [journalMode] is set to [SqliteJournalMode.WAL] in which case it is [SqliteSync.Normal].
    */
-  public var sync: SqliteSync = when(journalMode) {
+  public val sync: SqliteSync = when(journalMode) {
     SqliteJournalMode.WAL -> SqliteSync.Normal
     SqliteJournalMode.Delete,
     SqliteJournalMode.Truncate,
@@ -67,4 +81,18 @@ public class AndroidxSqliteConfiguration(
     SqliteJournalMode.WAL -> 4
     else -> 0
   },
-)
+) {
+  public fun copy(
+    isForeignKeyConstraintsEnabled: Boolean = this.isForeignKeyConstraintsEnabled,
+    journalMode: SqliteJournalMode = this.journalMode,
+    sync: SqliteSync = this.sync,
+  ): AndroidxSqliteConfiguration =
+    AndroidxSqliteConfiguration(
+      cacheSize = cacheSize,
+      isForeignKeyConstraintsEnabled = isForeignKeyConstraintsEnabled,
+      isForeignKeyConstraintsCheckedAfterCreateOrUpdate = isForeignKeyConstraintsCheckedAfterCreateOrUpdate,
+      journalMode = journalMode,
+      sync = sync,
+      readerConnectionsCount = readerConnectionsCount,
+    )
+}

library/src/commonMain/kotlin/com/eygraber/sqldelight/androidx/driver/AndroidxSqliteConfiguration.kt

@@ -15,8 +15,10 @@ import app.cash.sqldelight.db.SqlDriver
 import app.cash.sqldelight.db.SqlPreparedStatement
 import app.cash.sqldelight.db.SqlSchema
 import kotlinx.atomicfu.atomic
+import kotlinx.atomicfu.locks.ReentrantLock
 import kotlinx.atomicfu.locks.SynchronizedObject
 import kotlinx.atomicfu.locks.synchronized
+import kotlinx.atomicfu.locks.withLock
 
 internal expect class TransactionsThreadLocal() {
   internal fun get(): Transacter.Transaction?
@@ -37,7 +39,17 @@ public class AndroidxSqliteDriver(
   private val schema: SqlSchema<QueryResult.Value<Unit>>,
   private val configuration: AndroidxSqliteConfiguration = AndroidxSqliteConfiguration(),
   private val migrateEmptySchema: Boolean = false,
-  private val onConfigure: ConfigurableDatabase.() -> Unit = {},
+  /**
+   * A callback to configure the database connection when it's first opened.
+   *
+   * This lambda is invoked on the first interaction with the database, immediately before the schema
+   * is created or migrated. It provides an [AndroidxSqliteConfigurableDriver] as its receiver
+   * to allow for safe configuration of connection properties like journal mode or foreign key
+   * constraints.
+   *
+   * **Warning:** The [AndroidxSqliteConfigurableDriver] receiver is ephemeral and **must not** escape the callback.
+   */
+  private val onConfigure: AndroidxSqliteConfigurableDriver.() -> Unit = {},
   private val onCreate: AndroidxSqliteDriver.() -> Unit = {},
   private val onUpdate: AndroidxSqliteDriver.(Long, Long) -> Unit = { _, _ -> },
   private val onOpen: AndroidxSqliteDriver.() -> Unit = {},
@@ -50,7 +62,17 @@ public class AndroidxSqliteDriver(
     schema: SqlSchema<QueryResult.Value<Unit>>,
     configuration: AndroidxSqliteConfiguration = AndroidxSqliteConfiguration(),
     migrateEmptySchema: Boolean = false,
-    onConfigure: ConfigurableDatabase.() -> Unit = {},
+    /**
+     * A callback to configure the database connection when it's first opened.
+     *
+     * This lambda is invoked on the first interaction with the database, immediately before the schema
+     * is created or migrated. It provides an [AndroidxSqliteConfigurableDriver] as its receiver
+     * to allow for safe configuration of connection properties like journal mode or foreign key
+     * constraints.
+     *
+     * **Warning:** The [AndroidxSqliteConfigurableDriver] receiver is ephemeral and **must not** escape the callback.
+     */
+    onConfigure: AndroidxSqliteConfigurableDriver.() -> Unit = {},
     onCreate: SqlDriver.() -> Unit = {},
     onUpdate: SqlDriver.(Long, Long) -> Unit = { _, _ -> },
     onOpen: SqlDriver.() -> Unit = {},
@@ -70,6 +92,8 @@ public class AndroidxSqliteDriver(
     migrationCallbacks = migrationCallbacks,
   )
 
+  public class ForeignKeyConstraintCheckException(message: String) : Exception(message)
+
   @Suppress("NonBooleanPropertyPrefixedWithIs")
   private val isFirstInteraction = atomic(true)
 
@@ -102,24 +126,27 @@ public class AndroidxSqliteDriver(
   private val transactions = TransactionsThreadLocal()
 
   private val statementsCache = HashMap<SQLiteConnection, LruCache<Int, AndroidxStatement>>()
+  private val statementsCacheLock = ReentrantLock()
 
   private fun getStatementCache(connection: SQLiteConnection) =
-    when {
-      configuration.cacheSize > 0 ->
-        statementsCache.getOrPut(connection) {
-          object : LruCache<Int, AndroidxStatement>(configuration.cacheSize) {
-            override fun entryRemoved(
-              evicted: Boolean,
-              key: Int,
-              oldValue: AndroidxStatement,
-              newValue: AndroidxStatement?,
-            ) {
-              if(evicted) oldValue.close()
+    statementsCacheLock.withLock {
+      when {
+        configuration.cacheSize > 0 ->
+          statementsCache.getOrPut(connection) {
+            object : LruCache<Int, AndroidxStatement>(configuration.cacheSize) {
+              override fun entryRemoved(
+                evicted: Boolean,
+                key: Int,
+                oldValue: AndroidxStatement,
+                newValue: AndroidxStatement?,
+              ) {
+                if(evicted) oldValue.close()
+              }
             }
           }
-        }
 
-      else -> null
+        else -> null
+      }
     }
 
   private var skipStatementsCache = true
@@ -132,7 +159,7 @@ public class AndroidxSqliteDriver(
   /**
    * True if foreign key constraints are enabled.
    *
-   * This function will block until all connections have been updated.
+   * This function will block until all created connections have been updated.
    *
    * An exception will be thrown if this is called from within a transaction.
    */
@@ -147,7 +174,7 @@ public class AndroidxSqliteDriver(
   /**
    * Journal mode to use.
    *
-   * This function will block until all connections have been updated.
+   * This function will block until all created connections have been updated.
    *
    * An exception will be thrown if this is called from within a transaction.
    */
@@ -162,7 +189,7 @@ public class AndroidxSqliteDriver(
   /**
    * Synchronous mode to use.
    *
-   * This function will block until all connections have been updated.
+   * This function will block until all created connections have been updated.
    *
    * An exception will be thrown if this is called from within a transaction.
    */
@@ -383,7 +410,7 @@ public class AndroidxSqliteDriver(
         if(isFirstInteraction.value && !isNestedUnderCreateOrMigrate) {
           isNestedUnderCreateOrMigrate = true
 
-          ConfigurableDatabase(this).onConfigure()
+          AndroidxSqliteConfigurableDriver(this).onConfigure()
 
           val writerConnection = connectionPool.acquireWriterConnection()
           val currentVersion = try {
@@ -397,21 +424,24 @@ public class AndroidxSqliteDriver(
             connectionPool.releaseWriterConnection()
           }
 
-          if(currentVersion == 0L && !migrateEmptySchema || currentVersion < schema.version) {
+          val isCreate = currentVersion == 0L && !migrateEmptySchema
+          if(isCreate || currentVersion < schema.version) {
             val driver = this
             val transacter = object : TransacterImpl(driver) {}
 
-            transacter.transaction {
-              when(currentVersion) {
-                0L -> schema.create(driver).value
-                else -> schema.migrate(driver, currentVersion, schema.version, *migrationCallbacks).value
-              }
-              skipStatementsCache = configuration.cacheSize == 0
-              when(currentVersion) {
-                0L -> onCreate()
-                else -> onUpdate(currentVersion, schema.version)
+            writerConnection.withDeferredForeignKeyChecks(configuration) {
+              transacter.transaction {
+                when {
+                  isCreate -> schema.create(driver).value
+                  else -> schema.migrate(driver, currentVersion, schema.version, *migrationCallbacks).value
+                }
+                skipStatementsCache = configuration.cacheSize == 0
+                when {
+                  isCreate -> onCreate()
+                  else -> onUpdate(currentVersion, schema.version)
+                }
+                writerConnection.prepare("PRAGMA user_version = ${schema.version}").use { it.step() }
               }
-              writerConnection.prepare("PRAGMA user_version = ${schema.version}").use { it.step() }
             }
           } else {
             skipStatementsCache = configuration.cacheSize == 0
@@ -426,6 +456,52 @@ public class AndroidxSqliteDriver(
   }
 }
 
+private inline fun SQLiteConnection.withDeferredForeignKeyChecks(
+  configuration: AndroidxSqliteConfiguration,
+  block: () -> Unit,
+) {
+  if(configuration.isForeignKeyConstraintsEnabled) {
+    prepare("PRAGMA foreign_keys = OFF;").use(SQLiteStatement::step)
+  }
+
+  block()
+
+  if(configuration.isForeignKeyConstraintsEnabled) {
+    prepare("PRAGMA foreign_keys = ON;").use(SQLiteStatement::step)
+
+    if(configuration.isForeignKeyConstraintsCheckedAfterCreateOrUpdate) {
+      prepare("PRAGMA foreign_key_check;").use { check ->
+        val violations = mutableListOf<String>()
+        while(check.step()) {
+          val referencingTable = check.getText(0)
+          val referencingRowId = check.getInt(1)
+          val referencedTable = check.getText(2)
+          val referencingConstraintIndex = check.getInt(3)
+
+          violations.add(
+            """
+            |Constraint index: $referencingConstraintIndex 
+            |Referencing table: $referencingTable
+            |Referencing rowId: $referencingRowId
+            |Referenced table: $referencedTable
+            """.trimMargin(),
+          )
+        }
+
+        if(violations.isNotEmpty()) {
+          throw AndroidxSqliteDriver.ForeignKeyConstraintCheckException(
+            """
+            |The following foreign key constraints are violated:
+            |
+            |${violations.joinToString(separator = "\n\n")}
+            """.trimMargin(),
+          )
+        }
+      }
+    }
+  }
+}
+
 internal interface AndroidxStatement : SqlPreparedStatement {
   fun execute(): Long
   fun <R> executeQuery(mapper: (SqlCursor) -> QueryResult<R>): R