Swap to using configurate for configs - #313
Conversation
There was a problem hiding this comment.
Pull request overview
This PR migrates the configuration system from Jackson to Configurate, implementing a versioned configuration approach that supports automatic migration from version 1 to version 2. The migration consolidates config structure, renames fields for consistency, and removes the need for separate platform-specific config classes.
Key Changes:
- Replaces Jackson with Configurate for YAML configuration handling
- Introduces a unified
CoreConfiginterface with nested interfaces for different config sections - Implements automatic config migration using
ConfigurationTransformation.Versionedwith version 2 transformations - Adds platform-specific config exclusion through custom
@ExcludePlatformannotation
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| gradle/libs.versions.toml | Removes Jackson dependencies, adds Configurate dependencies with version references |
| core/build.gradle.kts | Replaces Jackson bundle with Configurate dependencies (core, interface, yaml, annotation processor) |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/CoreConfig.java | New unified config interface with nested interfaces for session, friend sync, and notification settings |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/ExcludePlatform.java | Custom annotation for excluding config fields from specific platforms |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/ConfigLoader.java | Handles config loading, migration transformations, and platform-specific field exclusion |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/StandaloneConfig.java | Deleted - replaced by CoreConfig interface |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/SessionConfig.java | Deleted - replaced by CoreConfig.SessionConfig nested interface |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/NotificationConfig.java | Deleted - replaced by CoreConfig.NotificationConfig nested interface |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/FriendSyncConfig.java | Deleted - replaced by CoreConfig.FriendSyncConfig nested interface |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/configs/ExtensionConfig.java | Deleted - replaced by CoreConfig interface |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/notifications/BaseNotificationManager.java | Updates to accept CoreConfig.NotificationConfig, adds null checks for config |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/notifications/SlackNotificationManager.java | Updates constructor parameter type to CoreConfig.NotificationConfig |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/notifications/EmptyNotificationManager.java | Passes null config instead of creating empty NotificationConfig |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/SessionManager.java | Updates to use CoreConfig.FriendSyncConfig, removes manual validation (now handled by @NumericRange) |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/SessionInfo.java | Adds constructor accepting CoreConfig.SessionConfig.SessionInfo, removes Jackson annotations |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/FriendManager.java | Updates to use CoreConfig.FriendSyncConfig, accesses expiry settings through expiry() method |
| core/src/main/java/com/rtm516/mcxboxbroadcast/core/Constants.java | Adds CONFIG_VERSION constant set to 2 |
| bootstrap/standalone/build.gradle.kts | Removes Jackson bundle dependency |
| bootstrap/standalone/src/main/java/com/rtm516/mcxboxbroadcast/bootstrap/standalone/StandaloneMain.java | Replaces Jackson-based config loading with ConfigLoader.loadConfig, updates config method calls |
| bootstrap/geyser/build.gradle.kts | Replaces Jackson relocation with Configurate relocation |
| bootstrap/geyser/src/main/java/com/rtm516/mcxboxbroadcast/bootstrap/geyser/MCXboxBroadcastExtension.java | Replaces custom ConfigLoader with core ConfigLoader.loadConfig, updates config method calls |
| bootstrap/geyser/src/main/java/com/rtm516/mcxboxbroadcast/bootstrap/geyser/ConfigLoader.java | Deleted - replaced by core ConfigLoader |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @Comment("The message to send when a friend has restrictions in place that prevent them from be friend with our account") | ||
| @DefaultString(""" | ||
| %s (%s) has restrictions in place that prevent them from be friend with our account.""") |
There was a problem hiding this comment.
The default string message has the same grammatical error. It should be "from being friends" instead of "from be friend".
| @Comment("The message to send when a friend has restrictions in place that prevent them from be friend with our account") | |
| @DefaultString(""" | |
| %s (%s) has restrictions in place that prevent them from be friend with our account.""") | |
| @Comment("The message to send when a friend has restrictions in place that prevent them from being friends with our account") | |
| @DefaultString(""" | |
| %s (%s) has restrictions in place that prevent them from being friends with our account.""") |
|
|
||
| @Comment(""" | ||
| The webhook url to send the message to | ||
| If you are using discord use a add "/slack" to the end of the webhook url""") |
There was a problem hiding this comment.
There's a grammatical error in the comment. It should be "add a" or simply "add" instead of "use a add".
| If you are using discord use a add "/slack" to the end of the webhook url""") | |
| If you are using discord add "/slack" to the end of the webhook url""") |
|
|
||
| @Comment(""" | ||
| The webhook url to send the message to | ||
| If you are using discord use a add "/slack" to the end of the webhook url""") |
There was a problem hiding this comment.
The webhookUrl field doesn't have a default value annotation. This could lead to null values when the config is first created, which might cause issues if notifications are enabled but no webhook URL is provided. Consider adding a @DefaultString("") annotation to provide an empty string as the default.
| If you are using discord use a add "/slack" to the end of the webhook url""") | |
| If you are using discord use a add "/slack" to the end of the webhook url""") | |
| @DefaultString("") |
|
|
||
| protected void sendNotification(String message) { | ||
| if (!config.enabled()) { | ||
| if (config != null || !config.enabled()) { |
There was a problem hiding this comment.
The condition logic is incorrect. It should use && (AND) instead of || (OR). Currently, this will always return early because config != null will be true for valid configs, preventing notifications from being sent. The correct logic should be: return if config is null OR if config is not enabled.
| if (config != null || !config.enabled()) { | |
| if (config == null || !config.enabled()) { |
| @Comment("The message to send when a friend has restrictions in place that prevent them from be friend with our account") | ||
| @DefaultString(""" | ||
| %s (%s) has restrictions in place that prevent them from be friend with our account.""") |
There was a problem hiding this comment.
There's a grammatical error in the comment. It should be "from being friends" instead of "from be friend".
| @Comment("The message to send when a friend has restrictions in place that prevent them from be friend with our account") | |
| @DefaultString(""" | |
| %s (%s) has restrictions in place that prevent them from be friend with our account.""") | |
| @Comment("The message to send when a friend has restrictions in place that prevent them from being friends with our account") | |
| @DefaultString(""" | |
| %s (%s) has restrictions in place that prevent them from being friends with our account.""") |
| Instant lastSeen = entry.getValue(); | ||
|
|
||
| if (lastSeen.isBefore(Instant.now().minusSeconds(friendSyncConfig.expireDays() * 24 * 3600))) { | ||
| if (lastSeen.isBefore(Instant.now().minusSeconds(friendSyncConfig.expiry().days() * 24 * 3600))) { |
There was a problem hiding this comment.
Potential overflow in int multiplication before it is converted to long by use in an invocation context.
Potential overflow in int multiplication before it is converted to long by use in an invocation context.
| if (lastSeen.isBefore(Instant.now().minusSeconds(friendSyncConfig.expiry().days() * 24 * 3600))) { | |
| if (lastSeen.isBefore(Instant.now().minusSeconds(TimeUnit.DAYS.toSeconds(friendSyncConfig.expiry().days())))) { |
|
Replace with f683180 |
Moves over to a versioned configuration system using configurate for easier modification of the config in the future.
Configurations will automatically be migrated on first run of this version.
Built ontop of the
feature/native-webrtcchanges.