Skip to content

Commit

Permalink
Correct migration callback AfterVersion range filter (#2394)
Browse files Browse the repository at this point in the history
Introduced in 1.5.0, SQLDelight had an off-by-one error
for determining when to invoke an AfterMigration callback.
This commit corrects the version range for which callbacks
are invoked.

Closes #2393.
  • Loading branch information
Kevin Cianfarini authored and AlecKazakova committed Nov 23, 2021
1 parent 5b70246 commit 601b290
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,9 @@ fun SqlDriver.Schema.migrateWithCallbacks(
) {
var lastVersion = oldVersion

// For each callback within the oldVersion..newVersion range, alternate between migrating
// For each callback within the [oldVersion..newVersion) range, alternate between migrating
// the schema and invoking each callback.
callbacks.filter { it.afterVersion in (oldVersion + 1) until newVersion }
callbacks.filter { it.afterVersion in oldVersion until newVersion }
.sortedBy { it.afterVersion }
.forEach { callback ->
migrate(driver, oldVersion = lastVersion, newVersion = callback.afterVersion + 1)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package com.squareup.sqldelight.logs

import com.squareup.sqldelight.db.AfterVersion
import com.squareup.sqldelight.db.SqlDriver
import com.squareup.sqldelight.db.migrateWithCallbacks
import kotlin.test.Test
import kotlin.test.assertFalse
import kotlin.test.assertTrue

class SqlDriverMigrationCallbackTest {

@Test fun migrationCallbackInvokedOnCorrectVersion() {
val schema = fakeSchema()

var callbackInvoked = false

schema.migrateWithCallbacks(
driver = FakeSqlDriver(),
oldVersion = 1,
newVersion = 2,
AfterVersion(1) { callbackInvoked = true }
)

assertTrue(callbackInvoked)
}

@Test fun migrationCallbacks() {
val schema = fakeSchema()

var callback1Invoked = false
var callback2Invoked = false
var callback3Invoked = false

schema.migrateWithCallbacks(
driver = FakeSqlDriver(),
oldVersion = 0,
newVersion = 4,
AfterVersion(1) { callback1Invoked = true },
AfterVersion(2) { callback2Invoked = true },
AfterVersion(3) { callback3Invoked = true },
)

assertTrue(callback1Invoked)
assertTrue(callback2Invoked)
assertTrue(callback3Invoked)
}

@Test fun migrationCallbackLessThanOldVersionNotCalled() {
val schema = fakeSchema()

var callback1Invoked = false
var callback2Invoked = false
var callback3Invoked = false

schema.migrateWithCallbacks(
driver = FakeSqlDriver(),
oldVersion = 2,
newVersion = 4,
AfterVersion(1) { callback1Invoked = true },
AfterVersion(2) { callback2Invoked = true },
AfterVersion(3) { callback3Invoked = true },
)

assertFalse(callback1Invoked)
assertTrue(callback2Invoked)
assertTrue(callback3Invoked)
}

@Test fun migrationCallbackGreaterThanNewVersionNotCalled() {
val schema = fakeSchema()

var callback1Invoked = false
var callback2Invoked = false
var callback3Invoked = false
var callback4Invoked = false

schema.migrateWithCallbacks(
driver = FakeSqlDriver(),
oldVersion = 0,
newVersion = 4,
AfterVersion(1) { callback1Invoked = true },
AfterVersion(2) { callback2Invoked = true },
AfterVersion(3) { callback3Invoked = true },
AfterVersion(4) { callback4Invoked = true },
)

assertTrue(callback1Invoked)
assertTrue(callback2Invoked)
assertTrue(callback3Invoked)
assertFalse(callback4Invoked)
}

private fun fakeSchema() = object : SqlDriver.Schema {
override val version: Int = 1
override fun create(driver: SqlDriver) = Unit
override fun migrate(driver: SqlDriver, oldVersion: Int, newVersion: Int) = Unit
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ class IntegrationTests {
QueryWrapper.Schema.migrate(
driver = database,
oldVersion = 0,
newVersion = 1
newVersion = 2
)
QueryWrapper.Schema.migrateWithCallbacks(
driver = database,
oldVersion = 1,
oldVersion = 2,
newVersion = QueryWrapper.Schema.version,
AfterVersion(1) { database.execute(null, "INSERT INTO test (value) VALUES('hello')", 0) },
AfterVersion(2) { database.execute(null, "INSERT INTO test2 (value, value2) VALUES('hello2', 'sup2')", 0) },
Expand Down

0 comments on commit 601b290

Please sign in to comment.