Add DPC community API integration for cross-server faction sharing#1959
Add DPC community API integration for cross-server faction sharing#1959
Conversation
- Add dpc-api config entries with safe defaults (enabled: false, login-reminder: true, etc.) - Add MfDpcApiService for async HTTP faction data sync via java.net.http.HttpClient - Add MfFactionDpcCommand with subcommands: optin, optout, reminder, shareip, discord - Register dpc subcommand in MfFactionCommand with aliases and tab completion - Add operator login reminder to PlayerJoinListener (shown when !enabled && loginReminder) - Add periodic async sync task (every 10 minutes) in MedievalFactions.kt - Add 4 bStats pie charts: dpc_api_opt_in, dpc_api_login_reminder, dpc_api_share_server_ip, dpc_api_discord_link_set - Add language strings to all 5 lang files (en_US, en_GB, de_DE, fr_FR, pt_BR) - Add DPC command documentation to COMMANDS.md - Add DPC config documentation to CONFIG.md - Add unit tests for MfFactionDpcCommand Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/e497d56e-78a1-43e7-a56e-4c7764cf07f6 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
…t, decouple DPC reminder Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/e497d56e-78a1-43e7-a56e-4c7764cf07f6 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds an opt-in integration that periodically syncs faction data to the DPC community API and exposes server-admin controls via a new /mf dpc subcommand, with associated config/docs/i18n and metrics.
Changes:
- Introduces
MfDpcApiServiceand schedules periodic async sync of faction data toPOST /api/v1/factions. - Adds
/mf dpc ...command handlers (opt-in/out + toggles) and an operator login reminder when not opted in. - Extends config, docs, bStats charts, and locale strings to support the new integration; adds command unit tests.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/kotlin/com/dansplugins/factionsystem/dpc/MfDpcApiService.kt | New service that builds JSON payloads and POSTs them asynchronously to the DPC API. |
| src/main/kotlin/com/dansplugins/factionsystem/MedievalFactions.kt | Registers new bStats charts and schedules periodic DPC sync task. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/dpc/MfFactionDpcCommand.kt | New /mf dpc subcommand implementation with config-backed toggles. |
| src/main/kotlin/com/dansplugins/factionsystem/command/faction/MfFactionCommand.kt | Routes the new dpc subcommand via aliases and tab completion. |
| src/main/kotlin/com/dansplugins/factionsystem/listener/PlayerJoinListener.kt | Adds op-only opt-in reminder message on join (when configured). |
| src/main/resources/config.yml | Adds dpc-api configuration section with defaults. |
| CONFIG.md | Documents the new dpc-api.* configuration options. |
| COMMANDS.md | Documents the new /faction dpc / /mf dpc commands and permission. |
| src/main/resources/lang/lang_en_US.properties | Adds DPC command/reminder strings. |
| src/main/resources/lang/lang_en_GB.properties | Adds DPC command/reminder strings. |
| src/main/resources/lang/lang_de_DE.properties | Adds DPC command/reminder strings. |
| src/main/resources/lang/lang_fr_FR.properties | Adds DPC command/reminder strings. |
| src/main/resources/lang/lang_pt_BR.properties | Adds DPC command/reminder strings. |
| src/test/kotlin/com/dansplugins/factionsystem/command/faction/dpc/MfFactionDpcCommandTest.kt | Adds unit tests covering the new command behavior. |
| val serverName = plugin.server.name | ||
|
|
||
| val jsonArray = JsonArray() | ||
| for (faction in factions) { | ||
| val obj = JsonObject() | ||
| obj.addProperty("name", faction.name) | ||
| obj.addProperty("serverId", serverName) |
There was a problem hiding this comment.
serverId is currently derived from plugin.server.name, which in Bukkit/Paper is the server implementation name (e.g., "CraftBukkit"/"Paper") rather than a unique identifier for this specific server. That will cause different servers to collide on the same serverId value in the DPC API payload. Use a stable per-server identifier instead (e.g., plugin.server.serverName if available, or a dedicated config value such as dpc-api.server-id).
| val serverName = plugin.server.name | |
| val jsonArray = JsonArray() | |
| for (faction in factions) { | |
| val obj = JsonObject() | |
| obj.addProperty("name", faction.name) | |
| obj.addProperty("serverId", serverName) | |
| val serverIdConfig = plugin.config.getString("dpc-api.server-id") | |
| val serverId = serverIdConfig?.takeIf { it.isNotBlank() } ?: plugin.server.serverName | |
| if (serverId.isBlank()) { | |
| plugin.logger.warning("DPC server ID is not configured and server name is blank. Skipping faction sync.") | |
| return | |
| } | |
| val jsonArray = JsonArray() | |
| for (faction in factions) { | |
| val obj = JsonObject() | |
| obj.addProperty("name", faction.name) | |
| obj.addProperty("serverId", serverId) |
|
@copilot address comments and also how do operators get an API key are we gonna have account creation in this PR? |
…ition - Replace plugin.server.name with dpc-api.server-id config for unique server identification - Add API key instructions to login reminder, CONFIG.md, and config.yml comments - Update all 5 lang files with setup instructions (visit dansplugins.com for key) Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/cf701e83-94fd-4778-87bd-fc73ae05dc90 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Addressed in 4890171:
|
|
@copilot align the solution with the latest changes in Dans-Plugins/dansplugins-dot-com#110 |
New /mf dpc subcommands: register, login, profile, generatekey, deletekey. MfDpcApiService gains account API methods (register, login, profile, API key create/delete) with JWT token storage per player. Updated all 5 locale files, COMMANDS.md, CONFIG.md, and tests (24 cases). Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/6b5b26ea-fd68-45ee-bfec-29f54a488143 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Implemented in e56dbcd. The plugin now aligns with the DPC API from dansplugins-dot-com PR #110: New
Tests expanded to 24 cases covering all new subcommands. All 5 locale files, Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
| if (shareServerIp) { | ||
| obj.addProperty("serverIp", plugin.server.ip.ifEmpty { plugin.server.port.toString() }) | ||
| } |
There was a problem hiding this comment.
When Server#getIp is empty, the code sends only the port as serverIp (e.g., 25565), which is not a valid address and is very likely not what the API expects. Build a proper host:port (and/or introduce a dedicated dpc-api.server-address config) instead of falling back to port.toString().
| val username = args[1] | ||
| val password = args[2] |
There was a problem hiding this comment.
Accepting passwords as command arguments is risky because commands are commonly logged by servers/proxies and may be visible to staff tooling. Prefer a non-logging input flow (e.g., conversation-style prompt collected from chat with cancellation/timeout, or directing users to register/login via the website), and avoid storing/echoing raw credentials anywhere.
| } else if (response.statusCode() == 401) { | ||
| onFailure("InvalidCredentials") | ||
| } else { | ||
| onFailure("HTTP ${response.statusCode()}") | ||
| } |
There was a problem hiding this comment.
The service returns internal/error-code strings like InvalidCredentials, NotLoggedIn, SessionExpired, and HTTP 500 which end up shown to players (via {0} placeholders). Map these to localized, player-friendly messages (or structured error types) and consider including a sanitized server-provided error message (e.g., JSON message) for non-2xx responses to make failures actionable.
| GateCreateAreaRestrictedBlock=O port�o que voc� est� tentando criar cont�m um bloco restrito. | ||
| CannotBreakBlockInWilderness=Voc� n�o pode quebrar blocos na natureza. | ||
| CannotPlaceBlockInWilderness=Voc� n�o pode colocar blocos na natureza. | ||
| CannotInteractBlockInWilderness=Voc� n�o pode interagir com blocos na natureza.CmdFactionAdmin=admin |
There was a problem hiding this comment.
This line appears to accidentally concatenate two properties (CannotInteractBlockInWilderness and CmdFactionAdmin), which would prevent CmdFactionAdmin from being parsed as its own key and would corrupt the wilderness message. Split it into two separate lines so both keys are correctly defined.
| CannotInteractBlockInWilderness=Voc� n�o pode interagir com blocos na natureza.CmdFactionAdmin=admin | |
| CannotInteractBlockInWilderness=Voc� n�o pode interagir com blocos na natureza. | |
| CmdFactionAdmin=admin |
| GateCreateAreaRestrictedBlock=Das Tor, das Sie zu erstellen versuchen, enth�lt einen eingeschr�nkten Block. | ||
| CannotBreakBlockInWilderness=Du kannst keine Bl�cke in der Wildnis brechen. | ||
| CannotPlaceBlockInWilderness=Du kannst keine Bl�cke in der Wildnis platzieren. | ||
| CannotInteractBlockInWilderness=Du kannst nicht mit Bl�cken in der Wildnis interagieren.CmdFactionAdmin=admin |
There was a problem hiding this comment.
Same issue as the pt_BR locale: two properties are concatenated on one line, which breaks parsing for CmdFactionAdmin and pollutes the wilderness message. Split into two separate property lines.
| CannotInteractBlockInWilderness=Du kannst nicht mit Bl�cken in der Wildnis interagieren.CmdFactionAdmin=admin | |
| CannotInteractBlockInWilderness=Du kannst nicht mit Bl�cken in der Wildnis interagieren. | |
| CmdFactionAdmin=admin |
| class MfDpcApiService(private val plugin: MedievalFactions) { | ||
|
|
||
| private val httpClient: HttpClient = HttpClient.newBuilder() | ||
| .connectTimeout(Duration.ofSeconds(10)) | ||
| .build() |
There was a problem hiding this comment.
There are thorough tests for MfFactionDpcCommand, but the new MfDpcApiService behavior is untested (token storage/clearing on 401, request/response parsing, and sync payload composition/headers). Consider adding tests by injecting an HTTP abstraction (or a configurable HttpClient) so responses can be simulated deterministically without actual network calls.
| class MfDpcApiService(private val plugin: MedievalFactions) { | |
| private val httpClient: HttpClient = HttpClient.newBuilder() | |
| .connectTimeout(Duration.ofSeconds(10)) | |
| .build() | |
| class MfDpcApiService( | |
| private val plugin: MedievalFactions, | |
| private val httpClient: HttpClient = HttpClient.newBuilder() | |
| .connectTimeout(Duration.ofSeconds(10)) | |
| .build() | |
| ) { |
|
@copilot address comments |
|
@copilot there are some failing unit tests and the docker build is running into resolve |
…match) Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/192d8dec-54ca-4deb-bca6-069282bbe893 Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Fixed both issues in cb82c3a:
Added in b655c9f. New |
| val jsonArray = JsonArray() | ||
| for (faction in factions) { | ||
| val obj = JsonObject() | ||
| obj.addProperty("name", truncate(faction.name, MAX_NAME)) | ||
| obj.addProperty("serverId", truncate(serverId, MAX_SERVER_ID)) | ||
| obj.addProperty("memberCount", maxOf(0, faction.members.size)) | ||
| obj.addProperty("description", truncate(faction.description, MAX_DESCRIPTION)) | ||
| if (serverIp != null) { | ||
| obj.addProperty("serverIp", serverIp) | ||
| } | ||
| if (discordLink != null) { | ||
| obj.addProperty("discordLink", discordLink) | ||
| } | ||
| jsonArray.add(obj) | ||
| } | ||
|
|
||
| val body = gson.toJson(jsonArray) |
There was a problem hiding this comment.
Avoid instantiating JsonArray / JsonObject directly; instead use models and Jackson for serialization
There was a problem hiding this comment.
Replaced JsonArray/JsonObject with a DpcFactionPayload data class and Gson serialization in 14c1dd8. The project uses Gson (not Jackson), so null fields in DpcFactionPayload are automatically omitted by Gson's default behavior. Required fields (name, serverId, description) are non-nullable; optional fields (serverIp, discordLink) are nullable with null default.
|
|
||
| val dpcApiService = MfDpcApiService(this) | ||
| // 12000 ticks = 10 minutes (20 ticks/sec * 60 sec * 10 min) | ||
| server.scheduler.runTaskTimerAsynchronously(this, Runnable { dpcApiService.syncFactions() }, 12000L, 12000L) |
There was a problem hiding this comment.
Make the initial delay and interval configurable via the config.yml
There was a problem hiding this comment.
Added dpc-api.sync-interval-minutes config option (default: 10, minimum: 1) in 14c1dd8. Documented in config.yml and CONFIG.md.
src/main/resources/config.yml
Outdated
| - *buttons | ||
| dpc-api: | ||
| enabled: false | ||
| url: "https://dansplugins.com" |
There was a problem hiding this comment.
I believe it is necessary to use a port here, so we should either document that or make that configurable
There was a problem hiding this comment.
Updated the config comment to document that a port can be included in the URL (e.g. "https://dansplugins.com:8080"). Also updated CONFIG.md with the same documentation. Since the url field is already a free-form URL string, operators can include any port — no additional config field needed. Done in 14c1dd8.
|
@copilot address comments |
…sync interval configurable, document URL port Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/c9c0cce8-17e1-4634-ba3e-36fbf151814a Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
… for sync interval Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/c9c0cce8-17e1-4634-ba3e-36fbf151814a Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Addressed all 3 review comments in 14c1dd8 and f67bb41:
|
docs/dpc-api-schema.asn1
Outdated
| name UTF8String (SIZE(1..64)), | ||
| serverId UTF8String (SIZE(1..64)), | ||
| memberCount INTEGER (0..MAX), | ||
| description UTF8String (SIZE(0..512)) OPTIONAL, |
There was a problem hiding this comment.
The schema marks description as OPTIONAL, but the implementation/tests always include description in the JSON payload (and DpcFactionPayload.description is non-nullable). Please align the schema with the actual payload contract (either make description required in the schema, or update the payload to omit it when empty/null).
| description UTF8String (SIZE(0..512)) OPTIONAL, | |
| description UTF8String (SIZE(0..512)), |
| val serverIp: String? = if (shareServerIp) { | ||
| val serverAddress = plugin.config.getString("dpc-api.server-address")?.takeIf { it.isNotBlank() } | ||
| if (serverAddress != null) { | ||
| truncate(serverAddress, MAX_SERVER_IP) | ||
| } else { | ||
| val ip = plugin.server.ip | ||
| val port = plugin.server.port | ||
| if (ip.isNotEmpty()) { | ||
| val addr = if (port != 25565) "$ip:$port" else ip | ||
| truncate(addr, MAX_SERVER_IP) | ||
| } else { |
There was a problem hiding this comment.
serverIp formatting/selection logic (explicit dpc-api.server-address vs auto-detected ip and conditional :port when port != 25565) isn't covered by tests. Adding test cases for (1) non-default port includes ":port" and (2) configured server-address overrides server binding would help prevent regressions.
|
@copilot address comments |
Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/7a580d18-6812-4a25-a178-f53402067f4e Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Addressed both reviewer comments in cf079db:
|
| val body = gson.toJson(payloads) | ||
| val url = apiUrl.trimEnd('/') + "/api/v1/factions" | ||
|
|
There was a problem hiding this comment.
dpc-api.url is treated as a base URL, but the code unconditionally appends /api/v1/factions. If an operator follows documentation/examples that include the endpoint path (or any base path), the request URL becomes duplicated (e.g. .../api/v1/factions/api/v1/factions). Consider either (1) documenting/enforcing that dpc-api.url must be the base host only, with validation that rejects/strips /api/v1/factions, or (2) allowing dpc-api.url to be the full endpoint and removing the hard-coded append (or appending only when missing).
| if (ip.isNotEmpty()) { | ||
| val addr = if (port != 25565) "$ip:$port" else ip | ||
| truncate(addr, MAX_SERVER_IP) | ||
| } else { | ||
| plugin.logger.warning("Server IP is empty and dpc-api.server-address is not configured. Omitting serverIp from sync payload.") | ||
| null |
There was a problem hiding this comment.
When share-server-ip is enabled and dpc-api.server-address is blank, many Bukkit servers report server.ip as an empty string (bind-all). This will log a warning every sync cycle and always omit serverIp, even though the operator likely just needs to set dpc-api.server-address. Consider reducing log spam (e.g., log once per plugin run / per enable) and/or making the warning explicitly instruct to set dpc-api.server-address for typical setups where the bound IP is empty.
| override fun onNext(item: ByteBuffer) { | ||
| body.append(StandardCharsets.UTF_8.decode(item)) | ||
| } |
There was a problem hiding this comment.
captureRequestBody() decodes each emitted ByteBuffer chunk independently via UTF_8.decode(item). If the publisher splits a multi-byte UTF-8 sequence across buffers, this can corrupt the decoded string and make tests flaky for non-ASCII payloads. Consider collecting the bytes (or using a CharsetDecoder that preserves partial sequences across chunks) and decoding once at the end.
|
@copilot You are a coding agent tasked with resolving all open comments and review threads Instructions1. Discover all comments
2. For each comment, do the following:
3. Types of issues to handle:
4. Do NOT:
After making changes
Context
|
…teBuffer decoding Agent-Logs-Url: https://github.com/Dans-Plugins/Medieval-Factions/sessions/c04624da-5bf9-4802-b321-1fd8e297e5df Co-authored-by: dmccoystephenson <21204351+dmccoystephenson@users.noreply.github.com>
Addressed all 3 open review comments in 665b3be:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Integrates Medieval Factions with the DPC community API (
POST /api/v1/factions) to allow opt-in sharing of faction data to the DPC website. Strictly opt-in, disabled by default.Config (
config.yml)dpc-apisection:enabled,url,key,server-id,login-reminder,share-server-ip,server-address,discord-link,sync-interval-minutes— all safe defaultsserver-idis a dedicated config option for unique server identification in the DPC registryserver-addressallows explicit server address (e.g.play.myserver.com:25565) for proxied setups; auto-detects from server binding when blankkeyincludes inline comment directing operators to https://dansplugins.com to register and generate an API keysync-interval-minutescontrols how often faction data is synced (default: 10 minutes); also used as initial delayurlis the full endpoint URL (default:https://dansplugins.com/api/v1/factions); supports port numbers (e.g.https://dansplugins.com:8080/api/v1/factions) and is used directly without any path appendingCommands (
MfFactionDpcCommand)All behind
mf.dpcpermission (declared inplugin.ymlwithdefault: op), wrappers around config writes:/mf dpc optin|optoutdpc-api.enabled/mf dpc reminder on|offdpc-api.login-reminder/mf dpc shareip on|offdpc-api.share-server-ip/mf dpc discord <link>|cleardpc-api.discord-linkThe
/mf dpccommand is discoverable via/mf help.HTTP Sync (
MfDpcApiService)java.net.http.HttpClient.sendAsync— no blocking the server threadHttpClientis an injectable constructor parameter (default: standard client) for testabilityrunTaskTimerAsynchronouslywith configurable interval fromdpc-api.sync-interval-minutes(default: 10 min)dpc-api.urlis the full endpoint URL used directly (no path appending); validated on each sync: must be non-blank, parseable, absolute, and use anhttporhttpsscheme; clear warnings logged on validation failureX-API-Keyheader on all requests;serverIp/discordLinkconditionally includedserverIpresolved once per sync cycle (outside the per-faction loop) fromdpc-api.server-addressconfig when set, otherwise builds properhost:portfrom server binding; omitted entirely when IP is empty; empty-IP warning logged once per plugin enable (not every cycle) with explicit guidance to setdpc-api.server-addressserverIdsourced fromdpc-api.server-idconfig (required; sync skips with warning if blank)DpcFactionPayloaddata class with Gson for JSON serialization instead of manual JsonObject/JsonArray constructionname: 64,serverId: 64,description: 512,serverIp: 253,discordLink: 512)memberCount: Included as a non-negative integer from the faction's member list sizediscordLinkvalidation: Only included if the configured value starts withhttps://discord.gg/orhttps://discord.com/; invalid values are omitted with a once-per-cycle warningOperator Login Reminder
PlayerJoinEventwhenisOp && !enabled && loginReminderdpc-api.keyanddpc-api.server-idbefore opting inoptin,shareip,discord, andreminder offcommandsAPI Key Acquisition
API keys are managed on the DPC website — operators visit dansplugins.com, create an account or sign in, then generate a key from their account page. This is documented in the config comments,
CONFIG.md, and the login reminder message. Account creation is handled by the DPC website, not this plugin.bStats
Four
SimplePiecharts:dpc_api_opt_in,dpc_api_login_reminder,dpc_api_share_server_ip,dpc_api_discord_link_setDocs & i18n
CONFIG.mdandCOMMANDS.mdupdatedCannotInteractBlockInWildernessandCmdFactionAdminwere concatenated on one line in all 5 locale filesdocs/dpc-api-schema.asn1: ASN.1 schema documenting the DPC API payload structure and field limits.github/copilot-instructions.md: Copilot instructions covering stack, build, DPC API conventions, and testing guidanceBuild & CI
build.gradle: AddedmavenLocal()as the first repository so locally-built Ponder artifacts are resolved whenrepo.dansplugins.comis unavailablebuild.yml: Added a "Build Ponder dependency" step that clonesDans-Plugins/Ponder, checks out the2.0.0tag, and runspublishToMavenLocalbefore the main buildbuild.yml: Added adocker-buildjob that runsdocker build .to verify the Dockerfile builds successfully on every push and PRrelease.yml: Same Ponder build step added beforeshadowJarDockerfile: Now self-contained — builds Ponder from source, then builds the Medieval Factions plugin (shadowJar) inside the container, eliminating the need for pre-built JARs in./build/libsDockerfile: Installs bothopenjdk-17-jdk(for Gradle toolchain compilation) andopenjdk-21-jdk(for BuildTools and MC 1.21 server runtime), withJAVA_HOMEset to JDK 17 for Gradle builds and JDK 21 for server runtimeTests
MfFactionDpcCommandTest— 12 cases covering permission denial, all subcommands, and edge casesMfDpcApiServiceTest— 9 cases using mockHttpClientto verify payload composition: correct fields for normal factions,serverIpomitted when disabled,discordLinkomitted when blank or invalid,memberCountpresent and non-negative, field truncation for oversized values, sync skipped whenserverIdis blank, non-default port includes:portin serverIp, configuredserver-addressoverrides server binding; test helper collects raw bytes and decodes once to handle multi-byte UTF-8 correctly💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.