Provide SqlDriver as a parameter to AfterVersion

[ahmedre]

In order to run an update, a SqlDriver is required in order to actually
run update queries within an AfterVersion. Due to the fact that the
callback is provided as a parameter to the AndroidSqliteDriver
constructor, this makes it difficult to get the database to do updates
without wrapping the existing AndroidSqliteDriver.Callback or a
SupportSQLiteOpenHelper.Callback instead. This patch adds the SqlDriver
as a parameter to the AfterVersion lambda to make this easier.

Fixes #2534 and fixes #2614.

[JakeWharton]

This is a binary-incompatible change and thus cannot be accepted.

[ahmedre]

Fair point - updating with a version that should be binary compatible - am open to redo-ing this with another method / approach if there's another approach that the maintainers prefer to go with.

[ahmedre]

this should be backward compatible safely - the kotlinx binary-compatibility-validator says that this is just the addition of another constructor (despite changing the "primary" constructor, since there isn't a primary constructor in Java).

[ahmedre]

one thing i thought about was maybe we could make this a BeforeVersion instead, but then it would be confusing (why use BeforeVersion instead of AfterVersion and why aren't they the same). Not happy with the name either, but naming is hard.

[ahmedre]

the access to afterVersion and block are internal in AfterVersion, so wanted to have this so AndroidSqliteDriver could also wrap.

[AlecKazakova]

looks good to me, I think I'd change the name to AfterVersionWithDriver, we're gearing up for a release so I'll check out this branch and make that change so this gets in

[AlecKazakova]

diff --git a/drivers/android-driver/src/main/java/com/squareup/sqldelight/android/AndroidSqliteDriver.kt b/drivers/android-driver/src/main/java/com/squareup/sqldelight/android/AndroidSqliteDriver.kt
--- drivers/android-driver/src/main/java/com/squareup/sqldelight/android/AndroidSqliteDriver.kt
+++ drivers/android-driver/src/main/java/com/squareup/sqldelight/android/AndroidSqliteDriver.kt
@@ -10,9 +10,9 @@
 import androidx.sqlite.db.SupportSQLiteStatement
 import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
 import com.squareup.sqldelight.Transacter
 import com.squareup.sqldelight.db.AfterVersion
-import com.squareup.sqldelight.db.AfterVersionWithParameter
+import com.squareup.sqldelight.db.AfterVersionWithDriver
 import com.squareup.sqldelight.db.SqlCursor
 import com.squareup.sqldelight.db.SqlDriver
 import com.squareup.sqldelight.db.SqlDriver.Schema
 import com.squareup.sqldelight.db.SqlPreparedStatement
@@ -155,20 +155,20 @@
   }
 
   open class Callback(
     private val schema: SqlDriver.Schema,
-    vararg callbacks: AfterVersionWithParameter,
+    vararg callbacks: AfterVersionWithDriver,
   ) : SupportSQLiteOpenHelper.Callback(schema.version) {
     private val callbacks = callbacks
 
     constructor(
       schema: SqlDriver.Schema
-    ) : this(schema, *emptyArray<AfterVersionWithParameter>())
+    ) : this(schema, *emptyArray<AfterVersionWithDriver>())
 
     constructor(
       schema: SqlDriver.Schema,
       vararg callbacks: AfterVersion
-    ) : this(schema, *callbacks.map { it.toAfterVersionWithParameter() }.toTypedArray() )
+    ) : this(schema, *callbacks.map { it.toAfterVersionWithDriver() }.toTypedArray() )
 
     override fun onCreate(db: SupportSQLiteDatabase) {
       schema.create(AndroidSqliteDriver(openHelper = null, database = db, cacheSize = 1))
     }
diff --git a/docs/common/migrations.md b/docs/common/migrations.md
--- docs/common/migrations.md
+++ docs/common/migrations.md
@@ -33,16 +33,16 @@
     AfterVersion(3) { database.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
 )

-Alternatively, it is often useful to receive the SqlDriver as a parameter. In those cases, you can use AfterVersionWithParameter class instead:
+Alternatively, it is often useful to receive the SqlDriver as a parameter. In those cases, you can use AfterVersionWithDriver class instead:

Database.Schema.migrateWithCallbacks(
    driver = database,
    oldVersion = 0,
    newVersion = Database.Schema.version,
-    AfterVersionWithParameter(3) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
+    AfterVersionWithDriver(3) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
)

In the following example, if you have 1.sqm, 2.sqm, 3.sqm, 4.sqm, and 5.sqm as migrations, the above callback will happen after 3.sqm completes when the database is on version 4. After the callback it will resume at 4.sqm and complete the remaining migrations, in this case 4.sqm and 5.sqm, meaning the final database version is 6.
diff --git a/runtime/src/commonMain/kotlin/com/squareup/sqldelight/db/SqlDriver.kt b/runtime/src/commonMain/kotlin/com/squareup/sqldelight/db/SqlDriver.kt
--- runtime/src/commonMain/kotlin/com/squareup/sqldelight/db/SqlDriver.kt
+++ runtime/src/commonMain/kotlin/com/squareup/sqldelight/db/SqlDriver.kt
@@ -104,18 +104,18 @@

