Add async PAPI leaderboard cache and placeholders - #5301
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds new PlaceholderAPI tokens to query mcMMO leaderboards by rank position (skill + overall/powerlevel), backed by an async, periodically refreshed in-memory snapshot cache to keep placeholder resolution fast and non-blocking.
Changes:
- Added an async
LeaderboardPlaceholderCachethat periodically refreshes top-N leaderboard snapshots and serves placeholder lookups from immutable, atomically swapped snapshots. - Registered new
%mcmmo_mctop_*:<position>%and%mcmmo_mctop_name_*:<position>%placeholders for all non-child skills plus overall aliases (overall,all,powerlevel). - Added config options for cache depth and refresh interval, plus unit/integration tests for parsing, cache behavior, and backend correctness.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderCache.java | Implements async, single-flight snapshot refresh and position lookup APIs (contains a critical scheduler callback issue). |
| src/main/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderInputParser.java | Adds shared parsing for :<position> placeholder params. |
| src/main/java/com/gmail/nossr50/placeholders/McTopNamePlaceholder.java | New placeholder to return player names for a leaderboard position. |
| src/main/java/com/gmail/nossr50/placeholders/McTopPositionPlaceholder.java | New placeholder to return values/levels for a leaderboard position. |
| src/main/java/com/gmail/nossr50/placeholders/PapiExpansion.java | Wires up the cache + registers new placeholders; adds shutdown hook. |
| src/main/java/com/gmail/nossr50/config/GeneralConfig.java | Adds config getters + validation for max tracked rank and refresh interval. |
| src/main/resources/config.yml | Documents and provides defaults for the new PlaceholderAPI leaderboard cache settings. |
| src/main/java/com/gmail/nossr50/mcMMO.java | Stores PapiExpansion reference and shuts it down/unregisters it during disable. |
| src/test/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderCacheTest.java | Unit tests for snapshot building, bounds behavior, and refresh failure semantics. |
| src/test/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderInputParserTest.java | Unit tests for position parsing behavior. |
| src/test/java/com/gmail/nossr50/placeholders/McTopPlaceholdersTest.java | Ensures name/value placeholders map to the same cached row and alias naming is correct. |
| src/test/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderFlatFileIntegrationTest.java | Integration tests against flatfile backend and refresh concurrency guard. |
| src/test/java/com/gmail/nossr50/placeholders/LeaderboardPlaceholderSqlIntegrationTest.java | Docker-tagged integration tests against MySQL/MariaDB backends. |
| Changelog.txt | Documents the new placeholders and config keys. |
| pom.xml | Bumps project version to 2.2.054-SNAPSHOT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
nossr50
force-pushed
the
papi_leaderboard_rfe
branch
4 times, most recently
from
July 8, 2026 00:37
43d911e to
dd512b2
Compare
nossr50
force-pushed
the
papi_leaderboard_rfe
branch
from
July 9, 2026 07:57
dd512b2 to
340f2e1
Compare
PapiExpansion splits incoming params on the first ':' and resolves the prefix against its token map, and every mctop placeholder depends on that seam delivering the position suffix. These tests pin the routed (empty string from an unrefreshed cache) versus unrouted (null) return contract for the value, name, and overall-alias variants, missing position segments, and case-insensitive token lookup.
Placeholder names were built with bare toLowerCase(), which follows the JVM default locale; on Turkish-locale JVMs enum names containing 'I' produce a dotless ı, so tokens like mctop_mining would register under a different spelling than documented. The case-insensitive token map happens to absorb the mismatch, but the registered names should not depend on that. All seven placeholder classes now lower-case with Locale.ENGLISH.
nossr50
force-pushed
the
papi_leaderboard_rfe
branch
from
July 9, 2026 18:11
340f2e1 to
3bec5a4
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds an async cache updated at regular configurable intervals, which allows placeholders to be used for
nleaderboard positions (default max tracked rank is 100).Some examples:
Powerlevel / Overall alias behavior:
overall,all, andpowerlevelall map to the same powerlevel leaderboard data.New config keys:
General.PlaceholderAPI.Leaderboards.Max_Tracked_Rank(default:100, clamped between 10 and 1000)General.Leaderboards.Refresh_Interval_Seconds.SQL(default:60)General.Leaderboards.Refresh_Interval_Seconds.FlatFile(default:600)Behavior notes:
Max_Tracked_Rankreturn empty.This PR addresses: #5171, #4800, and #4291
Refresh intervals
The leaderboard cache is refreshed on a timer, with a separate interval per database backend under
General.Leaderboards.Refresh_Interval_Seconds(Max_Tracked_Rankstays underPlaceholderAPI, since it caps in-memory placeholder storage).60s. SQL leaderboard queries are indexed and cheap, so they can refresh frequently. On SQL this interval only governs the placeholder cache;/mctopand/mcrankalways query live data.600s. A FlatFile rebuild scans the entire user file, which is expensive on large servers, so it refreshes far less often. Cache refreshes force a rebuild, so placeholder data is never more than one interval old; the same value throttles command-triggered rebuilds (/mctop,/mcrank), and concurrent callers race a CAS on the throttle so only one full-file scan runs at a time. Failed rebuilds don't consume the throttle window, so the next caller retries immediately instead of being stuck with stale or empty boards.60sfloor; values below 60 are treated as 60.PapiExpansionselects the interval based on the active backend.SQL leaderboard query performance
readLeaderboardsorts each leaderboard on a single column with a stable tiebreak onskills.user_id DESC, which matches the implicit PK ordering at the end of each InnoDB secondary index — the engine resolves the whole sort with a backward scan of the per-skill index and touches only the LIMIT window, with no filesort. Every leaderboard column carries a per-skill index (plustotal).The tiebreak choice, the indexes, and the join order are all required. The query pins
skillsas the driving table withSTRAIGHT_JOIN: the renamed-player filter is a range predicate on the indexedusercolumn, and on large tables MySQL 8 otherwise drives fromusersvia that index and filesorts every qualifying row, leaving the per-skill indexes unused. At 300k rows on MySQL 8 and MariaDB 10.11, indexed page-1 queries run ~1-2ms on both engines (median of 15 runs), versus roughly 40-320ms without the indexes depending on engine and cache warmth. A Docker-tagged test runsEXPLAINon the exact production query and asserts no plan step filesorts on either engine.Tie ordering: players with identical values order newest-registered-first on SQL (previously alphabetical), and
/mcrankuses the same tiebreak so rank numbers agree with/mctoppages and the placeholders. FlatFile keeps its alphabetical tie order.Ghost rows: the database keeps a row under
_INVALID_OLD_USERNAME_when a player renames. These were always meant to be hidden from/mctop, but the old filter used LIKE-style backslash escapes inside an equality comparison, which MySQL treats as literal backslashes — the filter never matched anything. The fixed filter actually excludes them, and/mcrank's rank counts now exclude them too, so ghost rows can neither appear on a page nor shift anyone's rank number.Automatic per-skill leaderboard indexes
CREATE TABLEschema.ADD_SKILL_LEADERBOARD_INDEXESupgrade that checksINFORMATION_SCHEMAwith a single grouped query and only adds the missing ones. This is portable across MySQL/MariaDB, whereasCREATE INDEX IF NOT EXISTSis not.seq_in_index = 1) — a composite index that merely contains the skill column elsewhere can't serve the leaderboardORDER BY, so it doesn't suppress creation of the dedicated index.idx_<skill>already exists but leads with a different column, mcMMO assumes it was created deliberately, logs one warning, and treats that column as handled instead of retrying the DDL on every startup.SkillTools.NON_CHILD_SKILLSplustotal, and the legacyupdateStructureADD COLUMN path indexes newly added skill columns as it creates them, so skills added by future updates are covered on both fresh and upgraded databases.FlatFile thread-safety
All FlatFile leaderboard scopes live in one immutable generation object behind a single
volatilereference, published inside the file lock right after the scan: rebuilds are serialized scan-to-publish, a slower older scan can never overwrite a newer one, readers never observe lists from two different scans, and/mcrankresolves every skill against the same generation. The rebuild throttle is anAtomicLongCAS so concurrent callers can't perform duplicate full-file scans; only successful rebuilds arm it, and leaderboards that were never successfully built bypass it entirely.Backend read contract
DatabaseManagergainsreadLeaderboardSnapshot(perScopeLimit), used by the cache: one call returns the top rows of every leaderboard scope, propagates backend read failures (instead of swallowing them into an empty result like the command-facingreadLeaderboard), and bypasses backend-level staleness. On FlatFile it forces a single rebuild and slices every scope from that one generation — which is why a cache refresh costs one file scan rather than one per skill. This is what lets the cache distinguish "database outage" from "genuinely empty leaderboard" and keep its last good snapshot.Tests
60srefresh floor, a snapshot-read-bypasses-throttle check, failed-rebuild retry behavior (both the throttled and snapshot paths), every-scope-in-one-rebuild coverage, and a concurrency regression test running forced rebuilds against concurrent page/rank reads.-Psql-tests(@Tag("docker")) and sharing one container pair per JVM: leaderboard indexes exist after a fresh install, the migration adds them on a pre-existing unindexed table and marks the upgrade complete, re-running the migration is idempotent, a composite index does not suppress the dedicated index, an index name collision is honored without wedging the migration, the legacy column-upgrade path indexes the columns it adds,EXPLAINshows no filesort on the leaderboard query,/mcrankagrees with/mctopwhen a renamed player's ghost row ties or outranks a real player, andreadLeaderboardSnapshotpropagates read failures thatreadLeaderboardswallows.