Streamline deprecated settings config value migration logic (#1633)

* Streamline deprecated settings config value migration logic

* Add "autoDownloadAheadLimit" config migration

For consistency

* Replace "exitCode" with "shutdownApp"

* Enhance shutdown logging to include exit reason

server/server-config/src/main/kotlin/suwayomi/tachidesk/server/ServerConfig.kt

@@ -8,6 +8,7 @@ package suwayomi.tachidesk.server
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 import com.typesafe.config.Config
+import io.github.config4k.toConfig
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -202,6 +203,7 @@ class ServerConfig(
             SettingsRegistry.SettingDeprecated(
                 replaceWith = "autoDownloadNewChaptersLimit",
                 message = "Replaced with autoDownloadNewChaptersLimit",
+                migrateConfigValue = { it.unwrapped() as? Int }
             ),
         readMigrated = { autoDownloadNewChaptersLimit.value },
         setMigrated = { autoDownloadNewChaptersLimit.value = it },
@@ -299,6 +301,13 @@ class ServerConfig(
             SettingsRegistry.SettingDeprecated(
                 replaceWith = "authMode",
                 message = "Removed - prefer authMode",
+                migrateConfigValue = {
+                    if (it.unwrapped() as? Boolean == true) {
+                        AuthMode.BASIC_AUTH.name
+                    } else {
+                        null
+                    }
+                }
             ),
         readMigrated = { authMode.value == AuthMode.BASIC_AUTH },
         setMigrated = { authMode.value = if (it) AuthMode.BASIC_AUTH else AuthMode.NONE },
@@ -613,6 +622,28 @@ class ServerConfig(
         SettingsRegistry.SettingDeprecated(
             replaceWith = "koreaderSyncStrategyForward, koreaderSyncStrategyBackward",
             message = "Replaced with koreaderSyncStrategyForward and koreaderSyncStrategyBackward",
+            migrateConfig = { value, config ->
+                val oldStrategy = (value.unwrapped() as? String)?.uppercase()
+
+                val (forward, backward) =
+                    when (oldStrategy) {
+                        "PROMPT" -> "PROMPT" to "PROMPT"
+                        "SILENT" -> "KEEP_REMOTE" to "KEEP_LOCAL"
+                        "SEND" -> "KEEP_LOCAL" to "KEEP_LOCAL"
+                        "RECEIVE" -> "KEEP_REMOTE" to "KEEP_REMOTE"
+                        "DISABLED" -> "DISABLED" to "DISABLED"
+                        else -> null to null
+                    }
+
+                if (forward != null && backward != null) {
+                    config
+                        .withValue("server.koreaderSyncStrategyForward", forward.toConfig("internal").getValue("internal"))
+                        .withValue("server.koreaderSyncStrategyBackward", backward.toConfig("internal").getValue("internal"))
+                        .withoutPath("server.koreaderSyncStrategy")
+                } else {
+                    config
+                }
+            }
         ),
         readMigrated = {
             // This is a best-effort reverse mapping. It's not perfect but covers common cases.
@@ -742,6 +773,7 @@ class ServerConfig(
             SettingsRegistry.SettingDeprecated(
                 replaceWith = "authUsername",
                 message = "Removed - prefer authUsername",
+                migrateConfigValue = { it.unwrapped() as? String },
             ),
         readMigrated = { authUsername.value },
         setMigrated = { authUsername.value = it },
@@ -755,6 +787,7 @@ class ServerConfig(
             SettingsRegistry.SettingDeprecated(
                 replaceWith = "authPassword",
                 message = "Removed - prefer authPassword",
+                migrateConfigValue = { it.unwrapped() as? String },
             ),
         readMigrated = { authPassword.value },
         setMigrated = { authPassword.value = it },

server/server-config/src/main/kotlin/suwayomi/tachidesk/server/settings/SettingsRegistry.kt

@@ -1,14 +1,28 @@
 package suwayomi.tachidesk.server.settings
 
+import com.typesafe.config.ConfigValue
+import com.typesafe.config.parser.ConfigDocument
 import kotlin.reflect.KClass
 
 /**
  * Registry to track all settings for automatic updating and validation
  */
 object SettingsRegistry {
+    /**
+     * Requires either [migrateConfigValue] or [migrateConfig] to be set.
+     * If neither is specified, the server will exit on startup due to being misconfigured.
+     */
     data class SettingDeprecated(
         val replaceWith: String? = null,
         val message: String,
+        /**
+         * For cases which do not require custom config miration logic.
+         */
+        val migrateConfigValue: ((value: ConfigValue) -> Any?)? = null,
+        /**
+         * For cases which require complete control over the config migration.
+         */
+        val migrateConfig: ((value: ConfigValue, config: ConfigDocument) -> ConfigDocument)? = null
     )
 
     interface ITypeInfo {

server/src/main/kotlin/suwayomi/tachidesk/server/ServerSetup.kt

@@ -37,7 +37,6 @@ import org.koin.core.context.startKoin
 import org.koin.core.module.Module
 import org.koin.dsl.module
 import suwayomi.tachidesk.global.impl.KcefWebView.Companion.toCefCookie
-import suwayomi.tachidesk.graphql.types.AuthMode
 import suwayomi.tachidesk.graphql.types.DatabaseType
 import suwayomi.tachidesk.i18n.LocalizationHelper
 import suwayomi.tachidesk.manga.impl.backup.proto.ProtoBackupExport
@@ -47,9 +46,12 @@ import suwayomi.tachidesk.manga.impl.update.Updater
 import suwayomi.tachidesk.manga.impl.util.lang.renameTo
 import suwayomi.tachidesk.server.database.databaseUp
 import suwayomi.tachidesk.server.generated.BuildConfig
+import suwayomi.tachidesk.server.settings.SettingsRegistry
 import suwayomi.tachidesk.server.util.AppMutex.handleAppMutex
 import suwayomi.tachidesk.server.util.ConfigTypeRegistration
+import suwayomi.tachidesk.server.util.ExitCode
 import suwayomi.tachidesk.server.util.SystemTray
+import suwayomi.tachidesk.server.util.shutdownApp
 import uy.kohesive.injekt.Injekt
 import uy.kohesive.injekt.api.get
 import xyz.nulldev.androidcompat.AndroidCompat
@@ -142,17 +144,18 @@ fun setupLogLevelUpdating(
     )
 }
 
-fun <T : Any> migrateConfig(
+fun migrateConfig(
     configDocument: ConfigDocument,
     config: Config,
     configKey: String,
     toConfigKey: String,
-    toType: (ConfigValue) -> T?,
+    toType: (ConfigValue) -> Any?,
 ): ConfigDocument {
     try {
         val configValue = config.getValue(configKey)
         val typedValue = toType(configValue)
         if (typedValue != null) {
+            logger.debug { "Migrating config value: $configKey -> $toConfigKey" }
             return configDocument.withValue(
                 toConfigKey,
                 typedValue.toConfig("internal").getValue("internal"),
@@ -309,59 +312,31 @@ fun applicationSetup() {
             // make sure the user config file is up-to-date
             GlobalConfigManager.updateUserConfig { config ->
                 var updatedConfig = this
-                updatedConfig =
-                    migrateConfig(
-                        updatedConfig,
-                        config,
-                        "server.basicAuthEnabled",
-                        "server.authMode",
-                        toType = {
-                            if (it.unwrapped() as? Boolean == true) {
-                                AuthMode.BASIC_AUTH.name
-                            } else {
-                                null
-                            }
-                        },
-                    )
-                updatedConfig =
-                    migrateConfig(
-                        updatedConfig,
-                        config,
-                        "server.basicAuthUsername",
-                        "server.authUsername",
-                        toType = { it.unwrapped() as? String },
-                    )
-                updatedConfig =
-                    migrateConfig(
-                        updatedConfig,
-                        config,
-                        "server.basicAuthPassword",
-                        "server.authPassword",
-                        toType = { it.unwrapped() as? String },
-                    )
 
-                // Migrate KOReader sync strategy from single to forward/backward strategies
-                try {
-                    val oldStrategy = config.getString("server.koreaderSyncStrategy")
-                    val (forward, backward) =
-                        when (oldStrategy.uppercase()) {
-                            "PROMPT" -> "PROMPT" to "PROMPT"
-                            "SILENT" -> "KEEP_REMOTE" to "KEEP_LOCAL"
-                            "SEND" -> "KEEP_LOCAL" to "KEEP_LOCAL"
-                            "RECEIVE" -> "KEEP_REMOTE" to "KEEP_REMOTE"
-                            "DISABLED" -> "DISABLED" to "DISABLED"
-                            else -> null to null
-                        }
+                val settingsRequiringMigration = SettingsRegistry.getAll().filterValues { it.deprecated?.replaceWith != null }
+                settingsRequiringMigration.forEach { (name, data) ->
+                    val configKey = "server.$name"
+                    val toConfigKey = "server.${data.deprecated!!.replaceWith}"
 
-                    if (forward != null && backward != null) {
-                        updatedConfig =
-                            updatedConfig
-                                .withValue("server.koreaderSyncStrategyForward", forward.toConfig("internal").getValue("internal"))
-                                .withValue("server.koreaderSyncStrategyBackward", backward.toConfig("internal").getValue("internal"))
-                                .withoutPath("server.koreaderSyncStrategy")
+                    if (data.deprecated!!.migrateConfig != null) {
+                        logger.debug { "Migrating config value: $configKey -> $toConfigKey" }
+                        updatedConfig = data.deprecated!!.migrateConfig!!(config.getValue(configKey), updatedConfig)
+                        return@forEach
                     }
-                } catch (_: ConfigException.Missing) {
-                    // Key doesn't exist, no migration needed
+
+                    if (data.deprecated!!.migrateConfigValue != null) {
+                        updatedConfig =
+                            migrateConfig(
+                                updatedConfig,
+                                config,
+                                configKey,
+                                toConfigKey,
+                                data.deprecated!!.migrateConfigValue!!,
+                            )
+                        return@forEach
+                    }
+
+                    shutdownApp(ExitCode.ConfigMigrationMisconfiguredFailure)
                 }
 
                 updatedConfig

server/src/main/kotlin/suwayomi/tachidesk/server/database/DBManager.kt

@@ -21,10 +21,11 @@ import suwayomi.tachidesk.graphql.types.DatabaseType
 import suwayomi.tachidesk.server.ApplicationDirs
 import suwayomi.tachidesk.server.ServerConfig
 import suwayomi.tachidesk.server.serverConfig
+import suwayomi.tachidesk.server.util.ExitCode
+import suwayomi.tachidesk.server.util.shutdownApp
 import uy.kohesive.injekt.Injekt
 import uy.kohesive.injekt.api.get
 import java.sql.SQLException
-import kotlin.system.exitProcess
 
 object DBManager {
     var db: Database? = null
@@ -90,7 +91,7 @@ fun databaseUp() {
     } catch (e: SQLException) {
         logger.error(e) { "Error up-to-database migration" }
         if (System.getProperty("crashOnFailedMigration").toBoolean()) {
-            exitProcess(101)
+            shutdownApp(ExitCode.DbMigrationFailure)
         }
     }
 }

server/src/main/kotlin/suwayomi/tachidesk/server/util/AppExit.kt

@@ -19,10 +19,12 @@ enum class ExitCode(
     MutexCheckFailedTachideskRunning(1),
     MutexCheckFailedAnotherAppRunning(2),
     WebUISetupFailure(3),
+    ConfigMigrationMisconfiguredFailure(4),
+    DbMigrationFailure(5),
 }
 
 fun shutdownApp(exitCode: ExitCode) {
-    logger.info { "Shutting Down Suwayomi-Server. Goodbye!" }
+    logger.info { "Shutting Down Suwayomi-Server. Goodbye! (reason= ${exitCode.code} (${exitCode.name}))" }
 
     exitProcess(exitCode.code)
 }