Review comments

CHANGELOG.md

@@ -10,7 +10,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - .
 
 ### Changed
-- (Database/H2) Use the latest H2 database engine
+- .
 
 ### Fixed
 - (CloudFlareInterceptor) Don't send the `cf_clearance` cookie back to Flaresolverr

server/src/main/kotlin/suwayomi/tachidesk/global/model/table/GlobalMetaTable.kt

@@ -13,6 +13,6 @@ import org.jetbrains.exposed.v1.core.dao.id.IntIdTable
  * Metadata storage for clients, server/global level.
  */
 object GlobalMetaTable : IntIdTable() {
-    val key = varchar("meta_key", 256)
+    val key = varchar("key", 256)
     val value = varchar("value", 4096)
 }

server/src/main/kotlin/suwayomi/tachidesk/manga/model/table/CategoryMetaTable.kt

@@ -15,7 +15,7 @@ import suwayomi.tachidesk.manga.model.table.CategoryMetaTable.ref
  * Metadata storage for clients, about Category with id == [ref].
  */
 object CategoryMetaTable : IntIdTable() {
-    val key = varchar("meta_key", 256)
+    val key = varchar("key", 256)
     val value = varchar("value", 4096)
     val ref = reference("category_ref", CategoryTable, ReferenceOption.CASCADE)
 }

server/src/main/kotlin/suwayomi/tachidesk/manga/model/table/ChapterMetaTable.kt

@@ -26,7 +26,7 @@ import suwayomi.tachidesk.manga.model.table.ChapterMetaTable.ref
  * }
  */
 object ChapterMetaTable : IntIdTable() {
-    val key = varchar("meta_key", 256)
+    val key = varchar("key", 256)
     val value = varchar("value", 4096)
     val ref = reference("chapter_ref", ChapterTable, ReferenceOption.CASCADE)
 }

server/src/main/kotlin/suwayomi/tachidesk/manga/model/table/MangaMetaTable.kt

@@ -26,7 +26,7 @@ import suwayomi.tachidesk.manga.model.table.MangaMetaTable.ref
  * }
  */
 object MangaMetaTable : IntIdTable() {
-    val key = varchar("meta_key", 256)
+    val key = varchar("key", 256)
     val value = varchar("value", 4096)
     val ref = reference("manga_ref", MangaTable, ReferenceOption.CASCADE)
 }

server/src/main/kotlin/suwayomi/tachidesk/manga/model/table/SourceMetaTable.kt

@@ -14,7 +14,7 @@ import suwayomi.tachidesk.manga.model.table.SourceMetaTable.ref
  * Metadata storage for clients, about Source with id == [ref].
  */
 object SourceMetaTable : IntIdTable() {
-    val key = varchar("meta_key", 256)
+    val key = varchar("key", 256)
     val value = varchar("value", 4096)
     val ref = long("source_ref")
 }

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

@@ -27,6 +27,7 @@ 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.time.Duration.Companion.minutes
 import kotlin.time.Duration.Companion.seconds
 
@@ -183,7 +184,7 @@ fun databaseUp() {
         }
         val migrations = loadMigrationsFrom("suwayomi.tachidesk.server.database.migration", ServerConfig::class.java)
         runMigrations(migrations)
