-
Notifications
You must be signed in to change notification settings - Fork 0
Implement configuration migration system and update ABI checks #270
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
ad55bff
feat: implement configuration migration system with builder pattern
twisti-dev eb2a029
chore: dumb abi
twisti-dev 7159ebf
feat: add configuration migration support with example config class
twisti-dev c137dde
feat: update ABI checks to use Kotlin ABI commands
twisti-dev 2b0491c
chore: bump version to 1.21.11-2.72.0
twisti-dev f2f0513
Merge branch 'version/1.21.11' into feat/config-versioning
twisti-dev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
...it-plugin-test/src/main/kotlin/dev/slne/surf/surfapi/bukkit/test/config/MyPluginConfig.kt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| package dev.slne.surf.surfapi.bukkit.test.config | ||
|
|
||
| import dev.slne.surf.surfapi.bukkit.test.plugin | ||
| import dev.slne.surf.surfapi.core.api.config.SpongeYmlConfigClass | ||
| import dev.slne.surf.surfapi.core.api.config.migration.ConfigMigration | ||
| import org.spongepowered.configurate.ConfigurationNode | ||
| import org.spongepowered.configurate.objectmapping.ConfigSerializable | ||
|
|
||
| /** | ||
| * Example config class with migrations. | ||
| * Before migration: | ||
| * ```yaml | ||
| * server: | ||
| * version: "1-20-4" | ||
| * deprecated-field: "please-remove-me" | ||
| * max-players: 0 | ||
| * ``` | ||
| */ | ||
| @ConfigSerializable | ||
| data class MyPluginConfig( | ||
| var release: String = "1.0.0", | ||
| var maxPlayers: Int = 100 | ||
| ) { | ||
| companion object : SpongeYmlConfigClass<MyPluginConfig>( | ||
| MyPluginConfig::class.java, | ||
| plugin.dataPath, | ||
| "migration-example-config.yml" | ||
| ) { | ||
| init { | ||
| migration(1, RenameServerVersionMigration) | ||
| migration(2, RemoveDeprecatedFieldMigration) | ||
| migration(3) { node -> | ||
| // inline migration: rename maxPlayers default | ||
| val mp = node.node("max-players") | ||
| if (!mp.virtual() && mp.getInt(0) == 0) { | ||
| mp.set(100) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| object RenameServerVersionMigration : ConfigMigration { | ||
| override fun migrate(node: ConfigurationNode) { | ||
| val old = node.node("server", "version") | ||
| if (!old.virtual()) { | ||
| node.node("server", "release").set(old.raw()) | ||
| old.raw(null) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| object RemoveDeprecatedFieldMigration : ConfigMigration { | ||
| override fun migrate(node: ConfigurationNode) { | ||
| node.node("deprecated-field").raw(null) | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,9 @@ | ||||||||||||
| package dev.slne.surf.surfapi.core.api.config | ||||||||||||
|
|
||||||||||||
| import dev.slne.surf.surfapi.core.api.config.manager.SpongeConfigManager | ||||||||||||
| import dev.slne.surf.surfapi.core.api.config.migration.ConfigMigration | ||||||||||||
| import dev.slne.surf.surfapi.core.api.config.migration.ConfigMigrationBuilder | ||||||||||||
| import org.spongepowered.configurate.ConfigurationNode | ||||||||||||
| import java.nio.file.Path | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
|
|
@@ -12,10 +15,36 @@ import java.nio.file.Path | |||||||||||
| * - persist changes via [save] | ||||||||||||
| * - reload the config from disk via [reloadFromFile] | ||||||||||||
| * - perform in-place mutations via [edit] | ||||||||||||
| * - register schema migrations via [migration] | ||||||||||||
| * | ||||||||||||
| * The actual manager instance is provided by subclasses and typically created by | ||||||||||||
| * a central configuration API (e.g. [surfConfigApi]). | ||||||||||||
| * | ||||||||||||
| * ## Versioned Migrations | ||||||||||||
| * | ||||||||||||
| * Migrations can be registered in the `init` block of companion objects: | ||||||||||||
| * | ||||||||||||
| * ```kotlin | ||||||||||||
| * @ConfigSerializable | ||||||||||||
| * data class MyConfig( | ||||||||||||
| * var release: String = "1.0.0" | ||||||||||||
| * ) { | ||||||||||||
| * companion object : SpongeYmlConfigClass<MyConfig>( | ||||||||||||
| * MyConfig::class.java, | ||||||||||||
| * Path("config/my-plugin"), | ||||||||||||
| * "my-config.yml" | ||||||||||||
| * ) { | ||||||||||||
| * init { | ||||||||||||
| * migration(1, RenameServerVersionMigration) | ||||||||||||
| * migration(2) { node -> | ||||||||||||
| * // inline migration | ||||||||||||
| * node.node("old-field").raw(null) | ||||||||||||
| * } | ||||||||||||
| * } | ||||||||||||
| * } | ||||||||||||
| * } | ||||||||||||
| * ``` | ||||||||||||
| * | ||||||||||||
| * @param C the type of the configuration data object. | ||||||||||||
| * @param configClass the Java class of [C], used by underlying config frameworks | ||||||||||||
| * to create and map configuration instances. | ||||||||||||
|
|
@@ -35,12 +64,63 @@ sealed class SpongeConfigClass<C>( | |||||||||||
| protected val fileName: String | ||||||||||||
| ) { | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * The migration builder that collects all registered migrations. | ||||||||||||
| * | ||||||||||||
| * Subclasses should use [migration] to register migrations rather than | ||||||||||||
| * accessing this directly. | ||||||||||||
| */ | ||||||||||||
| protected val migrationBuilder = ConfigMigrationBuilder() | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * The underlying configuration manager responsible for loading, saving, | ||||||||||||
| * and tracking the config instance of type [C]. | ||||||||||||
| */ | ||||||||||||
| abstract val manager: SpongeConfigManager<C> | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Registers a migration for the given target [version]. | ||||||||||||
| * | ||||||||||||
| * Migrations are applied in version order. For existing configs without a | ||||||||||||
| * version field, **all** registered migrations will be applied the first time. | ||||||||||||
| * | ||||||||||||
| * @param version the target version this migration upgrades to (must be >= 0) | ||||||||||||
| * @param migration the migration to apply | ||||||||||||
| */ | ||||||||||||
| protected fun migration(version: Int, migration: ConfigMigration) { | ||||||||||||
| migrationBuilder.migration(version, migration) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Registers an inline migration for the given target [version]. | ||||||||||||
| * | ||||||||||||
| * ```kotlin | ||||||||||||
| * init { | ||||||||||||
| * migration(1) { node -> | ||||||||||||
| * node.node("old-key").raw(null) | ||||||||||||
| * } | ||||||||||||
| * } | ||||||||||||
| * ``` | ||||||||||||
| * | ||||||||||||
| * @param version the target version this migration upgrades to (must be >= 0) | ||||||||||||
| * @param migration the migration lambda | ||||||||||||
| */ | ||||||||||||
| protected inline fun migration(version: Int, crossinline migration: (ConfigurationNode) -> Unit) { | ||||||||||||
| migrationBuilder.migration(version, ConfigMigration { node -> migration(node) }) | ||||||||||||
| } | ||||||||||||
|
|
||||||||||||
| /** | ||||||||||||
| * Sets the path in the config file where the version number is stored. | ||||||||||||
| * | ||||||||||||
| * Defaults to `"config-version"`. Call this in the `init` block before | ||||||||||||
| * any migration registrations if you need a custom key. | ||||||||||||
|
Comment on lines
+115
to
+116
|
||||||||||||
| * Defaults to `"config-version"`. Call this in the `init` block before | |
| * any migration registrations if you need a custom key. | |
| * By default, this delegates to [ConfigMigrationBuilder.versionKey], which uses | |
| * [ConfigMigrationBuilder.DEFAULT_VERSION_KEY] (currently `"_config_version"`) as the key. | |
| * Call this in the `init` block before any migration registrations if you need a custom key. |
Comment on lines
+175
to
+177
Copilot
AI
Mar 29, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
init() is documented as a no-op, but it now forces manager initialization (which can load/save the config and run migrations). Consider updating the KDoc to reflect the new behavior so callers understand the side effects.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The example YAML/migration uses kebab-case keys (
max-players), but Configurate object mapping will use the Kotlin property name (maxPlayers) unless annotated (e.g.,@Setting("max-players")). As written, this example/migration likely won't match the serialized keys; update the keys/migration paths or add@Settingso the demo reflects actual behavior.