• Represents a block of code [block] that should be executed during a migration after the migration
• has finished migrating to [afterVersion]. Unlike [AfterVersion], this version's lambda accepts a
• [SqlDriver] as a parameter to make migrations easier.
  */
  -class AfterVersionWithParameter(
  +class AfterVersionWithDriver(
  internal val afterVersion: Int,
  internal val block: (SqlDriver) -> Unit
  )

/**

• • Wrap an [AfterVersion] as an [AfterVersionWithParameter].

• • Wrap an [AfterVersion] as an [AfterVersionWithDriver].
    */
    -fun AfterVersion.toAfterVersionWithParameter() =

• AfterVersionWithParameter(afterVersion) { block() }
  +fun AfterVersion.toAfterVersionWithDriver() =

• AfterVersionWithDriver(afterVersion) { block() }

/**

• Run [SqlDriver.Schema.migrate] normally but execute [callbacks] during the migration whenever
• the it finished upgrading to a version specified by [AfterVersion.afterVersion]. This method
  @@ -126,22 +126,22 @@
  oldVersion: Int,
  newVersion: Int,
  vararg callbacks: AfterVersion
  ) {

• val wrappedCallbacks = callbacks.map { it.toAfterVersionWithParameter() }.toTypedArray()

• val wrappedCallbacks = callbacks.map { it.toAfterVersionWithDriver() }.toTypedArray()
  migrateWithCallbacks(driver, oldVersion, newVersion, *wrappedCallbacks)
  }

/**

• Run [SqlDriver.Schema.migrate] normally but execute [callbacks] during the migration whenever
• the it finished upgrading to a version specified by [AfterVersion.afterVersion]. This method

• • takes [AfterVersionWithParameter] callbacks, which receive a [SqlDriver] parameter when invoked.

• • takes [AfterVersionWithDriver] callbacks, which receive a [SqlDriver] parameter when invoked.
    */
    fun SqlDriver.Schema.migrateWithCallbacks(
    driver: SqlDriver,
    oldVersion: Int,
    newVersion: Int,

• vararg callbacks: AfterVersionWithParameter

• vararg callbacks: AfterVersionWithDriver
  ) {
  var lastVersion = oldVersion

  // For each callback within the [oldVersion..newVersion) range, alternate between migrating
  diff --git a/sqldelight-gradle-plugin/src/test/integration-migration-callbacks/src/test/java/com/squareup/sqldelight/integration/IntegrationTests.kt b/sqldelight-gradle-plugin/src/test/integration-migration-callbacks/src/test/java/com/squareup/sqldelight/integration/IntegrationTests.kt
  --- sqldelight-gradle-plugin/src/test/integration-migration-callbacks/src/test/java/com/squareup/sqldelight/integration/IntegrationTests.kt
  +++ sqldelight-gradle-plugin/src/test/integration-migration-callbacks/src/test/java/com/squareup/sqldelight/integration/IntegrationTests.kt
  @@ -1,9 +1,9 @@
  package com.squareup.sqldelight.integration

import com.google.common.truth.Truth.assertThat
import com.squareup.sqldelight.db.AfterVersion
-import com.squareup.sqldelight.db.AfterVersionWithParameter
+import com.squareup.sqldelight.db.AfterVersionWithDriver
import com.squareup.sqldelight.db.migrateWithCallbacks
import com.squareup.sqldelight.sqlite.driver.JdbcSqliteDriver
import com.squareup.sqldelight.sqlite.driver.JdbcSqliteDriver.Companion.IN_MEMORY
import org.junit.Test
@@ -34,11 +34,11 @@
QueryWrapper.Schema.migrateWithCallbacks(
driver = database,
oldVersion = 0,
newVersion = QueryWrapper.Schema.version,

•  AfterVersionWithParameter(1) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
  

•  AfterVersionWithParameter(2) { it.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
  

•  AfterVersionWithParameter(3) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello3', 'sup3')", 0) },
  

•  AfterVersionWithDriver(1) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
  

•  AfterVersionWithDriver(2) { it.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
  

•  AfterVersionWithDriver(3) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello3', 'sup3')", 0) },
  

  )

  val queryWrapper = QueryWrapper(database)
  val values = queryWrapper.testQueries.selectAll().executeAsList()
  @@ -83,11 +83,11 @@
  QueryWrapper.Schema.migrateWithCallbacks(
  driver = database,
  oldVersion = 2,
  newVersion = QueryWrapper.Schema.version,

•  AfterVersionWithParameter(1) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
  

•  AfterVersionWithParameter(2) { it.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
  

•  AfterVersionWithParameter(3) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello3', 'sup3')", 0) },
  

•  AfterVersionWithDriver(1) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
  

•  AfterVersionWithDriver(2) { it.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
  

•  AfterVersionWithDriver(3) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello3', 'sup3')", 0) },
  

  )

  val queryWrapper = QueryWrapper(database)
  val values = queryWrapper.testQueries.selectAll().executeAsList()
  @@ -123,12 +123,12 @@
  QueryWrapper.Schema.migrateWithCallbacks(
  driver = database,
  oldVersion = 0,
  newVersion = QueryWrapper.Schema.version,

•  AfterVersionWithParameter(1) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
  

•  AfterVersionWithParameter(2) { it.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
  

•  AfterVersionWithParameter(3) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello3', 'sup3')", 0) },
  

•  AfterVersionWithParameter(4) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello4', 'sup4')", 0) },
  

•  AfterVersionWithDriver(1) { it.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
  

•  AfterVersionWithDriver(2) { it.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
  

•  AfterVersionWithDriver(3) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello3', 'sup3')", 0) },
  

•  AfterVersionWithDriver(4) { it.execute(null, "INSERT INTO test2 (new_value, new_value2) VALUES('hello4', 'sup4')", 0) },
  

  )

  val queryWrapper = QueryWrapper(database)
  val values = queryWrapper.testQueries.selectAll().executeAsList()

[AlecKazakova]

LOL

[AlecKazakova]

trying to commit from vscode web did not work

[AlecKazakova]

are you able to make the change quickly to AfterVersionWithDriver

[ahmedre]

sure, on it

[ahmedre]

should be ready now.

[AlecKazakova]

just need to run ./gradlew spotlessApply

[AlecKazakova]

ran it

[ahmedre]

Thanks just missed it by a few seconds 😂