Migrations: Use explicit Migration interface instead of fun type

So the Java people don't have to return Unit everywhere. Also gives us the option to add further methods to `Migration`s in the future if necessary. Does have the downside of requiring Kotlin people to add an explicit `Migration` before the lambda for SAM conversion. But since each lambda will usually be on its own line anyway (unlike in the tests), that doesn't seem too bad.
EssentialGG · Oct 10, 2023 · 75dd63b · 75dd63b
1 parent a341670
commit 75dd63b
Show file tree

Hide file tree

Showing 5 changed files with 28 additions and 16 deletions.
diff --git a/api/Vigilance.api b/api/Vigilance.api
@@ -154,6 +154,10 @@ public final class gg/essential/vigilance/data/MethodBackedPropertyValue : gg/es
 	public fun invoke (Lgg/essential/vigilance/Vigilant;)V
 }
 
+public abstract interface class gg/essential/vigilance/data/Migration {
+	public abstract fun apply (Ljava/util/Map;)V
+}
+
 public abstract interface annotation class gg/essential/vigilance/data/Property : java/lang/annotation/Annotation {
 	public abstract fun allowAlpha ()Z
 	public abstract fun category ()Ljava/lang/String;

diff --git a/src/main/kotlin/gg/essential/vigilance/Vigilant.kt b/src/main/kotlin/gg/essential/vigilance/Vigilant.kt
@@ -69,7 +69,7 @@ abstract class Vigilant @JvmOverloads constructor(
      * must artificially modify that setting in a migration (e.g. increase its value by 1) and then change it back to
      * its actual value in a second migration (e.g. decrease its value again by 1).
      */
-    protected open val migrations: List<(MutableMap<String, Any?>) -> Unit> = emptyList()
+    protected open val migrations: List<Migration> = emptyList()
 
     private var dirty = false
     private var hasError = false

diff --git a/src/main/kotlin/gg/essential/vigilance/data/Migration.kt b/src/main/kotlin/gg/essential/vigilance/data/Migration.kt
@@ -0,0 +1,8 @@
+package gg.essential.vigilance.data
+
+import gg.essential.vigilance.Vigilant
+
+/** See [Vigilant.migrations]. */
+fun interface Migration {
+    fun apply(config: MutableMap<String, Any?>)
+}
diff --git a/src/main/kotlin/gg/essential/vigilance/impl/migration.kt b/src/main/kotlin/gg/essential/vigilance/impl/migration.kt
@@ -1,21 +1,20 @@
 package gg.essential.vigilance.impl
 
+import gg.essential.vigilance.data.Migration
 import gg.essential.vigilance.impl.nightconfig.core.Config
 
 private const val META_KEY = "__meta"
 private val VERSION_KEY = listOf(META_KEY, "version")
 private fun migrationLogKey(migration: Int): List<String> = listOf(META_KEY, "migration_log", "$migration")
 
-private typealias Migration = (MutableMap<String, Any?>) -> Unit
-
 internal fun migrate(root: Config, migrations: List<Migration>) {
     val fileVersion = root[VERSION_KEY] ?: 0
     if (fileVersion < migrations.size) {
         var oldMap = root.toMap()
         for (migration in fileVersion until migrations.size) {
             val newMap = oldMap.toMutableMap()
 
-            migrations[migration](newMap)
+            migrations[migration].apply(newMap)
 
             applyMigration(root, migration, oldMap, newMap)
 

diff --git a/src/test/kotlin/gg/essential/vigilance/impl/MigrationTest.kt b/src/test/kotlin/gg/essential/vigilance/impl/MigrationTest.kt
@@ -1,5 +1,6 @@
 package gg.essential.vigilance.impl
 
+import gg.essential.vigilance.data.Migration
 import gg.essential.vigilance.impl.nightconfig.core.Config
 import org.junit.jupiter.api.Assertions.assertEquals
 import org.junit.jupiter.api.Test
@@ -8,21 +9,21 @@ class MigrationTest {
     @Test
     fun testUpToDate() {
         val input = config("__meta" to mapOf("version" to 2), "test" to 42)
-        migrate(input, listOf({}, {}))
+        migrate(input, listOf(Migration {}, Migration {}))
         assertEquals(config("__meta" to mapOf("version" to 2), "test" to 42), input)
     }
 
     @Test
     fun testNoOpMigration() {
         val input = config("__meta" to mapOf("version" to 1), "test" to 42)
-        migrate(input, listOf({}, {}))
+        migrate(input, listOf(Migration {}, Migration {}))
         assertEquals(config("__meta" to mapOf("version" to 2), "test" to 42), input)
     }
 
     @Test
     fun testAddMigration() {
         val input = config("__meta" to mapOf("version" to 1), "test" to 42)
-        migrate(input, listOf({}, { it["new"] = 43 }))
+        migrate(input, listOf(Migration {}, Migration { it["new"] = 43 }))
         assertEquals(config(
             "__meta" to mapOf("version" to 2, "migration_log" to mapOf("1" to mapOf("added" to listOf("new")))),
             "test" to 42,
@@ -33,7 +34,7 @@ class MigrationTest {
     @Test
     fun testChangeMigration() {
         val input = config("__meta" to mapOf("version" to 1), "test" to 42)
-        migrate(input, listOf({}, { it["test"] = it["test"] as Int + 1 }))
+        migrate(input, listOf(Migration {}, Migration { it["test"] = it["test"] as Int + 1 }))
         assertEquals(config(
             "__meta" to mapOf("version" to 2, "migration_log" to mapOf("1" to mapOf("changed" to mapOf("test" to 42)))),
             "test" to 43,
@@ -43,7 +44,7 @@ class MigrationTest {
     @Test
     fun testRemoveMigration() {
         val input = config("__meta" to mapOf("version" to 1), "test" to 42)
-        migrate(input, listOf({}, { it.remove("test") }))
+        migrate(input, listOf(Migration {}, Migration { it.remove("test") }))
         assertEquals(config(
             "__meta" to mapOf("version" to 2, "migration_log" to mapOf("1" to mapOf("changed" to mapOf("test" to 42)))),
         ), input)
@@ -52,7 +53,7 @@ class MigrationTest {
     @Test
     fun testMultipleMigrations() {
         val input = config("__meta" to mapOf("version" to 1), "test" to 42)
-        migrate(input, listOf({}, { it["test"] = 1 }, { it["test"] = it["test"] as Int + 1 }))
+        migrate(input, listOf(Migration {}, Migration { it["test"] = 1 }, Migration { it["test"] = it["test"] as Int + 1 }))
         assertEquals(config(
             "__meta" to mapOf("version" to 3, "migration_log" to mapOf(
                 "1" to mapOf("changed" to mapOf("test" to 42)),
@@ -69,7 +70,7 @@ class MigrationTest {
             "test" to 42,
             "new" to 43,
         )
-        migrate(input, listOf {})
+        migrate(input, listOf(Migration {}))
         assertEquals(config("__meta" to mapOf("version" to 1), "test" to 42), input)
     }
 
@@ -79,7 +80,7 @@ class MigrationTest {
             "__meta" to mapOf("version" to 2, "migration_log" to mapOf("1" to mapOf("changed" to mapOf("test" to 42)))),
             "test" to 43,
         )
-        migrate(input, listOf {})
+        migrate(input, listOf(Migration {}))
         assertEquals(config("__meta" to mapOf("version" to 1), "test" to 42), input)
     }
 
@@ -88,7 +89,7 @@ class MigrationTest {
         val input = config(
             "__meta" to mapOf("version" to 2, "migration_log" to mapOf("1" to mapOf("changed" to mapOf("test" to 42)))),
         )
-        migrate(input, listOf {})
+        migrate(input, listOf(Migration {}))
         assertEquals(config("__meta" to mapOf("version" to 1), "test" to 42), input)
     }
 
@@ -101,7 +102,7 @@ class MigrationTest {
             )),
             "test" to 2,
         )
-        migrate(input, listOf {})
+        migrate(input, listOf(Migration {}))
         assertEquals(config("__meta" to mapOf("version" to 1), "test" to 42), input)
     }
 
@@ -115,10 +116,10 @@ class MigrationTest {
     @Test
     fun testInitialMigration() {
         val input = config("test" to 42)
-        migrate(input, listOf { config ->
+        migrate(input, listOf(Migration { config ->
             assert(config == mapOf("test" to 42))
             config["test"] = 43
-        })
+        }))
         assertEquals(config(
             "__meta" to mapOf("version" to 1, "migration_log" to mapOf("0" to mapOf("changed" to mapOf("test" to 42)))),
             "test" to 43,