-    } catch (e: Exception) {
+    } catch (e: SQLException) {
         logger.error(e) { "Error up-to-database migration" }
         if (System.getProperty("crashOnFailedMigration").toBoolean()) {
             shutdownApp(ExitCode.DbMigrationFailure)

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

@@ -9,12 +9,9 @@ import java.net.URLClassLoader
 import java.nio.file.Path
 import kotlin.io.path.Path
 import kotlin.io.path.absolutePathString
-import kotlin.io.path.bufferedReader
-import kotlin.io.path.bufferedWriter
 import kotlin.io.path.copyTo
 import kotlin.io.path.createDirectories
 import kotlin.io.path.deleteExisting
-import kotlin.io.path.deleteIfExists
 import kotlin.io.path.div
 import kotlin.io.path.exists
 import kotlin.io.path.name
@@ -49,10 +46,6 @@ object H2Migration {
         }
 
         val script = Path("$dbBase.${h2Old.substringAfterLast('.')}.sql")
-        script.deleteIfExists()
-
-        val modifiedScript = Path("$dbBase.${h2Old.substringAfterLast('.')}.modified.sql")
-        modifiedScript.deleteIfExists()
 
         // Backup original database.
         val backup = Path("$dbBase.mv.db.${h2Old.substringAfterLast('.')}.backup")
@@ -79,32 +72,19 @@ object H2Migration {
             libsDir.resolve("h2-$h2New.bin"),
         )
 
-        // Delete attempted migration if failed previously
-        val newDatabase = Path(rootDir, "database.${h2New.substringAfterLast('.')}.mv.db")
-        newDatabase.deleteIfExists()
-
-        val modifiedNewDatabase = Path(rootDir, "database.${h2Old.substringAfterLast('.')}.modified.${h2New.substringAfterLast('.')}.mv.db")
-        modifiedNewDatabase.deleteIfExists()
-
         runMigrationTool(
             migrationJar = migrationJar,
             libsDir = libsDir,
             mvStore = mvStore,
             script = script,
-            modifiedScript = modifiedScript,
             h2Old = h2Old,
             h2New = h2New,
         )
 
         // Move database to proper path
-        if (modifiedNewDatabase.exists()) {
-            modifiedNewDatabase.copyTo(mvStore, overwrite = true)
-            modifiedNewDatabase.deleteExisting()
-            newDatabase.deleteIfExists()
-        } else {
-            newDatabase.copyTo(mvStore, overwrite = true)
-            newDatabase.deleteExisting()
-        }
+        val newDatabase = Path(rootDir, "database.${h2New.substringAfterLast('.')}.mv.db")
+        newDatabase.copyTo(mvStore, overwrite = true)
+        newDatabase.deleteExisting()
 
         logger.info { "H2 migration completed successfully." }
     }
@@ -143,7 +123,6 @@ object H2Migration {
         libsDir: Path,
         mvStore: Path,
         script: Path,
-        modifiedScript: Path,
         h2Old: String,
         h2New: String,
     ) {
@@ -157,77 +136,32 @@ object H2Migration {
             val main =
                 clazz.getMethod("main", Array<String>::class.java)
 
-            try {
-                main.invoke(
-                    null,
-                    arrayOf(
-                        // h2 driver dir
-                        "-l",
-                        libsDir.absolutePathString(),
-                        // from version
-                        "-f",
-                        h2Old,
-                        // to version
-                        "-t",
-                        h2New,
-                        // user
-                        "-u",
-                        "",
-                        // password
-                        "-p",
-                        "",
-                        // database.mv.db
-                        "-d",
-                        mvStore.absolutePathString(),
-                        // database backup in SQL
-                        "-s",
-                        script.absolutePathString(),
-                    ),
-                )
-            } catch (e: Exception) {
-                // Modify raw .sql file as needed for compatibility
-                if (e.stackTraceToString().contains("Unknown data type: \"DATETIME\"; SQL statement:") && script.exists()) {
-                    script.bufferedReader().use { reader ->
-                        modifiedScript.bufferedWriter().use { writer ->
-                            reader.forEachLine { line ->
-                                writer.write(
-                                    line.replace(
-                                        "    \"EXECUTED_AT\" DATETIME(9) NOT NULL",
-                                        "    \"EXECUTED_AT\" TIMESTAMP(9) NOT NULL",
-                                    ),
-                                )
-                                writer.newLine()
-                            }
-                        }
-                    }
-
-                    main.invoke(
-                        null,
-                        arrayOf(
-                            // h2 driver dir
-                            "-l",
-                            libsDir.absolutePathString(),
-                            // from version
-                            "-f",
-                            h2Old,
-                            // to version
-                            "-t",
-                            h2New,
-                            // user
-                            "-u",
-                            "",
-                            // password
-                            "-p",
-                            "",
-                            // database.mv.db
-                            "-d",
-                            modifiedScript.absolutePathString(),
-                        ),
-                    )
-                } else {
-                    throw e
-                }
-            }
+            main.invoke(
+                null,
+                arrayOf(
+                    // h2 driver dir
+                    "-l",
+                    libsDir.absolutePathString(),
+                    // from version
+                    "-f",
+                    h2Old,
+                    // to version
+                    "-t",
+                    h2New,
+                    // user
+                    "-u",
+                    "",
+                    // password
+                    "-p",
+                    "",
+                    // database.mv.db
+                    "-d",
+                    mvStore.absolutePathString(),
+                    // database backup in SQL
+                    "-s",
+                    script.absolutePathString(),
+                ),
+            )
         }
     }
 }

server/src/main/kotlin/suwayomi/tachidesk/server/database/migration/M0023_CategoryMetaRefFix.kt

