Rewrite Migrations (#577)

* Rewrite Migrations

* Fix Detekt errors

* Do migrations synchronous

* Filter and sort migrations

* Review changes

* Review changes 2

* Fix Detekt errors

app/src/main/java/eu/kanade/tachiyomi/Migrations.kt

@@ -1,69 +0,0 @@
-package eu.kanade.tachiyomi
-
-import android.content.Context
-import eu.kanade.domain.source.service.SourcePreferences
-import eu.kanade.tachiyomi.data.backup.create.BackupCreateJob
-import eu.kanade.tachiyomi.data.library.LibraryUpdateJob
-import kotlinx.coroutines.CoroutineScope
-import kotlinx.coroutines.Dispatchers
-import logcat.LogPriority
-import mihon.domain.extensionrepo.exception.SaveExtensionRepoException
-import mihon.domain.extensionrepo.repository.ExtensionRepoRepository
-import tachiyomi.core.common.preference.Preference
-import tachiyomi.core.common.preference.PreferenceStore
-import tachiyomi.core.common.util.lang.launchIO
-import tachiyomi.core.common.util.system.logcat
-
-object Migrations {
-
-    /**
-     * Performs a migration when the application is updated.
-     *
-     * @return true if a migration is performed, false otherwise.
-     */
-    @Suppress("SameReturnValue", "MagicNumber")
-    fun upgrade(
-        context: Context,
-        preferenceStore: PreferenceStore,
-        sourcePreferences: SourcePreferences,
-        extensionRepoRepository: ExtensionRepoRepository,
-    ): Boolean {
-        val lastVersionCode = preferenceStore.getInt(Preference.appStateKey("last_version_code"), 0)
-        val oldVersion = lastVersionCode.get()
-        if (oldVersion < BuildConfig.VERSION_CODE) {
-            lastVersionCode.set(BuildConfig.VERSION_CODE)
-
-            // Always set up background tasks to ensure they're running
-            LibraryUpdateJob.setupTask(context)
-            BackupCreateJob.setupTask(context)
-
-            // Fresh install
-            if (oldVersion == 0) {
-                return false
-            }
-
-            val coroutineScope = CoroutineScope(Dispatchers.IO)
-
-            if (oldVersion < 7) {
-                coroutineScope.launchIO {
-                    for ((index, source) in sourcePreferences.extensionRepos().get().withIndex()) {
-                        try {
-                            extensionRepoRepository.upsertRepo(
-                                source,
-                                "Repo #${index + 1}",
-                                null,
-                                source,
-                                "NOFINGERPRINT-${index + 1}",
-                            )
-                        } catch (e: SaveExtensionRepoException) {
-                            logcat(LogPriority.ERROR, e) { "Error Migrating Extension Repo with baseUrl: $source" }
-                        }
-                    }
-                    sourcePreferences.extensionRepos().delete()
-                }
-            }
-        }
-
-        return false
-    }
-}

app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt

@@ -50,8 +50,6 @@ import cafe.adriel.voyager.navigator.NavigatorDisposeBehavior
 import cafe.adriel.voyager.navigator.currentOrThrow
 import com.google.accompanist.systemuicontroller.rememberSystemUiController
 import eu.kanade.domain.base.BasePreferences
-import eu.kanade.domain.source.service.SourcePreferences
-import eu.kanade.domain.ui.UiPreferences
 import eu.kanade.presentation.components.AppStateBanners
 import eu.kanade.presentation.components.DownloadedOnlyBannerBackgroundColor
 import eu.kanade.presentation.components.IncognitoModeBannerBackgroundColor
@@ -61,7 +59,6 @@ import eu.kanade.presentation.more.settings.screen.data.RestoreBackupScreen
 import eu.kanade.presentation.util.AssistContentScreen
 import eu.kanade.presentation.util.DefaultNavigatorScreenTransition
 import eu.kanade.tachiyomi.BuildConfig
-import eu.kanade.tachiyomi.Migrations
 import eu.kanade.tachiyomi.data.cache.ChapterCache
 import eu.kanade.tachiyomi.data.download.DownloadCache
 import eu.kanade.tachiyomi.data.notification.NotificationReceiver
@@ -89,7 +86,11 @@ import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.onEach
 import kotlinx.coroutines.launch
 import logcat.LogPriority
+import mihon.core.migration.Migrator
+import mihon.core.migration.migrations.migrations
 import tachiyomi.core.common.Constants
+import tachiyomi.core.common.preference.Preference
+import tachiyomi.core.common.preference.PreferenceStore
 import tachiyomi.core.common.util.lang.launchIO
 import tachiyomi.core.common.util.system.logcat
 import tachiyomi.domain.library.service.LibraryPreferences
@@ -105,9 +106,7 @@ import androidx.compose.ui.graphics.Color.Companion as ComposeColor
 
 class MainActivity : BaseActivity() {
 
-    private val sourcePreferences: SourcePreferences by injectLazy()
     private val libraryPreferences: LibraryPreferences by injectLazy()
-    private val uiPreferences: UiPreferences by injectLazy()
     private val preferences: BasePreferences by injectLazy()
 
     private val downloadCache: DownloadCache by injectLazy()
@@ -130,16 +129,7 @@ class MainActivity : BaseActivity() {
 
         super.onCreate(savedInstanceState)
 
-        val didMigration = if (isLaunch) {
+        val didMigration = migrate()
-            Migrations.upgrade(
-                context = applicationContext,
-                preferenceStore = Injekt.get(),
-                sourcePreferences = Injekt.get(),
-                extensionRepoRepository = Injekt.get(),
-            )
-        } else {
-            false
-        }
 
         // Do not let the launcher create a new activity http://stackoverflow.com/questions/16283079
         if (!isTaskRoot) {
@@ -350,6 +340,21 @@ class MainActivity : BaseActivity() {
         }
     }
 
+    private fun migrate(): Boolean {
+        val preferenceStore = Injekt.get<PreferenceStore>()
+        val preference = preferenceStore.getInt(Preference.appStateKey("last_version_code"), 0)
+        logcat { "Migration from ${preference.get()} to ${BuildConfig.VERSION_CODE}" }
+        return Migrator.migrate(
+            old = preference.get(),
+            new = BuildConfig.VERSION_CODE,
+            migrations = migrations,
+            onMigrationComplete = {
+                logcat { "Updating last version to ${BuildConfig.VERSION_CODE}" }
+                preference.set(BuildConfig.VERSION_CODE)
+            },
+        )
+    }
+
     /**
      * Sets custom splash screen exit animation on devices prior to Android 12.
      *

app/src/main/java/mihon/core/migration/Migration.kt

@@ -0,0 +1,19 @@
+package mihon.core.migration
+
+interface Migration {
+    val version: Float
+
+    suspend operator fun invoke(migrationContext: MigrationContext): Boolean
+
+    companion object {
+        const val ALWAYS = -1f
+
+        fun of(version: Float, action: suspend (MigrationContext) -> Boolean): Migration = object : Migration {
+            override val version: Float = version
+
+            override suspend operator fun invoke(migrationContext: MigrationContext): Boolean {
+                return action(migrationContext)
+            }
+        }
+    }
+}

app/src/main/java/mihon/core/migration/MigrationContext.kt

@@ -0,0 +1,10 @@
+package mihon.core.migration
+
+import uy.kohesive.injekt.Injekt
+
+class MigrationContext {
+
+    inline fun <reified T> get(): T? {
+        return Injekt.getInstanceOrNull(T::class.java)
+    }
+}

app/src/main/java/mihon/core/migration/Migrator.kt

@@ -0,0 +1,53 @@
+package mihon.core.migration
+
+import kotlinx.coroutines.runBlocking
+import tachiyomi.core.common.util.system.logcat
+
+object Migrator {
+
+    @SuppressWarnings("ReturnCount")
+    fun migrate(
+        old: Int,
+        new: Int,
+        migrations: List<Migration>,
+        dryrun: Boolean = false,
+        onMigrationComplete: () -> Unit
+    ): Boolean {
+        val migrationContext = MigrationContext()
+
+        if (old == 0) {
+            return migrationContext.migrate(
+                migrations = migrations.filter { it.isAlways() },
+                dryrun = dryrun
+            )
+                .also { onMigrationComplete() }
+        }
+
+        if (old >= new) {
+            return false
+        }
+
+        return migrationContext.migrate(
+            migrations = migrations.filter { it.isAlways() || it.version.toInt() in (old + 1)..new },
+            dryrun = dryrun
+        )
+            .also { onMigrationComplete() }
+    }
+
+    private fun Migration.isAlways() = version == Migration.ALWAYS
+
+    @SuppressWarnings("MaxLineLength")
+    private fun MigrationContext.migrate(migrations: List<Migration>, dryrun: Boolean): Boolean {
+        return migrations.sortedBy { it.version }
+            .map { migration ->
+                if (!dryrun) {
+                    logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" }
+                    runBlocking { migration(this@migrate) }
+                } else {
+                    logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" }
+                    true
+                }
+            }
+            .reduce { acc, b -> acc || b }
+    }
+}

app/src/main/java/mihon/core/migration/migrations/Migrations.kt

@@ -0,0 +1,10 @@
+package mihon.core.migration.migrations
+
+import mihon.core.migration.Migration
+
+val migrations: List<Migration>
+    get() = listOf(
+        SetupBackupCreateMigration(),
+        SetupLibraryUpdateMigration(),
+        TrustExtensionRepositoryMigration(),
+    )

app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt

@@ -0,0 +1,16 @@
+package mihon.core.migration.migrations
+
+import eu.kanade.tachiyomi.App
+import eu.kanade.tachiyomi.data.backup.create.BackupCreateJob
+import mihon.core.migration.Migration
+import mihon.core.migration.MigrationContext
+
+class SetupBackupCreateMigration : Migration {
+    override val version: Float = Migration.ALWAYS
+
+    override suspend fun invoke(migrationContext: MigrationContext): Boolean {
+        val context = migrationContext.get<App>() ?: return false
+        BackupCreateJob.setupTask(context)
+        return true
+    }
+}

app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt

@@ -0,0 +1,16 @@
+package mihon.core.migration.migrations
+
+import eu.kanade.tachiyomi.App
+import eu.kanade.tachiyomi.data.library.LibraryUpdateJob
+import mihon.core.migration.Migration
+import mihon.core.migration.MigrationContext
+
+class SetupLibraryUpdateMigration : Migration {
+    override val version: Float = Migration.ALWAYS
+
+    override suspend fun invoke(migrationContext: MigrationContext): Boolean {
+        val context = migrationContext.get<App>() ?: return false
+        LibraryUpdateJob.setupTask(context)
+        return true
+    }
+}

app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt

@@ -0,0 +1,35 @@
+package mihon.core.migration.migrations
+
+import eu.kanade.domain.source.service.SourcePreferences
+import logcat.LogPriority
+import mihon.core.migration.Migration
+import mihon.core.migration.MigrationContext
+import mihon.domain.extensionrepo.exception.SaveExtensionRepoException
+import mihon.domain.extensionrepo.repository.ExtensionRepoRepository
+import tachiyomi.core.common.util.lang.withIOContext
+import tachiyomi.core.common.util.system.logcat
+
+class TrustExtensionRepositoryMigration : Migration {
+    override val version: Float = 7f
+
+    override suspend fun invoke(migrationContext: MigrationContext): Boolean = withIOContext {
+        val sourcePreferences = migrationContext.get<SourcePreferences>() ?: return@withIOContext false
+        val extensionRepositoryRepository =
+            migrationContext.get<ExtensionRepoRepository>() ?: return@withIOContext false
+        for ((index, source) in sourcePreferences.extensionRepos().get().withIndex()) {
+            try {
+                extensionRepositoryRepository.upsertRepo(
+                    source,
+                    "Repo #${index + 1}",
+                    null,
+                    source,
+                    "NOFINGERPRINT-${index + 1}",
+                )
+            } catch (e: SaveExtensionRepoException) {
+                logcat(LogPriority.ERROR, e) { "Error Migrating Extension Repo with baseUrl: $source" }
+            }
+        }
+        sourcePreferences.extensionRepos().delete()
+        return@withIOContext true
+    }
+}

app/src/test/java/mihon/core/migration/MigratorTest.kt

@@ -0,0 +1,96 @@
+package mihon.core.migration
+
+import io.mockk.Called
+import io.mockk.spyk
+import io.mockk.verify
+import org.junit.jupiter.api.Assertions
+import org.junit.jupiter.api.Test
+
+class MigratorTest {
+
+    @Test
+    fun initialVersion() {
+        val onMigrationComplete: () -> Unit = {}
+        val onMigrationCompleteSpy = spyk(onMigrationComplete)
+        val didMigration = Migrator.migrate(
+            old = 0,
+            new = 1,
+            migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { false }),
+            onMigrationComplete = onMigrationCompleteSpy
+        )
+        verify { onMigrationCompleteSpy() }
+        Assertions.assertTrue(didMigration)
+    }
+
+    @Test
+    fun sameVersion() {
+        val onMigrationComplete: () -> Unit = {}
+        val onMigrationCompleteSpy = spyk(onMigrationComplete)
+        val didMigration = Migrator.migrate(
+            old = 1,
+            new = 1,
+            migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }),
+            onMigrationComplete = onMigrationCompleteSpy
+        )
+        verify { onMigrationCompleteSpy wasNot Called }
+        Assertions.assertFalse(didMigration)
+    }
+
+    @Test
+    fun smallMigration() {
+        val onMigrationComplete: () -> Unit = {}
+        val onMigrationCompleteSpy = spyk(onMigrationComplete)
+        val didMigration = Migrator.migrate(
+            old = 1,
+            new = 2,
+            migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }),
+            onMigrationComplete = onMigrationCompleteSpy
+        )
+        verify { onMigrationCompleteSpy() }
+        Assertions.assertTrue(didMigration)
+    }
+
+    @Test
+    fun largeMigration() {
+        val onMigrationComplete: () -> Unit = {}
+        val onMigrationCompleteSpy = spyk(onMigrationComplete)
+        val input = listOf(
+            Migration.of(Migration.ALWAYS) { true },
+            Migration.of(2f) { true },
+            Migration.of(3f) { true },
+            Migration.of(4f) { true },
+            Migration.of(5f) { true },
+            Migration.of(6f) { true },
+            Migration.of(7f) { true },
+            Migration.of(8f) { true },
+            Migration.of(9f) { true },
+            Migration.of(10f) { true },
+        )
+        val didMigration = Migrator.migrate(
+            old = 1,
+            new = 10,
+            migrations = input,
+            onMigrationComplete = onMigrationCompleteSpy
+        )
+        verify { onMigrationCompleteSpy() }
+        Assertions.assertTrue(didMigration)
+    }
+
+    @Test
+    fun withinRangeMigration() {
+        val onMigrationComplete: () -> Unit = {}
+        val onMigrationCompleteSpy = spyk(onMigrationComplete)
+        val didMigration = Migrator.migrate(
+            old = 1,
+            new = 2,
+            migrations = listOf(
+                Migration.of(Migration.ALWAYS) { true },
+                Migration.of(2f) { true },
+                Migration.of(3f) { false }
+            ),
+            onMigrationComplete = onMigrationCompleteSpy
+        )
+        verify { onMigrationCompleteSpy() }
+        Assertions.assertTrue(didMigration)
+    }
+}