From 860c18e56a95feba2e722622010224561c0ded75 Mon Sep 17 00:00:00 2001 From: Andreas Date: Sun, 24 Mar 2024 15:18:00 +0100 Subject: [PATCH 1/7] Rewrite Migrations --- .../java/eu/kanade/tachiyomi/Migrations.kt | 69 -------------- .../kanade/tachiyomi/ui/main/MainActivity.kt | 48 ++++++---- .../java/mihon/core/migration/Migration.kt | 19 ++++ .../mihon/core/migration/MigrationContext.kt | 10 ++ .../java/mihon/core/migration/Migrator.kt | 53 +++++++++++ .../core/migration/migrations/Migrations.kt | 10 ++ .../migrations/SetupBackupCreateMigration.kt | 16 ++++ .../migrations/SetupLibraryUpdateMigration.kt | 16 ++++ .../TrustExtensionRepositoryMigration.kt | 34 +++++++ .../java/mihon/core/migration/MigratorTest.kt | 95 +++++++++++++++++++ 10 files changed, 285 insertions(+), 85 deletions(-) delete mode 100644 app/src/main/java/eu/kanade/tachiyomi/Migrations.kt create mode 100644 app/src/main/java/mihon/core/migration/Migration.kt create mode 100644 app/src/main/java/mihon/core/migration/MigrationContext.kt create mode 100644 app/src/main/java/mihon/core/migration/Migrator.kt create mode 100644 app/src/main/java/mihon/core/migration/migrations/Migrations.kt create mode 100644 app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt create mode 100644 app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt create mode 100644 app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt create mode 100644 app/src/test/java/mihon/core/migration/MigratorTest.kt diff --git a/app/src/main/java/eu/kanade/tachiyomi/Migrations.kt b/app/src/main/java/eu/kanade/tachiyomi/Migrations.kt deleted file mode 100644 index 7bb0042e24..0000000000 --- a/app/src/main/java/eu/kanade/tachiyomi/Migrations.kt +++ /dev/null @@ -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 - } -} diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt index ae05ade4bd..ea9799acee 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt +++ b/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 @@ -80,6 +77,8 @@ import eu.kanade.tachiyomi.util.system.dpToPx import eu.kanade.tachiyomi.util.system.isNavigationBarNeedsScrim import eu.kanade.tachiyomi.util.system.openInBrowser import eu.kanade.tachiyomi.util.view.setComposeContent +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.awaitAll import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.collectLatest @@ -89,7 +88,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 +108,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 +131,7 @@ class MainActivity : BaseActivity() { super.onCreate(savedInstanceState) - val didMigration = if (isLaunch) { - Migrations.upgrade( - context = applicationContext, - preferenceStore = Injekt.get(), - sourcePreferences = Injekt.get(), - extensionRepoRepository = Injekt.get(), - ) - } else { - false - } + val migrations = migrate() // Do not let the launcher create a new activity http://stackoverflow.com/questions/16283079 if (!isTaskRoot) { @@ -250,7 +242,16 @@ class MainActivity : BaseActivity() { ShowOnboarding() } - var showChangelog by remember { mutableStateOf(didMigration && !BuildConfig.DEBUG) } + var showChangelog by remember { mutableStateOf(false) } + LaunchedEffect(Unit) { + if (migrations.isEmpty()) { + showChangelog = false + return@LaunchedEffect + } + val didMigration = awaitAll(*migrations).reduce { acc, aBoolean -> acc || aBoolean } + showChangelog = didMigration && !BuildConfig.DEBUG + } + if (showChangelog) { AlertDialog( onDismissRequest = { showChangelog = false }, @@ -350,6 +351,21 @@ class MainActivity : BaseActivity() { } } + private fun migrate(): Array> { + val preferenceStore = Injekt.get() + 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. * diff --git a/app/src/main/java/mihon/core/migration/Migration.kt b/app/src/main/java/mihon/core/migration/Migration.kt new file mode 100644 index 0000000000..8c3249d7ba --- /dev/null +++ b/app/src/main/java/mihon/core/migration/Migration.kt @@ -0,0 +1,19 @@ +package mihon.core.migration + +interface Migration { + val version: Float + + suspend fun action(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 fun action(migrationContext: MigrationContext): Boolean { + return action(migrationContext) + } + } + } +} diff --git a/app/src/main/java/mihon/core/migration/MigrationContext.kt b/app/src/main/java/mihon/core/migration/MigrationContext.kt new file mode 100644 index 0000000000..68ddf64641 --- /dev/null +++ b/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 get(): T? { + return Injekt.getInstanceOrNull(T::class.java) + } +} diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt new file mode 100644 index 0000000000..a47334a659 --- /dev/null +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -0,0 +1,53 @@ +package mihon.core.migration + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Deferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async +import tachiyomi.core.common.util.system.logcat + +object Migrator { + + fun migrate(old: Int, new: Int, migrations: List, dryrun: Boolean = false, onMigrationComplete: () -> Unit): Array> { + val migrationContext = MigrationContext() + val coroutineScope = CoroutineScope(Dispatchers.IO) + + val migrationsByVersion = migrations.groupBy { it.version.toInt() } + val always = listOf(Migration.ALWAYS.toInt()) + + if (old == 0) { + onMigrationComplete() + return with(coroutineScope) { + migrationContext.migrate(always, migrationsByVersion, dryrun) + } + } + + if (old >= new) { + return emptyArray() + } + + onMigrationComplete() + val versions = migrationsByVersion.keys.filter { version -> version in (old + 1)..new } + return with(coroutineScope) { + migrationContext.migrate(always + versions, migrationsByVersion, dryrun) + } + } + + context (CoroutineScope) + private fun MigrationContext.migrate(versions: List, migrationsByVersion: Map>, dryrun: Boolean): Array> { + return versions.sorted() + .flatMap { version -> + (migrationsByVersion[version] ?: emptyList()) + .sortedBy(Migration::version) + .map { migration -> + if (!dryrun) { + logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } + return@map async { migration.action(this@migrate) } + } + logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } + async { true } + } + } + .toTypedArray() + } +} diff --git a/app/src/main/java/mihon/core/migration/migrations/Migrations.kt b/app/src/main/java/mihon/core/migration/migrations/Migrations.kt new file mode 100644 index 0000000000..821434e2ee --- /dev/null +++ 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 + get() = listOf( + SetupBackupCreateMigration(), + SetupLibraryUpdateMigration(), + TrustExtensionRepositoryMigration(), + ) diff --git a/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt b/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt new file mode 100644 index 0000000000..0de4319708 --- /dev/null +++ b/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt @@ -0,0 +1,16 @@ +package mihon.core.migration.migrations + +import mihon.core.migration.Migration +import mihon.core.migration.MigrationContext +import eu.kanade.tachiyomi.App +import eu.kanade.tachiyomi.data.backup.create.BackupCreateJob + +class SetupBackupCreateMigration : Migration { + override val version: Float = Migration.ALWAYS + + override suspend fun action(migrationContext: MigrationContext): Boolean { + val context = migrationContext.get() ?: return false + BackupCreateJob.setupTask(context) + return true + } +} diff --git a/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt b/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt new file mode 100644 index 0000000000..767491c83c --- /dev/null +++ b/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt @@ -0,0 +1,16 @@ +package mihon.core.migration.migrations + +import mihon.core.migration.Migration +import mihon.core.migration.MigrationContext +import eu.kanade.tachiyomi.App +import eu.kanade.tachiyomi.data.library.LibraryUpdateJob + +class SetupLibraryUpdateMigration : Migration { + override val version: Float = Migration.ALWAYS + + override suspend fun action(migrationContext: MigrationContext): Boolean { + val context = migrationContext.get() ?: return false + LibraryUpdateJob.setupTask(context) + return true + } +} diff --git a/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt b/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt new file mode 100644 index 0000000000..d43c6647b2 --- /dev/null +++ b/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt @@ -0,0 +1,34 @@ +package mihon.core.migration.migrations + +import mihon.core.migration.Migration +import mihon.core.migration.MigrationContext +import eu.kanade.domain.source.service.SourcePreferences +import logcat.LogPriority +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 action(migrationContext: MigrationContext): Boolean = withIOContext { + val sourcePreferences = migrationContext.get() ?: return@withIOContext false + val extensionRepositoryRepository = migrationContext.get() ?: 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 + } +} diff --git a/app/src/test/java/mihon/core/migration/MigratorTest.kt b/app/src/test/java/mihon/core/migration/MigratorTest.kt new file mode 100644 index 0000000000..203d3e068f --- /dev/null +++ b/app/src/test/java/mihon/core/migration/MigratorTest.kt @@ -0,0 +1,95 @@ +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 spyk = spyk(onMigrationComplete) + val migrations = Migrator.migrate( + old = 0, + new = 1, + migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }), + onMigrationComplete = spyk + ) + verify { spyk() } + Assertions.assertEquals(1, migrations.size) + } + + @Test + fun sameVersion() { + val onMigrationComplete: () -> Unit = {} + val spyk = spyk(onMigrationComplete) + val migrations = Migrator.migrate( + old = 1, + new = 1, + migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }), + onMigrationComplete = spyk + ) + verify { spyk wasNot Called } + Assertions.assertEquals(0, migrations.size) + } + + @Test + fun smallMigration() { + val onMigrationComplete: () -> Unit = {} + val spyk = spyk(onMigrationComplete) + val migrations = Migrator.migrate( + old = 1, + new = 2, + migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }), + onMigrationComplete = spyk + ) + verify { spyk() } + Assertions.assertEquals(2, migrations.size) + } + + @Test + fun largeMigration() { + val onMigrationComplete: () -> Unit = {} + val spyk = 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 migrations = Migrator.migrate( + old = 1, + new = 10, + migrations = input, + onMigrationComplete = spyk + ) + verify { spyk() } + Assertions.assertEquals(10, migrations.size) + } + + @Test + fun withinRangeMigration() { + val onMigrationComplete: () -> Unit = {} + val spyk = spyk(onMigrationComplete) + val migrations = Migrator.migrate( + old = 1, + new = 2, + migrations = listOf( + Migration.of(Migration.ALWAYS) { true }, + Migration.of(2f) { true }, + Migration.of(3f) { true }), + onMigrationComplete = spyk + ) + verify { spyk() } + Assertions.assertEquals(2, migrations.size) + } +} From 28472e5b05baa803e8e96adec1d9a2c63fd9fb8b Mon Sep 17 00:00:00 2001 From: Andreas Date: Sun, 24 Mar 2024 19:22:41 +0100 Subject: [PATCH 2/7] Fix Detekt errors --- .../eu/kanade/tachiyomi/ui/main/MainActivity.kt | 1 + .../main/java/mihon/core/migration/Migrator.kt | 16 ++++++++++++++-- .../migrations/SetupBackupCreateMigration.kt | 4 ++-- .../migrations/SetupLibraryUpdateMigration.kt | 4 ++-- .../TrustExtensionRepositoryMigration.kt | 7 ++++--- .../java/mihon/core/migration/MigratorTest.kt | 3 ++- buildSrc/src/main/kotlin/detekt.gradle.kts | 2 +- 7 files changed, 26 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt index ea9799acee..7342cbc48b 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt @@ -248,6 +248,7 @@ class MainActivity : BaseActivity() { showChangelog = false return@LaunchedEffect } + @SuppressWarnings("SpreadOperator") val didMigration = awaitAll(*migrations).reduce { acc, aBoolean -> acc || aBoolean } showChangelog = didMigration && !BuildConfig.DEBUG } diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt index a47334a659..7fce38626c 100644 --- a/app/src/main/java/mihon/core/migration/Migrator.kt +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -8,7 +8,14 @@ import tachiyomi.core.common.util.system.logcat object Migrator { - fun migrate(old: Int, new: Int, migrations: List, dryrun: Boolean = false, onMigrationComplete: () -> Unit): Array> { + @SuppressWarnings("ReturnCount") + fun migrate( + old: Int, + new: Int, + migrations: List, + dryrun: Boolean = false, + onMigrationComplete: () -> Unit + ): Array> { val migrationContext = MigrationContext() val coroutineScope = CoroutineScope(Dispatchers.IO) @@ -34,7 +41,12 @@ object Migrator { } context (CoroutineScope) - private fun MigrationContext.migrate(versions: List, migrationsByVersion: Map>, dryrun: Boolean): Array> { + @SuppressWarnings("MaxLineLength") + private fun MigrationContext.migrate( + versions: List, + migrationsByVersion: Map>, + dryrun: Boolean + ): Array> { return versions.sorted() .flatMap { version -> (migrationsByVersion[version] ?: emptyList()) diff --git a/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt b/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt index 0de4319708..61291ce5ef 100644 --- a/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt +++ b/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt @@ -1,9 +1,9 @@ package mihon.core.migration.migrations -import mihon.core.migration.Migration -import mihon.core.migration.MigrationContext 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 diff --git a/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt b/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt index 767491c83c..a3c1f9cbb7 100644 --- a/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt +++ b/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt @@ -1,9 +1,9 @@ package mihon.core.migration.migrations -import mihon.core.migration.Migration -import mihon.core.migration.MigrationContext 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 diff --git a/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt b/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt index d43c6647b2..f548cb7459 100644 --- a/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt +++ b/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt @@ -1,9 +1,9 @@ package mihon.core.migration.migrations -import mihon.core.migration.Migration -import mihon.core.migration.MigrationContext 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 @@ -14,7 +14,8 @@ class TrustExtensionRepositoryMigration : Migration { override suspend fun action(migrationContext: MigrationContext): Boolean = withIOContext { val sourcePreferences = migrationContext.get() ?: return@withIOContext false - val extensionRepositoryRepository = migrationContext.get() ?: return@withIOContext false + val extensionRepositoryRepository = + migrationContext.get() ?: return@withIOContext false for ((index, source) in sourcePreferences.extensionRepos().get().withIndex()) { try { extensionRepositoryRepository.upsertRepo( diff --git a/app/src/test/java/mihon/core/migration/MigratorTest.kt b/app/src/test/java/mihon/core/migration/MigratorTest.kt index 203d3e068f..e8bcc39bc4 100644 --- a/app/src/test/java/mihon/core/migration/MigratorTest.kt +++ b/app/src/test/java/mihon/core/migration/MigratorTest.kt @@ -86,7 +86,8 @@ class MigratorTest { migrations = listOf( Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }, - Migration.of(3f) { true }), + Migration.of(3f) { true } + ), onMigrationComplete = spyk ) verify { spyk() } diff --git a/buildSrc/src/main/kotlin/detekt.gradle.kts b/buildSrc/src/main/kotlin/detekt.gradle.kts index 986138ee63..29e34bad44 100644 --- a/buildSrc/src/main/kotlin/detekt.gradle.kts +++ b/buildSrc/src/main/kotlin/detekt.gradle.kts @@ -23,7 +23,7 @@ private val scriptsFiles = "**/*.kts" detekt { buildUponDefaultConfig = true parallel = true - autoCorrect = false + autoCorrect = true ignoreFailures = false config.setFrom(configFile) baseline = file(baselineFile) From f9473c1ca2a6cabc0ff56e4766cbdf9818839ee6 Mon Sep 17 00:00:00 2001 From: Andreas Date: Sun, 24 Mar 2024 20:25:56 +0100 Subject: [PATCH 3/7] Do migrations synchronous --- .../kanade/tachiyomi/ui/main/MainActivity.kt | 16 ++---- .../java/mihon/core/migration/Migrator.kt | 33 ++++++------ .../java/mihon/core/migration/MigratorTest.kt | 54 +++++++++---------- buildSrc/src/main/kotlin/detekt.gradle.kts | 2 +- 4 files changed, 48 insertions(+), 57 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt index 7342cbc48b..7b8cbc8037 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt @@ -131,7 +131,7 @@ class MainActivity : BaseActivity() { super.onCreate(savedInstanceState) - val migrations = migrate() + val didMigration = migrate() // Do not let the launcher create a new activity http://stackoverflow.com/questions/16283079 if (!isTaskRoot) { @@ -242,17 +242,7 @@ class MainActivity : BaseActivity() { ShowOnboarding() } - var showChangelog by remember { mutableStateOf(false) } - LaunchedEffect(Unit) { - if (migrations.isEmpty()) { - showChangelog = false - return@LaunchedEffect - } - @SuppressWarnings("SpreadOperator") - val didMigration = awaitAll(*migrations).reduce { acc, aBoolean -> acc || aBoolean } - showChangelog = didMigration && !BuildConfig.DEBUG - } - + var showChangelog by remember { mutableStateOf(didMigration && !BuildConfig.DEBUG) } if (showChangelog) { AlertDialog( onDismissRequest = { showChangelog = false }, @@ -352,7 +342,7 @@ class MainActivity : BaseActivity() { } } - private fun migrate(): Array> { + private fun migrate(): Boolean { val preferenceStore = Injekt.get() val preference = preferenceStore.getInt(Preference.appStateKey("last_version_code"), 0) logcat { "Migration from ${preference.get()} to ${BuildConfig.VERSION_CODE}" } diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt index 7fce38626c..b0a3cb4e92 100644 --- a/app/src/main/java/mihon/core/migration/Migrator.kt +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -1,9 +1,8 @@ package mihon.core.migration import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Deferred import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.async +import kotlinx.coroutines.runBlocking import tachiyomi.core.common.util.system.logcat object Migrator { @@ -15,7 +14,7 @@ object Migrator { migrations: List, dryrun: Boolean = false, onMigrationComplete: () -> Unit - ): Array> { + ): Boolean { val migrationContext = MigrationContext() val coroutineScope = CoroutineScope(Dispatchers.IO) @@ -30,7 +29,7 @@ object Migrator { } if (old >= new) { - return emptyArray() + return false } onMigrationComplete() @@ -46,20 +45,22 @@ object Migrator { versions: List, migrationsByVersion: Map>, dryrun: Boolean - ): Array> { - return versions.sorted() - .flatMap { version -> - (migrationsByVersion[version] ?: emptyList()) - .sortedBy(Migration::version) - .map { migration -> - if (!dryrun) { - logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } - return@map async { migration.action(this@migrate) } - } + ): Boolean { + var aBoolean = false + for (version in versions.sorted()) { + val migrations = migrationsByVersion.getOrDefault(version, emptyList()).sortedBy(Migration::version) + for (migration in migrations) { + val success = runBlocking { + if (!dryrun) { + logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } + } else { logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } - async { true } } + migration.action(this@migrate) + } + aBoolean = success || aBoolean } - .toTypedArray() + } + return aBoolean } } diff --git a/app/src/test/java/mihon/core/migration/MigratorTest.kt b/app/src/test/java/mihon/core/migration/MigratorTest.kt index e8bcc39bc4..89fe4db8c8 100644 --- a/app/src/test/java/mihon/core/migration/MigratorTest.kt +++ b/app/src/test/java/mihon/core/migration/MigratorTest.kt @@ -11,49 +11,49 @@ class MigratorTest { @Test fun initialVersion() { val onMigrationComplete: () -> Unit = {} - val spyk = spyk(onMigrationComplete) - val migrations = Migrator.migrate( + val onMigrationCompleteSpy = spyk(onMigrationComplete) + val didMigration = Migrator.migrate( old = 0, new = 1, - migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }), - onMigrationComplete = spyk + migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { false }), + onMigrationComplete = onMigrationCompleteSpy ) - verify { spyk() } - Assertions.assertEquals(1, migrations.size) + verify { onMigrationCompleteSpy() } + Assertions.assertTrue(didMigration) } @Test fun sameVersion() { val onMigrationComplete: () -> Unit = {} - val spyk = spyk(onMigrationComplete) - val migrations = Migrator.migrate( + val onMigrationCompleteSpy = spyk(onMigrationComplete) + val didMigration = Migrator.migrate( old = 1, new = 1, migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }), - onMigrationComplete = spyk + onMigrationComplete = onMigrationCompleteSpy ) - verify { spyk wasNot Called } - Assertions.assertEquals(0, migrations.size) + verify { onMigrationCompleteSpy wasNot Called } + Assertions.assertFalse(didMigration) } @Test fun smallMigration() { val onMigrationComplete: () -> Unit = {} - val spyk = spyk(onMigrationComplete) - val migrations = Migrator.migrate( + val onMigrationCompleteSpy = spyk(onMigrationComplete) + val didMigration = Migrator.migrate( old = 1, new = 2, migrations = listOf(Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }), - onMigrationComplete = spyk + onMigrationComplete = onMigrationCompleteSpy ) - verify { spyk() } - Assertions.assertEquals(2, migrations.size) + verify { onMigrationCompleteSpy() } + Assertions.assertTrue(didMigration) } @Test fun largeMigration() { val onMigrationComplete: () -> Unit = {} - val spyk = spyk(onMigrationComplete) + val onMigrationCompleteSpy = spyk(onMigrationComplete) val input = listOf( Migration.of(Migration.ALWAYS) { true }, Migration.of(2f) { true }, @@ -66,31 +66,31 @@ class MigratorTest { Migration.of(9f) { true }, Migration.of(10f) { true }, ) - val migrations = Migrator.migrate( + val didMigration = Migrator.migrate( old = 1, new = 10, migrations = input, - onMigrationComplete = spyk + onMigrationComplete = onMigrationCompleteSpy ) - verify { spyk() } - Assertions.assertEquals(10, migrations.size) + verify { onMigrationCompleteSpy() } + Assertions.assertTrue(didMigration) } @Test fun withinRangeMigration() { val onMigrationComplete: () -> Unit = {} - val spyk = spyk(onMigrationComplete) - val migrations = Migrator.migrate( + 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) { true } + Migration.of(3f) { false } ), - onMigrationComplete = spyk + onMigrationComplete = onMigrationCompleteSpy ) - verify { spyk() } - Assertions.assertEquals(2, migrations.size) + verify { onMigrationCompleteSpy() } + Assertions.assertTrue(didMigration) } } diff --git a/buildSrc/src/main/kotlin/detekt.gradle.kts b/buildSrc/src/main/kotlin/detekt.gradle.kts index 29e34bad44..986138ee63 100644 --- a/buildSrc/src/main/kotlin/detekt.gradle.kts +++ b/buildSrc/src/main/kotlin/detekt.gradle.kts @@ -23,7 +23,7 @@ private val scriptsFiles = "**/*.kts" detekt { buildUponDefaultConfig = true parallel = true - autoCorrect = true + autoCorrect = false ignoreFailures = false config.setFrom(configFile) baseline = file(baselineFile) From 45fb16b08882a40f0e1196a72a8d5d5bf3bc0146 Mon Sep 17 00:00:00 2001 From: Andreas Date: Mon, 25 Mar 2024 16:26:16 +0100 Subject: [PATCH 4/7] Filter and sort migrations --- .../java/mihon/core/migration/Migrator.kt | 53 +++++++------------ 1 file changed, 20 insertions(+), 33 deletions(-) diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt index b0a3cb4e92..b561d49869 100644 --- a/app/src/main/java/mihon/core/migration/Migrator.kt +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -1,7 +1,5 @@ package mihon.core.migration -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.runBlocking import tachiyomi.core.common.util.system.logcat @@ -16,16 +14,13 @@ object Migrator { onMigrationComplete: () -> Unit ): Boolean { val migrationContext = MigrationContext() - val coroutineScope = CoroutineScope(Dispatchers.IO) - - val migrationsByVersion = migrations.groupBy { it.version.toInt() } - val always = listOf(Migration.ALWAYS.toInt()) if (old == 0) { onMigrationComplete() - return with(coroutineScope) { - migrationContext.migrate(always, migrationsByVersion, dryrun) - } + return migrationContext.migrate( + migrations = migrations.filter { it.isAlways() }, + dryrun = dryrun + ) } if (old >= new) { @@ -33,34 +28,26 @@ object Migrator { } onMigrationComplete() - val versions = migrationsByVersion.keys.filter { version -> version in (old + 1)..new } - return with(coroutineScope) { - migrationContext.migrate(always + versions, migrationsByVersion, dryrun) - } + return migrationContext.migrate( + migrations = migrations.filter { it.isAlways() || it.version.toInt() in (old + 1).. new }, + dryrun = dryrun + ) } - context (CoroutineScope) + private fun Migration.isAlways() = version == Migration.ALWAYS + @SuppressWarnings("MaxLineLength") - private fun MigrationContext.migrate( - versions: List, - migrationsByVersion: Map>, - dryrun: Boolean - ): Boolean { - var aBoolean = false - for (version in versions.sorted()) { - val migrations = migrationsByVersion.getOrDefault(version, emptyList()).sortedBy(Migration::version) - for (migration in migrations) { - val success = runBlocking { - if (!dryrun) { - logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } - } else { - logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } - } - migration.action(this@migrate) + private fun MigrationContext.migrate(migrations: List, dryrun: Boolean): Boolean { + return migrations.sortedBy { it.version } + .map { migration -> + if (!dryrun) { + logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } + } else { + logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } } - aBoolean = success || aBoolean + runBlocking { migration.action(this@migrate) } } - } - return aBoolean + .reduce { acc, b -> acc || b } } + } From b57699f98d98feef91b2a78fb6a85a171ca70af2 Mon Sep 17 00:00:00 2001 From: Andreas Date: Mon, 25 Mar 2024 16:51:33 +0100 Subject: [PATCH 5/7] Review changes --- app/src/main/java/mihon/core/migration/Migrator.kt | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt index b561d49869..5eb69747fe 100644 --- a/app/src/main/java/mihon/core/migration/Migrator.kt +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -16,22 +16,20 @@ object Migrator { val migrationContext = MigrationContext() if (old == 0) { - onMigrationComplete() return migrationContext.migrate( migrations = migrations.filter { it.isAlways() }, dryrun = dryrun - ) + ).also { onMigrationComplete() } } if (old >= new) { return false } - onMigrationComplete() 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 @@ -42,10 +40,11 @@ object Migrator { .map { migration -> if (!dryrun) { logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } + runBlocking { migration.action(this@migrate) } } else { logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } + true } - runBlocking { migration.action(this@migrate) } } .reduce { acc, b -> acc || b } } From 56782cb3090cbcf2d41e141e78574c8b77da3faf Mon Sep 17 00:00:00 2001 From: Andreas Date: Mon, 25 Mar 2024 17:01:24 +0100 Subject: [PATCH 6/7] Review changes 2 --- app/src/main/java/mihon/core/migration/Migration.kt | 4 ++-- app/src/main/java/mihon/core/migration/Migrator.kt | 8 +++++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/mihon/core/migration/Migration.kt b/app/src/main/java/mihon/core/migration/Migration.kt index 8c3249d7ba..2fa04d1c99 100644 --- a/app/src/main/java/mihon/core/migration/Migration.kt +++ b/app/src/main/java/mihon/core/migration/Migration.kt @@ -3,7 +3,7 @@ package mihon.core.migration interface Migration { val version: Float - suspend fun action(migrationContext: MigrationContext): Boolean + suspend operator fun invoke(migrationContext: MigrationContext): Boolean companion object { const val ALWAYS = -1f @@ -11,7 +11,7 @@ interface Migration { fun of(version: Float, action: suspend (MigrationContext) -> Boolean): Migration = object : Migration { override val version: Float = version - override suspend fun action(migrationContext: MigrationContext): Boolean { + override suspend operator fun invoke(migrationContext: MigrationContext): Boolean { return action(migrationContext) } } diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt index 5eb69747fe..44e4bdcd7e 100644 --- a/app/src/main/java/mihon/core/migration/Migrator.kt +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -19,7 +19,8 @@ object Migrator { return migrationContext.migrate( migrations = migrations.filter { it.isAlways() }, dryrun = dryrun - ).also { onMigrationComplete() } + ) + .also { onMigrationComplete() } } if (old >= new) { @@ -29,7 +30,8 @@ object Migrator { return migrationContext.migrate( migrations = migrations.filter { it.isAlways() || it.version.toInt() in (old + 1).. new }, dryrun = dryrun - ).also { onMigrationComplete() } + ) + .also { onMigrationComplete() } } private fun Migration.isAlways() = version == Migration.ALWAYS @@ -40,7 +42,7 @@ object Migrator { .map { migration -> if (!dryrun) { logcat { "Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } - runBlocking { migration.action(this@migrate) } + runBlocking { migration(this@migrate) } } else { logcat { "(Dry-run) Running migration: { name = ${migration::class.simpleName}, version = ${migration.version} }" } true From d2c6de8ea41a4c4383eaf24486ce7174a1488c7b Mon Sep 17 00:00:00 2001 From: Andreas Date: Mon, 25 Mar 2024 17:10:02 +0100 Subject: [PATCH 7/7] Fix Detekt errors --- app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt | 2 -- app/src/main/java/mihon/core/migration/Migrator.kt | 3 +-- .../core/migration/migrations/SetupBackupCreateMigration.kt | 2 +- .../core/migration/migrations/SetupLibraryUpdateMigration.kt | 2 +- .../migration/migrations/TrustExtensionRepositoryMigration.kt | 2 +- 5 files changed, 4 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt index 7b8cbc8037..d49a7fc689 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/main/MainActivity.kt @@ -77,8 +77,6 @@ import eu.kanade.tachiyomi.util.system.dpToPx import eu.kanade.tachiyomi.util.system.isNavigationBarNeedsScrim import eu.kanade.tachiyomi.util.system.openInBrowser import eu.kanade.tachiyomi.util.view.setComposeContent -import kotlinx.coroutines.Deferred -import kotlinx.coroutines.awaitAll import kotlinx.coroutines.channels.awaitClose import kotlinx.coroutines.flow.callbackFlow import kotlinx.coroutines.flow.collectLatest diff --git a/app/src/main/java/mihon/core/migration/Migrator.kt b/app/src/main/java/mihon/core/migration/Migrator.kt index 44e4bdcd7e..86288e2a0a 100644 --- a/app/src/main/java/mihon/core/migration/Migrator.kt +++ b/app/src/main/java/mihon/core/migration/Migrator.kt @@ -28,7 +28,7 @@ object Migrator { } return migrationContext.migrate( - migrations = migrations.filter { it.isAlways() || it.version.toInt() in (old + 1).. new }, + migrations = migrations.filter { it.isAlways() || it.version.toInt() in (old + 1)..new }, dryrun = dryrun ) .also { onMigrationComplete() } @@ -50,5 +50,4 @@ object Migrator { } .reduce { acc, b -> acc || b } } - } diff --git a/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt b/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt index 61291ce5ef..44c0b557cf 100644 --- a/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt +++ b/app/src/main/java/mihon/core/migration/migrations/SetupBackupCreateMigration.kt @@ -8,7 +8,7 @@ import mihon.core.migration.MigrationContext class SetupBackupCreateMigration : Migration { override val version: Float = Migration.ALWAYS - override suspend fun action(migrationContext: MigrationContext): Boolean { + override suspend fun invoke(migrationContext: MigrationContext): Boolean { val context = migrationContext.get() ?: return false BackupCreateJob.setupTask(context) return true diff --git a/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt b/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt index a3c1f9cbb7..65482cd897 100644 --- a/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt +++ b/app/src/main/java/mihon/core/migration/migrations/SetupLibraryUpdateMigration.kt @@ -8,7 +8,7 @@ import mihon.core.migration.MigrationContext class SetupLibraryUpdateMigration : Migration { override val version: Float = Migration.ALWAYS - override suspend fun action(migrationContext: MigrationContext): Boolean { + override suspend fun invoke(migrationContext: MigrationContext): Boolean { val context = migrationContext.get() ?: return false LibraryUpdateJob.setupTask(context) return true diff --git a/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt b/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt index f548cb7459..cacba046f2 100644 --- a/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt +++ b/app/src/main/java/mihon/core/migration/migrations/TrustExtensionRepositoryMigration.kt @@ -12,7 +12,7 @@ import tachiyomi.core.common.util.system.logcat class TrustExtensionRepositoryMigration : Migration { override val version: Float = 7f - override suspend fun action(migrationContext: MigrationContext): Boolean = withIOContext { + override suspend fun invoke(migrationContext: MigrationContext): Boolean = withIOContext { val sourcePreferences = migrationContext.get() ?: return@withIOContext false val extensionRepositoryRepository = migrationContext.get() ?: return@withIOContext false