@@ -10,10 +10,17 @@ package suwayomi.tachidesk.server.database.migration
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 import de.neonew.exposed.migrations.helpers.SQLMigration
-import suwayomi.tachidesk.server.database.migration.helpers.toSqlName
+import org.jetbrains.exposed.v1.jdbc.transactions.TransactionManager
 
 @Suppress("ClassName", "unused")
 class M0023_CategoryMetaRefFix : SQLMigration() {
+    fun String.toSqlName(): String =
+        TransactionManager.defaultDatabase!!.identifierManager.let {
+            it.quoteIfNecessary(
+                it.inProperCase(this),
+            )
+        }
+
     private val CategoryMetaTable by lazy { "CategoryMeta".toSqlName() }
     private val CategoryRefColumn by lazy { "category_ref".toSqlName() }
     private val CategoryTable by lazy { "Category".toSqlName() }

server/src/main/kotlin/suwayomi/tachidesk/server/database/migration/M0049_FixDuplicatedMetas.kt

@@ -8,7 +8,6 @@ package suwayomi.tachidesk.server.database.migration
  * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
 
 import de.neonew.exposed.migrations.helpers.SQLMigration
-import suwayomi.tachidesk.server.database.migration.helpers.toSqlName
 
 @Suppress("ClassName", "unused")
 class M0049_FixDuplicatedMetas : SQLMigration() {
@@ -16,7 +15,7 @@ class M0049_FixDuplicatedMetas : SQLMigration() {
         table: String,
         refColumn: String? = null,
     ): String {
-        val groupBy = listOfNotNull(refColumn, "KEY".toSqlName()).joinToString(", ")
+        val groupBy = listOfNotNull(refColumn, "KEY").joinToString(", ")
 
         return """
             DELETE FROM $table
@@ -31,11 +30,10 @@ class M0049_FixDuplicatedMetas : SQLMigration() {
             """.trimIndent()
     }
 
-    override val sql: String by lazy {
+    override val sql: String =
         createMigrationForTable("CATEGORYMETA", "CATEGORY_REF") +
             createMigrationForTable("CHAPTERMETA", "CHAPTER_REF") +
             createMigrationForTable("GLOBALMETA") +
             createMigrationForTable("MANGAMETA", "MANGA_REF") +
             createMigrationForTable("SOURCEMETA", "SOURCE_REF")
-    }
 }

server/src/main/kotlin/suwayomi/tachidesk/server/database/migration/M0055_RenameMetaKeys.kt

@@ -1,38 +0,0 @@
-package suwayomi.tachidesk.server.database.migration
-
-/*
- * Copyright (C) Contributors to the Suwayomi project
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at https://mozilla.org/MPL/2.0/. */
-
-import de.neonew.exposed.migrations.helpers.SQLMigration
-import suwayomi.tachidesk.graphql.types.DatabaseType
-import suwayomi.tachidesk.server.database.migration.helpers.toSqlName
-import suwayomi.tachidesk.server.serverConfig
-
-@Suppress("ClassName", "unused")
-class M0055_RenameMetaKeys : SQLMigration() {
-    fun postgresRename(table: String): String =
-        "ALTER TABLE $table " +
-            "RENAME COLUMN " + "KEY".toSqlName() + " TO META_KEY;"
-
-    fun h2Rename(table: String): String =
-        "ALTER TABLE $table " +
-            "ALTER COLUMN " + "KEY".toSqlName() + " RENAME TO META_KEY;"
-
-    fun createRenameMigration(table: String): String =
-        when (serverConfig.databaseType.value) {
-            DatabaseType.H2 -> h2Rename(table.toSqlName())
-            DatabaseType.POSTGRESQL -> postgresRename(table.toSqlName())
-        }
-
-    override val sql: String by lazy {
-        createRenameMigration("CATEGORYMETA") +
-            createRenameMigration("CHAPTERMETA") +
-            createRenameMigration("GLOBALMETA") +
-            createRenameMigration("MANGAMETA") +
-            createRenameMigration("SOURCEMETA")
-    }
-}

server/src/main/kotlin/suwayomi/tachidesk/server/database/migration/helpers/RenameFieldMigration.kt

@@ -1,9 +1,17 @@
 package suwayomi.tachidesk.server.database.migration.helpers
 
 import de.neonew.exposed.migrations.helpers.SQLMigration
+import org.jetbrains.exposed.v1.jdbc.transactions.TransactionManager
 import suwayomi.tachidesk.graphql.types.DatabaseType
 import suwayomi.tachidesk.server.serverConfig
 
+fun String.toSqlName(): String =
+    TransactionManager.current().db.identifierManager.let {
+        it.quoteIfNecessary(
+            it.inProperCase(this),
+        )
+    }
+
 abstract class RenameFieldMigration(
     tableName: String,
     originalName: String,

server/src/main/kotlin/suwayomi/tachidesk/server/database/migration/helpers/ToSqlName.kt

@@ -1,10 +0,0 @@
-package suwayomi.tachidesk.server.database.migration.helpers
-
-import org.jetbrains.exposed.v1.jdbc.transactions.TransactionManager
-
-fun String.toSqlName(): String =
-    TransactionManager.current().db.identifierManager.let {
-        it.quoteIfNecessary(
-            it.inProperCase(this),
-        )
-    }